fix(infra): log restart sentinel cleanup errors instead of silent catch by wendy-chsy · Pull Request #101687 · openclaw/openclaw

wendy-chsy · 2026-07-07T13:17:58Z

Summary

将 clearRestartSentinel() 和 removeLegacyRestartSentinel() 中的空 catch {} 改为记录错误
空 catch 块会静默吞噬数据库写入和文件删除的失败，导致过期 sentinel 标记遗留在数据库中
下次重启时可能错误触发意外的恢复流程

Real behavior proof

Behavior or issue addressed: 两个 try-catch 块使用空 catch 吞噬了所有异常，使开发者无法发现清理 restart sentinel 时发生的错误。

Real environment tested: 本地 Linux 环境 (Node 22.21.0, SQLite)，通过强制破坏 SQLite 数据库和文件系统结构来触发清理失败路径。

Scenario 1: SQLite 数据库损坏

[openclaw] Failed to clear restart sentinel: Error: file is not a database
    at configureSqliteWalMaintenance (.../src/infra/sqlite-wal.ts:327:6)
    at configureSqliteConnectionPragmas (.../src/infra/sqlite-wal.ts:375:23)
    at openOpenClawStateDatabase (.../src/state/openclaw-state-db.ts:935:21)
    at runOpenClawStateWriteTransaction (.../src/state/openclaw-state-db.ts:961:20)
    at clearRestartSentinel (.../src/infra/restart-sentinel.ts:215:5)
    ...
}

修复前：此异常被 catch {} 静默吞噬，无任何输出。
修复后：错误信息和堆栈完整打印到 stderr。

Scenario 2: State 目录被替换为文件（双重失败路径）

[openclaw] Failed to clear restart sentinel: Error: ENOTDIR: not a directory
[openclaw] Failed to remove legacy restart sentinel: [Error: ENOTDIR: not a directory]

修复前：两条错误均被静默吞噬。
修复后：DB 清理和旧版文件清理的失败路径均独立打印错误日志。

Exact steps or command run after this patch:
使用 tsx 脚本强制破坏 SQLite 数据库和文件系统，直接调用 clearRestartSentinel() 触发真实错误。

Evidence after fix:

27 个现有单元测试全部通过
强制触发两种不同的清理失败场景，均正确输出 [openclaw] Failed to ... 日志

What was not tested:

未在生产环境 (Gateway) 中验证错误恢复流程
console.error 的副作用本身不在单元测试中断言（但通过手动脚本确认了行为）

Tests and validation

所有 27 个现有测试通过，未改动测试文件。

验证命令:

pnpm test src/infra/restart-sentinel.test.ts

Risk checklist

User-facing behavior change? → No, 仅增加日志输出
Config/default surface change? → No
Security-sensitive change? → No
Compatibility concern? → No

AI-assisted: 使用 Claude Code 编写此修复。

clawsweeper · 2026-07-07T13:29:28Z

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 11:54 AM ET / 15:54 UTC.

Summary
The PR changes src/infra/restart-sentinel.ts so SQLite and legacy-file restart sentinel cleanup failures are logged instead of ignored.

PR surface: Source +4. Total +4 across 1 file.

Reproducibility: yes. Current main has empty catches on both cleanup paths, and the PR body includes live output from forced corrupted-database and invalid-state-path scenarios after the patch.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Risk before merge

[P1] Exact-head CI was still queued during live inspection, so final merge should wait for the current head checks or routine maintainer validation.

Maintainer options:

Decide the mitigation before merge
Land the narrow diagnostics change once exact-head checks complete, keeping restart sentinel cleanup best-effort while making cleanup failures visible.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P2] No repair job is appropriate because there is no narrow code finding; the remaining action is ordinary maintainer merge validation.

Security
Cleared: The diff only adds local stderr diagnostics and does not touch dependencies, workflows, secrets, permissions, package metadata, or code-execution surfaces.

Review details

Best possible solution:

Land the narrow diagnostics change once exact-head checks complete, keeping restart sentinel cleanup best-effort while making cleanup failures visible.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main has empty catches on both cleanup paths, and the PR body includes live output from forced corrupted-database and invalid-state-path scenarios after the patch.

Is this the best way to solve the issue?

Yes. Logging inside the existing cleanup catches is the narrowest maintainable fix; throwing would change best-effort restart cleanup semantics, and adding a new logger seam would be unnecessary churn here.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3d206140f3f0.

Label changes

Label justifications:

P3: This is a small infra diagnostics fix with limited blast radius and no config, dependency, security-boundary, or user-facing workflow change.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides copied live output showing both forced cleanup failure paths logging after the patch; private local details should stay redacted in any follow-up proof.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides copied live output showing both forced cleanup failure paths logging after the patch; private local details should stay redacted in any follow-up proof.

Evidence reviewed

PR surface:

Source +4. Total +4 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	6	2	+4
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	6	2	+4

What I checked:

