CLAUDE.md@tests git:20260923.1a5058dB

Immutable. This exact content is served forever at /api/v1/blob/8185c3aab2c3fc3e.

# tests — suite conventions + CI/lint gotchas

> ↑ [root](../CLAUDE.md)

Conventions for the pytest suite under `tests/`: where the test seams sit, how to keep cross-platform defenses honest, and the CI/lint traps. Root holds the two principles these build on ("tests pin contracts, not implementation"; "tests prefer real code paths, stub only I/O"). Engine internals live in [core](../src/grove/core/CLAUDE.md).

## Test seams

**When production stops calling a boundary, the tests that PATCH it keep passing and stop testing anything — so a patch target is part of the change.** Five `test_catalog_sources.py` tests patched `grove.daemon._catalog_sources.GitRepo.worktree_paths`/`list_files`; once the watch-root discovery consumed `DiagramGallery.census` instead of shelling out itself, that import was gone and the patches failed *loudly* (`ImportError: ... is not a package`), which is the lucky case. **The unlucky case is a patch on a symbol that still exists and is no longer reached**, which stays green forever. The fix is to state the new boundary on the double (`_Index.census` returning the scopes) rather than to re-point the patch one layer down — a double that answers the question production actually asks cannot go stale this way.

**Patch the public module surface, never a private symbol.** `tests/conftest.py` monkey-patches `grove.core.tmux.create_session` and friends directly — no Protocol abstraction. Rename a public function and the test breaks loudly; that is the design. A test that patches a private symbol turns that path into an implicit contract: move it and the patch silently no-ops while the test still passes.

**Promoting a private seam is a repo-wide rename, not a local test fix.** `mypy src/grove` does not inspect test imports, and a targeted run does not collect every consumer, so an old private import can survive both and fail only in full collection. Before or with the promotion, grep the whole repository for the old name and update every import, patch target, docstring, and assertion.

**Sandbox Grove paths by patching the Grove function, not the env var.** `monkeypatch.setattr("grove.core.paths.user_schema_path", lambda: tmp_path / "...")` is the right sandbox — deterministic on every OS, matching `tests/conftest.py::tmp_state_dir` (which already redirects `user_state_path` / `user_config_path` / `user_schema_path`). Do **not** `monkeypatch.setenv("LOCALAPPDATA", ...)`: recent `platformdirs` (≥4.x) resolves the user dir via `SHGetKnownFolderPath` through `ctypes` on Windows and only falls back to env vars when `ctypes` is unavailable, so `setenv` is a no-op there — the write lands outside `tmp_path` and `rglob` returns empty. Same rule for redirecting `~/.grove`, `~/.config/grove`, or any `platformdirs.user_*_dir` path.

**Every host path a test can write must be redirected by an AUTOUSE fixture, not an opt-in one.** `tmp_state_dir` is opt-in, so it only protects tests that already know to ask — and `init_log_path` was reachable without asking: `create()`/`resume()`/`respawn()` resolve it unconditionally whenever `init_script.enabled`, so the suite wrote its own fixture text into the developer's live `~/.local/state/grove/logs/`. **Test output masquerading as production diagnostics is the actual cost, not the disk space** — those files surface as the first "evidence" when someone debugs a real user-reported init failure. Hence the autouse `init_logs` (still requestable by name for the tests asserting on the log) alongside autouse `_isolated_agent_hook_paths`.

**`tmp_state_dir` does not cover every `platformdirs` path — it patches the FILE accessors, and some paths resolve `user_state_dir` themselves.** `paths.agent_workspace_config_dir` is one: `AgentSharePlan.seed()` creates it (and copies `.claude.json` into it) on every containerized provision, so any test driving a real provision wrote one directory per run into the developer's live `~/.local/state/grove/agent-config/`, opt-in isolation or not. It is now redirected by autouse `_isolated_agent_hook_paths` with the rest. The generalizable check when adding a path helper: does it go through an accessor a fixture already patches, or does it call `platformdirs` directly?

