Review round 5: one blocking, one major. Both documentation-only; the
runtime fixes from round 4 are unchanged.
**The pre-kill sample was overclaimed.** Acceptance 4 said the measured
group "describes the target that was attempted". It does not. `getpgid`
and `kill` remain separated by the same read-then-act window §1.5 is
built on, so the sample can be stale by the time the signal is
delivered. Moving it earlier removes a POST-HOC reading; it does not
make the reading contemporaneous. The criterion now says it records
pre-kill evidence about the attempted target, and adds an explicit
sentence forbidding any acceptance from claiming otherwise.
This is worth naming rather than quietly editing: the framing's whole
spine is that this telemetry establishes less than it appears to, and
round 4 fixed a real ordering defect and then inflated the fix's meaning
inside the document that governs it.
**`95897f7`'s commit message carries the same overclaim.** It is not
amended — force-pushing a branch under review to rewrite history would
hide the error rather than correct it. This message is the correction,
and the two read in sequence.
**A dead doc comment was documenting a constant.** The original
pre-Stage-B acceptance-1 block survived the rewrite and came to rest
above `const BASH`, so the file explained a string constant with the
sentence this entire lane exists to remove — "here they are asserted to
agree only because nothing has moved the terminal", describing a test
that no longer exists. Removed; only the BASH explanation remains.
Verified the overclaim is gone from the whole tree, not only the cited
line: no hits in `docs/` or `src/`.
Gates: 11 gates, 4,471 tests, zero failures.
Review round 4: three blocking, one major. All accepted.
**`measured_group` was sampled after the failure.** It ran inside
`signal_failure_report`, after the `kill` and after `observe_leader`,
while the framing and the function's own doc both said before. A
concurrent group change would have made the diagnostic report
post-failure state as evidence about the attempted target. It is now
sampled in `signal` before the kill and passed into the report, so the
field describes the target that was attempted rather than the state the
failure left behind.
**The Linux corroboration did not exercise the production lookup.** Its
helper read `portable_pty::process_group_leader` — the accessor this
lane stopped using on the signal path — so `pty_foreground_group` could
have fallen back on every call with every test still green. Forcing it
to always fall back demonstrates the hole exactly: the corroboration
fails, and the injected pin PASSES, because the injected tests supply
the group themselves and structurally cannot detect a broken lookup.
The helper now calls the production lookup, and the corroboration forces
only the kill so the report is built from a real terminal read. The
residual limitation is recorded rather than left to the green: on macOS
`pty_foreground_group` has no end-to-end coverage, because the platform
cannot produce the precondition.
**The framing did not update its own acceptance contract.** Revision 5
recorded Bet 1's falsification in the revision history and in the bet,
and left the normative criterion demanding the real-shell rewrite — the
"implementation quietly diverges from the contract" shape this project
already recorded as a lesson on #191/#188. Acceptance 1 now describes
the injected pin, 1a adds the corroboration and its macOS limitation,
and 4 states the sampling order. The ledger is synchronized: revision 6,
four commits, 4,471 tests, bash armed on Linux only.
**`TargetSource`'s doc had the wrong classification.** Two of the four
variants target the leader pid, not one, and the pid-versus-group split
does not line up with PTY-versus-pipe — which is why the fallback needed
its own variant rather than reusing `LeaderPid`.
Gates: 11 gates, 4,471 tests, zero failures.
CI falsified framing Bet 1. Both macOS legs reported
job control never moved the terminal off the leader
(leader=8542, foreground groups observed: [8542])
with the terminal staying on the leader for the entire 10s bounded wait.
Linux diverges reliably — 20/20 locally and green on both ubuntu legs —
so this is a platform difference rather than a flake, and rerunning past
it would have been wrong.
The framing named this outcome and prescribed the response, so that is
what ships rather than an improvised fix:
- The divergent case is pinned by INJECTING the foreground group at the
`signal_target` seam. Deterministic, runs on every platform. The
injection seam widens from failure-only to either outcome; the branch,
target choice, leader observation against the real child, and report
construction all remain production code.
- Verified still discriminating: the `leader_pid`-substitution mutation
fails it, `target=-1707909` against an expected `-1707910`. That was
the whole point of the original rewrite and it survives the fallback.
- The real shell is retained as corroboration in
`job_control_really_diverges_the_foreground_group`, Linux-only. It
skips on macOS by PLATFORM CHECK rather than by arming: the
precondition genuinely does not hold there, so running it would assert
a false claim about macOS instead of finding a bug.
- Framing revision 5 records the falsification and states exactly how
the injected pin is weaker — it proves the target is read from the
lookup rather than substituted from the leader; it does not by itself
prove any real shell produces that divergence.
`PMACS_REQUIRE_BASH` moves to Linux-only. The earlier reasoning for
arming both platforms — macOS is where the failures happen, so Linux-only
leaves it dark where it matters — was right about the diagnostic and
wrong about this test, which cannot produce its precondition on macOS at
all. Arming it there made a missing binary fatal for a test that can
never run. The measurement is recorded in ci.yml and the README so it is
not re-derived.
Gates on this tree: 11 gates, 4471 tests, zero failures.
Integrate canonical main, restore PTY errno capture through the safe
filedescriptor duplication bridge, narrow the macOS occurrence to what
it actually measured, and make the job-control fixture foreground and
discriminating.
Record the portable framing lane and exact recovery commands in the
active-work ledger.
Three blocking and three major findings, all accepted, all verified in
the code before acceptance.
- The PTY errno proposal is withdrawn. `nix::unistd::tcgetpgrp` needs
`F: AsFd`; `MasterPty` exposes only `Option<RawFd>`; every route from
a raw fd to `AsFd` is unsafe and the crate forbids unsafe. The claim is
reduced to distinguishing the fallback without the errno.
- Revision 2 called `getpgid` ungated. It is not: the gate is a
block-level `feature!` opened 168 lines above the function, and rev 2
read the four lines immediately above it. Both calls are available
because nix's own `signal` feature depends on `process`, verified with
`cargo tree` and by compiling them. The lane now declares `process`
explicitly rather than inheriting it.
- Bet 4's `setsid` fixture was impossible: a `spec.group` child is
already a process-group leader and a group leader's `setsid` fails
with EPERM. Replaced with a `setpgid`-into-an-existing-group fixture
with a readiness handshake and explicit cleanup.
- "Recoverable" is withdrawn. `tick_reap_ledger` drops its entry on any
probe error — including the EPERM this occurrence proves reachable —
and discards the SIGKILL result while marking the entry killed. The
claim is now that escalation remains scheduled, and that still-silent
path is parked as its own lane.
- Revision 2 falsified the wrong sentence. Stage A's disjointness claim
was about the PTY path and remains true; what the occurrence falsifies
is the ledger's own EPERM comment.
- Failed SIGUSR1 and SIGTERM are disposition-identical, because every
failed kill returns before the fatal-signal branch. Acceptance 3 now
separates the failure-format comparison from a successful-call
disposition control.
- Bet 5 proposed a test that already exists at src/process.rs:2501. It is
cited as ground truth and retained, not invented.
No code changes. Framing only, awaiting review round 3.
Stage B of the lane that merged as PR #176. Stage A parked every
tolerance rule behind one condition — evidence — and the macOS failure on
PR #191 is the first occurrence carrying Stage A's self-describing
format, so the evidence now exists.
It does not support any parked rule, and per review round 1 it does not
support an identity claim either. This lane is evidence collection only:
no tolerance rule, no retargeting, no disposition change.
Round 1 raised two blocking and two major findings. All four are
accepted, and all four were verified against the code first:
- the landed acceptance at src/process.rs:2400 asserts the same pid three
times, so substituting leader_pid for tcgetpgrp would pass it;
- a numeric pgid cannot establish group identity, and no portable
mechanism can (pidfd covers processes, not groups; macOS has none);
- the PTY foreground-lookup fallback renders identically to a normal pipe
child, and portable-pty discards the tcgetpgrp errno before pmacs sees
it;
- the report never names the signal, though disposition differs between
fatal and non-fatal ones.
Revision 1 was written to a session scratchpad rather than a branch, so
it was not on githubsucks and review necessarily landed on Stage A's
merged document instead. That is recorded in the revision history: work
is portable only after it is committed and pushed, and this lane broke
that rule on its first step.
No code changes. Framing only, awaiting review round 2.
Levi Neuwirth