fix(infra): archive plugin-state sidecar when canonical rows are equal or newer (#109832) by wangyan2026 · Pull Request #110083 · openclaw/openclaw

wangyan2026 · 2026-07-17T16:38:06Z

What Problem This Solves

The startup-migration gate refuses to report the gateway ready when a legacy plugin-state/state.sqlite sidecar exists whose rows are all already present in canonical shared state. Users upgrading from v2026.6.11 crash-loop because the gate treats "canonical row is newer" (legitimate live-write progression) as a conflict, leaving the sidecar in place and blocking boot.

Why This Change Was Made

The conflict detection in migrateLegacyPluginStateSidecar compared only value_json/created_at/expires_at for byte equality. When a canonical row was written by a live gateway after the migration imported the sidecar data, the mismatch was always classified as a conflict — even though the canonical data was strictly newer and the sidecar held stale, redundant data.

Codex review (July 17) confirmed: the importer is still active on current main, and #102780 did not cover this plugin-state namespace. Best solution requires row-aware comparison that archives the sidecar when canonical data is strictly newer.

User Impact

Users upgrading from v2026.6.11 to v2026.7.1 with a legacy plugin-state/state.sqlite sidecar no longer crash-loop
A redundant sidecar is automatically renamed to .migrated on the next startup — the sidecar data is never deleted, only retired
The error message when a real conflict exists now reports that rows differ without claiming which copy is newer
openclaw doctor --fix recovery path continues to work for the remaining (genuine) conflict case

Evidence

Real environment: Linux, Node v24.15.0 (SQLite 3.51.3), branch fix/plugin-state-sidecar-created-at-109832, SHA fccbafe765

Scenario Matrix

Sidecar	Canonical	created_at	Expected	Result
`{ok:true}`	`{ok:false}`	sidecar=1000, canonical=3000	Archive (canonical newer)	✅
`{ok:true}`	`{ok:true}`	equal (1000)	Archive (values match)	✅
`{ok:true}`	`{ok:false}`	equal (1000)	Conflict (ambiguous)	✅ P1 fix
`{ok:true}`	`{ok:false}`	sidecar=3000, canonical=1000	Conflict (sidecar newer)	✅
sidecar-unique	not present	N/A	Import	✅
canonical updated, sidecar expired	expired row	canonical=3000	Archive	✅

Test output (17 plugin-state tests, all pass)

$ npx [email protected] ./node_modules/.bin/vitest run doctor-state-migrations.test.ts -t "plugin-state"

 ✓ doctor-state-migrations.test.ts (92 tests | 75 skipped)
     ✓ auto-migrates the shipped plugin-state SQLite sidecar by itself
     ✓ auto-migrates the plugin-state sidecar when custom agent dirs skip session migration
     ✓ archives the plugin-state sidecar when shared state has a newer row with different value
     ✓ imports legacy-only rows and archives when remaining conflicts expired
     ✓ archives when canonical rows are newer than sidecar rows
     ✓ keeps the sidecar when the sidecar has a newer row than canonical state
     ✓ keeps the sidecar when sidecar and canonical have equal timestamps but different values
     ✓ archives the sidecar when conflicting rows already match
     ✓ lets live sidecar rows replace expired shared plugin state during migration
     ✓ archives fully covered plugin-state imports when the namespace is full

 Test Files  1 passed (1)
      Tests  17 passed | 75 skipped (92)

Codex Review P1 Resolution (July 17)

Finding	Status	Change
`>=` silently archives divergent rows with equal timestamps	✅ Fixed	`>=` → `>` at `plugin-state.ts:102`
Missing equal-timestamp divergent-value regression test	✅ Added	`"keeps the sidecar when sidecar and canonical have equal timestamps but different values"`

Because legacyPluginStateRowsMatch already verifies value_json, created_at, expires_at for exact equality, any row reaching the timestamp comparison has already failed field equality. Using >= with equal timestamps would silently discard the only recoverable sidecar copy of a divergent value. Strict > preserves the sidecar as a conflict in that ambiguous case.

Exact-head brokered AWS upgrade/startup proof (July 17)

Provider: Crabbox brokered AWS (cbx_13cca55fabc9, public network, no Tailscale, no hydration)
Exact PR head: 73cf2ba0c914dc08eba27f293c0cddc33de0edb9
Inspectable run: run_00dcaa1254c0
Trusted bootstrap verified the remote checkout SHA and confirmed the instance IAM credentials endpoint returned 404 before executing the contributor code.

PRECHECK canonical={"ok":false} canonical_newer=true sidecar={"ok":true}
FIRST_START archive=present canonical={"ok":false} readyz=ok
SECOND_START archive=present migration_warning=absent readyz=ok
✓ src/commands/doctor-state-migrations.test.ts (92 tests)
Test Files  1 passed (1)
Tests       92 passed (92)
PROOF_COMPLETE exact_head=73cf2ba0c914dc08eba27f293c0cddc33de0edb9

The fixture created a legacy plugin-state/state.sqlite row with created_at=1000 and a different, newer canonical row. The first real Gateway startup archived the sidecar to .migrated, preserved the canonical value, and reached /readyz. After a clean shutdown, the second Gateway startup reached /readyz without a plugin-state migration warning.

What Changed

src/infra/state-migrations.plugin-state.ts (4 lines changed):

>= → >: only a strictly newer canonical created_at triggers archival
Error message: "differs from shared state without a newer canonical timestamp. First key: ${key}"

src/commands/doctor-state-migrations.test.ts (+114/-11):

Updated 2 existing tests to expect archive (canonical newer scenarios)
Added equal-timestamp divergent-value regression test
Removed stale "dropped expired" assertion no longer applicable

What Did NOT Change

migrateLegacyPluginStateSidecar callers (3 call sites in state-migrations.doctor.ts)
Archive logic (.migrated rename, WAL/shm/journal sidecar suffixes)
runLegacyMigrationPlans / other channel migration paths
Startup gate severity (warnings remain fatal — separate concern per [Bug]: v2026.7.1 startup-migration checkpoint treats warning-class retained-source findings as fatal, crash-looping the gateway #109073)
Other sidecar types (task registry, flow runs, delivery queue)

Best-fix Verdict

Best fix: Yes. Strict > is the narrowest safe change. Codex review: "Change the timestamp comparison to strict > and add equal-timestamp divergent-row coverage" — both addressed.
Refactor needed: No.

Risks and Mitigations

Highest-risk area: Equal-timestamp divergent rows could theoretically be a race where canonical was written at the same millisecond as the sidecar import. In practice, this is vanishingly rare (separate SQLite databases, separate write paths). Even if it occurs, the sidecar is retained (fail-closed), and the diagnostic points to openclaw doctor --fix.
Compatibility: Fully backward compatible. The sidecar is never deleted; it is renamed to .migrated which is recoverable. No config, env, or schema changes.
Migration: Existing .migrated archives from prior runs are detected and skipped at the fileExists(sourcePath) check.

AI Assistance 🤖

AI-assisted: Yes
AI model: deepseek-v4-flash
Human confirmed understanding of code changes: Yes

clawsweeper · 2026-07-17T16:49:00Z

Codex review: needs changes before merge. Reviewed July 17, 2026, 6:23 PM ET / 22:23 UTC.

Summary
The branch archives a legacy plugin-state SQLite sidecar when a divergent canonical row has a strictly newer creation timestamp, and adds timestamp-conflict regression coverage.

PR surface: Source +4, Tests +102. Total +106 across 2 files.

Reproducibility: yes. for the proposed regression: create a live legacy row and a divergent canonical row with a later created_at but an expired expires_at; the changed comparison archives before expiry is considered.

Review metrics: 1 noteworthy metric.

Persistent-state precedence: 1 archival precedence rule changed. The changed comparison decides whether legacy plugin state is retained or retired during upgrade, so expiry semantics must remain part of the decision.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-state-migrations.test.ts, serialized state: src/infra/state-migrations.plugin-state.ts, unknown-data-model-change: src/commands/doctor-state-migrations.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🐚 platinum hermit
Patch quality: 🦪 silver shellfish
Result: blocked by patch quality or review findings.

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

Rank-up moves:

[P2] Fix the newer-but-expired canonical case and add the focused regression test.
Re-run the same redacted startup proof with that expiry-precedence scenario.

Risk before merge

[P2] A newer-but-expired canonical row can cause archival of a still-live legacy value, leaving the runtime with expired canonical state while the recoverable live sidecar is no longer migrated.
[P1] This migration governs persisted plugin/session state during startup, so an incorrect precedence rule can trade the reported readiness recovery for stale or missing state after upgrade.

Maintainer options:

Guard archival against expired canonical rows (recommended)
Only archive a divergent sidecar for a newer canonical row when that canonical row is still live, and add coverage for live legacy versus newer expired canonical state.
Accept canonical timestamp as authoritative
Land the current rule only if maintainers explicitly intend expired newer canonical rows to supersede live legacy state during migration.
Pause the precedence change
Keep the current conflict behavior until a complete expiration-and-timestamp precedence contract is agreed and tested.

Next step before merge

[P2] A narrow code-and-test repair can preserve the existing live-over-expired migration invariant without choosing a new product direction.

Security
Cleared: The diff only changes local SQLite migration precedence and tests; it adds no dependency, workflow, secret, permission, or supply-chain surface.

Review findings

[P1] Preserve live sidecar rows when canonical state has expired — src/infra/state-migrations.plugin-state.ts:102

Review details

Best possible solution:

Make canonical-newer archival conditional on the canonical row still being live; when canonical state is expired and the legacy row is live, preserve the existing precedence behavior by importing or retaining the live legacy row, with a focused regression test.

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

Yes for the proposed regression: create a live legacy row and a divergent canonical row with a later created_at but an expired expires_at; the changed comparison archives before expiry is considered.

Is this the best way to solve the issue?

No: strict > fixes equal-timestamp divergence, but timestamp ordering alone is not sufficient when the newer canonical record is expired; the migration must account for canonical liveness too.

Full review comments:

[P1] Preserve live sidecar rows when canonical state has expired — src/infra/state-migrations.plugin-state.ts:102
The new existingCreatedAt > rowCreatedAt branch runs before any canonical-expiry check. A divergent canonical row can be newer yet expired while the legacy row is still live; this archives the sidecar even though the existing migration contract allows live sidecar state to replace expired shared state. Require the canonical row to be live before treating it as authoritative, and add that regression case.
Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4f0171b1882f.

Label changes

Label changes:

add merge-risk: 🚨 availability: This path runs in the startup readiness gate, where unresolved migration outcomes can prevent the gateway from becoming ready.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides exact-head brokered live startup output showing archival, preserved canonical data, readiness across two starts, and focused test results; it should be extended after the expiry-precedence repair with the new scenario, redacting private paths or state values if posted.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🐚 platinum hermit and patch quality is 🦪 silver shellfish.
add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body provides exact-head brokered live startup output showing archival, preserved canonical data, readiness across two starts, and focused test results; it should be extended after the expiry-precedence repair with the new scenario, redacting private paths or state values if posted.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: ⏳ waiting on author.

Label justifications:

P0: The PR targets a startup migration path reported to crash-loop the gateway and leave users unable to reach readiness.
merge-risk: 🚨 compatibility: The changed migration rule affects upgrades from legacy sidecar state and can alter what persisted value survives.
merge-risk: 🚨 session-state: Plugin-state sidecars contain persisted plugin/session entries whose precedence is changed during startup migration.
merge-risk: 🚨 availability: This path runs in the startup readiness gate, where unresolved migration outcomes can prevent the gateway from becoming ready.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🐚 platinum hermit and patch quality is 🦪 silver shellfish.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body provides exact-head brokered live startup output showing archival, preserved canonical data, readiness across two starts, and focused test results; it should be extended after the expiry-precedence repair with the new scenario, redacting private paths or state values if posted.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides exact-head brokered live startup output showing archival, preserved canonical data, readiness across two starts, and focused test results; it should be extended after the expiry-precedence repair with the new scenario, redacting private paths or state values if posted.

Evidence reviewed

PR surface:

Source +4, Tests +102. Total +106 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	6	2	+4
Tests	1	112	10	+102
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	118	12	+106

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/commands/doctor-state-migrations.test.ts.
[P1] Run a redacted gateway startup fixture with a live legacy row and a newer expired canonical row; verify the resulting canonical value and a clean second startup.

What I checked:

Changed migration precedence: The new branch archives the sidecar solely when existing.created_at > row.created_at, before checking whether the canonical row is expired; a newer-but-expired canonical row can therefore suppress a still-live legacy row. (src/infra/state-migrations.plugin-state.ts:102, 73cf2ba0c914)
Existing expiry contract: The PR’s focused test inventory includes the established case that live sidecar rows replace expired shared plugin state, but it adds no inverse case for a live sidecar against a newer expired canonical row. (src/commands/doctor-state-migrations.test.ts, 73cf2ba0c914)
Canonical issue remains open: The linked report describes the same plugin-state-sidecar readiness failure and remains the canonical tracking item; this PR is a candidate fix, not a reason to close the issue.
After-fix runtime proof: The PR body includes exact-head brokered runtime output showing a newer canonical row, .migrated archival, /readyz on first and second startup, and focused test success; that proof does not cover the newer-but-expired canonical case. (73cf2ba0c914)
Feature-history routing: The current PR head includes a migration-conflict clarification by jalehman, while the earlier merged migration-convergence work was carried by obviyus; both are relevant reviewers for the state-migration invariant. (src/infra/state-migrations.plugin-state.ts, 474d660137d0)

Likely related people:

jalehman: Authored the current PR-head clarification commit and is assigned to this PR, directly connecting them to the present plugin-sidecar conflict behavior. (role: recent area contributor and assigned reviewer; confidence: high; commits: 73cf2ba0c914; files: src/infra/state-migrations.plugin-state.ts, src/commands/doctor-state-migrations.test.ts)
obviyus: Authored the merged archive-collision convergence work that established the surrounding startup-migration and retained-source behavior. (role: adjacent migration-convergence contributor; confidence: medium; commits: 474d660137d0; files: src/infra/state-migrations.plugin-state.ts, src/commands/doctor-state-migrations.test.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 (6 earlier review cycles)

reviewed 2026-07-17T16:45:11.171Z sha 92323fe :: needs real behavior proof before merge. :: [P1] Keep equal-timestamp divergent rows as conflicts
reviewed 2026-07-17T17:00:06.183Z sha 468f727 :: needs real behavior proof before merge. :: none
reviewed 2026-07-17T17:35:19.523Z sha fccbafe :: needs real behavior proof before merge. :: [P2] Describe equal-timestamp conflicts accurately
reviewed 2026-07-17T18:15:39.759Z sha fccbafe :: needs real behavior proof before merge. :: [P2] Describe equal-timestamp conflicts without a false ordering
reviewed 2026-07-17T21:58:25.261Z sha 73cf2ba :: needs real behavior proof before merge. :: none
reviewed 2026-07-17T22:03:08.724Z sha 73cf2ba :: needs real behavior proof before merge. :: none

… strictly newer (openclaw#109832) Conflict detection in migrateLegacyPluginStateSidecar compared rows for byte equality but treated every mismatch as a conflict. When canonical data was written by a live gateway after migration, the sidecar was retained as "conflicted", blocking startup readiness. Changes: - Use strict > for canonical timestamp comparison: only a strictly newer canonical row triggers archival. Equal-timestamp divergent rows remain as conflicts (Codex review P1 fix). - Update error message: "have different values; sidecar data is newer" instead of misleading "already existed in shared state" - 17 plugin-state migration tests covering all timestamp scenarios Related to openclaw#109832

jalehman · 2026-07-17T21:58:28Z

@clawsweeper re-review

clawsweeper · 2026-07-17T21:58:30Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The item is closed; no stale verdict was published.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29619651520
Updated: 2026-07-17T23:05:40.176Z

jalehman · 2026-07-17T22:19:30Z

@clawsweeper re-review

jalehman · 2026-07-17T22:20:18Z

@clawsweeper re-review

jalehman · 2026-07-17T22:53:30Z

The newer-but-expired canonical-row finding is not reachable in the current migration ordering.

Before existing is selected, the transaction deletes the matching canonical row when expires_at <= now (src/infra/state-migrations.plugin-state.ts:78-87). The timestamp comparison at line 102 therefore only evaluates a still-live canonical row.

The existing test lets live sidecar rows replace expired shared plugin state during migration covers the reported ordering: the sidecar fixture has created_at=1000; the canonical fixture omits createdAt, so the seed helper assigns Date.now() (strictly newer), and sets expiresAt=1. The test verifies the live sidecar value is imported and remains readable.

No runtime change is needed for this finding. Exact-head brokered AWS proof and the 92-test migration suite remain green.

jalehman · 2026-07-17T22:55:21Z

Merged via squash.

Prepared head SHA: 73cf2ba0c914dc08eba27f293c0cddc33de0edb9
Landed commit: ccaaa42f5265ab653ee8f562a38eebd55b3acea2

…l or newer (openclaw#109832) (openclaw#110083) * [AI] fix(infra): archive plugin-state sidecar when canonical rows are strictly newer (openclaw#109832) Conflict detection in migrateLegacyPluginStateSidecar compared rows for byte equality but treated every mismatch as a conflict. When canonical data was written by a live gateway after migration, the sidecar was retained as "conflicted", blocking startup readiness. Changes: - Use strict > for canonical timestamp comparison: only a strictly newer canonical row triggers archival. Equal-timestamp divergent rows remain as conflicts (Codex review P1 fix). - Update error message: "have different values; sidecar data is newer" instead of misleading "already existed in shared state" - 17 plugin-state migration tests covering all timestamp scenarios Related to openclaw#109832 * fix(state): clarify plugin sidecar conflicts --------- Co-authored-by: Josh Lehman <[email protected]>

openclaw-barnacle Bot added commands Command implementations size: S labels Jul 17, 2026

wangyan2026 force-pushed the fix/plugin-state-sidecar-created-at-109832 branch from 468f727 to fccbafe Compare July 17, 2026 17:29

jalehman self-assigned this Jul 17, 2026

fix(state): clarify plugin sidecar conflicts

73cf2ba

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 17, 2026

openclaw-barnacle Bot added triage: blank-template Candidate: PR template appears mostly untouched. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 17, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 17, 2026

jalehman merged commit ccaaa42 into openclaw:main Jul 17, 2026
190 of 203 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(infra): archive plugin-state sidecar when canonical rows are equal or newer (#109832)#110083

fix(infra): archive plugin-state sidecar when canonical rows are equal or newer (#109832)#110083
jalehman merged 2 commits into
openclaw:mainfrom
wangyan2026:fix/plugin-state-sidecar-created-at-109832

wangyan2026 commented Jul 17, 2026 •

edited by jalehman

Loading

Uh oh!

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wangyan2026 commented Jul 17, 2026 • edited by jalehman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Scenario Matrix

Test output (17 plugin-state tests, all pass)

Codex Review P1 Resolution (July 17)

Exact-head brokered AWS upgrade/startup proof (July 17)

What Changed

What Did NOT Change

Best-fix Verdict

Risks and Mitigations

AI Assistance 🤖

Uh oh!

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

Uh oh!

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

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

Uh oh!

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

Uh oh!

jalehman commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wangyan2026 commented Jul 17, 2026 •

edited by jalehman

Loading

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading

clawsweeper Bot commented Jul 17, 2026 •

edited

Loading