fix(runtime): stop shell commands when their Runtime Host dies by chihumyum · Pull Request #5986 · apache/maka

chihumyum · 2026-10-08T00:37:09Z

Summary

Pipe-mode shell commands are spawned detached, so on POSIX each one leads its
own process group. That makes timeout, Stop and abort work: the Host signals the
group and sweeps escaped descendants. It also means nothing stops them when the
Host itself dies without running cleanup (SIGKILL, crash, OOM kill, force quit).
The command is reparented to launchd/init and keeps running against the
workspace with no owner left to stop it or record its result. In a local live
run, a Bash command wrote its completion marker about 32 s after its Host was
SIGKILLed. shell-owner-death.test.ts below reproduces
this on main for both the foreground (bounded) and background (pipe driver)
paths, 4 of 4 failing.

This PR starts each pipe-mode command through a small owning supervisor,
@maka/runtime/dist/owned-process-main.js. It is launched with
process.execPath, which in packaged Desktop is Electron with
ELECTRON_RUN_AS_NODE, the same way the Runtime Host candidate itself is launched.

The supervisor is the detached process-group leader. It spawns the command
into its own group with the Host's stdio and descriptor payloads inherited
directly, so output never passes through IPC.
It holds a private IPC channel to the Host as a lifetime lease, the same
pattern candidate-launch-owner-guard.ts uses for Desktop → Host. When the
channel disconnects, the supervisor SIGKILLs its whole tree with the existing
terminateProcessTree, which also catches descendants that moved to another
group, and exits.
Host-initiated timeout, Stop and abort still signal the group, so the command
gets the signal directly. The supervisor does not forward it a second time
(tested: exactly one SIGTERM). The exit code or signal is mirrored back
unchanged.
If the supervisor dies while the Host is alive, the Host kills the orphaned
group and never reports that as success.

Covered paths: runShellWithBoundedTail / runProcessWithBoundedTail
(foreground Bash) and PipeProcessDriver (background pipe runs).

Two details a supervisor has to get right:

The shell_run.pid shown to users and the model stays the command's own PID.
With a supervisor in front, PipeProcessDriver.pid is the supervisor, which
ignores a lone SIGTERM because group signals already reach the command. If
that PID were recorded, a model-issued kill <pid> would silently do
nothing (measured: the command was still running 1.5 s later). The
supervisor now reports the command PID on admission, and
ShellRunProcessManager records that as pid. Group signalling still uses
the supervisor PID.
Mirroring a command's death signal could hang. Node ignores SIGPIPE and
uses SIGUSR1 to start its inspector, so a supervisor that simply re-raised
these never exited. A command killed by either stayed "running" until a
timeout, or forever for a background run. The supervisor now restores the
default disposition before re-raising, with a bounded exit fallback.

Root cause

spawn(..., { detached: true }) puts the command in a new session and process
group, and no OS mechanism ties its lifetime to the Host. Cleanup lived only in
Host callbacks (manageChildProcessLifecycle, ShellRunProcessManager), and
those cannot run after SIGKILL.

Verification

New tests fail on main and pass with this change:

Test	`main`	this PR
`shell-owner-death.test.ts`: `bounded`/`pipes` × `stop already requested: false/true` — owner SIGKILL terminates an admitted command before its delayed write	4 of 4 fail: "Command wrote after its owner was killed"	4 of 4 pass
`shell-run-manager.test.ts`: records the pipe command PID, not the supervisor that owns its process group	passes (guards existing behavior)	passes. Fails if only the supervisor is added (records the supervisor PID)
`owned-child-process.test.ts`: supervisor mirrors a command that dies from SIGPIPE / SIGUSR1	n/a (new module)	pass. With a plain re-raise, both hit the 10 s timeout and the stuck supervisors kept the test process alive

Other new tests: owned-child-process.test.ts covers argv, env, binary fd 3/7
payloads and exit 23 preserved; missing executable rejects with ENOENT;
the command's NODE_OPTIONS applies to the command only; POSIX cancellation
delivers SIGTERM exactly once; unexpected supervisor death kills the command.
pipe-process-driver.test.ts checks that the reported command PID differs from
the supervisor and that signalling it alone ends the command.

Suites and checks (macOS 27, arm64, Node 24.19.0):

