Skip to content

fix: pre-beta.2 state DB upgrade wedges gateway startup and doctor --fix#110197

Closed
MatthewSynthia wants to merge 1 commit into
openclaw:mainfrom
MatthewSynthia:fix/state-additive-not-null-109867
Closed

fix: pre-beta.2 state DB upgrade wedges gateway startup and doctor --fix#110197
MatthewSynthia wants to merge 1 commit into
openclaw:mainfrom
MatthewSynthia:fix/state-additive-not-null-109867

Conversation

@MatthewSynthia

@MatthewSynthia MatthewSynthia commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

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 and openclaw doctor --fix fail 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 adds original_media_root as TEXT NOT NULL with no DEFAULT. SQLite rejects that ALTER TABLE … ADD COLUMN whenever the table has rows — and on older SQLite builds (within OpenClaw's supported Node floor) even when it is empty — so both the startup ensureSchema path and the doctor repair path abort with Cannot 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 matching allowedColumnDefinitions entry in the maintenance schema-compatibility map — the same pattern already used for current_conversation_bindings.target_agent_id, whose additive DDL likewise carries a DEFAULT the 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), and record_json remains the durable full record.

User Impact

Upgrades from pre-beta.2 state databases now complete: the gateway starts normally and doctor --fix repairs 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):

pnpm test src/state/openclaw-state-db.test.ts
 Tests  80 passed | 1 skipped (81)
pnpm test src/infra/sqlite-schema-contract.test.ts src/state/openclaw-state-db.permissions.test.ts
 [test] passed 2 Vitest shards

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, and original_media_root backfills to ''.

A 5-case before/after matrix was also driven through the real production functions on pristine origin/main vs. this branch, scored by an external QC harness (Synthia anchor-check panel — objective pass/fail only):

  [PASS] main-repro/doctor-repair-legacy-with-row     main wedges with 'Cannot add a NOT NULL column with default value NULL'
  [PASS] main-repro/startup-open-legacy-with-row      main wedges with 'Cannot add a NOT NULL column with default value NULL'
  [PASS] after/doctor-repair-empty-legacy             upgrade succeeds, columns present
  [PASS] after/doctor-repair-legacy-with-row          upgrade succeeds, columns present, legacy row survives
  [PASS] after/startup-open-legacy                    upgrade succeeds, columns present
  [PASS] after/startup-open-legacy-with-row           upgrade succeeds, columns present
  [PASS] after/repair-idempotent-on-current           upgrade succeeds, columns present

Synthia QC panel: 8/8 checks passed, anchor fidelity 100.0

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_pending missing) × both upgrade paths, pre-STRICT user_version=2 databases, 0–25 rows including unicode/quote/2KB-string contents, plus doctor double-run idempotency:

fix branch:  48/48 scenarios upgrade cleanly (columns restored, rows survive, backfill '')
origin/main: 26/48 — all 22 failures are exactly the fixed class
             (original_media_root missing + populated table); no other failure mode
             surfaced, and no scenario passing on main regresses on this branch

Real doctor --fix behavior proof (source checkout via pnpm openclaw, isolated OPENCLAW_STATE_DIR, scratch paths redacted). Fixture: a real state database created by the production schema code, stripped to the pre-beta.2 15-column managed_outgoing_image_records shape with one legacy row inserted — the same reconstruction the issue's reporters describe:

# BEFORE — origin/main @ 0546e15c2e
$ openclaw doctor --fix --non-interactive
Config health-state write failed: Cannot add a NOT NULL column with default value NULL
◇ Doctor warnings ─────────────────────────────────────────────
│ - Failed migrating shared state database schema at
│   <state-dir>/state/openclaw.sqlite:
│   Error: Cannot add a NOT NULL column with default value NULL
│ Failed detecting Reef identity keys: PluginStateStoreError: ...
│ Failed detecting Reef registration state: PluginStateStoreError: ...

# AFTER — this branch, identical fixture
$ openclaw doctor --fix --non-interactive
└ Doctor complete.        # zero schema warnings in the full log

