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

test(process): make the group-directed acceptance discriminating (Bet 1) 

Framing §3 Bet 1 and acceptance 1. Committed alone, per the branch plan:
this bet decides whether the diagnostic is worth extending at all, so its
result belongs in history before anything depends on it.

The previous version of
`a_group_directed_kill_failure_reports_target_and_leader_separately`
spawned `/bin/sleep` on a PTY and asserted the same pid three times,
conceding in its own doc comment that the values "are asserted to agree
only because nothing has moved the terminal". The entire premise of the
diagnostic is that the terminal's foreground group and the spawned leader
are different entities, and no test exercised a case where they were.

The fixture now drives job control: `bash -m` runs a foreground job in a
fresh process group and hands it the terminal, so `tcgetpgrp` reports a
group that is not the leader. The trailing `; :` is load-bearing — with a
single simple command `bash -c` execs in place, which would leave the
leader owning the terminal and silently restore the agreement.

The bounded wait is also load-bearing rather than defensive. A probe of
this fixture observed the foreground group as the leader FIRST and only
then as the job's group, so measuring immediately would pin the
non-divergent case. The fixture additionally asserts the diverged group
still holds a live member, so a divergent number cannot come from a dead
group.

Falsified in both directions by substituting `leader_pid` for the
`tcgetpgrp` result in `signal_target`:

- the new test FAILS — target=-1020100 (leader) against the expected
  -1020103 (foreground group);
- the OLD test, restored verbatim alongside the same mutation, PASSES.

That pair is the finding: the previous acceptance pinned the
substitution as acceptable.

`/bin/bash` is declared as an optional test dependency and armed with
PMACS_REQUIRE_BASH on BOTH CI platforms, not only Linux — the signal
failures this diagnostic exists to explain have so far occurred only on
macOS. The guard tests the exact path the fixture spawns rather than
`which bash`, because a system with bash on PATH but not at `/bin/bash`
would pass a `which` guard and then fail the spawn.

Verified: both arming arms exercised against an absent path (unarmed
skips, armed fails by name); 20/20 repetitions of the test; `cargo fmt
--check`, `cargo clippy --workspace --all-targets -- -D warnings`, and
`cargo test --lib` (1877 passed) all clean; ci.yml parses.

No production behaviour changes.
This commit is contained in:
Levi Neuwirth 2026-07-30 12:49:47 -04:00
parent df50880762
commit 0d2ad49969
3 changed files with 168 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -218,12 +218,24 @@ jobs:
# binary is absent (a minimal container must not fail `--lib`
# without ever testing pmacs) and this variable is what makes the
# skip fatal where the tool is guaranteed.
#
# PMACS_REQUIRE_BASH arms the signal diagnostic's job-control
# divergence fixture, which needs `/bin/bash` specifically: it
# relies on `-m` putting a foreground job in its own process group
# and handing it the terminal, so that `tcgetpgrp` differs from the
# spawned leader. Without that divergence the group-directed
# acceptance cannot tell a real foreground-group read from a
# `leader_pid` substitution. Armed on BOTH platforms, unlike the
# variables above — macOS ships `/bin/bash`, and the macOS legs are
# exactly where the signal failures this diagnostic exists for have
# actually occurred.
- run: cargo test --all-targets --no-default-features --features ${{ matrix.lua }} -- --test-threads=1
env:
PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }}
PMACS_REQUIRE_SHELLS: ${{ runner.os == 'Linux' && '1' || '' }}
PMACS_REQUIRE_LUA: ${{ runner.os == 'Linux' && '1' || '' }}
PMACS_REQUIRE_SETSID: ${{ runner.os == 'Linux' && '1' || '' }}
PMACS_REQUIRE_BASH: '1'
- run: cargo test --doc --no-default-features --features ${{ matrix.lua }}
# The workspace default member is only the root `pmacs` package, so
# the runs above never execute pmacs-protocol's own tests — the

9
README.md
View File

@ -227,6 +227,15 @@ translation) are routed through trampolines that exec these tools.
**skips** when `setsid` is absent, so a minimal or BusyBox environment
still runs `cargo test --lib`; set `PMACS_REQUIRE_SETSID=1` to make
that skip a failure, as CI does on Linux.
- **`/bin/bash`** (**optional**, but armed on every CI platform). The
signal diagnostic's group-directed acceptance needs a terminal whose
foreground process group is *not* the spawned leader, and `bash -m`
produces exactly that by running a foreground job in its own process
group. The path matters: the test spawns `/bin/bash` directly rather
than resolving `bash` on `PATH`, and skips when that path is absent.
Set `PMACS_REQUIRE_BASH=1` to make the skip a failure, as CI does on
Linux **and** macOS — the failures this diagnostic exists to explain
have so far only appeared on macOS.
- **`git`** (added in M7.2). Required for any package operation:
the package fetcher shells out to `git` to clone, fetch, and
resolve refs, with a deterministic environment

162
src/process.rs
View File

