fix: pre-beta.2 state DB upgrade wedges gateway startup and doctor --fix#110197
fix: pre-beta.2 state DB upgrade wedges gateway startup and doctor --fix#110197MatthewSynthia wants to merge 1 commit into
Conversation
ensureAdditiveStateColumns added original_media_root as TEXT NOT NULL with no DEFAULT. SQLite rejects that ALTER on populated tables (and on all tables for older builds), so a pre-beta.2 state database wedged both gateway startup and doctor --fix with 'Cannot add a NOT NULL column with default value NULL'. Add DEFAULT '' plus the matching maintenance-compatibility allowlist entry; the canonical STRICT rebuild still normalizes the final shape. Closes openclaw#109867 Co-Authored-By: Claude Fable 5 <[email protected]>
|
Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 6:40 PM ET / 22:40 UTC. Summary PR surface: Source +10, Tests +85. Total +95 across 2 files. Reproducibility: yes. at source level: a pre-beta.2 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
Security Review detailsBest possible solution: Refresh the branch against current Do we have a high-confidence way to reproduce the issue? Yes, at source level: a pre-beta.2 Is this the best way to solve the issue? Yes, provisionally: a default-backed additive column plus a matching compatibility entry is the narrowest repair for SQLite's legacy-table limitation, provided it remains correct after a current-main migration-order review. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e805dbb615a8. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +10, Tests +85. Total +95 across 2 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 (3 earlier review cycles) |
Patrick-Erichsen
left a comment
There was a problem hiding this comment.
Reviewed 460b324340d715b06b4275908f4439ea41013b74.
No blocking findings.
Best-fix verdict: best fix. Both startup (ensureSchema) and Doctor repair already run additive-column migration before canonical index DDL. The actual first failure is adding original_media_root TEXT NOT NULL to the pre-beta.2 table. Supplying DEFAULT '' makes that additive migration executable, while the exact compatibility allowlist keeps the transitional definition recognized without weakening the canonical schema or normal typed writer.
Shipped-history check: v2026.7.2-beta.1 predeclared the table but had no SQLite managed-image writer; PR #108290 introduced the writer and the broken additive column in v2026.7.2-beta.2. Thus the backfill does not replace a meaningful shipped original_media_root value.
Proof:
- Repo-local autoreview in branch mode against the PR base/head: clean, 0 findings (0.94 confidence).
- Sanitized AWS Crabbox, pinned Node 24.15.0, exact SHA verified, public network/no Tailscale/no instance profile/no hydration:
src/infra/sqlite-schema-contract.test.ts19/19,src/state/openclaw-state-db.permissions.test.ts9/9,src/state/openclaw-state-db.test.ts81/81. Run: https://crabbox.openclaw.ai/portal/runs/run_09b45eb99eb4 - Current
origin/mainb440cfb15e698a12846145a1bf0bd17a9c5b317c: clean three-way merge andgit diff --checkclean.
Remaining gap: the fork's full GitHub CI workflow is action_required with zero jobs, so broad exact-head CI has not run. The available dependency/security guards and real-behavior proof are green; I did not run a packaged gateway smoke beyond the production-function migration tests above.
|
Thanks for the detailed investigation and proof. Closing because current The shipped-history boundary is decisive:
I also checked the supported runtime floors directly with the official Node binaries: Node 22.22.3, 24.15.0, and 25.9.0 (SQLite 3.51.3), plus Node 26.5.0 (SQLite 3.53.3). The new tests here manually insert that otherwise unreachable legacy row. Using So #109876 is the canonical fix for #109867; this PR is no longer needed. Thanks again for surfacing and thoroughly testing the edge case. |
Closes #109867
What Problem This Solves
Fixes an issue where users upgrading from
2026.7.2-beta.1(or any pre-beta.2 state database) would have the gateway fail to start andopenclaw doctor --fixfail during the shared-state schema migration, leaving the service stranded (reported on macOS LaunchAgent and independently reproduced on Ubuntu systemd in the issue thread).The failing step is the additive migration for
managed_outgoing_image_records: it addsoriginal_media_rootasTEXT NOT NULLwith noDEFAULT. SQLite rejects thatALTER TABLE … ADD COLUMNwhenever the table has rows — and on older SQLite builds (within OpenClaw's supported Node floor) even when it is empty — so both the startupensureSchemapath and the doctor repair path abort withCannot add a NOT NULL column with default value NULL, before the beta.2 index DDL is ever reached.Why This Change Was Made
The additive DDL becomes
original_media_root TEXT NOT NULL DEFAULT '', which is executable on every supported SQLite build regardless of table contents, plus the matchingallowedColumnDefinitionsentry in the maintenance schema-compatibility map — the same pattern already used forcurrent_conversation_bindings.target_agent_id, whose additive DDL likewise carries aDEFAULTthe canonical schema omits. The canonical STRICT-table rebuild still normalizes the final shape for databases below the current schema version; no schema-version bump is involved. The empty-string backfill only ever applies to rows written before the column existed (the table had no shipped writer then), andrecord_jsonremains the durable full record.User Impact
Upgrades from pre-beta.2 state databases now complete: the gateway starts normally and
doctor --fixrepairs the schema instead of wedging. Existing rows survive with the new columns backfilled. No behavior change for databases already on the current shape.Evidence
Focused test lanes (all green):
New regression tests build a real state DB, strip it to the pre-beta.2 15-column shape (drop the two agent indexes + three columns), insert a legacy row, then drive both the normal startup open and
repairOpenClawStateDatabaseSchema()— asserting the columns are restored, the legacy row survives, andoriginal_media_rootbackfills to''.A 5-case before/after matrix was also driven through the real production functions on pristine
origin/mainvs. this branch, scored by an external QC harness (Synthia anchor-check panel — objective pass/fail only):Note on the empty-table cases: they pass on main under Node 26's bundled SQLite (which permits the NOT NULL ALTER on empty tables), which is why the matrix's main-repro anchors are the populated-table cases; on older supported SQLite builds the empty-table path fails on main too — matching the issue's original empty-table report.
To rule out latent failure modes beyond the hand-picked cases, a seeded randomized upgrade matrix (48 scenarios, seed 109867, reproducible) was also driven through the real production functions: all 7 partial-column states a manual recovery can leave behind (any subset of
agent_id/original_media_root/cleanup_pendingmissing) × both upgrade paths, pre-STRICTuser_version=2databases, 0–25 rows including unicode/quote/2KB-string contents, plus doctor double-run idempotency:Real
doctor --fixbehavior proof (source checkout viapnpm openclaw, isolatedOPENCLAW_STATE_DIR, scratch paths redacted). Fixture: a real state database created by the production schema code, stripped to the pre-beta.2 15-columnmanaged_outgoing_image_recordsshape with one legacy row inserted — the same reconstruction the issue's reporters describe:AI-assisted: authored with Claude Code (Claude Fable 5); before/after matrix scored with the Synthia QC panel as described above. I understand what the code does and reviewed the diff by hand.