$ sqlite3 <state-dir>/state/openclaw.sqlite \
    "PRAGMA table_info(managed_outgoing_image_records);" | cut -d'|' -f2 | grep -E 'agent_id|original_media_root|cleanup_pending'
original_media_root
agent_id
cleanup_pending

$ sqlite3 <state-dir>/state/openclaw.sqlite \
    "SELECT attachment_id, quote(original_media_root), cleanup_pending FROM managed_outgoing_image_records;"
att-1|''|0

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.

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]>
@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. labels Jul 17, 2026
@clawsweeper

clawsweeper Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 6:40 PM ET / 22:40 UTC.

Summary
The PR changes the additive managed_outgoing_image_records.original_media_root migration to use TEXT NOT NULL DEFAULT '', adds the corresponding schema-compatibility allowlist entry, and tests startup and Doctor repair against populated legacy state databases.

PR surface: Source +10, Tests +85. Total +95 across 2 files.

Reproducibility: yes. at source level: a pre-beta.2 managed_outgoing_image_records table with rows reaches the additive NOT NULL column path during normal open and Doctor repair. The linked report and PR transcript provide matching real CLI failure evidence, although this review did not execute the fixture.

Review metrics: 1 noteworthy metric.

  • Persisted migration definitions: 1 additive SQLite column definition changed; 1 compatibility allowlist entry added. Both definitions must remain aligned so legacy upgrades complete without creating an unrecognized schema shape.

Stored data model
Persistent data-model change detected: database schema: src/state/openclaw-state-db.test.ts, serialized state: src/state/openclaw-state-db.test.ts, unknown-data-model-change: src/state/openclaw-state-db.test.ts. Confirm migration or upgrade compatibility proof before merge.

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:

  • Refresh against current main and rerun the focused state-schema upgrade tests.

Risk before merge

  • [P1] This alters the upgrade path for persisted SQLite state: maintainers should confirm that DEFAULT '' is acceptable for every legacy record and remains compatible with the canonical STRICT-table normalization after refreshing against current main.
  • [P1] Because this migration runs in gateway startup and openclaw doctor --fix, any conflict or changed migration ordering on current main could still affect availability for upgraded installations.

Maintainer options:

  1. Refresh and validate the upgrade path (recommended)
    Rebase or validate the clean three-way merge against current main, then rerun the focused state-schema migration tests before merge.
  2. Pause for migration-owner review
    Hold the PR if maintainers do not want an empty-string recovery value for legacy image records without a broader state-model decision.

Next step before merge

  • [P2] No mechanical repair is identified; the remaining action is a normal owner review of the refreshed persisted-state migration path.

Security
Cleared: The diff changes only state-schema migration logic and regression tests; it introduces no dependency, workflow, credential, permission, or supply-chain surface.

Review details

Best possible solution:

Refresh the branch against current main, then land the bounded default-backed additive migration only if the state-schema owner confirms that the legacy '' value is the intended canonical recovery value and focused upgrade tests still pass.

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

Yes, at source level: a pre-beta.2 managed_outgoing_image_records table with rows reaches the additive NOT NULL column path during normal open and Doctor repair. The linked report and PR transcript provide matching real CLI failure evidence, although this review did not execute the fixture.

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 changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P1: Affected beta upgrades can leave the gateway stopped and Doctor unable to repair the shared state database.
  • merge-risk: 🚨 compatibility: The patch changes how an existing persisted table is upgraded and backfills a required column for legacy rows.
  • merge-risk: 🚨 availability: The affected code executes during gateway startup and openclaw doctor --fix, so a migration regression can prevent service recovery.
  • 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 (terminal): The PR body includes a redacted before/after terminal transcript of real openclaw doctor --fix behavior on the legacy fixture, including the failing main error, successful repaired run, restored columns, and preserved legacy row.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes a redacted before/after terminal transcript of real openclaw doctor --fix behavior on the legacy fixture, including the failing main error, successful repaired run, restored columns, and preserved legacy row.
Evidence reviewed

PR surface:

Source +10, Tests +85. Total +95 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 11 1 +10
Tests 1 85 0 +85
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 96 1 +95

