Skip to content

#1094 Remove the ambient ripgrep dependency from the test suite - #1097

Merged
williamthorsen merged 9 commits into
mainfrom
1094
Jul 28, 2026
Merged

#1094 Remove the ambient ripgrep dependency from the test suite#1097
williamthorsen merged 9 commits into
mainfrom
1094

Conversation

@williamthorsen

@williamthorsen williamthorsen commented Jul 28, 2026

Copy link
Copy Markdown
Owner

What

Tests and the post-build smoke check no longer depend on ripgrep. Separately, the knowledge-base retrieval skills now fail with an explicit error when ripgrep returns output they cannot interpret, instead of reporting an empty result when matching notes exist.

Why

Thirty-seven tests across four files required rg on PATH, and a workflow modernization that dropped the step installing it left the build red. Most of those tests had no stake in ripgrep, reaching it only in passing, so an absent binary failed tests about candidate tables and filters with an error naming the recall module rather than the missing prerequisite.

Taking every test off the real binary also removes the only thing that would have caught a change to ripgrep's --json event format, so recall has to detect that condition itself.

Details

♻️ Refactoring

  • recallNotes accepts an optional process runner and reaches rg only through it, so its own tests assert the constructed argument list and exercise the exit-code and missing-binary branches without spawning anything. The runner's contract matches the shape packages/fleet/src/forge/github-adapter.ts already establishes.
  • searchNotes accepts an optional recall function, threaded through runRetrieve in both retrieve CLIs, so a test of scoping, filtering, or projection supplies hits directly and never reaches the process layer. The seam is recall rather than the process runner because threading the runner that far would force consumer tests to author ripgrep --json events carrying machine-absolute paths, which is matching reimplemented in test code.
  • searchKb throws when ripgrep returns output that parses to no entries. ripgrep exits 1 when nothing matched, so a zero exit carrying output guarantees at least one match event, and parsing none of it means the --json event shape no longer matches what the module reads.
  • searchKb maps parsed entries straight to hits, dropping a per-path de-duplication that parseRipgrepOutput already guarantees.

🧪 Tests

  • recall.test.ts pins the full ripgrep argument list, and asserts metacharacter escaping, disjunctive term combination, and alias expansion against the constructed pattern. All four failure branches (no-match exit, absent binary, other non-zero exits, unparseable output) run through a fake runner.
  • Structural path extraction and the snippet cap move to parseRipgrepOutput tests over synthetic events, replacing four tests that proved the same point by matching real notes under date-patterned paths.
  • Both retrieve CLI test files and search.test.ts declare which notes recall found, through a shared buildRecallStub helper that resolves basenames to absolute paths and attributes each hit to its in-scope KB. Declaring the hit set is a stronger statement than the implicit matching it replaces, and it lets several assertions tighten from a lower bound to an exact count.
  • The kb-retrieve-events smoke pairing scopes to a store the registry does not carry, which short-circuits before recall while still asserting a structural result rather than falling back to the bare no-args baseline.
  • Four fixture notes existed only to be matched by the real binary and are removed with the tests that read them.
  • Verified by prepending a directory holding an rg shim that logs its arguments and exits non-zero: the full repository suite, the post-build smoke run, and nmr build && nmr check:strict all pass with an empty log.

Closes #1094

@github-actions

Copy link
Copy Markdown

Dependency audit

Production dependency audit passed.

@williamthorsen williamthorsen added refactoring Improvement to code without change in functionality and removed tests Tests labels Jul 28, 2026
@williamthorsen williamthorsen self-assigned this Jul 28, 2026
Running the recall tests no longer requires ripgrep on PATH.

Recall now fails loudly when ripgrep reports matches whose output it cannot parse, rather than returning the same empty result a vault with nothing to find would produce.
Running the search tests no longer requires ripgrep on PATH.

A test names the notes recall found instead of relying on a query matching the right fixture notes, so what it asserts about scoping, filtering, and projection no longer depends on the matcher.
Running the retrieve CLI tests no longer requires ripgrep on PATH.

Each test states the notes recall found, so what it asserts about candidate tables, filters, and diagnostics no longer depends on a query happening to match the right fixture notes.

The --diataxis test recalls a reference note alongside the how-to notes, giving the filter it exercises something to exclude.
The post-build smoke run no longer requires ripgrep on PATH, so no step of the build depends on an external binary.

The kb-retrieve-events pairing scopes to an unregistered store, asserting the resolver reached a scope verdict and shaped its JSON result, rather than recalling a seeded event.
The recall stub builds its missing-path set from the optional argument directly, and the search test names a nested fixture path without composing two joins.
Comments on the recall seam, its stub, and the smoke pairing state the contract and the non-obvious invariants alone, dropping the rationale for introducing them and the description of what the tests did beforehand.
`parseRipgrepOutput` returns at most one entry per note path, so `searchKb` builds its hits with a direct map rather than a keyed map whose duplicate check can never fire.
Each of these tests declares the notes recall found, so the surviving candidate count is exactly one. The assertions state that count rather than a lower bound.
`multi-match.md`, `2026-05-01-meeting-notes.md`, `2024-archive/legacy-runbook.md`, and `2026-06-01/daily-log.md` each carried a unique term for a recall test that matched it through the real binary. Those tests now assert the ripgrep argument list and parse synthetic events, so no test reads any of the four notes.
@williamthorsen
williamthorsen marked this pull request as ready for review July 28, 2026 18:17
@williamthorsen
williamthorsen merged commit 164d0b6 into main Jul 28, 2026
4 of 6 checks passed
@williamthorsen
williamthorsen deleted the 1094 branch July 28, 2026 18:19
williamthorsen added a commit that referenced this pull request Aug 4, 2026
…e-v0.2.2 codeassembly-v0.4.0 factory-v0.2.2 codeassembly-mcp-v0.2.2

