From 028016333cb4c0de0f22a969eb7d7c23f5771913 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sun, 9 Aug 2026 19:41:57 +0200 Subject: [PATCH] docs: record review round 2, and R7's control finally discriminated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two files, no code. ## `docs/active-work.md` — review round 2 The lane's volatile block gains the round-2 record: the three findings, why P2a's fix is a mapped diagnostic and P2b's is escaping at presentation rather than rejection at the registry, and the seven new mutation checks. The gate outcome is recorded with its step counts and the two stop-signal facts — `journey_acceptance` 47/47 and all three `#pmacs.process.list()` leak detectors byte-identical to `main`. It also records what P2a's audit found and did NOT fix: `pmacs.process.spawn`'s other string fields still convert generically. That is pre-existing and out of this lane's diff, and it is named so it is not silently inherited by whoever reads the fixed `purpose` read and assumes the rest matches. ## `docs/ci-red-signatures.md` — R7's third occurrence, and U4 **R7 reproduced, and the control the second-occurrence note prescribed finally discriminated — against its own hypothesis.** Occurrence 2 left exactly one causal path open: the observing lane had added a GPU-heavy `render_offscreen` test to the same binary, and contention with a one-second socket handshake was plausible. That note prescribed the control to run if a third occurrence landed — with the added test removed, not at the merge base. A third occurrence landed, at the gate's `gpu` step, with all three fragments verified against the durable log. The control was run. **Ten full `-p pmacs-gpu` runs with the added test: 10/10 green. Ten with it `#[ignore]`d, nothing else changed: 1 failure in 10, all three fragments present.** Removing the suspect made the failure more frequent, so the concurrent-test path is excluded — no contention story from that test survives that direction. The more useful result is the rate. This is the first rerun in R7's history to reproduce anything at all, and it puts the failure at roughly 1-in-10 under ordinary `-p pmacs-gpu` load. Three sightings were not enough to bisect a handshake; 1-in-10 is. The row now says so, and tells the next agent to instrument which side closes the pipe rather than re-run for green. The lane is still not attributed — now for a measured reason rather than an argument from diff shape: the arm without the lane's only `pmacs-gpu` addition is the arm that went red. **U4** records the other two reds from that same gate run: `criterion_1_end_of_line_typing_completes_sub_frame_per_keystroke` and `composition_overhead_under_ten_percent`, both wall-clock budget assertions, failing together in `04-lib-crdt` and both green in isolation and in the next full run. Fragments captured, so unlike U1–U3 it is matchable — it is a `U` row for want of a mechanism, not for want of evidence. The signature named is **the pair**: two budget tests failing in one run and neither in the next is far more likely to be one loaded machine than two simultaneous regressions, and a future run that reds only one of them is a different incident. Neither row claims harmlessness, and the concurrent-worktree load confound is recorded as a rival explanation rather than as a finding, because it was not measured. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 63 +++++++++++++++++++++++++++++++++++++-- docs/ci-red-signatures.md | 63 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 121 insertions(+), 5 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 83f2701..5de19eb 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -358,8 +358,9 @@ the authoritative tip** — the ref, not a SHA. Recover with required field is exactly the kind of change that breaks a package fixture quietly. - **IMPLEMENTED at `1aca0ee`**, with review round 1's blocker fixed at - `2162737`. `tests/worker_identity_acceptance.rs` is the new suite: 18 - tests, plus one consumer-side witness beside the private renderer in + `2162737` and review round 2's three findings at `6661125`. + `tests/worker_identity_acceptance.rs` is the new suite: **24 tests**, + plus one consumer-side witness beside the private renderer in `pmacs-gpu`. - **`journey_acceptance` passed UNTOUCHED (47/47)** — the stop signal did not fire. Q#W-7 edits the `commit_to` guard family, so any of its @@ -426,6 +427,64 @@ the authoritative tip** — the ref, not a SHA. Recover with `pmacs.process.spawn("ls")` occurrences in `src/audit/mod.rs` and `tests/m7_9_acceptance.rs` are **audit fixture source text** — lexed, never executed — and are deliberately untouched. +- **Review round 2 — the display-text boundary, fixed at `6661125`.** + Three findings, and the fix is deliberately different in each place + because the constraint is. + - **P2a: invalid UTF-8 bypassed the `purpose` diagnostic.** + `required_purpose` read the field with `value.to_str()?`; Lua strings + are BYTE strings, so `purpose = string.char(255)` surfaced mlua's + generic conversion error before this lane's own message existed. It + refused before spawning, so nothing leaked — the defect was the + message. **Third occurrence of this class in the project** (the + destination-capture lane corrected the same shape two rounds ago), so + the whole diff was audited for it: exactly one more, + `_push_dispatch_name` taking `name: String`, now `mlua::String` with + an owned diagnostic. Those two are the only Lua-string reads this + lane added; every other binding it adds takes `()`. The remaining + `pmacs.process.spawn` fields (`label`, `command`, `args`, `env`, + `cwd`) still convert generically — **pre-existing, untouched, and + named here rather than silently inherited.** + - **P2b, half one: handler names are refused at the source.** + `pmacs.workers.register` type-checked and nothing more, which was + fine while the name died inside `dispatch`. It no longer dies there, + so the name now gets `purpose`'s meaningful-value standard plus + control characters. + - **P2b, half two: purposes are ESCAPED at presentation, not rejected + at the registry — consistent with the `#228` decision.** A purpose + may legitimately contain a newline (a path can; `pmacs-magit`'s spawn + purpose is an argv), so the one-line constraint belongs to the + surface that has one row. `purpose_for_one_row` states the property + it exists for — **a row must not be able to forge another row** — + escapes the Unicode `Cc` class (so ESC cannot open a terminal + sequence either), borrows unchanged when there is nothing to escape + (byte-identity is structural, not asserted), and does **not** escape + backslashes: no number of them makes a second row, and doubling them + would cost byte-identity for ordinary text. Two callers: the + `*workers*` rows and `ActivitySummary`, which exists for one consumer + with exactly one row. `pmacs.workers.snapshot()` is the + `describe-command` of this lane and stays raw — asserted, so a clip + that deleted the text everywhere would fail rather than pass. + - **P3: two stale recovery summaries**, both fixed section-locally — + the framing doc's "Implementation may proceed", and this file's claim + that Stage 1 took the "first two" of owner/purpose/parent. It takes + **one**: `owner` was removed in revision 2, and the claim that + argument overturned was still standing here. + - **Seven more mutation checks, each failing its own test and no + other** (30 for the lane): the two UTF-8 diagnostics, the two + register guards, the two escaping call sites, and + `purpose_for_one_row` neutered to the identity — which fails both + surfaces' tests and nothing else, since it is the shared helper. + - **All 13 gate steps green at `6661125`** (log + `20260809T173314Z-1552101`): lib 1920, lib-crdt 2105, + worker_identity 24, journey **47/47 UNTOUCHED**, statusline 7, + compile_mode 73, m8_6 12, m4 151, gpu 242. The three + `#pmacs.process.list()` leak detectors and `journey_acceptance` are + **byte-identical to `main`** in round 2 — the stop signals did not + fire, and round 2 edited no test outside its own suite. **The + preceding run of the same command was red on three tests and none of + them was this diff's** — R7 for the third time plus two wall-clock + budget tests; recorded in `docs/ci-red-signatures.md` rather than + re-run away silently. - **Surfaces that changed shape, for anyone rebasing onto this:** `AsyncRuntime::allocate`/`allocate_with_resource` collapsed into one private `JobSpec`-taking funnel; `register_external` grew a third diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index c94c73b..f54fb48 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -496,10 +496,10 @@ Stage 4; the lane touches no `pmacs-gpu` code at all. | **selector** | `-p pmacs-gpu attach::tests::managed_retry_survives_transients_and_uses_the_successful_stream` | | **job / flavor** | local (Linux), `cargo test --workspace --features crdt --no-fail-fast`, i.e. under full-sweep load | | **required fragments** | `transient sequence must attach` + `Handshake(Io(` + `BrokenPipe` (or `code: 32`) | -| **status** | **SECOND OCCURRENCE 2026-08-09 — causal status still UNRESOLVED** | -| **what IS established** | **two** occurrences at `pmacs-gpu/src/attach.rs:1680`, the second with all three fragments **verified** rather than inferred; the test drives a scripted transient-then-success sequence over a real socket pair | +| **status** | **THIRD OCCURRENCE 2026-08-09 — causal status still UNRESOLVED, but one candidate mechanism is now EXCLUDED** | +| **what IS established** | **three** occurrences at `pmacs-gpu/src/attach.rs:1680`, the second and third with all three fragments **verified** rather than inferred; the test drives a scripted transient-then-success sequence over a real socket pair. **The added GPU test is not the mechanism** — see the third-occurrence control below | | **what is NOT** | whether the broken pipe is the *fixture's* writer closing early or a real retry-path defect. **This row is not a claim that it is harmless** | -| **rerun evidence** | occurrence 1: 6 isolated runs green, plus a full `--workspace --features crdt` sweep green (113 targets). Occurrence 2: **30 green on the observing branch** (15 isolated selector, 15 full `-p pmacs-gpu`) **plus a 15-run merge-base control, also green**. Per the rerun rule all of this establishes **intermittence only** | +| **rerun evidence** | occurrence 1: 6 isolated runs green, plus a full `--workspace --features crdt` sweep green (113 targets). Occurrence 2: **30 green on the observing branch** (15 isolated selector, 15 full `-p pmacs-gpu`) **plus a 15-run merge-base control, also green**. Occurrence 3: 5 isolated selector runs green, 10 full `-p pmacs-gpu` runs green **with** the added test, and **1 failure in 10 with the added test `#[ignore]`d** — the first rerun in this row's history that reproduced anything. Per the rerun rule the green runs establish intermittence only; the red control run is what carries the exclusion | | **retirement** | hardening that removes the named mechanism plus a discriminating witness — or a diagnosis showing the fixture, not the code, closes the pipe | **Not attributed to the observing lane**, and in neither case is the @@ -537,6 +537,39 @@ one-second deadline. Contention is a plausible mechanism for a lands, **run the control with the added test removed** rather than at the merge base — that is the discriminating comparison this one was not. +**Third occurrence — worker identity Stage 1 review round 2, +2026-08-09, local (Linux). Same selector, same `gpu`-step flavor, all +three fragments verified** against the durable gate log +(`20260809T172606Z-1387979/11-gpu.log`): `transient sequence must +attach: Attach(Handshake(Io(Os { code: 32, kind: BrokenPipe, message: +"Broken pipe" })))`. A match on this file's own rule, not a `U` note. + +**The control the second-occurrence note prescribed was run, and this +time it discriminated — against the hypothesis.** Ten full +`PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu` runs with the added +`render_offscreen` test present: **10/10 green**. Ten more with that +test `#[ignore]`d, changing nothing else: **1 failure in 10**, carrying +all three required fragments +(`without/run-6.log`, `pmacs-gpu/src/attach.rs:1680`). + +So the concurrent-GPU-test path named above is **excluded**: removing +the suspect made the failure *more* frequent, not less, which no +contention story from that test survives. What the run does establish is +that **the failure reproduces on demand at roughly 1-in-10 under +ordinary `-p pmacs-gpu` load** — the first time any rerun in this row's +history has reproduced it at all. That is a materially better starting +point than three isolated sightings, and it is the fact a diagnosis +should be built on: the rate makes a bisect of `attach.rs`'s handshake +path affordable, where before it was not. + +**It is still not attributed to the observing lane**, and now for a +measured reason rather than an argument from diff shape: the arm without +the lane's only `pmacs-gpu` addition is the arm that went red. + +**What would retire it is unchanged** — the mechanism, not the rate. +The next agent to touch this row should reproduce at 1-in-10 and +instrument which side closes the pipe, rather than re-running for green. + ### U2 — `m6_1_pty_raw_mode_disables_kernel_echo`, one local occurrence Has a selector, which U1 lacks — but still no fragments, so it cannot @@ -582,6 +615,30 @@ it again here by piping a sweep through `grep`. The fix is mechanical: stream. A signature that is cheap to capture and impossible to reconstruct should never be traded for terminal brevity. +### U4 — two wall-clock budget tests fail together in one `lib-crdt` step + +Recorded during worker identity Stage 1 review round 2, 2026-08-09, in +the same gate run that produced R7's third occurrence. **Fragments were +captured**, so unlike U1–U3 this one is matchable — it is a `U` row +because it has one occurrence and no mechanism, not because the evidence +was lost. + +| field | value | +|---|---| +| **selector** | `--lib --features crdt optimistic::tests::criterion_1_end_of_line_typing_completes_sub_frame_per_keystroke` **and** `editor::tests::composition_overhead_under_ten_percent`, failing in the same run | +| **job / flavor** | local (Linux), `scripts/gate` step `04-lib-crdt`, with sibling worktrees building concurrently | +| **required fragments** | `criterion 1: per-keystroke orchestrator time` + `exceeds 1ms`; and `composition machinery added more than 10% overhead` | +| **status** | **new incident, one occurrence, not reproduced** | +| **what IS established** | both are **wall-clock budget assertions** — 1.264ms against a 1ms budget, and 1.297× against a 1.10× budget — so both are load-sensitive by construction. Both green in an isolated rerun of exactly those two selectors, and both green in the next full gate run of the same command (2105 passed) | +| **what is NOT** | whether the machine's concurrent load caused it. The confound is real (this machine runs one shared `CARGO_TARGET_DIR` and several worktrees) but **was not measured**, so it is a rival explanation, not a finding | +| **rival explanation not excluded** | a genuine regression in either path. Nothing in the observing diff touches the optimistic-echo orchestrator or the composition pipeline, but "my diff looks unrelated" is not evidence, and this row does not treat it as such | + +**Two budget tests failing in one run and neither in the next is the +signature worth matching**, more than either name alone: a real +regression in two unrelated subsystems at once is far less likely than +one loaded machine. If a future run reds **one** of these without the +other, that is a different incident and should be judged as one. + **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.