What I checked:

  • Production migration fix: The changed additive-column path supplies a default for original_media_root, avoiding SQLite's rejection of adding a required column without a default to a populated legacy table. (src/state/openclaw-state-db.ts:1515, 460b324340d7)
  • Schema compatibility contract: The PR adds the exact additive definition to the maintenance compatibility allowlist, keeping the repair verifier consistent with the intentional transitional SQLite definition. (src/state/openclaw-state-db.ts:102, 460b324340d7)
  • Regression coverage: The added tests construct a populated pre-beta.2 table shape and exercise both normal database open and repairOpenClawStateDatabaseSchema(), asserting restored columns and preservation of the legacy row. (src/state/openclaw-state-db.test.ts:1967, 460b324340d7)
  • Reported user impact: The linked issue documents an upgrade from 2026.7.2-beta.1 to 2026.7.2-beta.2 where the gateway stays stopped and Doctor repair fails against the legacy shared-state table shape.
  • Freshness check: The PR head is based on 72335d3d4a19d21b635e1d9e960afa24f3beec75, while the review context identifies current main as e805dbb615a801b4e45c8cacb27c4eef01ed2a52; review the migration after rebasing or merging the current three-way result. (e805dbb615a8)

Likely related people:

  • MatthewSynthia: Provided the concrete legacy-schema reproduction, production-path proof, and the focused source/test change; current-main feature-history ownership could not be established from the available read-only record. (role: current migration investigator; confidence: low; commits: 460b324340d7; files: src/state/openclaw-state-db.ts, src/state/openclaw-state-db.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 (3 earlier review cycles)
  • reviewed 2026-07-17T22:01:37.828Z sha 460b324 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-17T22:12:20.072Z sha 460b324 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-17T22:22:30.002Z sha 460b324 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. label Jul 17, 2026
@MatthewSynthia
MatthewSynthia marked this pull request as ready for review July 17, 2026 22:19
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 17, 2026
@Patrick-Erichsen Patrick-Erichsen self-assigned this Jul 18, 2026

@Patrick-Erichsen Patrick-Erichsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts 19/19, src/state/openclaw-state-db.permissions.test.ts 9/9, src/state/openclaw-state-db.test.ts 81/81. Run: https://crabbox.openclaw.ai/portal/runs/run_09b45eb99eb4
  • Current origin/main b440cfb15e698a12846145a1bf0bd17a9c5b317c: clean three-way merge and git diff --check clean.

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.

@steipete

Copy link
Copy Markdown
Contributor

Thanks for the detailed investigation and proof. Closing because current main already fixes the shipped failure, while the remaining reproducer in this PR constructs a legacy state that released OpenClaw could not create.

The shipped-history boundary is decisive:

  • v2026.7.2-beta.1 predeclared managed_outgoing_image_records, but had no SQLite writer or JSON-to-SQLite importer for it. The reachable pre-beta.2 table was therefore empty.
  • refactor(state): move managed image records to SQLite #108290 introduced the SQLite writer/importer in v2026.7.2-beta.2 together with the new typed columns.
  • fix(doctor): repair ALTER-appended operator approval schema instead of wedging startup #109876 is on current main and fixes the actual reported upgrade wedge: operator-approval repair no longer replays unrelated canonical indexes before additive state columns. Both Doctor repair and normal startup then add the managed-image columns before canonical index DDL. Current coverage in src/state/openclaw-state-db.test.ts exercises that empty pre-beta.2 shape.

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). ALTER TABLE ... ADD COLUMN ... TEXT NOT NULL succeeds on the reachable empty table in all four and fails only after a row is inserted.

The new tests here manually insert that otherwise unreachable legacy row. Using DEFAULT '' is not a complete recovery contract for such a row: runtime reads the typed original_media_root column, not record_json, and the managed-image path resolver requires an absolute media root. If we decide to support manually modified or otherwise noncanonical populated databases, that should be an explicit Doctor-owned, data-aware repair rather than a blank startup default.

So #109876 is the canonical fix for #109867; this PR is no longer needed. Thanks again for surfacing and thoroughly testing the edge case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

[Bug]: beta.2 state migration creates agent_id index before adding column, blocking gateway startup

3 participants