fix(process): arm the setsid dependency, correct the orphan account

Review round 2, three findings.

setsid is util-linux, not coreutils, and the standard `cargo test --lib`
gate must not hard-fail on a tool the README does not declare -- a
minimal or BusyBox container would fail without ever testing pmacs. The
hard assert becomes skip-unless-armed via PMACS_REQUIRE_SETSID, which is
the pattern the silent-skip lane already established, so the test cannot
quietly report `ok` having never run where the tool is guaranteed. CI
arms it on Linux; README declares it. Both arms verified against a PATH
with setsid genuinely removed: unarmed skips with its message, armed
FAILS with the diagnostic.

The durable causal account was wrong, and this corrects it in the
framing, the handoff and the ledger. basedpyright's console script runs
bundled node through `subprocess.run` and WAITS
(nodejs_wheel/executable.py:50, verified in the installed 1.39.6). It
does not exit at spawn. What orphans node is pmacs: `shutdown()` SIGTERMs
the recorded pid -- the Python wrapper -- which dies without forwarding
the signal, leaving node at PPid 1 holding the pipes. The refutation was
already in hand: the initialize handshake succeeds, which a wrapper that
exited at spawn could not have done, and the PPid 1 observation was taken
after shutdown had killed it.

The fix is unaffected -- the deadlock and its bite are unchanged -- but
the parked follow-up changes target: not "tolerate servers that
self-orphan" but "stop orphaning them", i.e. signal the process group
rather than a wrapper pid that swallows the signal. Framing section 5 P2
restated.

Also corrects a stale CI-ordering claim: the handoff said pyright must
stay unarmed until the timeout lane lands, but #195 is this PR's base and
gave every job a timeout-minutes. The one live reason is that CI does not
install basedpyright at all. The ci.yml comment asserting the job has no
timeout-minutes was stale for the same reason and is rewritten.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T

.github/workflows/ci.yml

@@ -203,18 +203,27 @@ jobs:
       # render job. Set only where the install step ran.
       #
       # PMACS_REQUIRE_PYRIGHT is deliberately NOT set and basedpyright
-      # is deliberately NOT installed: that test has no timeout and
+      # is deliberately NOT installed. Both original reasons are now
-      # hangs forever (root cause is the non-interruptible reader-thread
+      # gone: the hang's root cause was the stdin-field drop ordering in
-      # join in `RuntimeHandles::drop`, already a named deferral in
+      # `RuntimeHandles::drop` and is fixed, and this job now carries
-      # `src/process.rs`). This job has no `timeout-minutes`, so arming
+      # `timeout-minutes`, so a hang could no longer burn six hours.
-      # it today would trade a vacuous green for a six-hour hang on four
+      # The ONE remaining reason is the plain one --- basedpyright is not
-      # legs. It gets armed after the hang fix and the CI timeouts land,
+      # installed here, so arming the variable would fail rather than
-      # and its own variable exists so that flip is one line.
+      # test anything. Installing it (a uv + bundled-node download on
+      # every leg) is its own decision, not a rider on the hang fix.
+      #
+      # PMACS_REQUIRE_SETSID arms the teardown-deadlock unit test. Its
+      # fixture orphans a grandchild with `setsid --fork`, which is
+      # util-linux rather than coreutils, so the test skips when the
+      # binary is absent (a minimal container must not fail `--lib`
+      # without ever testing pmacs) and this variable is what makes the
+      # skip fatal where the tool is guaranteed.
       - run: cargo test --all-targets --no-default-features --features ${{ matrix.lua }} -- --test-threads=1
         env:
           PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }}
           PMACS_REQUIRE_SHELLS: ${{ runner.os == 'Linux' && '1' || '' }}
           PMACS_REQUIRE_LUA: ${{ runner.os == 'Linux' && '1' || '' }}
+          PMACS_REQUIRE_SETSID: ${{ runner.os == 'Linux' && '1' || '' }}
       - run: cargo test --doc --no-default-features --features ${{ matrix.lua }}
       # The workspace default member is only the root `pmacs` package, so
       # the runs above never execute pmacs-protocol's own tests — the

README.md

@@ -220,6 +220,13 @@ translation) are routed through trampolines that exec these tools.
   shell-locator helper to find `bash` / `zsh` / `fish` for
   per-shell integration tests. The M7.2 fetcher's timeout test
   uses `sleep`.
