From 027c7d18e079c0abedcd90b9b0c995aa7f64c66e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 15:58:34 -0400 Subject: [PATCH] test(lsp): name acc32 for what it pins; label the unpinned guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections found by bite-testing the suite rather than by reading it. **Acceptance 32 was mis-named.** It claimed to pin "the one-shot is removed BEFORE invocation". Biting that — moving the removal after the `pcall` — still passes, because `pcall` catches the raise either way and the removal runs regardless. The before/after ordering is unobservable unless a handler re-enters the drain, and nothing does. What the test actually pins is that removal is **unconditional**: the bite that gates it on `if ok then` fails 2 != 1, because the surviving registration is invoked a second time by the purge. Renamed and re-commented to say so. The implementation still removes before invoking, which is the right defensive order; it is simply not what the assertion proves. **The purge's generation check is defensive and untested**, now labelled in place instead of reading as covered. Reaching it needs a crash and its restart to both land in a gap with no `_async.tick`; the backoff is 500ms, so any tick in that window sees `crashed` and the absent-or-terminal test fires first. Every attempt to stage it deterministically exercised the `crashed` path instead. It stays as insurance for a stalled editor, and says that about itself. Bites recorded, all against the committed tree: - removal gated on a clean return -> acc32 fails (2 != 1). - purge driven by a `crashed`/`stopped` event seen in the drain, the design the framing originally implied -> the no-attachment case fails ("never called"), while the attached case still passes. That is the discrimination acceptance 34's second half exists for. - a resolver without `pmacs.fs.canonicalize` -> two servers, pinned as 34b's own falsification. --- builtin/runtime/lsp.lua | 12 ++++++++++++ tests/lsp_dispatch_seams_acceptance.rs | 22 ++++++++++++++-------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index eac5753..6a837f6 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -1683,6 +1683,18 @@ local function purge_dead_pending() for rid, entry in pairs(pend) do -- Absent or terminal, or the same sid running a NEW generation: -- in every case the request this entry awaits is unanswerable. + -- + -- The generation half is **defensive and not covered by the + -- acceptance suite**, stated plainly rather than left to look + -- tested. Reaching it requires a crash and its restart to both + -- fall inside a gap with no `_async.tick` — the crash backoff is + -- 500ms (`src/lsp.rs:1007`), so any tick during that window sees + -- `crashed` and the absent-or-terminal test above fires first. A + -- stalled or idle editor can produce such a gap, and then this is + -- the only thing standing between a one-shot and waiting forever + -- on a reply the dead generation owed. Every attempt to stage it + -- deterministically ended up exercising the `crashed` path + -- instead, so it is kept as insurance and labelled as such. if attempt == nil or attempt ~= entry.attempt then dead[#dead + 1] = rid end diff --git a/tests/lsp_dispatch_seams_acceptance.rs b/tests/lsp_dispatch_seams_acceptance.rs index d240ba4..cb9acff 100644 --- a/tests/lsp_dispatch_seams_acceptance.rs +++ b/tests/lsp_dispatch_seams_acceptance.rs @@ -336,16 +336,22 @@ fn acc33_raising_response_handler_does_not_stop_the_drain() { } // --------------------------------------------------------------------------- -// Acceptance 32 — the one-shot is removed BEFORE invocation. +// Acceptance 32 — the one-shot is removed exactly once, whether or not +// the handler raises. // -// Observed rather than asserted structurally: the handler raises, and -// the server is then stopped. If removal happened only on a clean -// return — or not at all — the purge below would invoke the same handler -// a second time with an error. The count is what pins it. +// Named for what it pins rather than for the framing's wording. Q#LN9 +// specifies removal *before* invocation, and the implementation does +// that — but bite-testing showed the before/after ordering is not +// observable on its own: `pcall` catches the raise either way, so +// removal after the call is behaviorally identical unless a handler +// re-enters the drain, which nothing does. What IS observable, and what +// this pins, is that removal is **unconditional**: the bite that moves +// it inside `if ok then` fails here 2 != 1, because the surviving +// registration gets invoked a second time by the purge. // --------------------------------------------------------------------------- #[test] -fn acc32_response_one_shot_is_removed_before_invocation() { +fn acc32_response_one_shot_is_removed_even_when_the_handler_raises() { let fx = Fixture::new(); let mut state = editor(); attached_rust(&mut state, &fx); @@ -375,8 +381,8 @@ fn acc32_response_one_shot_is_removed_before_invocation() { assert_eq!( eval::(&state, "return _G.calls"), 1, - "a delivered one-shot must not be re-invoked by the purge — it \ - was removed before the raising handler ran, not after" + "a delivered one-shot must not be re-invoked by the purge — \ + removal is unconditional, not gated on a clean return" ); }