npm --workspace @maka/runtime run test:dist: 3739 tests, 3725 passed, 0 failed, 14 skipped (Linux/Windows/pwsh gates).
npm --workspace @maka/runtime-host run test:dist: 2240 tests, 2228 passed, 0 failed, 12 skipped.
Full npm test (clean build, all 13 workspaces): 14362 node tests, 14324 passed, 0 failed, 38 skipped; eval Python tests 94 OK (13 skipped).
check:release (minus the dist-freshness check:stale): 214/214, including windows-package-source-closure. The Windows recovery path filter now lists owned-child-process.ts instead of shell-exec.ts/pipe-process-driver.ts, since the win32 branch moved; windows_recovery passed on CI.
npm run typecheck (14 workspaces), npm run lint, npm run format:check, npm run check:asf-headers: pass.
node scripts/windows-test-inventory.mjs --write, then npm run windows:inventory: pass. The three new POSIX-only skip declarations are classified platform-contract.
Packaging:
- CLI: MAKA_CLI_DEVELOPMENT_PEER_TARGET=none node scripts/release-cli-package.mjs --development
  passes validateStaging and validatePackedFiles with the two new required
  entries. A command run from the extracted tarball's @maka/runtime ran under
  the packed owned-process-main.js.
- Desktop .asar: a probe packed owned-child-process.js, owned-process-main.js
  and their two imports into an asar with @electron/asar and ran it with
  Electron 43.4.1 in ELECTRON_RUN_AS_NODE mode. Result: exit status 23 and
  split stdout/stderr preserved, and owner SIGKILL stopped the command before its
  delayed write. No credentials are needed. The probe is not part of this PR (see below).

Windows: not run locally. CI's test job is Linux-only, and windows_recovery originally ran none of these suites. It now runs owned-child-process, shell-owner-death and pipe-process-driver on windows-latest, and its path filter lists those test sources. shell-exec is not included: its suite uses POSIX shell syntax throughout and fails on Windows independently of this PR. The first Windows run drove b587b76: detach the supervisor on Windows, and don't treat a taskkill exit code as a lost result. On b587b76, windows_recovery passes: 18 tests, 10 passed, 0 failed, 8 POSIX-only skips. That includes the wrapped owner-death case, where a cmd /c root and its child are both stopped.

Cost

One extra Node (or Electron-as-Node) process per running pipe-mode command,
alive for exactly as long as the command.

Measured on Apple Silicon, 12 cores, macOS 27, with the machine quiet
(1-minute load about 6, Spotlight idle). main's runtime build ("direct") was
interleaved with this branch's ("supervised"). 10 warmup samples were discarded,
then n = 40 per cell (n = 20 for bursts). Values are p50 / p95 in ms.

Scenario	Node 24.19 direct	Node supervised	Electron 43.4.1 direct	Electron supervised
argv `/usr/bin/true`, end to end (bounded path)	1.1 / 1.4	26.6 / 29.2	1.3 / 1.9	80.5 / 90.2
shell `true`, end to end (foreground Bash path)	2.5 / 2.7	28.2 / 29.5	2.7 / 3.9	74.8 / 90.6
background run, start → `ready`	0.3 / 0.5	27.8 / 31.3	0.3 / 0.4	84.3 / 93.1
burst of 8 concurrent `/usr/bin/true` (wall time)	3.0 / 3.8	45.3 / 51.8	2.9 / 5.5	113.3 / 137.3

Memory per running command, from the supervisor process: RSS 46.6 MiB (Node) /
59.6 MiB (Electron). macOS physical footprint, which excludes pages shared with
other processes, is 13 MB / 15 MB.

So the CLI adds about 26 ms per command. Packaged Desktop, where the supervisor
is Electron in Node mode, adds about 72–84 ms per command and about 15 MB per
command that stays running (dev servers, watchers). That is small next to a
model turn, but noticeable in a turn that runs many short commands.

Alternative considered: one long-lived supervisor per Host

One leased "command broker" started with the Host would spawn every command and
kill all of its groups on disconnect. Per-command cost would drop to an IPC
round trip, and memory to one process in total. It is not a small change:

