fix(ui): treat started managed-service handoff as pending update instead of skipped#87968
fix(ui): treat started managed-service handoff as pending update instead of skipped#87968xydigit-sj wants to merge 1 commit into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 8:45 AM ET / 12:45 UTC. Summary PR surface: Source +5, Tests +32. Total +37 across 2 files. Reproducibility: yes. at source level: current main's gateway started-handoff response omits Review metrics: 1 noteworthy metric.
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: Land the UI fix after maintainers either accept the current handoff-state proof as enough to close the linked Dashboard update issue or change the PR body to keep that issue open for final upgraded-version proof. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main's gateway started-handoff response omits Is this the best way to solve the issue? Yes for the UI state layer: using the already-delivered AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 07a7d596ff6e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +5, Tests +32. Total +37 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 (1 earlier review cycle)
|
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
|
ClawSweeper's P1/P2 concerns from the initial review have been addressed in the latest push:
The remaining P1 blocker is real behavior proof from a live managed-service Dashboard update. I don't have access to a managed-service environment to capture screenshots or live runtime logs for this path. Would a maintainer be able to apply a proof override, or is there another way to satisfy this gate without a live managed-service setup? Happy to make additional code changes if needed. |
|
Hi maintainers, Following ClawSweeper's review, I acknowledge that this PR is missing real behavior proof from a live managed-service Dashboard update path. The fix has been verified through colocated unit tests that mock the exact gateway response shape for the managed-service handoff (
However, I don't have access to a managed-service OpenClaw install (e.g., systemd/package-manager supervised) in my local dev environment, which is needed to capture live Dashboard proof of the actual handoff flow. Would a maintainer with access to a managed-service test environment be able to either:
The code and tests are otherwise ready for review. Thanks! |
836f599 to
6d5b84e
Compare
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
6d5b84e to
28c4ac5
Compare
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
d76a802 to
7274584
Compare
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
968d2de to
67bc16e
Compare
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
eb6adce to
c356225
Compare
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
c356225 to
4dc41f6
Compare
|
Quick update on the two remaining CI failures on this run:
The On the real behavior proof: I still don't have access to a managed-service environment to capture live Dashboard proof. The patch is otherwise rated platinum hermit by ClawSweeper. If a maintainer is able to validate this path or apply a |
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
4dc41f6 to
70fe257
Compare
70fe257 to
7389efb
Compare
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
…ks after.version ClawSweeper review (openclaw#87968) identified two issues: 1. P2: The regression test used a fictional result.after.version that does not match the real gateway managed-service handoff payload. The real response contains status: "skipped", reason: "managed-service-handoff-started", and optionally before: { version }, but no after field. 2. P1: Because after.version was absent, pendingUpdateExpectedVersion became null, causing verifyPendingUpdateVersion to exit immediately and skip post-restart version verification. Fix: - Add optional updateAvailable to ConfigState so runUpdate can access the latest known version when the gateway handoff response does not include an after version. - Use a three-tier fallback: after.version -> updateAvailable.latestVersion -> null. - Update the regression test to use the real handoff payload shape and assert that pendingUpdateExpectedVersion is derived from updateAvailable. - Add a second test for the null-fallback path when neither after nor updateAvailable is present. Refs: openclaw#87889
|
Real behavior proof for the managed-service handoff UI fix. Environment
Steps
{
"ok": true,
"result": {
"status": "skipped",
"mode": "git",
"root": "/app",
"reason": "managed-service-handoff-started",
"before": { "version": "2026.6.2" },
"steps": [
{
"name": "managed-service update handoff",
"command": "openclaw update --yes",
"cwd": "/app",
"durationMs": 20,
"exitCode": null
}
],
"durationMs": 20
},
"handoff": {
"status": "started",
"pid": 607,
"command": "openclaw update --yes"
},
"restart": {
"ok": true,
"pid": 1,
"signal": "SIGUSR1",
"delayMs": 2000,
"reason": "update.run",
"mode": "emit",
"coalesced": false,
"cooldownMsApplied": 0,
"emitHooksQueued": false
}
}UI state after clicking "Update now" {
"connected": true,
"pendingUpdateHandoff": true,
"pendingUpdateExpectedVersion": "2026.6.8",
"updateStatusBanner": null,
"updateRunning": false,
"lastError": null
}Verification
Notes
|
|
@clawsweeper re-review Real behavior proof has been added to this PR (see comment on 2026-06-24). The proof demonstrates the managed-service Dashboard handoff state is now treated as pending update instead of skipped in a local Docker environment built from this branch. Please re-run review. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Closing as superseded by #112987.\n\nThe handoff-as-pending behavior this PR originally implemented has since landed in |
Summary
Fixes #87889.
When a user clicks "Update now" in the Dashboard on a global install with a managed-service supervisor, the Gateway starts a managed-service handoff and returns
ok=truewithresult.status="skipped"andreason="managed-service-handoff-started". The Dashboard code previously checkedstatus === "ok"to decide success, so the handoff response fell through to the skipped-update banner even though the handoff was successfully started.ClawSweeper review identified two additional issues in the initial PR; both are addressed in the latest push:
result.after.versionthat does not match the real gateway managed-service handoff payload. The real response containsstatus: "skipped",reason: "managed-service-handoff-started", and optionallybefore: { version }, but noafterfield.after.versionwas absent,pendingUpdateExpectedVersionbecamenull, causingverifyPendingUpdateVersionto exit immediately and skip post-restart version verification.Change
ui/src/ui/controllers/config.ts—runUpdatenow treats anyok=trueresponse as success/pending, regardless ofresult.status. This aligns the Dashboard with the Gateway contract:ok=truemeans either the update succeeded or the managed-service handoff was started.runUpdatenow derivespendingUpdateExpectedVersionthrough a three-tier fallback:result.after.version->updateAvailable.latestVersion->null. This ensures post-restart version verification stays active even when the gateway handoff response does not include anafterversion.managed-service-handoff-startedto the actionable guidance map so the banner text is informative if the handoff ever surfaces as an error.updateAvailabletoConfigStatesorunUpdatecan access the latest known version when needed.Before:
ok=true, result.status=skipped, reason=managed-service-handoff-started, handoff.status=startedstatus=skipped-> shows "Update skipped" error bannerAfter:
ok=true-> setspendingUpdateExpectedVersion(fromafter.versionorupdateAvailable.latestVersion) and returns without bannerFiles changed
ui/src/ui/controllers/config.ts— derive pending version viaafter.version ?? updateAvailable.latestVersion ?? null; add handoff guidance; add optionalupdateAvailabletoConfigStateui/src/ui/controllers/config.test.ts— 2 tests: regression test with real handoff payload shape (noafter, usesupdateAvailablefallback), plus boundary test for null-fallback when neitherafternorupdateAvailableis presentVerification
Real behavior proof
Behavior addressed: Dashboard shows "Update skipped" banner when a managed-service handoff is actually started (#87889).
Real environment tested: Local OpenClaw source checkout on branch
fix/dashboard-handoff-skipped-state, Linux, Node 22.Exact steps or command run after this patch:
Run the actual
runUpdatesource entrypoint with the real managed-service handoff payload (note thehandoff.status: "started"that the gateway returns):Evidence after fix: Terminal capture from running the patched
runUpdatesource entrypoint with the real managed-service handoff payload:Observed result after fix: When the gateway returns
ok=truewithstatus=skipped,reason=managed-service-handoff-started, andhandoff.status=started, the Dashboard no longer shows an error banner. It setspendingUpdateExpectedVersiontoupdateAvailable.latestVersion(2026.6.1) and markspendingUpdateHandoffastrue, so the post-restart version verification path can take over.What was not tested: Live browser Dashboard click-through with a managed-service supervisor (the proof exercises the actual
runUpdatesource entrypoint).Refs: #87889