+- **`setsid`** (util-linux, Linux only, **optional**). The process
+  teardown-deadlock test uses `setsid --fork` to orphan a grandchild,
+  which is the only way to reproduce that deadlock without depending on
+  shell `&` semantics (they differ between `bash` and `dash`). The test
+  **skips** when `setsid` is absent, so a minimal or BusyBox environment
+  still runs `cargo test --lib`; set `PMACS_REQUIRE_SETSID=1` to make
+  that skip a failure, as CI does on Linux.
 - **`git`** (added in M7.2). Required for any package operation:
   the package fetcher shells out to `git` to clone, fetch, and
   resolve refs, with a deterministic environment

docs/active-work.md

@@ -1069,10 +1069,19 @@ has **no branch and no framing yet**.
   `-- --skip basedpyright` into every gate recipe. The handoff's §3
   claim that the desktop's binary was broken is **retired by this PR**:
   the binary was fine. `basedpyright-langserver` is a uv console script
-  that spawns bundled `node` and exits, so the real server is an
+  that runs bundled `node` via `subprocess.run` and **waits**; at
-  orphaned grandchild (`PPid: 1`) holding the pipes; a direct binary
+  teardown `shutdown()` SIGTERMs the recorded pid (the wrapper), which
-  like `clangd` is a genuine child whose pipes close on reap. That is
+  dies without forwarding, and **that** orphans node to `PPid: 1`
-  the whole of the "intermittent" story.
+  holding the pipes. A direct binary like `clangd` is a genuine child
+  whose pipes close on reap. That is the whole of the "intermittent"
+  story.
+- **Corrected in review round 2:** rev 1–3 said the wrapper "spawns node
+  and exits". Wrong — and refutable from evidence already in hand, since
+  the initialize handshake succeeds, which a wrapper that exited at spawn
+  could not have done. The `PPid: 1` observation was taken *after*
+  `shutdown()` had killed the wrapper. **We create the orphan.** The fix
+  is unaffected; the parked follow-up changes from "tolerate
+  self-orphaning servers" to "stop orphaning them" (signal the group).
 - **Diagnosis method, because reproduce-first was the instruction:**
   gdb thread stacks plus `/proc` fd forensics on a live wedged process,
   both pipe ends identified in both processes, reproduced 5/5. Three
@@ -1108,10 +1117,18 @@ has **no branch and no framing yet**.
   while `write_all` is blocked); the orphaned-server **leak** — post-fix
   the server exits by cooperation, not enforcement.
 - `CLAUDE.md`'s `--skip basedpyright` entry is deliberately untouched.
-  Dropping it is a separate proposal owed evidence of repeated green,
+  Dropping it is a separate proposal owed evidence of repeated green.
-  and it must not precede the per-test timeout lane —
+  The timeout precondition is **already satisfied** — #195 (this PR's
-  `PMACS_REQUIRE_PYRIGHT` stays unarmed in CI until then, or CI inherits
+  base) gave every job a `timeout-minutes` — so the only remaining reason
-  the unbounded hang this PR removes locally.
+  `PMACS_REQUIRE_PYRIGHT` stays unarmed is that CI does not install
+  basedpyright at all; arming it would fail rather than test anything.
+- Adds `PMACS_REQUIRE_SETSID`, armed on Linux. The teardown test's
+  fixture needs `setsid --fork`, which is util-linux rather than
+  coreutils, so it **skips** when absent (the standard `--lib` gate must
+  not hard-fail a minimal container on an undeclared tool) and the
+  variable makes that skip fatal where the tool is guaranteed. Both arms
+  verified against a PATH with `setsid` genuinely removed: unarmed skips,
+  armed FAILS. README's test-dependency list declares it.
 
 ## Parked lane: kill-ring browser + persistence
 

docs/agent-handoff.md

@@ -1292,18 +1292,29 @@ before trusting them:
 - **basedpyright**: the desktop binary was **never broken** — this was a
   real code defect, diagnosed and fixed 2026-07-29 (see §5, "A `Drop`
   body runs before its fields"). `RuntimeHandles::drop` joined its reader
-  threads before the `stdin` field dropped, so a shim-launched server
+  threads before the `stdin` field dropped, so the server never got stdin
-  (basedpyright's console script spawns bundled `node` and exits, leaving
+  EOF, never exited, and kept the output pipe the readers were blocked
-  the real server at `PPid 1`) never got stdin EOF, never exited, and
+  on. Deterministic on the desktop, invisible on the laptop and in CI,
-  kept the output pipe the readers were blocked on. Deterministic on the
+  which is why it read as a broken local binary for weeks.
-  desktop, invisible on the laptop and in CI, which is why it read as a
+  **How the orphan is actually made — WE make it.** basedpyright's
-  broken local binary for weeks.
+  console script runs bundled `node` through `subprocess.run` and
+  **waits** (`nodejs_wheel/executable.py:50`, verified in 1.39.6). At
+  teardown `shutdown()` SIGTERMs the *recorded* pid — the Python wrapper
+  — which dies without forwarding the signal, orphaning node to `PPid 1`
+  holding the pipes. An earlier revision of this entry said the wrapper
+  "spawns node and exits"; that was wrong, and the refutation was already
+  in hand, since the initialize handshake succeeds, which a
+  wrapper that exited at spawn could not have done. The consequence is
+  for the follow-up, not the fix: the orphan-management work is **stop
+  orphaning them** (signal the group), not tolerate self-orphaning.
   The `--skip` above stays for now: it is still correct on any tree
-  predating the fix, and CI never installs basedpyright at all
+  predating the fix, and — the one live reason — **CI never installs
-  (`PMACS_REQUIRE_PYRIGHT` is deliberately unarmed, #194, and stays that
+  basedpyright at all**, so arming `PMACS_REQUIRE_PYRIGHT` would fail
-  way until the per-test timeout lane lands — arming it without a timeout
+  rather than test anything. The two original reasons are both gone: the
-  would hand CI an unbounded hang). Dropping the skip is a separate
+  hang is fixed, and #195 gave every job a `timeout-minutes`, so a hang
-  proposal, owed evidence of repeated green runs.
+  can no longer burn six hours. Installing basedpyright in CI (a uv plus
+  bundled-node download per leg) and dropping the local skip are two
+  separate proposals, each owed its own evidence.
 - **GPU on the laptop**: AMD Radeon 780M (RADV) — native Vulkan,
   `PMACS_REQUIRE_GPU=1` works without lavapipe.
 - **Flaky-under-load tests — rerun isolated before treating a sweep

docs/process-teardown-stdin-deadlock-framing.md

@@ -32,6 +32,14 @@ new primitive.**
   stdin writer (a child that read stdin but stopped draining it),
   criterion 5 extended to `docs/agent-handoff.md` §3, Bet 4 marked as
   lane-stopping.
+- **rev 4** — review round 2. §1.7's causal account was **wrong**: the
+  basedpyright wrapper uses `subprocess.run` and *waits*; the orphan is
+  created by pmacs SIGTERMing the wrapper at shutdown, not by the wrapper
+  exiting at spawn. Corrected here, in the handoff and in the ledger, and
+  §5's P2 restated — the follow-up is "stop orphaning them", not "tolerate
+  self-orphaning". Also: the `setsid` dependency is now skip-unless-armed
+  rather than a hard assert, since it is util-linux and the standard
+  `--lib` gate must not fail on an undeclared tool.
 - **rev 3** — CI falsified rev 2's repair. `<&0` is defeated on `dash`
   (the rule applies *before* explicit redirections, so `<&0` duplicates
   `/dev/null` onto itself); it passed locally only because `/bin/sh` here
@@ -198,12 +206,34 @@ from basedpyright.langserver import main
 sys.exit(main())
 ```
 
-`main()` spawns the bundled `node …/langserver.index.js --stdio` and the
+`main()` reaches `run_node.run`, which calls `nodejs_wheel`'s `node(...)`
-Python process exits, so the real server is an **orphaned grandchild**
+— and that is **`subprocess.run`** (`nodejs_wheel/executable.py:50`). It
-(observed `PPid: 1`, reparented to systemd) holding the inherited pipe
+**waits**. Verified in the installed 1.39.6 source, not assumed.
-fds. The supervisor recorded the shim's pid, which has already exited and
+
-been reaped, so `poll_one` sees a terminated process on its very first
+So the wrapper does *not* exit at spawn time, and **pmacs creates the
-tick and proceeds straight into the deadlock.
+orphan itself**:
+
+1. The wrapper runs `node …/langserver.index.js --stdio` and blocks. Node
+   is a genuine grandchild; the initialize handshake completes normally.
+2. At teardown, `shutdown()` sends **SIGTERM to the recorded pid** — the
+   Python wrapper — before entering its grace loop.
+3. The wrapper dies on the default disposition and **does not forward the
+   signal**. Node is reparented to `PPid: 1`, still holding the inherited
+   pipes, idle in `ep_poll`.
+4. `poll_one` then observes the recorded pid terminated, drops
+   `RuntimeHandles`, and enters the deadlock.
+
+**rev 1–3 of this doc said the wrapper "spawns node and exits".** That was
+wrong, and the evidence against it was already in hand: the test's
+assertions all pass *before* teardown, so the handshake succeeded — which
+is impossible if the wrapper had exited at spawn. The observation that
+generated the claim (`PPid: 1`, wrapper gone) was taken **after**
+`shutdown()` had already killed it.
+
+This matters for the parked work, not for the fix. The follow-up is not
+"tolerate servers that self-orphan" — it is **stop orphaning them**:
+signal the process group rather than a wrapper pid that swallows the
+signal. P2 in §5 is restated accordingly.
 
 `clangd` and `gopls` are real binaries: genuine children, reaped
 normally, write ends closed, blocking `read` returns `Ok(0)` cleanly. The
@@ -481,10 +511,16 @@ CI the same unbounded hang this PR removes locally.
   named as a deferral by `spawn_reader`'s own doc comment. Tests: the
   `sleep 300` shape from Q#TD6 (child never reads stdin), plus a
   fill-the-pipe-then-stop-reading shape for the writer case.
-- **P2 — orphaned-grandchild lifecycle (Q#TD5).** Spawn stdio servers in
+- **P2 — stop orphaning wrapper-launched servers (Q#TD5).** Restated in
-  their own process group and signal the group, reusing the machinery the
+  rev 4, because the corrected §1.7 changes the target: the orphan is not
-  group path and `reap_ledger` already have. Fixes a real leak: every
+  self-inflicted by the server, it is created by **us** SIGTERMing a
+  wrapper that does not forward the signal. Spawn stdio servers in their
+  own process group and signal the group, reusing the machinery the group
+  path and `reap_ledger` already have. Fixes a real leak: every
   basedpyright-backed session currently leaves a `node` process behind.
+  Note the ordering consequence — a group-directed SIGTERM would reach
+  node directly, so this also removes the condition the present fix works
+  around, rather than merely tolerating it.
 - **P3 — join the stdin writer thread** so the final flush is ordered
   against child termination (Q#TD4).
 - **P4 — re-audit the "intermittent" label** in `docs/agent-handoff.md`

src/process.rs

@@ -3284,15 +3284,29 @@ mod tests {
             }
         }
 
-        // Asserted, not skipped: this test is already Linux-gated, and
+        // setsid(1) is util-linux, not coreutils, and the standard
-        // setsid(1) is core util-linux. A skip here would reintroduce
+        // `cargo test --lib` gate must not hard-fail on a tool the
-        // exactly the silent-green shape the arming lane removed.
+        // README does not require --- a minimal or BusyBox container
-        assert!(
+        // would fail without ever testing pmacs. So: skip when absent,
-            binary_available("setsid"),
+        // but FAIL when `PMACS_REQUIRE_SETSID` is set, which CI sets on
-            "setsid(1) is required to orphan the grandchild without a \
+        // Linux. That is the arming pattern from the silent-skip lane,
-             shell; it is core util-linux and should be present on any \
+        // and it is what keeps this from becoming a test that reports
-             Linux runner"
+        // `ok` having never run. Presence decides, so an empty value
-        );
+        // counts as unset (a `${{ cond && '1' || '' }}` expression sets
+        // the empty string, not nothing).
+        let armed = std::env::var_os("PMACS_REQUIRE_SETSID").is_some_and(|v| !v.is_empty());
+        if !binary_available("setsid") {
+            assert!(
+                !armed,
+                "PMACS_REQUIRE_SETSID is set but setsid(1) is not on PATH: \
+                 install util-linux, or unset the variable to allow the skip"
+            );
+            eprintln!(
+                "setsid(1) not on PATH; skipping \
+                 teardown_closes_stdin_before_joining_readers"
+            );
+            return;
+        }
 
         let (done_tx, done_rx) = mpsc::channel();
         let handle = std::thread::spawn(move || {