Skip to content

fix(cli): make systemd npm updates restart-safe - #2411

Open
L42y wants to merge 1 commit into
first-tree-ai:mainfrom
L42y:fix/daemon-update-restart-safety-2394-clean
Open

L42y wants to merge 1 commit into
first-tree-ai:mainfrom
L42y:fix/daemon-update-restart-safety-2394-clean

Conversation

@L42y

@L42y L42y commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #2394

What changed

  • Managed Linux npm updates now run in a per-channel transient systemd-run unit outside the daemon service cgroup.
  • The worker stops the daemon before changing the global package tree, moves the package and channel bin links aside, and rolls them back if npm fails.
  • The daemon is started only after a complete install or a successful rollback; direct and manual npm updates retain their existing path.

Validation

  • pnpm check — passed (repository baseline warnings remain).
  • pnpm typecheck — passed.
  • Focused update, rollback, glue, supervisor, and daemon-start tests — passed.
  • Related update test files — passed.
  • Full root test command remains blocked by the host: server PostgreSQL tests cannot find a container runtime; the CLI batch run also hit unrelated Context Tree test timeouts under the shared host workload.

@yuezengwu yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new transient worker fixes the npm reify window, but it removes the existing service-unit refresh from managed Linux updates. systemctl stop terminates the daemon that is awaiting systemd-run; consequently createExecuteUpdate cannot reach refreshServiceUnit() after installation. A release that changes the systemd unit (especially ExecStart, the regression daemon refresh-unit was introduced to prevent) is then started using the old definition and can fail or crash-loop. Please run the newly installed binary’s unit-drift refresh from the surviving worker before starting the daemon, and cover that handoff in a test. I verified this against head 92c5bc0; pnpm check, pnpm typecheck, and 63 focused local tests passed, and the current 15 CI checks are green. The local host is macOS, so the new Linux-only tests did not execute locally.

"trap finish EXIT",
"trap 'exit 143' HUP INT TERM",
`npm_command="$1"; shift; package_name=${packageName}; bin_name=${binName}; alias_name=${aliasName}`,
`${systemctl} stop ${serviceUnit}`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stopping this unit kills the old daemon before createExecuteUpdate can reach its existing refreshServiceUnit() call in update-glue.ts. The worker later starts the service without rewriting a drifted unit from the newly installed binary. If a release changes ExecStart (for example, the historical client start to daemon start migration), the old unit can boot the new CLI with an obsolete command. Please move the unit-drift refresh into this surviving worker before restart and test that path.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Daemon auto-update is not restart-safe: killed npm install corrupts node_modules and crash-loops the whole host

2 participants