Merge pull request #194 from levineuwirth/silent-skip-arming

test: arm the silent skips, so external-tool tests stop passing vacuously

.github/workflows/ci.yml

@@ -85,12 +85,69 @@ jobs:
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
       - uses: Swatinem/rust-cache@v2
+      # Arm the external-tool-gated tests (see `TEST_IMPROVEMENT.md`
+      # §1.2). Before this step nothing installed these tools, so every
+      # test guarded on them returned early and reported GREEN without
+      # executing its body — a whole block of real-language-server and
+      # multi-shell coverage that had never once run in CI. Installing
+      # them is only half the fix; the `PMACS_REQUIRE_*` variables below
+      # are what turn a future missing tool back into a failure instead
+      # of silently restoring the vacuum.
+      #
+      # Linux only for now, deliberately. macOS would need the brew
+      # equivalents and roughly doubles the install cost on the slowest
+      # leg of the matrix; arming one platform already converts these
+      # from never-executed to executed, and the second is incremental.
+      # The tests still skip cleanly on macOS because the variables are
+      # unset there.
+      - name: Install external tools that gate acceptance tests (Linux)
+        if: runner.os == 'Linux'
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y clangd zsh fish lua5.4
+          # `locate_lua` looks for `lua` or `luajit` by name; the
+          # distro package installs `lua5.4` only.
+          sudo ln -sf "$(command -v lua5.4)" /usr/local/bin/lua
+          # rust-analyzer belongs HERE, not on the shared toolchain
+          # step. `components:` there applies to every matrix leg, so it
+          # would install the binary on macOS too — and *presence*, not
+          # PMACS_REQUIRE_LSP, is what decides whether a gated test body
+          # runs. That would have executed the rust-analyzer tests on
+          # macOS for the first time ever, on the legs that are both the
+          # CI critical path and the documented flake surface, while
+          # this lane's text claimed Linux only.
+          rustup component add rust-analyzer
+          # Versions are PINNED. `@latest` and bare `npm install -g`
+          # make CI behaviour drift with upstream releases: a bad gopls
+          # or yaml-language-server publish then breaks CI with no
+          # commit in this repository to bisect against.
+          go install golang.org/x/tools/gopls@v0.16.2
+          echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH"
+          npm install -g vscode-langservers-extracted@4.10.0 \
+                         yaml-language-server@1.15.0
       - run: cargo build --all-targets --no-default-features --features ${{ matrix.lua }}
       # Several acceptance binaries spawn real daemon / PTY child
       # processes. Keep the harness serial so macOS runners do not
       # expose cross-test process lifecycle races that are unrelated
       # to the behavior under test.
+      #
+      # PMACS_REQUIRE_* make a missing tool fatal rather than a silent
+      # skip, exactly as PMACS_REQUIRE_GPU already does for the headless
+      # render job. Set only where the install step ran.
+      #
+      # PMACS_REQUIRE_PYRIGHT is deliberately NOT set and basedpyright
+      # is deliberately NOT installed: that test has no timeout and
+      # hangs forever (root cause is the non-interruptible reader-thread
+      # join in `RuntimeHandles::drop`, already a named deferral in
+      # `src/process.rs`). This job has no `timeout-minutes`, so arming
+      # it today would trade a vacuous green for a six-hour hang on four
+      # legs. It gets armed after the hang fix and the CI timeouts land,
+      # and its own variable exists so that flip is one line.
       - run: cargo test --all-targets --no-default-features --features ${{ matrix.lua }} -- --test-threads=1
+        env:
+          PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }}
+          PMACS_REQUIRE_SHELLS: ${{ runner.os == 'Linux' && '1' || '' }}
+          PMACS_REQUIRE_LUA: ${{ runner.os == 'Linux' && '1' || '' }}
       - run: cargo test --doc --no-default-features --features ${{ matrix.lua }}
       # The workspace default member is only the root `pmacs` package, so
       # the runs above never execute pmacs-protocol's own tests — the

docs/active-work.md

