Skip to content

fix(doctor): repair ALTER-appended operator approval schema instead of wedging startup#109876

Merged
steipete merged 1 commit into
mainfrom
fix/operator-approval-migration-alter-shape
Jul 17, 2026
Merged

fix(doctor): repair ALTER-appended operator approval schema instead of wedging startup#109876
steipete merged 1 commit into
mainfrom
fix/operator-approval-migration-alter-shape

Conversation

@steipete

Copy link
Copy Markdown
Contributor

What Problem This Solves

Resolves a hard startup wedge where openclaw gateway refuses to start and openclaw doctor --fix circularly 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 the resolution_ref column.

Why This Change Was Made

The fail-closed matcher in openclaw-state-db-operator-approval-migration.ts only accepted the exact canonical-position legacy DDL. A real shipped upgrade path added resolution_ref via ALTER TABLE … ADD COLUMN (see ensureAdditiveStateColumns), producing a tail-appended nullable column. That shape failed the exact match, the repair no-oped, and assertCanonicalStateSchemaShape threw the same message doctor tells you to fix with doctor.

Three fixes, all fail-closed:

  1. The matcher now accepts exactly four enumerated shapes: canonical-position or ALTER-appended resolution_ref, each STRICT or non-STRICT. Arbitrary schema differences still refuse migration.
  2. The copy filters rows that cannot satisfy the canonical 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.
  3. The table rebuild now replays only the operator-approval index statements instead of the entire canonical schema — executing the full schema mid-repair fails on many-versions-behind databases whose other tables still lack columns the later additive/STRICT repairs add ("no such column: agent_id").

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 --fix completes (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/BLOB resolution_ref row filtering, fail-closed refusal of unrecognized shapes, non-circular doctor warning).
  • pnpm check:changed gate clean (format, typechecks, lint, boundaries, database-first guards).
  • Live proof: doctor --fix on the wedged production DB completed and the gateway now serves (previously hard-wedged).

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jul 17, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. 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 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, 7:50 AM ET / 11:50 UTC.

Summary
The PR extends doctor’s operator-approval schema repair to exact ALTER-appended legacy shapes, filters rows incompatible with the canonical constraint, recreates only operator-approval indexes, and removes circular repair advice.

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.

  • Recognized legacy schemas: 4 exact shapes accepted. The bounded count demonstrates that the repair expands shipped upgrade coverage without becoming a general destructive schema normalizer.

Stored data model
Persistent data-model change detected: database schema: src/state/openclaw-state-db-operator-approval-migration.test.ts, database schema: src/state/openclaw-state-db.test.ts, migration/backfill/repair: src/state/openclaw-state-db.test.ts, migration/backfill/repair: src/state/openclaw-state-db.ts, serialized state: src/state/openclaw-state-db-operator-approval-migration.test.ts, serialized state: src/state/openclaw-state-db-operator-approval-migration.ts, and 4 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #109867
Summary: The linked issue is the canonical production startup-wedge report; this PR is a candidate fix for its operator-rebuild path that prematurely replayed indexes referencing not-yet-added columns.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] The migration intentionally deletes approval rows whose resolution_ref cannot satisfy the canonical constraint; although these records are described as transient and invalid after restart, that destructive retention policy needs explicit maintainer acceptance.
  • [P1] If a shipped database has a fifth semantically equivalent DDL rendering outside the four exact forms, doctor will continue to refuse repair and the affected gateway can remain unavailable; the fail-closed behavior is deliberate but upgrade-sensitive.

Maintainer options:

  1. Accept the bounded migration policy (recommended)
    Merge after confirming that dropping invalid transient approval rows is the intended upgrade behavior for the four exact shipped schemas.
  2. Preserve invalid rows outside runtime state
    Revise the repair to quarantine or report incompatible rows without retaining them in the canonical runtime table.
  3. Pause the destructive repair
    Keep fail-closed startup behavior until maintainers define a different retention contract for legacy approval rows.

Next step before merge

  • [P2] A maintainer should explicitly accept the bounded destructive migration policy and then land the current head once required exact-head checks finish; no automated code repair is identified.

Maintainer decision needed

  • Question: Should doctor automatically rebuild these four shipped legacy operator-approval schemas while dropping rows whose resolution_ref cannot satisfy the current canonical constraint?
  • Rationale: The code can establish the exact migration mechanics, but only a maintainer can accept the product-level retention policy for persisted approval rows during an upgrade repair.
  • Likely owner: steipete — Authored the production-backed migration and is best positioned to confirm the intended lifecycle of these persisted approval rows.
  • Options:
    • Approve bounded repair (recommended): Accept deletion of unrepresentable transient approval rows and merge the exact-shape repair after exact-head checks complete.
    • Require row preservation: Hold the PR and require a quarantine or reconciliation design for invalid approval rows before rebuilding the table.

Security
Cleared: The patch adds no dependency, workflow, secret, permission, or code-fetching surface, and invalid approval rows are removed rather than weakened into the canonical authorization state.

Review details