Stdio and descriptors. Today the Host creates the pipes and the command
inherits them, plus raw sourceFd payloads. A broker's commands are its own
children, so output must either be relayed over IPC (extra copy, re-implemented
backpressure, and the retained-tail and live-emit caps seeing chunks one hop
later), or be handed over as fds. Node IPC can pass net.Socket/net.Server
handles only, not arbitrary fds, and Windows is more limited still.
Lifecycle model. manageChildProcessLifecycle and PipeProcessDriver are
built on a ChildProcess (exit/close/error ordering, the output-drain fences).
A broker yields a remote handle, so those semantics would need an emulating
adapter. That is the riskiest part to get subtly wrong.
Failure domain. One broker crash ends every running command. It then needs
supervision, restart, and rules for requests in flight.
Unchanged: signals (pgids are still direct OS targets) and owner-death
detection (same disconnect lease).

A middle ground with identical semantics: keep one supervisor per command but
make it a tiny native executable instead of a Node process. The repo already
ships native artifacts. That cuts startup to about a millisecond and RSS to
about a megabyte, at the cost of per-platform build and packaging work (it must
live outside app.asar). Linux PR_SET_PDEATHSIG alone is not enough, because
macOS has no equivalent. Given the Desktop numbers above (about 75 ms and 15 MB
per command), the native per-command supervisor is the follow-up I would do
first. It keeps every semantic and test in this PR and removes most of the cost.
The broker is only worth its complexity if process count itself becomes the
problem.

Review focus / risks

PID semantics. shell_run.pid is the command's own PID, as before. A
group kill by the model (kill -- -<pid>) now fails with ESRCH, because the
command is no longer the group leader. kill <pid> and kill -9 <pid> behave
as on main. StopBackgroundTask is unaffected.
Descriptors. The supervisor keeps its own copies of stdin and fd-payload
pipe ends until it exits, right after the command. A write that would hit
EPIPE when the command closes its stdin is now reported when the supervisor
exits. The probe measured both at exit (348 ms vs 390 ms for a command that
closes stdin and exits at 300 ms). The IPC fd does not leak into the command
(checked from /bin/sh).
Output vs admission ordering. Command output reaches the Host's pipes directly, while admission arrives over IPC, so output could be read before ready (Linux CI: a foreground Bash aborted on its first output failed startup with AbortError instead of returning a cancelled result). The client now pauses stdout/stderr until admission has been observed, and releases them in the check phase after ready resolves, or on startup failure or supervisor exit. That restores a direct spawn's "spawn before any output" order. owned-child-process.test.ts "command output never reaches the caller before admission" forces both to be pending at once and fails without the hold. The workspace executor abort test now aborts on the command's first output instead of after a fixed 100 ms, which also had to cover supervisor startup on a loaded runner.
Exit/message ordering. The Host treats a supervisor exit without a prior
completed message as a lost result. Node emits exit without ordering it
against pending IPC reads; only close waits for the channel. In 60 of 60
trials with the Host loop blocked across the command's exit, completed
arrived first. If this should not rest on that, the fix is to decide on the
channel's disconnect (EOF) instead of exit. I could not make a test fail
for it, so it is not in this PR.
Windows. If the supervisor alone dies while the Host lives, the Host does not run taskkill /T on the reaped PID, because the PID may be reused. The orphaned command is therefore not stopped until the Host itself goes. main cannot reach a dead root's children either, so this is not a regression.
After a supervisor exit. On POSIX the Host only signals the supervisor's process group, which cannot be reallocated while members remain. It does not walk a possibly reused PID's tree. Only a clean exit (code 0) without completed is reported as a lost result. Other exits may be the Host's own forced stop; on Windows, taskkill ends with an ordinary code.
Windows job objects. libuv kills non-detached children with their parent, but not those children's own children. On main, Host death therefore kills a cmd /c root but leaves the root's child running. The supervisor is detached on Windows too, so it outlives the Host, sees the lease drop and taskkill /Ts its tree. The command stays in the supervisor's job. The wrapped owner-death case covers exactly this.
Group-wide signals. A command's script shares the supervisor's process group. The supervisor ignores HUP, USR1, USR2, QUIT and ALRM on POSIX, so kill -HUP 0 and similar reach only the command's handling, as on main. The USR1 listener also keeps the Node inspector from starting.
Lease ends at root exit. Descendants still in the group after the root command exits (cmd &, a dev server started by a script that returns) are not owned, as on main. Holding the lease past root exit needs its own design and is a follow-up.
Startup and timeouts. Admission waits for the supervisor, with a 10 s startup timeout before the command is failed. A bounded command's timeoutMs starts at admission, as the background runner's already did, so the supervisor's startup doesn't use up the command's budget. A 100 ms timeout on a loaded Linux runner otherwise fired before the command ran (816dc0d).

Deliberately left out

PTY-mode commands (node-pty). They are not supervised. Closing the PTY master
on Host death hangs up the terminal, and the foreground group gets SIGHUP. That
ends most commands, but not ones that ignore SIGHUP.
Other spawn sites (MCP stdio servers, LSP, sandbox helpers outside the shell
runner), and double-forked daemons that leave the process tree entirely. These
are unchanged from main.
A standalone Electron .asar packaging probe script. It ran clean locally
without credentials, but nothing in CI or package.json would run it.
It hardcodes the module's import list, so it would go stale silently. The
durable home for that check is a smoke step in the packaged-app verifiers,
next to the existing filesystem-worker and PTY probes. That can be a follow-up.

AI use

Select exactly one:

No generative tool made a substantive contribution
Generative tooling made a substantive contribution

Tool(s) and scope: Codex wrote the supervisor on an experimental branch. Claude Code split it out, added the command-PID reporting and the SIGPIPE/SIGUSR1 signal mirroring with their tests, ran the verification and cost measurements above, and wrote this description. The commit carries Generated-by: Codex and Generated-by: Claude Code. Final review and merge remain with human maintainers.

Checklist

Tests cover the change and fail without it
Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

Yes — described under Summary above
No

Pipe-mode commands are spawned detached, so each leads its own process group and nothing stops it when the Host dies without running cleanup (SIGKILL, crash, OOM kill). The command is reparented and keeps writing to the workspace with no owner left to stop it or record its result. Start each bounded and background pipe command through a small owning supervisor (owned-process-main.js, launched with process.execPath, as Electron in Node mode under Desktop). The supervisor leads the process group, spawns the command into it with the Host's stdio and descriptor payloads inherited directly, and holds a private IPC channel to the Host as a lifetime lease; when the channel disconnects it SIGKILLs its tree. Host-initiated signals still reach the group once, and the command's exit code or signal is mirrored, including SIGPIPE and SIGUSR1, which Node would otherwise ignore or repurpose. Keep the shell run's reported pid the command's own, so `kill <pid>` behaves as before; group signalling uses the supervisor. Require the supervisor entry in CLI release artifacts. Each running pipe command now costs one extra process: about 26 ms per command under Node and 75 ms under Electron on an idle Apple Silicon machine, and 13-15 MB of physical footprint while it runs. Generated-by: Codex Generated-by: Claude Code

The Windows recovery path filter must equal the win32-branching closure of the suites that lane runs. The platform branch for spawning pipe-mode commands moved from shell-exec.ts and pipe-process-driver.ts into owned-child-process.ts, so the filter now lists that module instead.

A supervised command writes to the Host's pipes directly, while its admission arrives over the supervisor's IPC channel, so the Host could read output before `ready` resolved. A foreground Bash aborted on its first output then failed startup with an AbortError instead of returning a cancelled result, as seen on Linux CI. With a direct spawn the spawn event always preceded any output. Pause stdout and stderr until admission has been observed. Release them in the check phase after `ready` resolves, or when startup fails or the supervisor exits. The workspace executor abort test now aborts on the command's first output instead of after a fixed 100 ms, which also had to cover the supervisor's startup on a loaded runner.

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one. Findings were checked against the code and local Linux probes.

Thanks — the supervisor-as-lease design is well argued, and the escaped-descendant sweep ordering in terminateProcessTree (sweep, then group) makes the self-kill safe. Locally (Linux x64) the targeted suites pass. Probes: a setsid grandchild escaped while the command is running is killed on owner SIGKILL; a 4 s blocked Host event loop does not kill a running command (no false positive); 500 exit/completed ordering trials under load showed 0 lost results.

P2 — Windows is unverified (graded P2 by the review lead: a coverage gap, not an observed defect; but every Windows Bash command now goes through the supervisor). The PR body says Windows CI is the first real run of the IPC-disconnect → taskkill /T path, but CI test is ubuntu-only and windows_recovery runs a fixed allowlist (runtime-resume-crash, runtime-continuation-crash, host/storage suites). It runs none of owned-child-process, shell-owner-death, pipe-process-driver, or shell-exec. Adding owned-child-process.ts to the path filter only triggers the job. Every Bash call on Windows now goes through this path, so please get a Windows run of those suites before merge.

