Fix container image upgrade migrations before gateway readiness#101881
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 1:10 PM ET / 17:10 UTC. Summary PR surface: Source +392, Tests +427, Docs +68. Total +887 across 13 files. Reproducibility: yes. The linked issue includes a live official-image 2026.6.10 to 2026.6.11 container swap, and current main's Docker command enters the gateway path without a versioned startup checkpoint; I did not rerun the container flow in this read-only review. Review metrics: 3 noteworthy metrics.
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. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Merge only after a maintainer accepts the converge-or-fail-closed foreground gateway startup policy and exact-head CI/proof remain green for the startup, shared-state, plugin convergence, and docs surfaces. Do we have a high-confidence way to reproduce the issue? Yes. The linked issue includes a live official-image 2026.6.10 to 2026.6.11 container swap, and current main's Docker command enters the gateway path without a versioned startup checkpoint; I did not rerun the container flow in this read-only review. Is this the best way to solve the issue? Yes, if maintainers accept the startup policy. The patch uses the existing foreground gateway preflight boundary, reuses post-core convergence, and records a shared-state checkpoint; a state-only gate would leave the plugin convergence failure mode unresolved. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8fa83d27420b. Label changesLabel justifications:
Evidence reviewedPR surface: Source +392, Tests +427, Docs +68. Total +887 across 13 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 (14 earlier review cycles; latest 8 shown)
|
475d6db to
59132f9
Compare
6731f57 to
4132e14
Compare
4132e14 to
32dd2ee
Compare
There was a problem hiding this comment.
The PR is pointed at the right layer: foreground gateway / gateway run gets a version checkpoint, a shared-state lease, state migrations, and post-core plugin convergence before readiness. That matches the broad intent of #98565.
Findings:
- [blocking] Startup records the version checkpoint even when configured-plugin repair did not complete. In
runStartupUpgradeConvergence, convergence warnings only become blockers whenconvergence.erroredis true (src/commands/doctor-config-preflight.ts:132,src/commands/doctor-config-preflight.ts:320). ButrunPostCorePluginConvergencetreats repair/install failures fromrepairMissingConfiguredPluginInstallsas warnings and setserroredonly for payload smoke failures (src/cli/update-cli/post-core-plugin-convergence.ts:131,src/cli/update-cli/post-core-plugin-convergence.ts:173). The new test codifies that behavior by recording the startup checkpoint while plugin repair warnings are present (src/commands/doctor-config-preflight.state-migration.test.ts:329). That means a container image swap can still report healthy after a configured official plugin failed to install/repair, then persist the current OpenClaw version as migrated; future restarts skip the startup convergence pass for that version. That is the same false-ready, half-upgraded state #98565 was meant to close. Please either treat startup plugin repair warnings that represent failed required convergence as readiness blockers, or avoid recording the startup checkpoint until a later startup/doctor pass completes those repairs. Pure informational notices can stay nonblocking, but failed migration/repair work should not be checkpointed as success.
Best-fix verdict: right owner boundary, but too permissive on migration failure semantics. This is not ready until startup convergence failure and checkpointing match the issue intent.
Evidence reviewed: PR head 32dd2ee88a53, current issue #98565 body, Dockerfile:358, src/cli/program/config-guard.ts, src/commands/doctor-config-preflight.ts, src/infra/startup-migration-checkpoint.ts, src/state/openclaw-state-db.ts, src/cli/update-cli/post-core-plugin-convergence.ts, and the added tests/docs. CI is broadly green, but this is a source-level policy/correctness gap rather than a failing-test gap.
Signed-off-by: sallyom <somalley@redhat.com>
Signed-off-by: sallyom <somalley@redhat.com>
32dd2ee to
749f839
Compare
Fixed by keeping Covered by the updated regression test in |
|
as a last check before merge, I ran an upgrade and confirmed this in gateway logs and files:
{
So this shows the new image started against mounted state, ran the startup migration gate, wrote the per-version checkpoint, released the lease, and only then became ready. |
…claw#101881) * run all 'openclaw upgrade' migrations with container image upgrades Signed-off-by: sallyom <somalley@redhat.com> * fix: block gateway startup on plugin repair warnings Signed-off-by: sallyom <somalley@redhat.com> --------- Signed-off-by: sallyom <somalley@redhat.com>
…claw#101881) * run all 'openclaw upgrade' migrations with container image upgrades Signed-off-by: sallyom <somalley@redhat.com> * fix: block gateway startup on plugin repair warnings Signed-off-by: sallyom <somalley@redhat.com> --------- Signed-off-by: sallyom <somalley@redhat.com>
What Problem This Solves
Fixes #98565.
Replacing a Docker, Podman, or Kubernetes container image while preserving the mounted OpenClaw state volume could start
openclaw gateway rundirectly and skip the startup repair/convergence work normally reached throughopenclaw updateoropenclaw doctor --fix. That let a gateway become healthy while state and plugin install metadata still reflected the previous image.Why This Change Was Made
gateway healthalone.User Impact
Routine container image upgrades with the same mounted state should converge automatically before the gateway reports healthy. If repair cannot complete safely, startup fails closed instead of serving with half-upgraded state; operators can run
openclaw doctor --fixwith the same image and mounted state volume, then restart. Nonblocking plugin repair warnings are logged but do not stop startup, matching the currentopenclaw updatebehavior more closely.Evidence
node scripts/crabbox-wrapper.mjs run --provider blacksmith-testbox --timing-json -- pnpm test src/infra/startup-migration-checkpoint.test.ts src/commands/doctor-config-preflight.state-migration.test.tspassed on Testboxtbx_01kwzaarst9n88z2rxf30dz6s9: 6 checkpoint tests and 15 startup preflight tests.node scripts/crabbox-wrapper.mjs run --provider blacksmith-testbox --timing-json -- pnpm check:changed -- src/infra/startup-migration-checkpoint.ts src/infra/startup-migration-checkpoint.test.ts src/commands/doctor-config-preflight.ts src/commands/doctor-config-preflight.state-migration.test.ts src/state/openclaw-state-db.tspassed on Testboxtbx_01kwzade1d7rne5f0tgatwkaf3.tbx_01kwz5v7dp5vtycwmthhnw9jyw.provider=local-container, runtimepodman, leasecbx_3f10582cbbf0, Ubuntu 26.04, exit 0. A realpnpm openclaw gateway run --dev --allow-unconfigured --auth none --port 19565reached/health; checkpoint moved frombefore=2026.6.10toafter=2026.6.11;health={"ok":true,"status":"live"}..agents/skills/autoreview/scripts/autoreview --mode localclean after the narrowed startup-blocking policy and guard-order fixes..agents/skills/autoreview/scripts/autoreview --mode branch --base origin/mainclean on the full PR diff after committing.git diff --checkpassed.node scripts/check-docs-mdx.mjs docs README.md docs/install/docker.md docs/install/updating.mdpassed.node scripts/format-docs.mjs --check docs/install/docker.md docs/install/updating.mdpassed.node scripts/docs-link-audit.mjs -- docs/install/docker.md docs/install/updating.mdstill reports existing unrelated ClawHub route-link failures; this PR's new/install/docker#upgrading-container-imageslink is present in the changed docs.No Agent Transcript section included.