fix: make the in-app updater edition-aware - #370
Merged
Conversation
An update on a Hermes device ended red on `gateway_verify`. That step waits
for the OpenClaw gateway on port 18789, which this edition deliberately masks
and closes, so it could only ever throw — and being `failFast`, it stopped the
run there.
The run contradicted itself: `post_update`'s smoke test FAILS the install if
anything is listening on 18789, and the very next step demanded that something
was.
The visible damage was worse than a red row. `runUpdate` only persists
`update_completed` when nothing failed, so a Hermes box never recorded a
finished update — which gates the setup wizard's resume step and the versions
route, leaving the device presenting a completed update as unfinished.
Steps now carry an `applies()` predicate and inapplicable ones are dropped from
the list entirely, so they render as absent rather than as failures:
- openclaw_install / openclaw_patch — no `openclaw` binary on hermes
- gateway_setup / gateway_verify — no gateway unit, port closed
Each names the helper matching its own reason (binary vs gateway) rather than
one blanket edition check, so the two can diverge later without the filter
quietly going wrong. `dual` ships both harnesses and is unchanged.
Hermes provisioning becomes its own visible step. It already ran on every
update — inside `step_post_update`, wrapped in `|| echo "(non-fatal)"` — so a
device that failed to provision its entire edition still finished green with
the failure buried in the journal. Same work, moved to where it can be seen and
can fail honestly; `step_post_update` no longer calls it, so it runs once.
Fresh installs are unaffected — they call `step_hermes_edition` directly.
The scoped OpenClaw-only update now refuses on an edition without OpenClaw
instead of no-opping two steps and then failing on the same absent gateway.
`setup-hermes-edition.sh` passes HOME explicitly to every `runuser` call. All
three scripts resolve state from `${HOME:-/home/clawbox}`; this runs as root,
and relying on runuser to reset HOME would have let the dashboard-auth script
read the wrong config, find no credentials to verify, and mint a password the
proxy does not hold. Verified on hardware that the existing early-exit does
hold — the stored hash verifies the stored password, so re-running mints
nothing — and this makes that independent of runuser's semantics.
The updater tests now pin the SKU. They are routinely run on a device, where
/etc/clawbox/edition.env exists and is authoritative, so without pinning the
suite's result would depend on which box it ran on.
Review pass over the previous commit. Behaviour is unchanged.
The one real defect: the updater tests set CLAWBOX_EDITION_FILE themselves and
deleted it in afterEach. vitest.config.ts already points that variable at a
path that cannot exist, precisely so the suite stays hermetic on a device — so
the delete was removing a suite-wide guarantee for every later file in the
worker. Only the edition VALUE is pinned now.
Test consolidation:
- `loadUpdater(edition?)` replaces four hand-rolled resetModules/env/import
preambles, matching loadHarness() in harness-edition.test.ts.
- Two assertions that the exact-list test already subsumed are folded into it.
- Dropped a DISPATCH_STEPS check that duplicates install-edition-lock.test.ts.
- Replaced a source-text scan of setup-hermes-dashboard-auth.sh with a
behavioural test in the file that owns that script: run it twice, assert the
password and hash are untouched. Verified against the real script on device.
Added the cross-layer pin that edition-source.ts's docblock claims but nothing
enforced: `hasHermesHarness()` and install.sh's `has_hermes_harness()` must
agree on which SKUs run the Hermes harness, or a device either skips its own
provisioning or dispatches a step that immediately returns.
`applicableSteps()` drops a parameter that only ever took one value, and
`createInitialState()` loses its default so the list is passed explicitly at
every call site — the invariant that the same array feeds both `state.steps`
and the runner is easier to see when nothing resolves it implicitly.
Trimmed comments that restated the code or re-told the commit message.
Now that the updater filters `gateway_setup` out on hermes, the `is_hermes_edition` guard inside step_gateway_setup reads as dead code and invites deletion. It is not dead: step_post_update has no `applies()` predicate, runs on every SKU, and calls step_gateway_setup itself — so removing that guard would reinstall and enable an OpenClaw gateway on a Hermes box halfway through its own update, reintroducing this PR's bug one layer down. `install.sh --step gateway_setup` run by hand is the same hole. Comment at the guard, plus tests pinning both ends of the argument: post_update still calls gateway_setup, and gateway_setup (and the two openclaw steps) still refuse on hermes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughChangesEdition-aware provisioning
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🦀 ClawReviewScuttled over to say hello and get you oriented 🦀 This PR fixes a bug where in-app updates on Hermes devices always ended in failure: the At a glance
Good to know
— ClawReview 🦀. I set the scene; CodeRabbit reviews the code; you decide. Conventions: docs. |
github-actions
Bot
added
area: install
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
An in-app update on a Hermes device ends red on
gateway_verify. That step waits for the OpenClaw gateway on port 18789, which this edition deliberately masks and closes, so it can only ever throw — and because it isfailFast, the run stops there.The clearest way to see that this step was never meant to run on this SKU: the run contradicts itself.
post_updatecallsstep_update_smoke, which on hermes fails the install if anything is listening on 18789. The very next step demands that something is.The user-visible damage is worse than a red row.
runUpdateonly persistsupdate_completedwhen nothing failed, so a Hermes box never records a finished update. That flag gatesSetupWizard.tsx's resume step and the/update/versionsroute — the device keeps presenting a completed update as unfinished.What was already fine
Worth stating, because it narrowed the change considerably:
step_openclaw_install,step_openclaw_patchandstep_gateway_setupalready early-return on hermes insideinstall.sh. They were never failures — they cost time and printed skip lines, but reportedcompleted.step_post_update. What was missing was not the work but its visibility: it was wrapped in|| echo "Warning: … (non-fatal)", so a device that failed to provision its entire edition still finished the update green.The change
Steps carry an applicability predicate, and inapplicable ones are dropped from the list. A dropped step never renders, so it reads as absent rather than as a failure — no UI,
StepStatusor translation change was needed, sinceUpdateStep.tsxrendersstate.stepsverbatim andMeterderives its bounds from the array.openclaw_install/openclaw_patchopenclawIsAbsent())gateway_setup/gateway_verifygatewayIsAbsent())hermes_edition(new)hasHermesHarness())Each step names the helper matching its own reason for being skipped rather than one blanket edition check. The two predicates are identical today; keeping them distinct means they can diverge without the filter quietly going wrong.
dualships both harnesses and is unchanged.Provisioning becomes its own visible step.
hermes_editionruns afterpost_update(sostep_systemd_serviceshas already refreshed the unit files it reinstalls) and is no longer called from insidestep_post_update, so it runs exactly once and can fail honestly. Fresh installs are unaffected — they callstep_hermes_editiondirectly.The scoped OpenClaw-only update refuses on an edition without OpenClaw instead of no-opping two steps and then failing on the same absent gateway.
/setup-api/update/openclawwas reachable and unguarded.setup-hermes-edition.shpassesHOMEexplicitly to everyrunusercall. All three scripts it invokes resolve state from${HOME:-/home/clawbox}and it runs as root.runuserdoes resetHOMEtoday — verified on hardware, util-linux 2.37.2 — but relying on that would let the dashboard-auth script read the wrong config, find no credentials to verify, and mint a password the proxy does not hold.Idempotency
hermes_editionnow runs on every update, so re-running has to be free. Verified on a real Hermes box, in a throwaway root with overridden paths:creds_are_consistent()re-derives the scrypt hash from the stored plaintext and exits before any minting, so a working box is never rotated. This is now pinned by a behavioural test — run the script twice, assert password and hash unchanged — rather than a source-text scan.A guard that looks dead and is not
Once
applies()dropsgateway_setupon hermes, theis_hermes_editionguard insidestep_gateway_setupreads as dead code. It is not.step_post_updatehas no predicate, runs on every SKU, and callsstep_gateway_setupitself — deleting that guard would reinstall and enable an OpenClaw gateway on a Hermes box halfway through its own update, reintroducing this bug one layer down.install.sh --step gateway_setupby hand is the same hole. There is now a comment at the guard and tests pinning both ends of the argument.Testing
completedwithupdate_completedpersisted, the no-re-mint behaviour, and a cross-layer pin thathasHermesHarness()andhas_hermes_harness()agree.e2e-install's90-upgrade-main-to-beta.spec.tsis unaffected: it asserts every step instate.stepscompleted without naming them, and the container resolves asopenclaw, where the list is unchanged.A Hermes variant of the e2e-install upgrade spec is not feasible today, and this is why rather than an excuse.
CLAWBOX_EDITIONis not plumbed throughdocker-compose.test.ymlorentrypoint.sh.step_hermes_installhas no test-mode guard and shells out to an unpinned third-party installer, degrading to a warning on failure — leaving nohermesbinary, a crash-looping dashboard unit, andstep_validate_servicesreturning 1. That abortsinstall.shunderset -e, so.needs-installis never removed, the healthcheck never passes, andglobal-setup.tsburns its 40-minute timeout and fails every spec in the suite. The harness is alsoworkers: 1against one hardcoded container, and the edition is baked at install time with the gateway deleted and masked, so the two editions cannot coexist. Doing it properly needs a second compose identity and a decision about what "Hermes installed" means in CI.Pre-flight before the manual upgrade test
The whole fix keys off
readEdition(). Run this on the older Hermes box first:If none of those carry the SKU,
readEdition()falls back toopenclaw, the box runs the OpenClaw steps, and it fails atgateway_verifyexactly as before — making a correct fix look broken. Confirm the edition resolves before drawing any conclusion from the run.Also set the update target, since a box with no
.update-branchresolves via its current branch's upstream and can silently aim atmain:Expected on hermes:
bootstrap_updater → apt_update → nvidia_jetpack → performance_mode → chromium_install → vnc_install → restart → post_update → hermes_edition, endingcompleted, with no OpenClaw or gateway rows shown at all.Summary by CodeRabbit
New Features
Bug Fixes