Self-update: host stays alive but stops serving after a failed update #9459

Activity

1. github-actions commented on Nov 7, 2026

   

   github-actionsBot

   on Nov 7, 2026 – with GitHub Actions

   Contributor

   More actions

   The report checks out. The bug is in packages/host-service/src/self-update/SelfUpdater.ts. After run() calls stopServing(), any error it throws goes to the .catch in start(). That catch only calls fail(), which records the error and never calls exit(). The host has already stopped listening and dropped its relay, so the old process stays alive but can't answer. The "restart interrupted" marker written before the restart is never replaced, and that makes later automatic updates skip the target version.

   Affected code

   • packages/host-service/src/self-update/SelfUpdater.ts:157-161: run(root, input).catch((error) => this.fail(...)). No exit is reachable from here.
   • SelfUpdater.ts:217-221: fail() sets phase = "failed" and logs. It does not exit.
   • SelfUpdater.ts:255-264: sets phase = "restarting", writes the { outcome: "failed", error: "restart interrupted" } marker, then await this.deps.stopServing().
   • SelfUpdater.ts:285: await rm(${root}.bak, { recursive: true, force: true }) on the healthy path. This runs before the updated marker is written (:286) and before exit(0) (:296), so an error here skips both.
   • SelfUpdater.ts:301: await successor?.kill() on the rollback path. spawnDetachedHost().kill() throws on purpose when the child survives SIGKILL (:498-501, "refusing to replace its install").
   • SelfUpdater.ts:193-194: checkForUpdates() skips a target when lastResult.to === target && outcome !== "updated". This is why the stale marker blocks that version.

   Root cause

   From stopServing() onward, every path that completes normally ends in this.deps.exit(...), but an exception leaves run() before reaching one. Five calls after stopServing() can throw:

   1. successor.kill() at :301, the case described in the issue.
   2. rm(${root}.bak) at :285, the other case in the issue. Here the successor is already healthy and writeManifestPid has already pointed the manifest at it. The only effects are a leftover old process and a marker that wrongly says failed / "restart interrupted".
   3. The rollback file operations at :314-317: rm(failed), the two renameSync calls, and the second rm(failed). These have the same problem and the issue does not mention them. A failed renameSync(backup, root) is the worst case, because the install can be left half-moved.
   4. stopServing() itself, if it rejects partway through.

   Calls before stopServing() (the CLI update and parsing the output) are fine. They fail while the host is still serving, so staying alive in failed is correct there.

   Reproduction

   There is a harness in SelfUpdater.test.ts (fake spawnHost / pollHealth / exit, with h.exitCode). Two bun:test cases would show the bug:

   • kill() throws: make pollHealth return null, and make spawnHost return { pid: 111, isRunning: () => true, kill: async () => { throw new Error("did not exit"); } }. Wait until phase === "failed". Today h.exitCode is null, stopServing has run, and the marker still says "restart interrupted".
   • .bak removal fails: use a healthy successor, and make ${root}.bak impossible to remove. The simplest way is to inject rm as a dependency. Without that, you could chmod the parent directory, but that is platform-dependent. Today h.exitCode is null and the marker is not updated, even though the manifest pid was already rewritten.

   Manually: run the standalone host, start a self-update, and either make the new superset-host fail its health check and ignore SIGTERM/SIGKILL (for example, a stopped or uninterruptible process), or make <root>.bak impossible to remove. The old pid stays alive and does not answer on the port.

   Suggested fix

   The approach in the issue fits the code:

   • In the .catch at :158, after fail(...), call this.deps.exit(1) when the phase was restarting before the failure. Capture the phase before fail() overwrites it. Also write a failed marker that carries the real error, so the leftover marker doesn't say "restart interrupted". This covers every call after stopServing() listed above, not only kill().
   • When kill() throws, exit without restoring .bak. That matches what the error message promises.
   • Make the .bak removal at :285 best effort: wrap it in try/catch, log the error, and continue to the updated marker and exit(0). A leftover .bak is harmless, because the next --keep-backup update replaces it.
   • Optional: inject rm/renameSync through SelfUpdaterDeps so tests can fail them on purpose without filesystem permission tricks.

   Each case needs one regression test, using the existing harness pattern.