Skip to content

fix(infra): converge legacy state migrations on archive collisions#102780

Merged
obviyus merged 1 commit into
mainfrom
fix/state-migration-convergence
Jul 9, 2026
Merged

fix(infra): converge legacy state migrations on archive collisions#102780
obviyus merged 1 commit into
mainfrom
fix/state-migration-convergence

Conversation

@obviyus

@obviyus obviyus commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #102749. Startup legacy-state migrations never converged when a .migrated archive already existed for a still-present legacy source (plugins/installs.json, tasks/runs.sqlite sidecars, plugin-state sidecars, debug-proxy captures, Codex binding sidecars). The migration imported the data but left the source in place with a warning, runDoctorConfigPreflight fails closed on any migration warning, and the prescribed remedy (openclaw doctor --fix) reproduced the same warnings — permanently locking the gateway out of that state dir even though all data was migrated successfully.

Release note: gateway startup no longer permanently refuses to start when legacy state migrations leave already-imported files behind.

Why This Change Was Made

The migration result shape conflated "migration failed" with "migration succeeded but left residue", and the startup gate blocked on both. Mixed old/new CLI versions writing the same state dir is enough to trigger the lockout.

User Impact

  • Archive collisions now converge instead of blocking forever: if the leftover source is byte-identical to its existing .migrated archive it is removed (the data was imported this run and the archive already preserves the snapshot); otherwise it is archived to the first free .migrated.N name. Both paths are recorded as doctor changes; a second run is silent.
  • Deliberate residue is reported as a new optional notices channel (mirroring runPostCorePluginConvergence) instead of warnings: e.g. Codex binding sidecars whose session owner cannot be resolved are still imported and retained, but no longer block startup.
  • The startup gate (src/commands/doctor-config-preflight.ts) now blocks on real warnings and blockers only; notices are printed under Doctor notices.
  • Real failures (rename/parse errors, canonical-state conflicts) remain blocking warnings, unchanged.

Evidence

Focused suites (state-migrations*, doctor-config-preflight.state-migration, doctor-health-contributions, extensions/codex/doctor-contract-api): all pass via node scripts/run-vitest.mjs. pnpm tsgo:core, pnpm tsgo:core:test, pnpm tsgo:extensions: exit 0. New regression tests cover identical-bytes removal, .migrated.2 fallback, second-run silence, notice-vs-warning classification, and the gate passing on notices while still throwing on warnings.

Real behavior proof on a copy of the real state dir that reproduced the lockout (legacy installs.json + installs.json.migrated, runs.sqlite sidecars + archives, 11 unresolved Codex binding sidecars):

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations extensions: codex size: L maintainer Maintainer-authored PR labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 8:54 AM ET / 12:54 UTC.

Summary
The PR makes legacy state migration archive collisions converge, adds non-blocking migration notices, and updates Codex sidecar, doctor, startup-preflight, and migration tests.

PR surface: Source +184, Tests +269. Total +453 across 12 files.

Reproducibility: yes. at source and PR-proof level: current main leaves archive collisions as warnings and startup preflight fails on any startup migration warning, while the PR body reports a real-state before/after convergence proof.

Review metrics: 3 noteworthy metrics.

  • PR surface: 12 files changed, +558/-105. The diff is bounded to legacy migrations, Codex sidecars, doctor/preflight consumers, and focused tests.
  • Plugin doctor result contract: 1 optional result field added. The new notices field extends the plugin doctor migration result surface and must stay aligned across consumers.
  • Shared archive helper coverage: 4 helper cases added. The new tests cover plain archive, identical-source removal, suffix archive, and failed archive behavior for plugin migrations.

Stored data model
Persistent data-model change detected: database schema: extensions/codex/doctor-contract-api.test.ts, migration/backfill/repair: extensions/codex/doctor-contract-api.test.ts, migration/backfill/repair: src/commands/doctor-config-preflight.state-migration.test.ts, migration/backfill/repair: src/commands/doctor-config-preflight.ts, migration/backfill/repair: src/flows/doctor-health-contributions.test.ts, migration/backfill/repair: src/flows/doctor-health-contributions.ts, and 16 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #102749
Summary: This PR is the candidate fix for the canonical gateway startup migration convergence issue; adjacent legacy-state warning and plugin-index reports overlap but do not replace it.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

  • [P2] Maintainer should explicitly accept the persisted-state notice/archive policy before merge.

