fix: startup migrations block on recoverable legacy state#104529
Conversation
Signed-off-by: sallyom <[email protected]>
|
Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 11:10 AM ET / 15:10 UTC. Summary 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.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest 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- 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 changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +19, Tests +18. Total +37 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
Maintainer review is complete at exact head What I accepted:
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:
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. |
|
Merged via squash.
|
…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]>
Signed-off-by: sallyom <[email protected]>
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
filesrow 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 --fixcontainer. Existing canonical session and memory data remains authoritative, and genuinely indeterminate migration state still blocks readiness instead of being discarded.Evidence
session index ... has invalid entries, and conflicting legacy Memory Corefilesrows prevented archival.23Codex,28Memory Core) on Blacksmith Testboxtbx_01kx8tzebxmewt0db5a9msxxxn.pnpm check:changedpassed on Blacksmith Testbox, including extension production/test typechecks, lint, database-first storage guard, runtime-sidecar-loader guard, and import-cycle check.oxfmt0.58.0 check passed on all four changed files.e05f5a1adfaupgraded 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