Skip to content

fix: startup migrations block on recoverable legacy state#104529

Merged
steipete merged 1 commit into
openclaw:mainfrom
sallyom:investigate/codex-migration-loop
Jul 11, 2026
Merged

fix: startup migrations block on recoverable legacy state#104529
steipete merged 1 commit into
openclaw:mainfrom
sallyom:investigate/codex-migration-loop

Conversation

@sallyom

@sallyom sallyom commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Related: #103281

What Problem This Solves

Fixes an issue where container upgrades could remain unhealthy when startup migrations encountered recoverable legacy Codex session metadata and Memory Core index conflicts. The gateway would repeatedly refuse readiness even though the Codex sidecars had no possible session owner and the canonical Memory Core index was already authoritative.

Why This Change Was Made

Codex ownership discovery now ignores metadata-only session rows that have no transcript identity, while retaining fail-closed validation for rows that claim a session ID. Memory Core now treats only a legacy files row conflict as a derived-index convergence case: it keeps the canonical per-agent SQLite rows and archives the obsolete sidecar. Other malformed, ambiguous, vector, FTS, metadata, and reference conflicts remain untouched for review.

User Impact

Affected upgrades can complete during normal gateway startup without a separate openclaw doctor --fix container. Existing canonical session and memory data remains authoritative, and genuinely indeterminate migration state still blocks readiness instead of being discarded.

Evidence

  • Regressions reproduced the current-main blockers before the fix: a metadata-only session row caused session index ... has invalid entries, and conflicting legacy Memory Core files rows prevented archival.
  • Rebased-head focused plugin contract suites: 51 tests passed (23 Codex, 28 Memory Core) on Blacksmith Testbox tbx_01kx8tzebxmewt0db5a9msxxxn.
  • pnpm check:changed passed on Blacksmith Testbox, including extension production/test typechecks, lint, database-first storage guard, runtime-sidecar-loader guard, and import-cycle check.
  • Pinned oxfmt 0.58.0 check passed on all four changed files.
  • Fresh local autoreview reported no accepted or actionable findings.
  • Live container proof: a patch-identical image built from pre-rebase commit e05f5a1adfa upgraded the affected installation against its existing mounted volume and reached a healthy gateway startup without a separate doctor run.

AI-assisted: yes, with Codex. The author reviewed the migration behavior and confirmed the live container upgrade.

Agent Transcript

Selected redacted Codex session transcript
source: local Codex session
redaction: local paths, email, raw tool output, environment and authentication details omitted

[user]
I built and updated to main, and startup still refuses readiness:
- Left 1093 Codex binding sidecars in place because the session index has invalid entries.
- Two Memory Core legacy indexes conflict with canonical memory index rows.

[assistant]
This is a narrower follow-up to #103281. Runtime normalization accepts metadata-only session rows without `sessionId`, but the migration rejects that shape and makes ownership indeterminate for every sidecar. Memory Core's file/chunk index is derived and rebuildable, so a legacy `files` conflict can safely keep canonical per-agent rows and archive the obsolete sidecar. Genuine malformed session identities and non-`files` integrity conflicts must remain blocking.

[assistant]
The first implementation was too broad; existing integrity tests showed it would also suppress vector, FTS, metadata and schema conflicts. The fix was narrowed to `files` conflicts only.

[assistant]
Focused proof is green: Codex 23/23 and Memory Core 28/28. The full changed-file gate and fresh autoreview are also clean.

[user]
Will the fix clear the container upgrade during normal startup, or will a separate `openclaw doctor --fix` container still be required?

[assistant]
For these exact blockers, the patched image should converge automatically against the existing mounted volume: metadata-only rows cannot own Codex sidecars, canonical Memory Core rows remain authoritative, and the obsolete derived sidecars are archived. Truly malformed or ambiguous state still fails closed.

[user]
I have confirmed my container upgrade is healthy with your fixes.

@sallyom
sallyom marked this pull request as ready for review July 11, 2026 15:01
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. P1 High-priority user-facing bug, regression, or broken workflow. 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 11, 2026
@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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

Summary
The PR ignores transcript-less metadata rows during Codex sidecar ownership discovery and archives legacy Memory Core sidecars when only their derived files index conflicts with canonical per-agent rows.

PR surface: Source +19, Tests +18. Total +37 across 4 files.

Reproducibility: yes. Current-main normalization and migration source provide a high-confidence reproduction for the Codex blocker, and the Memory Core conflict path is directly exercised by focused fixtures; the PR also reports the real affected container before and after the patch.

