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

merge: integrate main @ a2a92bb; retire the merged signal lane 

`docs/active-work.md` auto-merged without conflict — this branch's lane
entry sits above the folding lane, clear of the six blocks #199 removed.
Verified rather than assumed: all lane headers checked afterwards.

The merge surfaced a stale statement on `main`. The process-signal lane
still read "PR #200 open, four review rounds closed, held for review",
which stopped being true when #200 merged as `a2a92bb`. Its arc is
complete — Stage A #176 and Stage B #200 both landed, and the items its
§5 parked become their own lanes — so under the rule #199 established
(a lane retires when its ARC is done, not when a PR merges) it is
removed and recorded under "Closed since the last snapshot".

Doing that here rather than deferring it: this PR already edits the
file, and leaving a known-false statement on `main` to keep a PR
single-purpose is the wrong trade. The alternative was letting it stand
until the reap-ledger lane, which is scoped next and inherits from the
very §5 list being retired.
This commit is contained in:
Levi Neuwirth 2026-07-30 15:49:32 -04:00
parent e1df0cff4d a2a92bb9bc
commit 55b4897c07
8 changed files with 1516 additions and 1334 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
.github/workflows/ci.yml vendored
View File

@ -218,12 +218,28 @@ 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
# 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: ${{ 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

1
Cargo.lock generated
View File