**`paths.ensure_dir` is a chokepoint for Grove's own writes but NOT for the fakes' — audit at the `pathlib` level.** Every host write in `src/grove` is preceded by `paths.ensure_dir(parent)`, so spying it finds engine leaks; it misses `FakeTmux.run_init_script`, which is handed a real path by the manager and `mkdir`s it itself. To prove isolation, patch `Path.mkdir`/`write_text`/`open`/`os.replace` session-wide and record any target under `$HOME` — that catches test-side writers too. The proof a redirect works is a **before/after file count on the real directory**, not a green suite: these writes never fail an assertion.

**Patching `<your_module>.subprocess.run` mutates the ONE shared `subprocess` module process-wide, not a copy scoped to your module.** `import subprocess; subprocess.run(...)` binds the name `subprocess` to the same global module object everywhere it's imported — `monkeypatch.setattr("grove.core.agents.onboarding.subprocess.run", fake)` therefore also intercepts `grove.core.git`'s own `subprocess.run` calls for the rest of that test (e.g. a CLI test that calls `detect_root` and then a subprocess-shelling engine call in the same invocation). Either (a) make the fake inspect `argv[0]` and pass through anything it doesn't care about to the real `subprocess.run` (captured before patching), or (b) patch the whole consuming function instead (`grove.core.agents.onboarding.register_mcp`) the same way `fake_tmux` replaces whole `grove.core.tmux` functions — see `tests/cli/test_onboarding_commands.py::fake_mcp_add` for the passthrough pattern. The passthrough form is mandatory whenever one test file must fake a subprocess boundary (e.g. `docker`) AND still drive real `git` in the same invocation: patching the whole consuming function isn't an option when the test wants the real git side effect.

### The false-green double — three shapes of the same bug

A double that is *incomplete* is worse than no double: the suite stays green while describing a program production does not run.

**A fake that SUBCLASSES a real boundary is only as offline as the methods it overrides.** `FakeCli` extends `DevcontainerCli` deliberately (so signature drift breaks loudly), but overriding only `probe`/`read_configuration`/`up` meant that the moment production called `exec` from the provisioner, every container test spawned a REAL `devcontainer exec` — silently, still green — and a real `docker build` cached a static tmux in the developer's live `~/.local/state/grove`. **Neither existing offline test could see it: one never creates a workspace and the other falls back to the host at arm 4, so both stop short of a real provision** — which needs the fake CLI + fake preflight injected, i.e. exactly the DI those tests deliberately avoid. `test_offline_guarantee.py` now drives the provision arm too, and asserts `state.container.provisioned` first so it cannot pass by never getting there. Two habits fall out: when adding a production call to an inherited boundary, check the fake overrides it; and when a fixture claims "no test needs X", the assertion of that claim must reach every arm that can call X, not just the arm that made the claim necessary.

**A double SIMPLER than the real dataclass tests a program production cannot run — and the fix belongs in the double, never in a `getattr` guard.** `_FakeWorkspaceState` in `test_projector.py` carried only `id` and `agent_session_id`, which sufficed while attribution keyed on a minted id alone. The moment it also resolved by cwd, the fake raised `AttributeError` on `worktree_path` — a field that is `str` and REQUIRED on the real `WorkspaceState`, so production can never see it missing. The tempting repair is a defensive `getattr(state, "worktree_path", None)` in the projector, and it is wrong twice: it makes production carry a branch for a state that cannot exist, and that branch SKIPS cwd indexing, so the very attribution being added silently does nothing. Widen the fake to the real field set instead.

**A fixture that injects only one half of a writer/reader pair proves nothing about either.** The history-route fixture supplied the daemon's reader while `JsonWorkspaceStore` lazily built its own writer — same file in production, so the code works, but the test read an instance nothing had written to and its kill-then-read case "passed" against a tombstone that never existed. Sharing the instance then turned three *other* tests red, which was the useful signal: they had been asserting an empty history for a freshly created workspace, i.e. the broken wiring. **When one double writes what another reads, inject one object; and when fixing that turns neighbouring tests red, read them before fixing them — they may have been pinning the bug.**

**Make a public payload assertion exhaustive when a newly inherited field needs a publication decision.** A subset check proves known fields but lets a new contract field become public by default without asking whether it belongs there. The public-share ticket test compares the entire serialized `TicketRef`, so adding `draft` forced the explicit decision to expose it alongside `status`, while deliberately stripped fields remained absent. Use this at a privacy or stability boundary, not merely to couple every internal response to all of its defaults.

**A double that never changes what the NEXT read reports is the same bug in its most invisible form.** `FakeCli.exec` scripted a fixed answer and the docker double answered a fixed session list — so a test could "start" an agent and every later read still described the world before it. That specifically hid the measured fact that `tmux new-session -d` exits 0 for a command that cannot run, so a failed start looked identical to a good one. The fix is for the start double to REGISTER what it was asked to start, with an explicit `starts=False` for the real failure. **When one double's action is another double's input, the two have to be wired together or neither can be wrong** — and the tell is that no assertion in the module reads a value the action produced.

**A monkeypatch stops at the process boundary, so a test that spawns a DAEMON must pin its config instead.** `tests/client/test_client_http.py` spawns a real `grove daemon serve` subprocess, which loads no conftest and honors no autouse fixture — so `_offline_container_runtime` never reached it, and once containers became the default runtime every workspace that file created was a REAL devcontainer (dozens of leaked `vsc-<title>-…` containers on the developer host; the file went 482s → 22.65s once pinned). **`test_offline_guarantee.py` is structurally unable to catch this**: its spy patches `subprocess` in the pytest process, so the argv it records is `grove daemon serve`, never the `devcontainer up` the grandchild runs. Config is the ONLY lever that crosses a process boundary — such a test pins `container.enabled=false` in the config the subprocess reads, which gates the create path and `ProjectInfra.registration_hook` together. It survives because **nothing fails when a unit test does real work — it just gets slower**, and a flipped default silently re-arms every test that never pinned it. The diagnostic to reuse: this surfaces as `TransportError` on `pause`/`kill`, because those ride the client's 30s `_DEFAULT_TIMEOUT_S` while `create`/`resume` get the 1260s `_LIFECYCLE_TIMEOUT_S` — **a verb-dependent timeout pattern means slowness, not a broken route**, and the fast verbs are the ones that report it.

**Hide EVERY system terminfo tree before testing a terminfo bundle, or the test passes while the bundle is broken.** ncurses searches `TERMINFO_DIRS` *before* its compiled-in fallbacks, not instead of them, so an image with its own database silently answers for a missing bundle entry — and images acquire one by accident (installing python3 on Alpine drops a real `/etc/terminfo`). Mask `/usr/share/terminfo`, `/etc/terminfo`, `/lib/terminfo`, `/usr/lib/terminfo` and `/usr/local/share/terminfo` (an empty read-only bind over each) and assert the mask took effect — a probe that only counts `ls` output lines reports "2" for two empty directories. Same false-green shape one level down: the double is the environment rather than a fake object.

**A redirected host path is about DETERMINISM as much as isolation.** The tmux payload cache had to be pointed into `tmp_path` not merely so tests stop writing to `~/.local/state/grove`, but because a developer who HAS built the bundle would otherwise get different answers from one who has not — a test that reads a real host artifact is a test whose result depends on the machine.

**No test hits the network — the release check is neutered by an autouse fixture.** `tests/conftest.py::_offline_release_check` patches `grove.core.release.fetch_latest_release_tag` → `None` for every test, so a default `ReleaseChecker` (the one the daemon/TUI build with no injected `fetcher`) reports "unknown" instead of GETting GitHub. Same discipline as the fake tmux/git seams. A test that wants a specific verdict injects a `fetcher` directly into `ReleaseChecker` (bypasses the seam) — see `test_release.py`, the daemon whoami tests, and the TUI worker test.

**Deterministic OTel ids do not prove replay idempotency.** Historical export
tests script Langfuse's trace-read boundary and prove completed checkpoints
skip, partial traces send only missing observation ids, ambiguous failures stay
uncheckpointed, and a changed payload under an accepted id conflicts rather than
updates. The ledger fixture is a separate temporary file from the disposable
usage cache, includes the destination in its key, and test manifests contain no
credentials. Pin a regression where the same trace id is completed for one
destination and remains pending for another.
Also pin asynchronous acceptance: a trace missing from the immediate read-back
is `submitted`, an immediate rerun emits nothing, and an expired-grace retry
sends only observation ids the remote still lacks.

**An env var the PRODUCT reads is a cascade layer too, and the developer running the suite is the one most likely to have it set.** `GROVE_PHASE_FILE` is exported into every Grove-launched agent, and the CLI now prefers it over cwd inference — so the moment the suite is run *by an agent inside a Grove workspace*, every cwd-inference test resolves that agent's real workspace instead of its fixture. It would pass on CI and fail only for the person changing this code, which is the worst available split. Hence autouse `_no_inherited_phase_file`, unconditional for the same reason `init_logs` is: **a test cannot opt into protection from an environment it does not know it has.** The general rule — when production starts reading an env var, add the autouse `delenv` in the same commit, and let the test that means that branch set it itself.

**A DECLARED env var (`x-env-var`) is a config CASCADE LAYER, so the developer's own shell can fail tests that pass in CI, and the failure names the assertion rather than the environment.** `GROVE_GITEA_BASE_URL` exported in a shell beats the config a ticket test writes to disk (the declared layer sits above the project file — see [core](../src/grove/core/CLAUDE.md)), so `tests/cli/test_tickets_commands.py` and `tests/daemon/test_tickets.py` fail with *"no enabled ticket provider owns https://gitea.example.com/…"* on a host that actually uses Gitea, and pass everywhere else. Nothing is neutralized globally because the layer is a feature. Diagnose by re-running under `env -u GROVE_<…>`; the tell is that the DEBUG log line reads `ignoring non-config env var GROVE_GITEA_BASE_URL` — which is the *other* env mechanism disclaiming it, not evidence that nothing consumed it. Generalizable: when a test's config assertion loses to a value nobody in the test wrote, enumerate the schema's `x-env-var` names before suspecting the test.

**The mewbo adapter is a SECOND network reacher, and it is not neutralized globally.** Its `list_sessions`/`discover_all` are REST calls whose config default is `http://127.0.0.1:5125` — reachable on a developer box actually running Mewbo — and they sit inside `all_adapters()`, so *any* test touching `SessionExplorer.list` or `SessionCatalog.scan` reaches for a socket. Unlike the release check it has no autouse fixture, because the adapter's own tests inject an `httpx.MockTransport` and a class-level autouse patch would break them. Neutralize per test-module instead (patch `MewboAdapter.list_sessions`/`discover_all`), as `tests/daemon/test_sessions_endpoints.py` does. Same reasoning applies to `grove.core.process.list_agent_runtimes`: a real `/proc` walk makes a liveness assertion depend on what the developer happens to be running, so patch it and build the runtimes the test means to assert on.

## Asking CI for less than everything

**Gates are reusable `workflow_call` units (`.github/workflows/_*.yml`) with two callers: `ci.yml` on push/PR and `dispatch.yml` on demand.** Both call the same file, so the on-demand path cannot drift from the automatic one — the usual failure of a hand-maintained "quick CI". Run a scoped suite on a worker instead of the developer host with `tea actions workflows dispatch dispatch.yml -i suite=python-tests -i paths=tests/core -i keyword=<expr>`; the inputs are pytest's own selectors (`paths`, `-k`, `-m`), so nothing needs maintenance as tests move.

**`tea -i` cannot carry a comma, and quoting does not help** — it splits every `-i` value on commas before parsing `key=value`, so `-i quality_gates=ruff,mypy` dies with `invalid input format "mypy"` (tea 0.14.0, measured 2026-09-15). The limit is tea's parser: the same value is accepted verbatim over the API (`tea api -X POST /repos/<o>/<r>/actions/workflows/dispatch.yml/dispatches -d '{"ref":…,"inputs":{…}}'`). This is why `paths` is SPACE separated — the common scoping case has to stay CLI-native.

**Prove a scoped run was not vacuous by reading the DESELECTED count, never the exit code.** `54 passed, 87 deselected` is evidence the selectors bit; `0 passed` and a green check look identical to a suite that collected nothing because a path typo silently matched no files. Same discipline as the false-green doubles below — a gate that runs nothing reports success.

**A `gates` subset must be matched comma-ANCHORED (`contains(format(',{0},', gates), ',ruff,')`).** A bare `contains(gates, 'ruff')` also fires `ruff-format`, and those are deliberately separate gates: `ruff check --fix` can reflow code and still leave the formatter red, so a green `ruff check` never implies a green `ruff format --check`. Expressions have no `replace()`, so the input carries no spaces.

**THIS RUNNER SERIALIZES ONE WAY OR ANOTHER, AND THE ONLY QUESTION IS WHETHER YOU SAY SO — three different gates learned it separately.** Each job container is capped (`--memory=2560m`, `--memory-swap` equal to it, and `/tmp` is RAM-backed) on a runner whose own `capacity` is 2, so anything genuinely concurrent competes for one budget: the uv legs over the tool cache (below), `webapp-e2e` against the 26-minute test leg (one run died in `npm install` with ENOSPC, the next took **49 Python tests** down with `No space left on device`), and e2e's own four shards against each other. **The shard case is the one worth copying, because it shows what a starved gate REPORTS:** 4 parallel shards gave `38 failed / 3 passed` then `49 failed / 2 passed`, every one a 90-second `locator.click` timeout and not a single assertion failure; `max-parallel: 1` — same total work, same per-shard verdicts — gave `9 failed / 38 passed` with real `toBeCloseTo` diffs. **A uniform timeout across unrelated specs is a machine that cannot paint; an assertion failure is a bug.** Read WHICH failed before reading how many, and when the failing leg *moves between runs on the same commit*, you are measuring the host. Do not raise the caps to fix it: their dated comments tie them to confirmed memory-corruption kernel panics on this host.

**Make each gate report its own verdict with STEPS in one job, never parallel jobs — act_runner's shared tool cache is not concurrency-safe.** The legibility problem is real: as one `run: make lint`, a ruff failure exited before mypy ran, so one red gate hid three unknown ones. But splitting into four parallel *jobs* made every run fail: the runner mounts a shared `act-toolcache` volume at `/opt/hostedtoolcache`, four legs hit `astral-sh/setup-uv` within the same second, and **exactly one leg per run died with a bare `❌ Failure - Main astral-sh/setup-uv@v3` carrying no message.** The diagnosis is that the victim MOVED between runs — mypy on one, `lint-imports` on the next, with the surviving gates all green — which is what separates a cache race from a real gate failure; a genuine break fails the same leg every time. Upstream added `runner.tool_cache_mode` in act_runner v3.2.0 for this, but that is runner config, not something a workflow can assert. **Sequential steps with `if: ${{ !cancelled() && … }}` give the same per-gate verdicts**, keep the job red, and cost one dependency install instead of N — which on a capacity-1 runner is strictly less work than "parallel" legs that were serializing anyway.

**`webapp/`'s `codegen:check` is NOT a Node-only gate — it needs a Python toolchain.** `scripts/codegen.ts` spawns `uv run python -c "…build_app(…).openapi()"` so the types it diffs always describe THIS checkout rather than whatever a running daemon serves. A job with only `setup-node` fails it with `✗ codegen — could not build the daemon's OpenAPI`, which reads as type drift and is actually a missing interpreter. Its CI job installs both toolchains unconditionally; gating the uv install on the `gates` input would leave any subset that includes codegen silently broken.

**A differential path filter must fail OPEN.** `ci.yml`'s `changes` job skips gates for untouched areas, but a first push, a force-push or a squash leaves the base sha unresolvable — and a gate whose error path is "skip the tests" reports green for work nothing checked. Run everything when the diff cannot be computed, and re-run both sides when `.github/workflows/` itself changed, since the gate definitions are what moved. Differential gating applies to push/PR ONLY: a dispatch that silently runs nothing defeats the point of asking for it.

## Cross-platform test gating

**CI is Linux-only (`ci.yml` runs `ubuntu-latest`, no OS matrix).** tmux is a hard runtime dependency (WSL2 on Windows) and native packaging was retired, so a `[ubuntu, macos, windows]` matrix burned minutes for no signal. The Windows/macOS defensive *code* stays (POSIX import guards, `os.replace`, `platformdirs` patching, the `bash` probe) — it is cheap correctness and dropping CI is not a license to write Linux-only code. But those defenses are **CI-unverified**: a Windows/macOS regression won't be caught automatically, so reason about path/subprocess/POSIX-module code by hand when you touch it.

**Compare resolved `Path` objects across tools, never raw strings.** Git CLI output emits `/`, Python `str(Path)` emits `\` on Windows; Linux makes the asymmetry invisible, so the bug only surfaces on Windows CI. Assert `Path(p).resolve() in {Path(q).resolve() for q in git_out}`. Same for any cross-tool path comparison (lazygit, gh, etc.).

**Gate shell-exec tests with an output probe, not `shutil.which("bash")`.** On Windows GitHub runners `bash.exe` resolves to the WSL launcher; with no installed distribution every `bash -c …` prints "Windows Subsystem for Linux has no installed distributions." and exits 1 — yet `which` still passes. Run a real probe (`bash -c "echo grove-shell-probe"`) and verify the output before skipping. (`git` escapes this only by accident: Windows runners ship real Git.)

**`uv sync --all-groups` fetches a group's direct-URL requirement even when you exclude that group, so name the group you want instead.** The `lint` and `test` jobs synced every group, which reached for the docs theme, pinned to a **GitHub release asset** with no index mirror to retry against. One `http2 ... refused stream` there failed the whole install over a package `make lint` never runs. **`--no-group docs` did NOT suppress the fetch** — that was the first fix and it failed identically. The evidence isolating the flag: the `Docs` workflow (`uv sync --group docs`) fetched the same wheel successfully in the same minutes, and the `test` job's install passed while `lint`'s failed on the identical command, so the fetch is flaky and `--all-groups` is only what makes it reachable. Both jobs now run `uv sync --frozen --group dev`: naming the group avoids the `--all-groups` path, and `--frozen` installs from `uv.lock` with no resolution step, so no URL metadata is ever needed. **Reproduce a CI install failure with the runner's own uv version** (`setup-uv` logs it) in a fresh clone with `UV_CACHE_DIR` pointed at an empty dir. A local checkout with a warm venv silently *removes* the excluded packages and exits 0, which reads as a pass and proves nothing. Pillow lives in `dev`, not `docs`, because `tests/tools/screenshots/` imports it: placement follows *which job needs the package*, not which concern the module reads as.

### A test can cover the right scenario and still be blind to the defect

**When a fold decides whether two records are the SAME THING, assert on the resulting record's CONTENTS — aggregate counters cannot see block-level corruption.** `test_resume_dedups_overlapping_records` already installed one session under two folders so the locate glob returned both, i.e. exactly the cross-file collision scenario. It asserted `human_turns`, `replies_per_turn`, `assistant_replies`, `tool_calls`, `tokens_in`, `tokens_out`, `state` — every one a reduction. The real defect (a merge that duplicated a content block inside one message) left all of them correct, because usage is counted once per logical record by design. The suite stayed green while the wire carried a duplicate `tool_use_id` that killed the browser.

Two riders from the same incident, both cheap and both general:

- **A mechanism with two halves needs a test per half.** The split-block test built every sibling line in ONE file, pinning "siblings merge" and saying nothing about "non-siblings must not". Half a mechanism is what shipped.
- **Pin the INVARIANT a consumer actually depends on, not only the mechanism that currently provides it.** "No `tool_use_id` repeats in a turn view" is one assertion, holds for every provider, and would have failed the moment the fold mis-merged — regardless of which mechanism caused it. That is the assertion that survives the next refactor of the thing underneath it.

## "Is this process dead" is a STATE question on CI, never a signal question

**`os.kill(pid, 0)` and `/proc/<pid>` existing both answer "yes" for a ZOMBIE, and CI's container PID 1 does not reap.** A killed grandchild is nobody's child in the test process, so under that runner it stays `Z` forever; a liveness poll built on either probe spins to its deadline and reports a leak that is not there. On a developer host something reaps it and the same test passes, so it presents as a flake. It bit twice in one PR — a new command-watch timeout guard and the pre-existing `test_wedged_tmux_server.py` `_alive` helper — each costing a CI round trip, because the first fix (poll until `ProcessLookupError`) used the very probe that cannot see it. **Read the state field from `/proc/<pid>/stat`, after the LAST `)`** (comm may contain spaces and parentheses), and treat `Z` as dead: it runs no code and holds no memory, which is every property these tests exist to pin. Mutation-check afterwards — accepting an extra state is exactly how a guard goes vacuous, and both survived disarming the kill.

**The same PR's third CI-only failure was a wall-clock window**: a test backdated a sample by two seconds and expected the script to observe exactly two, while the script re-read `time()` on a loaded runner and saw three (100% → 66%). A test whose denominator is elapsed real time asserts a range, or it measures the host.

## Lint

**NEVER write a Python numeric separator inside a SQL string — `duration_ms=90_000` is a Python habit that SQL does not share, and the failure is environment-dependent.** SQLite accepts underscores in numeric literals only from **3.46.0**; this host ships 3.50.4 and CI's runner is older, so three tests passed locally and failed on CI with `sqlite3.OperationalError: unrecognized token: "90_000"`. **A local run is structurally unable to catch this**, which is the whole reason it is written down: the separator is invisible to `ruff`, to `mypy` and to review, and it reads as correct Python because inside the quotes it is not Python at all. Python-side comparisons (`... .fetchone()[0] == 9_000`) are fine — the rule is about the string.

**Run the full `make lint` before pushing, never just `ruff check`.** `make lint` runs `ruff format --check` too. A `ruff check --fix` that reflows code can still leave a format-check failure, so a green `ruff check` is not enough.

**A method named after a builtin shadows that builtin in every LATER annotation in the same class.** `WorkspaceManager.list` makes `-> tuple[bool, list[str]]` on any method defined after it fail mypy with *"Function grove.core.manager.WorkspaceManager.list is not valid as a type"* — class-body scope wins, and `from __future__ import annotations` does not change it. A local `x: list[str] = []` *inside* a method body is fine (function scope never consults the class), which is why the trap only ever appears in signatures. Reach for `collections.abc.Sequence` (or another container type) rather than renaming a published method.

**Keep `import-linter` honest with `include_external_packages = true`.** Without it, contracts that forbid third-party modules (textual, rich, typer, click) silently pass.

## A green suite cannot see the terminal, so probe the terminal

**A steering change is only as verified as the composer it was run against, and the composer has configuration the suite cannot observe.** `FakeTmux` records the argv and reports success, so a delivery mechanism that is *wrong for a real pane* passes every test in this suite. Two findings from one live probe against a real `claude` in a real tmux, neither of which any fake could have produced:

- **A multi-line payload does NOT submit early**, which is the fact the whole restated-answer design rests on. All lines stage in the composer and only a lone Enter submits — but this is a property of the terminal and the app, not of Grove, so it is a measurement rather than an assumption.
- **`send-keys -l` corrupts a MODAL composer.** With vim keybindings on, Escape leaves the composer in normal mode and the following literal text runs as editor commands — observed putting the pane into VISUAL mode. A bracketed paste (`set-buffer` + `paste-buffer -p`) inserts verbatim in either mode with a following Enter still submitting. **Grove cannot see whether the user enabled vim mode, so "it works on my composer" is not evidence about anybody else's.**
- **A named-key API is not text steering.** Parameterize every closed enum member and assert one bare `send-keys -t <target> <key>` argv: no `-l`, paste-buffer calls, capture, or Enter retry. Those mechanisms are correct for prose delivery but would change a one-key operation's meaning. For the live proof, a raw-mode byte sink isolates delivery from application behavior: ordinary termios turns Ctrl+C into a signal before a reader can log it. Minimal container Python may omit `tty`; `stty raw -echo` before the reader tests the same terminal mode.

The method that made this cheap: drive the REAL production functions in-process (`uv run python` importing `grove.core.tmux`) against a throwaway `tmux` session, and read the pane back with `capture-pane`. Three traps, all paid for. **The mode indicator is in the pane's last line** (`-- INSERT --` / `-- VISUAL --`) and is the only visible evidence of the failure — an assertion on the composer's text alone reads a corrupted send as a merely empty one. **Set the state you mean to test**: an experiment run after a stray Escape is testing normal mode whether or not you intended it, which is how a probe accidentally reproduces the real bug and how it accidentally hides one. And **a bare `claude` does not inherit a Grove agent's env**, so a gateway-backed profile answers "Login expired" and no model turn is possible — a probe needing a real tool call has to go through a real Grove workspace, while a probe about the terminal does not.

## A SEQUENTIAL A/B on a loaded host measures the load, not the code

`tests/integration/test_real_tmux_git.py` waits 5 s for a real tmux pane to touch a marker file, so it is load-sensitive by construction — and on a host running a fleet plus a full `pytest`, that budget is genuinely reachable.

The trap is not the flakiness, it is the **method used to diagnose it**. Run six iterations in one checkout and then six in another and the numbers look decisive: measured `6/6` at the base commit against `1/6` on a branch, which reads as an obvious regression and sent a whole investigation after a change that touched nothing on that path. Re-running the *same* branch minutes later gave `6/6`. **Interleaved** — alternate the two checkouts within each round — both sides were `8/8`.

Three rules fall out, and the third is the one that costs the most when ignored:

- **Interleave the arms of any A/B whose subject is timing.** Sequential blocks confound the variable with whatever else the machine was doing between them.
- **One passing run does not refute a regression, and one failing run does not establish one.** A single "reverted it and it passed" was the 1-in-6 lucky pass, and it was reported here as a confirmation.
- **A worktree is the right isolation and `git reset --hard` inside one is not.** Recovering a discarded branch tip needs `git reflog` plus `git stash pop` for anything uncommitted; commit before you experiment, so the worst case is a reflog lookup rather than lost work.

## Session lessons

- **CLI output tests must scan for their marker line, never index `splitlines()[0]`.** A command that "falls back loudly" (prints a warning line before its normal output) and a test that parses only the first line are incompatible conventions — the warning becomes line 0 and silently breaks the assertion. Scan the full output for the marker instead of assuming a fixed line position.
- **Absence assertions must match the template markers they own, not every occurrence of a character in expanded output.** Rendered environment values can legitimately contain syntax-like characters (for example, a package path containing `@`), so rejecting any such character turns a valid host environment into a false failure. Scope the assertion to the unresolved placeholder grammar (`@NAME@` here), and keep the production input visible in the test.
- Engine-side gotchas that *manifest* as test traps live with the engine: e.g. ROOT-placement tests must spy that `worktree_remove` / `branch_delete` are never *called*, not merely that the repo survives (git itself refuses both, so a survival-only assertion passes with the gate missing) — see [core](../src/grove/core/CLAUDE.md).
- **Live-driving a real `claude` in tmux (manual e2e verification) has three traps**, each of which costs a failed run: (1) the boot marker must be claude's own chrome (`"Claude Code v"` / `"? for shortcuts"`) — the zsh prompt glyph is ALSO `❯`, so a prompt-glyph check fires at the shell stage; (2) an Enter sent while the TUI is still painting is **swallowed** — the text stages in the composer unsubmitted, so verify submission by re-capturing the pane ~4s later and re-send Enter if the text is still staged (this race also exists for real `send_message` steering into a just-booted agent); (3) the trust-dialog copy is "Quick safety check: … Is this a project you created or one you trust?" / "Yes, I trust this folder" — match on `trust this folder`, not "trust the files". Run the engine **in-process from the worktree** (`uv run python`), never through the installed `grove` (it serves the last-installed checkout, not your branch).