Skip to content

refactor: store gateway restart recovery in SQLite#110014

Merged
steipete merged 8 commits into
mainfrom
codex/sqlite-restart-sentinel
Jul 17, 2026
Merged

refactor: store gateway restart recovery in SQLite#110014
steipete merged 8 commits into
mainfrom
codex/sqlite-restart-sentinel

Conversation

@steipete

@steipete steipete commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Gateway restart recovery was still persisted as a JSON sidecar. That left OpenClaw-owned runtime state outside the canonical database and created crash windows between consuming a restart intent and durably recording its wake, continuation, or outbound delivery work.

Why This Change Was Made

This moves restart-sentinel ownership fully into the shared SQLite state database. Runtime code reads and writes only typed SQLite columns; startup and openclaw doctor --fix own the bounded legacy JSON import. Revision fences, a durable revision floor, permanent delivery receipts, and atomic delivery-attempt reservations make consumption and retry replay-safe without dual reads, dual writes, or file fallbacks. Existing schema tables are reused, so this does not bump the state schema version.

User Impact

Normal restart behavior stays the same. Upgrades preserve valid pending restart recovery, malformed transient legacy state is safely retired, and a crash during recovery no longer loses work, duplicates a completed delivery, or lets a stale consumer erase a newer restart intent. Restart notices retry in-process with a durable 45-attempt ceiling. Wake, continuation, notice, and outbound delivery ownership remain durable across replay.

Evidence

  • 870 focused tests passed across restart recovery, delivery queues, Doctor migration, managed-service handoff, and the database-first guard.
  • The final rebased retry implementation passed 166 focused gateway and outbound-queue tests, including atomic attempt reservation, ambiguous-delivery reconciliation, contention, and retry exhaustion.
  • The lightweight legacy-input detector regression passed 48/48 config-guard tests for both the retired file and interrupted Doctor claim.
  • The final internal-export cleanup passed 23/23 queue-storage tests; the result type remains local to its only implementation consumer.
  • The database-first preflight exception is restricted to the two exact retired basenames and the exact existence-check AST shape; 488/488 tooling tests and the live repository guard pass.
  • Source-blind live QA passed twice with a real Gateway child restart and one observed qa-channel wake per run (QA-RESTART-6ce38444, QA-RESTART-72274559).
  • Fresh structured autoreview of the final detector fix: clean, no accepted or actionable findings, 0.94 confidence. Independent adversarial review of the final retry state machine also found no actionable issue.
  • The earlier Blacksmith Testbox changed gate passed in 22m51s, including formatting, API/boundary guards, three tsgo lanes, core lint shards, the database-first legacy-store guard, and import-cycle checks: https://github.com/openclaw/openclaw/actions/runs/29582965875
  • Latest focused formatting, Madge import-cycle, Knip/deadcode, database-first, SDK baseline, max-lines, oxlint, and git diff --check proof is clean. Exact-head hosted CI passed for 8ccf13419f39f99ad1f30b10b977ad22e9a21970, including openclaw/ci-gate: https://github.com/openclaw/openclaw/actions/runs/29596380838/attempts/2

AI-assisted. I reviewed the design and implementation and understand the migration, queue ownership, and crash-recovery invariants.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime scripts Repository scripts size: XL maintainer Maintainer-authored PR labels Jul 17, 2026
@steipete
steipete force-pushed the codex/sqlite-restart-sentinel branch from 9a626ab to 6b29d3c Compare July 17, 2026 13:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a626abcaa

ℹ️ 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".

Comment thread src/gateway/server-restart-sentinel-notice.ts Outdated
@steipete
steipete force-pushed the codex/sqlite-restart-sentinel branch from 6b29d3c to c7714f4 Compare July 17, 2026 13:44
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 17, 2026
@clawsweeper

clawsweeper Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed July 17, 2026, 10:45 AM ET / 14:45 UTC.

Summary
Moves gateway restart recovery from a legacy JSON sidecar into shared SQLite with startup/Doctor migration, revision fencing, and permanent delivery receipts.

PR surface: Source +1319, Tests +811, Docs +12, Other +56. Total +2198 across 24 files.

Reproducibility: yes. at source level. Let startup outbound recovery complete before a restart notice fails to reproduce the retry gap; for the migration gap, leave only restart-sentinel.json and invoke a command gated by hasLegacyStateMigrationInputs().