Review metrics: 1 noteworthy metric.

  • Migration classifications changed: 2 recoverable states added. Both changes determine whether persisted legacy state is retained for review or archived automatically during startup.

Stored data model
Persistent data-model change detected: migration/backfill/repair: extensions/codex/doctor-contract-api.test.ts, unknown-data-model-change: extensions/memory-core/doctor-contract-api.test.ts, vector/embedding metadata: extensions/codex/doctor-contract-api.test.ts, vector/embedding metadata: extensions/memory-core/doctor-contract-api.test.ts, vector/embedding metadata: extensions/memory-core/doctor-contract-api.ts. Confirm migration or upgrade compatibility proof before merge.

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:

  • Refresh the branch onto current main and rerun the focused Codex/Memory Core migration suites plus pnpm check:changed on the exact head.
  • Obtain explicit maintainer acceptance of automatic startup archival for these two recoverable legacy states.

Risk before merge

  • [P1] The PR intentionally changes upgrade behavior from retaining two legacy sources for operator review to automatically archiving them after proving they are non-owning or derived; maintainers should explicitly accept that compatibility boundary.
  • [P1] The branch is behind current main, so the exact refreshed head should receive the focused and changed-file gates before merge even though no relevant base-path drift was found.

Maintainer options:

  1. Refresh and land narrow convergence (recommended)
    Refresh onto current main, rerun the focused migration suites and changed-file gate, then merge the two explicitly bounded recovery cases.
  2. Keep operator-mediated recovery
    Decline automatic startup archival and retain the separate doctor-repair requirement for these legacy states.

Next step before merge

  • [P2] The patch has no actionable correctness defect; the maintainer should decide the persisted-state upgrade boundary, refresh the behind branch, and require exact-head gates before merge.

Maintainer decision needed

  • Question: Should startup automatically archive these two proven-recoverable legacy states instead of requiring a separate doctor repair container?
  • Rationale: The implementation is technically narrow and supported, but it deliberately changes the upgrade boundary for persisted legacy state and the PR carries the protected maintainer label.
  • Likely owner: Peter Steinberger — Recent history attributes the Memory Core migration and startup archive boundary to this person, which is the more consequential persisted-state decision in the PR.
  • Options:
    • Accept narrow startup convergence (recommended): Merge after refreshing the head and rerunning focused checks, allowing only transcript-less metadata rows and rollback-safe derived files conflicts to converge automatically.
    • Require explicit doctor repair: Keep the current fail-closed behavior and require operators to run openclaw doctor --fix for these states despite their proven recoverability.

Security
Cleared: The diff adds no dependency, workflow, permission, publishing, secret-handling, or external code-execution surface, and no concrete security concern was found.

Review details

Best possible solution:

Land the narrow plugin-owned recovery rules after a current-main refresh and exact-head validation, preserving existing fail-closed paths for claimed-but-invalid Codex identities and every non-files Memory Core conflict.

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

Yes. Current-main normalization and migration source provide a high-confidence reproduction for the Codex blocker, and the Memory Core conflict path is directly exercised by focused fixtures; the PR also reports the real affected container before and after the patch.

Is this the best way to solve the issue?

Yes. The patch fixes the mismatch at the owning migration boundaries, uses explicit narrow predicates, rolls back partial Memory Core imports, and preserves every sibling state that is not proven recoverable.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P1: The current behavior can leave affected container upgrades unhealthy and unable to reach gateway readiness.
  • add merge-risk: 🚨 compatibility: Merging changes automatic upgrade treatment and archival of shipped legacy session and memory state.
  • add merge-risk: 🚨 availability: The changed migration classifications directly determine whether gateway startup proceeds or remains blocked.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The contributor reports a live patch-identical container upgrade against the affected mounted volume reaching healthy gateway startup without a separate doctor run, directly covering the changed behavior.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 contributor reports a live patch-identical container upgrade against the affected mounted volume reaching healthy gateway startup without a separate doctor run, directly covering the changed behavior.

Label justifications:

  • P1: The current behavior can leave affected container upgrades unhealthy and unable to reach gateway readiness.
  • merge-risk: 🚨 compatibility: Merging changes automatic upgrade treatment and archival of shipped legacy session and memory state.
  • merge-risk: 🚨 availability: The changed migration classifications directly determine whether gateway startup proceeds or remains blocked.
  • 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 contributor reports a live patch-identical container upgrade against the affected mounted volume reaching healthy gateway startup without a separate doctor run, directly covering the changed behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor reports a live patch-identical container upgrade against the affected mounted volume reaching healthy gateway startup without a separate doctor run, directly covering the changed behavior.
Evidence reviewed

PR surface:

Source +19, Tests +18. Total +37 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 20 1 +19
Tests 2 39 21 +18
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 59 22 +37

What I checked:

Likely related people:

  • Peter Steinberger: Git blame and file history attribute the current per-agent SQLite legacy sidecar import, rollback, retry, and archive path to recent commit d5fe0c5. (role: introduced Memory Core migration behavior; confidence: high; commits: d5fe0c5e73; files: extensions/memory-core/doctor-contract-api.ts, extensions/memory-core/doctor-contract-api.test.ts)
  • bdjben: The merged related pull request and commit history introduced the sidecar ownership classification that this PR narrows for metadata-only rows. (role: introduced Codex migration behavior; confidence: high; commits: d547621c338a, 1f3ea6faaab0; files: extensions/codex/src/migration/session-binding-sidecars.ts, extensions/codex/doctor-contract-api.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.

@steipete steipete self-assigned this Jul 11, 2026
@steipete

Copy link
Copy Markdown
Contributor

Maintainer review is complete at exact head 05375ba0c7e7cb17754a67d994c2b7bcd664ee89.

What I accepted:

  • Codex ignores only rows without a durable session id when proving binding ownership; claimed identities remain strictly validated.
  • Memory Core catches only a files-table conflict after the import savepoint rolls back, keeps canonical per-agent derived-index rows, and archives the obsolete sidecar. Metadata, chunks, vectors, FTS, references, malformed schemas, and setup failures remain retained for review.
  • Both changes stay in the owning plugin doctor migrations; no runtime fallback or compatibility reader was added.

Best-fix verdict: best. Alternatives at startup/runtime or broad conflict suppression would either preserve the readiness loop or weaken persisted-state safety. Direct official Codex contract inspection confirmed that session/thread identities are explicit UUID-backed protocol values: SessionId and ThreadId.

Proof:

  • Sanitized direct AWS Crabbox cbx_ea4bab98cf63, run run_7b536f2d401e: pnpm test extensions/codex/doctor-contract-api.test.ts extensions/memory-core/doctor-contract-api.test.ts — 23 Codex + 28 Memory Core tests passed at the exact head.
  • Fresh full-diff autoreview: clean, no accepted/actionable findings; patch correct at 0.90 confidence.
  • Hosted CI run 29157177311: exact head, successful.
  • OPENCLAW_TESTBOX=1 scripts/pr prepare-run 104529: hosted exact-head gates accepted and remote tree verified.

Known proof gap: I did not have an independent redacted copy of the reporter's mounted volume for a second source-blind before/after container replay. The PR records the contributor's live unchanged-volume upgrade reaching healthy Gateway startup, while the independent focused state matrix covers both recovery branches and their fail-closed siblings.

@steipete
steipete merged commit cdb58ca into openclaw:main Jul 11, 2026
213 of 220 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

steipete added a commit that referenced this pull request Jul 11, 2026
…faults for the stable cut (#104656)

* feat(openai): default new setups to GPT-5.6 (#103581)

* feat(openai): default fresh setup to GPT-5.6

* test(crestodian): expect GPT-5.6 Codex defaults

* test(crestodian): expect GPT-5.6 bootstrap default

(cherry picked from commit ab5d143)

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

(cherry picked from commit 474d660)

* fix(codex): prevent startup hangs after binding migration (#103281)

* fix(codex): archive imported orphan binding sidecars

* fix(codex): harden binding sidecar migration

Co-authored-by: Benjamin Badejo <[email protected]>

* chore: keep release notes out of contributor PRs

---------

Co-authored-by: Benjamin Badejo <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
(cherry picked from commit 1f3ea6f)

* fix(migrations): converge recoverable legacy state (#104529)

Signed-off-by: sallyom <[email protected]>
(cherry picked from commit cdb58ca)

* test(openai): align auth contract with provider default (#103760)

(cherry picked from commit 9b4c7c0)

* fix: clear remaining release validation blockers (#104555)

* test: align provider tool call lifecycles

* test: isolate plugin install repair migrations

* fix: accept catalog temperature compatibility

* fix: disable GPT-5.6 tool reasoning on completions

* style: avoid shadowed stream model

(cherry picked from commit 57af2bb)

* test(release): keep GPT-5.6 backport scoped

* test(release): restore doctor config type import

---------

Signed-off-by: sallyom <[email protected]>
Co-authored-by: Ayaan Zaidi <[email protected]>
Co-authored-by: Ben Badejo <[email protected]>
Co-authored-by: Benjamin Badejo <[email protected]>
Co-authored-by: Sally O'Malley <[email protected]>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions: codex extensions: memory-core Extension: memory-core 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. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants