neuwirth
/
pmacs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #176 from levineuwirth/pty-terminate-eperm 

fix(process): make a failing kill describe itself (diagnostic only)
This commit is contained in:
Levi Neuwirth 2026-07-26 20:23:27 +00:00 committed by GitHub
parent fe8b8ba4c6 c5c9f3f13a
commit bf8878f4fa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 782 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
docs/active-work.md
View File

@ -60,6 +60,92 @@ git status --short --branch
The `git log` command must expose `d152120` or a newer intentional main.
If it does not, stop and repair the remote/fetch configuration.
## PTY terminate diagnostic lane — IN REVIEW (PR #176)
- Portable branch: `githubsucks/pty-terminate-eperm`; worktree
`../pmacs-math-slice`. **PR #176**, base `main`, based on `ccf29e3`
with `c93f9ee` (#175) merged in.
- Approved framing: `docs/process-signal-tolerance-framing.md`
**revision 4**, after three review rounds.
- **Diagnostic only. No disposition change.** Every call that failed
before still fails, with no state transition and no reap-ledger
arming. `src/process.rs` is the only source file touched.
- **Why nothing is fixed:** revisions 1–3 each proposed a *tolerance*
rule and all three were rejected as unsound in the same way — each
concluded something about a process from something that was not about
that process. Rev 1 from an errno alone (EPERM means the caller lacks
permission, not that the id was recycled); rev 2 from `try_wait`,
which observes the spawned **leader** while a PTY signal targets
`-tcgetpgrp(...)`, entities that diverge exactly when job control has
moved the terminal; rev 3 from group-directed **ESRCH**, which proves
only that the selected foreground group vanished.
- **Two facts that killed the original argument.** `group = true` is
*rejected* for PTY mode at spawn (`src/process.rs:1428-1429`), so the
reap ledger never applies to the PTY path at all; and the ledger
comment (`:1075`) says EPERM "cannot happen for our own children" and
drops the entry for **bounded growth** — not a ruling that EPERM means
dead.
- **The CI evidence never established the child had exited.** The probe's
last source statement is a file write and CPython teardown does not
synchronise with it, so no tolerance rule could even be shown to fix
the symptom. That is the whole reason the lane is diagnostic.
- What ships: a failing `kill` now reports five separate facts — target
source, target kind/value, spawn-time group, errno, and the leader's
real `try_wait` state. The test seam injects the **kill result only**,
never the observation, so the real `ChildHandle::try_wait` runs against
the real child.
- **Not "strictly additive".** `try_wait` reaps and caches, so an exited
child may be reaped earlier than otherwise. Safe because
`portable-pty` 0.9.0 returns a `std::process::Child` on Unix and
delegates `try_wait` to it, so `poll_one` still sees the cached
status — pinned by an exactly-one-terminal-event test rather than
assumed.
- Round-1 review fixes: the exited-child tests no longer use a fixed
sleep as proof of exit (nix's `waitid` is unavailable on macOS and
`libc::waitid` needs `unsafe`, which the crate forbids), instead
driving the production diagnostic in a bounded loop until it observes
the exit; and every assertion is now exact message equality built from
the kernel-assigned pid, since the substring forms would have accepted
a hardcoded target or a wrong exit code.
- Bites, all verified rather than assumed: tolerating the failure fails
the disposition test; stubbing the leader observation fails three
tests including the one-event pin; a hardcoded target fails four; a
wrong exit code fails two.
- **The sweep found a real defect in these tests, not a flake.**
`observing_the_leader_does_not_consume_the_exit_event` failed with
"process ProcessId(26) is not running": the pid helper drained for
`Started`, and **`drain_until` ticks**. A tick can observe an
immediately-exiting child and move the record out of `Running`, after
which `signal` never reaches the diagnostic at all, so the bounded
loop spun to its limit. It passed standalone because the drain
returned on `Started` before `poll_one` saw the exit; only load lost
the race. Fast-exiting children now read the pid straight from the
supervisor record (no tick), and the loop fails fast if the record
left `Running`. **Verified under matched load: 0/15 with all 16 cores
saturated, while the old ticking helper fails 1/10 — the fix is
load-bearing.**
- Verification: fmt, `git diff --check`, strict workspace clippy clean;
lib 1,838 + CRDT 2,015 (both +6, exactly the new tests); GPU 202; M4
121; bottom-panel 46; compile-mode 67; vterm 9/6/5; **isolated-config
`--no-fail-fast` sweep 3,258 across 93 suites, zero failures**.
Earlier sweeps on this branch showed two failures and then one; the
totals reconcile (3,256/2 → 3,257/1 → 3,258/0, same test count). The
two that were genuinely unrelated —
`read_dir_supersede_cancels_in_flight_predecessor` (known
pre-existing) and
`headless_snapshot_round_trip_summary_restores_the_minimap` — are
load-contention flakes; the second is structurally unreachable from
this diff, since `pmacs-gpu` depends on `pmacs-protocol` and never on
`pmacs`.
- **Parked, each with its reason:** all tolerance rules (need the
evidence this PR produces); `terminate` idempotence for an
already-reaped process (independent fix, different failure, one
feature per PR); and `signal_target`'s read-then-kill of `tcgetpgrp`
— still the most likely real fix site.
- **The lane closes when this merges.** It does not wait for the flake
to recur; the next occurrence carries its own evidence under whoever's
PR, and a Stage B framing follows then.
## Lean 4 lane (Arc 8) — Stages 1, 2, 3a, 3b, 4a MERGED; 4b is next
- **Stages 1, 2, 3a and 3b are MERGED** — #160 (`main` @ `0827dd1`),

258
docs/process-signal-tolerance-framing.md Normal file
View File

@ -0,0 +1,258 @@
# Framing — make the PTY terminate failure self-describing (diagnostic only)
**Revision 4.** Status: awaiting review round 4. Lane:
`pty-terminate-eperm`, worktree `../pmacs-math-slice`, based on
`githubsucks/main` @ `ccf29e3`.
**Diagnostic only. No disposition changes, no tolerance rules, no
behavioural fix.** Every rule this document proposed across revisions 1
to 3 is parked (§5). The lane's entire deliverable is that the next
occurrence of the failure explains itself.
## Revision history
**Revision 3 → 4**, after review round 3 (two blocking, one major) and
its scope call. All accepted.
- **Group-directed ESRCH was also unsafe**, for the same reason EPERM
was: it proves the selected *foreground group* vanished, not that the
leader exited. A job-control race — foreground job exits after
`tcgetpgrp` and before `kill`, shell alive and not yet reclaiming the
terminal — would have been reported as success with the leader never
signalled. Rev 3's acceptance 7 pinned that unsafe behaviour. **All
tolerance is parked** (§5).
- **Rev 3's Stage A implemented Stage B.** It declared itself
diagnostic-only, then listed tolerance and bookkeeping acceptances.
Removed.
- **Q#PS6 (already-reaped `terminate` is `Ok`) is parked separately.**
It is an independent behavioural fix answering a different failure;
under one-feature/one-PR it does not ride with instrumentation.
- **"Strictly additive / cannot regress behaviour" was overstated** and
is narrowed (Q#PD3).
- The injected-kill seam is restored as an explicit decision (Q#PD4).
**Rounds 1–3, for the record.** Rev 1 classified on errno alone and
claimed a live owned child cannot yield EPERM — false. Rev 2 gated on
`try_wait`, which observes the leader while a PTY signal targets the
foreground group — unsound whenever those diverge, and it could not be
shown to fix the observed failure at all. Rev 3 corrected EPERM but left
ESRCH unsafe and mixed the stages. **Three consecutive designs were
wrong in the same direction: each tried to conclude something about a
process from something that was not about that process.**
## 0. Coherence impact (COHERENCE §20)
- **Journey step 8, "Open a terminal"** (§2), teardown half. **No grade
change and no behavioural change** — this lane only improves what a
failure reports.
- **Serves §9 (worker model), failure attribution**, in its most literal
sense: an error that names only an errno cannot be attributed.
- **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 (scouted @ `ccf29e3`, re-verified each revision)
### 1.1 The failure reports an errno and nothing else
`ProcessSupervisor::signal` (`src/process.rs:921`) maps the `kill`
failure to `format!("kill: {e}")` (`:931`). That string is everything a
reader gets.
### 1.2 The signal target is not the observation target
- **Signal target** — `signal_target` (`:687`) returns `-pgrp` for a
PTY, where `pgrp = master.process_group_leader()`: the tty's
**current foreground process group**, read at signal time.
- **Observation target** — `ChildHandle::try_wait` (`:668`) observes the
**spawned leader**.
They coincide only while the leader owns the terminal. Job control is
precisely the mechanism that makes them diverge, and the PTY path is
**always group-directed by design** — spawn rejects `group = true` for
PTY mode with the rationale that "PTY children already lead their own
session and are signaled group-wide" (`:1428-1429`).
**This is why every tolerance rule across rev 1–3 failed review**, and
why the diagnostic must record the target and the leader state as
*separate* facts.
### 1.3 The reap ledger is disjoint from this path
`tick_reap_ledger` (`:1075`) treats any probe error as "nothing left we
can reach" for **bounded growth**, asserting EPERM "cannot happen for our
own children". It is armed only for `proc.spec.group`, which PTY mode
cannot set. Rev 1's "asymmetry" argument was a misreading; withdrawn.
### 1.4 The observed failure, and the limits of the evidence
macOS CI, PR #172 (**docs-only** diff), `Test (macos-latest / luajit)`,
`acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel`
([attempt 1](https://github.com/levineuwirth/pmacs/actions/runs/30177276839/attempts/1)):
```
in function 'terminate'
cause: ExternalError(Process("kill: EPERM: Operation not permitted"))
```
**Established:** the errno, and the call path
(`pmacs.terminal.terminate` → `session.rs:566` → `signal`).
**Not established:** that the child had exited (the probe's last source
statement is a file write at
`tests/bottom_panel_stage1_acceptance.rs:2239`; CPython teardown follows
and does not synchronise with it); that any pgid was recycled; or what
the signal target actually was.
**This is the whole reason the lane is diagnostic.** Every candidate fix
needs at least one of those three facts, and none is available.
### 1.5 Caller inventory
| Caller | Disposition |
|---|---|
| `src/lsp.rs:1364`, `:2427` | discards (`let _ =`) |
| `src/mcp.rs:1229`, `:1239`, `:1915` | discards (`let _ =`) |
| `src/terminal/session.rs:319`, `:607`, `:635` | discards (`let _ =`) |
| **`src/terminal/session.rs:566`** (propagating at `:577`) | **propagates** as `TerminalError::Process` |
| supervisor-internal `shutdown` path | discards |
| `src/lua_bindings/mod.rs:8150`, `:8164` | propagates to Lua |
| `src/lua_bindings/mod.rs:8717` | propagates (via `session.rs:566`) |
| `src/daemon.rs:4162` | **test-only** `.expect`, not production |
No test in the repository asserts either error string, so widening the
message breaks nothing.
### 1.6 `portable-pty` caches the exit status on Unix
Pinned `portable-pty 0.9.0`: `spawn_command` returns
`std::process::Child` (`unix.rs:228`), and `impl Child for
std::process::Child::try_wait` delegates to
`std::process::Child::try_wait` (`lib.rs:271-277`), which caches into
`self.status`. Both `ChildHandle` variants therefore cache.
## 2. Decisions
### Q#PD1 — what the widened error records
On a `kill` failure in `signal`, the error carries:
| Field | Why |
|---|---|
| **target source** — `tcgetpgrp` vs `group` vs `leader-pid` fallback | which branch of `signal_target` (`:687`) ran |
| **target kind and value** — `-pgid` or `pid`, with the number | the entity actually signalled |
| **spawn-time pgid / leader pid** | a divergence from the target is the job-control hypothesis, visible only by comparison |
| **errno** | as today |
| **leader `try_wait` state** — `exited(status)` / `live` / `unobservable(e)` | separates "the leader is gone" from "the group we signalled is gone" — the distinction all three failed designs collapsed |
Every candidate Stage B rule is decidable from these five together, and
none is decidable from the errno alone.
### Q#PD2 — the disposition is preserved exactly
The call still fails, with the same `Err`, in every case. No state
transition changes, no ledger arming changes, no tolerance. A reader
diffing behaviour should find none.
### Q#PD3 — the honest claim is "no disposition change", not "strictly additive"
Rev 3 said the diagnostic was only an error-string change and could not
regress behaviour. **That overstated it.** `try_wait` on an exited child
**reaps it and caches the status**, so consulting it in the failure path
is an internal state change: the child may be reaped earlier than it
otherwise would be.
Observably safe, because both variants cache (§1.6) and `poll_one`
(`:1133`) will still see `Ok(Some(_))` and emit its event. But safe by
argument is not safe by assertion, so the terminate-failure-then-tick
event pin is retained (acceptance 5).
### Q#PD4 — the injected-kill seam injects the KILL, never the observation
Acceptance 5 needs a forced `kill` failure while the **real**
`ChildHandle::try_wait` runs against the **real** child. A stubbed
observation would bypass exactly the code path in question.
So the seam is a test-only override of the *kill attempt's result*,
consumed once by the signal path; everything downstream — target
selection, the observation, the error construction — runs for real. This
also makes the diagnostic's own fields testable without racing the
kernel.
### Q#PD5 — nothing else lands here
No tolerance rule, no idempotence change, no `signal_target` change. See
§5.
## 3. Bets (falsifiable)
- **B1 — The five fields are sufficient to discriminate the §1.4
hypotheses.** Falsified if a recurrence carries all five and still
leaves the cause ambiguous — which would itself be a finding worth
having.
- **B2 — Widening the message breaks no caller.** Evidence: §1.5, and no
test asserts the string.
*Retracted across revisions and not reinstated:* rev 1's "a live owned
child cannot yield EPERM"; rev 2's "exit observation suffices"; rev 2's
"this removes the failure class"; rev 3's "group ESRCH is safe to
tolerate".
## 4. Acceptance
1. A group-directed `kill` failure produces an error carrying all five
Q#PD1 fields, with the target rendered as `-pgid` and the leader
state distinct from it.
2. A leader-directed `kill` failure does the same, with the target
rendered as `pid` and the target source recorded as the fallback
branch.
3. The leader state renders each of `exited(status)`, `live`, and
`unobservable(e)` correctly.
4. **The disposition is unchanged**: every injected failure still
returns `Err`, with no state transition and no ledger arming
(Q#PD2). Falsified by revert — flipping any arm to `Ok` fails this.
5. **Forced injected kill failure against the real PTY child
observation**, then tick: exactly one exit event, with the correct
status (Q#PD3/Q#PD4). A fully stubbed observation does not satisfy
this and is rejected as vacuous.
6. The existing suites stay green, pinning "no behavioural change" from
the outside.
## 5. Parked (not deferred-and-forgotten — each needs its own evidence)
- **All tolerance rules.** Group-directed EPERM *and* ESRCH both fail on
the §1.2 entity split; leader-directed tolerance is plausible but
unmotivated until evidence shows the fallback branch is ever taken.
Needs Stage A evidence first.
- **Q#PS6, `terminate` on an already-reaped process returning `Ok`.**
Independent behavioural fix, different failure (§1.6 of rev 3), its
own lane under one-feature/one-PR.
- **`signal_target`'s read-then-kill of `tcgetpgrp`** — still the most
likely real fix site, still unframed.
- `terminate` cancelling pending restarts; PTYs in
`pmacs.process.list`; any change to `C-c` delivery.
## 6. Gates
Full suite per `CLAUDE.md`. Touched suites:
`bottom_panel_stage1_acceptance`, the vterm stages, and
`compile_mode_acceptance`. Sweep with `-- --skip basedpyright`.
## 7. Branch plan
`pty-terminate-eperm`, one PR, diagnostic only. This framing is its first
commit; the instrumentation and its tests are the second.
**The lane then closes.** It does not wait for the flake to recur: the
next occurrence — whenever it happens, under whoever's PR — carries its
own evidence, and Stage B is framed then. Math work proceeds immediately
after this lands.

445
src/process.rs
View File

@ -470,6 +470,13 @@ pub struct ProcessSupervisor {
/// TERM→KILL window used when arming the ledger. Constant
/// [`GROUP_TERM_GRACE`] in production; overridable in tests.
group_term_grace: Duration,
/// Q#PD4 test seam: forces the next `kill(2)` attempt in
/// [`Self::signal`] to fail with this errno, consumed once.
/// Always `None` in production — there is no way to set it outside
/// `cfg(test)`. It replaces the *kill result only*, so the leader
/// observation still runs against the real child handle; a stubbed
/// observation would bypass the code path under test.
forced_kill_errno: Option<nix::errno::Errno>,
}
/// One armed group in the reap ledger.
@ -684,7 +691,50 @@ impl ChildHandle {
}
}
fn signal_target(proc: &ManagedProcess, pid: u32) -> Result<Pid, String> {
/// 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.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum TargetSource {
/// The tty's current foreground process group, read at signal
/// time. Diverges from the leader exactly when job control has
/// moved the terminal.
ForegroundGroup,
/// A `group = true` pipe child leading its own process group.
SpawnGroup,
/// The child's own pid.
LeaderPid,
}
impl TargetSource {
fn as_str(self) -> &'static str {
match self {
Self::ForegroundGroup => "tcgetpgrp",
Self::SpawnGroup => "group",
Self::LeaderPid => "leader-pid",
}
}
/// Whether the target is a process group rather than one process.
fn is_group(self) -> bool {
matches!(self, Self::ForegroundGroup | Self::SpawnGroup)
}
}
/// 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).
#[derive(Debug, Clone, Copy)]
struct SignalTarget {
pid: Pid,
source: TargetSource,
}
fn signal_target(proc: &ManagedProcess, pid: u32) -> Result<SignalTarget, String> {
if let Some(runtime) = proc.runtime.as_ref()
&& let ChildHandle::Pty {
_master: master, ..
@ -692,7 +742,10 @@ fn signal_target(proc: &ManagedProcess, pid: u32) -> Result<Pid, String> {
&& let Some(pgrp) = master.process_group_leader()
&& pgrp > 0
{
return Ok(Pid::from_raw(-pgrp));
return Ok(SignalTarget {
pid: Pid::from_raw(-pgrp),
source: TargetSource::ForegroundGroup,
});
}
// `group = true` pipe children lead a fresh process group
// (`process_group(0)` at spawn ⇒ pgid == pid), so fatal signals
@ -700,11 +753,80 @@ fn signal_target(proc: &ManagedProcess, pid: u32) -> Result<Pid, String> {
// (Q#CM3).
if proc.spec.group {
let pgid = i32::try_from(pid).map_err(|e| e.to_string())?;
return Ok(Pid::from_raw(-pgid));
return Ok(SignalTarget {
pid: Pid::from_raw(-pgid),
source: TargetSource::SpawnGroup,
});
}
Ok(Pid::from_raw(
i32::try_from(pid).map_err(|e| e.to_string())?,
))
Ok(SignalTarget {
pid: Pid::from_raw(i32::try_from(pid).map_err(|e| e.to_string())?),
source: TargetSource::LeaderPid,
})
}
/// The spawned leader's state at the moment a `kill` failed (Q#PD1).
///
/// Deliberately reported *beside* the target rather than folded into a
/// verdict: for a PTY the two are different entities whenever job
/// control has moved the terminal, and three successive designs for
/// this code were unsound precisely because they collapsed them.
enum LeaderObservation {
Exited(TermStatus),
Live,
Unobservable(String),
NoRuntime,
}
impl LeaderObservation {
fn render(&self) -> String {
match self {
Self::Exited(TermStatus::Exited(code)) => format!("exited(code {code})"),
Self::Exited(TermStatus::Signaled(sig)) => format!("exited(signal {sig})"),
Self::Live => "live".to_owned(),
Self::Unobservable(e) => format!("unobservable({e})"),
Self::NoRuntime => "no-runtime".to_owned(),
}
}
}
/// Observe the spawned leader. Note this *reaps* an exited child and
/// caches its status; that is why Q#PD3 claims "no disposition change"
/// rather than "strictly additive", and why an event-count test pins
/// that `poll_one` still emits exactly one exit event afterwards.
fn observe_leader(proc: &mut ManagedProcess) -> LeaderObservation {
let Some(runtime) = proc.runtime.as_mut() else {
return LeaderObservation::NoRuntime;
};
match runtime.child.try_wait() {
Ok(Some(status)) => LeaderObservation::Exited(status),
Ok(None) => LeaderObservation::Live,
Err(e) => LeaderObservation::Unobservable(e),
}
}
/// 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.
fn signal_failure_report(
target: SignalTarget,
leader_pid: u32,
errno: nix::errno::Errno,
leader: &LeaderObservation,
) -> String {
let expected = if target.source.is_group() {
match i32::try_from(leader_pid) {
Ok(p) => format!(", expected_group=-{p}"),
Err(_) => String::new(),
}
} else {
String::new()
};
format!(
"kill: {errno} (target={} via {}, leader_pid={leader_pid}{expected}, leader={})",
target.pid.as_raw(),
target.source.as_str(),
leader.render(),
)
}
/// Termination status of one generation. Internal --- the supervisor
@ -807,9 +929,20 @@ impl ProcessSupervisor {
shut_down: false,
reap_ledger: HashMap::new(),
group_term_grace: GROUP_TERM_GRACE,
forced_kill_errno: None,
}
}
/// Q#PD4 test seam: make the next `kill(2)` attempt in
/// [`Self::signal`] report `errno` instead of calling the kernel.
/// Consumed by that one attempt. Everything downstream — target
/// selection, the leader observation against the real child, and
/// the error construction — runs unmodified.
#[cfg(test)]
fn force_next_kill_errno(&mut self, errno: nix::errno::Errno) {
self.forced_kill_errno = Some(errno);
}
/// Override the SIGTERM-to-SIGKILL grace window. Test helper.
pub fn set_grace_period(&mut self, d: Duration) {
self.grace_period = d;
@ -928,7 +1061,21 @@ impl ProcessSupervisor {
return Err(format!("process {id} is not running"));
};
let target = signal_target(proc, pid)?;
nix::sys::signal::kill(target, Some(signal)).map_err(|e| format!("kill: {e}"))?;
// 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
// construction all run for real. Consumed once.
let kill_result = match self.forced_kill_errno.take() {
Some(errno) => Err(errno),
None => nix::sys::signal::kill(target.pid, Some(signal)),
};
if let Err(errno) = kill_result {
// Q#PD1/Q#PD2: the failure describes itself; the
// 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));
}
if matches!(signal, Signal::SIGTERM | Signal::SIGKILL | Signal::SIGHUP) {
proc.state = ProcessState::Exiting {
pid,
@ -2131,6 +2278,290 @@ mod tests {
);
}
/// Spawn a PTY child that leads its own session and stays alive
/// until terminated, returning its id and OS pid.
///
/// `/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.
///
/// `drain_until` ticks, and a tick can observe a fast child's exit
/// and transition the record out of `Running` — after which
/// `signal` returns "is not running" and never reaches the
/// diagnostic at all. Any test whose child exits promptly must read
/// the pid this way. (Found by the parallel workspace sweep: the
/// drain-based helper raced only under load.)
fn record_pid(sup: &ProcessSupervisor, id: ProcessId) -> u32 {
match sup.processes.get(&id).expect("record").state {
ProcessState::Running { pid, .. } | ProcessState::Exiting { pid, .. } => pid,
ProcessState::Starting => panic!("spawn has not reported a pid yet"),
ProcessState::Terminated(_) => {
panic!("the record already left Running; the pid is unavailable")
}
}
}
/// Drain until `Started` and return the OS pid it carries. Safe
/// only for children that outlive the drain; see [`record_pid`].
fn spawn_started_pid(sup: &mut ProcessSupervisor, id: ProcessId) -> u32 {
let evs = drain_until(sup, id, Duration::from_secs(5), |evs| {
evs.iter()
.any(|e| matches!(e.kind, ProcessEventKind::Started { .. }))
});
evs.iter()
.find_map(|e| match e.kind {
ProcessEventKind::Started { pid } => Some(pid),
_ => None,
})
.expect("Started carries a pid")
}
/// Drive the production diagnostic until it observes the leader as
/// exited, bounded by `timeout`.
///
/// A fixed sleep is NOT proof of exit — on a loaded runner the child
/// can still be live, which would turn these tests into false
/// failures. This synchronises on the very observation under test.
/// Each failing attempt leaves the record untouched, because the
/// failure path returns before any bookkeeping (Q#PD2), so looping
/// is side-effect free.
fn terminate_until_leader_exited(
sup: &mut ProcessSupervisor,
id: ProcessId,
timeout: Duration,
) -> String {
let deadline = Instant::now() + timeout;
loop {
sup.force_next_kill_errno(nix::errno::Errno::EPERM);
let err = sup.terminate(id).expect_err("injected EPERM must fail");
if err.contains("leader=exited(") {
return err;
}
assert!(
!err.contains("is not running"),
"the record left Running before the diagnostic could run, so \
this test never exercised it: {err}"
);
assert!(
Instant::now() < deadline,
"leader never observed as exited within {timeout:?}: {err}"
);
std::thread::sleep(Duration::from_millis(10));
}
}
/// 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.
#[test]
fn a_group_directed_kill_failure_reports_target_and_leader_separately() {
let mut sup = ProcessSupervisor::new();
let (id, pid) = spawn_live_pty(&mut sup, "diag-group");
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)",
nix::errno::Errno::EPERM
);
assert_eq!(
err, expected,
"the report names the exact target the tty reported, the exact \
leader pid, and observes the leader as live"
);
let _ = sup.signal(id, Signal::SIGKILL);
}
/// 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.
#[test]
fn a_leader_directed_kill_failure_reports_the_fallback_branch() {
let mut sup = ProcessSupervisor::new();
let mut spec = ProcessSpec::new("diag-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::ESRCH);
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)",
nix::errno::Errno::ESRCH
);
assert_eq!(
err, expected,
"a non-group pipe child targets its own pid, and the group \
field is omitted where it has no meaning"
);
let _ = sup.signal(id, Signal::SIGKILL);
}
/// Q#PD1 acceptance 3 — every leader state renders distinctly. The
/// `Unobservable` and `NoRuntime` arms cannot be produced by a real
/// child on demand, so they are pinned directly; `live` and `exited`
/// are pinned through the real path by the tests around this one.
#[test]
fn every_leader_observation_renders_distinctly() {
assert_eq!(
LeaderObservation::Exited(TermStatus::Exited(0)).render(),
"exited(code 0)"
);
assert_eq!(
LeaderObservation::Exited(TermStatus::Signaled("SIGTERM".into())).render(),
"exited(signal SIGTERM)"
);
assert_eq!(LeaderObservation::Live.render(), "live");
assert_eq!(
LeaderObservation::Unobservable("try_wait: boom".into()).render(),
"unobservable(try_wait: boom)"
);
assert_eq!(LeaderObservation::NoRuntime.render(), "no-runtime");
}
/// Q#PD1 acceptance 3, exited arm through the REAL path — the leader
/// has genuinely exited and the report carries its exact code, not
/// merely "some exit".
#[test]
fn a_failure_after_the_child_exits_reports_the_leader_as_exited() {
let mut sup = ProcessSupervisor::new();
let mut spec = ProcessSpec::new("diag-exited", "/bin/sh");
spec.args = vec!["-c".into(), "exit 3".into()];
let id = sup.spawn(spec).expect("spawn");
// NOT `spawn_started_pid`: draining ticks, and this child exits
// immediately.
let pid = record_pid(&sup, id);
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))",
nix::errno::Errno::EPERM
);
assert_eq!(
err, expected,
"the exact exit code is observed from the real child, not \
inferred from the errno"
);
}
/// Q#PD2 acceptance 4 — **the disposition is unchanged.** An
/// injected failure still fails, and neither the state transition
/// nor the reap-ledger arming runs. This is the assertion that
/// separates a diagnostic from the tolerance rules three review
/// rounds rejected; flipping any arm to `Ok` fails it.
#[test]
fn an_injected_failure_changes_no_state_and_arms_no_ledger() {
let mut sup = ProcessSupervisor::new();
let mut spec = ProcessSpec::new("diag-disposition", "/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);
assert!(
sup.reap_ledger.is_empty(),
"precondition: nothing armed before the attempt"
);
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 group, leader_pid={pid}, expected_group=-{pid}, leader=live)",
nix::errno::Errno::EPERM
);
assert_eq!(err, expected, "a group=true pipe child reports via group");
assert!(
matches!(
sup.processes.get(&id).expect("record").state,
ProcessState::Running { .. }
),
"a failed kill must not transition the record to Exiting"
);
assert!(
sup.reap_ledger.is_empty(),
"a failed kill must not arm the reap ledger"
);
let _ = sup.signal(id, Signal::SIGKILL);
}
/// Q#PD3/Q#PD4 acceptance 5 — the diagnostic consults the REAL
/// `ChildHandle::try_wait` on the REAL child, which reaps it and
/// caches the status. `poll_one` must still emit exactly one exit
/// event, carrying the exact code.
///
/// A stubbed observation would bypass the double-`try_wait` path
/// entirely and pin nothing, so the injection replaces the kill
/// result only.
#[test]
fn observing_the_leader_does_not_consume_the_exit_event() {
let mut sup = ProcessSupervisor::new();
let mut spec = ProcessSpec::new("diag-one-event", "/bin/sh");
spec.args = vec!["-c".into(), "exit 7".into()];
spec.mode = ProcessMode::Pty {
rows: 24,
cols: 80,
mode: TerminalMode::Canonical,
};
let id = sup.spawn(spec).expect("spawn");
// NOT `spawn_started_pid`: draining ticks, and a tick can reap
// this immediately-exiting child before the diagnostic runs.
let _ = record_pid(&sup, id);
// Drives `observe_leader`, which try_waits the real PTY child
// for the first time and reaps it.
let err = terminate_until_leader_exited(&mut sup, id, Duration::from_secs(10));
assert!(
err.contains("leader=exited(code 7)"),
"the real handle was consulted and carries the exact code: {err}"
);
// The supervisor's own try_wait must still see that status.
let evs = drain_until(&mut sup, id, Duration::from_secs(5), has_exited);
let terminal: Vec<i32> = evs
.iter()
.filter_map(|e| match e.kind {
ProcessEventKind::Exited { code, .. } => Some(code),
ProcessEventKind::Signaled { .. } => Some(-1),
_ => None,
})
.collect();
assert_eq!(
terminal,
vec![7],
"exactly one terminal event survives the diagnostic's try_wait, \
carrying the child's real exit code"
);
}
#[test]
fn signal_terminates_a_running_child() {
let mut sup = ProcessSupervisor::new();