test: stop asserting a 200ms wall-clock budget when opening Help

AGENTS.md

@@ -64,7 +64,8 @@ and the spec chain):
 - **Presenter**: draw the two-column layout (ratatui)
 - **Input Dispatcher**: map key events → intents (crossterm)
 - **Session Controller**: orchestrate intents → state changes; holds in-memory session state
-- **Editor Launcher**: hand a file off to an external editor / new herdr pane
+- **Editor Launcher**: hand a file off to an external editor in-process, suspending and resuming the
+  TUI around it (NOT a herdr pane — see the herdr integration section)
 
 State is **in-memory and ephemeral only** except for the safe-to-delete, advisory
 `update-check.json` cache, which never changes the viewed root or git repo.
@@ -86,7 +87,7 @@ These shape every decision; violating one is a design error, not a style nit:
 
 ### Stack specifics
 
-- **Rust 1.96 (edition 2024)** + **ratatui 0.30.1** (uses `ratatui-core` 0.1.x) + **crossterm 0.29.0**
+- **Rust 1.96 (edition 2024)** + **ratatui 0.30.x** (uses `ratatui-core` 0.1.x) + **crossterm 0.29.0**
 - **`ansi-to-tui` 8.0.1** ingests the external renderers' ANSI output into ratatui spans, and
   doubles as the **AC-27 escape-neutralizer** (maps styling, drops cursor/screen-control). All file
   content flows through it.
@@ -149,6 +150,14 @@ cargo audit
 
 - **The spec is the contract.** To change scope/criteria/design/stack, edit the artifact at the
   **owning stage** and **re-run the readiness check**, don't ad-hoc-edit downstream specs.
+- **Never weaken a spec-backed assertion to make a change pass.** A test that names an acceptance
+  criterion, and a policy list an AC enumerates (e.g. the exhaustive read-only matrix in
+  `src/intent.rs::intent_effects_never_mutate_files_or_git_and_classify_annotation_edits`, which
+  AC-N3 defines row by row), are the contract in executable form: deleting an
+  entry to accommodate new code silently changes behaviour the spec mandates — in the case that
+  prompted this rule, a required user-visible notice became a silent no-op. If a criterion genuinely
+  should change, change it at the owning stage first (above) and say so in the PR. If a spec-backed
+  test looks wrong, STOP and ask rather than editing it away.
 - **Definition of done for a user-facing feature:** the feature isn't done until the docs match it,
   IN the same PR: `CHANGELOG.md` entry, the relevant `docs/` page (`docs/keys.md` for a key + the
   Shift-keys note for a capital-letter key, `docs/usage.md` for the feature, `docs/configuration.md`
@@ -159,14 +168,69 @@ cargo audit
   not `main`; always `git log main..HEAD` before committing/opening a PR, or strays get swept in.
 - Keep the deterministic tier green (fmt/clippy/`cargo audit`) and tests hermetic.
 
+### Tests prove things deterministically, or they don't count
+
+This suite runs on macOS, Linux and Windows CI runners whose timing and layout differ from any dev
+machine. Three rules, each learned from a real failure here. Unlike the drift guards below, these are
+prose, not build-failing checks — hold yourself to them.
+
+- **Don't assert a tight time budget, and don't prove a negative by sleeping.** Slack that is a small
+  multiple of the thing being measured is a coin flip on a loaded runner (a ~200ms budget asserted
+  under 300ms failed twice in one day), and a "nothing happened" poll passes vacuously when the thing
+  lands after the window closes. Prefer a synchronous, observable tell: `Controller::render_seq` is
+  bumped inside `dispatch_render` BEFORE the worker spawns, so an unchanged seq proves no render was
+  dispatched where counting a stub provider's calls only races it.
+  - **Where a wait is the point, split the claim in two.** Pin the budget's VALUE clock-free
+    (`tests/whats_new_composer.rs` asserts `WHATS_NEW_COMPOSE_TIMEOUT == 200ms` and that every
+    document receives exactly `opened_at + WHATS_NEW_COMPOSE_TIMEOUT`), and let the behavioural test
+    bound only the wait, ~10x the budget and orders of magnitude below the stall it distinguishes
+    from (`HELP_STALLED_RENDERER_MAX_WAIT`). Neither half alone is enough: a widened budget escapes
+    the loose bound, and a blocking call escapes the clock-free test.
+  - **The honest exception is a criterion that IS a latency budget.** AC-22/AC-23 mandate 300ms, so
+    `help_open_switch_scroll_each_within_300ms` (`tests/controller.rs`) must hold a stopwatch — that
+    is the spec, not a testing choice. Keep such tests, give them the widest slack the criterion
+    permits, and don't add new ones without a criterion behind them.
+  - **Two shapes to copy.** For "no work was dispatched", assert `Controller::render_seq` is
+    unchanged (`tests/controller_async.rs`) — it is bumped synchronously inside dispatch, before the
+    job reaches the render worker. For "bounded, not unbounded", bound a long-stalling fixture (60s
+    in `src/proc.rs` / `src/render.rs` / `src/update/gateway.rs`, 30s and an endless loop in
+    `tests/render_delegate.rs`) at a couple of seconds, rather than re-measuring the timeout you
+    passed in — generous enough for a loaded runner, tight enough to still reject a multi-second
+    tail.
+  - **Know the render worker's shape before reasoning about ordering.** There is ONE long-lived
+    worker (`Controller::spawn_worker`) that takes jobs over a channel in order and collapses a
+    backlog, so a newer result landing means every earlier job already finished or was collapsed.
+    That ordering is what lets a test assert with no wait at all; assuming thread-per-render instead
+    leads to inventing sleeps that prove nothing.
+  - **Force the race instead of hoping for it; document a limit only when you truly cannot.** The
+    end-to-end superseded-render tests in `tests/controller_async.rs` look like they prove `poll`'s
+    `seq == latest_seq` guard and do not: their polling loop drains the earlier result first, so
+    removing the guard leaves them green. A gated renderer (`GatedContent`) makes the ordering
+    happen on demand — hold one render open, dispatch a newer one behind it, release the first so
+    its result arrives stale — and that test DOES fail when the guard is removed. Reach for the gate
+    first. Where a race genuinely cannot be forced from outside, write the limit into the test's own
+    comment and name what would prove it, so nobody trusts it past its scope.
+- **Never send a key that assumes state the test has not observed.** In a pty journey `q`/Esc peels
+  ONE state layer per press (`src/controller/mod.rs`: selection → flash → committed search → zoom →
+  discard confirm → quit), and toggles like `z` flip whatever is actually there. A journey that
+  presses a toggle "to undo" a state it never asserted will break on the runner whose layout decided
+  otherwise — which is exactly how a pinned-preview e2e went red 4/4 on ubuntu while green everywhere
+  else. Drive the state you depend on, or make the tail independent of it, and say which in a comment.
+- **A negative that cannot be reproduced locally is not verified.** macOS-only green means nothing for
+  a Linux-only failure: reproduce in a Linux container (`rust:1.96-trixie`, mount the worktree
+  read-only, cache `CARGO_TARGET_DIR` in a volume) before claiming a fix, and say plainly when you
+  could not.
+
 ### Adding a keybinding or a config key (touchpoints + drift guards)
 
 Both surfaces are single-source-of-truth in code, with a build-failing test guarding the docs, so you
 never wire them in two places or let the docs drift.
 
 **A new keybinding / action.** `REGISTRY` in `src/input.rs` is the source of truth: the dispatcher,
 the `?` overlay's Keybindings section, and `[keys]` remapping all derive from it.
-1. Add the variant to the `Intent` enum in `src/intent.rs` (it lives in `Intent::ALL`, 39 today).
+1. Add the variant to the `Intent` enum in `src/intent.rs`, and to its `Intent::ALL` array (whose
+   length constant must be bumped with it — read the current count from the source, don't trust a
+   number written here).
 2. Add a `Binding { intent, name, default_keys, description, category }` row to `REGISTRY`
    (`category` must be one of `CATEGORY_ORDER`).
 3. Handle the intent in the session controller (`src/controller/`).
@@ -177,6 +241,17 @@ the `?` overlay's Keybindings section, and `[keys]` remapping all derive from it
    registry key is in `docs/keys.md`) and `configuration_doc_lists_every_remappable_intent` (every
    registry name is in `docs/configuration.md`).
 
+**A change to how an agent or a launcher invokes the viewer.** The bundled skill
+(`skills/herdr-file-viewer/SKILL.md`), the paste-in block in `docs/usage.md`, and the launcher scripts
+(`scripts/open-file-viewer*.sh`) all teach the same launch, and they have drifted apart before: the
+skill told agents to pass `--cwd` to `herdr plugin pane open`, which cannot spawn the manifest's
+RELATIVE pane command (and inside a built plugin checkout silently runs *that* checkout's binary),
+while the shipped launcher never passed it (#139). Change all of them together, and keep
+`no_documented_launch_passes_cwd_to_plugin_pane_open` (`tests/docs_consistency.rs`) honest — it holds
+the docs and the scripts to one rule so this divergence fails the build instead of reaching a user.
+The viewed root comes from the FOCUSED herdr pane's cwd (resolved to its worktree top level), never
+from a flag.
+
 **A new config key.** `src/config.rs` owns it: add the field to `Config`, resolve it in `resolve`
 into `EffectiveSettings`, and apply it at wiring time. **Docs (same PR):** document it in
 **`docs/configuration.md`**, add a commented `key = ...` line to **`config.example.toml`**, surface

src/controller/mod.rs

@@ -1618,6 +1618,19 @@ impl Controller {
         self.content_scroll
     }
 
+    /// The most recently dispatched render's sequence number.
+    ///
+    /// A read-only observability seam for tests: both [`dispatch_render`](Self::dispatch_render)
+    /// and [`dispatch_reflow`](Self::dispatch_reflow) bump this SYNCHRONOUSLY before the job is
+    /// sent to the (already running, single) render worker, so an unchanged value proves no render
+    /// was dispatched. That is the deterministic
+    /// way to assert a negative here — sleeping to "give a wrong render time to land" passes
+    /// vacuously whenever the render lands after the window (see AGENTS.md, "Tests prove things
+    /// deterministically").
+    pub fn render_seq(&self) -> u64 {
+        self.latest_seq
+    }
+
     /// Assemble the [`ViewState`] the Presenter draws from: the visible tree rows + cursor,
     /// the current content and notices, focus, and the observed width (the narrow-split
     /// input, AC-21).

src/proc.rs

@@ -106,9 +106,16 @@ mod tests {
         let started = Instant::now();
 
         assert_eq!(wait_bounded(&mut child, Duration::from_millis(100)), None);
+        // Bounded vs unbounded: the fixture child stalls for 60s, so this proves termination and
+        // reaping returned rather than waiting on it. 2s is 20x the requested timeout — ample for a
+        // loaded runner (the flakes this replaced were 314-340ms) while still rejecting a
+        // multi-second reap tail, which a 5s bound let through. Not `timeout + small slack` (100ms
+        // asserted under 250ms): that is a coin flip. See AGENTS.md, "Tests prove things
+        // deterministically".
         assert!(
-            started.elapsed() < Duration::from_millis(250),
-            "termination and reaping stay inside the renderer grace period"
+            started.elapsed() < Duration::from_secs(2),
+            "termination and reaping must return rather than wait out the stalled child: {:?}",
+            started.elapsed()
         );
         assert!(
             child.try_wait().unwrap().is_some(),

src/render.rs

@@ -1024,9 +1024,16 @@ mod tests {
         );
 
         assert!(matches!(result, Err(RendererError::Timeout)));
+        // The claim is BOUNDED vs UNBOUNDED, and the fixture stalls for 60s. 2s is 20x the
+        // requested timeout — ample for a loaded runner (the flakes this replaced were 314-340ms)
+        // while still rejecting a multi-second reap tail, which a 5s bound let through. It is
+        // deliberately NOT `timeout + small slack`: that shape (100ms asserted under 250ms) flaked
+        // on a loaded macOS runner and blocked an unrelated PR. The timeout's exact value is the
+        // caller's argument above, not a stopwatch's job. See AGENTS.md.
         assert!(
-            started.elapsed() < Duration::from_millis(250),
-            "the full capture window plus bounded reap tail must not become an unbounded wait"
+            started.elapsed() < Duration::from_secs(2),
+            "the full capture window plus bounded reap tail must not become an unbounded wait: {:?}",
+            started.elapsed()
         );
     }
 

tests/controller.rs

@@ -9324,6 +9324,19 @@ fn slow_markdown_renderers(marker: &std::path::Path) -> Renderers {
     }
 }
 
+/// How long the stalled-renderer fixture holds once it has written its handshake.
+const STALLED_RENDERER_WAIT: Duration = Duration::from_secs(60);
+
+/// The longest Help may take with a stalled renderer before this test calls it a regression.
+///
+/// Deliberately its own constant rather than a fraction of [`STALLED_RENDERER_WAIT`]: those two
+/// numbers answer different questions, and deriving one from the other silently changed the
+/// tolerated latency whenever the fixture's lifetime moved. 2 s is ~10x Help's own 200 ms budget
+/// (loaded-runner slack) and 30x below the stall, so it separates "fell back on the deadline" from
+/// "waited for the renderer" without measuring the runner's mood. It bounds only the WAIT; the
+/// budget's exact value is pinned clock-free in `tests/whats_new_composer.rs`.
+const HELP_STALLED_RENDERER_MAX_WAIT: Duration = Duration::from_secs(2);
+
 #[test]
 fn help_stalled_markdown_renderer_fixture() {
     if let Some(marker) = std::env::args().find_map(|argument| {
@@ -9332,14 +9345,26 @@ fn help_stalled_markdown_renderer_fixture() {
             .map(std::path::PathBuf::from)
     }) {
         std::fs::write(marker, "started").expect("fixture writes stall handshake");
-        std::thread::sleep(Duration::from_secs(60));
+        std::thread::sleep(STALLED_RENDERER_WAIT);
     }
 }
 
 #[test]
-fn open_help_uses_the_composers_single_200ms_budget() {
+fn open_help_falls_back_instead_of_waiting_for_a_stalled_renderer() {
     // T-29: a current-exe fixture writes its handshake before stalling. The handshake rules out a
     // missing/malformed renderer false positive before this test accepts Help's deadline fallback.
+    //
+    // This test owns ONE claim: opening Help does not wait on the renderer. A tight budget assertion
+    // does not belong here — `elapsed < 300ms` for a 200ms budget is a coin flip on a loaded CI
+    // runner, and it failed twice in one day on unrelated PRs before this was widened.
+    //
+    // The budget's VALUE is pinned clock-free in `tests/whats_new_composer.rs`:
+    // `one_absolute_deadline_is_shared_and_observed_remaining_decreases` asserts
+    // `WHATS_NEW_COMPOSE_TIMEOUT == 200ms` AND that every document receives exactly
+    // `opened_at + WHATS_NEW_COMPOSE_TIMEOUT` rather than a fresh timeout each, and
+    // `already_expired_open_uses_precomputed_fallbacks_without_delegation` asserts an expired
+    // deadline skips delegation entirely. Keep the budget's arithmetic there and the wait here:
+    // together they catch a widened budget (there) and a Help that blocks anyway (here).
     let dir = TempDir::new();
     let marker = std::env::temp_dir().join(format!(
         "hfv-help-stall-{}-{}",
@@ -9360,8 +9385,10 @@ fn open_help_uses_the_composers_single_200ms_budget() {
         "the current-exe renderer fixture must start and stall before Help falls back"
     );
     assert!(
-        elapsed < Duration::from_millis(300),
-        "T-29: Help must stay within its 200 ms budget plus bounded scheduling/reap slack: {elapsed:?}"
+        elapsed < HELP_STALLED_RENDERER_MAX_WAIT,
+        "T-29: Help must fall back on its own deadline, not wait for the stalled renderer \
+         (fixture holds {STALLED_RENDERER_WAIT:?}; this test tolerates \
+         {HELP_STALLED_RENDERER_MAX_WAIT:?}): {elapsed:?}"
     );
     assert!(
         ctrl.help_open(),