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

fix(test): retire R2 and R4 — two readiness predicates weaker than their assertions 

Stage 2 of `docs/macos-ci-signal-integrity-framing.md` revision 3
(acceptance 6–9). Two test races, both the same shape: the thing waited
for was weaker than the thing asserted, so the wait could return inside
the window the assertion needs closed.

R4 — `wait_for_file` returned as soon as `fs::read` succeeded, which
succeeds on a ZERO-BYTE FILE. The probe publishes with
`open(path,'wb').write(b'1')` and `open()` creates the file before
`write()` fills it, so the helper handed `[]` to a caller asserting
`== b"1"`. It now takes the expected bytes and waits while the file
holds a STRICT PREFIX of them — the states a write in flight can be
observed in — returning anything else immediately so the caller's
`assert_eq!` stays the discriminating assertion rather than becoming a
timeout inside a helper that does not know what was expected.

All four callers pass their expectation. `wait_for_published_file`, one
function away in the same suite, gated the real-TUI smoke's
`assert_eq!(…, b"1")` on the identical predicate and is fixed with it:
leaving it would have let R4 recur under a different selector, which the
registry would then have had to judge a new incident.

R2 — the USR1 fixture waited on `ProcessEventKind::Started`, emitted at
SPAWN, not when `/bin/sh` has parsed `trap '' USR1`. SIGUSR1's default
disposition is terminate, so a signal inside that window kills the
child. The child now publishes a marker AFTER the trap and the test
waits for that marker's CONTENT (the same zero-byte trap applies to a
shell's `>` redirection). `exec` replaces the forked `sleep`, so the
group holds exactly one process and the ignored disposition survives by
POSIX rather than by the shell's fork-suppression optimization — an
unstated dependency the old fixture had, since these signals are
group-directed and a forked `sleep` is an untrapped group member.

Four witnesses, each verified by REVERTING the fix and observing the
failure rather than by reasoning about it:

- `wait_for_file_does_not_return_a_zero_byte_readiness_file` fails
  `left: []`, `right: [49]` — R4's two required fragments, verbatim;
- `wait_for_file_does_not_return_a_partial_write` fails on the torn read
  a length check alone would admit;
- `wait_for_file_returns_divergent_content_rather_than_timing_out` fails
  against an over-strict helper that waits for an exact match;
- `usr1_readiness_waits_for_the_trap_not_for_the_spawn` fails
  `left: Some("SIGUSR1")`, `right: Some("SIGTERM")` with the readiness
  wait removed. Its fixture sleeps before `trap` so the pre-trap window
  is deliberate rather than load-dependent, and it proves survival by
  the child's EXIT DISPOSITION rather than by an absence observed within
  a window.

R1 is NOT touched — referred to the async-runtime lane (Q#MCI3), because
widening its budget would make it pass and measure nothing more. R3 is
NOT touched and remains UNRESOLVED, owned by the process-signal /
reap-ledger lanes.

`docs/ci-red-signatures.md` moves R2 and R4 to a "Retired rows" section
with their dispositions and adds the rule the file needed and lacked: a
red matching a retired row is a RECURRENCE that puts the retirement in
question, never a known flake. `docs/active-work.md` carries this lane
from its first commit rather than after review asks for it.

Repetition sets, not single runs: the two `--lib` process tests 15/15,
the whole `vterm_stage2_acceptance` suite 15/15 at default parallelism.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Levi Neuwirth 2026-08-05 13:41:05 +02:00
parent 12f2970ad8
commit bd532dde6b
No known key found for this signature in database
5 changed files with 432 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

131
docs/active-work.md
View File

@ -17,6 +17,13 @@ branch and no owner since #166. It is implemented on
(#215, in review), which this file required a lane for and did not have (#215, in review), which this file required a lane for and did not have
until review caught it — the #171 defect recurring. Its block is below. until review caught it — the #171 defect recurring. Its block is below.
**Updated later the same day.** #215 **merged** (`main` @ `12f2970`) and
that lane is **rewritten, not removed** — rule 4 removes a lane when its
ARC is done, and Stage 2 is the arc. Stage 2 (**hardening**) is now in
flight on `ci-signal-hardening`, with its own lane block, its own
checkpoint table, and a lane written **before** the PR was opened rather
than after review asked for it.
**Updated 2026-08-04.** Four PRs landed since: the CI CRDT coverage **Updated 2026-08-04.** Four PRs landed since: the CI CRDT coverage
lane #209, Distribution Stage 1 #211 (released as **v1.1.0**), the lane #209, Distribution Stage 1 #211 (released as **v1.1.0**), the
post-release accuracy pass #212, and **bottom-panel Stage 3 #213 — post-release accuracy pass #212, and **bottom-panel Stage 3 #213 —
@ -93,8 +100,9 @@ lesson, §1 for the two framings).
are identical on every machine. Remote names are otherwise are identical on every machine. Remote names are otherwise
machine-local: `origin` may name this canonical URL, a release mirror, machine-local: `origin` may name this canonical URL, a release mirror,
or something else, and therefore has no authority by name alone. or something else, and therefore has no authority by name alone.
- Canonical base at this snapshot: **`githubsucks/main` @ `f186253`** - Canonical base at this snapshot: **`githubsucks/main` @ `12f2970`** —
— bottom-panel Stage 3 **#213**, which completes Arc 7, atop the the macOS CI signal-integrity registry **#215**, atop `f186253`:
bottom-panel Stage 3 **#213**, which completes Arc 7, atop the
post-release accuracy pass **#212**, Distribution Stage 1 **#211** post-release accuracy pass **#212**, Distribution Stage 1 **#211**
(released as **v1.1.0**, the first release with prebuilt binaries), (released as **v1.1.0**, the first release with prebuilt binaries),
and the CI CRDT coverage lane **#209**. Beneath those, `cfc1710`: and the CI CRDT coverage lane **#209**. Beneath those, `cfc1710`:
@ -106,7 +114,7 @@ lesson, §1 for the two framings).
`Hello` still advertises v20** — two different facts, and #184 landed `Hello` still advertises v20** — two different facts, and #184 landed
only the first. only the first.
**The recovery floor advances with the base**, so the check below **The recovery floor advances with the base**, so the check below
now requires `f186253` or newer; a tree at `cfc1710` no longer now requires `12f2970` or newer; a tree at `f186253` no longer
passes. That is deliberate — a check accepting an older commit than passes. That is deliberate — a check accepting an older commit than
the declared base passes on a tree the rest of this file does not the declared base passes on a tree the rest of this file does not
describe. describe.
@ -145,7 +153,7 @@ git worktree list
git status --short --branch git status --short --branch
``` ```
The `git log` command must expose `f186253` — the base named above — or a The `git log` command must expose `12f2970` — the base named above — or a
newer intentional main. Keep this threshold and the canonical-base line in newer intentional main. Keep this threshold and the canonical-base line in
step: a recovery check that accepts an older commit than the base it step: a recovery check that accepts an older commit than the base it
declares canonical will pass on a tree the rest of this file does not declares canonical will pass on a tree the rest of this file does not
@ -161,31 +169,92 @@ form. All four steps ran clean. **The two-argument form still does not
work** for a remote-only branch (`fatal: invalid reference`), which is work** for a remote-only branch (`fatal: invalid reference`), which is
why every lane below spells out the `-b` form. why every lane below spells out the `-b` form.
## macOS CI signal integrity — STAGE 1 IN REVIEW, PR #215 ## macOS CI signal integrity — STAGE 1 MERGED (#215); STAGE 2 IN REVIEW
**This file requires a lane for every open PR** (see the #171/#174 note **This file requires a lane for every open PR** (see the #171/#174 note
above: an open PR is exactly the volatile work this file records, and above: an open PR is exactly the volatile work this file records, and
#171 drifted 153 commits while invisible here). #215 had none until this #171 drifted 153 commits while invisible here). #215 had none until
entry — the same defect, caught in review. review caught it; **Stage 2's lane is written with its first commit**,
which is where the same defect stops recurring.
- **Branch `macos-ci-signal-integrity`**, base `githubsucks/main` @ Framing `docs/macos-ci-signal-integrity-framing.md` **revision 3**,
`bfb97c6`. Framing `docs/macos-ci-signal-integrity-framing.md` already approved. Stage 1 is acceptance 1–5; Stage 2 is acceptance 6–9.
**revision 3**.
- **PR: <https://github.com/levineuwirth/pmacs/pull/215>** — Stage 1, ### Stage 2 — hardening, IN REVIEW
docs only. **Checkpoints, newest last** — a lane that records only the
*reviewed* head goes stale the moment a review round is pushed, which - **Branch `ci-signal-hardening`**, worktree `../pmacs-ci-signals`, base
is how #171 became invisible: `githubsucks/main` @ `12f2970` (the #215 merge). Opened from an
**isolated worktree** because the shared checkout was on another
lane's branch with clean-but-foreign state; never switch it.
- **PR: opened from this branch; the number and its first CI checkpoint
land in the follow-up commit** — this block is written *with* the
work, before the PR exists, so the row below is filled in rather than
invented. **Checkpoints, newest last**, because a lane that records
only one head goes stale on the next push:
| head | CI run | result |
|---|---|---|
| *(opening head)* | *(pending — filled in once the run exists)* | — |
**The branch tip is authoritative over any row here.** Verify with
`git rev-parse githubsucks/ci-signal-hardening` rather than trusting
the newest line.
- **What it ships — exactly two fixes, both at the readiness
predicate**, plus four witnesses:
- **R4.** `wait_for_file` takes the **expected bytes** and waits while
the file holds a *strict prefix* of them, so a zero-byte or torn
read is no longer a readiness signal. Four callers updated; **the
identical mechanism in `wait_for_published_file`**, one function
away in the same suite, is fixed with it — leaving it would have let
R4 recur under a different selector.
- **R2.** The USR1 fixture publishes a marker **after** `trap '' USR1`
and the test waits for that marker's **content**; `exec` replaces
the forked `sleep`, so the group holds one process and the ignored
disposition survives by POSIX rather than by a shell's
fork-suppression optimization.
- **The witnesses were verified by REVERTING each fix**, not by
reasoning. With the old predicate restored,
`wait_for_file_does_not_return_a_zero_byte_readiness_file` fails
`left: []`, `right: [49]` — **R4's two required fragments, verbatim**
— and with the readiness wait removed,
`usr1_readiness_waits_for_the_trap_not_for_the_spawn` fails
`left: Some("SIGUSR1")`, `right: Some("SIGTERM")`.
- **R1 is NOT touched** — referred to the async-runtime lane (Q#MCI3);
widening its budget would make it pass and measure nothing more.
**R3 is NOT touched** and stays **unresolved**; it belongs to the
process-signal / reap-ledger lanes and nothing here may make it look
otherwise.
- **Repetition sets, not single runs:** the two `--lib` process tests
**15/15**, the whole `vterm_stage2_acceptance` suite (9 tests, default
parallelism) **15/15**.
- **Verification:** fmt, diff-check, clippy with and without `crdt`,
`--lib` 1897, `--lib --features crdt` 2082, vterm Stage 2 9, m4 149,
required GPU 221, and the **full serialized sweep in BOTH feature
configurations against a `main` baseline** — the blindness the handoff
§5 warns about, which is why the baseline is taken first. Each `--lib`
figure is exactly one above #215's (1896 / 2081): the R2 witness.
Recovery from a clean checkout:
```sh
git fetch githubsucks
git worktree add ../pmacs-ci-signals \
-b ci-signal-hardening \
githubsucks/ci-signal-hardening
```
### Stage 1 — MERGED as #215
- **Branch `macos-ci-signal-integrity`**, merged to `main` @ `12f2970`
after two review rounds. Docs only. Final CI checkpoint:
| head | CI run | result | | head | CI run | result |
|---|---|---| |---|---|---|
| `d33bf4d` | [30950108477](https://github.com/levineuwirth/pmacs/actions/runs/30950108477) | 14/14 green — the reviewed head | | `d33bf4d` | [30950108477](https://github.com/levineuwirth/pmacs/actions/runs/30950108477) | 14/14 green — the reviewed head |
| `f76897c` | [30990816890](https://github.com/levineuwirth/pmacs/actions/runs/30990816890) | review round 1 | | `f76897c` | [30990816890](https://github.com/levineuwirth/pmacs/actions/runs/30990816890) | review round 1 |
| *(tip)* | — | review round 2: framing wording, the missing quote, this table | | `2e0617f` | — | review round 2: framing wording, the missing quote, the checkpoint table |
**The branch tip is authoritative over any row here.** Verify with - **What Stage 1 shipped:** `docs/ci-red-signatures.md`, the single
`git rev-parse githubsucks/macos-ci-signal-integrity` rather than
trusting the newest line.
- **What Stage 1 ships:** `docs/ci-red-signatures.md`, the single
authority for judging a red CI run. Rows key on **signature** — authority for judging a red CI run. Rows key on **signature** —
selector + job/flavor + every required fragment, normalized — so **a selector + job/flavor + every required fragment, normalized — so **a
test-name match confers nothing**. The rerun rule is replaced: a green test-name match confers nothing**. The rerun rule is replaced: a green
@ -202,30 +271,6 @@ entry — the same defect, caught in review.
not rows — unmatchable by construction, so a red in either is a new not rows — unmatchable by construction, so a red in either is a new
incident. The registry is therefore *stricter* than the list it incident. The registry is therefore *stricter* than the list it
replaces: nothing is pre-excused. replaces: nothing is pre-excused.
- **Verification:** fmt, diff-check, clippy with and without `crdt`,
`--lib` 1896, `--lib --features crdt` 2081, pmacs-protocol 19, m4 149,
required GPU 221. All three named tests pass locally on Linux —
consistent with R1–R4 being macOS occurrences, and **not evidence
about any of them**.
### Stage 2 — hardening, NOT started
Waits on #215's merge. Owns R2's trap-readiness fix and R4's
`wait_for_file` predicate, each with a **repetition set** rather than a
single green run, plus a discriminating witness that fails without the
fix. **R1 is referred to the async-runtime lane** (Q#MCI3) rather than
patched: widening its budget would make it pass and measure nothing
more. **R3 stays unresolved** and belongs to the process-signal /
reap-ledger lanes.
Recovery from a clean checkout:
```sh
git fetch githubsucks
git worktree add ../pmacs-ci-signals \
-b macos-ci-signal-integrity \
githubsucks/macos-ci-signal-integrity
```
## CI CRDT coverage — MERGED (#209); kept for its three follow-ons ## CI CRDT coverage — MERGED (#209); kept for its three follow-ons

4
docs/agent-handoff.md
View File

@ -1,7 +1,9 @@
# Agent handoff — cross-machine continuity # Agent handoff — cross-machine continuity
**Last updated: 2026-08-05.** The live CI-triage rule in §5 now points **Last updated: 2026-08-05.** The live CI-triage rule in §5 now points
at `docs/ci-red-signatures.md` (PR #215, in review), which keys on at `docs/ci-red-signatures.md` (**PR #215, merged as `main` @
`12f2970`**; its Stage 2 hardening is in flight — see
`docs/active-work.md`), which keys on
signature rather than test name; the hazards list this file used to signature rather than test name; the hazards list this file used to
carry is retired, and its two unevidenced entries are audit notes there. carry is retired, and its two unevidenced entries are audit notes there.
Previously **2026-08-04, as bottom-panel Stage 3 #213 — the adopter Previously **2026-08-04, as bottom-panel Stage 3 #213 — the adopter

106
docs/ci-red-signatures.md
View File

@ -79,10 +79,19 @@ Main-branch greens are **occurrence evidence** and accumulate toward a
rate. They retire nothing by themselves. Retired rows stay in this file rate. They retire nothing by themselves. Retired rows stay in this file
with their disposition, so a recurrence is recognisable. with their disposition, so a recurrence is recognisable.
**A red matching a RETIRED row is a recurrence, and it puts the
retirement in question — it is not a known flake.** The claim a
retirement makes is that the mechanism is gone; the same signature
afterwards falsifies that claim, which is a stronger finding than a live
row, not a weaker one. Reopen the row rather than rerunning.
--- ---
## Live rows ## Live rows
**Two of the four evidenced rows are live.** R2 and R4 were retired on
2026-08-05 and are below, under "Retired rows", with their dispositions.
### R1 — supersede cancellation budget ### R1 — supersede cancellation budget
| field | value | | field | value |
@ -102,22 +111,6 @@ test's own `tick()` + `sleep(1ms)` pump, so the interval is dominated by
when *the test* was scheduled. **Widening the budget would make it pass when *the test* was scheduled. **Widening the budget would make it pass
and measure nothing more.** and measure nothing more.**
### R2 — USR1 delivered before the trap is installed
| field | value |
|---|---|
| **selector** | `--lib process::tests::a_successful_signal_disposition_depends_on_whether_it_is_fatal` |
| **job / flavor** | macOS / lua54 |
| **required fragments** | `leader=exited(signal SIGUSR1)` — **one exact fragment, not two loose ones**. Split into `leader=exited(` and `SIGUSR1` it would match a child that exited by some *other* disposition while `SIGUSR1` appeared elsewhere in the output |
| **causal status** | **test race** |
| **evidence** | [#213 run 30927084982 attempt 1](https://github.com/levineuwirth/pmacs/actions/runs/30927084982/attempts/1) |
| **retirement** | the fixture proves the trap is installed, with a witness that fails without it |
Readiness is `ProcessEventKind::Started`, emitted at **spawn** — not when
`/bin/sh` has parsed `trap '' USR1`. SIGUSR1's default disposition is
terminate, so a signal inside that window kills the child. The fixture's
own comment states the requirement it does not enforce.
### R3 — live-leader EPERM with an unobservable group ### R3 — live-leader EPERM with an unobservable group
| field | value | | field | value |
@ -138,7 +131,58 @@ field able to disagree — could not be read at all.
**Do not treat a red matching this row as environmental.** A green rerun **Do not treat a red matching this row as environmental.** A green rerun
changes nothing about it. changes nothing about it.
### R4 — readiness predicate satisfied by an empty file ---
## Retired rows
**These stay here on purpose.** A retirement is a claim that a mechanism
is gone; keeping the signature is what makes a recurrence recognisable
as a falsification of that claim rather than as a fresh mystery. Both
were retired **causally** — the mechanism removed, plus a discriminating
witness that fails without the fix — never by a count of green runs.
### R2 — USR1 delivered before the trap is installed — RETIRED 2026-08-05
| field | value |
|---|---|
| **selector** | `--lib process::tests::a_successful_signal_disposition_depends_on_whether_it_is_fatal` |
| **job / flavor** | macOS / lua54 |
| **required fragments** | `leader=exited(signal SIGUSR1)` — **one exact fragment, not two loose ones**. Split into `leader=exited(` and `SIGUSR1` it would match a child that exited by some *other* disposition while `SIGUSR1` appeared elsewhere in the output |
| **causal status** | **test race** |
| **evidence** | [#213 run 30927084982 attempt 1](https://github.com/levineuwirth/pmacs/actions/runs/30927084982/attempts/1) |
| **retirement condition** | the fixture proves the trap is installed, with a witness that fails without it |
| **disposition** | **met.** The child publishes a readiness marker *after* `trap '' USR1`, the test waits for that marker's **content**, and `process::tests::usr1_readiness_waits_for_the_trap_not_for_the_spawn` is the witness |
Readiness was `ProcessEventKind::Started`, emitted at **spawn** — not when
`/bin/sh` has parsed `trap '' USR1`. SIGUSR1's default disposition is
terminate, so a signal inside that window kills the child. The fixture's
own comment stated the requirement it did not enforce.
Three things the fix and the witness settled that the row did not say:
- **Which call failed.** The fragment is rendered only on a *failed*
`kill`, and the USR1 that killed the child cannot itself have failed —
it is what did the killing. The failing call is therefore the SIGTERM
that follows, whose diagnostic reports the leader's earlier death.
**Why a group-directed TERM found no group is not established here**,
and the fix does not depend on the answer. **This says nothing about
R3**, whose leader was observed **live**.
- **The witness proves the predicate, not the platform.** The old fixture
passes on Linux; the window is real everywhere but only macOS ever
reported it. So the witness widens the pre-trap window *deliberately*
(the fixture sleeps before `trap`) instead of hoping a loaded runner
supplies one, and it proves survival by the child's **exit
disposition** — a child that took the USR1 reports
`Signaled { signal: "SIGUSR1" }` — rather than by an absence observed
within a window.
- **The fixture had a second, unnamed dependency.** These signals are
group-directed, so a forked `sleep` would be an *untrapped* member of
the same group. That it survived at all depended on the shell
suppressing the fork for the last command of a `-c` script — a bash
and dash optimization, not a guarantee. The fixture now says `exec`,
and an ignored disposition survives `exec` by POSIX.
### R4 — readiness predicate satisfied by an empty file — RETIRED 2026-08-05
| field | value | | field | value |
|---|---| |---|---|
@ -147,14 +191,28 @@ changes nothing about it.
| **required fragments** | `left: []` **and** `right: [49]` | | **required fragments** | `left: []` **and** `right: [49]` |
| **causal status** | **test race** | | **causal status** | **test race** |
| **evidence** | [#214 run 30932558752 attempt 1](https://github.com/levineuwirth/pmacs/actions/runs/30932558752/attempts/1) | | **evidence** | [#214 run 30932558752 attempt 1](https://github.com/levineuwirth/pmacs/actions/runs/30932558752/attempts/1) |
| **retirement** | `wait_for_file` requires the expected content, with a witness that fails against a zero-byte file | | **retirement condition** | `wait_for_file` requires the expected content, with a witness that fails against a zero-byte file |
| **disposition** | **met.** The helper takes the expected bytes and waits while the file holds a **strict prefix** of them; `wait_for_file_does_not_return_a_zero_byte_readiness_file` is the witness, and with the old predicate restored it fails with `left: []`, `right: [49]` — **this row's two fragments, verbatim** |
`wait_for_file` returns as soon as `fs::read` succeeds — which succeeds `wait_for_file` returned as soon as `fs::read` succeeded — which succeeds
on a **zero-byte file**. The probe writes readiness with on a **zero-byte file**. The probe writes readiness with
`open(path,'wb').write(b'1')`, and `open()` creates the file before `open(path,'wb').write(b'1')`, and `open()` creates the file before
`write()` fills it. The predicate is "readable"; the assertion is `write()` fills it. The predicate was "readable"; the assertion is
"contains `1`" (`49` is ASCII `'1'`). "contains `1`" (`49` is ASCII `'1'`).
Two notes for anyone who reads a future red here:
- **The same mechanism lived in a second helper.** `wait_for_published_file`,
one function away in the same suite, gated the real-TUI smoke's
`assert_eq!(…, b"1")` on the identical "readable" predicate. It was
fixed with the same predicate; leaving it would have let this row
recur under a different selector, which the registry would have had to
judge a new incident.
- **The helper returns divergent content instead of waiting for a
match**, so a child that publishes the *wrong* thing is reported as a
diff by the caller that owns the expectation, rather than as a timeout
in a helper that does not. That behaviour has its own witness.
--- ---
## Audit notes — historical claims with no linked occurrence ## Audit notes — historical claims with no linked occurrence
@ -219,6 +277,14 @@ replaces: nothing is pre-excused.
Four incidents, three tests, **four signatures**. Count signatures: the Four incidents, three tests, **four signatures**. Count signatures: the
process test contributed two, and only one of them is a test bug. process test contributed two, and only one of them is a test bug.
| date | row | event |
|---|---|---|
| 2026-08-05 | R2, R4 | **retired** — mechanism removed, discriminating witness added; see "Retired rows" |
**The retirements are not occurrences and do not close the log.** R1 and
R3 stay live, and each retired row keeps its signature so a later red
matching one reopens it.
**All four *evidenced* rows (R1–R4) are macOS.** That is a property of **All four *evidenced* rows (R1–R4) are macOS.** That is a property of
these occurrences, not of the file: **A1's job is `GPU Render these occurrences, not of the file: **A1's job is `GPU Render
(headless)`, which runs on Ubuntu**, and **A2's job was never (headless)`, which runs on Ubuntu**, and **A2's job was never

117
src/process.rs
View File

@ -2796,6 +2796,61 @@ mod tests {
.expect("Started carries a pid") .expect("Started carries a pid")
} }
/// Wait until `path` holds exactly `expected`.
///
/// **The predicate is the content, never the file's existence.** A
/// shell publishes readiness with `printf … > path`, and the `>`
/// redirection creates the file *before* the command that fills it —
/// so "it exists", and even "it is readable", is satisfied by a
/// zero-byte file. That is `docs/ci-red-signatures.md` R4's mechanism
/// arriving in a second place; a readiness gate written the weak way
/// re-opens the very window R2 is about.
fn wait_for_published(path: &std::path::Path, expected: &[u8], timeout: Duration) {
let deadline = Instant::now() + timeout;
loop {
if std::fs::read(path).is_ok_and(|bytes| bytes == expected) {
return;
}
assert!(
Instant::now() < deadline,
"child never published {expected:?} to {} within {timeout:?} \
(last read: {:?})",
path.display(),
std::fs::read(path).ok()
);
std::thread::sleep(Duration::from_millis(10));
}
}
/// `/bin/sh -c` argument for a child that ignores SIGUSR1, says so,
/// and then becomes `sleep` — the R2 fixture.
///
/// The order is the whole point:
///
/// - **`trap '' USR1` first.** SIGUSR1's default disposition is
/// terminate, and `ProcessEventKind::Started` is emitted when the
/// process is *spawned*, not when `/bin/sh` has parsed anything.
/// Waiting on `Started` therefore returns inside a window where the
/// signal kills the child.
/// - **The marker second**, so the test has something to wait on that
/// the shell can only publish once the trap exists.
/// - **`exec` last**, so the process group holds exactly one process.
/// An *ignored* disposition survives `exec`, while a forked `sleep`
/// would be an untrapped member of the same group — and these
/// signals are group-directed, so it would die and take the shell's
/// `wait` (and the group) with it.
///
/// `pre_trap` is the delay the R2 witness prepends to widen the
/// pre-trap window deliberately; the fixture itself passes `""`.
fn trapped_usr1_command(ready: &std::path::Path, pre_trap: &str) -> String {
let path = ready.to_str().expect("UTF-8 temp path");
assert!(
!path.contains('\''),
"the single-quoting below assumes no quote in the temp path: {path}"
);
format!("{pre_trap}trap '' USR1; printf trapped > '{path}'; exec sleep 30")
}
/// Drive the production diagnostic until it observes the leader as /// Drive the production diagnostic until it observes the leader as
/// exited, bounded by `timeout`. /// exited, bounded by `timeout`.
/// ///
@ -3223,13 +3278,20 @@ mod tests {
#[test] #[test]
fn a_successful_signal_disposition_depends_on_whether_it_is_fatal() { fn a_successful_signal_disposition_depends_on_whether_it_is_fatal() {
let mut sup = ProcessSupervisor::new(); let mut sup = ProcessSupervisor::new();
let temp = tempfile::TempDir::new().expect("tempdir");
let ready = temp.path().join("usr1-trapped");
let mut spec = ProcessSpec::new("diag-disposition-live", "/bin/sh"); let mut spec = ProcessSpec::new("diag-disposition-live", "/bin/sh");
// Ignore USR1 so the successful non-fatal signal cannot end the // Ignore USR1 so the successful non-fatal signal cannot end the
// child and confuse the state assertion with a real exit. // child and confuse the state assertion with a real exit — and
spec.args = vec!["-c".into(), "trap '' USR1; sleep 30".into()]; // then WAIT for the child to say it has done so. `Started` is
// emitted at spawn and proves nothing about what `/bin/sh` has
// parsed (`docs/ci-red-signatures.md` R2); see
// `trapped_usr1_command`.
spec.args = vec!["-c".into(), trapped_usr1_command(&ready, "")];
spec.group = true; spec.group = true;
let id = sup.spawn(spec).expect("spawn"); let id = sup.spawn(spec).expect("spawn");
let pid = spawn_started_pid(&mut sup, id); let pid = spawn_started_pid(&mut sup, id);
wait_for_published(&ready, b"trapped", Duration::from_secs(10));
sup.signal(id, Signal::SIGUSR1).expect("USR1 delivers"); sup.signal(id, Signal::SIGUSR1).expect("USR1 delivers");
assert!( assert!(
@ -3261,6 +3323,57 @@ mod tests {
nix::sys::signal::kill(Pid::from_raw(-i32::try_from(pid).unwrap()), Signal::SIGKILL); nix::sys::signal::kill(Pid::from_raw(-i32::try_from(pid).unwrap()), Signal::SIGKILL);
} }
/// R2's discriminating witness — readiness must gate on the **trap**,
/// not on the spawn.
///
/// `spawn_started_pid` returns when `ProcessEventKind::Started`
/// arrives, which is emitted at spawn; the shell has not parsed
/// `trap '' USR1` yet, and SIGUSR1's default disposition is
/// terminate. This fixture makes that window a deliberate second wide
/// instead of leaving it to a loaded runner, so the weak predicate is
/// *guaranteed* to return inside it — which is what makes this test
/// discriminating rather than lucky.
///
/// Survival is proved by the child's **exit disposition**, not by an
/// absence observed within a window: a child that took the USR1
/// reports `Signaled { signal: "SIGUSR1" }`, and one that ignored it
/// reports the SIGTERM sent afterwards. Remove the
/// `wait_for_published` line and this test reports `SIGUSR1`.
#[test]
fn usr1_readiness_waits_for_the_trap_not_for_the_spawn() {
let mut sup = ProcessSupervisor::new();
let temp = tempfile::TempDir::new().expect("tempdir");
let ready = temp.path().join("usr1-trapped");
let mut spec = ProcessSpec::new("diag-trap-readiness", "/bin/sh");
spec.args = vec!["-c".into(), trapped_usr1_command(&ready, "sleep 1; ")];
spec.group = true;
let id = sup.spawn(spec).expect("spawn");
let pid = spawn_started_pid(&mut sup, id);
// The weak predicate is already satisfied, a second before the
// trap exists. The strong one cannot be.
wait_for_published(&ready, b"trapped", Duration::from_secs(10));
sup.signal(id, Signal::SIGUSR1)
.expect("USR1 delivers to a child that has trapped it");
sup.terminate(id).expect("TERM delivers");
let events = drain_until(&mut sup, id, Duration::from_secs(5), has_exited);
let signal = events.iter().find_map(|e| match &e.kind {
ProcessEventKind::Signaled { signal } => Some(signal.clone()),
_ => None,
});
assert_eq!(
signal.as_deref(),
Some("SIGTERM"),
"a child that trapped USR1 before the signal must die of the \
TERM instead: {events:?}"
);
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, /// Q#DC4 / acceptance 4 — the measured group is a real observation,
/// not a restatement of the input. /// not a restatement of the input.
/// ///

154
tests/vterm_stage2_acceptance.rs
View File

@ -544,7 +544,10 @@ fn terminal_escape_gates_local_bindings_and_double_escape_sends_interrupt() {
)) ))
.eval::<AnyUserData>() .eval::<AnyUserData>()
.expect("open raw input probe"); .expect("open raw input probe");
assert_eq!(wait_for_file(&ready_path, Duration::from_secs(5)), b"1"); assert_eq!(
wait_for_file(&ready_path, b"1", Duration::from_secs(5)),
b"1"
);
let frontend_id = FrontendId::LOCAL; let frontend_id = FrontendId::LOCAL;
state.dispatch_key( state.dispatch_key(
@ -564,9 +567,10 @@ fn terminal_escape_gates_local_bindings_and_double_escape_sends_interrupt() {
KeyEvent::new(KeyCode::Char('w'), KeyModifiers::ALT), KeyEvent::new(KeyCode::Char('w'), KeyModifiers::ALT),
); );
let expected_input = b"\x1bv\x03\x1bw";
assert_eq!( assert_eq!(
wait_for_file(&input_path, Duration::from_secs(5)), wait_for_file(&input_path, expected_input, Duration::from_secs(5)),
b"\x1bv\x03\x1bw", expected_input,
"unescaped local bindings reach the child; C-c C-c sends one literal interrupt" "unescaped local bindings reach the child; C-c C-c sends one literal interrupt"
); );
} }
@ -718,42 +722,151 @@ fn wait_for_output(
/// The plain helper panics with only the missing path, which is the least /// The plain helper panics with only the missing path, which is the least
/// useful thing to know: a readiness file that never appears is exactly when /// useful thing to know: a readiness file that never appears is exactly when
/// "how far did startup get" decides where to look next. /// "how far did startup get" decides where to look next.
///
/// Carries [`wait_for_file`]'s `expected` predicate for the same reason:
/// this helper gates on a file the probe fills with `open(path,'wb')`
/// too, so "readable" would let a zero-byte read through here as well.
fn wait_for_published_file( fn wait_for_published_file(
pty: &mut PmacsPty, pty: &mut PmacsPty,
path: &Path, path: &Path,
expected: &[u8],
timeout: Duration, timeout: Duration,
startup: &[(&str, &Path)], startup: &[(&str, &Path)],
) -> Vec<u8> { ) -> Vec<u8> {
let deadline = Instant::now() + timeout; let deadline = Instant::now() + timeout;
loop { loop {
if let Ok(bytes) = fs::read(path) { if let Some(bytes) = settled_file(path, expected) {
return bytes; return bytes;
} }
assert!( assert!(
Instant::now() < deadline, Instant::now() < deadline,
"child never published {} within {timeout:?}\n startup: {}", "child never published {expected:?} to {} within {timeout:?}\n \
last read: {:?}\n startup: {}",
path.display(), path.display(),
fs::read(path).ok(),
describe_startup(pty, startup) describe_startup(pty, startup)
); );
thread::sleep(Duration::from_millis(20)); thread::sleep(Duration::from_millis(20));
} }
} }
fn wait_for_file(path: &Path, timeout: Duration) -> Vec<u8> { /// Wait until `path` holds bytes the caller can assert against, and
/// return them.
///
/// **The predicate is the content, not readability.** Every probe in this
/// suite publishes with `open(path,'wb').write(...)`, and `open()` creates
/// the file *before* `write()` fills it — so "`fs::read` succeeded" is
/// satisfied by a **zero-byte file** and this helper used to return `[]`
/// into a caller asserting `== b"1"`. That is `docs/ci-red-signatures.md`
/// R4, whose failure text is `left: []`, `right: [49]`.
///
/// The wait therefore continues while the file holds a **strict prefix**
/// of `expected` — the states a write in flight can be observed in — and
/// returns as soon as the bytes are anything else. Returning on
/// divergence rather than insisting on a match is deliberate: the
/// caller's `assert_eq!` stays the discriminating assertion, so wrong
/// content is reported as a diff by the test that owns the expectation,
/// not as a timeout in a helper that does not.
fn wait_for_file(path: &Path, expected: &[u8], timeout: Duration) -> Vec<u8> {
let deadline = Instant::now() + timeout; let deadline = Instant::now() + timeout;
loop { loop {
if let Ok(bytes) = fs::read(path) { if let Some(bytes) = settled_file(path, expected) {
return bytes; return bytes;
} }
assert!( assert!(
Instant::now() < deadline, Instant::now() < deadline,
"file was not published: {}", "file never settled: {} (expected {expected:?}, last read {:?})",
path.display() path.display(),
fs::read(path).ok()
); );
thread::sleep(Duration::from_millis(20)); thread::sleep(Duration::from_millis(20));
} }
} }
/// `Some(bytes)` once `path` is readable and no longer a write in
/// flight; `None` while it is missing or holds a strict prefix of
/// `expected`.
fn settled_file(path: &Path, expected: &[u8]) -> Option<Vec<u8>> {
let bytes = fs::read(path).ok()?;
let in_flight = bytes.len() < expected.len() && expected.starts_with(&bytes);
(!in_flight).then_some(bytes)
}
/// R4's discriminating witness — the zero-byte file, reproduced exactly.
///
/// `docs/ci-red-signatures.md` R4 is `wait_for_file` returning `[]` into a
/// caller asserting `== b"1"`, because `open(path,'wb')` creates the file
/// before `write()` fills it. The file here is created empty and filled
/// later, which makes that window certain instead of load-dependent.
///
/// Revert the predicate to "`fs::read` succeeded" and this fails with
/// `left: []`, `right: [49]` — R4's two required fragments, verbatim.
#[test]
fn wait_for_file_does_not_return_a_zero_byte_readiness_file() {
let temp = tempfile::TempDir::new().expect("tempdir");
let path = temp.path().join("ready");
// The state `open(path,'wb')` leaves behind before its `write`.
drop(fs::File::create(&path).expect("create the empty readiness file"));
let filled = path.clone();
let writer = thread::spawn(move || {
thread::sleep(Duration::from_millis(150));
fs::write(&filled, b"1").expect("fill the readiness file");
});
assert_eq!(wait_for_file(&path, b"1", Duration::from_secs(5)), b"1");
writer.join().expect("writer thread");
}
/// The same predicate, one step past R4: a **partial** write is a torn
/// read, not a readiness signal.
///
/// R4's occurrence was the zero-byte case because the marker is one byte.
/// The multi-byte callers in this suite (`b"\x1bv\x03\x1bw"`,
/// `b"VTERM_INPUT_SMOKE\n"`) can observe a prefix for the same reason, and
/// a helper that only rejected *empty* would hand one to `assert_eq!`.
#[test]
fn wait_for_file_does_not_return_a_partial_write() {
let expected = b"VTERM_INPUT_SMOKE\n";
let temp = tempfile::TempDir::new().expect("tempdir");
let path = temp.path().join("child-input");
drop(fs::File::create(&path).expect("create the empty input file"));
let filled = path.clone();
let writer = thread::spawn(move || {
thread::sleep(Duration::from_millis(100));
fs::write(&filled, b"VTERM").expect("partial write");
thread::sleep(Duration::from_millis(100));
fs::write(&filled, expected).expect("complete write");
});
assert_eq!(
wait_for_file(&path, expected, Duration::from_secs(5)),
expected
);
writer.join().expect("writer thread");
}
/// Content that is *wrong* rather than *unfinished* comes straight back,
/// so the caller's `assert_eq!` reports it.
///
/// This is what keeps the strengthened predicate from moving the
/// assertion into the helper: waiting for an exact match would turn a
/// child that published the wrong thing into a timeout in a function that
/// does not know what was expected of it, and delete the diff that says
/// what arrived instead.
#[test]
fn wait_for_file_returns_divergent_content_rather_than_timing_out() {
let temp = tempfile::TempDir::new().expect("tempdir");
let path = temp.path().join("child-size");
fs::write(&path, b"29 90\n").expect("publish the wrong geometry");
assert_eq!(
wait_for_file(&path, b"28 90\n", Duration::from_millis(500)),
b"29 90\n"
);
}
#[test] #[test]
#[allow(clippy::too_many_lines, reason = "one real-host lifecycle scenario")] #[allow(clippy::too_many_lines, reason = "one real-host lifecycle scenario")]
fn real_tui_terminal_smoke_restores_host_after_output_input_resize_scroll_copy_and_bell() { fn real_tui_terminal_smoke_restores_host_after_output_input_resize_scroll_copy_and_bell() {
@ -888,7 +1001,13 @@ fn real_tui_terminal_smoke_restores_host_after_output_input_resize_scroll_copy_a
// are protocol escapes pmacs writes directly, never painted cells, so the // are protocol escapes pmacs writes directly, never painted cells, so the
// differ cannot split them. // differ cannot split them.
assert_eq!( assert_eq!(
wait_for_published_file(&mut pty, &alt_ready_path, Duration::from_secs(10), startup), wait_for_published_file(
&mut pty,
&alt_ready_path,
b"1",
Duration::from_secs(10),
startup
),
b"1", b"1",
"alt-screen readiness breadcrumb was published but malformed" "alt-screen readiness breadcrumb was published but malformed"
); );
@ -904,12 +1023,19 @@ fn real_tui_terminal_smoke_restores_host_after_output_input_resize_scroll_copy_a
pty.write_input(b"\x03\x1bw") pty.write_input(b"\x03\x1bw")
.expect("escaped copy selection binding"); .expect("escaped copy selection binding");
wait_for_output(&mut pty, b"\x1b]52;c;", Duration::from_secs(5), startup); wait_for_output(&mut pty, b"\x1b]52;c;", Duration::from_secs(5), startup);
let input = wait_for_file(&input_path, Duration::from_secs(5)); let expected_input = b"VTERM_INPUT_SMOKE\n";
assert_eq!(input, b"VTERM_INPUT_SMOKE\n"); let input = wait_for_file(&input_path, expected_input, Duration::from_secs(5));
assert_eq!(input, expected_input);
pty.write_input(b"\x1b[200~PASTE_AFTER_EXIT\x1b[201~") pty.write_input(b"\x1b[200~PASTE_AFTER_EXIT\x1b[201~")
.expect("route a real host paste event"); .expect("route a real host paste event");
let size = String::from_utf8(wait_for_file(&size_path, Duration::from_secs(5))) // The probe writes `f'{lines} {columns}\n'`, so the settled content
.expect("UTF-8 stty size"); // carries the trailing newline the assertion below trims.
let size = String::from_utf8(wait_for_file(
&size_path,
b"28 90\n",
Duration::from_secs(5),
))
.expect("UTF-8 stty size");
assert_eq!(size.trim(), "28 90", "child PTY must receive cell geometry"); assert_eq!(size.trim(), "28 90", "child PTY must receive cell geometry");
thread::sleep(Duration::from_millis(200)); thread::sleep(Duration::from_millis(200));
pty.write_input(b"\x03\x18\x03").expect("quit pmacs"); pty.write_input(b"\x03\x18\x03").expect("quit pmacs");