fix(cli): wait for the current turn to finish before an auto-update restart

[kavish-19]

Fixes #994.

The bug

main() schedules checkForUpdates 100ms after spawning the binary. When it finds a newer version it stages the download and then unconditionally stops the running process to install it:

const stagedBinary = await stageBinary(latestVersion, getDownloadTargetKey(), { quiet: true })
runningProcess.removeListener('exit', exitListener)
await stopRunningProcess(runningProcess)   // SIGTERM, then SIGKILL after 5s

The wrapper has no way to know whether the user is mid-turn — it's a separate process that only sees the child's exit event, not its state. So a download that lands a few seconds into a session kills a turn that is still running, which is exactly what the reporter describes: "once the download finishes, the CLI automatically restarts, even if I'm in the middle of a session."

The fix

A small cross-process signal, in the spirit of the marker files terminal-watchdog.ts already uses for the same kind of wrapper/binary coordination:

• Binary side (cli/src/utils/run-activity-marker.ts): writes a marker file for the duration of a turn and removes it when idle or on exit. It subscribes once to the store's isChainInProgress, so every current and future site that toggles that flag is covered without touching any of them. Named by the process's own pid, which the wrapper already has from spawning it — no handshake needed.
• Wrapper side (cli/release-core/launcher.js): waitForRunIdle(pid) polls for that marker to clear, and checkForUpdates awaits it after staging and before stopping the process.

Bounded and best-effort in both directions, so it can only ever delay a restart, never prevent one:

• A missing marker — already idle, an older binary that predates this file, a process that died without cleaning up — resolves immediately and preserves today's restart-right-away behavior.
• A turn that never ends stops blocking the update after 10 minutes.

The staging download itself is unchanged and still happens up front; only the stop-and-swap waits.

Testing

New tests:

• Marker: writes while a turn is in progress, removes when idle, no-ops when the value doesn't actually change, and never stacks a duplicate exit handler.
• waitForRunIdle: returns immediately with no marker, waits and returns once the marker clears, and gives up at maxWaitMs without touching the marker.
• Extended the existing checkForUpdates source-order check to require the wait between staging and stopping.

Confirmed red against the unfixed code (all four new launcher assertions fail — waitForRunIdle doesn't exist) and green after.

bun test cli/src/__tests__/release/wrapper-safety.test.ts cli/src/utils/__tests__/run-activity-marker.test.ts
 23 pass / 0 fail

Also ran the full cli/src suite before and after: identical 34 pre-existing failures and 32 errors either way (they reproduce on unmodified main — an OSC 52 clipboard test plus missing @types/react-dom / tar in the local environment), with 6 new passing tests and no regressions. tsc --noEmit on the cli package reports nothing new in any touched file.

[codebuff-team]

Good diagnosis: checkForUpdates in launcher.js really does call stopRunningProcess unconditionally after staging, and the wrapper genuinely has no visibility into the child's internal state, so this is a real bug with a clear cause (issue #994).

The fix itself is sensible and cheap: a pid-named marker file toggled off isChainInProgress, polled by the wrapper with a bounded 10-minute fallback so a stuck turn can never permanently block an update. Subscribing once to the store instead of touching every site that sets isChainInProgress is the right layer for this — it stays correct as new call sites appear. Tests cover the marker's write/remove/no-duplicate-listener behavior and the three waitForRunIdle cases (no marker, clears mid-wait, gives up at the bound), which is more than most launcher changes in this repo get.

Two things worth thinking about before this lands for real, even if they don't block the idea:

1. Race window: waitForRunIdle is only called once, right after staging. If a turn starts between that check and the eventual stopRunningProcess call, it still gets killed. Given staging can take a while this window isn't negligible. A loop that re-checks right before the actual kill (or checks isChainInProgress again just before SIGTERM) would close this.
2. Marker staleness on hard kill: if the binary dies via SIGKILL, process.on('exit', clear) never runs and the marker leaks in os.tmpdir(). Pid reuse is rare but not impossible, and a leaked marker would falsely stall the next run's updates for up to 10 minutes. Worth stamping the marker with a start-time or session id you can also cross-check.

Neither is disqualifying — both degrade gracefully to "restart eventually happens" rather than silent breakage — but they're the kind of edge case a maintainer will ask about.

[kavish-19]

Thanks — took both. Point 2 is fixed in 3940411; point 1 I think rests on a misread of the ordering, details below.

2. Marker staleness on hard kill — real, fixed.

Agreed: SIGKILL or a native crash means process.on('exit', clear) never runs and the marker outlives the process, so a reused pid stalls the next run's updates for the full bound.

I went with clearing at spawn rather than stamping an identity into the marker. At the point spawnInstalledBinary has child.pid, the binary has not booted, let alone started a turn — so any marker at that path is definitionally an earlier process's, and no session id or start-time is needed to tell them apart. One rmSync on a path we already compute, versus a write format plus a parse-and-compare on the wrapper side. Covered by three tests: the clear itself, the no-marker case, and a source-order assertion that spawnInstalledBinary clears after it has a pid and before it returns the child.

1. Race window — the ordering is the other way round.

The sequence in checkForUpdates is:

const stagedBinary = await stageBinary(...)   // staging
await waitForRunIdle(runningProcess.pid)      // then wait
term.clearLine()
runningProcess.removeListener('exit', exitListener)
await stopRunningProcess(runningProcess)      // then stop

Staging happens before waitForRunIdle, not between it and the kill — so "given staging can take a while this window isn't negligible" doesn't apply. What actually sits in the gap is term.clearLine() and removeListener(): two synchronous calls, no await. The existing test in wrapper-safety.test.ts pins that order (stageIndex < waitIndex < stopIndex) precisely so a later edit can't reintroduce the window you're describing.

There is still a sub-millisecond TOCTOU gap, since these are separate processes and the user could hit Enter inside it. But that gap is irreducible without a handshake: re-checking immediately before SIGTERM moves the window, it doesn't close it, because the binary can always start a turn after the wrapper's last look. Closing it properly means the wrapper asking the binary to stop accepting turns and waiting for an ack — which is a bidirectional protocol, and this repo deliberately avoids pipes between these two processes.

For a bug whose current behavior is "kill unconditionally, mid-turn, every time", trading that for a sub-millisecond window seemed like the right amount of machinery. Happy to add the extra re-check if you'd still prefer it — it's cheap and harmless, I just don't want to claim it fixes something it doesn't.

Full suite is unchanged at 39 failures, identical to the pre-change baseline. tsc --noEmit clean on the touched files. launcher.js still fails prettier --check, but only on a pre-existing guard clause in checkForUpdates that fails on unmodified main too — my lines are clean, and I've left that one alone rather than bury the diff in an unrelated reformat.