[tusharui]

Summary

Closes #1341. search_file_contents could return 0 matches (with truncated: true)
when ripgrep's early-kill match budget (rgMaxCount) was consumed by matches inside
ignored directories. ripgrep still counts matches in ignored paths toward its
budget, so those lines were burned before reaching any kept result — and since the
only drop to ripgrep was the --max-count, the JS filter downstream never got a
kept line at all.

Fix

Two layers, applied together:

Layer 1 — don't count ignored output against the budget
runRipgrep now streams results via onLine and increments the kill counter only
for kept (non-ignored) type: "match" lines, against rgMaxCount = maxResults + contextLines.

Layer 2 — keep ignored paths out of ripgrep entirely
A merged ignore set (DEFAULT_IGNORE_DIRS → .gitignore → .nanocoderignore) is
written to a temp --ignore-file and wired into both searchProjectContents and
walkProjectEntries, so rg never traverses/matches ignored directories in the first place.

Correctness backstop: the existing downstream JS filter (projectIgnore.ignores) still applies.

Hardening (follow-up commit, from maintainer review)

• **/-prefixed rules kept verbatim (git depth-agnostic semantics)
• cross-drive guard for rules outside the project root
• temp ignore file written with mode: 0o600
• git-compliant trailing-whitespace trimming (escaped trailing space preserved)
• empty/comment-only .gitignore skips the temp file entirely
• bounded Layer-1 buffer: onLine is tri-state (keep | skip | stop); stdout holds only kept lines
• per-file prune propagated to walkUnsortedFileStream / searchProjectContents

Dev tooling (same branch)

• pnpm test:all now runs via scripts/test-all.mjs (cross-platform Node orchestrator,
  same 7 gates as scripts/test.sh), so it works on Windows too.

Testing

• New AVA tests: [Bug] search_file_contents returns 0 matches when ripgrep kills early on gitignored directory matches #1341 plain, [Bug] search_file_contents returns 0 matches when ripgrep kills early on gitignored directory matches #1341 with context, **/-anchored rebase guard,
  walkProjectEntries pruning, rebaseIgnoreLine unit suite, ! re-include,
  abort temp-dir cleanup
• Gates green (Linux CI + locally): test:format, test:lint, test:types,
  test:types:vscode, test:knip, test:audit
• Remaining local ava failures are pre-existing Windows-environment issues
  (confirmed reproducible on pre-fix code) — unaffected by this change

Changeset

.changeset/search-ignored-dir-budget.md — patch — Closes #1341

[github-actions]

nc-review: needs work — 1 blocking, 1 important, 2 nits

@tusharui — there is a blocking item below.

PR #1369 addresses a documented bug (issue #1341) where ripgrep's match-count kill budget could be exhausted by matches in default-ignored directories, leaving downstream JS filters with zero kept results. The described two-layer fix (only count kept matches; pass a merged ignore file to rg) is conceptually correct and aligns with the architecture in source/utils/file-search.ts. The changeset file is appropriate for a user-facing bug fix. However, only one of the seven claimed new tests ('a .nanocoderignore directory is pruned during traversal') is visible in source/utils/file-search.spec.ts on disk; the regression tests named for the bug (#1341 plain, #1341 with context, **/-anchored rebase guard, rebaseIgnoreLine unit suite, ! re-include, abort temp-dir cleanup) are not present, so the new behaviour has no test that would fail if it regressed. The cross-platform scripts/test-all.mjs change is unrelated to the bug fix and is bundled into this PR.

🔴 blocking · tests · source/utils/file-search.spec.ts

The PR description claims seven new AVA tests: '#1341 plain', '#1341 with context', '**/-anchored rebase guard', 'walkProjectEntries pruning', 'rebaseIgnoreLine unit suite', '! re-include', and 'abort temp-dir cleanup'. Only one — 'a .nanocoderignore directory is pruned during traversal, not just filtered from results' (around line 411) — is visible in the spec file on disk. The two regression tests that would actually fail if the Layer 1 (kept-match-only counting) fix regressed — ones that build a fixture with many matches in a default-ignored directory (e.g. dist/) and a few matches in source/, then call searchProjectContents with a small maxResults — are absent. Per the project rubric, a bug fix must include a regression test that actually exercises the bug; the existing tests in the file all have matches concentrated in a single non-ignored file and so would not catch a regression where rg again exhausts its budget on ignored matches. The Layer 2 path has the prune test above, but Layer 1 has no equivalent.

Reproducing #1341 in a test looks like this (sketch — adapt to the actual API):

mkdirSync(join(testDir, 'dist'), {recursive: true});
mkdirSync(join(testDir, 'src'), {recursive: true});
// dist/ contains 20 matches for the query
for (let i = 0; i < 20; i++) writeFileSync(join(testDir, 'dist', `f${i}.js`), 'searchTarget');
writeFileSync(join(testDir, 'src', 'app.ts'), 'searchTarget');
const result = await searchProjectContents('searchTarget', testDir, 5, false);
t.is(result.matches.length, 1);
t.is(result.matches[0]?.file, 'src/app.ts');

