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

.changeset/check-honors-gitignore.md

@@ -0,0 +1,47 @@
+---
+"@taskless/cli": patch
+---
+
+Keep a whole-project `check` out of the paths git ignores.
+
+`check` reported prose findings from inside gitignored directories. The case
+that surfaced it was a git worktree at `worktrees/<name>/`, which is a complete
+second checkout: every Vale rule fired again over another branch's documents,
+including code an agent was mid-edit on. That makes the finding count move when
+a worktree appears or disappears with nothing in the output explaining why, and
+the general shape is the same for `dist/`, vendored trees, and local scratch
+directories — a check reporting on files nobody maintains.
+
+Only one engine was wrong, which is why it was hard to attribute. ast-grep's
+walker is the `ignore` crate and `sgWalkArgv` has always passed `--no-ignore
+hidden` without `vcs`, so a bare scan already skipped `worktrees/`; measured
+against the pinned 0.41.0, it skips a hidden-_and_-ignored `.turbo/` too. 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. On a fixture repository with a worktree present, a bare `check`
+went from 6 findings to 4; the two that left were both Vale, both a second copy
+of a finding already reported against the tracked file.
+
+The set comes from `git ls-files --others --ignored --exclude-standard
+--directory -z`, which is the complement of the tracked-plus-untracked set the
+question is usually phrased as. The complement is the one that scales:
+`--directory` collapses a wholly-ignored directory to a single entry, so
+`node_modules/` costs one line rather than forty thousand, and the result is
+short enough to hand Vale as `--glob` exclusions without meeting `ARG_MAX`. No
+new dependency — `.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, and git already answers all of it in one call.
+
+The exclusion belongs to the walk `check` chose for itself. `check
+worktrees/probe` names an ignored path deliberately and still checks it, on the
+same terms as the existing `.taskless/` exclusion. A directory that is not a
+git repository, or a host with no `git` on its `PATH`, gets an empty ignore set
+and the walk that shipped before this change. Standing _inside_ an ignored
+directory is treated as explicit too: git answers `./` there, meaning
+"everything here", and honouring that would return an empty check with nothing
+saying why.
+
+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 is there to keep out.

packages/cli/src/rules/git-ignored.ts

@@ -0,0 +1,156 @@
+import { execFile } from "node:child_process";
+
+/**
+ * What git considers ignored, and how the engines are told to skip it.
+ *
+ * ## Why this exists
+ *
+ * A whole-project `check` reported findings from paths git ignores — build
+ * output, vendored trees, and (the case that surfaced it, taskless/cli#166) a
+ * git worktree at `worktrees/<name>/`, which is a complete second checkout. A
+ * worktree is the worst shape of the bug because it duplicates *every* finding
+ * and attributes each copy to a branch the user is not working on, so the
+ * finding count moves when a worktree appears or disappears with nothing in the
+ * output explaining why.
+ *
+ * ## The two engines do not need the same amount of help
+ *
+ * **ast-grep already honors `.gitignore` and always has.** Its walker is the
+ * `ignore` crate, and `sgWalkArgv` passes `--no-ignore hidden` *only* —
+ * deliberately not `vcs`, which is the value that would switch VCS ignore files
+ * off. Measured against the pinned 0.41.0 in a repository ignoring
+ * `worktrees/`: a bare scan reports nothing under `worktrees/probe/`, and it
+ * reports nothing under a hidden-*and*-ignored `packages/cli/.turbo/` either,
+ * which is the case that would expose `--no-ignore hidden` if that flag had
+ * quietly disabled more than hidden-file skipping. So nothing in `scan.ts`
+ * changes; `check-gitignore.test.ts` pins the behavior so a future flag edit
+ * cannot take it away silently.
+ *
+ * **Vale honors nothing.** It has no notion of a VCS, walks `.` as handed to
+ * it, and reads hidden directories by default — so every gitignored document in
+ * the tree was linted. That is the whole of taskless/cli#166, and this module
+ * is what closes it.
+ *
+ * ## Why `git ls-files` and not a `.gitignore` parser
+ *
+ * The styleguide's rule about not adding a dependency to answer a question the
+ * existing toolchain can answer applies squarely. `.gitignore` is not one file
+ * and not one syntax question: the real answer folds in nested `.gitignore`s at
+ * every level, `.git/info/exclude`, the user's global `core.excludesFile`, and
+ * negation patterns that re-include a path a parent excluded. A parsing library
+ * approximates that; git *is* it, it is already required for `check`'s other
+ * work (see `util/git-remote.ts`), and it answers in one call.
+ *
+ * The call is `--others --ignored --exclude-standard --directory`, which is the
+ * *complement* of the set the issue proposed (`--cached --others
+ * --exclude-standard`). Both describe the same partition; the complement is
+ * chosen because it is the one that scales. The positive set is every file in
+ * the project — thousands of paths that would have to reach Vale as positional
+ * arguments and would meet `ARG_MAX` on a large repository. The complement is
+ * short, because `--directory` collapses an entirely-ignored directory to a
+ * single entry: `node_modules/` is one line, not forty thousand, and this
+ * repository yields thirty entries in total.
+ */
+
+/**
+ * Paths git ignores under `cwd`, relative to it, directories with a trailing
+ * `/`.
+ *
+ * `-z` rather than newline-delimited output: a filename may legally contain a
+ * newline, and without `-z` git renders such a path quoted and C-escaped, so a
+ * line-split would produce two entries neither of which names a real file.
+ *
+ * Every failure mode returns an empty list, and that is the required behavior
+ * rather than a swallowed error: a directory that is not a git repository has
+ * nothing ignored, and it is the same answer a host with no `git` on its PATH
+ * must get. The engines then walk exactly as they did before this module
+ * existed, which is the non-git fallback the issue asks for.
+ */
+export async function listGitIgnoredEntries(cwd: string): Promise<string[]> {
+  const stdout = await new Promise<string>((resolve) => {
+    execFile(
+      "git",
+      [
+        "ls-files",
+        "--others",
+        "--ignored",
+        "--exclude-standard",
+        "--directory",
+        "-z",
+      ],
+      { cwd, maxBuffer: 16 * 1024 * 1024 },
+      (error, output) => {
+        resolve(error ? "" : output);
+      }
+    );
+  });
+
+  const entries = [
+    ...new Set(stdout.split("\0").filter((entry) => entry !== "")),
+  ];
+
+  // A walk root that is itself ignored answers `./`, and that single entry
+  // means "everything here". Measured by running `check` from inside a
+  // gitignored `build/`: git reports `./` and nothing else. Excluding it would
+  // hand the user an empty result set for a directory they deliberately stood
+  // in, with nothing in the output explaining where their findings went — the
+  // same silent-count-change complaint the issue opened with, inverted.
+  //
+  // Standing in a directory is as explicit as naming it, so the answer is the
+  // one an explicit path gets: ignore nothing. This does not currently change
+  // behavior — Vale's glob does not match `README.md` against `./**` — but that
+  // is a property of Vale's matcher, not a decision anyone made, and the whole
+  // failure mode is invisible. Decided here instead.
+  return entries.some((entry) => ROOT_ENTRIES.has(entry)) ? [] : entries;
+}
+
+/** How git can spell "the walk root itself" in a `--directory` listing. */
+const ROOT_ENTRIES = new Set(["./", "."]);
+
+/**
+ * Characters that would make an entry mean something other than itself once it
+ * is spliced into Vale's `--glob` alternation.
+ *
+ * A comma is the dangerous one: `buildValeGlob` joins patterns with `,` inside
+ * `!{…}`, so a filename containing a comma would split into two patterns, and
+ * both halves would be wrong. The rest are glob metacharacters that would turn
+ * a literal path into a matcher.
+ *
+ * An entry carrying any of them is left out of the exclusion rather than
+ * escaped. Vale's glob dialect is not ours to guess at, and the cost of leaving
+ * it out is that one pathologically-named ignored path is still linted — which
+ * is exactly the behavior that shipped before this module, so it is a gap
+ * rather than a regression. {@link isGitIgnoredPath} does not share the
+ * restriction, so such a path is still kept out of the skip notice.
+ */
+const GLOB_METACHARACTERS = /[*?[\]{},\\!]/;
+
+/**
+ * The ignored entries, rendered as patterns for Vale's `--glob`.
+ *
+ * A directory entry becomes `dir/**` rather than `dir/`, because Vale matches
+ * the pattern against files and never against the directory itself. A file
+ * entry is used verbatim: git already reports it as a path relative to the
+ * project root, and every pattern in the combined alternation is matched
+ * path-wise (see `buildValeGlob` in `formats.ts` for why that is a property of
+ * the whole expression rather than of one branch).
+ */
+export function gitIgnoredExclusionGlobs(entries: string[]): string[] {
+  return entries
+    .filter((entry) => !GLOB_METACHARACTERS.test(entry))
+    .map((entry) => (entry.endsWith("/") ? `${entry}**` : entry));
+}
+
+/**
+ * Whether `path` — relative to the project root — falls inside `entries`.
+ *
+ * Plain string work, deliberately: the entries are literal paths from git, so
+ * answering this with a glob engine would reintroduce the metacharacter
+ * problem {@link gitIgnoredExclusionGlobs} has to duck. This is what keeps the
+ * converter skip notice from naming files Vale was never going to open.
+ */
+export function isGitIgnoredPath(path: string, entries: string[]): boolean {
+  return entries.some((entry) =>
+    entry.endsWith("/") ? path.startsWith(entry) : path === entry
+  );
+}

packages/cli/src/rules/scan.ts

@@ -164,6 +164,18 @@ const EXCLUDED_DIRECTORIES = [TASKLESS_DIRECTORY, GIT_DIRECTORY] as const;
  * measured to pull `dist/` into the scan — a rule has no business reporting
  * findings in build output or vendored dependencies.
  *
+ * That omission is the whole of this engine's `.gitignore` handling, and it is
+ * load-bearing rather than incidental: ast-grep's walker is the `ignore` crate,
+ * so leaving `vcs` off is what keeps a bare scan out of `worktrees/`, `dist/`
+ * and vendored trees. Re-measured for taskless/cli#166 against 0.41.0 — a bare
+ * scan reports nothing under a gitignored `worktrees/probe/`, and nothing under
+ * a hidden-*and*-ignored `packages/cli/.turbo/` either, which is the case that
+ * would show up if `--no-ignore hidden` had ever disabled more than hidden-file
+ * skipping. Vale, which honors none of this, is handed the same set explicitly;
+ * see `rules/git-ignored.ts` for why the two engines need different amounts of
+ * help to agree. `test/check-gitignore.test.ts` pins both sides, so a later
+ * edit to this argv cannot quietly hand the ignored tree back to the scan.
+ *
  * **A `--globs` exclusion of `.taskless/`**, when we are the ones who chose to
  * walk the whole project. That directory is hidden, so it was never scanned
  * before and reaching it is not a fix: it is CLI-managed config the user did

