Skip to content

Add .packmindignore support for CLI lint command#195

Merged
cteyton merged 9 commits intomainfrom
cli-packmind-ignore
Mar 12, 2026
Merged

Add .packmindignore support for CLI lint command#195
cteyton merged 9 commits intomainfrom
cli-packmind-ignore

Conversation

@cteyton
Copy link
Copy Markdown
Contributor

@cteyton cteyton commented Mar 6, 2026

Explanation

Add support for .packmindignore files in the CLI lint command. This allows users to exclude additional files/folders from analysis without modifying CLI source code, similar to .gitignore.

The .packmindignore file supports:

  • One pattern per line (same glob syntax as existing ListFiles.matchesGlobPattern())
  • Comments (# lines) and empty lines are ignored
  • Hierarchical lookup: walks up from linted directory to git repo root, merging all found .packmindignore files
  • Patterns are additive on top of existing hardcoded excludes (node_modules, dist, .min., .map., .git)

Type of Change

  • Bug fix
  • New feature
  • Improvement/Enhancement
  • Refactoring
  • Documentation
  • Breaking change

Affected Components

  • Domain packages affected: packmind-cli
  • Frontend / Backend / Both: CLI only
  • Breaking changes (if any): None

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing completed
  • Test coverage maintained or improved

Test Details:

  • PackmindIgnoreReader.spec.ts: 7 tests covering parsing, comment stripping, whitespace trimming, multi-level merging, and edge cases
  • lintHandler.spec.ts: 4 new tests verifying ignore patterns are read and passed to both lintFilesFromConfig and lintFilesAgainstRule
  • All 1747 existing tests continue to pass

TODO List

  • CHANGELOG Updated
  • Documentation Updated

Reviewer Notes

  • The lintHandler was refactored to consolidate 3 separate tryGetGitRepositoryRoot calls into 1, which is a minor improvement alongside the feature
  • .packmindignore patterns apply to all lint modes: full scan, --changed-files, --changed-lines, and --rule

🤖 Generated with Claude Code

cteyton and others added 2 commits March 6, 2026 17:19
- Create PackmindIgnoreReader service that walks up directories to collect ignore patterns
- Add ignorePatterns to LintFilesFromConfig and LintFilesAgainstRule command types
- Merge .packmindignore patterns with hardcoded excludes in both lint use cases
- Read .packmindignore in lintHandler and pass patterns to use cases
- Consolidate tryGetGitRepositoryRoot calls in lintHandler

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 6, 2026

Greptile Summary

This PR adds .packmindignore file support to the CLI lint command, letting users exclude files and directories from analysis via glob patterns without touching CLI source code. The implementation is clean, well-tested, and also ships two meaningful silent bug fixes.

Key changes:

  • New PackmindIgnoreReader service walks from the linted directory up to the git root collecting patterns, or restricts to just the linted directory when not in a git repo (addressing the unbounded-walk concern from prior review).
  • PackmindIgnoreReader.parseIgnoreFile now correctly re-throws non-ENOENT errors; lintHandler catches any error and warns rather than failing silently or crashing.
  • DEFAULT_EXCLUDES is extracted into ListFiles.ts as a shared constant, eliminating the previously duplicated inline array.
  • Bug fix (previously silent): The old default exclude patterns .min. and .map. were matched against exact path segment names, so they never actually excluded minified or source-map files. The replacement patterns *.min.* and *.map now work correctly.
  • Bug fix: matchesGlobPattern now escapes regex metacharacters (., [, ], etc.) in user-supplied patterns before converting glob wildcards to regex, preventing crashes and incorrect matches on patterns like src/[legacy].
  • lintHandler is refactored to make a single tryGetGitRepositoryRoot call instead of three, and threads ignorePatterns into both lintFilesAgainstRule and lintFilesFromConfig.
  • Documentation in linter.mdx accurately describes the new feature including syntax, file location rules, and always-excluded defaults.

Confidence Score: 5/5

  • This PR is safe to merge — all previous review concerns have been addressed and no new issues were found.
  • The feature is well-scoped, the implementation is correct, two silent pre-existing bugs are fixed as a bonus, error handling is appropriate throughout, and tests cover all meaningful edge cases including permission errors, multi-level merging, and null git-root behaviour. No critical logic, security, or correctness issues remain.
  • No files require special attention.

Important Files Changed

Filename Overview
apps/cli/src/application/services/PackmindIgnoreReader.ts New service that walks the directory tree from startDirectory up to stopDirectory (or just startDirectory when null) collecting .packmindignore patterns. Non-ENOENT errors are properly re-thrown, and the null-stop behavior is now guarded to avoid picking up unrelated ancestor patterns.
apps/cli/src/application/services/ListFiles.ts Introduces DEFAULT_EXCLUDES constant (centralising previously duplicated inline arrays) and fixes a latent bug: old patterns .min. and .map. were matched against exact path segments and thus never excluded anything; the new .min. and *.map glob patterns now work correctly. Also adds regex metacharacter escaping before glob-to-regex conversion, fixing crashes on patterns like src/[legacy].
apps/cli/src/infra/commands/lintHandler.ts Consolidates three tryGetGitRepositoryRoot calls into one, reads .packmindignore patterns upfront with a warning-and-continue error handler, and threads ignorePatterns into both lintFilesAgainstRule and lintFilesFromConfig. Logic is clean and well-structured.
apps/cli/src/application/useCases/LintFilesFromConfigUseCase.ts Adds optional ignorePatterns to the command, merges them with DEFAULT_EXCLUDES before calling listFilesInDirectory. Clean, minimal change.
apps/cli/src/application/useCases/LintFilesAgainstRuleUseCase.ts Mirrors the LintFilesFromConfigUseCase change: accepts ignorePatterns, merges with DEFAULT_EXCLUDES, and passes the combined array to listFilesInDirectory.

Sequence Diagram

sequenceDiagram
    participant CLI as lintHandler
    participant Git as PackmindCliHexa
    participant IR as PackmindIgnoreReader
    participant LF as ListFiles
    participant UC as LintUseCase

    CLI->>Git: tryGetGitRepositoryRoot(absolutePath)
    Git-->>CLI: gitRoot (string | null)

    CLI->>IR: readIgnorePatterns(absolutePath, gitRoot)
    Note over IR: Walk dirs from absolutePath → gitRoot<br/>(or only absolutePath if gitRoot is null)
    IR-->>CLI: ignorePatterns[]

    CLI->>UC: execute({ path, ignorePatterns, ... })
    Note over UC: excludes = [...DEFAULT_EXCLUDES, ...ignorePatterns]
    UC->>LF: listFilesInDirectory(path, [], excludes)
    LF-->>UC: files[]
    UC-->>CLI: violations[]
Loading

Last reviewed commit: d6fb758

Comment thread apps/cli/src/application/services/PackmindIgnoreReader.ts
Comment thread apps/cli/src/application/useCases/LintFilesFromConfigUseCase.ts
Comment thread apps/cli/src/infra/commands/lintHandler.spec.ts
- Bound directory walk to startDirectory when no git root to avoid collecting unrelated ancestor patterns
- Extract DEFAULT_EXCLUDES shared constant from duplicated hardcoded arrays
- Fix misleading double-slash in lintHandler test assertion

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread apps/cli/src/infra/commands/lintHandler.ts Outdated
@cteyton
Copy link
Copy Markdown
Contributor Author

cteyton commented Mar 6, 2026

@greptile update your review

Comment thread apps/cli/src/application/useCases/LintFilesFromConfigUseCase.ts
Comment thread apps/cli/src/application/services/PackmindIgnoreReader.ts
cteyton and others added 3 commits March 9, 2026 10:34
…Reader

- Check for ENOENT specifically in parseIgnoreFile, re-throw other errors
- Wrap readIgnorePatterns call in lintHandler with try/catch to log warning
- Add tests for unreadable .packmindignore and lintHandler error fallback

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The `.min.` and `.map.` patterns never matched any files because
matchesGlobPattern treated them as exact directory segment matches.
Changed to `*.min.*` and `*.map` glob patterns which correctly go
through the regex conversion branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stRuleUseCase

Verify that ignorePatterns passed to execute() are merged with DEFAULT_EXCLUDES
when calling listFilesInDirectory.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread apps/cli/src/application/services/ListFiles.ts
Comment thread apps/doc/linter/linter.mdx
- Escape `.`, `[`, `(`, `{` and other special chars before glob-to-regex
  conversion, preventing `*.min.*` from false-matching `admin.js` and
  preventing SyntaxError crashes on user patterns containing `[` or `(`
- Add regression tests: admin.js not excluded, app.min.js excluded,
  bracket pattern does not crash
- Fix docs: correct default exclusion patterns from `.min.*`/`.map.*`
  to `*.min.*`/`*.map` in linter.mdx

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

@cteyton cteyton merged commit a354471 into main Mar 12, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant