Tests depend on an ambient ripgrep binary by shelling out to it

[williamthorsen]

Problem

packages/agents/src/kb-search/recall.ts shells out to rg, and its tests exercise it by spawning the real binary. Four vitest files (37 tests) and one post-build smoke pairing therefore require rg on PATH.

Most of those tests have no interest in ripgrep. They reach it transitively through searchNotes → recallNotes, so an absent binary fails tests about candidate tables and filters with an error pointing at recall.ts.

CI has been red since 23d9143e deleted the setup-command that installed ripgrep, during an unrelated workflow modernization (example failing run).

Context

Site	Tests	Reaches rg
kb-retrieve/__tests__/cli.test.ts	15	Transitively
kb-search/__tests__/recall.test.ts	12	Directly
kb-retrieve-events/__tests__/cli.test.ts	6	Transitively
kb-search/__tests__/search.test.ts	4	Transitively
scripts/testing/smoke-test-utils.ts, kb-retrieve-events pairing	1	Via the built bundle

What this repo owns and must test: the argument list built in runRipgrep, the parsing of rg --json into path plus snippet, per-note dedup and the three-line snippet cap, alias expansion, tokenizing, and missing-KB skipping. All are pure and testable without a process.

What ripgrep owns and this repo should not test: exit code 1 meaning no matches, --context emitting neighbor events, --glob filtering as documented.

packages/fleet/src/forge/github-adapter.ts already establishes the pattern, defining an injectable process runner so that "CI needs no gh binary".

#1096 covers the same class of dependency on git.

Proposed solution

rg stays the production recall backend, rather than being replaced by a pure-Node search. A Node implementation would drop the prerequisite from the shipped skill, but ripgrep honors .gitignore by default, so matching would either change for vaults carrying ignore files or need that behavior reimplemented. Changing shipped recall semantics to solve a test problem is the wrong trade.

No test spawns a process, and none is added that spawns one on demand. A deliberate-only integration test would not earn its keep here: excluded from CI by design, it runs only when someone already suspects a problem, so it cannot catch drift automatically.

The recall path becomes injectable so consumer tests never reach the binary, and recall's own tests assert the constructed argument list and the parsing instead of the binary's behavior. The kb-retrieve-events smoke pairing moves off the recall path: resolver, record layer, and note-io are already covered by the kb-update-events pairing, and the bundle-load risk the smoke harness exists for is caught by any successful invocation.

Because nothing will then exercise real ripgrep, recall must distinguish "ripgrep found nothing" from "ripgrep emitted output we could not parse". Today a schema change makes every event unparseable and surfaces as an empty result, reporting no matches for a vault full of them.

No CI setup-command is restored. The end state needs no external binary on the CI path.

Acceptance criteria

Must have

• No test in the repository spawns an rg process, in the vitest suite or the post-build smoke run.
• nmr build && nmr check:strict passes on a machine with no rg on PATH.
• The argument list passed to rg is asserted by a test.
• When ripgrep reports matches but no output line parses, recall surfaces an error rather than an empty result.
• New and modified behavior in this change is covered by tests.

Should have

• The kb-retrieve-events smoke pairing still asserts a structural result rather than falling back to the bare no-args baseline.