Merge pull request #216 from levineuwirth/ci-signal-hardening

CI signal hardening — retire R2 and R4 with discriminating witnesses
This commit is contained in:
Levi Neuwirth 2026-08-05 15:39:28 +00:00 committed by GitHub
commit 2657568ade
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 586 additions and 88 deletions

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
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
lane #209, Distribution Stage 1 #211 (released as **v1.1.0**), the
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
machine-local: `origin` may name this canonical URL, a release mirror,
or something else, and therefore has no authority by name alone.
- Canonical base at this snapshot: **`githubsucks/main` @ `f186253`**
— bottom-panel Stage 3 **#213**, which completes Arc 7, atop the
- Canonical base at this snapshot: **`githubsucks/main` @ `12f2970`** —
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**
(released as **v1.1.0**, the first release with prebuilt binaries),
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
only the first.
**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
the declared base passes on a tree the rest of this file does not
describe.
@ -145,7 +153,7 @@ git worktree list
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
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
@ -161,31 +169,145 @@ form. All four steps ran clean. **The two-argument form still does not
work** for a remote-only branch (`fatal: invalid reference`), which is
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
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
entry — the same defect, caught in review.
#171 drifted 153 commits while invisible here). #215 had none until
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` @
`bfb97c6`. Framing `docs/macos-ci-signal-integrity-framing.md`
**revision 3**.
- **PR: <https://github.com/levineuwirth/pmacs/pull/215>** — Stage 1,
docs only. **Checkpoints, newest last** — a lane that records only the
*reviewed* head goes stale the moment a review round is pushed, which
is how #171 became invisible:
Framing `docs/macos-ci-signal-integrity-framing.md` **revision 3**,
already approved. Stage 1 is acceptance 15; Stage 2 is acceptance 69.
### Stage 2 — hardening, IN REVIEW
- **Branch `ci-signal-hardening`**, worktree `../pmacs-ci-signals`, base
`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: <https://github.com/levineuwirth/pmacs/pull/216>**. This block
was written *with* the work, before the PR existed, so the row below
was 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 |
|---|---|---|
| `2d9c678` | [31003333581](https://github.com/levineuwirth/pmacs/actions/runs/31003333581) | **14/14 green** — the opening head, and the last one carrying code. Includes **both macOS legs**: R2's job (macOS / lua54) and R4's (macOS / luajit) |
| `668fc72` | [31006160334](https://github.com/levineuwirth/pmacs/actions/runs/31006160334) | **14/14 green** — this ledger block, docs only |
| *(tip)* | — | the checkpoint row for `668fc72`; a table can never carry the head that adds it |
**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. Each `--lib` figure is exactly one above #215's
(1896 / 2081): the R2 witness.
- **Full serialized sweep in BOTH feature configurations, baseline
first** — the blindness handoff §5 warns about, and the baseline is
what makes any failure attributable:
| tree | config | suites | passed | failed |
|---|---|---|---|---|
| `main` @ `12f2970` | luajit | 108 | 3691 | 0 |
| `main` @ `12f2970` | luajit,crdt | 108 | 3959 | 0 |
| branch | luajit | 108 | 3695 | 0 |
| branch | luajit,crdt | 108 | 3962 | **1 — see below** |
**Every total is exactly +4 on its baseline** — 3691→3695 and
3959→3963 (3962 passed plus the one failure). Four witnesses, and
nothing else moved.
- **The one branch-`crdt` failure is recorded rather than rerun away.**
`lsp_dispatch_seams_acceptance
acc33_apply_edit_still_handled_with_a_response_subscriber` —
*"the executeCommand response reaches its one-shot, left: 0, right:
1"*. It is a **new incident** by the registry's rules (no row's
fragments match), and it is **not attributable to this branch**, on a
structural argument rather than on the green rerun:
- the only Rust change is inside `#[cfg(test)] mod tests` in
`src/process.rs`, which is compiled **only into the lib test
target** — an integration-test binary links the non-`cfg(test)`
lib, so this suite's artifact is what `main` builds;
- the other change is a different test binary entirely;
- CI's **`Test (crdt)` job passed at this same head**, and it runs
this suite;
- it occurred while a **second full workspace sweep** was running
concurrently in another worktree, against a deadline-based fake-LSP
one-shot;
- the suite then ran **15/15 as a repetition set** — which, per this
project's own rerun rule, establishes **intermittence only** and is
the weakest of these five points, not the argument.
**No registry row is opened**, because the registry judges red **CI**
runs and keys on linked CI occurrences; this is a local observation
under known contention. If it appears in CI, it is a first recorded
occurrence and gets a row then.
- **A SHARED `CARGO_TARGET_DIR` MAKES A LOCAL SWEEP UNATTRIBUTABLE, and
it bit this lane.** The branch `crdt` sweep first reported 7 failures
in three suites while the baseline `crdt` sweep was clean. All three
spawn the **real `pmacs` binary out of the target directory**, and the
failure text named its own cause: *"daemon does not advertise required
capabilities … start the daemon built with the `crdt` feature"*. A
concurrent `cargo test --workspace` in a **different worktree**, with
default (non-`crdt`) features and the same `CARGO_TARGET_DIR`, had
overwritten `target/debug/pmacs` mid-sweep. Confirmed by `pgrep` while
it was happening, and discriminated by re-running the same three
suites from the same tree with a **dedicated** `CARGO_TARGET_DIR`:
41/41 green, then the whole config swept clean the same way. **Give a
sweep its own target directory whenever another worktree is live** —
a feature-flavored binary is a shared mutable file, not a build
artifact private to your invocation.
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 |
|---|---|---|
| `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 |
| *(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
`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
- **What Stage 1 shipped:** `docs/ci-red-signatures.md`, the single
authority for judging a red CI run. Rows key on **signature**
selector + job/flavor + every required fragment, normalized — so **a
test-name match confers nothing**. The rerun rule is replaced: a green
@ -202,30 +324,6 @@ entry — the same defect, caught in review.
not rows — unmatchable by construction, so a red in either is a new
incident. The registry is therefore *stricter* than the list it
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 R1R4 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

View File

@ -1,7 +1,9 @@
# Agent handoff — cross-machine continuity
**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
carry is retired, and its two unevidenced entries are audit notes there.
Previously **2026-08-04, as bottom-panel Stage 3 #213 — the adopter
@ -315,6 +317,18 @@ someone forgot.
`gpu_invocation_acceptance` tests fail on a missing `pmacs-gpu`
binary. `cargo build --workspace --no-default-features --features
luajit,crdt` is the invocation that produces both binaries.
- **A shared `CARGO_TARGET_DIR` makes a local sweep unattributable.**
Every worktree on this machine resolves to the same target directory,
so `target/debug/pmacs` is a **shared mutable file**: a concurrent
`cargo test --workspace` in another worktree, at default (non-`crdt`)
features, overwrites the binary that a running `crdt` sweep is
spawning. That produced seven failures in three real-daemon suites
during the Stage 2 hardening sweep while the baseline was clean, and
the failure text named its own cause — *"start the daemon built with
the `crdt` feature"*. Confirmed with `pgrep` and discriminated by
re-running from the same tree with a **dedicated**
`CARGO_TARGET_DIR`. **Check for other live worktrees before believing
a sweep failure, and give the sweep its own target directory.**
- **A local sweep is blind to whichever feature configuration it does
not build.** Stage 3's census and every verification sweep ran
`--features luajit` WITHOUT `crdt`, so no crdt-gated suite was

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
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
**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
| 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
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
| field | value |
@ -138,7 +131,69 @@ field able to disagree — could not be read at all.
**Do not treat a red matching this row as environmental.** A green rerun
changes nothing about it.
### R4 — readiness predicate satisfied by an empty file
**R2's retirement does not touch this row, and must not be read as
touching it.** The 2026-08-05 hardening changed that test's *fixture*
a readiness gate and an `exec` — and changed **no product code at all**;
the same group-directed `kill` runs against the same supervisor. What
the fixture change does do is alter the shape of the group being
signalled (one process now, where a forked `sleep` could make two), so
**a change in how often this row appears would be evidence about
frequency, not about cause**. A red carrying these fragments after that
date is this same unresolved row, and its retirement is still a
diagnosis by the process-signal / reap-ledger lanes.
---
## 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 |
|---|---|
@ -147,14 +202,28 @@ changes nothing about it.
| **required fragments** | `left: []` **and** `right: [49]` |
| **causal status** | **test race** |
| **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
`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'`).
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
@ -219,6 +288,14 @@ replaces: nothing is pre-excused.
Four incidents, three tests, **four signatures**. Count signatures: the
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 (R1R4) are macOS.** That is a property of
these occurrences, not of the file: **A1's job is `GPU Render
(headless)`, which runs on Ubuntu**, and **A2's job was never

View File

@ -1,9 +1,14 @@
# Framing — macOS CI signal integrity: a signature registry, then hardening
**Revision 3.** Status: **Stage 1 implemented** on
`macos-ci-signal-integrity`, PR #215. Scouted against
**Revision 4.** Status: **Stage 1 MERGED** as #215 (`main` @ `12f2970`);
**Stage 2 implemented** on `ci-signal-hardening`. Scouted against
`githubsucks/main` @ `bfb97c6`.
**Revision 4 records implementation findings, not a new design round.**
The acceptance in §4 is unchanged and was approved at revision 3; §8
below is what building Stage 2 established, including two places where
this document's own account of a mechanism was imprecise.
**Revision 2 → 3** exists because the implementation discovered a state
the contract did not allow, and the contract — not the implementation —
was what needed changing. Acceptance 3 offered a binary: carry an
@ -356,15 +361,80 @@ survives on reputation. Stage 2 adds the repetition sets of acceptance 9.
---
## 7. Branch plan
## 7. Stage 2 implementation notes (revision 4)
Two PRs, in this order:
Stage 2 landed acceptance 69 on `ci-signal-hardening`. Four findings,
two of which correct this document.
### 7.1 §1.3 named the right window and the wrong assertion
`leader=exited(signal SIGUSR1)` is rendered **only on a failed `kill`**,
by `signal_failure_report`. The USR1 cannot be the call that failed — it
is the call that did the killing — so the failing call is the **SIGTERM
that follows**, and the assertion that blew up is
`.expect("TERM delivers")`, not the `Running` state check. §1.3's "the
record is `exited(signal SIGUSR1)` instead of `Running`" reads as though
`ProcessState` carries that value; it does not, and nothing ticks
between the two calls, so the state assertion could not have seen an
exit. **The row's fragment and its mechanism were both right.** Why a
group-directed TERM then found no group is *not* established, and the
fix does not depend on it.
### 7.2 The fixture had a second dependency nobody had named
These signals are **group-directed**, so the fixture's `sleep 30` — if
the shell *forks* it — is an **untrapped member of the same group**, and
the USR1 kills it even when the trap is installed correctly. That the
old fixture survived at all depended on bash and dash suppressing the
fork for the last command of a `-c` script when no non-ignored trap is
set. Verified locally, and the suppression is visible in `ps`: with
`trap '' USR1` present, the only member of the group is the **shell's
own pid** running `sleep` — the shell replaced itself, so there was
never an untrapped child to kill.
**An optimization is not a guarantee.** The fixture now says `exec`, so
the group holds exactly one process and the ignored disposition survives
by POSIX. This is a second readiness-shaped defect in the same fixture:
the test depended on a state it never established.
### 7.3 §1.5 scoped the fix one function too narrowly
`wait_for_published_file` — in the same suite, one function above
`wait_for_file` — gates the real-TUI smoke's `assert_eq!(…, b"1")` on
the identical "`fs::read` succeeded" predicate. §1.5's note that the
*bottom-panel* helper already rejects empty reads is about a different
file and correctly refuses scope creep there; it does not cover this
one. Fixing only `wait_for_file` would have left R4's mechanism live
under a different selector, where the registry would have had to judge
the recurrence a **new incident**.
### 7.4 What a witness had to be, in each case
- **R4's witness reproduces the row.** The file is created empty and
filled later, so the window is certain rather than load-dependent, and
with the old predicate restored it fails `left: []`, `right: [49]`
the row's two required fragments, verbatim.
- **R2's witness could not reproduce the row on Linux**, because the old
fixture passes here; the pre-trap window is real everywhere but only
macOS ever reported the failure. So the witness **widens the window
deliberately** (the fixture sleeps before `trap`) and proves survival
by the child's **exit disposition**`Signaled { signal: "SIGUSR1" }`
versus `"SIGTERM"` — rather than by an absence observed within a
window, which would have been another timing assumption in a lane
about timing assumptions.
- Bet 1 is therefore **supported but not yet resolved**: both mechanisms
are gone and both are witnessed, but the bet is falsified only by a
recurrence in CI, which only time can supply.
## 8. Branch plan
Two PRs, in this order — **both now exist**:
1. **`macos-ci-signal-integrity`** — the registry, the pointer
rewrites, the audit, and the rerun rule. No code.
2. **A hardening PR** — the `wait_for_file` predicate and the USR1 trap
readiness, each with a repetition set. Signature 1 is referred to the
async-runtime design question rather than patched.
rewrites, the audit, and the rerun rule. No code. **Merged as #215.**
2. **`ci-signal-hardening`** — the `wait_for_file` predicate and the
USR1 trap readiness, each with a repetition set. Signature 1 is
referred to the async-runtime design question rather than patched.
Quarantine, if it happens, is a third and is scoped by what hardening
fails to fix.

View File

@ -2796,6 +2796,61 @@ mod tests {
.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
/// exited, bounded by `timeout`.
///
@ -3223,13 +3278,20 @@ mod tests {
#[test]
fn a_successful_signal_disposition_depends_on_whether_it_is_fatal() {
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");
// 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()];
// child and confuse the state assertion with a real exit — and
// 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;
let id = sup.spawn(spec).expect("spawn");
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");
assert!(
@ -3261,6 +3323,57 @@ mod tests {
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,
/// not a restatement of the input.
///

View File

@ -544,7 +544,10 @@ fn terminal_escape_gates_local_bindings_and_double_escape_sends_interrupt() {
))
.eval::<AnyUserData>()
.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;
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),
);
let expected_input = b"\x1bv\x03\x1bw";
assert_eq!(
wait_for_file(&input_path, Duration::from_secs(5)),
b"\x1bv\x03\x1bw",
wait_for_file(&input_path, expected_input, Duration::from_secs(5)),
expected_input,
"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
/// useful thing to know: a readiness file that never appears is exactly when
/// "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(
pty: &mut PmacsPty,
path: &Path,
expected: &[u8],
timeout: Duration,
startup: &[(&str, &Path)],
) -> Vec<u8> {
let deadline = Instant::now() + timeout;
loop {
if let Ok(bytes) = fs::read(path) {
if let Some(bytes) = settled_file(path, expected) {
return bytes;
}
assert!(
Instant::now() < deadline,
"child never published {} within {timeout:?}\n startup: {}",
"child never published {expected:?} to {} within {timeout:?}\n \
last read: {:?}\n startup: {}",
path.display(),
fs::read(path).ok(),
describe_startup(pty, startup)
);
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;
loop {
if let Ok(bytes) = fs::read(path) {
if let Some(bytes) = settled_file(path, expected) {
return bytes;
}
assert!(
Instant::now() < deadline,
"file was not published: {}",
path.display()
"file never settled: {} (expected {expected:?}, last read {:?})",
path.display(),
fs::read(path).ok()
);
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]
#[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() {
@ -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
// differ cannot split them.
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",
"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")
.expect("escaped copy selection binding");
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));
assert_eq!(input, b"VTERM_INPUT_SMOKE\n");
let expected_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~")
.expect("route a real host paste event");
let size = String::from_utf8(wait_for_file(&size_path, Duration::from_secs(5)))
.expect("UTF-8 stty size");
// The probe writes `f'{lines} {columns}\n'`, so the settled content
// 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");
thread::sleep(Duration::from_millis(200));
pty.write_input(b"\x03\x18\x03").expect("quit pmacs");