From 76cb13bda8b1e3bdc3abf884119c91efde54532e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 11:39:40 -0400 Subject: [PATCH 1/8] docs(process): frame the signal diagnostic completeness lane (rev 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage B of the lane that merged as PR #176. Stage A parked every tolerance rule behind one condition — evidence — and the macOS failure on PR #191 is the first occurrence carrying Stage A's self-describing format, so the evidence now exists. It does not support any parked rule, and per review round 1 it does not support an identity claim either. This lane is evidence collection only: no tolerance rule, no retargeting, no disposition change. Round 1 raised two blocking and two major findings. All four are accepted, and all four were verified against the code first: - the landed acceptance at src/process.rs:2400 asserts the same pid three times, so substituting leader_pid for tcgetpgrp would pass it; - a numeric pgid cannot establish group identity, and no portable mechanism can (pidfd covers processes, not groups; macOS has none); - the PTY foreground-lookup fallback renders identically to a normal pipe child, and portable-pty discards the tcgetpgrp errno before pmacs sees it; - the report never names the signal, though disposition differs between fatal and non-fatal ones. Revision 1 was written to a session scratchpad rather than a branch, so it was not on githubsucks and review necessarily landed on Stage A's merged document instead. That is recorded in the revision history: work is portable only after it is committed and pushed, and this lane broke that rule on its first step. No code changes. Framing only, awaiting review round 2. --- ...-signal-diagnostic-completeness-framing.md | 338 ++++++++++++++++++ 1 file changed, 338 insertions(+) create mode 100644 docs/process-signal-diagnostic-completeness-framing.md diff --git a/docs/process-signal-diagnostic-completeness-framing.md b/docs/process-signal-diagnostic-completeness-framing.md new file mode 100644 index 0000000..37dbb95 --- /dev/null +++ b/docs/process-signal-diagnostic-completeness-framing.md @@ -0,0 +1,338 @@ +# Framing — make the signal diagnostic discriminating (evidence collection) + +**Revision 2.** Status: awaiting review round 2. Lane: +`process-signal-diagnostic-completeness`, worktree +`../pmacs-signal-identity`, based on `githubsucks/main` @ `391d38a`. + +**This is Stage B of the lane whose Stage A merged as PR #176** +(`docs/process-signal-tolerance-framing.md`, revision 4). Stage A made a +failing `kill` self-describing and parked every tolerance rule behind one +condition: evidence. Evidence has arrived (§1.2). It does not support any +of the parked rules, and — per review round 1 — it does not support an +identity claim either. + +**Evidence collection only. No tolerance rule, no change to which +process gets signalled, no disposition change.** Everything behavioural +is parked in §5. + +## Revision history + +**Revision 1 → 2**, after review round 1 (two blocking, two major). All +four accepted; all four verified against the code before acceptance. + +- **Rev 1 proposed measuring the pgid and then *targeting* it.** That is + a behavioural change resting on an identity claim a numeric pgid cannot + support (§1.5). Retargeting is parked (§5); this lane only *records*. +- **Rev 1 repeated the defect it was written to fix.** Its Bet 1 asserted + `getpgid(child) == pid`, which an implementation that ignored `getpgid` + and returned `pid` would satisfy — the same non-discrimination the + review found in the landed acceptance (§1.4). Every measurement in this + revision now requires a case where the two values **differ**. +- **Rev 1 scoped the PTY path out.** Review showed the PTY fallback is + indistinguishable from a normal pipe child in the report (§1.6), which + is a defect in the diagnostic this lane owns. Scope now includes it, + and the rev 1 sentence "this lane does not touch the PTY path" is + withdrawn. +- **Rev 1 never noticed the report omits the signal** (§1.7). + +**Rev 1 also mis-stated where the lane lived.** It was written to a +session scratchpad, which is not portable and was not on `githubsucks`, +so review round 1 necessarily landed on Stage A's revision 4 instead. +That is why round 1's line references point at the merged document. The +findings apply regardless — three of the four are defects in code that is +on `main` right now — but the process error is recorded here because +"work is portable only after it is committed and pushed" is a standing +project rule and this lane broke it on its first step. + + +## 0. Coherence impact (COHERENCE §20) + +- **Journey step 8, "Open a terminal"** (§2), teardown half, plus every + compile/grep run through `spec.group`. **No grade change and no + behavioural change.** +- **Serves §9 (worker model), failure attribution.** Stage A made the + failure describe itself; this lane makes the description + *discriminating* — today several distinct failures render identically. +- **Interaction islands: none. Config registry: not adopted. + Background-work attribution: unchanged.** +- **No audited claim in COHERENCE.md changes**, so under §25 no + COHERENCE edit rides this PR. + + +## 1. Ground truth (verified at `391d38a`) + +### 1.1 Stage A landed and has now fired + +`signal_failure_report` and `LeaderObservation` merged as **PR #176 on +2026-07-26** (`62316a9`). The occurrence below is the first failure +carrying the new format rather than a bare errno. Stage A is the reason +this document can exist. + +### 1.2 The new occurrence, verbatim + +PR #191, `Test (macos-latest / lua54)`, +[run 30553376486](https://github.com/levineuwirth/pmacs/actions/runs/30553376486/job/90907461258), +`process::tests::repeated_terminate_does_not_extend_ledger_deadline`. +1873 passed, 1 failed. **The rerun of the identical head passed 12/12**, +so the failure is intermittent, not deterministic: + +``` +re-terminate: "kill: EPERM: Operation not permitted + (target=-8619 via group, leader_pid=8619, expected_group=-8619, leader=live)" +``` + +Established: the target source is `group` (the `spec.group` pipe path, +`signal_target` `:774-780`; `sh_group_spec` `:3402` sets it), **not** the +PTY path; and `leader=live`, from a real `try_wait` against the real +child, so the leader had not exited and had not been reaped. + +### 1.3 Two written premises are falsified + +- **`src/process.rs:1246-1247`** — `tick_reap_ledger` justifies treating + any probe error as "nothing left we can reach" with the comment + "**EPERM cannot happen for our own children**". §1.2 is a + counterexample: our own child, alive, EPERM. +- **Stage A §1.3** — "the reap ledger is disjoint from this path", on the + grounds that the ledger arms only for `proc.spec.group` and PTY mode + cannot set it. §1.2's process **is** a `spec.group` process. Not + disjoint. + +Stage A's §1.2 entity-split analysis concerns the PTY path, where the +target is read from `tcgetpgrp`. **It does not apply to §1.2's +occurrence**, where the target is computed as `-leader_pid` with no read. + +### 1.4 The landed acceptance cannot discriminate (round 1, P1) + +`a_group_directed_kill_failure_reports_target_and_leader_separately` +(`:2400`) spawns a PTY child and asserts the exact string + +``` +target=-{pid} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live +``` + +— the same `pid` three times. **An implementation that ignored +`tcgetpgrp` entirely and substituted `leader_pid` would pass.** The test's +own doc comment concedes it: "here they are asserted to agree only +because nothing has moved the terminal." + +The premise of the whole diagnostic is that these two entities can +diverge, and no test exercises a case where they do. This is the +vacuous-assertion family already recorded in the handoff. + +### 1.5 A numeric pgid cannot establish identity (round 1, P1) + +Rev 1 proposed reading the real pgid and targeting it. Review is right +that this does not establish identity: the value is read before `kill`, +the read-then-kill window remains, and a *number* cannot distinguish the +original group from a recycled one. + +**There is no portable mechanism that closes this.** `pidfd_open` + +`pidfd_send_signal` close pid reuse for a single *process* on Linux; +there is no process-*group* equivalent, and macOS has no pidfd at all. +Since the failures are macOS-only so far, no available mechanism makes +group signalling identity-safe. + +**Therefore this lane records and does not retarget.** Acceptance +criteria state what was observed, never that the observation is +sufficient. + +One narrowing fact, stated because it constrains the hypothesis space +and *not* as an identity claim: POSIX does not free a child's pid until +the parent reaps it, and §1.2 observed `leader=live` from a `try_wait` +that had not reaped. While that pid is held, no other process can be +assigned it, so no *new* group can be created bearing that pgid value +during the window. This narrows recycling as a candidate **for that one +occurrence**; it says nothing about whether the group still contained a +signallable member, which is the question EPERM actually turns on. + +### 1.6 The PTY fallback is invisible in the report (round 1, P2) + +`signal_target` (`:757-785`): when the PTY branch's +`master.process_group_leader()` returns `None`, control falls through — +`spec.group` is rejected at spawn for PTY mode — and returns +`TargetSource::LeaderPid`, rendered "leader-pid" (`:738`). **A normal +pipe child renders identically.** Two different situations, one string. + +`portable-pty`'s implementation +(`portable-pty-0.9.0/src/unix.rs:374`) is: + +```rust +fn process_group_leader(&self) -> Option { + match unsafe { libc::tcgetpgrp(self.fd.0.as_raw_fd()) } { + pid if pid > 0 => Some(pid), + _ => None, + } +} +``` + +The errno is discarded, so "the tty has no foreground group" and +"`tcgetpgrp` failed" are already merged before pmacs sees the result. + +**`nix::unistd::tcgetpgrp` returns `Result`** (`nix-0.29.0/src/unistd.rs:368`, +ungated, and `pub mod unistd` at `lib.rs:183` is unconditional), so pmacs +can make this call itself and keep the errno **without `unsafe`** — which +matters because the crate is `#![forbid(unsafe_code)]`. + +### 1.7 The report omits which signal failed (round 1, P2) + +`signal_failure_report` (`:830-850`) takes target, leader pid, errno and +leader observation. **Not the signal.** `signal` (`:1074`) has it. + +This is not cosmetic: `signal` transitions state and arms the reap ledger +only for `SIGTERM | SIGKILL | SIGHUP` (`:1099-1116`), and the public Lua +surface accepts INT, USR1, USR2 and QUIT as well (`src/lua_bindings/mod.rs:8627-8629`). +A failed `SIGUSR1` and a failed `SIGTERM` have different consequences and +currently produce indistinguishable text. + +### 1.8 The disposition consequence, worse for the first call + +`signal` returns `Err` **before** the state transition and **before** +arming the ledger: + +| Which `terminate` hits EPERM | Consequence | +|---|---| +| A **later** one (§1.2's case) | Caller sees `Err`; state already `Exiting`, ledger already armed, so SIGKILL escalation still happens. | +| The **first** one | State stays `Running`, ledger never armed. **No escalation is ever scheduled** — the child is abandoned. | + +Only the recoverable variant has been observed. This lane **pins** the +first-call variant and changes nothing about it (§5). + +### 1.9 Limits of the evidence + +- **Not reproduced locally.** Development is Linux; the failures are + macOS-only. No claim in this lane rests on a local repro of the EPERM. +- **Frequency: two occurrences, in different paths** — PR #172 was the + PTY path (`acc28`, luajit), §1.2 the group path (lua54). This is not + one flaky test. +- **The mechanism is not established**, and this lane does not propose + one. That is the point of the split. + + +## 2. Questions + +- **Q#DC1** — Can the two entities be made to diverge in a test? *Yes: + under a PTY with job control enabled, a shell places a background job + in its own process group and hands it the terminal, so `tcgetpgrp` != + leader pid. §3 Bet 1 builds exactly that.* +- **Q#DC2** — Should the PTY fallback get its own `TargetSource`? + *Proposed: yes, and pmacs should call `nix::unistd::tcgetpgrp` itself so + the fallback can report the errno rather than inheriting portable-pty's + discarded `None`.* +- **Q#DC3** — Should the report name the signal? *Proposed: yes, with a + contrasting non-fatal signal tested.* +- **Q#DC4** — Should the measured pgid be reported for `spec.group` + children? *Proposed: yes, as an observation clearly distinct from the + assumed value, and with no claim of sufficiency (§1.5).* +- **Q#DC5** — Should anything be retargeted or tolerated? **No. Parked.** + + +## 3. Bets + +Each bet names what falsifies it and what falsification teaches. + +- **Bet 1 — the divergence is constructible.** A PTY fixture where the + foreground group is *not* the leader: job control on, a background job + given the terminal. The rewritten acceptance asserts both exact values + and that they **differ**. + - *Falsified if* the fixture cannot be made deterministic in CI (shell + job-control timing). Then the lane says so and falls back to pinning + divergence at the `signal_target` unit level with an injected + foreground group, which is weaker and must be labelled as weaker. + - This is the finding that matters most: without it, the entire + diagnostic remains unverified in the only case it exists for. + +- **Bet 2 — the PTY fallback is reachable and distinguishable.** A test + drives the branch where the foreground-group lookup fails and asserts a + source string distinct from a pipe child's. + - *Falsified if* the branch cannot be reached without faking the + lookup. Then the seam is made injectable exactly as Stage A made the + kill injectable (Q#PD4), and that is stated rather than hidden. + +- **Bet 3 — naming the signal is free.** Thread `signal` into the report. + - *Falsified if* any existing exact-string test cannot be updated + mechanically. Those four sites (`:2408`, `:2435`, `:2485`, `:2517`) + are the highest-risk part of the diff: **a wholesale rewrite of + expected strings is how a format regression hides**, so each is + updated individually and listed in the PR body with before and after. + +- **Bet 4 — the measured pgid can disagree with the assumed one, and the + test proves the measurement is real.** A child that calls `setsid`, so + its pgid is genuinely not its parent-assumed value, is measured and the + two values asserted **different**. + - *Falsified if* no such case can be built — in which case the + measurement is unfalsifiable and should not ship, exactly per §1.4's + lesson. + +- **Bet 5 — §1.8's first-call abandonment is real.** Inject EPERM on the + *first* terminate; assert no ledger entry and state still `Running`. + - *Falsified if* the ledger is armed anyway, meaning §1.8 misreads + `signal`. + - **Pins current behaviour; does not fix it.** + + +## 4. Acceptance + +1. A PTY job-control fixture in which `tcgetpgrp` != leader pid, with + both exact values asserted and asserted to differ. The landed test at + `:2400` is **rewritten**, not supplemented, since it currently pins a + substitution as acceptable. +2. The PTY foreground-lookup fallback reports a source distinct from a + pipe child's leader-pid, with a test driving the real branch. +3. The report names the signal; at least one non-fatal signal + (`SIGUSR1`) is tested alongside `SIGTERM`, including that its + disposition differs. +4. For `spec.group` children the report carries the measured pgid as a + field distinct from the assumed one, renderable as unobservable, and + a test asserts a case where they **differ**. +5. Every exact-string test updated individually, each listed in the PR + body with before and after. No blanket rewrite. +6. §1.8's first-call abandonment pinned, labelled as pinning a known gap. +7. `docs/agent-handoff.md` records that "EPERM cannot happen for our own + children" is false, with the run link; the comment at `:1246` is + corrected in the same PR. +8. **No acceptance claims the telemetry establishes group identity.** + §1.5 governs; the PR body repeats it. + + +## 5. Parked + +- **Retargeting to the measured pgid.** Behavioural, and unsupported by + §1.5. Needs this lane's evidence first. +- **Any tolerance rule for EPERM or ESRCH.** Unmotivated across Stage A's + three revisions and still unmotivated. +- **§1.8's first-call abandonment.** Its own lane; disposition change. +- **Q#PS6** — `terminate` on an already-reaped process returning `Ok`. +- **`signal_target`'s read-then-kill of `tcgetpgrp`** — Stage A called it + "the most likely real fix site, still unframed". Still is. This lane + makes it *observable*, not fixed. +- **`compile_mode_acceptance` reading the developer's real + `~/.config/pmacs/init.lua`** — separate defect (11 local failures, + invisible in CI), unrelated to signals. + + +## 6. Gates + +Standard suite, each its own step with a real exit status, nothing after +the command that could mask it: `cargo fmt --check`; `cargo clippy +--workspace --all-targets -- -D warnings`; `cargo test --lib`; `cargo +test --lib --features crdt`; `compile_mode_acceptance`; +`terminal_copy_mode_acceptance` (both feature configurations); +`bottom_panel_stage1_acceptance`; `cargo test --test m4_acceptance -- +--skip basedpyright`; `PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`; +`git diff --check`. + +**All local runs use an isolated `XDG_CONFIG_HOME`** — without it +`compile_mode_acceptance` fails 11 tests for reasons unrelated to this +lane. + +**PTY job-control tests are the load-sensitive kind.** They are run +repeatedly, and the PR body records the repetition count rather than a +single green. + + +## 7. Branch plan + +One branch, one PR. Order: Bet 1 first and alone, because it is the +finding that decides whether the diagnostic is worth extending at all. If +the divergence fixture cannot be made deterministic, the rest of the lane +is re-scoped rather than pushed through. From 872e4aacad6d13340fb9113f786a17f2aa98616f Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 12:09:34 -0400 Subject: [PATCH 2/8] =?UTF-8?q?docs(process):=20framing=20revision=203=20?= =?UTF-8?q?=E2=80=94=20close=20review=20round=202?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three blocking and three major findings, all accepted, all verified in the code before acceptance. - The PTY errno proposal is withdrawn. `nix::unistd::tcgetpgrp` needs `F: AsFd`; `MasterPty` exposes only `Option`; every route from a raw fd to `AsFd` is unsafe and the crate forbids unsafe. The claim is reduced to distinguishing the fallback without the errno. - Revision 2 called `getpgid` ungated. It is not: the gate is a block-level `feature!` opened 168 lines above the function, and rev 2 read the four lines immediately above it. Both calls are available because nix's own `signal` feature depends on `process`, verified with `cargo tree` and by compiling them. The lane now declares `process` explicitly rather than inheriting it. - Bet 4's `setsid` fixture was impossible: a `spec.group` child is already a process-group leader and a group leader's `setsid` fails with EPERM. Replaced with a `setpgid`-into-an-existing-group fixture with a readiness handshake and explicit cleanup. - "Recoverable" is withdrawn. `tick_reap_ledger` drops its entry on any probe error — including the EPERM this occurrence proves reachable — and discards the SIGKILL result while marking the entry killed. The claim is now that escalation remains scheduled, and that still-silent path is parked as its own lane. - Revision 2 falsified the wrong sentence. Stage A's disjointness claim was about the PTY path and remains true; what the occurrence falsifies is the ledger's own EPERM comment. - Failed SIGUSR1 and SIGTERM are disposition-identical, because every failed kill returns before the fatal-signal branch. Acceptance 3 now separates the failure-format comparison from a successful-call disposition control. - Bet 5 proposed a test that already exists at src/process.rs:2501. It is cited as ground truth and retained, not invented. No code changes. Framing only, awaiting review round 3. --- ...-signal-diagnostic-completeness-framing.md | 453 ++++++++++-------- 1 file changed, 261 insertions(+), 192 deletions(-) diff --git a/docs/process-signal-diagnostic-completeness-framing.md b/docs/process-signal-diagnostic-completeness-framing.md index 37dbb95..7d98975 100644 --- a/docs/process-signal-diagnostic-completeness-framing.md +++ b/docs/process-signal-diagnostic-completeness-framing.md @@ -1,107 +1,120 @@ # Framing — make the signal diagnostic discriminating (evidence collection) -**Revision 2.** Status: awaiting review round 2. Lane: +**Revision 3.** Status: awaiting review round 3. Lane: `process-signal-diagnostic-completeness`, worktree -`../pmacs-signal-identity`, based on `githubsucks/main` @ `391d38a`. +`../pmacs-signal-identity`, based on `githubsucks/main` @ `4cd4a7b` +(re-measure at branch time; this is a reading, not a constant). **This is Stage B of the lane whose Stage A merged as PR #176** (`docs/process-signal-tolerance-framing.md`, revision 4). Stage A made a -failing `kill` self-describing and parked every tolerance rule behind one -condition: evidence. Evidence has arrived (§1.2). It does not support any -of the parked rules, and — per review round 1 — it does not support an -identity claim either. +failing `kill` self-describing and parked every tolerance rule behind +evidence. Evidence arrived (§1.2). It supports none of the parked rules, +no identity claim, and — per review round 2 — no claim that today's +escalation path is safe. -**Evidence collection only. No tolerance rule, no change to which -process gets signalled, no disposition change.** Everything behavioural -is parked in §5. +**Evidence collection only. No tolerance rule, no change to which process +gets signalled, no disposition change.** Everything behavioural is in §5. ## Revision history -**Revision 1 → 2**, after review round 1 (two blocking, two major). All -four accepted; all four verified against the code before acceptance. +**Revision 2 → 3**, after review round 2 (three blocking, three major). +All six accepted; all six verified in the code before acceptance. -- **Rev 1 proposed measuring the pgid and then *targeting* it.** That is - a behavioural change resting on an identity claim a numeric pgid cannot - support (§1.5). Retargeting is parked (§5); this lane only *records*. -- **Rev 1 repeated the defect it was written to fix.** Its Bet 1 asserted - `getpgid(child) == pid`, which an implementation that ignored `getpgid` - and returned `pid` would satisfy — the same non-discrimination the - review found in the landed acceptance (§1.4). Every measurement in this - revision now requires a case where the two values **differ**. -- **Rev 1 scoped the PTY path out.** Review showed the PTY fallback is - indistinguishable from a normal pipe child in the report (§1.6), which - is a defect in the diagnostic this lane owns. Scope now includes it, - and the rev 1 sentence "this lane does not touch the PTY path" is - withdrawn. -- **Rev 1 never noticed the report omits the signal** (§1.7). +- **The PTY errno proposal had no safe fd bridge.** Withdrawn and reduced + (§1.6). This was rev 2's central new proposal and it does not survive + `#![forbid(unsafe_code)]`. +- **Rev 2 said `getpgid` was "ungated". It is not** (§1.5a). The claim + came from reading the four lines above the function; the gate is a + block-level `feature!` opened 168 lines earlier. Same error shape as + the truncated-output trap already in the handoff, committed inside a + document about non-discriminating evidence. +- **Bet 4's `setsid` fixture was impossible** (§3, Bet 4). A `spec.group` + child is already a process-group leader, and a group leader's `setsid` + fails with EPERM. +- **"Recoverable" was unsupported** (§1.8). The ledger drops its entry on + *any* probe error — including the EPERM this occurrence proves can + happen — and discards the `SIGKILL` result while marking the entry + killed. +- **Rev 2 falsified the wrong Stage A sentence** (§1.3). Stage A's + disjointness claim was about the **PTY** path and remains true. +- **Rev 2's signal-disposition argument was wrong** (§1.7). Failed + signals are all disposition-identical. +- **Bet 5 proposed a test that already exists** (§1.9). Cited as ground + truth now, not invented. -**Rev 1 also mis-stated where the lane lived.** It was written to a -session scratchpad, which is not portable and was not on `githubsucks`, -so review round 1 necessarily landed on Stage A's revision 4 instead. -That is why round 1's line references point at the merged document. The -findings apply regardless — three of the four are defects in code that is -on `main` right now — but the process error is recorded here because -"work is portable only after it is committed and pushed" is a standing -project rule and this lane broke it on its first step. +**Revision 1 → 2**, after review round 1 (two blocking, two major); all +accepted. Rev 1 proposed *retargeting* to a measured pgid — a behavioural +change resting on an identity claim a number cannot support; it asserted +`getpgid(child) == pid`, which an implementation ignoring `getpgid` would +satisfy; it scoped the PTY path out; and it never noticed the report +omits the signal. + +**Rev 1 was written to a session scratchpad rather than a branch**, so it +was never on `githubsucks` and review round 1 necessarily landed on +Stage A's merged document. Recorded because "work is portable only after +it is committed and pushed" is a standing rule this lane broke on its +first step. ## 0. Coherence impact (COHERENCE §20) - **Journey step 8, "Open a terminal"** (§2), teardown half, plus every - compile/grep run through `spec.group`. **No grade change and no + compile/grep run through `spec.group`. **No grade change, no behavioural change.** - **Serves §9 (worker model), failure attribution.** Stage A made the failure describe itself; this lane makes the description - *discriminating* — today several distinct failures render identically. + *discriminating*, because several distinct failures render identically + today. - **Interaction islands: none. Config registry: not adopted. Background-work attribution: unchanged.** -- **No audited claim in COHERENCE.md changes**, so under §25 no - COHERENCE edit rides this PR. +- **No audited claim in COHERENCE.md changes**; under §25 no COHERENCE + edit rides this PR. -## 1. Ground truth (verified at `391d38a`) +## 1. Ground truth (verified at `4cd4a7b`) ### 1.1 Stage A landed and has now fired `signal_failure_report` and `LeaderObservation` merged as **PR #176 on -2026-07-26** (`62316a9`). The occurrence below is the first failure -carrying the new format rather than a bare errno. Stage A is the reason -this document can exist. +2026-07-26** (`62316a9`). §1.2 is the first failure carrying the new +format rather than a bare errno. Stage A is why this document can exist. ### 1.2 The new occurrence, verbatim PR #191, `Test (macos-latest / lua54)`, [run 30553376486](https://github.com/levineuwirth/pmacs/actions/runs/30553376486/job/90907461258), `process::tests::repeated_terminate_does_not_extend_ledger_deadline`. -1873 passed, 1 failed. **The rerun of the identical head passed 12/12**, -so the failure is intermittent, not deterministic: +1873 passed, 1 failed. **A rerun of the identical head passed 12/12**, so +the failure is intermittent, not deterministic: ``` re-terminate: "kill: EPERM: Operation not permitted (target=-8619 via group, leader_pid=8619, expected_group=-8619, leader=live)" ``` -Established: the target source is `group` (the `spec.group` pipe path, -`signal_target` `:774-780`; `sh_group_spec` `:3402` sets it), **not** the -PTY path; and `leader=live`, from a real `try_wait` against the real -child, so the leader had not exited and had not been reaped. +Established: the target source is `group` — the `spec.group` pipe path +(`signal_target` `:774-780`; `sh_group_spec` `:3402`) — **not** the PTY +path; and `leader=live`, from a real `try_wait` against the real child, +so the leader had neither exited nor been reaped. -### 1.3 Two written premises are falsified +### 1.3 What is actually falsified -- **`src/process.rs:1246-1247`** — `tick_reap_ledger` justifies treating - any probe error as "nothing left we can reach" with the comment - "**EPERM cannot happen for our own children**". §1.2 is a +- **`src/process.rs:1246-1247` is falsified.** `tick_reap_ledger` + justifies treating any probe error as "nothing left we can reach" with + the comment "**EPERM cannot happen for our own children**". §1.2 is a counterexample: our own child, alive, EPERM. -- **Stage A §1.3** — "the reap ledger is disjoint from this path", on the - grounds that the ledger arms only for `proc.spec.group` and PTY mode - cannot set it. §1.2's process **is** a `spec.group` process. Not - disjoint. +- **Stage A §1.3 is *not* falsified.** It said the ledger is disjoint + from **the PTY path**, because the ledger arms only for + `proc.spec.group` and PTY mode cannot set it. That remains true. §1.2 + is the separate `spec.group` path. Rev 2 conflated "this path" with + "the signal path generally" and claimed a falsification it had not + made. -Stage A's §1.2 entity-split analysis concerns the PTY path, where the -target is read from `tcgetpgrp`. **It does not apply to §1.2's -occurrence**, where the target is computed as `-leader_pid` with no read. +Stage A's entity-split analysis concerns the PTY path, where the target +is read from `tcgetpgrp`. It does not apply to §1.2, where the target is +computed as `-leader_pid` with no read at all. -### 1.4 The landed acceptance cannot discriminate (round 1, P1) +### 1.4 The landed acceptance cannot discriminate `a_group_directed_kill_failure_reports_target_and_leader_separately` (`:2400`) spawns a PTY child and asserts the exact string @@ -111,50 +124,71 @@ target=-{pid} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=liv ``` — the same `pid` three times. **An implementation that ignored -`tcgetpgrp` entirely and substituted `leader_pid` would pass.** The test's -own doc comment concedes it: "here they are asserted to agree only -because nothing has moved the terminal." +`tcgetpgrp` and substituted `leader_pid` would pass.** The test's own doc +comment concedes it: "here they are asserted to agree only because +nothing has moved the terminal." The premise of the diagnostic is that +these entities can diverge, and nothing exercises a case where they do. -The premise of the whole diagnostic is that these two entities can -diverge, and no test exercises a case where they do. This is the -vacuous-assertion family already recorded in the handoff. +### 1.5 A numeric pgid cannot establish identity -### 1.5 A numeric pgid cannot establish identity (round 1, P1) +The value is read before `kill`, the window remains open, and a *number* +cannot distinguish the original group from a recycled one. -Rev 1 proposed reading the real pgid and targeting it. Review is right -that this does not establish identity: the value is read before `kill`, -the read-then-kill window remains, and a *number* cannot distinguish the -original group from a recycled one. +**No portable mechanism closes this.** `pidfd_open` + `pidfd_send_signal` +close pid reuse for a single *process* on Linux; there is no +process-*group* equivalent, and macOS has no pidfd. The failures are +macOS-only, so nothing available makes group signalling identity-safe. -**There is no portable mechanism that closes this.** `pidfd_open` + -`pidfd_send_signal` close pid reuse for a single *process* on Linux; -there is no process-*group* equivalent, and macOS has no pidfd at all. -Since the failures are macOS-only so far, no available mechanism makes -group signalling identity-safe. +**This lane therefore records and does not retarget.** No acceptance +claims the telemetry is sufficient. -**Therefore this lane records and does not retarget.** Acceptance -criteria state what was observed, never that the observation is -sufficient. +One narrowing fact, stated as narrowing and **not** as identity: POSIX +does not free a child's pid until the parent reaps it, and §1.2 observed +`leader=live` from a `try_wait` that had not reaped. While that pid is +held, no *new* group can be created bearing that pgid value. This makes +recycling an unlikely explanation **for that one occurrence**, and says +nothing about whether the group still held a signallable member — which +is what EPERM actually turns on. -One narrowing fact, stated because it constrains the hypothesis space -and *not* as an identity claim: POSIX does not free a child's pid until -the parent reaps it, and §1.2 observed `leader=live` from a `try_wait` -that had not reaped. While that pid is held, no other process can be -assigned it, so no *new* group can be created bearing that pgid value -during the window. This narrows recycling as a candidate **for that one -occurrence**; it says nothing about whether the group still contained a -signallable member, which is the question EPERM actually turns on. +### 1.5a The nix surface is gated, and available for a non-obvious reason -### 1.6 The PTY fallback is invisible in the report (round 1, P2) +Both calls this lane would use live inside block-level gates: + +- `getpgid` (`nix-0.29.0/src/unistd.rs:335`) is inside + `feature! { #![feature = "process"] }` opened at `:167`. +- `tcgetpgrp` (`:368`) is inside + `feature! { #![all(feature = "process", feature = "term")] }` at `:360`. + +pmacs declares `nix` with `features = ["signal", "user", "fs", "term", +"socket", "poll"]` — **`process` is not listed**. It is enabled anyway +because **nix's own `signal` feature depends on `process`**, verified +with `cargo tree -e features -i nix:0.29.0`: + +``` +├── nix feature "process" +│ └── nix feature "signal" +│ └── pmacs v1.0.0 +``` + +Confirmed by compiling both calls against the real dependency graph. + +**This is stable but implicit.** The lane adds `process` to pmacs' own +feature list so the dependency is declared rather than inherited — a +one-line change that makes a real requirement visible. + +*Rev 2 asserted `getpgid` was "ungated", from reading the four lines +above it. The gate was 168 lines up. Recorded because it is the same +defect class this document exists to fix.* + +### 1.6 The PTY fallback is invisible, and the errno cannot be recovered `signal_target` (`:757-785`): when the PTY branch's `master.process_group_leader()` returns `None`, control falls through — `spec.group` is rejected at spawn for PTY mode — and returns `TargetSource::LeaderPid`, rendered "leader-pid" (`:738`). **A normal -pipe child renders identically.** Two different situations, one string. +pipe child renders identically.** Two situations, one string. -`portable-pty`'s implementation -(`portable-pty-0.9.0/src/unix.rs:374`) is: +`portable-pty` (`portable-pty-0.9.0/src/unix.rs:374`) discards the errno: ```rust fn process_group_leader(&self) -> Option { @@ -165,155 +199,193 @@ fn process_group_leader(&self) -> Option { } ``` -The errno is discarded, so "the tty has no foreground group" and -"`tcgetpgrp` failed" are already merged before pmacs sees the result. +**Rev 2 proposed calling `nix::unistd::tcgetpgrp` ourselves to keep the +errno. That is withdrawn.** `tcgetpgrp` requires `F: AsFd`, and +`MasterPty` exposes only `fn as_raw_fd(&self) -> Option` +(`portable-pty-0.9.0/src/lib.rs:114`). Every route from a `RawFd` to +something implementing `AsFd` — `BorrowedFd::borrow_raw`, +`OwnedFd::from_raw_fd`, `File::from_raw_fd` — is `unsafe`, and the crate +is `#![forbid(unsafe_code)]`. There is no safe bridge, and duplicating +the fd does not create one because the duplication itself needs the same +unsafe conversion. -**`nix::unistd::tcgetpgrp` returns `Result`** (`nix-0.29.0/src/unistd.rs:368`, -ungated, and `pub mod unistd` at `lib.rs:183` is unconditional), so pmacs -can make this call itself and keep the errno **without `unsafe`** — which -matters because the crate is `#![forbid(unsafe_code)]`. +**Reduced claim:** the fallback is distinguished **without** the errno. +`None` is all pmacs can observe, and the report says exactly that. The +errno is recorded here as unavailable-by-construction so a later lane +does not re-propose it. -### 1.7 The report omits which signal failed (round 1, P2) +### 1.7 The report omits which signal failed `signal_failure_report` (`:830-850`) takes target, leader pid, errno and -leader observation. **Not the signal.** `signal` (`:1074`) has it. +leader observation — **not the signal**. `signal` (`:1074`) has it, and +the public Lua surface accepts INT, USR1, USR2 and QUIT besides the fatal +three (`src/lua_bindings/mod.rs:8627-8629`). A failed `SIGUSR1` and a +failed `SIGTERM` are today textually indistinguishable. -This is not cosmetic: `signal` transitions state and arms the reap ledger -only for `SIGTERM | SIGKILL | SIGHUP` (`:1099-1116`), and the public Lua -surface accepts INT, USR1, USR2 and QUIT as well (`src/lua_bindings/mod.rs:8627-8629`). -A failed `SIGUSR1` and a failed `SIGTERM` have different consequences and -currently produce indistinguishable text. +**Rev 2 justified this by claiming their dispositions differ. That was +wrong.** `signal` returns `Err` at `:1092-1098`, *before* the fatal-signal +branch at `:1099`, so **every failed kill is disposition-identical** +regardless of signal. The disposition difference is real only for +**successful** calls. The reporting gap stands on its own: you cannot +tell which signal failed. Acceptance 3 separates the two. -### 1.8 The disposition consequence, worse for the first call +### 1.8 The disposition consequence, and why "recoverable" was wrong `signal` returns `Err` **before** the state transition and **before** arming the ledger: | Which `terminate` hits EPERM | Consequence | |---|---| -| A **later** one (§1.2's case) | Caller sees `Err`; state already `Exiting`, ledger already armed, so SIGKILL escalation still happens. | +| A **later** one (§1.2's case) | Caller sees `Err`. State is already `Exiting` and a ledger entry **remains scheduled**. | | The **first** one | State stays `Running`, ledger never armed. **No escalation is ever scheduled** — the child is abandoned. | -Only the recoverable variant has been observed. This lane **pins** the -first-call variant and changes nothing about it (§5). +**Rev 2 called the first row "recoverable" and claimed "SIGKILL +escalation still happens". Unsupported.** `tick_reap_ledger` +(`:1249-1254`): -### 1.9 Limits of the evidence +```rust +if nix::sys::signal::kill(Pid::from_raw(-*pgid), None).is_err() { + return false; // drops on ANY error, incl. EPERM +} +if now >= entry.deadline && !entry.killed { + let _ = nix::sys::signal::kill(..., Some(Signal::SIGKILL)); // result discarded + entry.killed = true; // marked killed regardless +} +``` -- **Not reproduced locally.** Development is Linux; the failures are - macOS-only. No claim in this lane rests on a local repro of the EPERM. -- **Frequency: two occurrences, in different paths** — PR #172 was the - PTY path (`acc28`, luajit), §1.2 the group path (lua54). This is not - one flaky test. +Since §1.3 falsifies the premise that EPERM cannot occur here, an EPERM +probe **drops the entry and cancels escalation silently**, and a failed +`SIGKILL` is recorded as if it succeeded. So the honest statement is that +escalation remains *scheduled*, not that it happens. + +**This still-silent path is parked, explicitly** (§5) rather than +absorbed: it is a second site with its own disposition questions, and +folding it in would repeat Stage A rev 3's error of implementing Stage B +inside Stage A. + +### 1.9 The first-call variant is already pinned + +`an_injected_failure_changes_no_state_and_arms_no_ledger` (`:2501`) +already spawns a `spec.group` child, injects EPERM on the **first** +`terminate`, and asserts `Running` plus an empty ledger. **Rev 2's Bet 5 +proposed inventing it.** It is ground truth, and its exact-string +assertion (`:2517`) is one of the four sites acceptance 5 must update. + +### 1.10 Limits of the evidence + +- **Not reproduced locally.** Development is Linux; failures are + macOS-only. No claim rests on a local repro of the EPERM. +- **Two occurrences, in different paths** — PR #172 was the PTY path + (`acc28`, luajit), §1.2 the group path (lua54). Not one flaky test. - **The mechanism is not established**, and this lane does not propose - one. That is the point of the split. + one. ## 2. Questions - **Q#DC1** — Can the two entities be made to diverge in a test? *Yes: - under a PTY with job control enabled, a shell places a background job - in its own process group and hands it the terminal, so `tcgetpgrp` != - leader pid. §3 Bet 1 builds exactly that.* + under a PTY with job control on, a shell places a background job in its + own process group and gives it the terminal, so `tcgetpgrp` != leader + pid.* - **Q#DC2** — Should the PTY fallback get its own `TargetSource`? - *Proposed: yes, and pmacs should call `nix::unistd::tcgetpgrp` itself so - the fallback can report the errno rather than inheriting portable-pty's - discarded `None`.* -- **Q#DC3** — Should the report name the signal? *Proposed: yes, with a - contrasting non-fatal signal tested.* + *Proposed: yes, reporting only that the lookup yielded `None` (§1.6).* +- **Q#DC3** — Should the report name the signal? *Proposed: yes, on the + reporting argument alone (§1.7).* - **Q#DC4** — Should the measured pgid be reported for `spec.group` - children? *Proposed: yes, as an observation clearly distinct from the - assumed value, and with no claim of sufficiency (§1.5).* -- **Q#DC5** — Should anything be retargeted or tolerated? **No. Parked.** + children? *Proposed: yes, as an observation distinct from the assumed + value, with no sufficiency claim (§1.5).* +- **Q#DC5** — Retarget or tolerate anything? **No. Parked.** ## 3. Bets -Each bet names what falsifies it and what falsification teaches. - - **Bet 1 — the divergence is constructible.** A PTY fixture where the - foreground group is *not* the leader: job control on, a background job + foreground group is not the leader: job control on, a background job given the terminal. The rewritten acceptance asserts both exact values - and that they **differ**. - - *Falsified if* the fixture cannot be made deterministic in CI (shell - job-control timing). Then the lane says so and falls back to pinning - divergence at the `signal_target` unit level with an injected - foreground group, which is weaker and must be labelled as weaker. - - This is the finding that matters most: without it, the entire - diagnostic remains unverified in the only case it exists for. + **and that they differ**. + - *Falsified if* the fixture cannot be made deterministic in CI. Then + the lane falls back to pinning divergence at the `signal_target` unit + level with an injected foreground group, and labels that as weaker. + - Without this the diagnostic remains unverified in the only case it + exists for. - **Bet 2 — the PTY fallback is reachable and distinguishable.** A test - drives the branch where the foreground-group lookup fails and asserts a - source string distinct from a pipe child's. + drives the branch where the lookup returns `None` and asserts a source + string distinct from a pipe child's. - *Falsified if* the branch cannot be reached without faking the - lookup. Then the seam is made injectable exactly as Stage A made the - kill injectable (Q#PD4), and that is stated rather than hidden. + lookup — in which case the seam is made injectable exactly as Stage A + made the kill injectable (Q#PD4), stated rather than hidden. - **Bet 3 — naming the signal is free.** Thread `signal` into the report. - - *Falsified if* any existing exact-string test cannot be updated - mechanically. Those four sites (`:2408`, `:2435`, `:2485`, `:2517`) - are the highest-risk part of the diff: **a wholesale rewrite of - expected strings is how a format regression hides**, so each is - updated individually and listed in the PR body with before and after. + - *Falsified if* any exact-string test cannot be updated mechanically. -- **Bet 4 — the measured pgid can disagree with the assumed one, and the - test proves the measurement is real.** A child that calls `setsid`, so - its pgid is genuinely not its parent-assumed value, is measured and the - two values asserted **different**. - - *Falsified if* no such case can be built — in which case the - measurement is unfalsifiable and should not ship, exactly per §1.4's - lesson. - -- **Bet 5 — §1.8's first-call abandonment is real.** Inject EPERM on the - *first* terminate; assert no ledger entry and state still `Running`. - - *Falsified if* the ledger is armed anyway, meaning §1.8 misreads - `signal`. - - **Pins current behaviour; does not fix it.** +- **Bet 4 — a `spec.group` child's measured pgid can be made to differ + from its pid.** **Not via `setsid`:** `spec.group` sets + `process_group(0)` before exec, so the recorded child is already a + process-group leader, and a group leader's `setsid` fails with EPERM. + Forking a `setsid` helper does not help either — `getpgid(recorded_pid)` + still observes the wrapper. + The fixture instead has the recorded child **`setpgid` into another + existing group in the same session**, with a readiness handshake before + the measurement and explicit cleanup of the anchor group afterwards. + - *Falsified if* no such fixture is deterministic — in which case the + measurement is unfalsifiable and **does not ship**, per §1.4's lesson. ## 4. Acceptance -1. A PTY job-control fixture in which `tcgetpgrp` != leader pid, with - both exact values asserted and asserted to differ. The landed test at - `:2400` is **rewritten**, not supplemented, since it currently pins a - substitution as acceptable. +1. A PTY job-control fixture where `tcgetpgrp` != leader pid, both exact + values asserted and asserted to differ. The test at `:2400` is + **rewritten**, not supplemented — it currently pins a substitution as + acceptable. 2. The PTY foreground-lookup fallback reports a source distinct from a - pipe child's leader-pid, with a test driving the real branch. -3. The report names the signal; at least one non-fatal signal - (`SIGUSR1`) is tested alongside `SIGTERM`, including that its - disposition differs. + pipe child's, with a test driving the real branch. **No errno claim** + (§1.6). +3. The report names the signal, split into two independent checks: + (a) a **failure-format** comparison showing `SIGUSR1` and `SIGTERM` + failures differ *in text only*, both leaving state and ledger + unchanged; and (b) a **successful-call disposition control** showing + a successful `SIGUSR1` does not transition state or arm the ledger + while a successful `SIGTERM` does. 4. For `spec.group` children the report carries the measured pgid as a - field distinct from the assumed one, renderable as unobservable, and - a test asserts a case where they **differ**. -5. Every exact-string test updated individually, each listed in the PR - body with before and after. No blanket rewrite. -6. §1.8's first-call abandonment pinned, labelled as pinning a known gap. -7. `docs/agent-handoff.md` records that "EPERM cannot happen for our own + field distinct from the assumed one, renderable as unobservable, with + a test asserting a case where they **differ** (§3 Bet 4). +5. All four exact-string sites — `:2408`, `:2435`, `:2485`, `:2517` — + updated **individually**, each listed in the PR body with before and + after. No blanket rewrite: that is how a format regression hides. +6. `:2501`'s existing first-call pin is **retained and cited**, updated + only for the new format. +7. `process` added to pmacs' declared `nix` features (§1.5a). +8. `docs/agent-handoff.md` records that "EPERM cannot happen for our own children" is false, with the run link; the comment at `:1246` is corrected in the same PR. -8. **No acceptance claims the telemetry establishes group identity.** - §1.5 governs; the PR body repeats it. +9. **No acceptance claims the telemetry establishes group identity** + (§1.5), and none claims escalation is guaranteed (§1.8). The PR body + repeats both. ## 5. Parked -- **Retargeting to the measured pgid.** Behavioural, and unsupported by - §1.5. Needs this lane's evidence first. +- **The reap ledger's silent cancellation** (§1.8): an EPERM probe drops + the entry and a failed `SIGKILL` is marked as killed. Now known + reachable. **Its own lane** — disposition questions, second site. +- **Retargeting to the measured pgid.** Behavioural; unsupported by §1.5. - **Any tolerance rule for EPERM or ESRCH.** Unmotivated across Stage A's three revisions and still unmotivated. -- **§1.8's first-call abandonment.** Its own lane; disposition change. +- **§1.8's first-call abandonment.** Pinned at `:2501`, not fixed here. - **Q#PS6** — `terminate` on an already-reaped process returning `Ok`. -- **`signal_target`'s read-then-kill of `tcgetpgrp`** — Stage A called it - "the most likely real fix site, still unframed". Still is. This lane - makes it *observable*, not fixed. +- **`signal_target`'s read-then-kill of `tcgetpgrp`** — Stage A's "most + likely real fix site, still unframed". This lane makes it *observable*, + not fixed. - **`compile_mode_acceptance` reading the developer's real - `~/.config/pmacs/init.lua`** — separate defect (11 local failures, - invisible in CI), unrelated to signals. + `~/.config/pmacs/init.lua`** — separate defect, 11 local failures, + invisible in CI. ## 6. Gates -Standard suite, each its own step with a real exit status, nothing after -the command that could mask it: `cargo fmt --check`; `cargo clippy +Standard suite, each its own step with a real exit status and nothing +after the command that could mask it: `cargo fmt --check`; `cargo clippy --workspace --all-targets -- -D warnings`; `cargo test --lib`; `cargo test --lib --features crdt`; `compile_mode_acceptance`; `terminal_copy_mode_acceptance` (both feature configurations); @@ -322,17 +394,14 @@ test --lib --features crdt`; `compile_mode_acceptance`; `git diff --check`. **All local runs use an isolated `XDG_CONFIG_HOME`** — without it -`compile_mode_acceptance` fails 11 tests for reasons unrelated to this -lane. +`compile_mode_acceptance` fails 11 tests for unrelated reasons. -**PTY job-control tests are the load-sensitive kind.** They are run -repeatedly, and the PR body records the repetition count rather than a -single green. +**PTY job-control tests are load-sensitive.** Run repeatedly; the PR body +records the repetition count, not a single green. ## 7. Branch plan -One branch, one PR. Order: Bet 1 first and alone, because it is the -finding that decides whether the diagnostic is worth extending at all. If -the divergence fixture cannot be made deterministic, the rest of the lane -is re-scoped rather than pushed through. +One branch, one PR. Bet 1 first and alone: it decides whether the +diagnostic is worth extending at all. If the divergence fixture cannot be +made deterministic, the lane is re-scoped rather than pushed through. From df508807629103d4131126be62c0b7aae066ca3f Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 12:28:48 -0400 Subject: [PATCH 3/8] =?UTF-8?q?docs(process):=20framing=20revision=204=20?= =?UTF-8?q?=E2=80=94=20correct=20review=20assumptions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrate canonical main, restore PTY errno capture through the safe filedescriptor duplication bridge, narrow the macOS occurrence to what it actually measured, and make the job-control fixture foreground and discriminating. Record the portable framing lane and exact recovery commands in the active-work ledger. --- docs/active-work.md | 41 ++++- ...-signal-diagnostic-completeness-framing.md | 164 +++++++++++++----- 2 files changed, 154 insertions(+), 51 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index a863a4f..99613f7 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -1,9 +1,9 @@ # Active work — cross-machine resume ledger -**Snapshot: 2026-07-29.** This file records volatile work that has not -landed on `main`. Read it after `docs/agent-handoff.md`. Remove completed -entries when their PR merges; do not let this become a second permanent -backlog. +**Snapshot: 2026-07-29; process-signal lane updated 2026-07-30.** This +file records volatile work that has not landed on `main`. Read it after +`docs/agent-handoff.md`. Remove completed entries when their PR merges; +do not let this become a second permanent backlog. **No lane below is retained past its merge.** This snapshot removes the resource-op delete guard (#186) and dired Stage 2 framing (#171) lanes @@ -110,6 +110,39 @@ declares canonical will pass on a tree the rest of this file does not describe. If it does not, stop and repair the remote/fetch configuration. +## Process-signal diagnostic completeness — REVISION 4 AWAITING REVIEW + +- **Portable branch:** `githubsucks/process-signal-diagnostic-completeness`; + worktree `../pmacs-signal-identity`. Framing only, no runtime changes + and no PR. The governing document is + `docs/process-signal-diagnostic-completeness-framing.md`, revision 4. +- **State:** canonical `githubsucks/main` @ `4cd4a7b` is integrated. + Revision 4 is awaiting review round 4. The branch corrects revision + 3's claimed base, its overstatement of what the macOS group-target + EPERM established, its foreground/background job-control fixture, and + its missed safe PTY-fd bridge. `filedescriptor::OwnedHandle::dup` + preserves the `tcgetpgrp` errno without `unsafe` in pmacs. +- **Boundary:** evidence collection only. No signal retargeting, + tolerance, disposition change, or reap-ledger repair is authorized. + Group identity remains unprovable; the ledger's silent cancellation is + parked as its own lane. +- **Verification:** exact `portable-pty 0.9.0`, `filedescriptor 0.8.3`, + nix 0.29.0, `src/process.rs`, and branch-ancestry surfaces were read. + Documentation-only verification is `git diff --check` plus the + canonical-main ancestry check; no runtime gate is claimed for a + framing-only revision. +- **Recovery from a clean checkout:** + + ```sh + git fetch githubsucks --prune + git worktree add ../pmacs-signal-identity \ + -b process-signal-diagnostic-completeness \ + githubsucks/process-signal-diagnostic-completeness + cd ../pmacs-signal-identity + git merge-base --is-ancestor 4cd4a7b HEAD + git status --short --branch + ``` + ## The CRDT half of the test corpus is dark in CI — NEEDS A LANE - **No branch, no framing yet.** Found while gating #166, then measured diff --git a/docs/process-signal-diagnostic-completeness-framing.md b/docs/process-signal-diagnostic-completeness-framing.md index 7d98975..1326292 100644 --- a/docs/process-signal-diagnostic-completeness-framing.md +++ b/docs/process-signal-diagnostic-completeness-framing.md @@ -1,6 +1,6 @@ # Framing — make the signal diagnostic discriminating (evidence collection) -**Revision 3.** Status: awaiting review round 3. Lane: +**Revision 4.** Status: awaiting review round 4. Lane: `process-signal-diagnostic-completeness`, worktree `../pmacs-signal-identity`, based on `githubsucks/main` @ `4cd4a7b` (re-measure at branch time; this is a reading, not a constant). @@ -17,12 +17,38 @@ gets signalled, no disposition change.** Everything behavioural is in §5. ## Revision history +**Revision 3 → 4**, after review round 3 (three blocking, one major). +All four accepted and checked against the exact APIs, process model, and +branch ancestry before revision. + +- **Rev 3's "no safe fd bridge" conclusion was still too absolute** + (§1.6). `filedescriptor::OwnedHandle::dup` accepts an `AsRawFd` through + its safe `AsRawFileDescriptor` blanket implementation and returns an + owned value implementing `AsFd`. A lifetime-tied wrapper around + `MasterPty::as_raw_fd` therefore bridges to + `nix::unistd::tcgetpgrp` with no `unsafe` in pmacs. The crate is + already resolved through `portable-pty`; this lane declares it + directly and restores errno capture. +- **The occurrence did not prove "our own child, alive, EPERM"** (§1.3). + The failed target was a *group* and `try_wait` observed the leader + process. No measurement established that the leader still belonged to + that group. What is invalidated is using ownership of the spawned + child to dismiss an arbitrary group-target error. +- **Bet 1 called a terminal-owning job "background"** (§3). A background + group is, by definition, not the terminal's foreground group. The + fixture now names `/bin/bash`, launches a foreground job in its own + group, and waits for the actual terminal handoff before measuring. +- **The branch-base line described the scout, not the ancestry.** Rev 3's + merge-base with `4cd4a7b` was still `391d38a`. Canonical main is now + integrated, and the lane is recorded in `docs/active-work.md`. + **Revision 2 → 3**, after review round 2 (three blocking, three major). All six accepted; all six verified in the code before acceptance. -- **The PTY errno proposal had no safe fd bridge.** Withdrawn and reduced - (§1.6). This was rev 2's central new proposal and it does not survive - `#![forbid(unsafe_code)]`. +- **Rev 3 concluded that the PTY errno proposal had no safe fd bridge.** + It therefore withdrew and reduced rev 2's central new proposal (§1.6). + Revision 4 supersedes that conclusion after checking + `filedescriptor`'s safe duplication API. - **Rev 2 said `getpgid` was "ungated". It is not** (§1.5a). The claim came from reading the four lines above the function; the gate is a block-level `feature!` opened 168 lines earlier. Same error shape as @@ -32,9 +58,9 @@ All six accepted; all six verified in the code before acceptance. child is already a process-group leader, and a group leader's `setsid` fails with EPERM. - **"Recoverable" was unsupported** (§1.8). The ledger drops its entry on - *any* probe error — including the EPERM this occurrence proves can - happen — and discards the `SIGKILL` result while marking the entry - killed. + *any* probe error — including an EPERM that ownership of the recorded + child cannot rule out for a group target — and discards the `SIGKILL` + result while marking the entry killed. - **Rev 2 falsified the wrong Stage A sentence** (§1.3). Stage A's disjointness claim was about the **PTY** path and remains true. - **Rev 2's signal-disposition argument was wrong** (§1.7). Failed @@ -97,12 +123,21 @@ Established: the target source is `group` — the `spec.group` pipe path path; and `leader=live`, from a real `try_wait` against the real child, so the leader had neither exited nor been reaped. -### 1.3 What is actually falsified +### 1.3 What the occurrence actually invalidates -- **`src/process.rs:1246-1247` is falsified.** `tick_reap_ledger` - justifies treating any probe error as "nothing left we can reach" with - the comment "**EPERM cannot happen for our own children**". §1.2 is a - counterexample: our own child, alive, EPERM. +- **`src/process.rs:1246-1247` uses an invalid premise.** + `tick_reap_ledger` justifies treating any probe error as "nothing left + we can reach" with the comment "**EPERM cannot happen for our own + children**". But the operation is group-directed: ownership of the + spawned child says nothing unless that child is still a member of the + targeted group. +- **§1.2 does not prove EPERM was "for our own child".** The failed + target was group `-8619`; `leader=live` observed process `8619`. + Nothing measured `getpgid(8619)`, so the occurrence establishes only + that a group target computed from the spawn-time assumption returned + EPERM while the leader process was alive. That is enough to invalidate + the comment as a reason to discard arbitrary group errors, but not to + attribute the errno to the child. - **Stage A §1.3 is *not* falsified.** It said the ledger is disjoint from **the PTY path**, because the ledger arms only for `proc.spec.group` and PTY mode cannot set it. That remains true. §1.2 @@ -180,7 +215,7 @@ one-line change that makes a real requirement visible. above it. The gate was 168 lines up. Recorded because it is the same defect class this document exists to fix.* -### 1.6 The PTY fallback is invisible, and the errno cannot be recovered +### 1.6 The PTY fallback is invisible; a safe owned-dup bridge preserves errno `signal_target` (`:757-785`): when the PTY branch's `master.process_group_leader()` returns `None`, control falls through — @@ -199,20 +234,38 @@ fn process_group_leader(&self) -> Option { } ``` -**Rev 2 proposed calling `nix::unistd::tcgetpgrp` ourselves to keep the -errno. That is withdrawn.** `tcgetpgrp` requires `F: AsFd`, and +`nix::unistd::tcgetpgrp` requires `F: AsFd`, while `MasterPty` exposes only `fn as_raw_fd(&self) -> Option` -(`portable-pty-0.9.0/src/lib.rs:114`). Every route from a `RawFd` to -something implementing `AsFd` — `BorrowedFd::borrow_raw`, -`OwnedFd::from_raw_fd`, `File::from_raw_fd` — is `unsafe`, and the crate -is `#![forbid(unsafe_code)]`. There is no safe bridge, and duplicating -the fd does not create one because the duplication itself needs the same -unsafe conversion. +(`portable-pty-0.9.0/src/lib.rs:114`). Rev 3 inspected only the standard +library's raw-to-owned constructors and concluded every bridge required +`unsafe`. That missed the safe duplication abstraction already in the +dependency graph: -**Reduced claim:** the fallback is distinguished **without** the errno. -`None` is all pmacs can observe, and the report says exactly that. The -errno is recorded here as unavailable-by-construction so a later lane -does not re-propose it. +- `filedescriptor::OwnedHandle::dup(&F)` is safe + (`filedescriptor-0.8.3/src/lib.rs:230`); +- on Unix, `filedescriptor` implements `AsRawFileDescriptor` for every + `T: AsRawFd` (`src/unix.rs:20`); +- `OwnedHandle` implements `AsFd` (`src/unix.rs:64`). + +A small wrapper holds a borrow of `MasterPty` for its lifetime and +implements the safe `AsRawFd` trait by returning the master's reported +fd. `OwnedHandle::dup` consumes that borrowed view immediately and +returns an independently owned duplicate; `tcgetpgrp(&owned)` then +preserves the `Errno`. **No raw-to-owned constructor and no `unsafe` +appears in pmacs.** `filedescriptor 0.8.3` is already in `Cargo.lock` +through `portable-pty`; this lane adds it as a direct dependency because +pmacs now calls its API. + +`OwnedHandle::dup` is itself fallible and preserves its Unix +`std::io::Error` source. That failure must not be collapsed into the +terminal query. The PTY result is therefore four-way and discriminating: + +- a positive pgid selects the foreground group, as today; +- a duplicate failure falls back to the leader and reports the + `duplicate-master-fd` stage plus its OS errno; +- a `tcgetpgrp` error falls back to the leader and reports the errno; +- absence of a master fd is a distinct unavailable source, not forged + into an errno. ### 1.7 The report omits which signal failed @@ -253,10 +306,13 @@ if now >= entry.deadline && !entry.killed { } ``` -Since §1.3 falsifies the premise that EPERM cannot occur here, an EPERM -probe **drops the entry and cancels escalation silently**, and a failed -`SIGKILL` is recorded as if it succeeded. So the honest statement is that -escalation remains *scheduled*, not that it happens. +If a ledger probe returns EPERM, it **drops the entry and cancels +escalation silently**; if its `SIGKILL` fails, the result is recorded as +if it succeeded. §1.2 did not observe either ledger call — it observed a +later explicit `SIGTERM` to the same assumed group number — so the +ledger failure is an exposed, still-unmeasured hazard rather than an +observed occurrence. The honest statement remains that escalation is +*scheduled*, not that it happens. **This still-silent path is parked, explicitly** (§5) rather than absorbed: it is a second site with its own disposition questions, and @@ -284,11 +340,12 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. ## 2. Questions - **Q#DC1** — Can the two entities be made to diverge in a test? *Yes: - under a PTY with job control on, a shell places a background job in its - own process group and gives it the terminal, so `tcgetpgrp` != leader - pid.* + under a PTY, `/bin/bash` with job control enabled launches a + **foreground** job in its own process group and hands it the terminal, + so `tcgetpgrp` != leader pid.* - **Q#DC2** — Should the PTY fallback get its own `TargetSource`? - *Proposed: yes, reporting only that the lookup yielded `None` (§1.6).* + *Proposed: yes. A failed duplicate or terminal lookup reports its stage + and errno; a missing master fd reports unavailable (§1.6).* - **Q#DC3** — Should the report name the signal? *Proposed: yes, on the reporting argument alone (§1.7).* - **Q#DC4** — Should the measured pgid be reported for `spec.group` @@ -300,9 +357,12 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. ## 3. Bets - **Bet 1 — the divergence is constructible.** A PTY fixture where the - foreground group is not the leader: job control on, a background job - given the terminal. The rewritten acceptance asserts both exact values - **and that they differ**. + foreground group is not the leader: `/bin/bash --noprofile --norc -m` + launches a foreground child in a fresh process group. The fixture + performs a bounded wait until `tcgetpgrp` itself reports the non-leader + group, asserts that group still has a live member as the positive + control, and only then injects the failing `kill`. The rewritten + acceptance asserts both exact values **and that they differ**. - *Falsified if* the fixture cannot be made deterministic in CI. Then the lane falls back to pinning divergence at the `signal_target` unit level with an injected foreground group, and labels that as weaker. @@ -310,8 +370,9 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. exists for. - **Bet 2 — the PTY fallback is reachable and distinguishable.** A test - drives the branch where the lookup returns `None` and asserts a source - string distinct from a pipe child's. + drives all three non-success arms: a duplicate errno, a `tcgetpgrp` + errno, and a missing master fd. Each source is distinct from a pipe + child's and from the others. - *Falsified if* the branch cannot be reached without faking the lookup — in which case the seam is made injectable exactly as Stage A made the kill injectable (Q#PD4), stated rather than hidden. @@ -339,8 +400,12 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. **rewritten**, not supplemented — it currently pins a substitution as acceptable. 2. The PTY foreground-lookup fallback reports a source distinct from a - pipe child's, with a test driving the real branch. **No errno claim** - (§1.6). + pipe child's. Separate tests drive the duplicate-error and + `tcgetpgrp`-error arms and assert the exact stage and errno; a third + drives the unavailable-fd arm. If one cannot be produced reliably + through a real PTY, the lookup result is injected while the branch, + target choice, real child observation, and report construction remain + production code (§3 Bet 2). 3. The report names the signal, split into two independent checks: (a) a **failure-format** comparison showing `SIGUSR1` and `SIGTERM` failures differ *in text only*, both leaving state and ledger @@ -355,10 +420,13 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. after. No blanket rewrite: that is how a format regression hides. 6. `:2501`'s existing first-call pin is **retained and cited**, updated only for the new format. -7. `process` added to pmacs' declared `nix` features (§1.5a). -8. `docs/agent-handoff.md` records that "EPERM cannot happen for our own - children" is false, with the run link; the comment at `:1246` is - corrected in the same PR. +7. `process` added to pmacs' declared `nix` features, and + `filedescriptor 0.8` declared directly for the safe PTY-fd duplicate + (§1.5a, §1.6). +8. `docs/agent-handoff.md` records that ownership of the recorded child + cannot justify dismissing an error from a group target, with the run + link and §1.2's measurement limit; the comment at `:1246` is corrected + in the same PR without claiming that the child itself received EPERM. 9. **No acceptance claims the telemetry establishes group identity** (§1.5), and none claims escalation is guaranteed (§1.8). The PR body repeats both. @@ -366,9 +434,11 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. ## 5. Parked -- **The reap ledger's silent cancellation** (§1.8): an EPERM probe drops - the entry and a failed `SIGKILL` is marked as killed. Now known - reachable. **Its own lane** — disposition questions, second site. +- **The reap ledger's silent cancellation** (§1.8): if a probe returns + EPERM the entry is dropped, and a failed `SIGKILL` is marked as killed. + The explicit-signal occurrence exposes the premise but did not observe + either ledger call. **Its own lane** — disposition questions, second + site. - **Retargeting to the measured pgid.** Behavioural; unsupported by §1.5. - **Any tolerance rule for EPERM or ESRCH.** Unmotivated across Stage A's three revisions and still unmotivated. From 0d2ad499691a66a7d0acb0f633cb0ea9c32f212c Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 12:49:47 -0400 Subject: [PATCH 4/8] test(process): make the group-directed acceptance discriminating (Bet 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Framing §3 Bet 1 and acceptance 1. Committed alone, per the branch plan: this bet decides whether the diagnostic is worth extending at all, so its result belongs in history before anything depends on it. The previous version of `a_group_directed_kill_failure_reports_target_and_leader_separately` spawned `/bin/sleep` on a PTY and asserted the same pid three times, conceding in its own doc comment that the values "are asserted to agree only because nothing has moved the terminal". The entire premise of the diagnostic is that the terminal's foreground group and the spawned leader are different entities, and no test exercised a case where they were. The fixture now drives job control: `bash -m` runs a foreground job in a fresh process group and hands it the terminal, so `tcgetpgrp` reports a group that is not the leader. The trailing `; :` is load-bearing — with a single simple command `bash -c` execs in place, which would leave the leader owning the terminal and silently restore the agreement. The bounded wait is also load-bearing rather than defensive. A probe of this fixture observed the foreground group as the leader FIRST and only then as the job's group, so measuring immediately would pin the non-divergent case. The fixture additionally asserts the diverged group still holds a live member, so a divergent number cannot come from a dead group. Falsified in both directions by substituting `leader_pid` for the `tcgetpgrp` result in `signal_target`: - the new test FAILS — target=-1020100 (leader) against the expected -1020103 (foreground group); - the OLD test, restored verbatim alongside the same mutation, PASSES. That pair is the finding: the previous acceptance pinned the substitution as acceptable. `/bin/bash` is declared as an optional test dependency and armed with PMACS_REQUIRE_BASH on BOTH CI platforms, not only Linux — the signal failures this diagnostic exists to explain have so far occurred only on macOS. The guard tests the exact path the fixture spawns rather than `which bash`, because a system with bash on PATH but not at `/bin/bash` would pass a `which` guard and then fail the spawn. Verified: both arming arms exercised against an absent path (unarmed skips, armed fails by name); 20/20 repetitions of the test; `cargo fmt --check`, `cargo clippy --workspace --all-targets -- -D warnings`, and `cargo test --lib` (1877 passed) all clean; ci.yml parses. No production behaviour changes. --- .github/workflows/ci.yml | 12 +++ README.md | 9 +++ src/process.rs | 162 +++++++++++++++++++++++++++++++++++---- 3 files changed, 168 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c9a9983..56afb21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -218,12 +218,24 @@ jobs: # 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. + # + # PMACS_REQUIRE_BASH arms the signal diagnostic's job-control + # divergence fixture, which needs `/bin/bash` specifically: it + # relies on `-m` putting a foreground job in its own process group + # and handing it the terminal, so that `tcgetpgrp` differs from the + # spawned leader. Without that divergence the group-directed + # acceptance cannot tell a real foreground-group read from a + # `leader_pid` substitution. Armed on BOTH platforms, unlike the + # variables above — macOS ships `/bin/bash`, and the macOS legs are + # exactly where the signal failures this diagnostic exists for have + # actually occurred. - 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' || '' }} + PMACS_REQUIRE_BASH: '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 diff --git a/README.md b/README.md index cd91a87..c9de0a8 100644 --- a/README.md +++ b/README.md @@ -227,6 +227,15 @@ translation) are routed through trampolines that exec these tools. **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. +- **`/bin/bash`** (**optional**, but armed on every CI platform). The + signal diagnostic's group-directed acceptance needs a terminal whose + foreground process group is *not* the spawned leader, and `bash -m` + produces exactly that by running a foreground job in its own process + group. The path matters: the test spawns `/bin/bash` directly rather + than resolving `bash` on `PATH`, and skips when that path is absent. + Set `PMACS_REQUIRE_BASH=1` to make the skip a failure, as CI does on + Linux **and** macOS — the failures this diagnostic exists to explain + have so far only appeared on macOS. - **`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 diff --git a/src/process.rs b/src/process.rs index f0b851c..13716e7 100644 --- a/src/process.rs +++ b/src/process.rs @@ -2304,18 +2304,6 @@ mod tests { /// `/bin/sleep` directly rather than through a shell: a shell may /// place the command in a different foreground process group, and /// these tests assert the exact target the tty reports. - fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> (ProcessId, u32) { - let mut spec = ProcessSpec::new(name, "/bin/sleep"); - spec.args = vec!["30".into()]; - spec.mode = ProcessMode::Pty { - rows: 24, - cols: 80, - mode: TerminalMode::Canonical, - }; - let id = sup.spawn(spec).expect("spawn"); - (id, spawn_started_pid(sup, id)) - } - /// The OS pid straight from the supervisor's own record, WITHOUT /// ticking. /// @@ -2396,24 +2384,168 @@ mod tests { /// designs for this code collapsed the two; the report keeps them /// apart, and here they are asserted to agree only because nothing /// has moved the terminal. + /// The job-control shell the divergence fixture drives. Named once so + /// the availability guard and the spawn cannot drift apart. + const BASH: &str = "/bin/bash"; + + /// The tty's current foreground process group, read through the same + /// `MasterPty` accessor production uses. `None` for a pipe + /// generation, or when the terminal reports no foreground group. + fn foreground_pgid(sup: &ProcessSupervisor, id: ProcessId) -> Option { + let runtime = sup.processes.get(&id)?.runtime.as_ref()?; + match &runtime.child { + ChildHandle::Pty { + _master: master, .. + } => master.process_group_leader(), + ChildHandle::Pipes(_) => None, + } + } + + /// Fixture for Q#DC1: a PTY child whose terminal foreground group is + /// genuinely **not** the spawned leader. + /// + /// `bash -m` enables job control, so it runs the script's command in + /// a fresh process group and hands that group the terminal. The + /// trailing `; :` matters — with a single simple command `bash -c` + /// execs in place, which would leave the leader owning the terminal + /// and silently restore the very agreement this fixture exists to + /// break. + /// + /// **The wait is load-bearing, not defensive.** The handoff is not + /// instantaneous: a probe of this exact fixture observed the + /// foreground group as the leader first and only then as the job's + /// group. Measuring immediately would pin the non-divergent case and + /// the test would assert the opposite of its purpose. + /// + /// Returns `(id, leader_pid, foreground_pgid)` with the two pids + /// known to differ and the foreground group known to hold a live + /// member. + fn spawn_pty_with_diverged_foreground_group( + sup: &mut ProcessSupervisor, + name: &str, + ) -> (ProcessId, u32, i32) { + let mut spec = ProcessSpec::new(name, BASH); + spec.args = vec![ + "--noprofile".into(), + "--norc".into(), + "-m".into(), + "-c".into(), + "sleep 30; :".into(), + ]; + spec.mode = ProcessMode::Pty { + rows: 24, + cols: 80, + mode: TerminalMode::Canonical, + }; + let id = sup.spawn(spec).expect("spawn"); + let leader = spawn_started_pid(sup, id); + + let leader_i32 = i32::try_from(leader).expect("pid fits i32"); + let deadline = Instant::now() + Duration::from_secs(10); + let mut observed: Vec = Vec::new(); + let mut diverged = None; + while Instant::now() < deadline { + if let Some(fg) = foreground_pgid(sup, id) { + if observed.last() != Some(&fg) { + observed.push(fg); + } + if fg > 0 && fg != leader_i32 { + diverged = Some(fg); + break; + } + } + std::thread::sleep(Duration::from_millis(25)); + } + + let fg = diverged.unwrap_or_else(|| { + panic!( + "job control never moved the terminal off the leader \ + (leader={leader}, foreground groups observed: {observed:?})" + ) + }); + + // Positive control: a divergent number proves nothing if the + // group is already dead. The signal target must be a group that + // could actually receive a signal. + nix::sys::signal::kill(Pid::from_raw(-fg), None).unwrap_or_else(|e| { + panic!("foreground group {fg} has no live member ({e}); divergence is vacuous") + }); + + (id, leader, fg) + } + + /// Q#DC1 / acceptance 1 — a group-directed failure names the target, + /// the branch that chose it, the expected group, the errno, and the + /// leader's own state, as five separate facts **that are not the same + /// fact repeated**. + /// + /// The pre-Stage-B version of this test spawned `/bin/sleep` on a PTY + /// and asserted the same pid three times, conceding in its own + /// comment that the values "are asserted to agree only because + /// nothing has moved the terminal". An implementation that ignored + /// `tcgetpgrp` entirely and substituted `leader_pid` passed it. Since + /// the whole premise of the diagnostic is that these two entities can + /// diverge, that test pinned the substitution as acceptable. + /// + /// This version drives job control so the terminal genuinely belongs + /// to a different process group, and asserts the two values **differ** + /// as well as asserting each exactly. #[test] fn a_group_directed_kill_failure_reports_target_and_leader_separately() { + // Guard on the exact path the fixture spawns, not on `which bash`. + // The two can disagree — a system with bash on PATH but not at + // `/bin/bash` would pass a `which` guard and then fail the spawn, + // which is the guard failing to guard the thing it names. + if !std::path::Path::new(BASH).exists() { + let armed = std::env::var_os("PMACS_REQUIRE_BASH").is_some_and(|v| !v.is_empty()); + assert!( + !armed, + "PMACS_REQUIRE_BASH is set but {BASH} does not exist: the \ + job-control divergence fixture cannot run" + ); + eprintln!( + "{BASH} not present; skipping \ + a_group_directed_kill_failure_reports_target_and_leader_separately" + ); + return; + } + let mut sup = ProcessSupervisor::new(); - let (id, pid) = spawn_live_pty(&mut sup, "diag-group"); + let (id, pid, fg) = spawn_pty_with_diverged_foreground_group(&mut sup, "diag-group"); + + let leader_i32 = i32::try_from(pid).expect("pid fits i32"); + assert_ne!( + fg, leader_i32, + "the fixture must make the foreground group differ from the leader; \ + equal values would let a leader_pid substitution pass" + ); sup.force_next_kill_errno(nix::errno::Errno::EPERM); let err = sup.terminate(id).expect_err("injected EPERM must fail"); let expected = format!( - "kill: {} (target=-{pid} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)", + "kill: {} (target=-{fg} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)", nix::errno::Errno::EPERM ); assert_eq!( err, expected, - "the report names the exact target the tty reported, the exact \ + "the report names the exact group the tty reported, the exact \ leader pid, and observes the leader as live" ); + // The target came from the terminal, not from the leader. Stated + // as its own assertion so a regression that reintroduces the + // substitution fails here by name rather than inside a long + // string comparison. + assert!( + err.contains(&format!("target=-{fg} via tcgetpgrp")), + "the target must be the terminal's foreground group: {err}" + ); + assert!( + !err.contains(&format!("target=-{pid} via tcgetpgrp")), + "the target must NOT be the leader pid: {err}" + ); + let _ = sup.signal(id, Signal::SIGKILL); } From b27df705bbfccc33da4b510aee3d7dd24f0d2df6 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 13:08:47 -0400 Subject: [PATCH 5/8] feat(process): make the signal diagnostic discriminating (Bets 2-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Framing acceptances 2, 3, 4, 5, 7 and 8. Evidence collection only: no tolerance rule, no change to which process is signalled, no disposition change. Three distinct failures previously rendered as one string. **The PTY fallback is now named.** When a PTY's foreground-group lookup yields no group, the target falls back to the leader — and until now that rendered "leader-pid", identical to a pipe child that never had a terminal. `portable-pty::MasterPty::process_group_leader` collapses every failure into `None` before pmacs can see it, so the errno was gone too. pmacs now performs the query itself and reports four distinct outcomes: no master fd, a failed duplicate with its errno, a failed `tcgetpgrp` with its errno, and a non-positive answer. Doing that without `unsafe` is the interesting part. `nix::unistd:: tcgetpgrp` needs `AsFd`; `MasterPty` exposes only `Option`; and every std route between them is `unsafe`, which this crate forbids. `filedescriptor::OwnedHandle::dup` takes any `AsRawFd` through a safe blanket impl and returns an owned handle that IS `AsFd`, so a lifetime-tied view implementing one safe trait is the whole bridge. The borrow is what makes it sound: the view cannot outlive the master, so the descriptor cannot close underneath it. **The report names the signal.** A failed SIGUSR1 and a failed SIGTERM were the same text. Note this is a reporting gap only — every failed `kill` returns before the fatal-signal branch, so failed signals are disposition-identical whatever they are. A separate control pins that the fatal/non-fatal difference is real for calls that SUCCEED, which is what gives the first test its meaning. **`measured_group` is a real observation.** `expected_group` is `-leader_pid`, and on the spawn-group path the target is `-leader_pid` too, so the report printed the same number three times and their agreement was arithmetic rather than evidence. `getpgid` supplies the one field that can disagree. It establishes no identity — it is read inside the same read-then-act window, and no portable mechanism closes that for a group. Bites, each by an actual revert, all observed to fail: - collapsing the PTY fallback back into a bare "leader-pid"; - dropping `signal=` from the report; - making the measured group restate the pid it was handed; - replacing the job-control fixture with a plain `sleep`, as a positive control on the divergence fixture itself. All four exact-string sites were updated individually, never by a blanket rewrite: a wholesale rewrite of expected strings is how a format regression hides. `:2501`'s first-call disposition pin is retained and updated for the new format rather than replaced. `nix`'s `process` feature is now declared explicitly. It already arrived transitively — nix's own `signal` feature depends on it — which is stable but invisible, and a real requirement resting on another feature's internals is one refactor away from vanishing. `filedescriptor` is declared directly for the same reason: pmacs now calls its API. The reap ledger's comment claiming "EPERM cannot happen for our own children" is corrected. Its bounded-growth policy is unchanged, but the justification was wrong: the probe targets a group, and owning the spawned child says nothing about a group unless the child is still a member — which nothing measures. The handoff records this together with the limit of the evidence: the occurrence does NOT establish that the child itself received EPERM. --- Cargo.lock | 1 + Cargo.toml | 18 +- docs/agent-handoff.md | 25 ++ src/process.rs | 523 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 540 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b80dbae..610ddd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2574,6 +2574,7 @@ dependencies = [ "codebook-tree-sitter-latex", "crossbeam", "crossterm", + "filedescriptor", "loro", "mlua", "nix 0.29.0", diff --git a/Cargo.toml b/Cargo.toml index 4e21592..e8319f9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -262,12 +262,28 @@ arborium-lean = "2.18" # T M4.4 process supervisor: signal sending without `unsafe`. Keep # the feature surface tight to keep build time low. `poll` feeds the # compile-mode group readers (cancellable poll-based reads, Q#CM3). -nix = { version = "0.29", default-features = false, features = ["signal", "user", "fs", "term", "socket", "poll"] } +# +# `process` is listed explicitly even though it already arrives +# transitively: nix's own `signal` feature depends on it, so `getpgid` +# and `tcgetpgrp` compile today without being asked for. That is +# stable but invisible, and a real requirement that depends on another +# feature's internals is one refactor away from vanishing. The signal +# diagnostic calls both directly. +nix = { version = "0.29", default-features = false, features = ["signal", "user", "fs", "term", "socket", "poll", "process"] } # T M4.4 PTY mode: portable abstraction over openpty / fork+exec # with controlling-tty wiring. The crate uses internal `unsafe` # but exposes a fully safe API; pmacs's own `unsafe_code = "forbid"` # rule still holds. portable-pty = "0.9" +# Safe file-descriptor duplication, already in the tree through +# `portable-pty`. Declared directly because the signal diagnostic calls +# `OwnedHandle::dup` itself: `MasterPty` exposes only a `RawFd`, and +# every std route from a raw fd to something implementing `AsFd` is +# `unsafe`. `dup` takes any `AsRawFd` through a safe blanket impl and +# returns an owned handle that IS `AsFd`, which is what lets pmacs call +# `nix::unistd::tcgetpgrp` — and keep the errno portable-pty discards — +# without a single `unsafe` block of its own. +filedescriptor = "0.8" # T M4.5 LSP wire format: JSON-RPC 2.0 bodies inside Content-Length # framing. Used only on LSP and similar protocols that require JSON # specifically; in-process workers continue to use MessagePack diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index e381de0..f31f2fc 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -223,6 +223,31 @@ commands, read `docs/active-work.md` immediately after this file. remain parked pending the evidence this diagnostic produces, as does `terminate` idempotence for an already-reaped process (a different failure, so a different PR). + - **The diagnostic fired, and what it showed.** macOS CI, PR #191, + [run 30553376486](https://github.com/levineuwirth/pmacs/actions/runs/30553376486/job/90907461258): + `target=-8619 via group, leader_pid=8619, expected_group=-8619, + leader=live` — the `spec.group` pipe path, not the PTY path, with the + leader observed alive by a real `try_wait`. A rerun of the identical + head passed 12/12, so it is intermittent. + - **Owning the child does not license dismissing a group error, and + the occurrence does not prove the child received EPERM.** The failed + target was the *group* `-8619`; `try_wait` observed the *process* + `8619`. Nothing measured `getpgid(8619)`, so the two are not known to + refer to the same thing. What is settled is narrower and still + enough: a group target computed from the spawn-time `pgid == pid` + assumption returned EPERM while the leader was alive, which retires + "EPERM cannot happen for our own children" as a reason to discard an + arbitrary group-directed error. Attributing the errno to the child + would repeat the exact error that killed three tolerance rules — + concluding something about one entity from something about another. + - **A field named like an observation can be a restatement of its + input.** `expected_group` is `-leader_pid`, and on the spawn-group + path the target is `-leader_pid` too, so the report printed the same + number three times and their agreement was arithmetic. Stage B adds a + `measured_group` from `getpgid`, which is the only field able to + disagree — and it still establishes no identity, because it is read + inside the same read-then-act window and no portable mechanism closes + that for a *group* (`pidfd` covers a process; macOS has neither). - **Lean 4 arc (Arc 8) — stages 1, 2, 3a, 3b, 4a, 4b ALL LANDED** (`docs/lean4-mode-framing.md`; #160, #161, #167, #170, #179, #181). pmacs edits Lean 4: `arborium-lean` highlighting, a `lean4` major mode, `⟨⟩ ⦃⦄ ⟮⟯` pairs, and a `lake serve` language diff --git a/src/process.rs b/src/process.rs index 13716e7..7f412ae 100644 --- a/src/process.rs +++ b/src/process.rs @@ -477,6 +477,9 @@ pub struct ProcessSupervisor { /// observation still runs against the real child handle; a stubbed /// observation would bypass the code path under test. forced_kill_errno: Option, + /// Test seam for the PTY foreground-group lookup (see + /// `force_next_pty_lookup_failure`). Always `None` outside tests. + forced_pty_lookup: Option, } /// One armed group in the reap ledger. @@ -726,16 +729,58 @@ enum TargetSource { ForegroundGroup, /// A `group = true` pipe child leading its own process group. SpawnGroup, - /// The child's own pid. + /// The child's own pid, for a pipe child that leads no group. LeaderPid, + /// A **PTY** child whose foreground-group lookup did not yield a + /// group, so the target fell back to the leader pid. + /// + /// Distinct from [`Self::LeaderPid`] on purpose. Before this + /// variant existed both rendered "leader-pid", so a PTY whose + /// terminal query failed was indistinguishable in the report from + /// an ordinary pipe child that never had a terminal — two very + /// different situations reading as one. + PtyForegroundFallback(PtyLookupFailure), +} + +/// Why a PTY's foreground-group lookup produced no group. +/// +/// Each arm is a different fact and none is forged into another: a +/// missing fd is not an errno, and a failure to *duplicate* the master +/// is not a failure to *query* the terminal. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum PtyLookupFailure { + /// The master reported no file descriptor to query. + NoMasterFd, + /// Duplicating the master fd failed, so the terminal was never + /// queried at all. + Duplicate(nix::errno::Errno), + /// `tcgetpgrp` itself failed on a successfully duplicated fd. + Query(nix::errno::Errno), + /// The terminal answered, but with a non-positive group id, which + /// names no group. + NonPositive(i32), +} + +impl PtyLookupFailure { + fn render(self) -> String { + match self { + Self::NoMasterFd => "no-master-fd".to_owned(), + Self::Duplicate(e) => format!("duplicate-master-fd: {e}"), + Self::Query(e) => format!("tcgetpgrp: {e}"), + Self::NonPositive(v) => format!("tcgetpgrp-non-positive: {v}"), + } + } } impl TargetSource { - fn as_str(self) -> &'static str { + fn render(self) -> String { match self { - Self::ForegroundGroup => "tcgetpgrp", - Self::SpawnGroup => "group", - Self::LeaderPid => "leader-pid", + Self::ForegroundGroup => "tcgetpgrp".to_owned(), + Self::SpawnGroup => "group".to_owned(), + Self::LeaderPid => "leader-pid".to_owned(), + Self::PtyForegroundFallback(why) => { + format!("pty-leader-fallback({})", why.render()) + } } } @@ -745,6 +790,85 @@ impl TargetSource { } } +/// A lifetime-tied view of a `MasterPty`'s file descriptor. +/// +/// `MasterPty` exposes only `Option`, and every std route from a +/// raw fd to something implementing `AsFd` — `BorrowedFd::borrow_raw`, +/// `OwnedFd::from_raw_fd`, `File::from_raw_fd` — is `unsafe`, which this +/// crate forbids. `filedescriptor::OwnedHandle::dup` accepts any +/// `AsRawFd` through a safe blanket impl and hands back an owned handle +/// that *is* `AsFd`, so implementing this one safe trait is the whole +/// bridge. +/// +/// The borrow is what makes it sound: the view cannot outlive the master +/// it read the descriptor from, so the fd cannot have been closed +/// underneath it. +struct MasterFdView<'a> { + fd: std::os::fd::RawFd, + _master: &'a (dyn portable_pty::MasterPty + Send), +} + +impl std::os::fd::AsRawFd for MasterFdView<'_> { + fn as_raw_fd(&self) -> std::os::fd::RawFd { + self.fd + } +} + +/// Recover the OS errno from a `filedescriptor` error. +/// +/// Its error type is an enum of thiserror variants, each carrying a +/// `std::io::Error` as a `#[source]` rather than exposing +/// `raw_os_error` itself. Walking the source chain and downcasting keeps +/// every variant working, including ones added later, instead of +/// matching the one arm that exists today. +/// +/// Returns `UnknownErrno` when the chain carries no OS error, rather +/// than inventing a plausible one — a forged errno in a diagnostic is +/// worse than an honest absence. +fn os_errno_of(err: &filedescriptor::Error) -> nix::errno::Errno { + let mut current: Option<&(dyn std::error::Error + 'static)> = Some(err); + while let Some(e) = current { + if let Some(io) = e.downcast_ref::() + && let Some(code) = io.raw_os_error() + { + return nix::errno::Errno::from_raw(code); + } + current = e.source(); + } + nix::errno::Errno::UnknownErrno +} + +/// Read the terminal's foreground process group, keeping the errno. +/// +/// `portable_pty::MasterPty::process_group_leader` collapses every +/// failure into `None`, so pmacs could not tell "this tty has no +/// foreground group" from "the query failed and here is why". This does +/// the query itself and returns the reason on every non-success path. +fn pty_foreground_group( + master: &(dyn portable_pty::MasterPty + Send), +) -> Result { + let Some(fd) = master.as_raw_fd() else { + return Err(PtyLookupFailure::NoMasterFd); + }; + let view = MasterFdView { + fd, + _master: master, + }; + let owned = filedescriptor::OwnedHandle::dup(&view) + .map_err(|e| PtyLookupFailure::Duplicate(os_errno_of(&e)))?; + match nix::unistd::tcgetpgrp(&owned) { + Ok(pgrp) => { + let raw = pgrp.as_raw(); + if raw > 0 { + Ok(raw) + } else { + Err(PtyLookupFailure::NonPositive(raw)) + } + } + Err(e) => Err(PtyLookupFailure::Query(e)), + } +} + /// The entity a signal was actually aimed at, plus the branch that /// chose it. Carried so a failure can report the target as a fact /// separate from the leader's state (Q#PD1). @@ -754,18 +878,36 @@ struct SignalTarget { source: TargetSource, } -fn signal_target(proc: &ManagedProcess, pid: u32) -> Result { +fn signal_target( + proc: &ManagedProcess, + pid: u32, + forced_lookup: Option, +) -> Result { if let Some(runtime) = proc.runtime.as_ref() && let ChildHandle::Pty { _master: master, .. } = &runtime.child - && let Some(pgrp) = master.process_group_leader() - && pgrp > 0 { - return Ok(SignalTarget { - pid: Pid::from_raw(-pgrp), - source: TargetSource::ForegroundGroup, - }); + // A PTY child is always group-directed when the terminal names a + // foreground group. When it does not, the target falls back to + // the leader — and *why* it fell back is carried into the source + // so the report can say it. Previously every one of these paths + // produced a bare `LeaderPid`, identical to a pipe child that + // never had a terminal at all. + let lookup = match forced_lookup { + Some(failure) => Err(failure), + None => pty_foreground_group(master.as_ref()), + }; + return match lookup { + Ok(pgrp) => Ok(SignalTarget { + pid: Pid::from_raw(-pgrp), + source: TargetSource::ForegroundGroup, + }), + Err(why) => Ok(SignalTarget { + pid: Pid::from_raw(i32::try_from(pid).map_err(|e| e.to_string())?), + source: TargetSource::PtyForegroundFallback(why), + }), + }; } // `group = true` pipe children lead a fresh process group // (`process_group(0)` at spawn ⇒ pgid == pid), so fatal signals @@ -824,12 +966,45 @@ fn observe_leader(proc: &mut ManagedProcess) -> LeaderObservation { } } -/// Render a failing `kill` as the five facts of Q#PD1. The disposition -/// is unchanged (Q#PD2) — this only replaces a message that said -/// nothing but the errno. +/// The leader's process group as the kernel reports it, for a target +/// that was *computed* from the spawn-time assumption `pgid == pid`. +/// +/// `expected_group` is that assumption restated — it is `-leader_pid`, +/// and on the `SpawnGroup` path the target is `-leader_pid` too, so the +/// two agreeing is arithmetic rather than evidence. This is the only +/// field in the report that can disagree with the input, which is what +/// makes it worth printing. +/// +/// **It does not establish identity** (framing §1.5). It is read before +/// the `kill` in the same read-then-act window, and a number cannot +/// distinguish the original group from a recycled one. No portable +/// mechanism can: `pidfd` closes pid reuse for a process, not a group, +/// and macOS has none at all. This records an observation; it settles +/// nothing. +fn measured_group_of(leader_pid: u32) -> String { + let Ok(raw) = i32::try_from(leader_pid) else { + return ", measured_group=unobservable(pid out of range)".to_owned(); + }; + match nix::unistd::getpgid(Some(Pid::from_raw(raw))) { + Ok(pgid) => format!(", measured_group=-{}", pgid.as_raw()), + Err(e) => format!(", measured_group=unobservable({e})"), + } +} + +/// Render a failing `kill` as the facts of Q#PD1. The disposition is +/// unchanged (Q#PD2) — this only replaces a message that said nothing +/// but the errno. +/// +/// The signal is named because it could not be recovered otherwise: a +/// failed `SIGUSR1` and a failed `SIGTERM` were previously identical +/// text. Note this is a *reporting* gap only — every failed `kill` +/// returns before the fatal-signal branch, so failed signals are +/// disposition-identical whatever they are. The disposition difference +/// is real only for calls that succeed. fn signal_failure_report( target: SignalTarget, leader_pid: u32, + signal: Signal, errno: nix::errno::Errno, leader: &LeaderObservation, ) -> String { @@ -841,10 +1016,19 @@ fn signal_failure_report( } else { String::new() }; + // Only the spawn-group path computes its target from the assumption, + // so it is the only one where a measurement can contradict anything. + // A PTY target came from the terminal and a leader-directed target is + // not a group at all. + let measured = if matches!(target.source, TargetSource::SpawnGroup) { + measured_group_of(leader_pid) + } else { + String::new() + }; format!( - "kill: {errno} (target={} via {}, leader_pid={leader_pid}{expected}, leader={})", + "kill: {errno} (signal={signal:?}, target={} via {}, leader_pid={leader_pid}{expected}{measured}, leader={})", target.pid.as_raw(), - target.source.as_str(), + target.source.render(), leader.render(), ) } @@ -950,6 +1134,7 @@ impl ProcessSupervisor { reap_ledger: HashMap::new(), group_term_grace: GROUP_TERM_GRACE, forced_kill_errno: None, + forced_pty_lookup: None, } } @@ -963,6 +1148,21 @@ impl ProcessSupervisor { self.forced_kill_errno = Some(errno); } + /// Test seam for the PTY foreground-group lookup, on the same terms + /// as [`Self::force_next_kill_errno`] and for the same reason. + /// + /// The three non-success arms — no master fd, a failed duplicate, a + /// failed `tcgetpgrp` — cannot be produced on demand from a healthy + /// PTY: they need an exhausted descriptor table or a master that has + /// stopped being a terminal. Injecting only the *lookup result* + /// leaves the branch itself, the fallback target choice, the leader + /// observation against the real child, and the report construction + /// all running as production code. Consumed by one call. + #[cfg(test)] + fn force_next_pty_lookup_failure(&mut self, failure: PtyLookupFailure) { + self.forced_pty_lookup = Some(failure); + } + /// Override the SIGTERM-to-SIGKILL grace window. Test helper. pub fn set_grace_period(&mut self, d: Duration) { self.grace_period = d; @@ -1080,7 +1280,8 @@ impl ProcessSupervisor { else { return Err(format!("process {id} is not running")); }; - let target = signal_target(proc, pid)?; + let forced_lookup = self.forced_pty_lookup.take(); + let target = signal_target(proc, pid, forced_lookup)?; // Q#PD4: the seam injects the KILL attempt's result only — // never the observation below — so target selection, the real // `ChildHandle::try_wait` against the real child, and the error @@ -1094,7 +1295,7 @@ impl ProcessSupervisor { // disposition is unchanged — this still returns `Err`, // with no state transition and no ledger arming. let leader = observe_leader(proc); - return Err(signal_failure_report(target, pid, errno, &leader)); + return Err(signal_failure_report(target, pid, signal, errno, &leader)); } if matches!(signal, Signal::SIGTERM | Signal::SIGKILL | Signal::SIGHUP) { proc.state = ProcessState::Exiting { @@ -1243,9 +1444,24 @@ impl ProcessSupervisor { let now = Instant::now(); self.reap_ledger.retain(|pgid, entry| { // ESRCH: no such group — done. Any other probe error is - // also treated as "nothing left we can reach" (EPERM - // cannot happen for our own children) so the ledger - // cannot grow without bound. + // also treated as "nothing left we can reach", so the + // ledger cannot grow without bound. + // + // **That is a bounded-growth policy, not a claim that the + // group is gone.** This comment previously justified it with + // "EPERM cannot happen for our own children". That reasoning + // does not hold: the probe targets a *group*, and owning the + // spawned child says nothing about a group unless the child + // is still a member of it — which nothing here measures. A + // group-directed EPERM against a live leader has since been + // observed in CI (macOS, PR #191, run 30553376486), via an + // explicit signal rather than this probe. + // + // So this arm can silently cancel an escalation, and the + // `SIGKILL` below can fail while the entry is marked killed. + // Both are known and deliberately unchanged here: the + // diagnostic lane that found them does not alter + // disposition. Fixing it is its own lane. if nix::sys::signal::kill(Pid::from_raw(-*pgid), None).is_err() { return false; } @@ -2388,6 +2604,20 @@ mod tests { /// the availability guard and the spawn cannot drift apart. const BASH: &str = "/bin/bash"; + /// A plain PTY child, for tests that care about the PTY *branch* + /// rather than about job control. + fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> (ProcessId, u32) { + let mut spec = ProcessSpec::new(name, "/bin/sleep"); + spec.args = vec!["30".into()]; + spec.mode = ProcessMode::Pty { + rows: 24, + cols: 80, + mode: TerminalMode::Canonical, + }; + let id = sup.spawn(spec).expect("spawn"); + (id, spawn_started_pid(sup, id)) + } + /// The tty's current foreground process group, read through the same /// `MasterPty` accessor production uses. `None` for a pipe /// generation, or when the terminal reports no foreground group. @@ -2524,7 +2754,7 @@ mod tests { let err = sup.terminate(id).expect_err("injected EPERM must fail"); let expected = format!( - "kill: {} (target=-{fg} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)", + "kill: {} (signal=SIGTERM, target=-{fg} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)", nix::errno::Errno::EPERM ); assert_eq!( @@ -2549,6 +2779,247 @@ mod tests { let _ = sup.signal(id, Signal::SIGKILL); } + /// Q#DC2 / acceptance 2 — a PTY whose foreground-group lookup fails + /// is distinguishable from a pipe child that never had a terminal. + /// + /// Before this, both rendered "leader-pid". The PTY fallback was + /// therefore invisible: a terminal query that failed, and a process + /// with no terminal at all, produced the same word. Each arm now + /// names its own stage, and `portable-pty`'s + /// `process_group_leader` — which collapses every failure into + /// `None` before pmacs can see it — is bypassed so the errno + /// survives. + #[test] + fn a_pty_foreground_lookup_failure_names_its_stage() { + let arms = [ + (PtyLookupFailure::NoMasterFd, "no-master-fd".to_owned()), + ( + PtyLookupFailure::Duplicate(nix::errno::Errno::EMFILE), + format!("duplicate-master-fd: {}", nix::errno::Errno::EMFILE), + ), + ( + PtyLookupFailure::Query(nix::errno::Errno::ENOTTY), + format!("tcgetpgrp: {}", nix::errno::Errno::ENOTTY), + ), + ( + PtyLookupFailure::NonPositive(0), + "tcgetpgrp-non-positive: 0".to_owned(), + ), + ]; + + for (failure, rendered) in arms { + let mut sup = ProcessSupervisor::new(); + let (id, pid) = spawn_live_pty(&mut sup, "diag-pty-fallback"); + + sup.force_next_pty_lookup_failure(failure); + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + + // The target falls back to the leader — positive, not a + // negated group — and the source says why. + let expected = format!( + "kill: {} (signal=SIGTERM, target={pid} via pty-leader-fallback({rendered}), leader_pid={pid}, leader=live)", + nix::errno::Errno::EPERM + ); + assert_eq!(err, expected, "arm {failure:?} must name its own stage"); + + // And it must NOT read like a pipe child. + assert!( + !err.contains("via leader-pid,"), + "a PTY fallback must not render as a bare pipe leader target: {err}" + ); + + let _ = sup.signal(id, Signal::SIGKILL); + } + } + + /// The companion half of acceptance 2: a genuine pipe child still + /// renders "leader-pid", so the two really are distinct strings + /// rather than both having moved. + /// + /// Asserted here as well as in the leader-directed test because a + /// rename of one side would otherwise pass every test — the pair is + /// the point, not either string alone. + #[test] + fn a_pipe_child_still_renders_a_bare_leader_target() { + let mut sup = ProcessSupervisor::new(); + let mut spec = ProcessSpec::new("diag-pipe-leader", "/bin/sleep"); + spec.args = vec!["30".into()]; + let id = sup.spawn(spec).expect("spawn"); + let pid = spawn_started_pid(&mut sup, id); + + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + + assert!( + err.contains(&format!("target={pid} via leader-pid,")), + "a pipe child with no group renders the bare leader source: {err}" + ); + assert!( + !err.contains("pty-leader-fallback"), + "a pipe child never took the PTY branch: {err}" + ); + + let _ = sup.signal(id, Signal::SIGKILL); + } + + /// Q#DC3 / acceptance 3(a) — the report names the signal, so two + /// failures that differ only in which signal was sent are no longer + /// the same text. + /// + /// **They differ in text only.** Every failed `kill` returns before + /// the fatal-signal branch, so both leave the state and the ledger + /// exactly as they were. That is asserted here rather than assumed, + /// because revision 2 of the framing claimed the opposite. + #[test] + fn a_failed_signal_names_which_signal_and_changes_nothing() { + let mut reports = Vec::new(); + for signal in [Signal::SIGTERM, Signal::SIGUSR1] { + let mut sup = ProcessSupervisor::new(); + let mut spec = ProcessSpec::new("diag-signal-name", "/bin/sh"); + spec.args = vec!["-c".into(), "sleep 30".into()]; + spec.group = true; + let id = sup.spawn(spec).expect("spawn"); + let pid = spawn_started_pid(&mut sup, id); + + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup + .signal(id, signal) + .expect_err("injected EPERM must fail"); + + assert!( + err.contains(&format!("signal={signal:?},")), + "the report must name {signal:?}: {err}" + ); + assert!( + matches!( + sup.processes.get(&id).expect("record").state, + ProcessState::Running { .. } + ), + "a failed {signal:?} must not transition the record" + ); + assert!( + sup.reap_ledger.is_empty(), + "a failed {signal:?} must not arm the ledger" + ); + + reports.push(err.replace(&format!("{pid}"), "")); + let _ = nix::sys::signal::kill( + Pid::from_raw(-i32::try_from(pid).unwrap()), + Signal::SIGKILL, + ); + } + + assert_ne!( + reports[0], reports[1], + "SIGTERM and SIGUSR1 failures must no longer be identical text" + ); + } + + /// Q#DC3 / acceptance 3(b) — the disposition control. A *successful* + /// non-fatal signal changes nothing, while a *successful* fatal one + /// transitions the record and arms the ledger. + /// + /// This is the check that gives the previous test its meaning: it + /// shows the fatal/non-fatal distinction is real, and therefore that + /// "failed signals are disposition-identical" is a statement about + /// the failure path rather than about signals generally. + #[test] + fn a_successful_signal_disposition_depends_on_whether_it_is_fatal() { + let mut sup = ProcessSupervisor::new(); + let mut spec = ProcessSpec::new("diag-disposition-live", "/bin/sh"); + // Ignore USR1 so the successful non-fatal signal cannot end the + // child and confuse the state assertion with a real exit. + spec.args = vec!["-c".into(), "trap '' USR1; sleep 30".into()]; + spec.group = true; + let id = sup.spawn(spec).expect("spawn"); + let pid = spawn_started_pid(&mut sup, id); + + sup.signal(id, Signal::SIGUSR1).expect("USR1 delivers"); + assert!( + matches!( + sup.processes.get(&id).expect("record").state, + ProcessState::Running { .. } + ), + "a successful non-fatal signal leaves the record Running" + ); + assert!( + sup.reap_ledger.is_empty(), + "a successful non-fatal signal arms no ledger entry" + ); + + sup.terminate(id).expect("TERM delivers"); + assert!( + matches!( + sup.processes.get(&id).expect("record").state, + ProcessState::Exiting { .. } + ), + "a successful fatal signal transitions the record to Exiting" + ); + assert!( + !sup.reap_ledger.is_empty(), + "a successful fatal signal arms the group reap ledger" + ); + + let _ = + nix::sys::signal::kill(Pid::from_raw(-i32::try_from(pid).unwrap()), Signal::SIGKILL); + } + + /// Q#DC4 / acceptance 4 — the measured group is a real observation, + /// not a restatement of the input. + /// + /// `expected_group` is `-leader_pid` by construction, so on the + /// spawn-group path it can never disagree with the target. The + /// measured field is the only one that can, and this proves it does: + /// a child placed into an *anchor* group reports that group, not its + /// own pid. + /// + /// Without this the field would be exactly the vacuous readout the + /// framing was written to eliminate — an implementation returning + /// `-pid` unconditionally would satisfy every other test. + #[test] + fn the_measured_group_reports_the_real_group_not_the_pid() { + use std::os::unix::process::CommandExt as _; + + // An anchor process leading its own group. + let mut anchor = std::process::Command::new("/bin/sleep"); + anchor.arg("30"); + anchor.process_group(0); + let mut anchor = anchor.spawn().expect("spawn anchor"); + let anchor_pgid = i32::try_from(anchor.id()).expect("pid fits i32"); + + // A second process placed INTO the anchor's group, so its pgid + // is genuinely not its own pid. + let mut joiner = std::process::Command::new("/bin/sleep"); + joiner.arg("30"); + joiner.process_group(anchor_pgid); + let mut joiner = joiner.spawn().expect("spawn joiner"); + let joiner_pid = joiner.id(); + + assert_ne!( + i32::try_from(joiner_pid).unwrap(), + anchor_pgid, + "precondition: the joiner must not be the anchor itself" + ); + + let rendered = measured_group_of(joiner_pid); + assert_eq!( + rendered, + format!(", measured_group=-{anchor_pgid}"), + "the measurement must report the group the kernel actually has" + ); + assert_ne!( + rendered, + format!(", measured_group=-{joiner_pid}"), + "and must NOT restate the pid it was given" + ); + + let _ = joiner.kill(); + let _ = joiner.wait(); + let _ = anchor.kill(); + let _ = anchor.wait(); + } + /// Q#PD1 acceptance 2 — a leader-directed failure records the /// fallback branch and a positive target, and omits the group field /// that would be meaningless for it. Exact message again. @@ -2564,7 +3035,7 @@ mod tests { let err = sup.terminate(id).expect_err("injected ESRCH must fail"); let expected = format!( - "kill: {} (target={pid} via leader-pid, leader_pid={pid}, leader=live)", + "kill: {} (signal=SIGTERM, target={pid} via leader-pid, leader_pid={pid}, leader=live)", nix::errno::Errno::ESRCH ); assert_eq!( @@ -2614,7 +3085,7 @@ mod tests { let err = terminate_until_leader_exited(&mut sup, id, Duration::from_secs(10)); let expected = format!( - "kill: {} (target={pid} via leader-pid, leader_pid={pid}, leader=exited(code 3))", + "kill: {} (signal=SIGTERM, target={pid} via leader-pid, leader_pid={pid}, leader=exited(code 3))", nix::errno::Errno::EPERM ); assert_eq!( @@ -2646,7 +3117,7 @@ mod tests { let err = sup.terminate(id).expect_err("injected EPERM must fail"); let expected = format!( - "kill: {} (target=-{pid} via group, leader_pid={pid}, expected_group=-{pid}, leader=live)", + "kill: {} (signal=SIGTERM, target=-{pid} via group, leader_pid={pid}, expected_group=-{pid}, measured_group=-{pid}, leader=live)", nix::errno::Errno::EPERM ); assert_eq!(err, expected, "a group=true pipe child reports via group"); From 66f73013fcee4cad5d2d4610b90729878cd15c6d Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 14:10:39 -0400 Subject: [PATCH 6/8] =?UTF-8?q?test(process):=20take=20Bet=201's=20fallbac?= =?UTF-8?q?k=20=E2=80=94=20macOS=20does=20not=20diverge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI falsified framing Bet 1. Both macOS legs reported job control never moved the terminal off the leader (leader=8542, foreground groups observed: [8542]) with the terminal staying on the leader for the entire 10s bounded wait. Linux diverges reliably — 20/20 locally and green on both ubuntu legs — so this is a platform difference rather than a flake, and rerunning past it would have been wrong. The framing named this outcome and prescribed the response, so that is what ships rather than an improvised fix: - The divergent case is pinned by INJECTING the foreground group at the `signal_target` seam. Deterministic, runs on every platform. The injection seam widens from failure-only to either outcome; the branch, target choice, leader observation against the real child, and report construction all remain production code. - Verified still discriminating: the `leader_pid`-substitution mutation fails it, `target=-1707909` against an expected `-1707910`. That was the whole point of the original rewrite and it survives the fallback. - The real shell is retained as corroboration in `job_control_really_diverges_the_foreground_group`, Linux-only. It skips on macOS by PLATFORM CHECK rather than by arming: the precondition genuinely does not hold there, so running it would assert a false claim about macOS instead of finding a bug. - Framing revision 5 records the falsification and states exactly how the injected pin is weaker — it proves the target is read from the lookup rather than substituted from the leader; it does not by itself prove any real shell produces that divergence. `PMACS_REQUIRE_BASH` moves to Linux-only. The earlier reasoning for arming both platforms — macOS is where the failures happen, so Linux-only leaves it dark where it matters — was right about the diagnostic and wrong about this test, which cannot produce its precondition on macOS at all. Arming it there made a missing binary fatal for a test that can never run. The measurement is recorded in ci.yml and the README so it is not re-derived. Gates on this tree: 11 gates, 4471 tests, zero failures. --- .github/workflows/ci.yml | 24 +-- README.md | 22 ++- ...-signal-diagnostic-completeness-framing.md | 42 ++++- src/process.rs | 162 ++++++++++++------ 4 files changed, 175 insertions(+), 75 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56afb21..0bc7ef3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -220,22 +220,26 @@ jobs: # skip fatal where the tool is guaranteed. # # PMACS_REQUIRE_BASH arms the signal diagnostic's job-control - # divergence fixture, which needs `/bin/bash` specifically: it - # relies on `-m` putting a foreground job in its own process group - # and handing it the terminal, so that `tcgetpgrp` differs from the - # spawned leader. Without that divergence the group-directed - # acceptance cannot tell a real foreground-group read from a - # `leader_pid` substitution. Armed on BOTH platforms, unlike the - # variables above — macOS ships `/bin/bash`, and the macOS legs are - # exactly where the signal failures this diagnostic exists for have - # actually occurred. + # corroboration test, which needs `/bin/bash` and `-m` putting a + # foreground job in its own process group with the terminal. + # + # Linux only, and NOT because macOS lacks bash — it ships 3.2. CI + # measured the difference: on both macOS legs a non-interactive + # `bash -m` kept the terminal on the leader for a full 10s wait, so + # the divergence the test needs does not occur there. The test skips + # on non-Linux by platform check; arming it on macOS would only make + # a missing binary fatal for a test that cannot run anyway. + # + # The divergent case itself is pinned on every platform by + # injecting the foreground group instead (framing Bet 1's stated + # fallback, after the real fixture failed its own falsifier). - 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' || '' }} - PMACS_REQUIRE_BASH: '1' + PMACS_REQUIRE_BASH: ${{ 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 diff --git a/README.md b/README.md index c9de0a8..6225678 100644 --- a/README.md +++ b/README.md @@ -227,15 +227,19 @@ translation) are routed through trampolines that exec these tools. **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. -- **`/bin/bash`** (**optional**, but armed on every CI platform). The - signal diagnostic's group-directed acceptance needs a terminal whose - foreground process group is *not* the spawned leader, and `bash -m` - produces exactly that by running a foreground job in its own process - group. The path matters: the test spawns `/bin/bash` directly rather - than resolving `bash` on `PATH`, and skips when that path is absent. - Set `PMACS_REQUIRE_BASH=1` to make the skip a failure, as CI does on - Linux **and** macOS — the failures this diagnostic exists to explain - have so far only appeared on macOS. +- **`/bin/bash`** (**optional**, Linux only). The signal diagnostic's + job-control corroboration test needs a terminal whose foreground + process group is not the spawned leader, which `bash -m` produces by + running a foreground job in its own process group. The path matters: + the test spawns `/bin/bash` directly rather than resolving `bash` on + `PATH`, and skips when that path is absent. Set `PMACS_REQUIRE_BASH=1` + to make the skip a failure, as CI does on Linux. + + **It is deliberately not armed on macOS**, which ships bash 3.2 but + where a non-interactive `bash -m` was measured in CI to keep the + terminal on the leader — so the divergence the test needs never + happens there. The divergent case is pinned on every platform by + injecting the foreground group instead. - **`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 diff --git a/docs/process-signal-diagnostic-completeness-framing.md b/docs/process-signal-diagnostic-completeness-framing.md index 1326292..7dad7fe 100644 --- a/docs/process-signal-diagnostic-completeness-framing.md +++ b/docs/process-signal-diagnostic-completeness-framing.md @@ -1,6 +1,6 @@ # Framing — make the signal diagnostic discriminating (evidence collection) -**Revision 4.** Status: awaiting review round 4. Lane: +**Revision 5.** Status: implemented; PR open. Lane: `process-signal-diagnostic-completeness`, worktree `../pmacs-signal-identity`, based on `githubsucks/main` @ `4cd4a7b` (re-measure at branch time; this is a reading, not a constant). @@ -17,6 +17,33 @@ gets signalled, no disposition change.** Everything behavioural is in §5. ## Revision history +**Revision 4 → 5**, after implementation. **Bet 1 was falsified by CI**, +and the framing's own fallback is what shipped. + +- **`bash -m` does not diverge on macOS.** Both macOS legs reported + `job control never moved the terminal off the leader (leader=8542, + foreground groups observed: [8542])` — the terminal stayed with the + leader for the entire 10s bounded wait. It diverges reliably on Linux + (20/20 locally, green on both ubuntu legs), so this is a platform + difference, not a flake, and rerunning would have been wrong. +- **The stated fallback was taken**: the divergent case is now pinned by + **injecting** the foreground group at the `signal_target` seam, and + §3 Bet 1 records it as *weaker* than a real shell rather than + quietly equivalent. Verified still discriminating — the + `leader_pid`-substitution mutation fails it (`target=-1707909` against + an expected `-1707910`). +- **The real fixture is retained as corroboration**, Linux-only, under + `job_control_really_diverges_the_foreground_group`. It is skipped on + macOS by platform check rather than by arming, because the + precondition genuinely does not hold there; running it would assert a + false claim about macOS instead of finding a bug. +- **`PMACS_REQUIRE_BASH` moves to Linux-only.** Rev 4's reasoning for + arming it on both platforms — "macOS is where the failures happen, so + arming it Linux-only leaves it dark where it matters" — was correct + about the *diagnostic* and wrong about *this test*, which cannot + produce its precondition on macOS at all. The diagnostic's macOS + coverage comes from the injected pin, which runs everywhere. + **Revision 3 → 4**, after review round 3 (three blocking, one major). All four accepted and checked against the exact APIs, process model, and branch ancestry before revision. @@ -366,8 +393,17 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. - *Falsified if* the fixture cannot be made deterministic in CI. Then the lane falls back to pinning divergence at the `signal_target` unit level with an injected foreground group, and labels that as weaker. - - Without this the diagnostic remains unverified in the only case it - exists for. + - **OUTCOME: falsified on macOS.** Both macOS legs observed the + terminal stay with the leader for the full bounded wait; Linux + diverges reliably. The fallback shipped: the divergent case is + pinned by injection everywhere, and the real shell corroborates it + on Linux only. + - **The injected pin is weaker, and here is exactly how.** It proves + the target is read from the *lookup* rather than substituted from + the leader — the substitution mutation still fails it. It does + **not**, by itself, prove any real shell produces that divergence; + `job_control_really_diverges_the_foreground_group` carries that, on + one platform. - **Bet 2 — the PTY fallback is reachable and distinguishable.** A test drives all three non-success arms: a duplicate errno, a `tcgetpgrp` diff --git a/src/process.rs b/src/process.rs index 7f412ae..70c1c80 100644 --- a/src/process.rs +++ b/src/process.rs @@ -478,8 +478,8 @@ pub struct ProcessSupervisor { /// observation would bypass the code path under test. forced_kill_errno: Option, /// Test seam for the PTY foreground-group lookup (see - /// `force_next_pty_lookup_failure`). Always `None` outside tests. - forced_pty_lookup: Option, + /// `force_next_pty_lookup`). Always `None` outside tests. + forced_pty_lookup: Option>, } /// One armed group in the reap ledger. @@ -881,7 +881,7 @@ struct SignalTarget { fn signal_target( proc: &ManagedProcess, pid: u32, - forced_lookup: Option, + forced_lookup: Option>, ) -> Result { if let Some(runtime) = proc.runtime.as_ref() && let ChildHandle::Pty { @@ -895,7 +895,7 @@ fn signal_target( // produced a bare `LeaderPid`, identical to a pipe child that // never had a terminal at all. let lookup = match forced_lookup { - Some(failure) => Err(failure), + Some(outcome) => outcome, None => pty_foreground_group(master.as_ref()), }; return match lookup { @@ -1151,16 +1151,26 @@ impl ProcessSupervisor { /// Test seam for the PTY foreground-group lookup, on the same terms /// as [`Self::force_next_kill_errno`] and for the same reason. /// - /// The three non-success arms — no master fd, a failed duplicate, a - /// failed `tcgetpgrp` — cannot be produced on demand from a healthy - /// PTY: they need an exhausted descriptor table or a master that has - /// stopped being a terminal. Injecting only the *lookup result* - /// leaves the branch itself, the fallback target choice, the leader - /// observation against the real child, and the report construction - /// all running as production code. Consumed by one call. + /// Injects either outcome. The failure arms — no master fd, a failed + /// duplicate, a failed `tcgetpgrp` — cannot be produced on demand + /// from a healthy PTY: they need an exhausted descriptor table or a + /// master that has stopped being a terminal. + /// + /// The **success** arm exists because a genuinely divergent + /// foreground group is not portable. `bash -m` produces one on + /// Linux and **does not on macOS**, where the terminal stays with + /// the leader for the whole wait (observed in CI on both macOS + /// legs). Injecting the group keeps the divergent case pinned + /// everywhere; `job_control_really_diverges_the_foreground_group` + /// corroborates it against a real shell where the platform allows. + /// + /// Either way the injection covers only the *lookup result*: the + /// branch, the target choice, the leader observation against the + /// real child, and the report construction all run as production + /// code. Consumed by one call. #[cfg(test)] - fn force_next_pty_lookup_failure(&mut self, failure: PtyLookupFailure) { - self.forced_pty_lookup = Some(failure); + fn force_next_pty_lookup(&mut self, outcome: Result) { + self.forced_pty_lookup = Some(outcome); } /// Override the SIGTERM-to-SIGKILL grace window. Test helper. @@ -2706,50 +2716,46 @@ mod tests { /// Q#DC1 / acceptance 1 — a group-directed failure names the target, /// the branch that chose it, the expected group, the errno, and the - /// leader's own state, as five separate facts **that are not the same - /// fact repeated**. + /// leader's own state, as facts **that are not the same fact + /// repeated**. /// - /// The pre-Stage-B version of this test spawned `/bin/sleep` on a PTY - /// and asserted the same pid three times, conceding in its own - /// comment that the values "are asserted to agree only because - /// nothing has moved the terminal". An implementation that ignored - /// `tcgetpgrp` entirely and substituted `leader_pid` passed it. Since - /// the whole premise of the diagnostic is that these two entities can - /// diverge, that test pinned the substitution as acceptable. + /// The pre-Stage-B version spawned `/bin/sleep` on a PTY and asserted + /// the same pid three times, conceding in its own comment that the + /// values "are asserted to agree only because nothing has moved the + /// terminal". An implementation that ignored `tcgetpgrp` and + /// substituted `leader_pid` passed it — so it pinned the substitution + /// as acceptable. /// - /// This version drives job control so the terminal genuinely belongs - /// to a different process group, and asserts the two values **differ** - /// as well as asserting each exactly. + /// **The foreground group is injected, not produced by a shell.** + /// Framing Bet 1 wagered that a real job-control fixture would be + /// deterministic in CI; it is not. `bash -m` diverges reliably on + /// Linux and never on macOS, where CI observed the terminal stay with + /// the leader for a full 10s wait on both legs. The framing's stated + /// fallback is this: pin the divergence at the `signal_target` level + /// with an injected foreground group, and **say plainly that it is + /// weaker** than a real one. + /// + /// What it still proves: the target is read from the *lookup* rather + /// than substituted from the leader, because the two values differ + /// here and the assertion names both. What it no longer proves on its + /// own: that a real shell ever produces that divergence — + /// `job_control_really_diverges_the_foreground_group` carries that, + /// on the platforms where it is real. #[test] fn a_group_directed_kill_failure_reports_target_and_leader_separately() { - // Guard on the exact path the fixture spawns, not on `which bash`. - // The two can disagree — a system with bash on PATH but not at - // `/bin/bash` would pass a `which` guard and then fail the spawn, - // which is the guard failing to guard the thing it names. - if !std::path::Path::new(BASH).exists() { - let armed = std::env::var_os("PMACS_REQUIRE_BASH").is_some_and(|v| !v.is_empty()); - assert!( - !armed, - "PMACS_REQUIRE_BASH is set but {BASH} does not exist: the \ - job-control divergence fixture cannot run" - ); - eprintln!( - "{BASH} not present; skipping \ - a_group_directed_kill_failure_reports_target_and_leader_separately" - ); - return; - } - let mut sup = ProcessSupervisor::new(); - let (id, pid, fg) = spawn_pty_with_diverged_foreground_group(&mut sup, "diag-group"); + let (id, pid) = spawn_live_pty(&mut sup, "diag-group"); + // A foreground group that is deliberately NOT the leader. let leader_i32 = i32::try_from(pid).expect("pid fits i32"); + let fg = leader_i32 + 1; assert_ne!( fg, leader_i32, - "the fixture must make the foreground group differ from the leader; \ - equal values would let a leader_pid substitution pass" + "the injected group must differ from the leader or this test \ + cannot distinguish a substitution" ); + sup.force_next_pty_lookup(Ok(fg)); sup.force_next_kill_errno(nix::errno::Errno::EPERM); let err = sup.terminate(id).expect_err("injected EPERM must fail"); @@ -2759,17 +2765,16 @@ mod tests { ); assert_eq!( err, expected, - "the report names the exact group the tty reported, the exact \ + "the report names the exact group the lookup returned, the exact \ leader pid, and observes the leader as live" ); - // The target came from the terminal, not from the leader. Stated - // as its own assertion so a regression that reintroduces the - // substitution fails here by name rather than inside a long - // string comparison. + // Stated separately so a regression that reintroduces the + // substitution fails by name rather than inside a long string + // comparison. assert!( err.contains(&format!("target=-{fg} via tcgetpgrp")), - "the target must be the terminal's foreground group: {err}" + "the target must be the group the lookup returned: {err}" ); assert!( !err.contains(&format!("target=-{pid} via tcgetpgrp")), @@ -2779,6 +2784,57 @@ mod tests { let _ = sup.signal(id, Signal::SIGKILL); } + /// Corroboration for the injected divergence above: a **real** shell + /// under job control does hand the terminal to a different process + /// group, and the production lookup reads it. + /// + /// Linux-only by arming. macOS is not a skip-because-untested: CI + /// observed `bash -m` there keep the terminal on the leader for the + /// entire bounded wait, on both legs, so the precondition this test + /// needs genuinely does not hold on that platform. Running it there + /// would assert a false claim about macOS rather than find a bug. + #[test] + fn job_control_really_diverges_the_foreground_group() { + if !std::path::Path::new(BASH).exists() { + let armed = std::env::var_os("PMACS_REQUIRE_BASH").is_some_and(|v| !v.is_empty()); + assert!( + !armed, + "PMACS_REQUIRE_BASH is set but {BASH} does not exist: the \ + job-control divergence fixture cannot run" + ); + eprintln!( + "{BASH} not present; skipping job_control_really_diverges_the_foreground_group" + ); + return; + } + if !cfg!(target_os = "linux") { + eprintln!( + "job control does not hand over the terminal for a \ + non-interactive `bash -m` on this platform; skipping" + ); + return; + } + + let mut sup = ProcessSupervisor::new(); + let (id, pid, fg) = spawn_pty_with_diverged_foreground_group(&mut sup, "diag-jobctl"); + + let leader_i32 = i32::try_from(pid).expect("pid fits i32"); + assert_ne!( + fg, leader_i32, + "a real job-control shell must move the terminal off the leader" + ); + + // And the production lookup — not the fixture's own polling — + // reports that same group. + let observed = foreground_pgid(&sup, id).expect("PTY reports a foreground group"); + assert_eq!( + observed, fg, + "the production accessor must see what the fixture waited for" + ); + + let _ = sup.signal(id, Signal::SIGKILL); + } + /// Q#DC2 / acceptance 2 — a PTY whose foreground-group lookup fails /// is distinguishable from a pipe child that never had a terminal. /// @@ -2811,7 +2867,7 @@ mod tests { let mut sup = ProcessSupervisor::new(); let (id, pid) = spawn_live_pty(&mut sup, "diag-pty-fallback"); - sup.force_next_pty_lookup_failure(failure); + sup.force_next_pty_lookup(Err(failure)); sup.force_next_kill_errno(nix::errno::Errno::EPERM); let err = sup.terminate(id).expect_err("injected EPERM must fail"); From 95897f75636a0bd3ffa10b4a1cadbac8bce04e42 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 14:27:46 -0400 Subject: [PATCH 7/8] fix(process): sample the group before the kill; make corroboration real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 4: three blocking, one major. All accepted. **`measured_group` was sampled after the failure.** It ran inside `signal_failure_report`, after the `kill` and after `observe_leader`, while the framing and the function's own doc both said before. A concurrent group change would have made the diagnostic report post-failure state as evidence about the attempted target. It is now sampled in `signal` before the kill and passed into the report, so the field describes the target that was attempted rather than the state the failure left behind. **The Linux corroboration did not exercise the production lookup.** Its helper read `portable_pty::process_group_leader` — the accessor this lane stopped using on the signal path — so `pty_foreground_group` could have fallen back on every call with every test still green. Forcing it to always fall back demonstrates the hole exactly: the corroboration fails, and the injected pin PASSES, because the injected tests supply the group themselves and structurally cannot detect a broken lookup. The helper now calls the production lookup, and the corroboration forces only the kill so the report is built from a real terminal read. The residual limitation is recorded rather than left to the green: on macOS `pty_foreground_group` has no end-to-end coverage, because the platform cannot produce the precondition. **The framing did not update its own acceptance contract.** Revision 5 recorded Bet 1's falsification in the revision history and in the bet, and left the normative criterion demanding the real-shell rewrite — the "implementation quietly diverges from the contract" shape this project already recorded as a lesson on #191/#188. Acceptance 1 now describes the injected pin, 1a adds the corroboration and its macOS limitation, and 4 states the sampling order. The ledger is synchronized: revision 6, four commits, 4,471 tests, bash armed on Linux only. **`TargetSource`'s doc had the wrong classification.** Two of the four variants target the leader pid, not one, and the pid-versus-group split does not line up with PTY-versus-pipe — which is why the fallback needed its own variant rather than reusing `LeaderPid`. Gates: 11 gates, 4,471 tests, zero failures. --- docs/active-work.md | 29 ++++--- ...-signal-diagnostic-completeness-framing.md | 53 ++++++++++-- src/process.rs | 85 ++++++++++++++----- 3 files changed, 128 insertions(+), 39 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 33ad59d..28be8ca 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -122,11 +122,13 @@ If it does not, stop and repair the remote/fetch configuration. - **Portable branch:** `githubsucks/process-signal-diagnostic-completeness`; worktree `../pmacs-signal-identity`. Governing document: - `docs/process-signal-diagnostic-completeness-framing.md`, **revision 4, - approved** after three review rounds. -- **State:** implemented and gated; canonical `githubsucks/main` @ - `b8e18f6` (the ledger absorption #199) is integrated. Two commits — - Bet 1 alone, then Bets 2–4 — per the framing's branch plan. + `docs/process-signal-diagnostic-completeness-framing.md`, **revision 6**; + the contract was approved at revision 4 and has been revised twice + since to match what shipped. +- **State:** **PR #200 open**, four review rounds closed, held for + review. Canonical `githubsucks/main` @ `b8e18f6` (the ledger + absorption #199) is integrated. Four commits: Bet 1 alone, Bets 2–4, + Bet 1's fallback after CI falsified it, then round 4's corrections. - **Boundary, unchanged:** evidence collection only. No signal retargeting, tolerance, disposition change, or reap-ledger repair. **Group identity remains unprovable** (framing §1.5): the measured @@ -143,7 +145,7 @@ If it does not, stop and repair the remote/fetch configuration. claiming "EPERM cannot happen for our own children" is corrected **without** claiming the child itself received EPERM. - **Verification at the merged tree** (not inherited from the pre-merge - head): 11 gates, **4469 tests, zero failures** — fmt, diff-check, + head): 11 gates, **4471 tests, zero failures** — fmt, diff-check, clippy, `--lib`, `--lib --features crdt`, compile-mode, copy-mode in both feature configurations, bottom-panel Stage 1, M4 with the basedpyright skip, and required GPU. The job-control divergence @@ -157,11 +159,16 @@ If it does not, stop and repair the remote/fetch configuration. Separately, the **pre-Stage-B test was restored verbatim under the substitution mutation and PASSED**, which is the finding that justified rewriting it rather than adding to it. -- **`/bin/bash` is a declared optional test dependency**, armed by - `PMACS_REQUIRE_BASH` on **both** CI platforms rather than Linux only: - the failures this diagnostic exists to explain have so far occurred - only on macOS, so arming it Linux-only would leave it dark exactly - where it matters. +- **Bet 1 was falsified by CI and the framing's fallback shipped.** + `bash -m` diverges on Linux and **never on macOS**, where both legs + observed the terminal stay with the leader for a full 10s wait. The + divergent case is now pinned by **injecting** the foreground group + (runs everywhere, weaker); a Linux-only corroboration drives a real + shell and is the **only** test exercising `pty_foreground_group` + end-to-end. `/bin/bash` is a declared optional test dependency armed + by `PMACS_REQUIRE_BASH` on **Linux only** — macOS ships bash but + cannot produce the precondition, so arming it there would make a + missing binary fatal for a test that can never run. - **Recovery from a clean checkout:** ```sh diff --git a/docs/process-signal-diagnostic-completeness-framing.md b/docs/process-signal-diagnostic-completeness-framing.md index 7dad7fe..5c41617 100644 --- a/docs/process-signal-diagnostic-completeness-framing.md +++ b/docs/process-signal-diagnostic-completeness-framing.md @@ -1,6 +1,6 @@ # Framing — make the signal diagnostic discriminating (evidence collection) -**Revision 5.** Status: implemented; PR open. Lane: +**Revision 6.** Status: implemented; PR open, review round 4 closed. Lane: `process-signal-diagnostic-completeness`, worktree `../pmacs-signal-identity`, based on `githubsucks/main` @ `4cd4a7b` (re-measure at branch time; this is a reading, not a constant). @@ -17,6 +17,32 @@ gets signalled, no disposition change.** Everything behavioural is in §5. ## Revision history +**Revision 5 → 6**, after review round 4 (three blocking, one major). +All four accepted. + +- **`measured_group` was sampled AFTER the failed `kill`** and after + `observe_leader`, while both the framing and the function's own doc + said before. A concurrent group change would have made the diagnostic + report post-failure state as evidence about the attempted target. It + is now sampled in `signal` before the kill and passed into the report. +- **The Linux corroboration did not exercise the production lookup.** + Its helper read `portable_pty::process_group_leader` — the accessor + this lane stopped using — so `pty_foreground_group` could have fallen + back on every call with every test still green. Demonstrated: forcing + it to always fall back leaves the injected pin **passing** and only + the corroboration failing. The helper now calls the production lookup, + and the corroboration forces *only* the kill so the report is built + from a real terminal read. +- **This document did not update its own acceptance contract** (§4.1). + Revision 5 recorded the falsification in the revision history and Bet + 1 but left the normative criterion demanding the real-shell rewrite — + the exact "implementation quietly diverges from the contract" shape + this project recorded as a lesson on #191/#188. +- **`TargetSource`'s doc had the wrong classification.** Two of four + variants now target the leader pid, not one, and the pid-versus-group + split does not line up with PTY-versus-pipe — which is *why* + `PtyForegroundFallback` needed its own variant. + **Revision 4 → 5**, after implementation. **Bet 1 was falsified by CI**, and the framing's own fallback is what shipped. @@ -431,10 +457,24 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. ## 4. Acceptance -1. A PTY job-control fixture where `tcgetpgrp` != leader pid, both exact - values asserted and asserted to differ. The test at `:2400` is - **rewritten**, not supplemented — it currently pins a substitution as +1. The divergent case is pinned on **every** platform: a group-directed + failure whose target differs from the leader pid, with both exact + values asserted and asserted to differ. The pre-Stage-B test is + **rewritten**, not supplemented — it pinned a substitution as acceptable. + + **The divergence is injected at the `signal_target` seam**, per Bet + 1's falsification: a real `bash -m` fixture diverges on Linux and + never on macOS. The injected form is weaker and §3 Bet 1 says how. + +1a. **A Linux-only corroboration** drives a real job-control shell, + forces *only* the kill failure, and asserts the production report + names the real foreground group. This is the sole test that exercises + `pty_foreground_group` end-to-end — every other test supplies the + group itself and therefore cannot detect a lookup that always falls + back. **Residual limitation, stated rather than buried: on macOS the + production lookup has no end-to-end coverage**, because the platform + cannot produce the precondition. 2. The PTY foreground-lookup fallback reports a source distinct from a pipe child's. Separate tests drive the duplicate-error and `tcgetpgrp`-error arms and assert the exact stage and errno; a third @@ -450,7 +490,10 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. while a successful `SIGTERM` does. 4. For `spec.group` children the report carries the measured pgid as a field distinct from the assumed one, renderable as unobservable, with - a test asserting a case where they **differ** (§3 Bet 4). + a test asserting a case where they **differ** (§3 Bet 4). **It is + sampled before the `kill`**, not during report construction, so it + describes the target that was attempted rather than the state left + behind by the failure. 5. All four exact-string sites — `:2408`, `:2435`, `:2485`, `:2517` — updated **individually**, each listed in the PR body with before and after. No blanket rewrite: that is how a format regression hides. diff --git a/src/process.rs b/src/process.rs index 70c1c80..ea9b377 100644 --- a/src/process.rs +++ b/src/process.rs @@ -717,10 +717,18 @@ impl ChildHandle { /// Which branch of [`signal_target`] chose the target (Q#PD1). /// /// Recorded on failure because the branches differ in what a failing -/// `kill` can possibly mean: only [`Self::LeaderPid`] aims at the -/// spawned child itself. The other two aim at a *group*, which for a -/// PTY is read from the terminal and can belong to something the -/// supervisor never spawned. +/// `kill` can possibly mean. Two of the four aim at the spawned child +/// itself — [`Self::LeaderPid`] for a pipe child with no group, and +/// [`Self::PtyForegroundFallback`] for a PTY whose terminal named no +/// group. The other two aim at a *group*: +/// [`Self::SpawnGroup`] at one computed from the spawn-time `pgid == +/// pid` assumption, and [`Self::ForegroundGroup`] at one read from the +/// terminal, which can belong to something the supervisor never spawned. +/// +/// The pid-versus-group split is the classification that matters here, +/// and it does **not** line up with the PTY-versus-pipe split — which is +/// exactly why the fallback needed its own variant instead of reusing +/// `LeaderPid`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum TargetSource { /// The tty's current foreground process group, read at signal @@ -976,7 +984,7 @@ fn observe_leader(proc: &mut ManagedProcess) -> LeaderObservation { /// makes it worth printing. /// /// **It does not establish identity** (framing §1.5). It is read before -/// the `kill` in the same read-then-act window, and a number cannot +/// the `kill`, in the same read-then-act window, and a number cannot /// distinguish the original group from a recycled one. No portable /// mechanism can: `pidfd` closes pid reuse for a process, not a group, /// and macOS has none at all. This records an observation; it settles @@ -1007,6 +1015,7 @@ fn signal_failure_report( signal: Signal, errno: nix::errno::Errno, leader: &LeaderObservation, + measured: Option<&str>, ) -> String { let expected = if target.source.is_group() { match i32::try_from(leader_pid) { @@ -1016,15 +1025,11 @@ fn signal_failure_report( } else { String::new() }; - // Only the spawn-group path computes its target from the assumption, - // so it is the only one where a measurement can contradict anything. - // A PTY target came from the terminal and a leader-directed target is - // not a group at all. - let measured = if matches!(target.source, TargetSource::SpawnGroup) { - measured_group_of(leader_pid) - } else { - String::new() - }; + // Supplied by the caller, which samples it BEFORE the `kill`. Doing + // it here would describe the group as it stands *after* the failure + // and after `observe_leader`, which is post-hoc state presented as + // evidence about the attempted target. + let measured = measured.unwrap_or(""); format!( "kill: {errno} (signal={signal:?}, target={} via {}, leader_pid={leader_pid}{expected}{measured}, leader={})", target.pid.as_raw(), @@ -1292,6 +1297,13 @@ impl ProcessSupervisor { }; let forced_lookup = self.forced_pty_lookup.take(); let target = signal_target(proc, pid, forced_lookup)?; + // Sample the real group BEFORE signalling. Only the spawn-group + // path computes its target from the `pgid == pid` assumption, so + // it is the only one a measurement can contradict; a PTY target + // came from the terminal and a leader-directed target is not a + // group at all. + let measured = + matches!(target.source, TargetSource::SpawnGroup).then(|| measured_group_of(pid)); // Q#PD4: the seam injects the KILL attempt's result only — // never the observation below — so target selection, the real // `ChildHandle::try_wait` against the real child, and the error @@ -1305,7 +1317,14 @@ impl ProcessSupervisor { // disposition is unchanged — this still returns `Err`, // with no state transition and no ledger arming. let leader = observe_leader(proc); - return Err(signal_failure_report(target, pid, signal, errno, &leader)); + return Err(signal_failure_report( + target, + pid, + signal, + errno, + &leader, + measured.as_deref(), + )); } if matches!(signal, Signal::SIGTERM | Signal::SIGKILL | Signal::SIGHUP) { proc.state = ProcessState::Exiting { @@ -2628,15 +2647,19 @@ mod tests { (id, spawn_started_pid(sup, id)) } - /// The tty's current foreground process group, read through the same - /// `MasterPty` accessor production uses. `None` for a pipe + /// The tty's current foreground process group, read through the + /// **production** lookup — not `portable_pty`'s + /// `process_group_leader`, which this crate no longer uses on the + /// signal path. Reading it any other way would let + /// `pty_foreground_group` fall back on every call while every test + /// that depends on it stayed green. `None` for a pipe /// generation, or when the terminal reports no foreground group. fn foreground_pgid(sup: &ProcessSupervisor, id: ProcessId) -> Option { let runtime = sup.processes.get(&id)?.runtime.as_ref()?; match &runtime.child { ChildHandle::Pty { _master: master, .. - } => master.process_group_leader(), + } => pty_foreground_group(master.as_ref()).ok(), ChildHandle::Pipes(_) => None, } } @@ -2824,12 +2847,28 @@ mod tests { "a real job-control shell must move the terminal off the leader" ); - // And the production lookup — not the fixture's own polling — - // reports that same group. - let observed = foreground_pgid(&sup, id).expect("PTY reports a foreground group"); + // Force ONLY the kill failure. The lookup is left alone, so + // `pty_foreground_group` runs for real against a real terminal + // and the report below is built from what it returned. + // + // This is the assertion that makes the injected pin meaningful: + // without it, `pty_foreground_group` could fall back on every + // call and every other test here would still pass, because they + // all supply the group themselves. + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + + let expected = format!( + "kill: {} (signal=SIGTERM, target=-{fg} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)", + nix::errno::Errno::EPERM + ); assert_eq!( - observed, fg, - "the production accessor must see what the fixture waited for" + err, expected, + "the production lookup must report the real foreground group" + ); + assert!( + !err.contains("pty-leader-fallback"), + "a healthy terminal must not take the fallback branch: {err}" ); let _ = sup.signal(id, Signal::SIGKILL); From 262b7c0b6f649350913d66d284692925bd72647d Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 30 Jul 2026 14:41:50 -0400 Subject: [PATCH 8/8] docs(process): stop overclaiming the pre-kill sample; drop a dead comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 5: one blocking, one major. Both documentation-only; the runtime fixes from round 4 are unchanged. **The pre-kill sample was overclaimed.** Acceptance 4 said the measured group "describes the target that was attempted". It does not. `getpgid` and `kill` remain separated by the same read-then-act window §1.5 is built on, so the sample can be stale by the time the signal is delivered. Moving it earlier removes a POST-HOC reading; it does not make the reading contemporaneous. The criterion now says it records pre-kill evidence about the attempted target, and adds an explicit sentence forbidding any acceptance from claiming otherwise. This is worth naming rather than quietly editing: the framing's whole spine is that this telemetry establishes less than it appears to, and round 4 fixed a real ordering defect and then inflated the fix's meaning inside the document that governs it. **`95897f7`'s commit message carries the same overclaim.** It is not amended — force-pushing a branch under review to rewrite history would hide the error rather than correct it. This message is the correction, and the two read in sequence. **A dead doc comment was documenting a constant.** The original pre-Stage-B acceptance-1 block survived the rewrite and came to rest above `const BASH`, so the file explained a string constant with the sentence this entire lane exists to remove — "here they are asserted to agree only because nothing has moved the terminal", describing a test that no longer exists. Removed; only the BASH explanation remains. Verified the overclaim is gone from the whole tree, not only the cited line: no hits in `docs/` or `src/`. Gates: 11 gates, 4,471 tests, zero failures. --- ...process-signal-diagnostic-completeness-framing.md | 11 +++++++++-- src/process.rs | 12 ------------ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/docs/process-signal-diagnostic-completeness-framing.md b/docs/process-signal-diagnostic-completeness-framing.md index 5c41617..b945da7 100644 --- a/docs/process-signal-diagnostic-completeness-framing.md +++ b/docs/process-signal-diagnostic-completeness-framing.md @@ -492,8 +492,15 @@ assertion (`:2517`) is one of the four sites acceptance 5 must update. field distinct from the assumed one, renderable as unobservable, with a test asserting a case where they **differ** (§3 Bet 4). **It is sampled before the `kill`**, not during report construction, so it - describes the target that was attempted rather than the state left - behind by the failure. + records pre-kill evidence about the target that was attempted rather + than state left behind by the failure. + + **It does not describe the group at the moment the `kill` executed.** + `getpgid` and `kill` remain separated by the read-then-act window + §1.5 describes, so the sample can be stale by the time the signal is + delivered. Moving it earlier removes a *post-hoc* reading; it does + not make the reading contemporaneous, and no acceptance may claim it + does. 5. All four exact-string sites — `:2408`, `:2435`, `:2485`, `:2517` — updated **individually**, each listed in the PR body with before and after. No blanket rewrite: that is how a format regression hides. diff --git a/src/process.rs b/src/process.rs index ea9b377..33fdf8f 100644 --- a/src/process.rs +++ b/src/process.rs @@ -2617,18 +2617,6 @@ mod tests { } } - /// Q#PD1 acceptance 1 — a group-directed failure names the target, - /// the branch that chose it, the expected group, the errno, and the - /// leader's own state, as five separate facts. - /// - /// Asserted as an exact message against the pid the kernel actually - /// assigned, so a hardcoded target could not satisfy it. The leader - /// field is the one that matters: for a PTY the signal goes to the - /// terminal's foreground group, a different entity from the spawned - /// child whenever job control has moved the terminal. Three rejected - /// designs for this code collapsed the two; the report keeps them - /// apart, and here they are asserted to agree only because nothing - /// has moved the terminal. /// The job-control shell the divergence fixture drives. Named once so /// the availability guard and the spawn cannot drift apart. const BASH: &str = "/bin/bash";