@ -2574,6 +2574,7 @@ dependencies = [
"codebook-tree-sitter-latex",
"crossbeam",
"crossterm",
"filedescriptor",
"loro",
"mlua",
"nix 0.29.0",

18
Cargo.toml
View File

@ -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

13
README.md
View File

@ -227,6 +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**, 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

1357
docs/active-work.md
View File

File diff suppressed because it is too large Load Diff

88
docs/agent-handoff.md
View File

@ -58,9 +58,18 @@ reads it the way you just did.
For volatile branches, checkpoints, verification, and recovery
commands, read `docs/active-work.md` immediately after this file.
## 1. Where the project stands (2026-07-28)
## 1. Where the project stands (2026-07-30)
- `main` @ `6c9e765` (the dired Stage 2 framing #171 and the resource-op
- **`main` @ `4cd4a7b`.** Eight PRs landed since the previous anchor, in
this order: the generated-buffer immutability **framing** #188, the
resource-op delete-guard **implementation** #190, silent-skip arming
#192/#193/#194, CI timeouts and concurrency #195, the process teardown
stdin-deadlock fix #197, dired Stage 2a #196, generated-buffer
immutability **Stage 1** #191, and bottom-panel **Stage 2B-3** #198.
Each has its own bullet below; this line is the head-of-`main` anchor
and nothing else.
- **Previous anchor, retained for provenance:** `6c9e765` (the dired
Stage 2 framing #171 and the resource-op
delete guard framing #186 — both framing-only, no runtime code, no
implementation started — atop
the docs-only coherence listview correction #189,
@ -223,6 +232,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
@ -766,6 +800,56 @@ commands, read `docs/active-work.md` immediately after this file.
`mark_document_stale` takes no `LspServerId` at all while creating
URI keys across three stores. A route purge keyed on request
responses cannot cover either.
- **Resource-op delete-guard implementation LANDED — #190**, atop the
framing #186 below. The pre-filesystem refusal now exists: a
synchronous `apply_resource_op` delete that would destroy unsaved work
is refused before the filesystem is touched, rather than after.
- **A refusal that no production path can reach passes every
direct-call test.** The guard is asserted through the outermost
user-reachable seam and falsified by revert, not by calling the
check directly. This is the general rule now recorded in §5.
- **Delete refusals must be visible.** Review round 2 found them
silent — the operation declined and the user learned nothing.
Reporting goes through `pmacs.editor.set_status`; `pmacs.error` is
still a channel defined only by a test stub (§5).
- **A URI-keyed store is not one store.** Purging a route keyed on
request responses covers neither `mark_document_stale` (which takes
no `LspServerId` while creating URI keys across three stores) nor
`DiagnosticView`, whose URI is fixed at construction.
- **dired Stage 2a LANDED — #196** (`docs/dired-stage2-framing.md`
rev 9, §5/§6/§10 — the substrate transaction only, **no dired
surface**). Rename and delete reconciliation across the path owners a
rename actually crosses.
- **A rename is a transaction across five owners**, and this is the
fact that forced the 2a/2b split: the buffer path, the buffer name,
the URI-keyed LSP stores plus `DiagnosticView`, dired's pathless
handles, and a captured Lua local that no transaction can reach.
Stage 2b owns everything needing new Rust primitives.
- **New LSP resource subscribers must not swallow reconciliation
failures**, and `forget_uri` must not leave purged requests live in
`LspClient.pending` — both were review findings, both now pinned.
- **Generated-buffer immutability Stage 1 LANDED — #191**, adopting the
contract framed in #188. `dired.lua`'s `paint` and `listview.lua`'s
`render` write through `pmacs.buffer.set_generated_contents`; zero
`bypass_intercept` writes remain in either file.
- **Why these two families first, and it is not the cheap half.**
`compile.lua` and `builtin/commands/default.lua` rebind all seven
undo chords to a no-op; `dired.lua` and `listview.lua` rebind
**nothing**, so a bare `C-/` emptied a listing and a panel. Stage 1
closes the only two families reachable without `M-x`.
- **The framing owns the acceptance contract; the implementation
adopts it.** Review round 5 found #191 had locally restated Stage 1
criteria while #188 still carried the originals. Where an
implementation finds a criterion impossible, the framing is revised
and re-approved first — it is not narrowed in place.
- **Stage 2 still owes everything with new Rust in it:**
`Buffer::apply_generated_edit`, the `{ generated = true }` option,
Q#GB10's path-backed refusal, Q#GB15's `identity_protected`,
Q#GB5's `ensure_slot` lock, and the remaining 13 write sites.
- **Generated-buffer immutability framing LANDED — #188**
(`docs/generated-buffer-immutability-framing.md`, revision 7; six
review rounds, thirty-two findings). Revision 7 is the governing
contract for the whole arc.
- **Resource-op delete guard framing LANDED (document only) — #186**
(`docs/resource-op-delete-guard-framing.md`, revision 5; five review
rounds). **Approved as a framing; no runtime code.** It owns the

563
docs/process-signal-diagnostic-completeness-framing.md Normal file
View File

@ -0,0 +1,563 @@
# Framing — make the signal diagnostic discriminating (evidence collection)
**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).
**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
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 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.
- **`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.
- **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.
- **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
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 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
signals are all disposition-identical.
- **Bet 5 proposed a test that already exists** (§1.9). Cited as ground
truth now, not invented.
**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, no
behavioural change.**
- **Serves §9 (worker model), failure attribution.** Stage A made the
failure describe itself; this lane makes the description
*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**; under §25 no COHERENCE
edit rides this PR.
## 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`). §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. **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`) — **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 What the occurrence actually invalidates
- **`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
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 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
`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` 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.
### 1.5 A numeric pgid cannot establish identity
The value is read before `kill`, the window remains open, 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.
**This lane therefore records and does not retarget.** No acceptance
claims the telemetry 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.
### 1.5a The nix surface is gated, and available for a non-obvious reason
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; 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 —
`spec.group` is rejected at spawn for PTY mode — and returns
`TargetSource::LeaderPid`, rendered "leader-pid" (`:738`). **A normal
pipe child renders identically.** Two situations, one string.
`portable-pty` (`portable-pty-0.9.0/src/unix.rs:374`) discards the errno:
```rust
fn process_group_leader(&self) -> Option<libc::pid_t> {
match unsafe { libc::tcgetpgrp(self.fd.0.as_raw_fd()) } {
pid if pid > 0 => Some(pid),
_ => None,
}
}
```
`nix::unistd::tcgetpgrp` requires `F: AsFd`, while
`MasterPty` exposes only `fn as_raw_fd(&self) -> Option<RawFd>`
(`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:
- `filedescriptor::OwnedHandle::dup<F: AsRawFileDescriptor>(&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
`signal_failure_report` (`:830-850`) takes target, leader pid, errno and
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.
**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, 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 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. |
**Rev 2 called the first row "recoverable" and claimed "SIGKILL
escalation still happens". Unsupported.** `tick_reap_ledger`
(`:1249-1254`):
```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
}
```
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
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.
## 2. Questions
- **Q#DC1** — Can the two entities be made to diverge in a test? *Yes:
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. 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`
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
- **Bet 1 — the divergence is constructible.** A PTY fixture where the
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.
- **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`
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.
- **Bet 3 — naming the signal is free.** Thread `signal` into the report.
- *Falsified if* any exact-string test cannot be updated mechanically.
- **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. 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
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
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, with
a test asserting a case where they **differ** (§3 Bet 4). **It is
sampled before the `kill`**, not during report construction, so it
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.
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, 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.
## 5. Parked
- **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.
- **§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'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.
## 6. Gates
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);
`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 unrelated reasons.
**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. 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.

794
src/process.rs
View File

@ -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<nix::errno::Errno>,
/// Test seam for the PTY foreground-group lookup (see
/// `force_next_pty_lookup`). Always `None` outside tests.
forced_pty_lookup: Option<Result<i32, PtyLookupFailure>>,
}
/// One armed group in the reap ledger.
@ -714,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
@ -726,16 +737,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 +798,85 @@ impl TargetSource {
}
}
/// A lifetime-tied view of a `MasterPty`'s file descriptor.
///
/// `MasterPty` exposes only `Option<RawFd>`, 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::<std::io::Error>()
&& 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<i32, PtyLookupFailure> {
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 +886,36 @@ struct SignalTarget {
source: TargetSource,
}
fn signal_target(proc: &ManagedProcess, pid: u32) -> Result<SignalTarget, String> {
fn signal_target(
proc: &ManagedProcess,
pid: u32,
forced_lookup: Option<Result<i32, PtyLookupFailure>>,
) -> Result<SignalTarget, String> {
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(outcome) => outcome,
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,14 +974,48 @@ 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,
measured: Option<&str>,
) -> String {
let expected = if target.source.is_group() {
match i32::try_from(leader_pid) {
@ -841,10 +1025,15 @@ fn signal_failure_report(
} 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} (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 +1139,7 @@ impl ProcessSupervisor {
reap_ledger: HashMap::new(),
group_term_grace: GROUP_TERM_GRACE,
forced_kill_errno: None,
forced_pty_lookup: None,
}
}
@ -963,6 +1153,31 @@ 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.
///
/// 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(&mut self, outcome: Result<i32, PtyLookupFailure>) {
self.forced_pty_lookup = Some(outcome);
}
/// Override the SIGTERM-to-SIGKILL grace window. Test helper.
pub fn set_grace_period(&mut self, d: Duration) {
self.grace_period = d;
@ -1080,7 +1295,15 @@ 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)?;
// 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
@ -1094,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, 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 {
@ -1243,9 +1473,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;
}
@ -2304,18 +2549,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.
///
@ -2384,39 +2617,492 @@ 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.
/// 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";
/// 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
/// **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<i32> {
let runtime = sup.processes.get(&id)?.runtime.as_ref()?;
match &runtime.child {
ChildHandle::Pty {
_master: master, ..
} => pty_foreground_group(master.as_ref()).ok(),
ChildHandle::Pipes(_) => None,
}
}
/// Fixture for Q#DC1: a PTY child whose terminal foreground group is
/// genuinely **not** the spawned leader.
///
/// 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.
/// `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<i32> = 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 facts **that are not the same fact
/// repeated**.
///
/// 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.
///
/// **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() {
let mut sup = ProcessSupervisor::new();
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 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");
let expected = format!(
"kill: {} (target=-{pid} 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!(
err, expected,
"the report names the exact target the tty reported, the exact \
"the report names the exact group the lookup returned, the exact \
leader pid, and observes the leader as live"
);
// 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 group the lookup returned: {err}"
);
assert!(
!err.contains(&format!("target=-{pid} via tcgetpgrp")),
"the target must NOT be the leader pid: {err}"
);
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"
);
// 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!(
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);
}
/// 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(Err(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}"), "<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.
@ -2432,7 +3118,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!(
@ -2482,7 +3168,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!(
@ -2514,7 +3200,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");