P2: (inline) post-exit terminateProcessTree targets a reaped, possibly reused PID.

P3 (scope, not a regression from main): (inline) the lease ends when the root command exits, so descendants left in the group, such as a backgrounded dev server, are unowned again.

P3s: (inline) exit/completed ordering; group-wide signals that the command's own script sends (kill -HUP 0, kill -USR1 0); a stop that lands between the launch message and spawn(). Also on tests: shell-owner-death covers only the direct command. A grandchild-after-root-exit case and a setsid-escapee case would pin the semantics.

Other checks: no protocol/epoch change (stays 205). Sandbox interaction looks fine: bwrap --die-with-parent now binds to the supervisor, and bwrap --new-session --unshare-pid plus seatbelt signal (target same-sandbox) stop sandboxed commands from signalling the supervisor. Build immutable tarball fails on an unrelated production npm audit high finding that also hits dependabot PRs, so the new packed-file requirement in release-cli-package.mjs was not exercised in CI.

After the supervisor exits without reporting, its PID may be reaped and reused, so walking its tree could reach unrelated processes. Signal only its POSIX process group, which cannot be reallocated while members remain, and skip taskkill on Windows. Decide such an exit once the IPC channel has drained rather than on `exit`, and report a normal exit without a result as an integrity error instead of the command's status. The command shares the supervisor's process group, so a script that signals its own group could end the supervisor or start its inspector. Ignore HUP, USR1, USR2, QUIT and ALRM in the supervisor on POSIX. Spawn the command in the check phase after the launch message, so a group stop pending in the same turn is handled before the command starts. Tests cover a script trapping and sending group signals, a lost lease, and a setsid descendant of a live command on owner death.

Every Windows Bash command now runs under the owning supervisor, but no Windows lane ran its suites. Add owned-child-process, shell-owner-death, pipe-process-driver and shell-exec to windows_recovery, and list their Windows-branching test sources in the lane's path filter.

chihumyum · 2026-10-08T01:29:30Z

On the Windows P2: you're right. My description was wrong that Windows CI was already the first real run, because test is Linux-only and windows_recovery ran none of these suites. 92bd3bc adds owned-child-process, shell-owner-death, pipe-process-driver and shell-exec to windows_recovery, and lists their Windows-branching test sources in its path filter (the source-closure and CI workflow policy tests pass). That gives this PR a real Windows run. I'll fix whatever it finds before asking for merge. The description is updated accordingly.

🤖 Addressed by Claude Code

The first Windows run showed two problems. On Windows the Host's forced stop is taskkill, which ends the supervisor with an ordinary exit code. Treating every non-zero exit without `completed` as a lost result turned timeouts and aborts into integrity errors. Only a clean exit without a result is reported now. libuv puts non-detached children in a job that is killed with their parent, but not their own children. A non-detached supervisor therefore died with the Host before it could stop anything, and a shell root's child survived, as on main. Detach the supervisor on every platform. It now outlives the Host long enough to stop its whole tree; the command stays in the supervisor's job. Add a `wrapped` owner-death case, a shell root running the script as its child, which fails on main. Skip the stop-requested variant on Windows, where a stop is an immediate forced kill. Accept EOF for a broken stdin pipe on Windows. Drop shell-exec from the Windows lane: its suite uses POSIX shell syntax throughout and fails on Windows without this change.

chihumyum · 2026-10-08T01:49:50Z

Windows run for the P2: windows_recovery on b587b76 passes the ownership suites on windows-latest, 18 tests: 10 passed, 0 failed, 8 POSIX-only skips. The first run (92bd3bc) found two real problems, both fixed in b587b76:

On Windows the Host's own taskkill ends the supervisor with an ordinary exit code, so only a clean exit without completed now counts as a lost result.
libuv's job object killed the non-detached supervisor together with the Host, before it could stop the command's own children. The supervisor is now detached on Windows too.

The new wrapped case (a cmd /c root whose child does the work) passes there and fails on main. Build immutable tarball still fails only on the unrelated production npm audit finding (GHSA-6qxp-vccf-f47h in @modelcontextprotocol/client 2.1.0).

🤖 Addressed by Claude Code

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of bab002a9 → b587b768 (2cad001, 92bd3bc, b587b76), compared against the same merge-base 3597abe8. Verdict: COMMENT. No P0, P1 or P2 issues remain. Both prior P2s are fixed. What's left is P3 only.