Current main behavior: clearRestartSentinel() on current main deletes the SQLite restart sentinel inside a try block and then silently ignores any thrown cleanup error before continuing to legacy cleanup. (src/infra/restart-sentinel.ts:213, 3d206140f3f0)
Legacy cleanup behavior: removeLegacyRestartSentinel() on current main similarly ignores rm() failures for the legacy restart-sentinel file. (src/infra/restart-sentinel.ts:235, 3d206140f3f0)
Patch shape: The PR head only replaces those two empty catches with console.error calls and preserves the best-effort cleanup control flow. (src/infra/restart-sentinel.ts:224, c89de4e07e9a)
Caller path: Gateway restart recovery calls clearRestartSentinel() after consuming a restart sentinel, so cleanup failure logging makes stale-sentinel cleanup failures observable without changing restart delivery behavior. (src/gateway/server-restart-sentinel.ts:589, 3d206140f3f0)
Adjacent tests: The adjacent test suite covers normal sentinel import, legacy-file supersession, invalid payload cleanup, and clear behavior; it does not currently assert cleanup-error logging. (src/infra/restart-sentinel.test.ts:145, 3d206140f3f0)
Real behavior proof: The PR body includes copied after-fix live output for a corrupted SQLite database and an invalid state path, showing both new [openclaw] Failed to ... diagnostics. (c89de4e07e9a)

Likely related people:

vincentkoc: Recent history shows fix(deadcode): move restart sentinels to sqlite, which owns the SQLite cleanup path this PR changes. (role: recent area contributor; confidence: high; commits: 514b3365b54c, c6aa355b5c89; files: src/infra/restart-sentinel.ts, src/gateway/server-restart-sentinel.ts, src/infra/restart-sentinel.test.ts)
joshavant: Merged restart-continuation work touched the sentinel module, tests, gateway recovery path, and restart callers around the same invariant. (role: restart-continuation feature contributor; confidence: high; commits: 584fa3215c19; files: src/infra/restart-sentinel.ts, src/infra/restart-sentinel.test.ts, src/gateway/server-restart-sentinel.ts)
steipete: Recent gateway startup and state-runtime history includes restart sentinel startup-probe work near the affected cleanup boundary. (role: adjacent state/runtime contributor; confidence: medium; commits: 3a05c7127af6, 694ca50e9775; files: src/gateway/server-startup-post-attach.ts, src/infra/restart-sentinel.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (2 earlier review cycles)

reviewed 2026-07-07T13:33:02.275Z sha 5cf04b6 :: needs real behavior proof before merge. :: none
reviewed 2026-07-07T15:17:03.475Z sha 5cf04b6 :: needs maintainer review before merge. :: none

wendy-chsy · 2026-07-07T15:08:05Z

@clawsweeper re-review

clawsweeper · 2026-07-07T15:08:09Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

…t catch The empty catch blocks in clearRestartSentinel() and removeLegacyRestartSentinel() silently swallowed DB write and file-deletion failures, leaving stale sentinel artifacts that could trigger spurious recovery flows on next restart.

wendy-chsy · 2026-07-07T15:41:14Z

@clawsweeper re-review

clawsweeper · 2026-07-07T15:41:16Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

steipete · 2026-07-16T07:54:43Z

Superseded by #106385, landed in 47ffb49f71422502e67515bad9ff063ea95afb26.

The landed fix includes this PR's legacy cleanup diagnostic, uses the redacted restart-sentinel subsystem logger instead of raw console output, and also covers SQLite clear, read, and presence failures with regression and real-filesystem proof. Contributor credit for @wendy-chsy is preserved in the commit trailer and changelog.

Thank you for identifying the missing legacy-cleanup visibility and for the original fix.

openclaw-barnacle Bot added the size: XS label Jul 7, 2026

wendy-chsy force-pushed the fix/restart-sentinel-silent-catch branch from 5cf04b6 to c89de4e Compare July 7, 2026 15:40

clawsweeper Bot mentioned this pull request Jul 8, 2026

fix(infra): log error from restart sentinel clear instead of silent catch #102146

Closed

zhangguiping-xydt mentioned this pull request Jul 10, 2026

fix(infra): keep restart log tails UTF-16 safe #103757

Merged

steipete mentioned this pull request Jul 16, 2026

fix(restart-sentinel): report persistence cleanup failures #106385

Merged

steipete closed this Jul 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(infra): log restart sentinel cleanup errors instead of silent catch#101687

fix(infra): log restart sentinel cleanup errors instead of silent catch#101687
wendy-chsy wants to merge 1 commit into
openclaw:mainfrom
wendy-chsy:fix/restart-sentinel-silent-catch

wendy-chsy commented Jul 7, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

wendy-chsy commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

wendy-chsy commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wendy-chsy commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Real behavior proof

Tests and validation

Risk checklist

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wendy-chsy commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wendy-chsy commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wendy-chsy commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading