From bf1a0ac1be9f32e8c011ea28d8f5d39f3d4b0b72 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 11 Aug 2026 17:18:47 +0200 Subject: [PATCH] fix(tests): D3 round three --- two witness overclaims, and the CI red was a core-count assumption Review round three on PR #235, plus the diagnosis of its first CI run: all five test legs failed on one test, deterministically, while sixteen local cores stayed green. The mid-walk cancellation bound could not bite the per-entry poll alone. The cancel lands two files into a 41-file directory --- root contributes three dir entries --- so with the per-entry poll deleted the directory finishes and the per-DIRECTORY poll catches at seen == 44, under the old bound of 60. The bound is now 40 against an expected exactly-35 (3 + one 32-entry poll stride), and the entry-poll-only bite goes red at 44. Verified both ways. The retirement helper observed a REQUEST, not settlement: it returned as soon as an active row showed cancel_requested, which a worker that ignored the token and completed successfully would satisfy. It now waits for a completed row with status == "cancelled", making the lane's "settles cancelled" claim true at the witness, not just at the Rust layer. The CI red: d3_pump(1600) between the mid-walk join and the late.bbb write assumed the held walk would complete within 1.6 s. On a 3-thread CI pool, 8 sleeps of 1200 ms drain in ~3.6 s of waves, so the file landed before the held walk even STARTED and folded into the joiner's baseline --- exactly the fold the test exists to assert for mid.bbb, applied to the wrong file. Deterministic on every 2-4-core runner, invisible on 16 cores. The drain is now an observable condition --- at least one post-join walk completed and none active --- with the saturation sleeps at 800 ms, and the three saturation tests plus the whole eighteen-test family re-run green under taskset -c 0-3, the CI pool shape reproduced locally. A fixed-duration pump against pool-dependent timing is a core-count assumption in disguise; the lane records it as such. Superseded round-one text in the lane (the fallback "unreachability" claim round two disproved) is corrected in place. One gate run also hit the live attach-retry BrokenPipe row --- fourth occurrence, all three required fragments verified against the durable sweep log, recorded in docs/ci-red-signatures.md. This lane touches no pmacs-gpu code, no wire, and no protocol; the same sweep passed twice earlier the same day on materially the same tree. The retirement bar (mechanism, not rate) is unchanged. Co-Authored-By: Claude Fable 5 --- docs/active-work.md | 18 +++++++++++ docs/ci-red-signatures.md | 17 +++++++++++ src/fs.rs | 16 ++++++---- tests/m4_acceptance.rs | 64 ++++++++++++++++++++++++++++----------- 4 files changed, 92 insertions(+), 23 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 4b500b7..8b3f4cd 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -268,6 +268,24 @@ implementation-time facts worth keeping: either copy alone is masked by the other; only biting both goes red. The sweep exists for seam-cancelled members, the unregister path for idle groups whose next scan may be seconds away. +- **Round three, post-PR: two witness overclaims, and the PR's first + CI red — my own fixed-duration pump.** The mid-walk bound (60) could + not tell a deleted per-entry poll from the real code — the cancel + lands two files into a 41-file directory, so the per-DIRECTORY poll + stops a poll-less walk at 44; the bound is now 40 against an + expected exactly-35, and the entry-poll-only bite goes red at 44. + The retirement helper accepted `cancel_requested` on an active row — + a request, not settlement; it now waits for a `cancelled` + COMPLETION. And all five CI test legs failed deterministically where + sixteen local cores stayed green: `d3_pump(1600)` wrote the + discriminating file before the held walk even STARTED on a + 3-thread pool (8×1200 ms sleeps drain in ~3.6 s of waves), folding + it into the baseline. The drain is now an observable condition + (a post-join walk completed and none active), the sleeps are 800 ms, + and the three saturation tests plus the whole family were re-run + green under `taskset -c 0-3` — the CI pool shape, reproduced + locally. **A fixed-duration pump against pool-dependent timing is a + core-count assumption in disguise.** - **A second pre-commit round found three more** (implementation review, not framing): mid-walk cancellation was unwitnessed — both Rust cancel tests pre-cancelled and the acceptance test cancelled a diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 414be2f..49b00b3 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -570,6 +570,23 @@ the lane's only `pmacs-gpu` addition is the arm that went red. 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. +**Fourth occurrence — D3 file-watch scheduler (PR #235), 2026-08-11, +local (Linux), at the gate's SWEEP step** (`cargo test --workspace +--no-fail-fast`, default features — U3's flavor, this time with the +fragments captured). All three required fragments verified against the +durable gate log +(`pmacs-fdccc423/gate-logs/20260811T150651Z-1481359/08-sweep.log`): +`transient sequence must attach: Attach(Handshake(Io(Os { code: 32, +kind: BrokenPipe, message: "Broken pipe" })))`, `attach.rs:1680`. +242/243 in the target; the same sweep had passed twice earlier the same +day on materially the same tree (the diff between runs was a test file +and docs — **no `pmacs-gpu` code, no wire, no protocol**, the +strongest non-attribution shape this row has had). Ambient context, +recorded not asserted: load average ~5.2 and four leaked +`pmacs --daemon` processes resident. Consistent with the established +~1-in-10-under-load rate; adds no new mechanism evidence. The +retirement bar is unchanged. + ### U2 — `m6_1_pty_raw_mode_disables_kernel_echo`, THIRD known occurrence **Corrected 2026-08-09 after review.** A previous edit of this row diff --git a/src/fs.rs b/src/fs.rs index 6df4f7c..7c1d48a 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -1214,13 +1214,17 @@ mod tests { matches!(result, Err(FsError::Cancelled)), "mid-walk cancel must surface as Cancelled" ); - // 126 entries total (3 dirs + 123 files). The next entry poll - // after the cancel at entry 5 is at most one - // READDIR_CANCEL_POLL_EVERY stride away. + // 126 entries total (3 dirs + 123 files). The cancel lands at + // entry 5 (root's three dir entries, then two files), so the + // per-entry poll stops the walk at exactly 35 = 3 + 32 (one + // READDIR_CANCEL_POLL_EVERY stride). The bound sits BELOW 44: + // with the per-entry poll deleted, the 41-file directory runs + // to completion and the per-directory poll catches at 44 --- a + // bound of 60 could not tell the two apart (review round 3). assert!( - seen.get() < 60, - "the walk must stop near the mid-walk cancel, not run the \ - whole tree ({} of 126 entries processed)", + seen.get() < 40, + "the walk must stop within one poll stride of the cancel \ + ({} of 126 entries processed; expected 35)", seen.get() ); } diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index 093d0b6..58f6930 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -5800,6 +5800,24 @@ fn d3_walk_job_count(state: &pmacs::editor::EditorState) -> i64 { .expect("walk-count probe must not error") } +/// Number of `fs_walk_tree` jobs currently ACTIVE (running or queued). +fn d3_active_walk_count(state: &pmacs::editor::EditorState) -> i64 { + state + .lua_host + .lua() + .load( + "(function() + local n = 0 + for _, r in ipairs(pmacs.workers.snapshot().active) do + if r.kind == 'fs_walk_tree' then n = n + 1 end + end + return n + end)()", + ) + .eval() + .expect("active-walk probe must not error") +} + /// Pump until at least one `fs_walk_tree` job is active (typically /// queued behind a saturated pool). Panics on timeout. fn d3_wait_for_active_walk(state: &mut pmacs::editor::EditorState) { @@ -5833,11 +5851,13 @@ fn d3_wait_for_active_walk(state: &mut pmacs::editor::EditorState) { } } -/// Pump until some `fs_walk_tree` job is observed cancelled — either -/// `cancel_requested` while active or a `cancelled` completion in the -/// ring. Panics on timeout. +/// Pump until some `fs_walk_tree` job has SETTLED cancelled — a +/// `cancelled` completion in the ring, not merely `cancel_requested` +/// on an active row (review round 3: a worker that ignored the token +/// and completed successfully would satisfy a request-level check). +/// Panics on timeout. fn d3_wait_for_walk_cancelled(state: &mut pmacs::editor::EditorState) { - let deadline = Instant::now() + Duration::from_secs(4); + let deadline = Instant::now() + Duration::from_secs(6); loop { state.tick_processes(); state.tick_lsp(); @@ -5847,13 +5867,7 @@ fn d3_wait_for_walk_cancelled(state: &mut pmacs::editor::EditorState) { .lua() .load( "(function() - local snap = pmacs.workers.snapshot() - for _, r in ipairs(snap.active) do - if r.kind == 'fs_walk_tree' and r.cancel_requested then - return true - end - end - for _, r in ipairs(snap.completed) do + for _, r in ipairs(pmacs.workers.snapshot().completed) do if r.kind == 'fs_walk_tree' and r.status == 'cancelled' then return true end @@ -6116,7 +6130,7 @@ fn m4_24_d3_join_mid_walk_queues_one_immediate_baseline() { .lua_host .lua() .load(format!( - "for _ = 1, {} do pmacs.workers.sleep(1200) end", + "for _ = 1, {} do pmacs.workers.sleep(800) end", std::thread::available_parallelism().map_or(8, std::num::NonZeroUsize::get) + 4 )) .exec() @@ -6138,9 +6152,25 @@ fn m4_24_d3_join_mid_walk_queues_one_immediate_baseline() { .expect("edit to trigger join"); std::fs::write(watch.join("mid.bbb"), b"m\n").expect("write mid.bbb"); - // Drain: sleeps expire, the queued walk runs, the follow-up - // baselines the joiner, and a later file is delivered to it. - d3_pump(&mut state, 1600); + // Drain DETERMINISTICALLY, not for a fixed duration: the held + // walk must complete and a post-join walk (the queued baseline) + // must have settled before `late.bbb` exists. On a small CI pool + // the sleep waves take seconds, and a fixed 1.6 s pump wrote the + // file before the held walk even started — folding it into the + // baseline. That was PR #235's first CI red: deterministic on + // every 2–4-core runner, invisible on sixteen local cores. + let walks_at_join = d3_walk_job_count(&state); + let deadline = Instant::now() + Duration::from_secs(15); + loop { + d3_pump(&mut state, 100); + if d3_walk_job_count(&state) > walks_at_join && d3_active_walk_count(&state) == 0 { + break; + } + assert!( + Instant::now() < deadline, + "the queued baseline walk never settled" + ); + } let late_uri = format!("file://{}", watch.join("late.bbb").display()); std::fs::write(watch.join("late.bbb"), b"l\n").expect("write late.bbb"); assert!( @@ -6230,7 +6260,7 @@ fn m4_24_d3_retirement_stops_scans_and_a_fresh_group_rebaselines() { .lua_host .lua() .load(format!( - "for _ = 1, {} do pmacs.workers.sleep(1200) end", + "for _ = 1, {} do pmacs.workers.sleep(800) end", std::thread::available_parallelism().map_or(8, std::num::NonZeroUsize::get) + 4 )) .exec() @@ -6308,7 +6338,7 @@ fn m4_24_d3_live_cancel_preserves_snapshot_and_cadence() { .lua_host .lua() .load(format!( - "for _ = 1, {} do pmacs.workers.sleep(1200) end", + "for _ = 1, {} do pmacs.workers.sleep(800) end", std::thread::available_parallelism().map_or(8, std::num::NonZeroUsize::get) + 4 )) .exec()