Best 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 changes

Label justifications:

  • P0: Affected upgrades leave the gateway stopped and doctor unable to provide an in-product recovery path, requiring manual SQLite intervention.
  • merge-risk: 🚨 compatibility: The PR changes destructive upgrade handling for persisted operator-approval tables and intentionally filters rows that cannot enter the canonical schema.
  • merge-risk: 🚨 availability: An incomplete or incorrect legacy-shape match would leave affected installations unable to start the gateway or complete doctor repair.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR provides after-fix proof from the affected production database: doctor completed the migrations and the previously wedged gateway served again.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR provides after-fix proof from the affected production database: doctor completed the migrations and the previously wedged gateway served again.
Evidence reviewed

PR surface:

Source +46, Tests +110. Total +156 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 54 8 +46
Tests 2 118 8 +110
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 172 16 +156

What I checked:

  • Current-main gap: At the reviewed main SHA, the migration matcher accepts the prior canonical-position schema but not the shipped ALTER-appended nullable resolution_ref shape described by the production reproduction. (src/state/openclaw-state-db-operator-approval-migration.ts:92, 6745f9a55328)
  • Bounded schema recognition: The proposed matcher derives and accepts only four enumerated legacy DDL forms: canonical-position or ALTER-appended resolution_ref, each in STRICT and non-STRICT form; unrecognized constraint or type changes remain fail-closed. (src/state/openclaw-state-db-operator-approval-migration.ts:92, 1c31a6feb3db)
  • Upgrade-safe rebuild scope: The repair recreates only the operator-approval indexes instead of replaying the whole canonical schema while other legacy tables may still lack later additive columns. (src/state/openclaw-state-db-operator-approval-migration.ts, 1c31a6feb3db)
  • Regression coverage: Tests cover ALTER-appended STRICT and non-STRICT tables, valid-row preservation, NULL/short/BLOB resolution-reference filtering, refusal of unknown schemas, and non-circular doctor warnings. (src/state/openclaw-state-db-operator-approval-migration.test.ts:24, 1c31a6feb3db)
  • Real behavior proof: The PR reports running doctor --fix against the affected production database, migrating the operator-approval table and 83 tables to STRICT typing, followed by a serving gateway; the linked issue independently supplies the deterministic failure and recovery output. (1c31a6feb3db)
  • Re-review continuity: The previous review had no findings. The force-pushed current head retains the same bounded repair intent and adds no supported new blocker in the supplied current-head diff. (1c31a6feb3db)

Likely related people:

  • steipete: Authored the current production-backed repair and its regression coverage, and is the clearest available owner for the intentional migration and row-retention policy. (role: state-migration repair author and recent area contributor; confidence: high; commits: 1c31a6feb3db; files: src/state/openclaw-state-db-operator-approval-migration.ts, src/state/openclaw-state-db-operator-approval-migration.test.ts, src/state/openclaw-state-db.ts)
  • lamkan0210: Reported the linked upgrade path, exact pre-upgrade schema, startup outage, manual recovery, and post-repair gateway proof that exercise the same migration ordering boundary. (role: production reproducer for adjacent migration failure; confidence: medium; files: src/state/openclaw-state-db.ts, src/state/openclaw-state-db-operator-approval-migration.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 (1 earlier review cycle)
  • reviewed 2026-07-17T09:47:05.868Z sha c7c7b8c :: needs maintainer review before merge. :: none

@steipete
steipete force-pushed the fix/operator-approval-migration-alter-shape branch from c7c7b8c to 1c31a6f Compare July 17, 2026 11:44
@steipete
steipete merged commit 62407b3 into main Jul 17, 2026
114 checks passed
@steipete
steipete deleted the fix/operator-approval-migration-alter-shape branch July 17, 2026 12:47
@steipete

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@vectorcontext

Copy link
Copy Markdown

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 — doctor --fix fails with no such column: agent_id, gateway crash-loops until systemd gives up, state DB stays at schema v2 with the pre-beta.2 15-column managed_outgoing_image_records shape (table empty, PRAGMA integrity_check = ok).

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 openclaw doctor --fix --non-interactive completed successfully: operator approvals migrated, 83 tables migrated to STRICT typing. PRAGMA user_version = 3.

Note on DEFAULT '': The issue report's original recovery SQL used TEXT NOT NULL without DEFAULT (worked on macOS). On the Linux/WSL2 Node-bundled SQLite, ADD COLUMN ... NOT NULL without a DEFAULT was rejected even on an empty table. Using DEFAULT '' worked across both platforms and the subsequent STRICT-table rebuild normalized the shape. PR #110197 identified this same root cause.

Post-recovery gotcha: If openclaw plugins update was run while the DB was wedged, the npm install succeeds but the CLI dies at the final state write — the plugin version is not recorded. After DB recovery + gateway restart, gateway status --deep still reports plugin drift. Re-running the same openclaw plugins update @openclaw/<pkg>@2026.7.2-beta.2 commands after recovery resolves it.

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.

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

Labels

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. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor 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