codeassembly-v0.4.0
- #1153 feat: Make codeassembly and kb CLI tools publishable (#1164)
- #1091 fix: Anchor a project-deployed link where its target deploys (#1159)
- #1156 refactor: Rename packages to publishable names (#1157)
- #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155)
- #1110 tooling: Migrate Vitest to nmr's centralized model (#1154)
- refactor: Refine typescript-preferences
- #1150 fix: State doc-description form and make comment mood opt-in (#1151)
- deps: Upgrade first-party linting deps to latest version
- #1137 feat: Establish personal rulebooks for code layout and TypeScript preferences (#1144)
- #1126 feat: Check a package's own guidance content before it ships (#1138)
- refactor: Fix lint
- tooling: Remove redundant lint rules
- #1133 fix: Reject an anchor link that names no heading (#1135)
- #1122 feat: Capture lede decisions as an accumulating corpus (#1132)
- #1125 feat: Honor invocation tokens in rulebook bodies (#1129)
- #1107 feat: Render rulebook links and path tokens per harness (#1124)
- #1115 feat: Sync guidance at build and install so an upgrade cannot leave it stale (#1123)
- #1114 feat: Adopt a dependency's guidance by naming the package (#1121)
- #1088 feat: Deliver project ambient rulebooks per harness (#1113)
- #1087 refactor: Rename the authoring rulebook and mark its enforced rules (#1092)
- #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102)
- #1094 refactor: Remove the ambient ripgrep dependency from the test suite (#1097)
- deps: Upgrade all deps to latest version
- tooling: Upgrade all deps to latest version & modernize configs
- refactor: Fix lint
- #1077 feat: Show the proposed edit above post-review menu options (#1078)
- #1068 feat: Deliver ambient rulebooks mechanically, retiring GLOBAL.md (#1075)
- #1069 feat: Add a no-second-person rule to the lede-voice doctrine (#1070)
- #1050 feat: Add a redundancy rule to the lede-voice doctrine (#1058)
- #1035 internal: Add lifecycle workspace with the canonical envelope, vocabulary & lane fold (#1049)
- #1006 internal: Extend lifecycle-event instrumentation to five high-traffic skills (#1034)
- #1028 feat: Rule out absence-of-removed-code tests (#1032)
- #114 feat: Let reviewers emit gated insights into review artifacts (#1031)
- #1027 internal: Retire input.received and redundant skill.progress emits (#1030)
- #1026 fix: Name the side effects an approval ask authorizes (#1029)
- #1022 fix: Normalize action and question label identifiers across asks blocks (#1025)

factory-v0.2.2
- #1156 refactor: Rename packages to publishable names (#1157)
- deps: Upgrade all deps to latest version
- #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155)
- #1110 tooling: Migrate Vitest to nmr's centralized model (#1154)
- tooling: Remove redundant lint rules
- deps: Upgrade deps to latest version
- deps: Upgrade all deps to latest version
- deps: Upgrade all deps to latest version
- tooling: Upgrade all deps to latest version & modernize configs
- refactor: Fix lint
- deps: Upgrade all deps to latest version

kb-v0.3.0
- #1153 feat: Make codeassembly and kb CLI tools publishable (#1164)
- #1156 refactor: Rename packages to publishable names (#1157)
- #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155)
- #1110 tooling: Migrate Vitest to nmr's centralized model (#1154)
- tooling: Remove redundant lint rules
- #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102)
- tooling: Upgrade all deps to latest version & modernize configs
- refactor: Fix lint

codeassembly-lifecycle-v0.2.0
- #1153 feat: Make codeassembly and kb CLI tools publishable (#1164)
- #1156 refactor: Rename packages to publishable names (#1157)
- #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155)
- #1110 tooling: Migrate Vitest to nmr's centralized model (#1154)
- tooling: Remove redundant lint rules
- #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102)
- tooling: Upgrade all deps to latest version & modernize configs
- #1038 feat: Add the read-only git adapter for worktree and base-branch ground truth (#1059)
- #1051 feat: Bound fold memory and rescan cost with a retention window (#1057)
- #1035 internal: Add lifecycle workspace with the canonical envelope, vocabulary & lane fold (#1049)

codeassembly-mcp-v0.2.2
- deps: Upgrade all deps to latest version
- #1156 refactor: Rename packages to publishable names (#1157)
- #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155)
- #1110 tooling: Migrate Vitest to nmr's centralized model (#1154)
- #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102)
- tooling: Upgrade all deps to latest version & modernize configs
- refactor: Fix lint

codeassembly-run-core-v0.2.2
- #1156 refactor: Rename packages to publishable names (#1157)
- #1152 tooling: Run every test in the default gate, classified by what it reaches (#1155)
- #1110 tooling: Migrate Vitest to nmr's centralized model (#1154)
- #1095 tooling: Move compilation out of the install lifecycle into a bootstrap step (#1102)
- tooling: Upgrade all deps to latest version & modernize configs
- refactor: Fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring Improvement to code without change in functionality scope:agents

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant