fix: keep a whole-project check out of gitignored paths

[thecodedrift]

What

A whole-project taskless check no longer reports findings from paths git ignores. An explicitly named path is still checked, and a non-git directory still gets the walk that shipped before.

Why

check reported prose findings from inside gitignored directories. The reported case was a git worktree at worktrees/<name>/ — a complete second checkout — so every Vale rule fired a second time over another branch's documents, including code an agent was mid-edit on. The finding count then moves when a worktree appears or disappears with nothing in the output explaining why. The general shape is the same for dist/, vendored trees, and scratch directories: a check reporting on files nobody maintains.

Only one engine was actually wrong

Worth flagging, because the issue says "every rule fires a second time" and that turns out to be half right.

ast-grep's walker is the ignore crate, and sgWalkArgv has always passed --no-ignore hidden without vcs — so a bare scan already skipped ignored trees. Re-measured against the pinned 0.41.0: nothing under a gitignored worktrees/probe/, and nothing under a hidden-and-ignored .turbo/ either, which is the case that would expose --no-ignore hidden if it had ever disabled more than hidden-file skipping. Vale has no notion of a VCS and walked everything.

So the two static engines disagreed about which files the project contains, and only the prose findings duplicated — which matches the issue's own reproduction, where all three extra findings were the docs-npx-cli Vale rule.

Nothing in scan.ts changes behavior. What changes is that the property is now documented as load-bearing and pinned by a test, so a later --no-ignore vcs cannot hand the ignored tree back to the scan silently.

Measured

On a fixture repository (mixed-engines-project, two sg rules and two Vale rules) with worktrees/ gitignored and git worktree add worktrees/probe present:

Run	Before	After
bare check, no worktree	4	4
bare check, worktree present	6	4
check worktrees/probe	4	4

The two findings that leave are both Vale, both a second copy of a finding already reported against the tracked file. Both ast-grep findings were already absent from the ignored copy before the change.

Mechanism

git ls-files --others --ignored --exclude-standard --directory -z, run once per whole-project Vale invocation, rendered into Vale's single --glob alternation.

• git, not a parser. .gitignore is not one file or one syntax question once nested ignore files, .git/info/exclude, a global core.excludesFile and negation patterns are involved. A library approximates that; git is it, check already shells out to git elsewhere, and it answers in one call. No new dependency.
• The complement, not the set the issue proposed. --cached --others is every file in the project — thousands of positional arguments and an ARG_MAX problem on a large repository. --directory collapses a wholly-ignored directory to one entry, so node_modules/ is one line rather than forty thousand.
• -z. A filename may legally contain a newline; without it git quotes and C-escapes such a path and a line split yields two entries naming nothing.
• Explicit paths are exempt, on the same terms as the existing .taskless/ exclusion and via the same isWholeProjectWalk predicate — so check . counts as whole-project and check worktrees/probe does not.
• Non-git falls back. Every failure mode returns an empty ignore set: not a repository, no git on PATH, git errors. The engines then walk exactly as before.
• Standing inside an ignored directory ignores nothing. Measured: from inside a gitignored build/, git answers ./ — "everything here" — and honouring that would return an empty check with nothing saying why. Today Vale's matcher happens not to match README.md against ./**, so this is a hardening of a behavior that was accidentally correct, not a bug fix.
• Entries carrying glob metacharacters are dropped rather than escaped. A comma is the dangerous one: buildValeGlob joins with , inside !{…}, so one comma would split into two wrong patterns. Dropping such an entry means one pathologically-named ignored path is still linted — which is what shipped before, so it is a gap, not a regression. The converter skip notice uses literal string matching and has no such restriction.

Also

The converter skip notice no longer names files inside ignored paths. An .adoc under worktrees/ is not a file this run declined to convert — it is a file this run was never going to open, and naming it would send the reader to investigate a directory the exclusion exists to keep out.

Verification

pnpm typecheck, pnpm lint, pnpm test (840 tests) all pass. The new bare-walk assertion was confirmed non-vacuous by reverting the source and rebuilding: it fails, and only it fails.

Fixes #166

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[thecodedrift]

@claude /review

[claude]

Claude finished @thecodedrift's task in 3m 12s —— View job

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains only the triggering @claude /review comment and a Copilot review that failed with a quota error (no actual findings recorded) — there is nothing previously raised to classify against, so this is effectively the first substantive review of this PR.

• Read .prior-review.json (no prior findings)
• Gather PR diff (origin/main...HEAD)
• Review packages/cli/src/rules/git-ignored.ts (new)
• Review packages/cli/src/rules/scan.ts changes
• Review packages/cli/src/rules/vale/run.ts changes
• Review test coverage in packages/cli/test/check-gitignore.test.ts
• Review changeset
• Post inline comments for concrete issues
• Post final summary

Assessment

This is a well-scoped, well-documented fix. Walked the whole diff line by line, traced listGitIgnoredEntries / gitIgnoredExclusionGlobs / isGitIgnoredPath to their only call site (vale/run.ts — confirmed via grep, no other consumers), checked the ROOT_ENTRIES / walk-root-is-ignored handling against its test, and cross-checked isWholeProjectWalk semantics for ./explicit-path cases against both the ast-grep and Vale code paths.

• scan.ts — comment-only change (documents that --no-ignore hidden without vcs was already load-bearing). No behavior change, confirmed by diff.
• git-ignored.ts — the git ls-files --others --ignored --exclude-standard --directory -z approach is sound: -z correctly avoids the newline-in-filename quoting trap, --directory avoids ARG_MAX, and every git-failure path degrades to an empty ignore set (fail open to the pre-existing walk), matching the PR's stated design. The "walk root itself is ignored → ignore nothing" special case is deliberately conservative and is covered by a test.
• vale/run.ts — wholeProject-gating of the new exclusion matches the existing .taskless/** precedent (explicit paths stay exempt), and filtering the converter-skip notice through isGitIgnoredPath (unfiltered by glob metacharacter restrictions, per the doc comment) correctly avoids naming a file the run never intended to look at.
• Tests — good coverage: bare walk vs. explicit path vs. non-git fallback, directory collapsing, the ignored-root edge case, and the glob-metacharacter-drop behavior for both the exclusion globs and the notice matcher. The non-vacuousness of the bare-walk assertion is called out in the PR body as manually verified by reverting the source.

One inline finding posted, non-blocking:

• packages/cli/src/rules/vale/run.ts:221-250 — listGitIgnoredEntries (git subprocess) and findConverterDependentFiles (fs glob walk) are independent of each other's results and currently run as two sequential awaits rather than concurrently via Promise.all. Not a correctness issue — just avoidable latency added ahead of the Vale subprocess spawn on every whole-project run.

No correctness, security, or test-adequacy issues found beyond that.

[claude]

[New] Minor efficiency nit: listGitIgnoredEntries (a git subprocess) and findConverterDependentFiles (an fs glob walk) are independent — neither's result feeds the other, only the final .filter(...) combines them — but they're awaited sequentially:

const ignoredEntries = wholeProject
  ? await listGitIgnoredEntries(options.cwd)
  : [];
...
const converterDependent = await findConverterDependentFiles(options.cwd, paths);

Running them concurrently (Promise.all) would shave the git spawn + directory walk latency off every whole-project Vale invocation instead of paying for both in series before Vale itself even starts. Not a correctness issue, just avoidable added latency on a path that already spawns a Vale subprocess afterward.