Prior findings

P2 Windows unverified: fixed. windows_recovery now runs owned-child-process, shell-owner-death and pipe-process-driver on windows-latest: 18 tests, 10 pass, 8 POSIX-only skips, 0 fail. The new wrapped (cmd /c root) owner-death case passes there. That first run found two real Windows defects, the libuv job and taskkill exit codes, and b587b76 fixes both. shell-exec is out of the lane because its suite is POSIX syntax; the bounded owner-death mode still covers that driver on Windows.
P2 post-exit tree walk on a reaped PID: fixed on the exit path. After the supervisor exits, the Host only does kill(-pgid, SIGKILL), and on win32 it skips taskkill. One narrower residual is left on the failed path (inline).
P3 exit/completed ordering: fixed. A missing result is now decided after disconnect. I checked that the IPC fd is not inherited by the command (a grandchild's /proc/<pid>/fd shows only 0-2), so disconnect follows supervisor death promptly and the wait can't hang. Non-zero exits without completed are deliberately treated as the command's status now, because Windows taskkill can't be told apart (inline note for POSIX).
P3 group signals: fixed. HUP/USR1/USR2/QUIT/ALRM are ignored on POSIX. I confirmed on Node 22 that a no-op SIGUSR1 listener keeps the inspector from starting, and the new test asserts that stderr has no Debugger listening. Other default-terminating signals (PROF, VTALRM, XCPU, XFSZ, SYS, TRAP) and TSTP are still uncovered; that's rare enough to treat as a nit.
P3 stop before spawn: narrowed, not closed (inline). The author acknowledges this.
P3 lease ends at root exit: open, deferred as a follow-up design. That's reasonable, since main has the same gap. A tracking issue would keep it from getting lost.
P3 tests: addressed. The setsid escapee and wrapped cases were added. Not pinning the grandchild-after-exit leak as expected behaviour is the right call.

New in this round (P3): Windows detached: true console side effects, a missing count guard on the new Windows CI step, and the failed path tree walk (all inline).

Status. Mergeable (BLOCKED only on review). CI: test and windows_recovery pass. Build immutable tarball still fails on the unrelated production npm audit high finding (@modelcontextprotocol/client), so the packed owned-process-main.js check in release-cli-package.mjs is still not exercised. No runtime-host protocol change; the epoch stays 205.

Local (Linux x64, exact head). owned-child-process 10/10, shell-owner-death 6/6, pipe-process-driver 2/2, shell-exec 13 pass (2 environmental skips), and shell-run-manager 63 pass (4 environmental skips). The ownership suites ran 5 more times with no flakes.

Astro-Han · 2026-10-08T03:18:13Z

+      // the supervisor out of the Host's libuv job, which would kill it with
+      // the Host before it could stop the command's descendants; the command
+      // itself stays in the supervisor's job.
+      detached: true,


P3 (Windows, please verify manually). detached on win32 means DETACHED_PROCESS, so the supervisor has no console. As I read libuv, the command spawn in the supervisor uses inherited fds ([0, 1, 2, ...]), so it doesn't get CREATE_NO_WINDOW. The console-subsystem root (cmd.exe, bash.exe, etc.) would then allocate a fresh console per command, hidden only through SW_HIDE. Before this change it inherited the Host's (windowless) console. CI can't show this. Could someone check packaged Desktop on Windows 11 with Windows Terminal set as the default terminal for a console flash, and look at the per-command conhost cost? If either is a problem, CREATE_NO_WINDOW on the command spawn (or an allocated hidden console in the supervisor) would avoid it.

Agreed on the reading, and it predates the detach change. The supervisor's command spawn passes inherited fds, so libuv never adds CREATE_NO_WINDOW. A console-subsystem root then gets its own console, hidden only via SW_HIDE. The earlier non-detached supervisor had no console either (CREATE_NO_WINDOW), so it was affected the same way. main also lacks CREATE_NO_WINDOW for commands with source-fd inputs. I can't run packaged Desktop on Windows 11 here, so I'm leaving this open for a manual check. If it flashes, the fix is to have the supervisor hand the command non-inherited stdio, or a native launcher that passes CREATE_NO_WINDOW.

🤖 Addressed by Claude Code

…lane A `failed` message means the supervisor is already exiting on its own, so a tree walk from its PID could start after it is reaped. Signal only its process group on that path, as after an exit. On POSIX the Host's own stops end the supervisor by signal, so an exit code without `completed` can only be a supervisor fault. Report it as a lost result there, rather than as the command exiting with that code. Windows keeps only a clean exit, since taskkill also ends with a code. The Windows ownership step now asserts its TAP counts, like the other gates in that job, so a widened POSIX-only skip cannot leave it green while it tests less.

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of b587b768 → a2ff3fb2 (one commit, same merge-base 3597abe8). Verdict: COMMENT. No P0, P1 or P2. Two prior P3s are fixed, one is partly fixed, and two remain open by agreement.

Prior findings

Tree walk on failed (owned-child-process.ts:130): fixed. failed now only signals the supervisor's POSIX group. That is sufficient, because the supervisor sends failed only when the command's spawn errors, so nothing else exists to kill. The tree walk is kept for the startup timeout and launch send errors, while the supervisor is alive.
Windows test count (windows-recovery.yml:256): fixed. The step uses TAP plus exact 18/10/8 assertions, the same as the owner-death step, and it is green on windows-latest at a2ff3fb.
Supervisor fault reported as exit 1 (:186): partly fixed. See inline. The new branch only catches exit codes, but loseOwner ends the supervisor by SIGKILLing its own group.
Stop-before-spawn (owned-process-main.ts:119): open, declined. SIGKILL escalation bounds it, and the command is never left running. Accepted.
Windows detached console flash (:80): open. It still needs a manual check on packaged Desktop with Windows 11 and Windows Terminal before release.
The lease ending at root exit stays deferred. A tracking issue would help.

New code. No false positives found in the POSIX lost widening. The Host's own stops all end the supervisor by signal or after completed, and timeout or send errors set failureReported. One side effect: a supervisor that fails to boot now also emits a lost its result error after ready rejects. That is benign.

Status. MERGEABLE, BLOCKED on review only. test, windows_recovery and the addon builds pass. Build immutable tarball still fails only on the unrelated npm audit finding (GHSA-6qxp-vccf-f47h), so the packed owned-process-main.js is still not exercised in CI. Main moved to 72afbf1 with no overlap, and the merge is clean. No protocol change, so no epoch claim is needed.

An uncaught exception or rejection in the supervisor stopped its tree with its own group's SIGKILL, so the Host saw a signal exit it could not tell from a forced stop, and reported it as the command being killed. While the lease is still up, report the fault as a failure first, then stop the tree. A one-second fallback stops it even if the report hangs. The test plays the Host against the real supervisor, injecting the fault through a preload rather than a product hook. The Windows ownership step now expects 19 tests, 11 passed, 8 skipped.

The bounded runner started its timeout when it spawned the supervisor, so the supervisor's startup consumed part of the command's budget. On a loaded Linux runner a 100 ms timeout fired before the command had run at all, and the escalation test found no heartbeat file. With a direct spawn the budget belonged to the command, and the background runner already arms its timeout after admission. Start the timer once the supervisor admits the command. A supervisor that never admits it still fails through its own startup timeout.

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of a2ff3fb2 → 816dc0d0 (two commits, same merge-base 3597abe8). Verdict: COMMENT. No P0, P1 or P2.

Supervisor fault reported as a SIGKILLed command: fixed. fault() reports failed while the lease is up, and the new preload-based test pins it against the real supervisor. One residual is noted inline: the Host's immediate group kill now races the supervisor's own tree walk.
Timeout starts at admission (shell-exec.ts): looks correct. It is guarded against settle and abort, and an unadmitted supervisor still fails through its startup timeout.
Still open: stop-before-spawn (declined, accepted); the Windows console flash, which needs a manual packaged-Desktop check; the lease ending at root exit (deferred).
Status: MERGEABLE, BLOCKED on review only. test, windows_recovery (19/11/8) and the addon builds pass. Build immutable tarball still fails only on the unrelated CLI npm audit finding. The merge with main f10a74d8 is clean, and there is no protocol change.

Since a fault after admission is reported as `failed`, the Host's immediate group kill on that message hit the supervisor while its own tree walk was still reading the process table, so descendants that had left the group survived. On `failed` after admission the Host now only reports the error and leaves the stop to the supervisor. The group kill on its exit stays the backstop. A spawn failure before admission still signals the group at once. A fault while the `completed` report is in flight no longer sends `failed`, and the Host ignores a `failed` that follows `completed`, so a valid result is never turned into an error. Tests cover the Host leaving a reporting supervisor alive with the backstop still stopping the command, and the supervisor's walk reaching a setsid descendant on a fault. The Windows ownership step now expects 20 tests, 12 passed, 8 skipped.

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of 816dc0d0 → 0e242642 (one commit, same merge-base 3597abe8). No P0, P1 or P2.

Host group kill racing the supervisor's tree walk on a post-admission failed: fixed. owned-child-process.ts:174-184: a pre-admission failed still kills the group. After admission, the Host only reports the error and leaves the walk to the supervisor, with the group kill on exit as the backstop (completed stays unset). The real-supervisor fault test now also pins a setsid descendant, and a new Host-side test pins "no kill mid-walk, backstop on exit".
A fault while completed is in flight no longer becomes failed (owned-process-main.ts:54), and the Host ignores a failed that follows completed.
I checked the callers. shell-exec does not signal on 'error'. The background runner's handleIntegrityFailure sends SIGTERM first (which the supervisor ignores after admission) and walks from the Host, so neither path reintroduces the race.
Still open: stop-before-spawn (declined, accepted) and the Windows console flash (manual packaged-Desktop check).

CI: test, windows_recovery (20/12/8) and the addons pass. Build immutable tarball still fails only on the unrelated CLI production npm audit (1 high). The merge with main is clean, and there is no protocol change.

Main renamed the filesystem worker bundle to .mjs; keep that name and this branch's owned-process-main.js entry in both release file checks. The supervisor runs in the Host's context, outside the Windows sandbox, so the module-type lookup that broke the worker does not apply to it.

Astro-Han

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of #5986: 0e242642 to 81b05aba.

What changed: there is one new commit, 81b05aba "Merge upstream/main into fix/shell-owner-death". It contains no new author code. The merge-base moved from 3597abe8 to 6fa48516 (current main). The author's diff against the new merge-base is identical to the previous one except for the merge resolution below.

Merge resolution: scripts/release-cli-package.mjs conflicted with #6016, which renamed the filesystem worker bundle to .mjs. The resolution is correct in both validateStaging and validatePackedFiles:

It keeps main's filesystem-worker.mjs.
It keeps this branch's owned-process-main.js entry.

Checked via git show --remerge-diff. The .mjs rename was needed because the AppContainer sandbox denies the package.json module-type lookup. That reason does not apply to the supervisor. spawnOwnedProcess launches owned-process-main.js directly from the Host with process.execPath, so the sandbox only wraps the command the supervisor spawns. Keeping .js is fine.

Interaction with recent main merges:

#5495 (session-catalog protocol, epoch 212) does not touch any file this PR touches.
#5512 (Agent Graph results) does not touch any file this PR touches.
This PR does not touch protocol files, so there is no epoch claim.
#5977 is still open and shares no files with this PR.
The only files that overlap with what main changed since the old merge-base are release-cli-package.mjs (resolved above) and .github/workflows/windows-recovery.yml. The workflow change auto-merged: main's fork/schedule guard and this branch's lane are both present.

CI on 81b05aba: all green.

test: pass
windows_recovery: pass
Addons ×4: pass
Build immutable tarball: pass. It was red at 0e242642 from the @modelcontextprotocol/client audit, which #5996 fixed; that fix is now merged in.
Validate installed CLI on Linux x64, Linux arm64, macOS and Windows: pass
Qualify released State Roots: pass
CLI Eval: skipped by design

Mergeable; blocked only on required review.

Prior findings:

N-1 (faulting supervisor killed mid-walk): fixed in the previous round. Unchanged.
P3-a stop-before-spawn: open, declined and accepted.
P3-d Windows console flash (owned-child-process.ts, windowsHide plus detached supervisor): open. A manual check on a packaged Desktop on Windows 11 with Windows Terminal is still recommended. CI cannot observe it.
Lease ends at root exit: deferred.

New findings: none.

Verdict: COMMENT. No P0, P1 or P2. The previous CI blocker is cleared by the main merge.

Astro-Han

Approved at @Astro-Han's explicit request. Our automated review of this exact head found no P0–P2 (remaining notes are P3 and listed in the earlier review comments), and CI is green.

github-actions Bot added the effort/L Under 1000 readable lines label Oct 8, 2026

chihumyum added 2 commits October 8, 2026 08:42