packages/cli/src/rules/vale/run.ts

@@ -5,6 +5,11 @@ import { StringDecoder } from "node:string_decoder";
 import type { CheckResult } from "../../types/check";
 import { ASSEMBLED_VALE_CONFIG } from "../engines";
 
+import {
+  gitIgnoredExclusionGlobs,
+  isGitIgnoredPath,
+  listGitIgnoredEntries,
+} from "../git-ignored";
 import { buildPath } from "../scan";
 import { isWholeProjectWalk } from "../walk-scope";
 import { findValeBinary, valeUnavailableMessage } from "./binary";
@@ -203,8 +208,27 @@ export async function runVale(
   // findings with it. Honouring the request would cost the user the rest of
   // their check, so the request is declined and reported instead. See
   // `formats.ts` for the tier table this is derived from.
+  //
+  // What git ignores, on a whole-project walk only — same terms as
+  // `.taskless/**` above, and for a sharper version of the same reason. Vale
+  // has no notion of a VCS: it walked into build output, vendored trees, and a
+  // git worktree at `worktrees/<name>/`, which is a complete second checkout,
+  // so every prose rule fired again against another branch's documents
+  // (taskless/cli#166). ast-grep needed no equivalent — its walker honors
+  // `.gitignore` already — which is precisely why the two engines disagreed
+  // about which files the project contains. Asked only when we chose `.`
+  // ourselves, so `check worktrees/probe` still checks what it was handed.
+  const ignoredEntries = wholeProject
+    ? await listGitIgnoredEntries(options.cwd)
+    : [];
+
   const exclude = [
-    ...(wholeProject ? [`${TASKLESS_DIRECTORY}/**`] : []),
+    ...(wholeProject
+      ? [
+          `${TASKLESS_DIRECTORY}/**`,
+          ...gitIgnoredExclusionGlobs(ignoredEntries),
+        ]
+      : []),
     ...converterExclusionGlobs(),
   ];
   const globArgument = buildValeGlob(exclude);
@@ -214,8 +238,18 @@ export async function runVale(
   // its walker declined to open, so once the glob has done its job the skipped
   // files are unrecoverable from the output — and a fix whose only visible
   // effect is that some findings are quietly missing is the bug it replaced.
+  //
+  // The ignored paths are filtered back out for the same reason the notice
+  // exists at all: it must describe the run that happened. An `.adoc` inside
+  // `worktrees/` is not a file this run declined to convert — it is a file this
+  // run was never going to look at, and naming it would send the reader to
+  // investigate a directory the fix above deliberately excluded.
+  const converterDependent = await findConverterDependentFiles(
+    options.cwd,
+    paths
+  );
   const skipped = skippedFilesNotice(
-    await findConverterDependentFiles(options.cwd, paths)
+    converterDependent.filter((file) => !isGitIgnoredPath(file, ignoredEntries))
   );
 
   // `--` separates flags from positional paths, so a path beginning with `-`