fix(restart-sentinel): report persistence cleanup failures#106385
Conversation
5a82d45 to
9be7a92
Compare
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 7:54 PM ET / 23:54 UTC. Summary PR surface: Source +10, Tests +87. Total +97 across 2 files. Reproducibility: yes. Controlled database-boundary failures on current main reproduce the silent catches, while the same paths on the PR head emit warnings and retain their fallback results. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the warning-only error visibility and deterministic regression coverage after refreshing against current main, while retaining the existing best-effort return contracts and avoiding any broader restart or database policy change. Do we have a high-confidence way to reproduce the issue? Yes. Controlled database-boundary failures on current main reproduce the silent catches, while the same paths on the PR head emit warnings and retain their fallback results. Is this the best way to solve the issue? Yes. Logging at the three ownership-boundary catches is the narrowest maintainable fix because callers intentionally depend on non-throwing best-effort behavior, and deterministic fault injection directly covers that invariant. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 345083a5ab95. Label changesLabel justifications:
Evidence reviewedPR surface: Source +10, Tests +87. Total +97 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (4 earlier review cycles)
|
9123044 to
c12170a
Compare
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
c12170a to
436fc87
Compare
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Replace empty catch blocks in clearRestartSentinel, readRestartSentinel, and hasRestartSentinel with sentinelLog.warn(...) calls so SQLite errors are visible in diagnostics while preserving best-effort fallback semantics. Tests use deterministic fault injection via vi.mock on openclaw-state-db.js with hoisted throw flags, replacing the previous non-deterministic state-dir removal approach. Each error path asserts both the warning message and the correct fallback return value.
436fc87 to
e088bb5
Compare
clawsweeper
Bot
added
rating: 🐚 platinum hermit
Co-authored-by: wendy-chsy <[email protected]>
steipete
changed the title
|
Land-ready maintainer pass complete at final head What changed:
Proof:
Earlier synthetic-merge failures were reproduced as deterministic regressions in the then-current |
|
Merged via squash.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 814eafc1cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **macOS remote node readiness:** take the main-session key from the node hello snapshot instead of opening an operator connection during node admission, preventing remote tunnel recovery from leaving Computer Use and node exec stuck in lifecycle transition. | ||
| - **Claude CLI context budgets:** honor Anthropic model and per-agent `contextTokens` limits by passing the effective limit to Claude Code's native auto-compactor and persisting the same prepared budget in OpenClaw session state. Fixes #80933. (#93198) Thanks @mushuiyu886. | ||
| - **Transcript read failures:** propagate permission and I/O failures from streaming JSONL session reads instead of treating unreadable transcripts as empty. (#106412) Thanks @zenglingbiao. | ||
| - **Restart sentinel diagnostics:** report SQLite read/write and legacy-file cleanup failures while preserving best-effort restart recovery behavior. (#106385) Thanks @zenglingbiao and @wendy-chsy. |
There was a problem hiding this comment.
This commit changes runtime code and tests, so this is not the release-only changelog generation step; AGENTS.md:304 says CHANGELOG.md is release-only and should not be edited for normal fixes because release generation owns it. Keeping this manual entry can make the generated release notes duplicate or drift from the commit-derived changelog; put the release-note context in the commit/PR body instead and let the release process generate this line.
Useful? React with 👍 / 👎.
…106385) * fix(restart-sentinel): log DB errors instead of silently swallowing them Replace empty catch blocks in clearRestartSentinel, readRestartSentinel, and hasRestartSentinel with sentinelLog.warn(...) calls so SQLite errors are visible in diagnostics while preserving best-effort fallback semantics. Tests use deterministic fault injection via vi.mock on openclaw-state-db.js with hoisted throw flags, replacing the previous non-deterministic state-dir removal approach. Each error path asserts both the warning message and the correct fallback return value. * fix(restart-sentinel): report persistence failures Co-authored-by: wendy-chsy <[email protected]> * ci: retrigger pull request checks * chore(ci): prune stale max-lines baseline * chore(ci): retest against repaired protocol baseline * chore: stabilize restart sentinel changelog entry --------- Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: wendy-chsy <[email protected]>
2 participants
What Problem This Solves
Restart sentinel persistence deliberately uses best-effort reads and cleanup during gateway recovery. SQLite failures and legacy-file removal failures were silently swallowed, leaving operators without any diagnostic signal when stale restart state could survive or pending recovery state could become unreadable.
Why This Change Was Made
clearRestartSentinel()completes,readRestartSentinel()returnsnull, andhasRestartSentinel()returnsfalseon persistence failures.restart-sentinelsubsystem logger andformatErrorMessageinstead of raw console output.restart-sentinel.jsonremoval path absorbed from fix(infra): log restart sentinel cleanup errors instead of silent catch #101687.User Impact
Operators now get actionable restart-sentinel warnings for SQLite clear/read/check failures and failed legacy cleanup. Gateway shutdown and recovery remain best effort and retain their existing fallback values.
Evidence
pnpm test src/infra/restart-sentinel.test.ts— 32 tests passed.null, has returnedfalse, and all three redacted subsystem warnings were emitted.git diff --checkpassed.