Review metrics: 2 noteworthy metrics.

  • Legacy state cutover: 1 sidecar retired, 1 bounded importer added. Every command path that can read restart recovery must either invoke that importer or intentionally require Gateway/Doctor first.
  • State schema version: 0 bumps. The patch reuses existing tables but changes row authority, revision-floor, and permanent completion-retention semantics that require explicit upgrade proof.

Stored data model
Persistent data-model change detected: database schema: docs/refactor/database-first.md, migration/backfill/repair: docs/gateway/restart-recovery.md, migration/backfill/repair: docs/refactor/database-first.md, migration/backfill/repair: src/infra/state-migrations.restart-sentinel.test.ts, migration/backfill/repair: src/infra/state-migrations.restart-sentinel.ts, migration/backfill/repair: test/scripts/check-database-first-legacy-stores.test.ts, and 15 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • Restore or explicitly schedule restart-notice recovery after transient failure.
  • [P2] Add lone-sentinel fast-detection coverage for guarded CLI migration paths.

Risk before merge

  • [P1] A transient channel failure after startup recovery has completed can postpone the user-visible restart result until another Gateway restart or recovery event.
  • [P1] An upgrade containing only restart-sentinel.json can bypass guarded automatic migration, making status-style reads temporarily hide valid pending recovery state.
  • [P1] The cutover materially changes persisted queue retention and restart-sentinel row semantics without a schema-version bump, so focused fresh-install and upgrade-path coverage remains necessary.

Maintainer options:

  1. Repair both recovery gaps (recommended)
    Preserve the SQLite design while adding fast legacy-input detection and a bounded or explicitly scheduled retry for restart notices created after startup recovery.
  2. Accept delayed or hidden recovery
    Merge unchanged only if maintainers intentionally accept that transient notices may wait for another restart and status commands may miss a lone legacy sentinel until Gateway or Doctor runs.
  3. Pause the storage cutover
    Hold the PR if the migration and retry invariants cannot be made complete without broadening the refactor further.

Next step before merge

  • [P2] The protected maintainer-labeled implementation PR should stay with its current review owners; repair the two concrete branch defects and refresh review rather than dispatching a separate ClawSweeper fix PR.

Security
Cleared: The diff adds no dependency, workflow, permission, secret-access, or downloaded-code surface, and the legacy importer is bounded and validation-oriented.

Review findings

  • [P2] Schedule recovery after the restart notice send fails — src/gateway/server-restart-sentinel-notice.ts:84
  • [P2] Detect lone legacy restart sentinels before guarded reads — src/infra/restart-sentinel.ts:93
Review details

Best possible solution:

Keep the SQLite-only runtime and single bounded legacy importer, but add the legacy sentinel and claim suffix to the fast migration detector, explicitly schedule or perform bounded recovery after a newly failed restart notice, and cover both paths with focused upgrade and post-startup-failure tests.

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

Yes at source level. Let startup outbound recovery complete before a restart notice fails to reproduce the retry gap; for the migration gap, leave only restart-sentinel.json and invoke a command gated by hasLegacyStateMigrationInputs().

Is this the best way to solve the issue?

No, not yet. SQLite ownership and bounded migration are the correct architecture, but the implementation must preserve the existing bounded notice retry and ensure every guarded read detects the legacy source.

Full review comments:

  • [P2] Schedule recovery after the restart notice send fails — src/gateway/server-restart-sentinel-notice.ts:84
    After a transient send error this path only records failure metadata and returns. If startup outbound recovery already completed, the new row receives no in-process retry and the restart result can remain pending until another restart; preserve the prior bounded retry or explicitly trigger recovery for this queue id.
    Confidence: 0.97
  • [P2] Detect lone legacy restart sentinels before guarded reads — src/infra/restart-sentinel.ts:93
    Commands gated by hasLegacyStateMigrationInputs() can skip autoMigrateLegacyState when restart-sentinel.json is the only legacy artifact. Because this read is now SQLite-only, status-style commands report no pending state until Gateway or Doctor happens to migrate it; add the sentinel path and claim suffix to the fast detector.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2c1c5b3a97a3.

Label changes

Label justifications:

  • P2: The PR has two bounded but user-visible restart-recovery defects with limited blast radius rather than an emergency runtime outage.
  • merge-risk: 🚨 compatibility: The runtime file fallback is removed, but the fast migration-input detector can skip a valid lone legacy sentinel during upgrade.
  • merge-risk: 🚨 message-delivery: A transiently failed restart notice can remain queued without another in-process delivery attempt after startup recovery has completed.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): Two reported source-blind live Gateway child restarts each produced exactly one qa-channel wake, which proves the central after-fix happy path; the two identified failure paths still need focused regression coverage rather than new contributor proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. Two reported source-blind live Gateway child restarts each produced exactly one qa-channel wake, which proves the central after-fix happy path; the two identified failure paths still need focused regression coverage rather than new contributor proof.
