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.