A test like this would have failed on the base code (0 matches) and passes after the fix (1 match). Without it, the fix could regress silently.

🟠 important · scope · scripts/test-all.mjs

The diff adds a new scripts/test-all.mjs cross-platform orchestrator and updates package.json to point test:all at it. This is a drive-by dev-tooling change bundled into a bug-fix PR. The two changes have different reviewers, different rollback profiles, and different test gates. Per the project rubric, 'drive-by refactors mixed into a functional change are harder to review and harder to revert'. The Windows-compatibility fix for the test runner deserves its own PR with its own discussion, especially since it changes the default for every contributor who runs pnpm test:all.

⚪ nit · correctness · source/utils/file-search.ts

I cannot fully verify the correctness of the new runRipgrep budget logic without seeing the actual diff. The base code increments on every type: 'match' JSON line regardless of which file it came from, so the conceptual fix (only increment for lines that pass a kept-filter callback) is sound, but the contract between runRipgrep's new onLine tri-state return and the call sites in searchProjectContents / walkUnsortedFileStream / the sorted walkProjectEntries path is worth checking: the sorted path in walkProjectEntries does not pass onLine and just consumes stdout wholesale, so the Layer 1 fix only applies where onLine is wired in. If the sorted path still runs rg on content search (it doesn't, it uses --files), it would need the same treatment; for the --files walk the budget concern is maxRawFilesScanned, which is already capped correctly. Worth a maintainer's eye during merge review.

⚪ nit · completeness · .changeset/search-ignored-dir-budget.md

The changeset is present and well-shaped (closes the issue, patch bump, names the right package). No issue here — flagging only because the rubric asks reviewers to confirm the changeset, and this one is correct.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}

[will-lamerton]

Thanks @tusharui, the search fix itself looks right: counting only kept matches plus the merged --ignore-file addresses #1341, and the regression tests are there (the bot's blocking item missed them). Requesting changes mainly on scope.

Please split out of this PR:

• source/tool-calling/xml-parser.ts - a real behaviour change to fallback tool-call parsing with no tests. It's a good fix (fenced blocks inside <content> and = in values like --color=auto are dropped on main), so it deserves its own PR with those cases as specs.
• The nosemgrep annotations - Semgrep is green on main without them, and the bash-sandbox.ts one sits on the wrong line for a line-level suppression.
• pnpm-workspace.yaml (ignoreGhsas: null -> {}) - pnpm rewrite churn, please revert.
• scripts/test-all.mjs / test:all - separate tooling PR.

Smaller points on the search change:

• --ignore-file anchored rules (e.g. /src/generated/) don't prune when the path goes through a symlink, since rg anchors at the resolved cwd. macOS tmpdir() hits this, so the "cwd-anchored ... do not over-prune" test passes only via Layer 1 there. Results stay correct thanks to Layer 1, but realpath-ing the search root would make Layer 2 do what its doc comment says.
• "walkProjectEntries prunes ignored directories during traversal" passes on the JS filter alone; to prove traversal pruning it needs a signal like maxRawFilesScanned, as the existing .nanocoderignore test does.
• In the searchProjectContents onLine, check parsed.type before reading parsed.data.path (or use data?.) so a line without data can't throw inside the stream handler.

Please also merge main for a fresh CI run; the current failure (UserInput redoes an undone edit with ctrl+y) is unrelated.

[github-actions]

Hi @tusharui, thanks for this PR! It looks like a maintainer has left feedback
or review activity and there are still some outstanding items to wrap up.

Whenever you get a chance, could you take a look at the open comments?
If anything is unclear or you'd like a hand, just reply here and we'll help you get it across the line.

[addyCooks]

Thanks @tusharui, the scope is clean now and all the review points are in. I checked it: against main's file-search.ts 9 of the new tests fail, and #1341 reproduces with the real ripgrep (searching src/ with a root .gitignore that ignores generated/ gives "No matches found" on main and finds src/main.ts here). Types, lint, knip and Semgrep are clean, CI is green, and per-search overhead is about 5 ms.

Approving 🎉. @will-lamerton your review points (scope split, realpath for anchored rules, traversal-pruning test, narrowing the JSON line on type) look addressed to me; could you re-check?

[github-actions]

Hi @tusharui, thanks for this PR! It looks like a maintainer has left feedback
or review activity and there are still some outstanding items to wrap up.

Whenever you get a chance, could you take a look at the open comments?
If anything is unclear or you'd like a hand, just reply here and we'll help you get it across the line.

[github-actions]

Hi @tusharui, just a heads-up that this PR still has outstanding items
and hasn't seen any activity in a while.

If we don't hear back within about a week, we'll close it to keep the queue tidy.
You can always reopen it later. Happy to help if anything is blocking you - just say the word.

[github-actions]

Closing this PR for now since there hasn't been any activity from @tusharui after the earlier heads-up.No worries at all - this isn't a rejection. If you'd like to pick it back up, just push a new commit or leave a comment and reopen it. Thanks for the contribution!