Skip to content

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

Open
kavish-19 wants to merge 1 commit into
CodebuffAI:mainfrom
kavish-19:fix/defer-auto-update-restart-until-idle
Open

fix(cli): wait for the current turn to finish before an auto-update restart#1258
kavish-19 wants to merge 1 commit into
CodebuffAI:mainfrom
kavish-19:fix/defer-auto-update-restart-until-idle

Conversation

@kavish-19

@kavish-19 kavish-19 commented Sep 3, 2026

Copy link
Copy Markdown

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.

…estart

main() schedules checkForUpdates 100ms after spawning the binary. When it
finds a newer version it stages the download and then unconditionally
SIGTERMs (SIGKILL after 5s) the running process to install it -- with no way
to know whether the user is mid-turn, because the wrapper is a separate
process that only sees the child's exit event, not its React state. A
download that lands a few seconds into a session therefore kills a turn
that is still running, which is what CodebuffAI#994 reports.

Adds a small cross-process signal in the spirit of the existing
terminal-watchdog marker files: the binary writes an activity marker for the
duration of a turn (subscribed once to the store's isChainInProgress, so
every current and future call site is covered) and removes it when idle or
on exit. The wrapper waits for that marker to clear before stopping the
process for an update.

Best-effort and bounded in both directions: 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, and a turn that never ends stops blocking the update after
10 minutes.

Tests: three for the marker's write/remove/idempotence, three for
waitForRunIdle's immediate, waits-then-clears, and gives-up-at-the-bound
paths, plus the existing checkForUpdates source-order check extended to
require the wait between staging and stopping. Confirmed red against the
unfixed code, green after; the full cli suite shows the same 34 pre-existing
failures before and after.

Claude-Session: https://claude.ai/code/session_018vPhyqaaoKa8cgs7GEnyq5
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): When there is an update, it automatically restarts, even if I am in the middle of a session.

1 participant