Evidence reviewed

PR surface:

Source +1319, Tests +811, Docs +12, Other +56. Total +2198 across 24 files.

View PR surface stats
Area Files Added Removed Net
Source 12 1717 398 +1319
Tests 9 1019 208 +811
Docs 2 14 2 +12
Config 0 0 0 0
Generated 0 0 0 0
Other 1 61 5 +56
Total 24 2811 613 +2198

What I checked:

  • Unresolved notice retry regression: After a transient restart-notice send failure, the new helper marks the durable row failed without retrying or scheduling another in-process recovery pass; startup recovery may already have completed, leaving the user-visible notice pending until a later restart. The prior review identified this at the earlier head, and the subsequent commit is described as migration-timeout test work rather than a notice-delivery repair. (src/gateway/server-restart-sentinel-notice.ts:84, b8dc30a72a2e)
  • Missing fast migration input: The current-head review shows that hasLegacyStateMigrationInputs() does not recognize restart-sentinel.json or its claim suffix, so commands using that guard can skip migration and report no pending restart state until Gateway startup or Doctor performs the import. (src/cli/program/config-guard.ts, b8dc30a72a2e)
  • SQLite-only read exposes skipped migration: The changed read path returns only the typed SQLite sentinel and no longer imports the legacy file itself, making correct migration dispatch mandatory before status-style reads. (src/infra/restart-sentinel.ts:93, b8dc30a72a2e)
  • Existing delivery behavior: The behavior appears to date to commit 1c9f62fad3d09bbe6df7c407e3d75f163560c181, which introduced durable restart notice delivery with a bounded second attempt after transient failure. (src/gateway/server-restart-sentinel.ts:131, 1c9f62fad3d0)
  • Real behavior proof: The PR body reports two source-blind live Gateway child restarts with distinct run identifiers and exactly one observed qa-channel wake in each run; this convincingly proves the central happy path, though not either failure path. (c7714f4a9107)
  • Current main still needs the refactor: Current main still contains filesystem-backed restart-sentinel import and cleanup behavior; the PR's central SQLite cutover has not independently landed on main. (src/infra/restart-sentinel.ts:1, 2c1c5b3a97a3)

Likely related people:

  • VACInc: Commit 1c9f62fad3d0 introduced restart-session wake routing, durable notice queueing, and the bounded transient retry that the refactor must preserve. (role: introduced behavior; confidence: high; commits: 1c9f62fad3d0; files: src/gateway/server-restart-sentinel.ts, src/gateway/server-restart-sentinel.test.ts)
  • steipete: Prior merged history includes the outbound session-context refactor, and this branch owns the current SQLite restart-recovery, migration, and queue changes. (role: recent area contributor; confidence: medium; commits: a1628d89ec, 0f81165ed6f9, c7714f4a9107; files: src/gateway/server-restart-sentinel.ts, src/gateway/server-restart-sentinel-notice.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 (1 earlier review cycle)
  • reviewed 2026-07-17T13:49:26.841Z sha c7714f4 :: found issues before merge. :: [P2] Schedule recovery after the restart notice send fails

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8dc30a72a

ℹ️ 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".

Comment thread src/infra/restart-sentinel.ts
@steipete
steipete force-pushed the codex/sqlite-restart-sentinel branch from b8dc30a to acc33ba Compare July 17, 2026 16:10
@openclaw-barnacle openclaw-barnacle Bot added the cli CLI command changes label Jul 17, 2026
@steipete steipete self-assigned this Jul 17, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 513f28af35

ℹ️ 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".

Comment thread src/cli/program/config-guard.ts
@steipete
steipete merged commit 5366b6d into main Jul 17, 2026
192 of 194 checks passed
@steipete
steipete deleted the codex/sqlite-restart-sentinel branch July 17, 2026 16:43
@steipete

Copy link
Copy Markdown
Contributor Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 18, 2026
* refactor: move restart sentinel state to sqlite

* fix: satisfy restart sentinel architecture gates

* test: avoid cold startup migration timeout

* fix: bound durable restart notice retries

* test: type restart notice contention mock

* fix: detect legacy restart sentinel before reads

* fix: keep delivery attempt result internal

* fix: narrow restart sentinel preflight guard
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli CLI command changes docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. scripts Repository scripts size: XL status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant