fix(cli): let ast-grep rules see inside hidden directories - #153
fix(cli): let ast-grep rules see inside hidden directories#153thecodedrift wants to merge 2 commits into
Conversation
ast-grep's walker skips dot-directories by default, so no `sg` rule could match anything under `.github/`, `.circleci/`, `.vscode/` or `.husky/` — `check` reported nothing and exited 0 on files it flags correctly one directory over. Pass `--no-ignore hidden` from `runAstGrepScan` and the runtime narrow. Measured against the pinned ast-grep 0.41.0, `hidden` is the only value that reaches those directories, and `vcs` is deliberately not passed so `.gitignore` is still respected. `.taskless/` is hidden too, so the wider walk reached CLI-managed config that was never meant to be scanned: a rule definition is structured YAML full of `id:`/`language:`/`severity:` keys, and an ordinary user-written Yaml rule fires on the CLI's own rule files. Exclude it via `--globs`, only on a whole-project walk, matching the rule the Vale runner already follows — an explicit path stays a request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms
|
@claude /review |
|
Claude finished @thecodedrift's task in 3m 13s —— View job Review mode: incremental — read 0 prior review thread(s) before reviewing.
ApproachEight parallel finder passes covered the diff from different angles (line-by-line, removed-behavior, cross-file tracing, duplication, simplification, efficiency, altitude, and CLAUDE.md conventions), followed by manual verification of every claim against the actual source before posting. Two findings were dropped after verification failed to confirm them cleanly; one is flagged with explicit uncertainty because I could not verify it against the actual Findings (posted inline where the diff allows it)1. 2. 3. Vale's 4. The ast-grep vendor-contract test wasn't updated for the new argv — Not raised
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
`sgWalkArgv` gated its `.taskless/` exclusion on `paths.length === 0`, but `filterExistingPaths` normalizes a positional path resolving to cwd into the literal string ".", so `taskless check .` arrived with `paths = ["."]`. A length test reads that as a user-named path and skips the exclusion, which put the self-flagging bug back for the most ordinary way of asking for a whole-project check: `check` was clean while `check .` reported findings inside `.taskless/`. Vale was already wrong in the same way at `run.ts`, independently of the hidden-directory change and reachable by the same invocation — and since Vale reads hidden directories by default, it had been reporting prose findings in `.taskless/` on any `check .`. The defect is two engines inferring the same thing from the same wrong signal, so the predicate is now shared in `walk-scope.ts` rather than fixed in one copy. Also excludes `.git/` from the wider walk. Measured against 0.41.0: ast-grep has no exclusion of its own for it and `.gitignore` does not list it, so the default hidden-directory skip was the only thing holding it back. A whole-project scan descended into `.git/objects` and `.git/logs` on every run, and `.git/hooks/*` are real source files that match language rules never meant to lint VCS internals. Repeated `--globs` flags were measured to both apply, unlike Vale's `--glob` where the last silently wins, so the exclusions are separate flags rather than one brace pattern. An explicit path is still a request: `check .taskless` reports it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms
Both findings were real and both are fixed in 85c0a16. Replies are on the inline threads; summarizing here since they interacted. Finding 1 ( Finding 2 ( Also worth recording from the fix: ast-grep honors repeated Three tests added, each confirmed to fail without its corresponding fix: Noted and not actioned: Copilot's comment is a quota-limit notice with no findings. — AI Coding Agent |
ast-grep's file walker skips dot-directories unless told otherwise, and
runAstGrepScannever told it otherwise. Nosgrule could match anything under.github/,.circleci/,.vscode/, or.husky/—checkreported nothing and exited 0 on a workflow file it flags correctly the moment the same bytes live in a non-hidden directory.Found while investigating #151: it is what made ast-grep look like it does not support YAML. It does — the blind spot was the directory, not the language.
The flag
Measured all six
--no-ignorevalues against 0.41.0 on a fixture with hidden, gitignored, and plain copies of a match:.github/dist/hiddendot,exclude,global,parentvcshiddenis the only value that does the job, andvcsis the one that must not be passed —.gitignoreis still respected, so the wider walk does not start reporting findings indist/.Also fixed in
rules/runtime/narrow.ts, which runs the samesg scanshape, so a runtime rule's capture could not reach.github/either. Thesg testpath needs nothing: it has no--no-ignoreoption at all and does not walk the project — it reads thetestDirpaths the assembled config names, and already reads hidden.tests/today..taskless/had to be excluded.taskless/is itself hidden, so the wider walk reached the CLI's own config — and a rule's YAML definition flagged itself:Every taskless sg rule file contains
id:,language:,severity:,message:,rule:, so any YAML rule a user writes would fire on config they did not author. Excluded with--globs '!**/.taskless/**'; the**/prefix is load-bearing, since--globsis gitignore-style and a pattern containing/is root-anchored, which misses a.taskless/inside a monorepo package. Measured —!.taskless/**and!.tasklessboth leave the nested one exposed.Vale had already solved this, at
vale/run.ts:145, applying its exclusion only when the CLI chose.itself: "An explicit path is a request, and silently declining to check a file someone named would be worse than checking one they did not." The first cut here excluded.taskless/unconditionally, which would have re-created the two-engines-disagree failure this PR is about. It now matches Vale's semantics, in one exported function both call sites share. Vale needs no change.Tests
Two, both asserting both halves so the fix cannot regress in either direction:
.github/is scanned while gitignoredbuild/is not (the temp dir isgit inited first — ast-grep honours.gitignoreonly inside a repository, so without that the negative assertion passes vacuously), and alanguage: Yamlrule does not flag.taskless/while still flagging.github/.Both were confirmed to fail with the fix reverted.
Refs #151