@ -2304,18 +2304,6 @@ mod tests {
/// `/bin/sleep` directly rather than through a shell: a shell may
/// place the command in a different foreground process group, and
/// these tests assert the exact target the tty reports.
fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> (ProcessId, u32) {
let mut spec = ProcessSpec::new(name, "/bin/sleep");
spec.args = vec!["30".into()];
spec.mode = ProcessMode::Pty {
rows: 24,
cols: 80,
mode: TerminalMode::Canonical,
};
let id = sup.spawn(spec).expect("spawn");
(id, spawn_started_pid(sup, id))
}
/// The OS pid straight from the supervisor's own record, WITHOUT
/// ticking.
///
@ -2396,24 +2384,168 @@ mod tests {
/// designs for this code collapsed the two; the report keeps them
/// apart, and here they are asserted to agree only because nothing
/// has moved the terminal.
/// The job-control shell the divergence fixture drives. Named once so
/// the availability guard and the spawn cannot drift apart.
const BASH: &str = "/bin/bash";
/// The tty's current foreground process group, read through the same
/// `MasterPty` accessor production uses. `None` for a pipe
/// generation, or when the terminal reports no foreground group.
fn foreground_pgid(sup: &ProcessSupervisor, id: ProcessId) -> Option<i32> {
let runtime = sup.processes.get(&id)?.runtime.as_ref()?;
match &runtime.child {
ChildHandle::Pty {
_master: master, ..
} => master.process_group_leader(),
ChildHandle::Pipes(_) => None,
}
}
/// Fixture for Q#DC1: a PTY child whose terminal foreground group is
/// genuinely **not** the spawned leader.
///
/// `bash -m` enables job control, so it runs the script's command in
/// a fresh process group and hands that group the terminal. The
/// trailing `; :` matters — with a single simple command `bash -c`
/// execs in place, which would leave the leader owning the terminal
/// and silently restore the very agreement this fixture exists to
/// break.
///
/// **The wait is load-bearing, not defensive.** The handoff is not
/// instantaneous: a probe of this exact fixture observed the
/// foreground group as the leader first and only then as the job's
/// group. Measuring immediately would pin the non-divergent case and
/// the test would assert the opposite of its purpose.
///
/// Returns `(id, leader_pid, foreground_pgid)` with the two pids
/// known to differ and the foreground group known to hold a live
/// member.
fn spawn_pty_with_diverged_foreground_group(
sup: &mut ProcessSupervisor,
name: &str,
) -> (ProcessId, u32, i32) {
let mut spec = ProcessSpec::new(name, BASH);
spec.args = vec![
"--noprofile".into(),
"--norc".into(),
"-m".into(),
"-c".into(),
"sleep 30; :".into(),
];
spec.mode = ProcessMode::Pty {
rows: 24,
cols: 80,
mode: TerminalMode::Canonical,
};
let id = sup.spawn(spec).expect("spawn");
let leader = spawn_started_pid(sup, id);
let leader_i32 = i32::try_from(leader).expect("pid fits i32");
let deadline = Instant::now() + Duration::from_secs(10);
let mut observed: Vec<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 five separate facts **that are not the same
/// fact repeated**.
///
/// The pre-Stage-B version of this test spawned `/bin/sleep` on a PTY
/// and asserted the same pid three times, conceding in its own
/// comment that the values "are asserted to agree only because
/// nothing has moved the terminal". An implementation that ignored
/// `tcgetpgrp` entirely and substituted `leader_pid` passed it. Since
/// the whole premise of the diagnostic is that these two entities can
/// diverge, that test pinned the substitution as acceptable.
///
/// This version drives job control so the terminal genuinely belongs
/// to a different process group, and asserts the two values **differ**
/// as well as asserting each exactly.
#[test]
fn a_group_directed_kill_failure_reports_target_and_leader_separately() {
// Guard on the exact path the fixture spawns, not on `which bash`.
// The two can disagree — a system with bash on PATH but not at
// `/bin/bash` would pass a `which` guard and then fail the spawn,
// which is the guard failing to guard the thing it names.
if !std::path::Path::new(BASH).exists() {
let armed = std::env::var_os("PMACS_REQUIRE_BASH").is_some_and(|v| !v.is_empty());
assert!(
!armed,
"PMACS_REQUIRE_BASH is set but {BASH} does not exist: the \
job-control divergence fixture cannot run"
);
eprintln!(
"{BASH} not present; skipping \
a_group_directed_kill_failure_reports_target_and_leader_separately"
);
return;
}
let mut sup = ProcessSupervisor::new();
let (id, pid) = spawn_live_pty(&mut sup, "diag-group");
let (id, pid, fg) = spawn_pty_with_diverged_foreground_group(&mut sup, "diag-group");
let leader_i32 = i32::try_from(pid).expect("pid fits i32");
assert_ne!(
fg, leader_i32,
"the fixture must make the foreground group differ from the leader; \
equal values would let a leader_pid substitution pass"
);
sup.force_next_kill_errno(nix::errno::Errno::EPERM);
let err = sup.terminate(id).expect_err("injected EPERM must fail");
let expected = format!(
"kill: {} (target=-{pid} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)",
"kill: {} (target=-{fg} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)",
nix::errno::Errno::EPERM
);
assert_eq!(
err, expected,
"the report names the exact target the tty reported, the exact \
"the report names the exact group the tty reported, the exact \
leader pid, and observes the leader as live"
);
// The target came from the terminal, not from the leader. Stated
// as its own assertion so a regression that reintroduces the
// substitution fails here by name rather than inside a long
// string comparison.
assert!(
err.contains(&format!("target=-{fg} via tcgetpgrp")),
"the target must be the terminal's foreground group: {err}"
);
assert!(
!err.contains(&format!("target=-{pid} via tcgetpgrp")),
"the target must NOT be the leader pid: {err}"
);
let _ = sup.signal(id, Signal::SIGKILL);
}