feat(rules): add include/exclude file filter support in rule.json

Allow users to configure include/exclude glob patterns in
  .open-code-review/rule.json (global or project-level) to control
  which files are reviewed. Exclude has highest priority, include
  penetrates default path exclusions but not the extension allowlist.
  Only one layer takes effect following --rule > project > global priority.
This commit is contained in:
kite 2026-05-25 20:19:02 +08:00
parent d44fd31989
commit db5bc4b8a9
5 changed files with 311 additions and 50 deletions

View file

@ -84,5 +84,3 @@ jobs:
- name: Publish to npm
run: npm publish --access public --provenance
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

View file

@ -45,7 +45,7 @@ func runReview(args []string) error {
return fmt.Errorf("resolve repo: %w", err)
}
resolver, err := rules.NewResolver(repoDir, opts.rulePath)
resolver, fileFilter, err := rules.NewResolver(repoDir, opts.rulePath)
if err != nil {
return fmt.Errorf("load rules: %w", err)
}
@ -92,6 +92,7 @@ func runReview(args []string) error {
DiffMap: diffMap,
Template: *tpl,
SystemRule: resolver,
FileFilter: fileFilter,
LLMClient: llmClient,
Tools: tools,
PlanToolDefs: planToolDefs,

View file

@ -41,6 +41,10 @@ type Args struct {
// SystemRule holds path-based review rules loaded from a JSON config.
SystemRule rules.Resolver
// FileFilter holds user-configured include/exclude path patterns from rule.json.
// When nil, only the default extension and path filters apply.
FileFilter *rules.FileFilter
// LLM client for model inference.
LLMClient llm.LLMClient
@ -230,8 +234,7 @@ func (a *Agent) Run(ctx context.Context) ([]model.LlmComment, error) {
reviewCount := a.countReviewable(a.diffs)
fmt.Fprintf(stdout.Writer(), "[ocr] %d file(s) changed, reviewing %d in %s\n", totalChanged, reviewCount, a.args.RepoDir)
a.diffs = a.filterUnsupportedExts(a.diffs)
a.diffs = a.filterExcludedPaths(a.diffs)
a.diffs = a.filterDiffs(a.diffs)
if len(a.diffs) == 0 {
fmt.Fprintln(stdout.Writer(), "[ocr] No supported files changed. Skipping review.")
@ -568,7 +571,7 @@ func (a *Agent) filterLargeDiffs(diffs []model.Diff) []model.Diff {
return kept
}
// countReviewable counts diffs that will survive both filters and are not pure deletions.
// countReviewable counts diffs that will survive all filters and are not pure deletions.
func (a *Agent) countReviewable(diffs []model.Diff) int {
count := 0
for _, d := range diffs {
@ -576,11 +579,7 @@ func (a *Agent) countReviewable(diffs []model.Diff) int {
if path == "/dev/null" {
path = d.OldPath
}
ext := a.extFromPath(path)
if ext != "" && !allowedext.IsAllowedExt(ext) {
continue
}
if allowedext.IsExcludedPath(path) {
if !a.shouldReview(path) {
continue
}
if d.IsDeleted {
@ -591,33 +590,39 @@ func (a *Agent) countReviewable(diffs []model.Diff) int {
return count
}
// filterUnsupportedExts drops diffs whose file extensions are not in the supported types allowlist.
func (a *Agent) filterUnsupportedExts(diffs []model.Diff) []model.Diff {
var kept []model.Diff
skipped := 0
// shouldReview applies the four-gate filter algorithm:
// 1. User exclude → skip
// 2. Default extension check → skip if not allowed
// 3. User include → keep (penetrates default path exclusion)
// 4. Default path exclusion → skip
//
// If none of the gates trigger, the file is kept for review.
func (a *Agent) shouldReview(path string) bool {
f := a.args.FileFilter
for _, d := range diffs {
path := d.NewPath
if path == "/dev/null" {
path = d.OldPath
}
ext := a.extFromPath(path)
if ext != "" && !allowedext.IsAllowedExt(ext) {
fmt.Fprintf(stdout.Writer(), "[ocr] Skipping %s — extension %q not in supported file types\n", d.NewPath, ext)
skipped++
continue
}
kept = append(kept, d)
if f != nil && f.IsUserExcluded(path) {
return false
}
if skipped > 0 {
fmt.Fprintf(stdout.Writer(), "[ocr] Skipped %d file(s) with unsupported extensions\n", skipped)
ext := a.extFromPath(path)
if ext != "" && !allowedext.IsAllowedExt(ext) {
return false
}
return kept
if f != nil && f.HasInclude() && f.IsUserIncluded(path) {
return true
}
if allowedext.IsExcludedPath(path) {
return false
}
return true
}
// filterExcludedPaths drops diffs whose file paths match any default exclude pattern.
func (a *Agent) filterExcludedPaths(diffs []model.Diff) []model.Diff {
// filterDiffs drops diffs that should not be reviewed based on user-configured
// include/exclude patterns and default extension/path filters.
func (a *Agent) filterDiffs(diffs []model.Diff) []model.Diff {
var kept []model.Diff
skipped := 0
@ -626,8 +631,8 @@ func (a *Agent) filterExcludedPaths(diffs []model.Diff) []model.Diff {
if path == "/dev/null" {
path = d.OldPath
}
if allowedext.IsExcludedPath(path) {
fmt.Fprintf(stdout.Writer(), "[ocr] Skipping %s — matches exclude pattern\n", path)
if !a.shouldReview(path) {
fmt.Fprintf(stdout.Writer(), "[ocr] Skipping %s — filtered by path/extension rules\n", path)
skipped++
continue
}
@ -635,7 +640,7 @@ func (a *Agent) filterExcludedPaths(diffs []model.Diff) []model.Diff {
}
if skipped > 0 {
fmt.Fprintf(stdout.Writer(), "[ocr] Excluded %d file(s) by pattern\n", skipped)
fmt.Fprintf(stdout.Writer(), "[ocr] Filtered %d file(s) by include/exclude rules\n", skipped)
}
return kept
}

View file

@ -142,7 +142,53 @@ type ProjectRuleEntry struct {
// ProjectRule holds rules loaded from <repoDir>/.open-code-review/rule.json.
type ProjectRule struct {
Rules []ProjectRuleEntry `json:"rules"`
Rules []ProjectRuleEntry `json:"rules"`
Include []string `json:"include,omitempty"`
Exclude []string `json:"exclude,omitempty"`
}
// FileFilter holds the merged user-configured include/exclude glob patterns
// collected from all rule.json layers (custom, project, global).
type FileFilter struct {
Include []string
Exclude []string
}
// HasInclude reports whether any include patterns are configured.
func (f *FileFilter) HasInclude() bool {
return len(f.Include) > 0
}
// IsUserExcluded reports whether the given path matches any user exclude pattern.
func (f *FileFilter) IsUserExcluded(path string) bool {
lowerPath := strings.ToLower(path)
for _, pattern := range f.Exclude {
expanded := expandBraces(pattern)
for _, p := range expanded {
if matched, _ := doublestar.Match(p, lowerPath); matched {
return true
}
}
}
return false
}
// IsUserIncluded reports whether the given path matches any user include pattern.
// Returns false when Include is empty (no user include restriction defined).
func (f *FileFilter) IsUserIncluded(path string) bool {
if !f.HasInclude() {
return false
}
lowerPath := strings.ToLower(path)
for _, pattern := range f.Include {
expanded := expandBraces(pattern)
for _, p := range expanded {
if matched, _ := doublestar.Match(p, lowerPath); matched {
return true
}
}
}
return false
}
// composedResolver implements Resolver with layered priority.
@ -158,17 +204,19 @@ type composedResolver struct {
// 2. Project-local .open-code-review/rule.json (first match wins)
// 3. Global ~/.open-code-review/rule.json (first match wins)
// 4. Embedded system default rules
func NewResolver(repoDir, customRulePath string) (Resolver, error) {
//
// It also returns a FileFilter with the merged include/exclude patterns from all layers.
func NewResolver(repoDir, customRulePath string) (Resolver, *FileFilter, error) {
sysRule, err := LoadDefault()
if err != nil {
return nil, err
return nil, nil, err
}
var customRule *ProjectRule
if customRulePath != "" {
cr, err := loadRuleFile(customRulePath)
if err != nil {
return nil, err
return nil, nil, err
}
customRule = cr
}
@ -177,17 +225,41 @@ func NewResolver(repoDir, customRulePath string) (Resolver, error) {
if repoDir != "" {
pr, err := loadProjectRule(repoDir)
if err != nil {
return nil, err
return nil, nil, err
}
projectRule = pr
}
globalRule, err := loadGlobalRule()
if err != nil {
return nil, err
return nil, nil, err
}
return &composedResolver{custom: customRule, project: projectRule, global: globalRule, system: sysRule}, nil
filter := buildFileFilter(customRule, projectRule, globalRule)
return &composedResolver{custom: customRule, project: projectRule, global: globalRule, system: sysRule}, filter, nil
}
// buildFileFilter picks the highest-priority layer that has any include/exclude
// configured. Priority order: custom (--rule) > project > global.
func buildFileFilter(layers ...*ProjectRule) *FileFilter {
for _, pr := range layers {
if pr == nil {
continue
}
if len(pr.Include) == 0 && len(pr.Exclude) == 0 {
continue
}
f := &FileFilter{}
for _, p := range pr.Include {
f.Include = append(f.Include, strings.ToLower(p))
}
for _, p := range pr.Exclude {
f.Exclude = append(f.Exclude, strings.ToLower(p))
}
return f
}
return nil
}
func loadGlobalRule() (*ProjectRule, error) {

View file

@ -176,7 +176,8 @@ func truncate(s string, maxLen int) string {
}
func TestNewResolver_DefaultOnly(t *testing.T) {
resolver, err := NewResolver(t.TempDir(), "")
resolver, _, err := NewResolver(t.TempDir(), "")
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
@ -187,7 +188,8 @@ func TestNewResolver_DefaultOnly(t *testing.T) {
}
func TestNewResolver_ProjectFileMissing(t *testing.T) {
resolver, err := NewResolver(t.TempDir(), "")
resolver, _, err := NewResolver(t.TempDir(), "")
if err != nil {
t.Fatalf("NewResolver should not fail when project rule is missing: %v", err)
}
@ -208,7 +210,7 @@ func TestNewResolver_ProjectRuleHighestPriority(t *testing.T) {
t.Fatalf("write rule.json: %v", err)
}
resolver, err := NewResolver(dir, "")
resolver, _, err := NewResolver(dir, "")
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
@ -241,7 +243,7 @@ func TestNewResolver_ProjectRuleFallsBackToSystem(t *testing.T) {
t.Fatalf("write rule.json: %v", err)
}
resolver, err := NewResolver(dir, "")
resolver, _, err := NewResolver(dir, "")
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
@ -260,7 +262,7 @@ func TestNewResolver_CustomRuleOverridesDefault(t *testing.T) {
t.Fatalf("write custom rule: %v", err)
}
resolver, err := NewResolver(t.TempDir(), customPath)
resolver, _, err := NewResolver(t.TempDir(), customPath)
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
@ -296,7 +298,7 @@ func TestNewResolver_CustomOverridesProject(t *testing.T) {
t.Fatalf("write rule.json: %v", err)
}
resolver, err := NewResolver(repoDir, customPath)
resolver, _, err := NewResolver(repoDir, customPath)
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
@ -330,12 +332,195 @@ func TestNewResolver_ProjectFileMalformed(t *testing.T) {
t.Fatalf("write: %v", err)
}
_, err := NewResolver(dir, "")
_, _, err := NewResolver(dir, "")
if err == nil {
t.Errorf("expected error for malformed project rule.json")
}
}
func TestFileFilter_IsUserExcluded(t *testing.T) {
f := &FileFilter{
Exclude: []string{"**/generated/**", "**/*.pb.go", "vendor/**/*.{go,js}"},
}
tests := []struct {
path string
want bool
}{
{"src/generated/api.java", true},
{"pkg/foo.pb.go", true},
{"vendor/lib/util.go", true},
{"vendor/lib/util.js", true},
{"src/main.go", false},
{"src/generated.go", false},
}
for _, tt := range tests {
t.Run(tt.path, func(t *testing.T) {
if got := f.IsUserExcluded(tt.path); got != tt.want {
t.Errorf("IsUserExcluded(%q) = %v, want %v", tt.path, got, tt.want)
}
})
}
}
func TestFileFilter_IsUserIncluded(t *testing.T) {
f := &FileFilter{
Include: []string{"src/**/*.java", "src/**/*.{kt,kts}"},
}
tests := []struct {
path string
want bool
}{
{"src/main/foo.java", true},
{"src/main/bar.kt", true},
{"src/build.kts", true},
{"test/main.java", false},
{"src/main/util.go", false},
}
for _, tt := range tests {
t.Run(tt.path, func(t *testing.T) {
if got := f.IsUserIncluded(tt.path); got != tt.want {
t.Errorf("IsUserIncluded(%q) = %v, want %v", tt.path, got, tt.want)
}
})
}
}
func TestFileFilter_IsUserIncluded_EmptyInclude(t *testing.T) {
f := &FileFilter{}
if f.IsUserIncluded("anything.java") {
t.Errorf("expected false when include is empty")
}
}
func TestFileFilter_CaseInsensitive(t *testing.T) {
f := &FileFilter{
Include: []string{"src/**/*.java"},
Exclude: []string{"**/generated/**"},
}
if !f.IsUserIncluded("SRC/Main/Foo.Java") {
t.Errorf("expected case-insensitive include match")
}
if !f.IsUserExcluded("SRC/Generated/Api.java") {
t.Errorf("expected case-insensitive exclude match")
}
}
func TestNewResolver_FileFilterMerged(t *testing.T) {
repoDir := t.TempDir()
ocrDir := filepath.Join(repoDir, ".open-code-review")
if err := os.MkdirAll(ocrDir, 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
projJSON := `{"rules":[],"include":["src/**/*.java"],"exclude":["**/generated/**"]}`
if err := os.WriteFile(filepath.Join(ocrDir, "rule.json"), []byte(projJSON), 0o644); err != nil {
t.Fatalf("write: %v", err)
}
_, filter, err := NewResolver(repoDir, "")
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
if filter == nil {
t.Fatal("expected non-nil FileFilter")
}
if !filter.HasInclude() {
t.Error("expected HasInclude to be true")
}
if !filter.IsUserIncluded("src/main/foo.java") {
t.Error("expected src/main/foo.java to be included")
}
if !filter.IsUserExcluded("src/generated/api.java") {
t.Error("expected src/generated/api.java to be excluded")
}
}
func TestNewResolver_FileFilterNilWhenEmpty(t *testing.T) {
_, filter, err := NewResolver(t.TempDir(), "")
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
if filter != nil {
t.Errorf("expected nil FileFilter when no include/exclude configured, got %+v", filter)
}
}
func TestNewResolver_FileFilterPriorityOverride(t *testing.T) {
repoDir := t.TempDir()
ocrDir := filepath.Join(repoDir, ".open-code-review")
if err := os.MkdirAll(ocrDir, 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
projJSON := `{"rules":[],"include":["src/**/*.java"],"exclude":["**/gen/**"]}`
if err := os.WriteFile(filepath.Join(ocrDir, "rule.json"), []byte(projJSON), 0o644); err != nil {
t.Fatalf("write: %v", err)
}
customDir := t.TempDir()
customJSON := `{"rules":[],"include":["lib/**/*.kt"],"exclude":["**/tmp/**"]}`
customPath := filepath.Join(customDir, "custom.json")
if err := os.WriteFile(customPath, []byte(customJSON), 0o644); err != nil {
t.Fatalf("write: %v", err)
}
_, filter, err := NewResolver(repoDir, customPath)
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
if filter == nil {
t.Fatal("expected non-nil FileFilter")
}
// Custom (--rule) has highest priority, so only its patterns take effect
if !filter.IsUserIncluded("lib/util.kt") {
t.Error("expected custom include to be active")
}
if !filter.IsUserExcluded("lib/tmp/cache.kt") {
t.Error("expected custom exclude to be active")
}
// Project patterns should NOT be active since custom overrides
if filter.IsUserIncluded("src/main/foo.java") {
t.Error("project include should not be active when custom is present")
}
if filter.IsUserExcluded("src/gen/api.java") {
t.Error("project exclude should not be active when custom is present")
}
}
func TestNewResolver_FileFilterFallsToProject(t *testing.T) {
repoDir := t.TempDir()
ocrDir := filepath.Join(repoDir, ".open-code-review")
if err := os.MkdirAll(ocrDir, 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
projJSON := `{"rules":[],"include":["src/**/*.java"],"exclude":["**/gen/**"]}`
if err := os.WriteFile(filepath.Join(ocrDir, "rule.json"), []byte(projJSON), 0o644); err != nil {
t.Fatalf("write: %v", err)
}
// Custom rule has no include/exclude — should fall through to project
customDir := t.TempDir()
customJSON := `{"rules":[{"path":"**/*.go","rule":"custom-go"}]}`
customPath := filepath.Join(customDir, "custom.json")
if err := os.WriteFile(customPath, []byte(customJSON), 0o644); err != nil {
t.Fatalf("write: %v", err)
}
_, filter, err := NewResolver(repoDir, customPath)
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
if filter == nil {
t.Fatal("expected non-nil FileFilter from project layer")
}
if !filter.IsUserIncluded("src/main/foo.java") {
t.Error("expected project include to take effect when custom has none")
}
}
func TestNewResolver_BraceExpansionInProjectRule(t *testing.T) {
dir := t.TempDir()
ocrDir := filepath.Join(dir, ".open-code-review")
@ -347,7 +532,7 @@ func TestNewResolver_BraceExpansionInProjectRule(t *testing.T) {
t.Fatalf("write: %v", err)
}
resolver, err := NewResolver(dir, "")
resolver, _, err := NewResolver(dir, "")
if err != nil {
t.Fatalf("NewResolver: %v", err)
}