Risk before merge

  • [P1] The PR changes persisted legacy-source archival/removal semantics and adds an optional plugin doctor notices channel, so maintainers need to accept that upgrade policy before merge.
  • [P2] The startup gate now treats selected imported residue as non-blocking notices; wrong classification could either keep users locked out or allow readiness with residue that should still block.

Maintainer options:

  1. Accept convergence policy and merge (recommended)
    Maintainers can accept the byte-safe archive/removal plus notices policy and land after exact-head CI and proof remain green.
  2. Request a packaged upgrade replay
    Maintainers can require one additional packaged upgrade replay on a copied affected state dir before accepting the availability risk.
  3. Pause for migration policy
    Maintainers can pause the PR if they want a broader policy for all imported-but-retained legacy residues before changing startup blocking behavior.

Next step before merge

  • [P2] Protected maintainer handling and persisted-state availability/compatibility risk need human acceptance rather than a new ClawSweeper fix PR.

Maintainer decision needed

  • Question: Should maintainers accept this persisted-state convergence policy for startup migrations now that core, Codex, and shared plugin-helper archive collisions are aligned?
  • Rationale: The code path affects upgrade-time cleanup and whether gateway startup fails closed, so automation should not decide the availability and compatibility policy by itself.
  • Likely owner: steipete — The decision crosses plugin SDK helper behavior and Codex migration ownership where steipete has the strongest feature-history signal.
  • Options:
    • Accept and land (recommended): Accept byte-identical removal, .migrated.N archival, and imported-residue notices as the shared migration policy, then merge after exact-head checks remain green.
    • Require packaged upgrade replay: Ask for one more packaged upgrade replay on an affected state copy before merge if maintainers want broader runtime proof.
    • Pause for broader policy: Hold the PR if maintainers want a single explicit migration policy covering all legacy residue before this warning downgrade ships.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes local migration cleanup and tests without new dependencies, workflows, permissions, or secret handling.

Review details

Best possible solution:

Land this unified convergence policy after maintainer acceptance and exact-head checks remain green, while keeping true import conflicts warning-blocking.

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

Yes at source and PR-proof level: current main leaves archive collisions as warnings and startup preflight fails on any startup migration warning, while the PR body reports a real-state before/after convergence proof.

Is this the best way to solve the issue?

Yes: current head applies the same convergence behavior to core and shared plugin helpers, keeps real failures as warnings, and downgrades only verified imported Codex residue to notices.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 07a7d596ff6e.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body gives before/after live output from a copied affected state dir, including first-run convergence, dashboard HTTP 200, and a silent second run; the Real behavior proof check passed.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P0: The PR targets a linked gateway startup lockout where affected users cannot recover without manual state-file deletion.
  • merge-risk: 🚨 compatibility: The diff changes persisted legacy state archival/removal behavior and adds an optional plugin doctor migration result channel.
  • merge-risk: 🚨 availability: The diff changes which startup migration outcomes block gateway readiness, so a bad classification could affect gateway startup.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster 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 gives before/after live output from a copied affected state dir, including first-run convergence, dashboard HTTP 200, and a silent second run; the Real behavior proof check passed.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body gives before/after live output from a copied affected state dir, including first-run convergence, dashboard HTTP 200, and a silent second run; the Real behavior proof check passed.
Evidence reviewed

PR surface:

Source +184, Tests +269. Total +453 across 12 files.

View PR surface stats
Area Files Added Removed Net
Source 7 289 105 +184
Tests 5 269 0 +269
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 12 558 105 +453

Acceptance criteria:

  • [P1] gh pr checks 102780 --watch=false.
  • [P1] Review exact-head migration helper and startup-preflight behavior before merge.

What I checked:

Likely related people:

  • steipete: Prior commits consolidated plugin doctor migration seams and moved Codex app-server thread bindings into SQLite plugin state, both central to this PR's helper and Codex migration changes. (role: feature-history owner; confidence: high; commits: eafe2a8d0bb6, 0d981095d430, f91de52f0d23; files: src/plugins/doctor-state-migration-fs.ts, src/plugin-sdk/runtime-doctor.ts, extensions/codex/src/migration/session-binding-sidecars.ts)
  • vincentkoc: Recent merged work touched the same state-migration archive and SQLite sidecar behavior, including the narrower plugin install-index conflict fix. (role: recent area contributor; confidence: high; commits: 5cebe9666790, 6326395c0ae4, 7485dd649262; files: src/infra/state-migrations.ts, src/infra/state-migrations.debug-proxy.ts, src/commands/doctor-state-migrations.test.ts)
  • sallyom: Recent startup migration readiness work introduced the gateway fail-closed path that blocks readiness when migration warnings remain. (role: startup readiness contributor; confidence: medium; commits: b81666ca6af2; files: src/commands/doctor-config-preflight.ts)
  • obviyus: Prior merged state-migration changes and the current PR both touch the migration/import area involved in this bug. (role: adjacent migration contributor and current PR author; confidence: medium; commits: 0d41316bcc34, 709855d623b2, a00d175b0d32; files: src/infra/state-migrations.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 (1 earlier review cycle)
  • reviewed 2026-07-09T12:21:05.105Z sha 31765e7 :: found issues before merge. :: [P1] Route archive collisions through the shared plugin helper

@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. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 9, 2026
@obviyus
obviyus force-pushed the fix/state-migration-convergence branch from 31765e7 to a00d175 Compare July 9, 2026 12:25
@obviyus

obviyus commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

[P1] Route archive collisions through the shared plugin helper — fixed on head a00d175. Correct catch: archiveLegacyStateSource in src/plugins/doctor-state-migration-fs.ts (the shared helper behind the device-pair, voice-call, memory-core, phone-control, msteams, nostr, acpx, memory-wiki, and active-memory doctor migrations) still turned archive collisions into blocking warnings. It now applies the same convergence semantics as the core helpers: identical bytes → remove the leftover source (the archive already preserves the snapshot); differing bytes → archive under the first free .migrated.N; real rename/read failures remain warnings. Either way data is never dropped — removal happens only when the archive holds byte-identical content. New focused suite src/plugins/doctor-state-migration-fs.test.ts covers all four paths (plain archive, identical-removal, suffix archive, failure-stays-warning).

On "only verified imported residue becomes non-blocking": the helper is invoked by plugin migrations after their import step, and the non-blocking conversion here is byte-safety based, not trust based — a source is deleted only when its exact bytes already exist in the archive, and otherwise it is preserved under a new archive name. The unresolved-owner Codex sidecar notice similarly fires only after registerIfAbsent imported the binding rows (see extensions/codex/src/migration/session-binding-sidecars.ts); conflict retains (canonical plugin state changed) remain blocking warnings.

Proof on the new head: node scripts/run-vitest.mjs run src/plugins/doctor-state-migration-fs.test.ts → pass; full focused set (state-migrations*, preflight, health-contributions, codex doctor-contract) → pass; pnpm tsgo:core, tsgo:core:test, tsgo:extensions → exit 0. The real-state before/after proof in the PR body is unchanged and still valid.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels Jul 9, 2026
@obviyus
obviyus merged commit 474d660 into main Jul 9, 2026
116 of 122 checks passed
@obviyus
obviyus deleted the fix/state-migration-convergence branch July 9, 2026 13:28
@shad0wca7 shad0wca7 mentioned this pull request Jul 9, 2026
Closed
@sasan1200 sasan1200 mentioned this pull request Jul 9, 2026
Closed
This was referenced Jul 13, 2026
Merged
Closed
Merged
Open
@BrunoCerberus BrunoCerberus mentioned this pull request Jul 14, 2026
Closed
This was referenced Jul 16, 2026
Closed
Closed
This was referenced Jul 17, 2026
Closed
Closed
@wangyan2026 wangyan2026 mentioned this pull request Jul 17, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Jul 17, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

commands Command implementations extensions: codex maintainer Maintainer-authored PR merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: startup legacy-state migration never converges when .migrated archive already exists; gateway permanently refuses to start

1 participant

@obviyus