Skip to content

fix(update): prefer package root as managed-service handoff cwd (fixes #87889)#88224

Closed
ztexydt-cqh wants to merge 1 commit into
openclaw:mainfrom
ztexydt-cqh:fix/dashboard-update-handoff-cwd-87889
Closed

fix(update): prefer package root as managed-service handoff cwd (fixes #87889)#88224
ztexydt-cqh wants to merge 1 commit into
openclaw:mainfrom
ztexydt-cqh:fix/dashboard-update-handoff-cwd-87889

Conversation

@ztexydt-cqh

Copy link
Copy Markdown
Contributor

Summary

Fixes #87889 — Dashboard updates for global installs are skipped with managed-service-handoff-started, while CLI updates succeed.

Root Cause

resolveManagedServiceHandoffCwd preferred os.homedir() over the package root when choosing the working directory for the managed-service update handoff script. Because the handoff script runs node <argv1> update ... with a relative argv1 (e.g. dist/index.js), running in the wrong directory caused the entry path to be unresolvable. The update silently failed after gateway restart.

CLI updates bypass the handoff path and call runGatewayUpdate directly with the correct cwd, which is why they succeeded while Dashboard updates failed.

Changes

src/gateway/server-methods/update-managed-service-handoff.ts

  • Move root to the first candidate in resolveManagedServiceHandoffCwd so the handoff script runs in the package root directory.

src/gateway/server-methods/update-managed-service-handoff.test.ts

  • Update the test to assert that cwd matches the provided root instead of os.homedir().
  • Create a real temporary directory for root so fs.stat resolves it successfully.

Real behavior proof

Behavior or issue addressed: Dashboard-initiated global install updates now run the handoff script in the correct package root instead of os.homedir().

Real environment tested: Code analysis and unit-test validation on the PR branch.

Exact steps or command run after this patch:

  1. Read resolveManagedServiceHandoffCwd to confirm root is now the first candidate.
  2. Read update.ts to confirm the handoff cwd is derived from resolveManagedServiceHandoffCwd(params.root).
  3. Run the existing unit test strips process supervisor hints while preserving service identity for the CLI handoff.

Evidence after fix:

  • resolveManagedServiceHandoffCwd candidate order is now [root, os.homedir(), os.tmpdir(), path.dirname(process.execPath)].
  • The test options.cwd and helperParams.cwd both assert against the provided root directory.

Observed result after fix:

  • The handoff test passes with cwd set to the actual root temp directory.
  • No change to CLI update behavior (it never used the handoff path).

What was not tested:

  • Live Dashboard update on a real globally-installed gateway instance.
  • End-to-end systemd/launchd handoff restart validation.

Testing

node scripts/run-vitest.mjs src/gateway/server-methods/update-managed-service-handoff.test.ts

Risks

Risk is very low. The change only reorders the directory fallback list in resolveManagedServiceHandoffCwd. The previous behavior (always falling back to os.homedir()) was the unintended default because os.homedir() is almost always a valid directory.

Fixes #87889

When a global install triggered an update from the Dashboard, the managed
service handoff script fell back to os.homedir() as its working directory.
This caused the update command (e.g. node dist/index.js update ...) to run
in the wrong directory, so the relative entry path could not be resolved and
the update silently failed after gateway restart.

CLI updates bypass the handoff path and call runGatewayUpdate directly with
the correct cwd, which is why they succeeded while Dashboard updates failed.

Changes:
- update-managed-service-handoff.ts: move root to the first candidate in
  resolveManagedServiceHandoffCwd so the handoff runs in the package root.
- update-managed-service-handoff.test.ts: update test to assert cwd matches
  the provided root instead of os.homedir().

Fixes openclaw#87889
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 30, 2026
@clawsweeper

clawsweeper Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 12:32 AM ET / 04:32 UTC.

Summary
The PR reorders managed-service update handoff cwd candidates so the package root is tried before homedir/tmp fallback directories and updates the focused handoff test expectation.

PR surface: Source 0, Tests +2. Total +2 across 2 files.

Reproducibility: yes. at source level, but not as a live user flow: current main passes the package root to the handoff helper while the helper still chooses os.homedir() first. I did not establish a real managed-service Dashboard update reproduction in this read-only review.

Review metrics: 3 noteworthy metrics.

  • Moved implementation path: 1 helper moved on main. The PR changes the old gateway server-methods path, while the current implementation lives under src/infra.
  • Cwd fallback order: 1 fallback order changed. Managed-service update cwd selection is compatibility-sensitive because it can affect package-manager config, relative entry paths, and restart recovery.
  • Closing issue references: 1 linked issue. Merging this PR as-is would close the canonical Dashboard update report while companion UI/full-flow work remains open.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #87889
Summary: This PR is a backend cwd candidate for the canonical Dashboard update issue; the open UI PR covers an overlapping user-visible state path rather than superseding this backend change.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • Refresh the change onto src/infra/update-managed-service-handoff.ts and update the focused test there.
  • [P1] Add redacted live proof from a managed global/package Dashboard update showing handoff, restart, and the upgraded version afterward.
  • Coordinate or remove the closing reference so the canonical issue stays open until the backend and UI paths are resolved.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides code analysis and unit-test validation only; a contributor or maintainer should add redacted live Dashboard/managed-service proof, then update the PR body for a fresh ClawSweeper review.

Mantis proof suggestion
A real managed-service Dashboard update is the clearest proof for the user-visible update flow and final version state. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify a managed global Gateway Dashboard update starts handoff, restarts, and reports the upgraded version without a skipped banner.

Risk before merge

  • [P1] The branch is dirty because current main moved the helper to src/infra/update-managed-service-handoff.ts; the cwd-order change must be refreshed onto the current file before merge.
  • [P1] The supplied proof is code analysis plus a focused unit test, not a live managed global/package Dashboard update showing handoff, restart, and the upgraded version.
  • [P1] The PR uses closing syntax for the canonical Dashboard update issue while a companion UI-state PR remains open, so merging only this backend fix could close the issue before the full user flow is proven.
  • [P2] Changing managed-service cwd fallback order touches update compatibility behavior established by the prior stable-cwd hardening, so maintainers should validate root-first behavior against real package-root deletion/restart cases.

Maintainer options:

  1. Refresh and prove the managed update path (recommended)
    Apply the cwd-order change to src/infra/update-managed-service-handoff.ts and add redacted proof from a managed global/package Dashboard update showing handoff, restart, and the final upgraded version.
  2. Accept source-level proof intentionally
    Maintainers can merge after refresh with an explicit proof override if they are comfortable owning the untested managed-service Dashboard path and the cwd fallback change.
  3. Pause for a canonical replacement
    If the stale branch is not worth refreshing, keep it paused or close it only after a viable replacement PR covers the backend cwd fix and links this contribution.

Next step before merge

  • [P1] Keep this in human review because the useful backend fix needs branch refresh, live proof or maintainer override, and linked-issue coordination rather than a safe automated code repair alone.

Security
Cleared: The diff changes managed-service cwd fallback order and a focused test only; no dependency, workflow, permission, secret, package-publishing, or downloaded-code surface changes were found.

Review details

Best possible solution:

Refresh the backend cwd fix onto the current src/infra helper, preserve the embedded helper's fallback for disappearing roots, coordinate the linked issue closure with the companion UI work, and add redacted live managed-service update proof before merge.

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

Yes at source level, but not as a live user flow: current main passes the package root to the handoff helper while the helper still chooses os.homedir() first. I did not establish a real managed-service Dashboard update reproduction in this read-only review.

Is this the best way to solve the issue?

Yes for the backend cwd theory: preferring the package root before generic directories is a narrow repair for relative entry paths. The patch still needs to be refreshed onto the moved helper and proven against the prior stable-cwd fallback design in a real managed-service update.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against de95726177b6.

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: The PR changes the cwd fallback used by managed-service update helper processes, which can affect upgrade behavior and cwd-derived package-manager assumptions.

Label justifications:

  • P2: This is a normal-priority user-facing Dashboard/global update bug with a narrow backend handoff surface and a CLI/manual update workaround.
  • merge-risk: 🚨 compatibility: The PR changes the cwd fallback used by managed-service update helper processes, which can affect upgrade behavior and cwd-derived package-manager assumptions.
  • merge-risk: 🚨 other: The PR's closing reference could close the canonical Dashboard update issue before companion UI work and live full-flow proof settle the remaining user-visible path.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides code analysis and unit-test validation only; a contributor or maintainer should add redacted live Dashboard/managed-service proof, then update the PR body for a fresh ClawSweeper review.
Evidence reviewed

PR surface:

Source 0, Tests +2. Total +2 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 1 0
Tests 1 5 3 +2
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 6 4 +2

What I checked:

Likely related people:

  • jason-allen-oneal: Authored merged PR fix(update): harden managed handoff cwd #83875, which introduced the managed handoff cwd hardening and fallback behavior adjacent to this PR's change. (role: cwd handoff history; confidence: high; commits: 1bafc23ae356; files: src/infra/update-managed-service-handoff.ts, src/gateway/server-methods/update.test.ts)
  • vincentkoc: Merged the prior cwd-hardening PR and current-main blame ties the moved helper and update handoff caller to the current source tree. (role: recent area contributor and merger; confidence: high; commits: 1bafc23ae356, 459bcd6198e3; files: src/infra/update-managed-service-handoff.ts, src/gateway/server-methods/update.ts)
  • joshavant: Authored merged Linux service auto-update handoff work that uses the same managed-service update path and is relevant to live proof expectations. (role: adjacent update handoff contributor; confidence: medium; commits: 8673c65c6bad; files: src/infra/update-startup.ts, src/infra/update-managed-service-handoff.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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels May 30, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update from Dashboard skipped, works from CLI

1 participant