Skip to content

fix(coding-agent): finalize timed-out worker stops instead of stranding registrations - #851

Merged
snimu merged 13 commits into
mainfrom
snimu/worker-shutdown-finalize
Aug 11, 2026
Merged

fix(coding-agent): finalize timed-out worker stops instead of stranding registrations#851
snimu merged 13 commits into
mainfrom
snimu/worker-shutdown-finalize

Conversation

@snimu

@snimu snimu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Problem

When a worker refused to exit in time (say it was suspended), the stop gave up halfway. That left behind a dead worker that was still registered, with a stop marker saved on disk. The session was then stuck with "Session worker is not connected" until someone repaired it by hand. This is the incident behind #836.

On top of that, the liveness check counted zombie processes as alive, so cleanup could wait forever on a process that was already dead.

Fix

  • If a stop times out, the supervisor now keeps watching in the background: after 5 seconds it force-kills the process, waits for it to die, and then finishes the cleanup it started (archive if requested, remove the registration).
  • Liveness checks now treat zombies as dead (isProcessAlive / new isZombieProcess in shared utils).

What this does not change

Only the timed-out case gets new behavior. If the user retries the worker in the meantime (#836), the retry wins and the cleanup backs off. Normal stops are untouched.

Validation

  • 3 unit tests: cleanup after the process dies, force-kill escalation, backing off after a retry.
  • Zombie detection tested against a real zombie process.
  • A real end-to-end test: suspend a worker, let the stop time out, and watch the supervisor clean it up by itself.
  • npm run check passed.

Part 2 of 3, on top of #850. Next: #852.


Note

High Risk
Changes how the daemon supervisor kills processes and removes worker registrations, including PID recycling and identity edge cases—core session infrastructure with safety-sensitive signaling behavior.

Overview
When a session worker does not exit within the stop deadline, the supervisor no longer leaves a tombstoned registration stranded forever. It schedules background finalization that keeps polling (with throttled identity checks), escalates to SIGKILL after a grace period, and retries archive/cleanup until the registration is removed or the stop is rescinded.

Worker stops are bound to the process generation (pid + processStartId) for the whole stop path: signals go out only when identity is current, recycled pids are treated as gone, and transient identity failures are treated conservatively so live workers are not cleaned up or unrelated processes are not killed. Relaunches and rescinded tombstones abort in-flight stop cleanup.

Shared liveness helpers in child-process.ts split cheap kill(0) existence from zombie-aware isProcessAlive (zombies count as dead). Adoption of an intentional stop no longer SIGKILLs before the normal stop flow.

Daemon shutdown treats WorkerStopTimeoutError as non-fatal so shutdown can finish while tombstoned workers are finalized in the background.

Reviewed by Cursor Bugbot for commit 0b0c43c. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Finalize timed-out worker stops via background SIGKILL instead of stranding registrations

  • Timed-out worker stops now schedule background finalization (scheduleWorkerStopFinalization) that periodically checks process liveness, escalates to SIGKILL once identity is confirmed, and retries cleanup until the registration is removed.
  • Adds process identity tracking (processStartId) to avoid signalling recycled PIDs — SIGKILL is only sent when the PID's start ID matches the stopped worker's recorded identity.
  • Introduces effectiveWorkerState so disconnected "ready" workers are reported as "recovering" and tombstoned workers as "stopping", with handleList returning honest states for busy-daemon checks.
  • Bumps DAEMON_SCHEMA_REVISION to 16 and adds "stopping" to the DaemonWorkerLifecycle and SessionSummary unions to surface the new state to clients.
  • Shutdown now catches WorkerStopTimeoutError and leaves tombstoned workers for recovery rather than failing the shutdown sequence.
  • Risk: Workers that time out during stop are now cleaned up asynchronously; if the supervisor restarts before finalization completes, recovery relies on the tombstone left in the descriptor.

Macroscope summarized da89f2d.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
@snimu

snimu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in a096b3d. The finalizer now snapshots pid, processStartId, and stopRevision at schedule time and rechecks the stop generation inside the loop before every poll and before SIGKILL escalation, so a rescinded/retried stop aborts it immediately and a relaunched worker's new pid is never followed. A pid whose observed processStartId no longer matches is treated as gone and never signalled; stopWorker's signalling is identity-aware too. Added regression tests for the retry-relaunch race and the recycled-pid case.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
@snimu

snimu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the three remaining findings in 62a5265:

  • Fail-closed identity: with a recorded processStartId, an unobservable identity now counts as a different process, so a recycled pid is never signalled even when /proc/ps observation fails.
  • Missing start id: when a worker never had a recorded identity, the finalizer records one at schedule time (the process was provably alive moments earlier), so the eventual stopWorker is identity-guarded too.
  • Transient cleanup failure: finalization now retries (5s backoff) until the registration is gone or the stop is rescinded, instead of stranding the tombstone permanently.
  • Polling cost: every poll now uses a cheap kill(0) existence probe; the ps-backed zombie/identity checks are throttled to once per 500ms in both the stop wait loop and the finalizer.

Regression tests added for the transient-failure retry; existing recycled-pid and retry-relaunch tests updated for the new probe.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
@snimu

snimu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in ee9e59d (and a6e481d on #852). The identity check is now directional instead of a single boolean:

  • workerProcessIdentity() returns current / replaced / gone / unknown.
  • Signalling (SIGTERM/SIGKILL) requires confirmed current — a recycled pid is never signalled.
  • Cleanup (registration removal, archive, reclaim) requires confirmed gone/replaced — an unknown verdict from a transient getProcessStartId failure keeps waiting and never orphans a possibly-live worker.

Regression test added: with a live pid and an unobservable identity, the finalizer neither signals nor cleans up.

@snimu

snimu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 22dbec4. stopWorker's cleanup tail now verifies the registered process is still the one it stopped after every await (including catalog archival): if a retry rescinded the stop and relaunched the worker mid-await, the stale invocation aborts with an error instead of deleting the successor's registration and descriptor. Regression test covers the relaunch-during-archival race.

Also merged the latest main (including #836) through the stack.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
@snimu

snimu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 46c8241. The throttled cache is now used only for read-only wait-loop polling; both SIGKILL sites (stopWorker's force escalation and the finalizer) run a fresh, unthrottled identity check immediately before signalling, so a pid recycled inside the 500ms window is never killed. Regression test flips the observed identity between the last throttled poll and the SIGKILL deadline and asserts the signal is withheld.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
@alexanderkjeldaas

Copy link
Copy Markdown

This is causing me endless issues. Thanks for the fix.

@snimu

snimu commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in c864b10. killed is now only set when the SIGKILL was actually sent: a transiently unobservable identity at the deadline skips that attempt but keeps escalation armed, so a later pass that re-verifies the original process still kills it. Regression test covers an identity outage spanning the deadline followed by recovery.

Also merged the latest main through the stack; the schema bump moved to revision 15 because main took revision 14 for the telemetry opt-out.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
@snimu

snimu commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Both fixed in 37345d5:

  • Mid-stop relaunch SIGKILL: all of stopWorker's polling and signalling (SIGTERM gate, throttled wait-loop verdict, fresh pre-SIGKILL check) now bind to entryPid/entryStartId captured at entry instead of reading the mutable worker.descriptor, so a retry that swaps in a successor process mid-stop can never be signalled by the stale stop. The shared helper is now pid-based (processIdentity(pid, startId)), which fix(coding-agent): self-heal stale worker registrations on resume #852's reclaim reuses unchanged.
  • Rescinded-before-relaunch cleanup: the cleanup guard (assertStopStillApplies) now also aborts when a removeDescriptor stop has lost its tombstone (stopRequestedAt cleared), catching a rescission that lands before recoverWorker assigns the successor pid. Regression test covers exactly that window.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
alexzhang13 added a commit that referenced this pull request Aug 11, 2026
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a960653. Configure here.

Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts Outdated
Comment thread packages/coding-agent/src/modes/daemon/daemon-supervisor.ts
alexzhang13
alexzhang13 previously approved these changes Aug 11, 2026
snimu and others added 13 commits August 11, 2026 10:02
…ng registrations

When a worker does not exit within the stop deadline, the supervisor now
keeps watching the process, escalates to SIGKILL, and completes the
interrupted cleanup once the process dies. Process liveness checks also
treat zombie processes as dead so cleanup is not deferred forever.
… generation

The background finalizer now snapshots pid, processStartId, and
stopRevision when scheduled and aborts if the stop is rescinded or the
worker is relaunched, so it can never SIGKILL a retried worker or an
unrelated process that reused the pid. stopWorker signalling is likewise
identity-aware.
- Fail closed when a recorded processStartId cannot be observed, so a
  recycled pid is never signalled even if identity observation fails.
- Record a schedule-time identity for workers that never had one.
- Retry transient finalization cleanup failures instead of stranding the
  dead registration permanently.
- Probe liveness with a cheap kill(0) on every poll and throttle the
  ps-backed zombie/identity checks so wedged workers cannot saturate the
  supervisor event loop.
…gone

stopWorker used the identity check as a liveness predicate, so a
transient getProcessStartId failure could skip signalling and delete the
registration of a still-running worker. Identity verdicts are now
directional: only a confirmed-current pid is signalled, only a
confirmed-gone/replaced pid is cleaned up, and an unknown verdict keeps
waiting.
…ched mid-await

stopWorker can yield during archival while a retry rescinds the stop and
relaunches the worker on the same registration. The cleanup tail now
verifies the registered process is still the one it stopped before
removing the registration or descriptor, so a relaunched worker is never
orphaned by a stale stop invocation.
The throttled identity cache can be up to 500ms old, long enough for a
pid to be recycled. Both SIGKILL sites (stopWorker force escalation and
the stop finalizer) now run a fresh identity check immediately before
signalling; the cache remains only for read-only wait-loop polling.
…ages

A transiently unobservable identity at the escalation deadline now skips
that attempt without marking the kill done, so a later pass that
re-verifies the original process still escalates instead of leaving a
wedged worker registered forever.
…scinded stops

All stopWorker polling and signalling now use the pid and start identity
captured at entry, so a retry relaunching the worker mid-stop can never
be SIGKILLed through the mutable descriptor. The cleanup guard also
aborts when a removeDescriptor stop lost its tombstone, catching a
rescission that lands before the successor pid does.
@snimu
snimu force-pushed the snimu/worker-shutdown-finalize branch from da89f2d to 0b0c43c Compare August 11, 2026 08:02
@snimu
snimu merged commit e9ef577 into main Aug 11, 2026
17 checks passed
@snimu
snimu deleted the snimu/worker-shutdown-finalize branch August 11, 2026 08:13
@snimu snimu mentioned this pull request Aug 11, 2026
9 tasks
sethkarten pushed a commit that referenced this pull request Aug 11, 2026
Patch release. Bug fixes, small UX additions behind existing surfaces, and a
dependency consolidation; no breaking changes, per the no-major-releases
policy.

Contents since v0.7.1:
- #838 in-place queue editing (Alt+Up/Alt+Down browse, Enter/Alt+Enter apply)
  and queue preservation on interrupt
- #850/#851/#852 worker lifecycle truthfulness, timed-out stop finalization,
  and stale-registration self-heal (the "Session worker is not connected"
  family)
- #1226 Down Arrow stays in a nonempty prompt until the cursor reaches the end
- #767 independent expand/collapse for tool calls, a2a messages, and thinking
- #1135 agents view keeps expansion state when leaving and returning
- #647 login URL copy action
- #521 privacy-safe agent analytics with disclosure and opt-out
- #846 Homebrew ownership preserved on self-update
- #772 sent a2a messages show only message text when expanded
- #632 consolidated dependency updates (undici 7.29, biome 2.5.5, marked 18,
  typescript 7 dev-only, typebox 1.3, aws-sdk bedrock, vitest 4.1.10, et al.)
- #1132 stale Gemini test model update (test-only)

Missing changelog entries for #838/#850/#851/#852 are added under 0.7.2.

Lockstep bump across the root package and the four published packages;
example and private workspaces untouched. Lockfile updated surgically
(version fields and inter-package ranges only).
0oAstro pushed a commit to 0oAstro/fulcrum that referenced this pull request Aug 11, 2026
…ng registrations (PrimeIntellect-ai#851)

* fix(coding-agent): finalize timed-out worker stops instead of stranding registrations

When a worker does not exit within the stop deadline, the supervisor now
keeps watching the process, escalates to SIGKILL, and completes the
interrupted cleanup once the process dies. Process liveness checks also
treat zombie processes as dead so cleanup is not deferred forever.

* fix(coding-agent): bind stop finalization to the exact worker process generation

The background finalizer now snapshots pid, processStartId, and
stopRevision when scheduled and aborts if the stop is rescinded or the
worker is relaunched, so it can never SIGKILL a retried worker or an
unrelated process that reused the pid. stopWorker signalling is likewise
identity-aware.

* fix(coding-agent): harden stop finalization identity checks and retries

- Fail closed when a recorded processStartId cannot be observed, so a
  recycled pid is never signalled even if identity observation fails.
- Record a schedule-time identity for workers that never had one.
- Retry transient finalization cleanup failures instead of stranding the
  dead registration permanently.
- Probe liveness with a cheap kill(0) on every poll and throttle the
  ps-backed zombie/identity checks so wedged workers cannot saturate the
  supervisor event loop.

* fix(coding-agent): treat unobservable process identity as alive, not gone

stopWorker used the identity check as a liveness predicate, so a
transient getProcessStartId failure could skip signalling and delete the
registration of a still-running worker. Identity verdicts are now
directional: only a confirmed-current pid is signalled, only a
confirmed-gone/replaced pid is cleaned up, and an unknown verdict keeps
waiting.

* fix(coding-agent): abort stale stop cleanup when the worker is relaunched mid-await

stopWorker can yield during archival while a retry rescinds the stop and
relaunches the worker on the same registration. The cleanup tail now
verifies the registered process is still the one it stopped before
removing the registration or descriptor, so a relaunched worker is never
orphaned by a stale stop invocation.

* fix(coding-agent): re-verify process identity at SIGKILL time

The throttled identity cache can be up to 500ms old, long enough for a
pid to be recycled. Both SIGKILL sites (stopWorker force escalation and
the stop finalizer) now run a fresh identity check immediately before
signalling; the cache remains only for read-only wait-loop polling.

* fix(coding-agent): keep SIGKILL escalation armed through identity outages

A transiently unobservable identity at the escalation deadline now skips
that attempt without marking the kill done, so a later pass that
re-verifies the original process still escalates instead of leaving a
wedged worker registered forever.

* fix(coding-agent): bind stopWorker to its entry process and detect rescinded stops

All stopWorker polling and signalling now use the pid and start identity
captured at entry, so a retry relaunching the worker mid-stop can never
be SIGKILLed through the mutable descriptor. The cleanup guard also
aborts when a removeDescriptor stop lost its tombstone, catching a
rescission that lands before the successor pid does.

* fix(coding-agent): require worker identity before stop escalation

* fix(coding-agent): reject unidentified worker pids

Fixes PrimeIntellect-ai#851

* fix(coding-agent): keep unknown worker identities untrusted

Fixes PrimeIntellect-ai#851

* fix(coding-agent): finalize worker stops during shutdown

Fixes PrimeIntellect-ai#851

* fix(coding-agent): bound worker finalization during shutdown

---------

Co-authored-by: Alex Zhang <alex.lx.zhang@gmail.com>
0oAstro pushed a commit to 0oAstro/fulcrum that referenced this pull request Aug 11, 2026
Patch release. Bug fixes, small UX additions behind existing surfaces, and a
dependency consolidation; no breaking changes, per the no-major-releases
policy.

Contents since v0.7.1:
- PrimeIntellect-ai#838 in-place queue editing (Alt+Up/Alt+Down browse, Enter/Alt+Enter apply)
  and queue preservation on interrupt
- PrimeIntellect-ai#850/PrimeIntellect-ai#851/PrimeIntellect-ai#852 worker lifecycle truthfulness, timed-out stop finalization,
  and stale-registration self-heal (the "Session worker is not connected"
  family)
- PrimeIntellect-ai#1226 Down Arrow stays in a nonempty prompt until the cursor reaches the end
- PrimeIntellect-ai#767 independent expand/collapse for tool calls, a2a messages, and thinking
- PrimeIntellect-ai#1135 agents view keeps expansion state when leaving and returning
- PrimeIntellect-ai#647 login URL copy action
- PrimeIntellect-ai#521 privacy-safe agent analytics with disclosure and opt-out
- PrimeIntellect-ai#846 Homebrew ownership preserved on self-update
- PrimeIntellect-ai#772 sent a2a messages show only message text when expanded
- PrimeIntellect-ai#632 consolidated dependency updates (undici 7.29, biome 2.5.5, marked 18,
  typescript 7 dev-only, typebox 1.3, aws-sdk bedrock, vitest 4.1.10, et al.)
- PrimeIntellect-ai#1132 stale Gemini test model update (test-only)

Missing changelog entries for PrimeIntellect-ai#838/PrimeIntellect-ai#850/PrimeIntellect-ai#851/PrimeIntellect-ai#852 are added under 0.7.2.

Lockstep bump across the root package and the four published packages;
example and private workspaces untouched. Lockfile updated surgically
(version fields and inter-package ranges only).
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.

4 participants