Repository navigation
fix: stop Claude and agent helper processes when Orca quits or crashes (STA-9254, 1 of 2) - #25715
Merged
Merged
Conversation
…d relay a provider's last output A one-shot CLI (claude -p, codex exec -) reads its request until stdin ends, so the supervisor's session rule (stdin end means the owner is closing) would stop it about 1 s into its answer. The one-shot lifetime passes stdin end through and leaves only the owner-death watch and explicit signals to stop it. The supervisor also exited as soon as its provider did, dropping output still in the pipes when the owner reads slowly. It now waits, bounded, for the provider's output to be relayed before exiting.
…or on POSIX The Claude model-list probe, model discovery for every agent, and commit message, pull request and branch name generation all spawned the agent CLI as a plain child of Orca. If Orca quit, crashed or was killed while one was running, nothing stopped it, and a CLI that hung kept running after Orca was gone. They now run under the provider supervisor that native chat already uses, in its one-shot lifetime, so Orca's exit stops the agent's whole process group however Orca exits. A timeout or cancel asks the supervisor to stop (SIGTERM) and only tears the tree down once it has had its full stop time; killing the supervisor first would orphan the agent's group. A missing binary now reaches Orca as the supervisor's exit 127, which is mapped back to the existing not-found message. Windows and WSL keep spawning the agent directly.
…p every spawn error back, and share one stop ladder - The supervisor read the provider's command and arguments from one base64 JSON env string. Linux caps a single env string at 128 KiB, so an argv prompt of about 90-120 KiB, which passes the 120 KiB per-argument guard, failed execve with E2BIG. The provider argv now follows the supervisor script's '--' as real arguments; the env keeps only small fields. - A supervisor that cannot start its provider reports Node's spawn error line and exits 127. That line now becomes the same error a direct spawn emits, so ENOENT still reads as 'not found on PATH' and EACCES or any other spawn error reads as 'failed to start'. - stopSupervisedProvider is the one ask, wait and force ladder: it gives a supervisor its full stop time before forcing. Agent one-shots, the Codex app-server close and the Claude child exit proof now share it, with the same requests, bounds and forced steps as before.
…derr line Node throws most spawn failures (ENOEXEC, ENOTDIR, ELOOP, EPERM, ...) instead of emitting them, and the supervisor had no catch, so it died with exit 1 and a stack trace that the user saw as the agent's failure. A thrown or emitted spawn failure now exits 127 with one marked line carrying whether it was thrown, its code and its message. Orca reads only the last stderr line, so a runtime warning printed earlier cannot hide it, and maps it to the message a direct spawn gave: thrown is 'could not be started', ENOENT is 'not found on PATH', and any other emitted error is 'failed to start'.
…r and hand them to the provider The supervisor runs Electron in Node mode, which honours NODE_OPTIONS and NODE_REPL_EXTERNAL_MODULE. A user value such as a --require of a missing file stopped the supervisor from starting, breaking even native CLIs like Codex that never load it. The launch now takes both out of the supervisor's environment, carries them in its spec, and restores them for the provider only, so a Node-based CLI still gets them.
… own breadcrumb site The forced tree teardown recorded every self-initiated kill as the Codex app-server's, so a forced commit-message or model-discovery stop read as a Codex teardown in crash breadcrumbs. The teardown now takes the caller's site; source-control stops pass the site their Windows tree kill already uses.
…tput limit; name the direct-child suites The commit-message suites that drive fake children spawn them directly, the unsupervised shape Windows and WSL use, so they now say so. The supervised POSIX stop gets its own compositions: a timed-out Codex generation settles at once but holds the Codex home until its supervisor has stopped (faithful fake, fake timers), and an agent that floods past the output limit is stopped through its real supervisor with no process left behind.
…h and cover supervised discovery The commit-message suites that drive fake children mocked the supervisor away on POSIX, so they asserted a direct root SIGKILL that production no longer takes there. They now pin the platform to Windows (with an empty PATH, so host installs cannot answer a bare agent name) and assert the Windows kill, taskkill included. The three tests that check the host's own discovery spawn shape run on the host and read the agent argv past the supervisor's '--'. Model discovery gets its supervised composition: a timed-out Codex discovery settles at once but holds the Codex home until its supervisor has stopped.
…e spawn error it was Native chat's exit errors carry the provider's stderr tail into Details. Under the supervisor a missing CLI left the supervisor's internal spawn-failure report there instead of Node's own 'spawn <cmd> ENOENT'. The report, its parser and a display formatter now live in one module; the Codex app-server and Claude stream-json exit errors pass the tail through the formatter, which turns a report back into the spawn error and leaves any other stderr unchanged.
…crashing on its missing pipes When the provider spawn fails outright (EMFILE, ENFILE), Node emits 'error' later and leaves the child with no pid and no stdio. Piping stdin into the missing pipe threw first, so the supervisor died with exit 1 and a stack trace and never wrote its spawn-failure report. The pipes are now wired only for a provider that started.
Agent one-shots stop their supervisor with SIGTERM through the shared stop ladder, watching the child's own exit. That adapter moves into one helper beside the ladder, so other supervised children can use it with their own stop request instead of a copy. The caller's breadcrumb site still reaches the forced teardown. Same request, wait and force as before.
… its owner dies The owner-death watch went straight to the group SIGTERM and cancelled any stdin-end grace, so when Orca quit or crashed a session provider such as the Codex app-server never saw the EOF that lets it finish writing its state (auth.json, the state database). A session whose owner is gone now closes as an owner's stdin end does: the provider's stdin is ended, it gets the stdin-end grace, and only then the SIGTERM and SIGKILL ladder. A one-shot already had its EOF at the end of its request, so its owner's death still stops it at once.
…r exits on a stop A requested stop waited out the whole SIGTERM grace for the provider's group even after the provider itself had exited, so a SIGTERM-ignoring helper it left behind held every stop for up to 3 s. Under a stop, the rest of the group is now SIGKILLed as soon as the provider has exited, the same rule its own exit already follows.
…tput limit Model discovery's supervised stop was covered only under timeout. A Codex discovery that floods past the output limit now runs through a real supervisor: it settles with the too-much-data error, its agent is stopped through the supervisor, and the next discovery on the same Codex home starts only after that agent is gone. The direct-child suites' headers now list exactly the supervised cases that are covered.
…er closes it Owner death gave every session provider the stdin-end grace, so after an Orca crash a Claude session, whose close is a stdin end plus SIGTERM, could keep working on its turn for a second with nobody watching. The spawn spec now names the provider's close request: 'stdin-end' (the Codex app-server drains and exits on EOF, then gets its grace) or 'stdin-end-and-sigterm' (Claude; the default). A gone owner gets that same request. One constant per provider feeds both its spawn spec and its owner-side close, through one requestProviderClose, so the two cannot drift. One-shots still stop at once.
…ts provider exits Killing the rest of the group the moment the provider exited under a stop also reached native chat's closes, so an MCP server, a tool's child or a dev server still in Claude's or Codex's group was SIGKILLed mid-cleanup instead of getting the rest of the SIGTERM grace. The early group kill now applies only to one-shots, where the saved wait was the point; a session's stop is back to waiting out the grace for its group.
…itly The spawn-spec assertion matched the default close request, so dropping Claude's explicit request still passed. The test now checks that the spec is built with the exit-proof ladder's own constant.
Other Codex app-server spawns will name the same close request as the connection does. Holding it in its own small module lets them import it without the connection itself.
Main's #24989 moved the provider supervisor and its teardown into src/main/provider-process/. PR1's changes move with them: the one-shot lifetime, the provider argv on the supervisor's argv, the spawn-failure report and its formatter, the no-pid guard, close-request mirroring, the one-shot-only leader-exit reap and the shared stops. Main's mechanisms replace PR1's where they overlap: the teardown takes the caller's required site label (PR1's optional site is dropped), and the provider's env is resolved once by resolveProviderChildEnv before the supervisor keeps the Node options for the provider alone.
…in the CLI project The command-line build runs the short-lived Codex app-server session, which will name the same close request as the Codex connection. Listing the close request, the provider supervisor it takes its type from, and the spawn-failure report the supervisor uses lets the CLI project typecheck that import without pulling the connection in.
Merged
8 of 9 tasks
…e reports one On Linux, glibc's execvp hands an executable that is not a program to /bin/sh, so both a direct and a supervised spawn run it and it exits 127; only macOS throws ENOEXEC. The not-a-program case now runs on macOS only; the path-through-a-file case (ENOTDIR) still covers a thrown start failure everywhere.
Main's #25204 moved every structured provider's close onto one shared close (closeProviderProcess, driven by a close policy) and spawn (spawnManagedProviderProcess). That is the same concept as PR1's stopSupervisedProvider and requestProviderClose, so PR1's copies are removed and its callers move onto main's: - The Codex connection and the Claude spawn and exit proof take main's versions. - The close request a gone owner gets is derived from the provider's close policy inside spawnManagedProviderProcess (signalSupervisorOnClose -> 'stdin-end-and-sigterm', else 'stdin-end'), so one source drives both the owner's close and the supervisor's. - stopSupervisedChildProcess (agent one-shots) runs main's closeProviderProcess with the root-only policy and the caller's close request. - The Codex close-request constant and its CLI build entries go: nothing uses them now.
brennanb2025
marked this pull request as ready for review
October 6, 2026 08:49
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (24)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe POSIX provider supervisor now supports session and one-shot lifetimes, configurable close requests, provider-only Node environment settings, and structured spawn-failure reports. Non-Windows source-control agents now launch through the supervisor and use supervised shutdown. Claude and Codex diagnostics format reported spawn failures for display, and local model discovery and source-control processing route those failures to existing error handling. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to This change supervises one-shot agent CLI processes on POSIX so they stop when Orca quits or crashes. No concrete merge-blocking issue was identified. Linux behavior has not been verified by the author, which is worth a check in normal QA. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
brennanb2025
marked this pull request as draft
October 6, 2026 09:05
Contributor
Author
Status: reviewed, live-QA'd, CI greenThe problem. When Orca quit or crashed while it was briefly running an agent CLI in the background (the Claude model list, model discovery, or commit-message / PR / branch-name generation), nothing stopped that CLI. A CLI that hung kept running after Orca was gone, together with anything it had started. What changes for you. On macOS and Linux those CLIs, and their children, now stop when Orca quits, crashes or is force-killed. Error messages are unchanged. Windows and WSL behave as before (STA-9579). This is step 1 of 2; step 2 (#25753) covers the short-lived Codex servers and the OpenCode launch preflight. Review rounds
Each fix was ablated: undoing it makes its test fail. Live QA (macOS, isolated rig, stand-in CLI, driven by Grok)
The real CIHead Not verified
|
brennanb2025
marked this pull request as ready for review
October 6, 2026 10:21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Orca sometimes runs an agent's command-line tool (the CLI) briefly in the background: to ask Claude for its model list, to list another agent's models, or to write a commit message, pull request text or a branch name. If Orca quit or crashed while one of those was running, nothing stopped it. A CLI that hung kept running after Orca was gone.
Now each of those runs under a supervisor: a small watcher process that stops the CLI when Orca goes away. That covers a normal quit, a crash, and Orca being force-killed alike.
What Changed
Before:
claude -p …briefly. Model discovery for other agents and commit-message / PR / branch-name generation run an agent CLI the same way.claudethat ignores end of input, every launch left one process behind.After (macOS and Linux):
The plan, and which step this is:
Mechanism:
src/main/provider-process/provider-process-supervisor.ts). Native chat already runs Claude and Codex under it. It starts the CLI in its own process group, checks every 100 ms that Orca is still its parent, and stops the whole group when Orca is gone. "Stopping the group" means SIGTERM, a grace period, then SIGKILL, and the supervisor exits only once the group is gone.closeProviderProcess, driven by each provider's close policy, and spawns them throughspawnManagedProviderProcess. This PR reuses that and adds no second stop sequence. What it adds:spawnSourceControlAgent) runs every POSIX CLI under the supervisor in one-shot mode. Its stop (stopSupervisedChildProcess) is main'scloseProviderProcesswith the root-only policy: ask the supervisor (SIGTERM), wait its full stop time, and only then force the process tree. Forcing it sooner would kill the supervisor and orphan the CLI's group. After a forced teardown, main's 1 s forced-exit grace for the CLI's own exit now applies to these one-shots too.closeRequestin its spec).spawnManagedProviderProcessderives it from the provider's close policy, so a provider's own close and its owner-death close come from one source:spawn <cmd> ENOENTtext, not the marker.NODE_OPTIONSno longer reach the supervisor itself; they are handed to the CLI. Before, a bad value could stop even a non-Node CLI such as Codex. The CLI's environment is still resolved once, by the sharedresolveProviderChildEnv; the supervisor only keeps these two Node startup keys out of its own.Why
The failure is a missing owner: nothing stops the CLI when Orca goes away, however it goes away. The supervisor already gives native chat exactly that guarantee in production, so this reuses it rather than adding a second mechanism.
Alternatives considered:
Differences from the common pattern:
NODE_OPTIONSkept out of the supervisor. Its stops are otherwise unchanged: a session's group keeps the full SIGTERM grace after the CLI exits. The Codex connection and Claude suites stay green.--versionand status one-shots (STA-9577), and the Claude account and login commands (STA-9578).Cost:
Linked Issue
Part of STA-9254 (Linear): step 1 of 2. Step 2 is the stacked PR for the Codex app-server sessions, Codex backfill recovery and the OpenCode launch preflight.
Visual Proof
Nothing on screen changes; the change is which processes are still running after Orca exits. These are the recorded process checks from the live QA run (macOS, isolated rig, a stand-in
claudethat ignores end of input and starts asleepgrandchild, the Claude model-list probe in flight). Liveness was read per pid with start-time checks to rule out pid reuse.Before (
main17ecee6): the hung probe and its grandchild outlive Orca.After a normal quit, both are still alive 20.8 s later, reparented to pid 1.
After Orca is killed, both are still alive when the 20 s watch ends.
After (this PR): everything is gone in under half a second.
After a normal quit, the stand-in, its grandchild and the supervisor are gone 0.48 s after the quit.
After Orca is killed, the stand-in, its grandchild and the supervisor are gone 0.37-0.38 s after the kill.
Testing
Live QA (macOS, on ed5201b). A rig with an isolated profile and a stand-in
claudethat ignores end of input. Process ids were recorded and checked by pid. The later commits don't change one-shot quit or crash behaviour.Automated (macOS).
NODE_OPTIONSreach only the CLI;Not verified:
The Linux runtime. The E2BIG fix is covered only by a unit test of the argument and environment sizes, because macOS has no per-string limit.
The real Claude, Codex and OpenCode CLIs. Every test used stand-ins.
Windows CI is pending.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)