fix(doctor): repair ALTER-appended operator approval schema instead of wedging startup#109876
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 7:50 AM ET / 11:50 UTC. Summary PR surface: Source +46, Tests +110. Total +156 across 4 files. Reproducibility: yes. The linked issue gives an exact legacy database shape and deterministic upgrade/doctor failure, while the PR reports successful doctor and gateway recovery against the affected production database. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the exact-shape, fail-closed doctor repair after confirming that invalid pre-reference approval rows are disposable runtime state, preserving valid rows and refusing every unrecognized schema rather than broadening runtime compatibility. Do we have a high-confidence way to reproduce the issue? Yes. The linked issue gives an exact legacy database shape and deterministic upgrade/doctor failure, while the PR reports successful doctor and gateway recovery against the affected production database. Is this the best way to solve the issue? Yes. Restricting repair to exact shipped schemas and replaying only the rebuilt table’s indexes is narrower and safer than executing the entire canonical schema mid-migration; the remaining question is acceptance of the transient-row discard policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6745f9a55328. Label changesLabel justifications:
Evidence reviewedPR surface: Source +46, Tests +110. Total +156 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
Review history (1 earlier review cycle)
|
…f wedging startup
c7c7b8c to
1c31a6f
Compare
|
Merged via squash.
|
…f wedging startup (openclaw#109876)
|
Verified the manual recovery from issue #109867 on a WSL2 Linux systemd installation (npm global, upgrading 2026.7.2-beta.1 → beta.2). Reproduction: Identical to the issue report — Manual recovery performed: ALTER TABLE managed_outgoing_image_records ADD COLUMN original_media_root TEXT NOT NULL DEFAULT '';
ALTER TABLE managed_outgoing_image_records ADD COLUMN agent_id TEXT;
ALTER TABLE managed_outgoing_image_records ADD COLUMN cleanup_pending INTEGER NOT NULL DEFAULT 0 CHECK (cleanup_pending IN (0, 1));Then Note on Post-recovery gotcha: If Great fix — the fail-closed matcher accepting the ALTER-appended shape and the filtered copy for nonconforming rows are both important for real upgrade paths where the additive migration has partially run. |
What Problem This Solves
Resolves a hard startup wedge where
openclaw gatewayrefuses to start andopenclaw doctor --fixcircularly reports "has a legacy operator approval schema; run openclaw doctor --fix to migrate it" — leaving the user with no working path. Hit in production on a macOS install whose state database predates theresolution_refcolumn.Why This Change Was Made
The fail-closed matcher in
openclaw-state-db-operator-approval-migration.tsonly accepted the exact canonical-position legacy DDL. A real shipped upgrade path addedresolution_refviaALTER TABLE … ADD COLUMN(seeensureAdditiveStateColumns), producing a tail-appended nullable column. That shape failed the exact match, the repair no-oped, andassertCanonicalStateSchemaShapethrew the same message doctor tells you to fix with doctor.Three fixes, all fail-closed:
resolution_ref, each STRICT or non-STRICT. Arbitrary schema differences still refuse migration.resolution_ref TEXT NOT NULL CHECK(length = 43, charset)constraint (pre-ref rows hold NULL; non-STRICT tables can even hold BLOBs). Approvals are transient runtime state, so nonconforming rows are dropped rather than aborting the repair and re-wedging startup.Also: when the repair itself refuses or fails inside doctor, the warning no longer recursively recommends
doctor --fix.User Impact
Users on older state databases can upgrade again:
openclaw doctor --fixcompletes (verified against the affected production database: operator approvals migrated, 83 tables migrated to STRICT typing) and the gateway starts. No behavior change for canonical databases.Evidence
node scripts/run-vitest.mjs src/state/openclaw-state-db-operator-approval-migration.test.ts src/state/openclaw-state-db.test.ts— 85 tests pass (new coverage: ALTER-appended STRICT/non-STRICT repair, NULL/short/BLOBresolution_refrow filtering, fail-closed refusal of unrecognized shapes, non-circular doctor warning).pnpm check:changedgate clean (format, typechecks, lint, boundaries, database-first guards).