@@ -717,6 +717,109 @@ has **no branch and no framing yet**.
   warns against quoting a stale figure; it does not replace that
   section's per-target census, which was not re-derived.
 
+## Test-improvement arc, lane 2 — silent-skip arming
+
+- Portable branch: `githubsucks/silent-skip-arming`, worktree
+  `../pmacs-skiparm`. Implements `TEST_IMPROVEMENT.md` §1.2 and §5.4.
+- **Base, measured at write time rather than quoted:**
+
+  ```
+  $ git log --oneline -1 githubsucks/main
+  5e186c7 Merge pull request #193 from levineuwirth/test-improvement-audit
+  ```
+
+  The previous revision of this entry said "base measured at write
+  time, pasted below" and then pasted nothing: the script meant to
+  substitute it reported success and silently matched no text, and the
+  claim was not re-read. Recorded because it is the same defect this
+  ledger keeps catching one level up — **asserting a measurement is not
+  making one, and a tool reporting success is not the measurement
+  either.**
+- Recovery from a clean checkout:
+  `git fetch githubsucks && git worktree add ../pmacs-skiparm
+  -b silent-skip-arming githubsucks/silent-skip-arming`.
+- **The defect:** `let Ok(_) = which_binary(x) else { eprintln!(..);
+  return; }` reports GREEN when the tool is absent, and CI installed
+  none of the tools. A block of real-language-server and multi-shell
+  tests had therefore **never once executed their bodies** in CI while
+  reporting success. A suite that cannot distinguish "passed" from
+  "never ran" is worse than a missing one, because it reads as
+  coverage.
+- **The fix is the project's own pattern.** `PMACS_REQUIRE_*` already
+  makes a missing GPU fatal for `vterm_stage3_acceptance`; this adds
+  `PMACS_REQUIRE_LSP`, `PMACS_REQUIRE_SHELLS` and `PMACS_REQUIRE_LUA`,
+  plus the CI step that installs the tools. Per-tool variables, not one
+  blanket flag, so a tool that must stay unarmed keeps its decision
+  visible at the call site.
+- **`basedpyright` is deliberately NOT installed and NOT armed.** Its
+  test has no timeout and hangs forever — root cause is the
+  non-interruptible reader-thread join in `RuntimeHandles::drop`,
+  already a named deferral in `src/process.rs`. The `test` job has no
+  `timeout-minutes` either. Arming it today would trade a vacuous green
+  for a six-hour hang across four legs. `PMACS_REQUIRE_PYRIGHT` exists
+  and is never set, so the flip is one line once lane 4 (the hang) and
+  lane 3 (timeouts) land. **Do not arm it before both.**
+- **A trap found while writing the workflow, not after:** the natural
+  Actions idiom `${{ runner.os == 'Linux' && '1' || '' }}` sets the
+  variable to the EMPTY STRING elsewhere, and `var_os().is_some()` is
+  true for `Some("")`. That would have armed the guard on exactly the
+  runners with no tools installed. The helper therefore treats empty as
+  unset. `PMACS_REQUIRE_GPU` has the same latent shape and is safe only
+  because it is set literally.
+- **Verified by execution in all three states**, on a tool genuinely
+  absent from this machine (`vscode-json-language-server`): unset ->
+  skips green; armed -> hard failure naming the CI step; empty string
+  -> skips green. The armed failure is the bite, and on `main` it
+  cannot occur because no guard exists.
+- **The tests pass when they actually run** — which was the open
+  question, since none of them had. Armed locally: 11 `m6_5` + 8 `m6_8`
+  REPL tests green, and all six real-LSP tests (clangd x2, gopls x2,
+  rust-analyzer x2) green individually.
+- **rust-analyzer is installed in the Linux-gated step, not via the
+  toolchain action's `components:`.** The first revision put it there,
+  which applies to *every* matrix leg — and **presence, not
+  `PMACS_REQUIRE_LSP`, is what decides whether a gated test body
+  runs**. That would have executed the two rust-analyzer tests on macOS
+  for the first time ever, on the legs that are simultaneously the CI
+  critical path and the documented flake surface, while this entry
+  claimed Linux only. The variables not being set there would only have
+  meant absence was tolerated; it would not have kept the tests
+  skipped. Text and workflow now agree.
+- **Tool versions are pinned** (`gopls@v0.16.2`,
+  `vscode-langservers-extracted@4.10.0`,
+  `yaml-language-server@1.15.0`). `@latest` and bare `npm install -g`
+  make CI drift with upstream releases, so a bad publish breaks CI with
+  no commit here to bisect against. Caching the built `gopls` on the
+  pinned version is a follow-up, not done here.
+- **§1.2 is NOT fully closed by this lane.** The guards arm the
+  *entry* skip only. `tests/m4_acceptance.rs`'s mid-test rust-analyzer
+  bail ("workspace likely still indexing; skipping") survives, so even
+  armed, that test's only assertion can still vanish under load —
+  precisely when a regression would show. Mid-test skips are their own
+  shape and want their own pass.
+- **Not this lane's to fix, recorded so it is not mistaken for
+  oversight:** the generated-buffer immutability lane above still reads
+  "PR #188 OPEN, PROPOSED" and #188 has merged. Rule 4 forbids
+  relabelling it and permits removal only once its durable facts reach
+  `docs/agent-handoff.md`, which #188 did not touch — it changed the
+  framing and this ledger only. So the absorption is genuinely owed,
+  and the natural carrier is the arc's own next PR (#191, Stage 1),
+  not a testing lane reaching across into someone else's arc.
+- **Follow-up owed after this merges:** delete
+  `githubsucks/handoff-2026-07-20`. Removing the documentation lane
+  removes the only pointer to that branch, so nothing will otherwise
+  remind anyone it still exists on the remote.
+- Linux only for now, deliberately: macOS needs the brew equivalents
+  and roughly doubles install cost on the slowest matrix leg. The
+  variables stay unset there, so those tests skip cleanly.
+- Also removes the **documentation lane**, whose disposition the ledger
+  left undecided pending confirmation that its branch carried nothing
+  unique. Confirmed by measurement: `githubsucks/handoff-2026-07-20` is
+  **1 ahead, 365 behind**, and its entire unique diff is four doc files
+  at 42 insertions against 88 deletions — merging it would *revert*
+  current documentation. The section said "whoever confirms the branch
+  carries nothing unique removes the section"; this is that.
+
 ## Parked lane: kill-ring browser + persistence
 
 - Portable branch: `githubsucks/kill-ring-browser`
@@ -739,32 +842,6 @@ git worktree add --track \
   githubsucks/kill-ring-browser
 ```
 
-## Documentation lane — STALE, AND ITS DISPOSITION IS UNDECIDED
-
-> **Measured 2026-07-28, not inferred:** `githubsucks/handoff-2026-07-20`
-> is at `c11d7e7`, **1 commit ahead of `main` and 320 behind**. Its
-> whole diff against `main` is four documentation files
-> (`docs/active-work.md`, `docs/agent-handoff.md`,
-> `docs/roadmap-2026-07.md`, `docs/vterm-framing.md`), every one of
-> which has been rewritten repeatedly since by the landed-doc PRs
-> #156/#168/#169/#172/#180. Rule 4 removes a lane on merge *or
-> abandonment*, and this one looks abandoned in substance — but "looks
-> abandoned" is not the same as a decision, and no PR was ever opened
-> for it. **This snapshot deliberately annotates rather than deletes:
-> whoever confirms the branch carries nothing unique removes the
-> section.** The bullets below are its original claims, preserved as
-> written and now unverified.
-
-- Portable branch: `githubsucks/handoff-2026-07-20`
-- Carries synchronized `AGENTS.md` / `CLAUDE.md`, this ledger, the
-  durable handoff refresh, and the keybinding reference correction.
-- It changes no runtime code.
-- Review and merge this documentation branch separately; it must not be
-  folded into a feature framing branch.
-- Now also absorbs both landed arcs: Vterm Stage 1 (#126) and the config
-  registry (#127). Canonical `main` is merged into it up to `2e37c04`,
-  so its diff against `main` is documentation only.
-
 ## Closed since the last snapshot
 
 - **Terminal configuration + copy mode arc — BOTH STAGES MERGED, lane

docs/agent-handoff.md

@@ -1443,6 +1443,22 @@ round-trip cannot detect a discriminant shift.
 
 ## 5. Hard-won ops lessons
 
+- **A gate summary assembled through a pipe can report success over a
+  failure.** `cmd | tail -2` returns **`tail`'s** exit status, not
+  `cmd`'s — in `fish` and `bash` alike — so a chain of
+  `cargo test ... | tail -2 && cargo test ... | tail -2 && echo "ALL
+  GATES CLEAN"` prints the clean line even when a suite failed. This
+  is not carelessness that closer reading would catch: the failure is
+  **structurally invisible** in the summary the PR then cites. It
+  happened while gating the silent-skip lane, and a `pmacs-gpu`
+  failure was reported as clean.
+
+  Either check `$pipestatus[1]` in fish (`${PIPESTATUS[0]}` in bash),
+  or — better — redirect each gate to a file and read the file
+  afterwards, which also preserves the full log this section already
+  asks you to keep. Same family as the skip-reports-`ok` lesson below
+  and the double-invocation traps: **the thing that summarizes a gate
+  must not be able to lose the gate's verdict.**
 - **A test that skips on a missing precondition reports `ok`, and a gate log
   cannot tell that apart from a pass.** `vterm_stage3_acceptance::a37` — the
   only acceptance driving a real daemon, a real PTY and a real wgpu render

