[brennanb2025]

	Files	Added	Deleted	Net
Test	5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​452	$\color{#cf222e}{\Huge{\mathbf{−}}}$​9	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​443
Prod	5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​101	$\color{#cf222e}{\Huge{\mathbf{−}}}$​28	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​73

ELI5

On macOS and Linux, Orca runs each Codex chat under a small watcher process. Orca remembers the watcher's process ID and later judges whether the chat's agent is still running by checking whether that ID is alive. Today, if something politely asks the watcher to stop, it dies on the spot and leaves the real Codex process running. Orca then sees the watcher gone, concludes Codex is gone too, and lets a second Codex attach to the same conversation while the first one is still writing to it. This change makes the watcher stop Codex first and only then exit, so "watcher gone" really means "Codex gone".

What Changed

In the code the watcher is the POSIX provider supervisor, an inline Node script, and the process it runs (Codex) is the provider. The provider runs in its own process group, so the supervisor can signal it and the processes it starts together. The supervisor's own behaviour changes in seven ways, and two callers now wait long enough for it.

The supervisor

• One stop. Every way the supervisor stops takes the same path: SIGTERM the provider's process group, SIGKILL it if it is still there 3 s later (the time it gets to save its state and exit), and exit only once the group is gone. The three triggers are:
  • its owner (the Orca process that launched it) dies, seen by the parent-death watch;
  • stdin ends and the provider has not exited within 1 s;
  • the supervisor itself receives SIGTERM, SIGINT or SIGHUP.
• Owner death. Before, owner death sent an immediate SIGKILL. Now it too gives the provider a SIGTERM and 3 s to flush. That costs at most 3 s more.
• Signals. Before, the supervisor had no signal handlers, so the default action killed it at once and left the separate provider group running. Now, when a signal stopped it, it dies by that same signal after the group is gone, so anything watching it sees the same outcome as before, only later.
• Handlers before the spawn. The signal handlers are registered before the provider is spawned. A stop that lands in the instant after the spawn therefore still stops the provider, instead of killing the supervisor by default action with the provider already running.
• Owner. The spawn spec now carries the pid of the Orca process that launched the supervisor. Before it spawns anything, the supervisor checks that its parent is that pid; if not, it exits without spawning. The parent-death watch compares against the same pid. Before, it compared against whatever parent it saw after it had already spawned the provider.
• Timings. The stop is now: stdin end, 1 s for the provider to exit on its own, SIGTERM, 3 s to save its state and exit, SIGKILL, then at most 1.5 s for the kernel to finish the kill. Before, both waits were 1.25 s. They are two separate constants (PROVIDER_STDIN_END_GRACE_MS, PROVIDER_SIGTERM_GRACE_MS) and two spec fields instead of one hard-coded number. Codex keeps the defaults, and building a spec refuses either wait above its default, so no launch can carry a wait that recovery and close do not wait out.
• Closed output pipe. When Orca dies, the supervisor's stdout pipe has no reader. Provider output in the moment before the parent-death watch fired raised an unhandled EPIPE that killed the supervisor with the provider group still running. The supervisor now ignores errors on its own stdout and stderr, as it already did on its stdin and the provider's pipes, so the watch still stops the group first.

The callers that wait for it

• Recovery timing. Each chat has an owner record that names the one agent process allowed to write to its conversation; for Codex on POSIX that is the supervisor's pid. When a restarted Orca finds that process still alive while recovering the chat, it sends SIGTERM, checks whether the process is gone, then sends SIGKILL. It used to send SIGKILL about 1 s after SIGTERM, which is before the supervisor's own stop can finish. SIGKILL cannot be handled, so it killed the supervisor mid-stop and recreated the original bug. The SIGTERM stage now lasts longer than the longest stop any supervisor can make, PROVIDER_SUPERVISOR_MAX_STOP_MS: stdin end, 1 s, SIGTERM, 3 s, SIGKILL, then at most 1.5 s for the group to disappear. That is 1000 + 3000 + 1500 = 5500 ms, so recovery keeps checking for about 5.75 s before SIGKILL. It is derived from that maximum in code, so raising the maximum widens the stage with it, and a longer wait fails at spec construction instead of silently reopening the gap. Recovery returns as soon as the owner is proven gone, so it only waits that long when a provider ignores SIGTERM.
• Close timing. close() ends the supervisor's stdin, then forces the process tree if it has not exited. On POSIX it used to force after 1.5 s. Now it waits PROVIDER_SUPERVISOR_MAX_STOP_MS (5.5 s), and Windows keeps 1.5 s. Under SIGTERM-first stopping, the 1.5 s force landed mid-stop:
  • the provider had already died of SIGTERM;
  • its SIGTERM-ignoring child process had already been handed to a new parent, outside the supervisor's process tree, so the list of processes the force captures before killing (the descendant snapshot) missed it;
  • the force then killed the supervisor before its SIGKILL.

The 40-launch teardown test caught this: all 40 child processes survived. A Codex that exits on stdin end still closes as fast as before (about 5 ms). Only a provider that ignores stdin end waits for the supervisor's own stop.

Quit budget. On quit, Orca gives the shutdown of all chats 8 s (CHILD_EVICTION_TIMEOUT_MS in structured-agent-session-host-teardown.ts; chats shut down in parallel). Before stopping each agent it waits up to 1 s for already-delivered events to be saved, so the worst case is 1 s plus the 5.5 s longest stop, which leaves 1.5 s to spare. A test fails if those two plus 1 s of headroom reach 8 s.

How a second Codex got in, before: recovery SIGTERMs the recorded pid, which is the supervisor → the supervisor dies by default action → the next check finds the pid gone → recovery treats the owner as proven dead and clears it → a new Codex is made the owner while the old Codex app-server is still running on the same thread.

After: the SIGTERM reaches the supervisor's handler, which was registered before the provider existed → the provider group gets SIGTERM, then SIGKILL 3 s later if anything in it is still running → the supervisor exits only once the group is empty → the next check finds the pid gone, and that is now true of the provider too → clearing the owner is sound.

The window before the owner was known. The provider used to be spawned before the supervisor read its parent's pid. If Orca died in that gap, the supervisor recorded its new parent (PID 1, or whichever process the system hands orphaned processes to) as its owner, and the parent-death watch never fired.

That gap is narrower than it sounds. On main, Orca's death already closes the supervisor's stdin, and the stdin-end path stops the provider group after a short wait, even inside the window. A provider is stranded only if some other process holds the write end of that pipe open. This PR makes the protection direct instead of relying on that: the supervisor checks its parent against the owner pid from the spec before it spawns anything, so a supervisor whose parent is already not Orca never starts a provider.

Why

Everything that judges Codex by its recorded pid (the owner check, recovery, the exit relay) assumed the watcher dies only after Codex does. The watcher broke that by dying on any polite stop, which let recovery hand a live conversation to a second Codex. Making the watcher stop Codex first restores that guarantee in one process, so no reader needs to learn a new field.

Put as one rule: while any member of the provider group is alive, the supervisor pid is alive. The alternatives each add a guard somewhere else instead:

• Record the provider's pid or group alongside the supervisor's. That is a new saved field on the owner record, and every reader of the owner's identity would need to learn about it.
• Have recovery signal the provider group directly. Recovery would need the provider's process group ID, which nothing records.
• Scan for processes carrying the spawn tag (an environment variable Orca sets on Codex). Every process Codex starts inherits that tag, so it cannot tell Codex from the programs Codex started; fix(native-chat): stop killing processes that only inherited a chat's spawn tag #23460 removes the startup sweep that killed processes this way.

The pre-spawn check follows the same rule: re-derive the owner from a fact the spawner states, rather than trust whatever the supervisor happened to observe after the spawn. No field is persisted. The spec lives only in the supervisor's environment, and the script and the spec are always built by the same build, so no version skew is possible.

What SIGKILL to the supervisor still cannot cover: an uncatchable SIGKILL sent before its stop finishes. Orca's own senders are covered. Recovery sends SIGTERM first and waits out the supervisor's longest stop. The Codex connection's close waits the same bound before its forced teardown. That teardown freezes the supervisor, kills the captured child processes by identity, and only then kills the supervisor. One pre-existing exception is unchanged here: if the descendant snapshot cannot be taken, teardown SIGKILLs the supervisor directly (codex-app-server-process-teardown.ts:117-119). A SIGKILL from outside Orca (a user's kill -9) still orphans the group. The stdin-close backstop cannot help there either, because the provider's stdin belongs to the dead supervisor.

Differences from the common pattern

The usual way to build a watcher like this: pass SIGINT or SIGTERM on to the program it runs, SIGKILL the program if it has not exited after a grace period, and exit only once the program and everything it started are gone. This PR does the same. Where it differs:

• The watcher stands outside the group it stops. Codex and every program it starts form one "process group": a set of processes the operating system lets you signal all at once, and ask whether any member is still alive. The watcher is deliberately not a member, so it can keep asking "is anything in Codex's group still running?" and exit only once the answer is no. In the usual setup the watcher is in the group itself, so that question always answers yes while the watcher runs.
• The same stop, whatever the reason. The usual watcher passes on whichever signal it received. Here, whatever stops the watcher (Orca sending it a signal, Orca dying, or Codex's input closing), Codex's group always gets SIGTERM first. When a signal stopped the watcher, the watcher then dies of that same signal, so whoever sent it sees the result they expect, only after Codex is gone.
• Timings. After Codex's input closes, Codex gets 1 s to exit on its own, then SIGTERM, then 3 s to save its state and exit before SIGKILL. That now matches comparable desktop apps. An earlier revision used 1.25 s for both waits, the shortest of them, which gave Codex the least time to write out its state.
• A limit on the wait after SIGKILL. The usual watcher waits for the group to empty with no limit. Here it waits at most 1.5 s after SIGKILL and then exits anyway, with exit code 1. A killed process can never run again, so this wait only covers the operating system finishing the kill; ending it early cannot let a second Codex in. Waiting forever could hang Orca's close or recovery on a process stuck inside the operating system, for example one blocked on a hung network drive.
• Checks the usual watcher lacks. The watcher is told which Orca process owns it and refuses to start Codex if that process is not its parent. It sets up its signal handling before starting Codex, so a stop that arrives in that first instant still works. It also handles SIGHUP, the signal sent when a terminal closes.

Linked Issue

N/A (maintainer follow-up to #23460, which removes the startup sweep that killed processes by their inherited spawn tag).

Visual Proof

This is a process-lifetime change with no UI change. The evidence is timings from a live run on a macOS machine: an isolated build and profile, a structured Codex chat running a 90 s loop, a watcher polling every 10 ms, and a second watcher inside the provider group that logs the signal it receives. Base = 9795563695, PR = 3b374050c6. The build was confirmed per run from the built bundle.

Scenario	Base	This PR
Stop the recorded supervisor pid with Orca running (what recovery does), provider ignoring stdin end	Supervisor dead at +13 ms; provider and its tool still alive after 30 s while Orca started a second provider for the same chat (two writers)	Group gets SIGTERM, gone at +21 ms; supervisor exits after it (+33 ms); the turn reads interrupted and the next send is answered
kill -9 of Orca's main mid-tool, provider ignoring stdin end	SIGKILL at +78 ms; the tool group lingered until +897 ms	SIGTERM at +66 ms; the group is gone by +81 ms
kill -9 of Orca's main mid-tool, stock Codex	The supervisor died before its group (+36 ms vs +195 ms)	The group is gone (+36 ms) before the supervisor (+73 ms)
Clean quit with two Codex chats (idle / mid-tool / provider ignoring stdin end)	170 / 201 / 1439 ms	190 / 153 / 1390 ms (measured with the earlier 1.25 s stdin-end wait; that wait is now 1 s, see the timing re-run under Testing)

After a crash and relaunch, and after a clean restart, a send is answered.

Testing

Real-process tests (macOS). Each new test was confirmed red with its core line deleted, then restored from the commit.

• codex-app-server-posix-supervisor.integration.test.ts
  • SIGTERM to the supervisor: the provider and a SIGTERM-ignoring grandchild are gone, the group is already empty when the supervisor's exit is observed, and the supervisor dies by SIGTERM. Deleting the handler line makes the supervisor exit at once while the group lives.
  • A short SIGTERM wait (200 ms) from the spec SIGKILLs a SIGTERM-ignoring provider in under the 3 s default. Deleting the handler makes it exit in about 4 ms with the provider alive. Deleting the SIGTERM wait from the spec makes the stop never finish.
  • An owner pid that is not the parent: the supervisor exits 1 and never runs the provider (a touch marker is never created). Deleting the check makes the provider run.
  • The owner is SIGKILLed while another process holds the supervisor's stdin open. The group and the supervisor are gone within 3 s, but not before the 300 ms SIGTERM wait, because the SIGTERM-ignoring grandchild lasts until SIGKILL.
    • Deleting the parent-death watch makes the group survive.
    • Restoring the immediate SIGKILL on owner death turns this test red: the group is gone after about 104 ms, before the 300 ms wait.
  • Owner death with a provider that records the signal it receives: the provider saw SIGTERM. With the immediate SIGKILL restored, it saw nothing (red).
  • A stop that lands in the spawn window: a --require preload wraps spawn to SIGTERM the supervisor the moment the provider exists. The supervisor dies by SIGTERM and the provider is gone. With the handler registration moved back after the spawn, the provider survives (red).
  • With the real defaults, a provider that ignores both stdin end and SIGTERM gets SIGTERM no sooner than 1 s after stdin end and lives at least 3 s more before SIGKILL. With both waits set back to 1.25 s it is red (SIGTERM to exit 1292 ms).
  • A provider that exits on stdin end closes with exit code 0 in under the 1 s wait, so the fast path adds no wait.
  • (in codex-app-server-posix-supervisor.test.ts) Either wait one millisecond above its default is refused; the default itself is accepted. Deleting the bound check turns this test red.
  • The same owner death while the provider streams output: the group is still stopped. Without the stdout/stderr error handlers the supervisor dies of EPIPE first and the provider survives (red 3 of 3, green 3 of 3).
• structured-agent-session-recovery-resolution.test.ts: recovery runs against a real supervisor whose provider ignores SIGTERM, and clears the owner on proof of death only after the provider is gone. With main's recovery timing restored, the provider is still alive when the owner is cleared. Deleting the SIGTERM handler has the same result.
• structured-agent-session-host-teardown.test.ts: the 1 s wait before the stop, plus the longest stop, plus 1 s, stays under quit's 8 s (7500 ms). Raising the SIGTERM wait to 6 s turns it red (9500 vs 8000), and so does raising the 1 s wait to 2 s (8500 vs 8000).
• Timing re-run on macOS with real processes, 5 runs each, times from stdin end (median, ms). Before = 3b374050c6 (1.25 s and 1.25 s), after = this head.

Provider	SIGTERM sent	Group gone	Supervisor exits
Exits on stdin end, before	none	5	6
Exits on stdin end, after	none	≤ 4	4
Exits on SIGTERM, before	1252	1272	1280
Exits on SIGTERM, after	1001	1006	1030
Ignores SIGTERM (with a SIGTERM-ignoring child), before	1251	2523	2546
Ignores SIGTERM (with a SIGTERM-ignoring child), after	1001	4019	4040

• Existing suites pass: src/main/codex, src/main/native-chat/agent-session-wire, and src/shared/child-process, including the child-process import ratchet and the 40-iteration teardown integration tests (about 8 to 9 s each now that each close can wait 5.5 s).

  • With close() back at its 1.5 s force, that teardown test goes red: all 40 SIGTERM-ignoring child processes survive.
  • The connection unit tests' fake-timer advances now use the same close bound. Two timing tests failed under full-suite load (load average about 120) and pass in isolation. They pass or fail the same way with main's recovery file.
  • Re-run at this head: src/main/codex and src/shared/child-process 1743 pass; src/main/native-chat/agent-session-wire 821 pass. One lease-renewal test that never starts a supervisor (structured-agent-session-lease-renewer.test.ts) failed once under load average about 35 and passed 3 of 3 on its own.

• pnpm tc:node, pnpm tc:web, oxlint on the changed files, check:code-quality:changed and audit:anti-slop all pass.

• Not run: Linux (the tests are POSIX and not macOS-specific, and CI covers Linux), and a live Codex app-server stop. Windows is unaffected, because it does not use the supervisor.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

AI Disclosure

Written with AI assistance (Claude), and reviewed and validated by the author.

Review

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

• SSH / remote: the supervisor runs on whichever host spawns Codex, and the owner pid is that spawner's own pid, so remote hosts behave the same way. There is no wire change.
• Windows: unchanged. There is no supervisor on win32.
• Mixed versions: the script and its spec always come from the same build, and nothing is persisted or sent over the wire. One skew remains: after an update, a new build recovering a session whose supervisor an older build launched is signalling a supervisor without handlers, so that one recovery still behaves as before this change. The longer SIGTERM stage does not change how it ends: the first check after SIGTERM finds the old supervisor gone.
• The waits, recovery and quit stay coupled in code: recovery's SIGTERM stage and close are derived from the longest stop, a spec above either default wait is refused, and a test holds the longest stop under quit's 8 s. A later change that lengthens a wait (for example for Claude) widens recovery and close with it, or fails a test, rather than reopening the SIGKILL-mid-stop gap.
• If the supervisor ever overruns its own 5.5 s: close then forces the process tree, which can take up to about 5.5 s more (1 s process snapshot, 3.5 s kill check, 1 s exit wait), so that chat's stop can pass quit's 8 s. This PR introduces that case: on main, close forced the tree after 1.5 s, which stayed under 8 s but killed the supervisor mid-stop (the bug above). An earlier revision of this PR had the same overrun (4 s + 5.5 s). The supervisor only overruns if the operating system cannot finish a kill. If it happens, quit still finishes, and that chat is left marked as owned, so the next launch cleans it up the same way it does after a crash.
• Out of scope, follow-up after this PR: when the descendant snapshot fails, Codex teardown SIGKILLs the supervisor directly (codex-app-server-process-teardown.ts:117-119), which can orphan the provider group.

Author X handle: @BrennanKB5

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 2fb4a9e5-784e-4b0b-9981-4d4c053c1f81

📥 Commits

Reviewing files that changed from the base of the PR and between 3b37405 and 20e81ad.

📒 Files selected for processing (5)

• src/main/codex/codex-app-server-posix-supervisor.integration.test.ts
• src/main/codex/codex-app-server-posix-supervisor.test.ts
• src/main/codex/codex-app-server-posix-supervisor.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

---

📝 Walkthrough

Walkthrough

The POSIX provider supervisor now accepts owner and grace-period options and uses SIGTERM followed by bounded SIGKILL escalation to stop the provider process group. Connection closing and structured recovery use the supervisor stop limit for their waits. New unit and integration tests cover supervisor shutdown, owner loss, connection-close timing, and recovery after supervisor exit.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 20e81

The POSIX provider shutdown changes appear ready to merge after normal checks. No actionable shutdown or recovery regression was established.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 20e81

The shutdown sequence is designed to keep the supervisor alive until its provider stops, and callers now allow more time for that sequence. Unusual cleanup failures can still leave provider lifetime uncertain, so session ownership during recovery merits attention.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly affected authority is local signalling of a Codex provider process group and the lifetime of its associated session. The inspected paths do not establish an expansion to another process group or service; upstream launch-input trust was not verified.

Security Findings and Attack Paths

• inferred — A failed reap can leave provider lifetime unproven after supervisor exit, undermining any inference that a dead supervisor proves a dead provider. The same exit-on-failed-reap branch is present in the inspected preceding version, so its condition and exposure are not established as introduced or worsened here.

Trust Boundaries and Controls

• observed — Owner-parent validation gates provider startup. Connection close waits for supervisor cleanup before invoking a process-tree termination fallback and reports failure if tree exit cannot be proven.

Resilience and Maintainability Implications

• observed — Recovery probes the recorded owner after SIGTERM before considering SIGKILL, and distinguishes proven-dead from unproven outcomes when choosing its lease-transition path. The downstream semantics of the unproven-owner transition were not established here.

Hardening Proposals

• proposed — Keep an explicit, independently verifiable “provider group not proven empty” state when cleanup fails, rather than relying on supervisor-PID disappearance alone when resolving session ownership.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly describes the primary change: ensuring the provider supervisor outlives and cleanly stops its provider group.
Description check	✅ Passed	The description is comprehensive and covers the change, motivation, implementation, testing, platform impact, linked follow-up issue, and checklist. The Review section is empty, but this is non-critic…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

✅ No new issues found.

Reviewed changes

• Unified provider-group stop. Every supervisor trigger now SIGTERMs the detached provider group, escalates to SIGKILL after the spec grace, and exits only once kill(-pgid, 0) reports the group gone; a received signal is re-raised afterward so a watcher still sees death by signal.
• Owner pid and grace travel in the spec. supervisedPosixLaunch stamps ownerPid/graceMs, refuses a grace above MAX_PROVIDER_SUPERVISOR_GRACE_MS, and the supervisor refuses to spawn when its parent is already not ownerPid. The handlers are registered before the spawn.
• Closed output pipe no longer ends the supervisor. error handlers on process.stdout/process.stderr join the existing ones, so a dead owner's EPIPE cannot kill the pid before it reaps the group.
• Recovery and close() wait out the full stop. STOP_PROBES.SIGTERM rises 4 → 17 (≈4.25 s) and close() waits PROVIDER_SUPERVISOR_MAX_STOP_MS (4 s) on POSIX, both derived from the supervisor's max stop so the SIGKILL cannot land mid-stop.
• Tests. New real-process integration coverage for signal-handed stops, the spawn window, owner death, the grace bound, and recovery eviction; connection unit tests use the new close bound.

ℹ️ The reaping wait depends on the host reaping orphaned grandchildren

The stop logic is correct, but the new integration tests assume the supervisor's kill(-pgid, 0) probe clears once the group is SIGKILLed. On a host whose PID 1 does not reap orphans, the SIGTERM-ignoring grandchild stays a zombie with a process-table entry, the group never clears, and the supervisor takes the full grace + 1500 ms before exiting 1. I reproduced this here and confirmed the pre-existing teardown integration test fails identically, so it is host behavior rather than a regression in this PR; GitHub's runner reaps and passes. No change requested.

Technical details

# Zombie members keep `kill(-pgid, 0)` true

## Affected sites
- `src/main/codex/codex-app-server-posix-supervisor.ts` — `providerGroupExists()` / `waitForProviderGroupExit()` clear only when the group has no process-table entry, which a zombie still does.
- `src/main/codex/codex-app-server-posix-supervisor.integration.test.ts` — the SIGTERM and owner-death cases assert the group clears inside their windows.
- `src/main/codex/codex-app-server-teardown.integration.test.ts` — pre-existing, same assumption.

## What I observed
- In this sandbox (PID 1 = `su`, no reaper) the grandchild is SIGKILLed but reads `/proc/<pid>/stat` state `Z` with PPID 1, and `kill(-pgid, 0)` keeps succeeding, so `reapOwnedProviderGroup()` returns false and `stopProviderGroup` exits 1.
- 5 of the new integration tests fail for this reason; the pre-existing teardown integration test fails the same way.

## Required outcome
- No code change requested. The bounded wait plus exit-1 fallback keeps the invariant sound (a zombie cannot run user code); this note only flags that the tests are host-reaper sensitive.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[brennanb2025]

Extra QA: does a Codex tool command outlive an Orca crash? PASS, 3 of 3 runs on this PR (3b374050c6), on macOS with the real Codex CLI in an isolated background build.

Codex was running sleep 120 when Orca's main process was killed with kill -9. In every run the sleep, Codex and the supervisor were all gone within about 0.5 s, and nothing was alive two minutes later.

Why this PR matters here: Codex starts each shell command in its own process group, so the supervisor's group signal can't reach the command directly; Codex kills it during its own shutdown. On main the supervisor force-kills Codex about 100 ms after Orca dies, with no grace, which can beat that shutdown and leave the command running. An earlier run on main did leave a sleep 120 running to completion. This PR asks Codex to stop first and allows 1.25 s before force-killing; Codex finished within 0.45 s every time. The remaining exposure is a Codex shutdown that takes longer than 1.25 s.

[brennanb2025]

Status: ready for review. Head 3b374050c6, based on main, CI green (27 pass, 0 fail). Merge this before #23476, which is stacked on it.

Review: two review loops; the second was clean. The design was challenged by an adversarial review before it was built.

Readiness check: passed with no release-blocking findings. It ran at the end of the review only, not at the start as well; that's disclosed here. One minor finding, deferred: the provider-neutral supervisor still lives in a Codex module, and recovery imports its timing from there; it moves when #23476 reuses it for Claude.

Live QA:

• Codex on macOS (real CLI): a Codex sleep 120 does not outlive an Orca crash, 3 of 3 runs; details in the earlier comment.
• Full app on a remote Mac (a Claude worker ran it because the Grok quota was exhausted): quitting Orca with one idle and one busy Claude chat stops both, and the relaunched app answers the next message. Force-killing Orca leaves Claude running on this PR alone, the same as main; fix(claude): run structured Claude under the POSIX provider supervisor #23476 is what puts Claude under the supervisor.

Follow-up (not this PR): the Codex connection still picks its stop wait with its own platform check (codex-app-server-connection.ts:258); #23476's spawn spec now carries a supervised flag that can replace it.

[greptile-apps]

Confidence Score: 4/5

[High risk] Changes process lifecycle and shutdown timing across supervisor and session recovery.

The PR does not yet appear safe to merge because forced teardown can still interrupt a delayed supervisor stop and leave provider-group children running.

Findings

1.  Forced stop can orphan children ▶

Summary

This PR makes the POSIX provider supervisor stop and reap its provider group before exiting, and extends Codex close and session recovery waits to accommodate that stop. Since the previous review, the quit-budget test was updated to count the pre-stop snapshot drain.

_{Reviews (2) · Last reviewed commit: "test(codex): count eviction's pre-stop d..."}

[greptile-apps]

Forced stop can orphan children

If the host is under load, the supervisor's timers can run late, but close() forces teardown at exactly the nominal 5,500 ms stop time. If the provider has exited while a child in its process group ignores SIGTERM, teardown can stop the supervisor before it sends SIGKILL to that group. The reparented child may be missing from the descendant snapshot and keep running after close. Recovery has the same narrow timing margin: it waits only one extra 250 ms probe before SIGKILLing the supervisor. Both paths need time to observe the supervisor finishing its stop.

Knowledge Base Used: Agent workspaces and sessions

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental pass over the one commit after 3b374050c6, which splits the supervisor's single grace into the two waits the PR body now documents. No new defects; the ordering guarantees still hold at the wider bound.

• Split the supervisor grace into two named waits. DEFAULT/MAX_PROVIDER_SUPERVISOR_GRACE_MS (both 1250) became PROVIDER_STDIN_END_GRACE_MS (1000, given before SIGTERM) and PROVIDER_SIGTERM_GRACE_MS (3000, given before SIGKILL); the spec now carries stdinEndGraceMs/sigtermGraceMs, each bounded by its own default in assertGraceWithin, so no spec can carry a wait recovery and close() don't cover.
• Raised the longest stop to 5.5 s. PROVIDER_SUPERVISOR_MAX_STOP_MS is now the sum of both graces plus the 1.5 s reap, and both recovery's SIGTERM stage (ceil(MAX/250)+1 = 23 probes, ≈5.75 s) and POSIX close() derive from it, keeping recovery's SIGKILL after the supervisor's own group SIGKILL (≤4.5 s).
• New timing coverage. Integration tests assert the stdin-end fast path adds no wait and that SIGTERM lands ≥1 s after stdin end with SIGKILL ≥3 s later; the teardown test pins MAX_STOP + 1 s under quit's 8 s child-eviction budget, and the connection unit tests use the new close bound.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental pass over 189fce772f, the one commit after 20e81ad43f. It makes the quit-budget test count the eviction's pre-stop drain, so the phase bound is accurate rather than undercounted. No new defects.

• Drain timeout exported and folded into the quit budget. SNAPSHOT_DRAIN_TIMEOUT_MS is now exported from structured-agent-session-eviction.ts, and structured-agent-session-host-teardown.test.ts asserts 1_000 + 5_500 + 1_000 < 8_000. Both terms genuinely run inside the evict-owned-sessions phase — snapshot-before-stop drains the sink (≤1 s) before stop-provider-child closes the Codex connection (≤5.5 s on POSIX) — so the tightened assertion reflects the real worst case.

The invariant and its ordering guarantees are unchanged from the prior pass: while any member of the provider group is alive, the supervisor pid is alive, and recovery's SIGTERM stage plus POSIX close() both outlast the supervisor's longest stop.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}