tests/m4_acceptance.rs

@@ -1298,7 +1298,7 @@ fn fake_spec(label: &str) -> LspServerSpec {
 #[test]
 fn m4_5_rust_analyzer_initializes() {
     let Ok(_) = which_binary("rust-analyzer") else {
-        eprintln!("rust-analyzer not on PATH; skipping");
+        support::skip_or_fail("rust-analyzer", "PMACS_REQUIRE_LSP");
         return;
     };
     let (sup, mgr) = make_lsp_test_manager();
@@ -1338,7 +1338,7 @@ fn m4_5_rust_analyzer_initializes() {
 #[test]
 fn m4_5_basedpyright_initializes_and_negotiates_encoding() {
     let Ok(_) = which_binary("basedpyright-langserver") else {
-        eprintln!("basedpyright-langserver not on PATH; skipping");
+        support::skip_or_fail("basedpyright-langserver", "PMACS_REQUIRE_PYRIGHT");
         return;
     };
     let (sup, mgr) = make_lsp_test_manager();
@@ -1419,7 +1419,7 @@ fn assert_lsp_initializes_and_negotiates(
 #[test]
 fn m4_5_clangd_initializes_and_negotiates_encoding() {
     let Ok(_) = which_binary("clangd") else {
-        eprintln!("clangd not on PATH; skipping");
+        support::skip_or_fail("clangd", "PMACS_REQUIRE_LSP");
         return;
     };
     assert_lsp_initializes_and_negotiates("clangd", "cpp", "clangd", &["--background-index"]);
@@ -1431,7 +1431,7 @@ fn m4_5_clangd_initializes_and_negotiates_encoding() {
 #[test]
 fn m4_5_gopls_initializes_and_negotiates_encoding() {
     let Ok(_) = which_binary("gopls") else {
-        eprintln!("gopls not on PATH; skipping");
+        support::skip_or_fail("gopls", "PMACS_REQUIRE_LSP");
         return;
     };
     assert_lsp_initializes_and_negotiates("gopls", "go", "gopls", &[]);
@@ -3428,6 +3428,9 @@ fn m4_11_snippets_surface_through_completion() {
 use pmacs::definition::DefinitionKey;
 use pmacs::formatting::FormattingKey;
 
+#[path = "support/mod.rs"]
+mod support;
+
 /// Acceptance (1/3): a `textDocument/definition` request round-trips
 /// through the manager and lands in the definition store as a parsed
 /// `Location` list.
@@ -5536,7 +5539,7 @@ fn m4_27_real_gopls_analyzes_module_via_auto_attach() {
     use pmacs::editor::EditorState;
 
     let Ok(gopls) = which_binary("gopls") else {
-        eprintln!("gopls not on PATH; skipping");
+        support::skip_or_fail("gopls", "PMACS_REQUIRE_LSP");
         return;
     };
     let gopls = gopls.display().to_string();
@@ -5633,7 +5636,7 @@ fn m4_28_real_clangd_diagnostics_and_semantic_tokens_via_auto_attach() {
     use pmacs::editor::EditorState;
 
     let Ok(clangd) = which_binary("clangd") else {
-        eprintln!("clangd not on PATH; skipping");
+        support::skip_or_fail("clangd", "PMACS_REQUIRE_LSP");
         return;
     };
     let clangd = clangd.display().to_string();
@@ -5731,7 +5734,7 @@ fn m4_29_real_rust_analyzer_inlay_hints_via_auto_attach() {
     use pmacs::editor::EditorState;
 
     let Ok(rust_analyzer) = which_binary("rust-analyzer") else {
-        eprintln!("rust-analyzer not on PATH; skipping");
+        support::skip_or_fail("rust-analyzer", "PMACS_REQUIRE_LSP");
         return;
     };
     let rust_analyzer = rust_analyzer.display().to_string();
@@ -6880,7 +6883,7 @@ fn m4_real_json_provider_receives_config_and_reports_diagnostics() {
     use pmacs::editor::EditorState;
 
     let Ok(command) = which_binary("vscode-json-language-server") else {
-        eprintln!("vscode-json-language-server not on PATH; skipping");
+        support::skip_or_fail("vscode-json-language-server", "PMACS_REQUIRE_LSP");
         return;
     };
     let command = command.display().to_string();
@@ -6949,7 +6952,7 @@ fn m4_real_yaml_provider_pulls_config_and_reports_diagnostics() {
     use pmacs::editor::EditorState;
 
     let Ok(command) = which_binary("yaml-language-server") else {
-        eprintln!("yaml-language-server not on PATH; skipping");
+        support::skip_or_fail("yaml-language-server", "PMACS_REQUIRE_LSP");
         return;
     };
     let command = command.display().to_string();

tests/m6_5_repl_acceptance.rs

@@ -35,6 +35,9 @@ use std::path::PathBuf;
 use std::sync::{Mutex, MutexGuard};
 use std::time::{Duration, Instant};
 
+#[path = "support/mod.rs"]
+mod support;
+
 static PUMP_TEST_LOCK: Mutex<()> = Mutex::new(());
 
 fn pump_test_guard() -> MutexGuard<'static, ()> {
@@ -196,7 +199,7 @@ fn m6_5_ret_submits_input_to_process() {
 #[test]
 fn m6_5_ctrl_d_on_empty_prompt_closes_stdin() {
     let Some(bash) = locate_shell("bash") else {
-        eprintln!("skipping: bash not on PATH (set PMACS_TEST_BASH to override)");
+        support::skip_or_fail_overridable("bash", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_BASH");
         return;
     };
     let setup = format!(
@@ -290,7 +293,7 @@ fn m6_5_ctrl_d_on_nonempty_input_deletes_char_forward() {
 )]
 fn m6_5_ctrl_c_sends_sigint() {
     let Some(sleep) = locate_shell("sleep") else {
-        eprintln!("skipping: sleep not on PATH (set PMACS_TEST_SLEEP to override)");
+        support::skip_or_fail_overridable("sleep", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_SLEEP");
         return;
     };
     let setup = format!(
@@ -344,7 +347,7 @@ fn m6_5_ctrl_c_sends_sigint() {
 )]
 fn m6_5_exit_marker_uses_basename_with_leading_newline() {
     let Some(false_bin) = locate_shell("false") else {
-        eprintln!("skipping: false not on PATH (set PMACS_TEST_FALSE to override)");
+        support::skip_or_fail_overridable("false", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_FALSE");
         return;
     };
     let setup = format!(
@@ -425,7 +428,7 @@ fn run_shell_smoke_test(shell_path: &std::path::Path, argv_extra: &[&str]) {
 #[test]
 fn m6_5_repl_spawns_bash() {
     let Some(bash) = locate_shell("bash") else {
-        eprintln!("skipping: bash not on PATH (set PMACS_TEST_BASH to override)");
+        support::skip_or_fail_overridable("bash", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_BASH");
         return;
     };
     run_shell_smoke_test(&bash, &["-i"]);
@@ -435,7 +438,7 @@ fn m6_5_repl_spawns_bash() {
 #[test]
 fn m6_5_repl_spawns_zsh() {
     let Some(zsh) = locate_shell("zsh") else {
-        eprintln!("skipping: zsh not on PATH (set PMACS_TEST_ZSH to override)");
+        support::skip_or_fail_overridable("zsh", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_ZSH");
         return;
     };
     run_shell_smoke_test(&zsh, &["-i"]);
@@ -446,7 +449,7 @@ fn m6_5_repl_spawns_zsh() {
 #[test]
 fn m6_5_repl_spawns_fish() {
     let Some(fish) = locate_shell("fish") else {
-        eprintln!("skipping: fish not on PATH (set PMACS_TEST_FISH to override)");
+        support::skip_or_fail_overridable("fish", "PMACS_REQUIRE_SHELLS", "PMACS_TEST_FISH");
         return;
     };
     run_shell_smoke_test(&fish, &["-i"]);
@@ -458,8 +461,10 @@ fn m6_5_repl_spawns_fish() {
 #[test]
 fn m6_5_repl_spawns_lua() {
     let Some(lua) = locate_shell("lua").or_else(|| locate_shell("luajit")) else {
-        eprintln!(
+        support::skip_or_fail_overridable(
-            "skipping: lua/luajit not on PATH (set PMACS_TEST_LUA or PMACS_TEST_LUAJIT to override)"
+            "lua/luajit",
+            "PMACS_REQUIRE_LUA",
+            "PMACS_TEST_LUA or PMACS_TEST_LUAJIT",
         );
         return;
     };

tests/m6_8_multi_repl_acceptance.rs

@@ -68,6 +68,9 @@ use pmacs::editor::EditorState;
 use std::path::{Path, PathBuf};
 use std::time::{Duration, Instant};
 
+#[path = "support/mod.rs"]
+mod support;
+
 // ---------------------------------------------------------------------------
 // Test harness
 // ---------------------------------------------------------------------------
@@ -94,8 +97,10 @@ fn locate_lua() -> Option<PathBuf> {
             }
         }
     }
-    eprintln!(
+    support::skip_or_fail_overridable(
-        "skipping: lua/luajit not on PATH (set PMACS_TEST_LUA or PMACS_TEST_LUAJIT to override)"
+        "lua/luajit",
+        "PMACS_REQUIRE_LUA",
+        "PMACS_TEST_LUA or PMACS_TEST_LUAJIT",
     );
     None
 }

tests/support/mod.rs

@@ -0,0 +1,104 @@
+//! Shared test-support helpers.
+//!
+//! Included by `#[path = "support/mod.rs"] mod support;` rather than
+//! copied. Files under `tests/` subdirectories are not compiled as
+//! their own test binaries, so this costs nothing — and
+//! `m6_8_multi_repl_acceptance.rs` previously carried a comment saying
+//! cross-test-binary sharing "would need a fixture crate", which is not
+//! so. A correct helper in one file and a degraded copy in another is
+//! this suite's most repeated defect shape; sharing removes the way it
+//! happens.
+//!
+//! **Why this is separate from `tests/common/`, which also exists.**
+//! `tests/common/mod.rs` re-exports `daemon` and `pty` — real daemon
+//! spawning and PTY plumbing. Including it to reach a six-line
+//! environment check would compile that machinery into three test
+//! binaries that spawn neither, for no benefit. `support` is the
+//! dependency-free half: helpers any test binary can take without
+//! taking a subsystem with them. Two directories is a cost worth
+//! naming rather than leaving to be rediscovered; if a third appears,
+//! consolidate instead of continuing the pattern.
+
+#![allow(dead_code)]
+
+/// Report a missing external tool, and turn the skip into a HARD
+/// FAILURE when the environment has promised the tool is present.
+///
+/// The bare shape this replaces —
+///
+/// ```ignore
+/// let Ok(_) = which_binary("gopls") else {
+///     eprintln!("gopls not on PATH; skipping");
+///     return;
+/// };
+/// ```
+///
+/// passes GREEN when the tool is absent, and is why a large block of
+/// external-tool-gated tests had never once executed their bodies in
+/// CI: nothing installed the tools, so every one of them reported
+/// success without running. A suite that cannot tell "passed" from
+/// "never ran" is worse than a missing suite, because it reads as
+/// coverage.
+///
+/// `PMACS_REQUIRE_*` is the project's own fix, already load-bearing for
+/// `PMACS_REQUIRE_GPU` in `vterm_stage3_acceptance`: CI installs the
+/// tool, sets the variable, and absence becomes a failure that names
+/// the step that should have provided it. Locally the variable is
+/// unset, so the skip still works and nobody needs the whole toolchain
+/// to run the suite.
+///
+/// Deliberately per-tool rather than one blanket variable: a tool that
+/// must stay unarmed (because arming it would hang, or because CI does
+/// not install it yet) keeps its own variable that CI never sets, and
+/// that decision is then visible at the call site instead of buried in
+/// a workflow file.
+/// True when `var` is set to a non-empty value.
+///
+/// Emptiness matters, and the reason is a trap rather than a nicety.
+/// The natural GitHub Actions idiom for a conditional environment
+/// variable —
+///
+/// ```yaml
+/// PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }}
+/// ```
+///
+/// sets the variable to the EMPTY STRING on every other platform, not
+/// to nothing. A bare `var_os(..).is_some()` is therefore true there,
+/// which would arm the guard on exactly the runners that have none of
+/// the tools installed and fail every one of them. Treating empty as
+/// unset makes the common workflow spelling safe instead of subtly
+/// wrong.
+fn armed(var: &str) -> bool {
+    std::env::var_os(var).is_some_and(|v| !v.is_empty())
+}
+
+#[track_caller]
+pub fn skip_or_fail(tool: &str, require_var: &str) {
+    assert!(
+        !armed(require_var),
+        "{require_var} is set, but `{tool}` is not on PATH. \
+         The CI step that installs it did not run, or installed it \
+         somewhere not on PATH. This is a hard failure precisely so \
+         the test cannot report green without executing."
+    );
+    eprintln!("{tool} not on PATH; skipping (set {require_var} to make this fatal)");
+}
+
+/// As [`skip_or_fail`], for tools whose PATH lookup can be overridden
+/// by a `PMACS_TEST_*` variable. The skip notice keeps naming that
+/// override, because losing it would make the local escape hatch
+/// undiscoverable — the REPL suites are routinely run on machines
+/// without zsh or fish.
+#[track_caller]
+pub fn skip_or_fail_overridable(tool: &str, require_var: &str, override_var: &str) {
+    assert!(
+        !armed(require_var),
+        "{require_var} is set, but `{tool}` is not on PATH and {override_var} \
+         is unset or points at nothing. The CI step that installs it did not \
+         run, or installed it somewhere not on PATH."
+    );
+    eprintln!(
+        "skipping: {tool} not on PATH (set {override_var} to override, \
+         or {require_var} to make this fatal)"
+    );
+}