Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -435,12 +435,19 @@ model_tier: medium
effort: medium
file_globs:
- "**/*.go"
- "!**/*_test.go"
applies_when:
- Go files changed
required_on_match: true
needs_full_file_content: false
```

Prefix a `file_globs` entry with `!` to exclude matching paths from that
agent's scope. Exclusions apply after all includes, regardless of declaration
order, and every nonempty `file_globs` list must contain at least one include.
For example, `**/*.tsx` plus `!**/*.test.*` matches rendered TypeScript files
without assigning their tests to the reviewer.

Use `model_tier: small|medium|large` for portable shared catalogs. It means the
minimum acceptable reviewer tier for that agent, not a direct model pick. Use
`model_id: <provider-model-id>` only when an agent intentionally requires one
Expand Down
81 changes: 76 additions & 5 deletions internal/agents/agents.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,79 @@ type Agent struct {
Overridden []string `json:"overridden,omitempty"`
}

type compiledFileGlob struct {
matcher glob.Glob
rootMatcher glob.Glob
}

func (g compiledFileGlob) matches(file string) bool {
return g.matcher.Match(file) || g.rootMatcher != nil && g.rootMatcher.Match(file)
}

// FileGlobSet is a validated agent file-scope expression. Positive patterns
// include files and !-prefixed patterns exclude matching files.
type FileGlobSet struct {
includes []compiledFileGlob
excludes []compiledFileGlob
}

// CompileFileGlobs validates and compiles an agent's file_globs contract.
func CompileFileGlobs(patterns []string) (FileGlobSet, error) {
var set FileGlobSet
for _, rawPattern := range patterns {
excluded := strings.HasPrefix(rawPattern, "!")
pattern := strings.TrimPrefix(rawPattern, "!")
if pattern == "" {
return FileGlobSet{}, fmt.Errorf("file_glob %q is invalid", rawPattern)
}
matcher, err := glob.Compile(pattern, '/')
if err != nil {
return FileGlobSet{}, fmt.Errorf("file_glob %q is invalid: %w", rawPattern, err)
}
compiled := compiledFileGlob{matcher: matcher}
if strings.HasPrefix(pattern, "**/") {
compiled.rootMatcher, err = glob.Compile(strings.TrimPrefix(pattern, "**/"), '/')
if err != nil {
return FileGlobSet{}, fmt.Errorf("file_glob %q is invalid: %w", rawPattern, err)
}
}
if excluded {
set.excludes = append(set.excludes, compiled)
} else {
set.includes = append(set.includes, compiled)
}
}
if len(patterns) > 0 && len(set.includes) == 0 {
return FileGlobSet{}, errors.New("file_globs requires at least one include file_glob")
}
return set, nil
}

// Matches reports whether the file is included and not excluded.
func (s FileGlobSet) Matches(file string) bool {
included := false
for _, pattern := range s.includes {
if pattern.matches(file) {
included = true
break
}
}
if !included {
return false
}
for _, pattern := range s.excludes {
if pattern.matches(file) {
return false
}
}
return true
}

// HasExclusions reports whether the set contains any exclusion patterns.
func (s FileGlobSet) HasExclusions() bool {
return len(s.excludes) > 0
}

// RepoReader is the narrow read seam needed to load repo-local agent files.
type RepoReader interface {
ListTreeAtRef(ctx context.Context, ref gitprovider.PRRef, gitRef string, treePath string) ([]gitprovider.TreeEntry, error)
Expand Down Expand Up @@ -740,11 +813,9 @@ func validateAgentYAML(categoryName, agentName string, index agentYAML) error {
if len(index.FileGlobs) == 0 {
return fmt.Errorf("%w: agent %s:%s required_on_match requires file_globs", ErrInvalid, categoryName, agentName)
}
for _, pattern := range index.FileGlobs {
if _, err := glob.Compile(pattern, '/'); err != nil {
return fmt.Errorf("%w: agent %s:%s file_glob %q is invalid: %w", ErrInvalid, categoryName, agentName, pattern, err)
}
}
}
if _, err := CompileFileGlobs(index.FileGlobs); err != nil {
return fmt.Errorf("%w: agent %s:%s %w", ErrInvalid, categoryName, agentName, err)
}
return nil
}
Expand Down
15 changes: 15 additions & 0 deletions internal/agents/agents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,21 @@ func TestLoadRejectsInvalidAgentModelMetadata(t *testing.T) {
index: "name: reviewer\ndescription: desc\nmodel_tier: medium\neffort: medium\nfile_globs:\n - '[bad'\nrequired_on_match: true\n",
want: `file_glob "[bad" is invalid`,
},
{
name: "only exclusion globs",
Comment thread
zzwong marked this conversation as resolved.
index: "name: reviewer\ndescription: desc\nmodel_tier: medium\neffort: medium\nfile_globs:\n - '!**/*.test.*'\n",
Comment thread
zzwong marked this conversation as resolved.
want: "requires at least one include file_glob",
},
{
name: "empty exclusion glob",
index: "name: reviewer\ndescription: desc\nmodel_tier: medium\neffort: medium\nfile_globs:\n - '**/*.go'\n - '!'\n",
want: `file_glob "!" is invalid`,
},
{
name: "invalid exclusion glob",
index: "name: reviewer\ndescription: desc\nmodel_tier: medium\neffort: medium\nfile_globs:\n - '**/*.go'\n - '![bad'\n",
want: `file_glob "![bad" is invalid`,
},
}

for _, tt := range tests {
Expand Down
88 changes: 85 additions & 3 deletions internal/pipeline/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,7 @@ func ensureRequiredOnMatchAgents(selection llm.Selection, catalog agents.Catalog
// Files/AllowedFiles) need no widening, and AllowedFiles is only extended
// when it is what defines the agent's scope.
func ensureSelectedGlobCoverage(selection llm.Selection, catalog agents.Catalog, changedFiles []string) llm.Selection {
selection = reconcileSelectedGlobScopes(selection, catalog, changedFiles)
if len(selection.SelectedAgents) == 0 {
return selection
}
Expand Down Expand Up @@ -1601,6 +1602,68 @@ func ensureSelectedGlobCoverage(selection llm.Selection, catalog agents.Catalog,
return selection
}

// reconcileSelectedGlobScopes enforces exclusions on assignments proposed by
// the selector or restored from a saved cohort. Agents without exclusions keep
// the existing broad-assignment behavior. A broad assignment with exclusions
// is materialized so excluded files cannot re-enter through implicit scope.
func reconcileSelectedGlobScopes(selection llm.Selection, catalog agents.Catalog, changedFiles []string) llm.Selection {
changed := stringSet(changedFiles)
reconciled := selection
reconciled.SelectedAgents = nil
for _, selected := range selection.SelectedAgents {
agent, ok := catalog.Find(selected.AgentID)
if !ok || !hasExclusionGlob(agent.FileGlobs) {
reconciled.SelectedAgents = append(reconciled.SelectedAgents, selected)
continue
}

if len(selected.Files) == 0 && len(selected.AllowedFiles) == 0 {
matched := matchingChangedFiles(agent.FileGlobs, changedFiles)
if len(matched) == 0 {
continue
}
selected.Files = append([]string(nil), matched...)
Comment thread
zzwong marked this conversation as resolved.
selected.AllowedFiles = append([]string(nil), matched...)
} else {
selected.Files = matchingAssignedFiles(agent.FileGlobs, selected.Files, changed)
selected.AllowedFiles = matchingAssignedFiles(agent.FileGlobs, selected.AllowedFiles, changed)
if len(selected.Files) > 0 && len(selected.AllowedFiles) == 0 {
selected.AllowedFiles = append([]string(nil), selected.Files...)
}
if len(selected.Files) == 0 && len(selected.AllowedFiles) == 0 {
continue
}
}
reconciled.SelectedAgents = append(reconciled.SelectedAgents, selected)
}
return reconciled
}

func hasExclusionGlob(patterns []string) bool {
set, err := agents.CompileFileGlobs(patterns)
return err == nil && set.HasExclusions()
}

func matchingChangedFiles(patterns, changedFiles []string) []string {
var matched []string
for _, file := range changedFiles {
if globsMatchFile(patterns, file) && !slices.Contains(matched, file) {
matched = append(matched, file)
}
}
return matched
}

func matchingAssignedFiles(patterns, files []string, changed map[string]bool) []string {
var matched []string
for _, file := range files {
if changed[file] && globsMatchFile(patterns, file) && !slices.Contains(matched, file) {
matched = append(matched, file)
}
}
return matched
}

func rebaseReviewerCohort(req Request, catalog agents.Catalog, cohort ledger.ReviewerCohort, changedFiles []string, maxAgents int, adapter string) (llm.Selection, map[string]string, error) {
freshError := func(format string, args ...any) (llm.Selection, map[string]string, error) {
return llm.Selection{}, nil, fmt.Errorf("pipeline: "+format+"; pass --fresh-session to select a new reviewer cohort", args...)
Expand All @@ -1615,6 +1678,7 @@ func rebaseReviewerCohort(req Request, catalog agents.Catalog, cohort ledger.Rev
type candidate struct {
member ledger.ReviewerCohortMember
agent agents.Agent
globs agents.FileGlobSet
files []string
}
candidates := make([]candidate, 0, len(cohort.Members))
Expand All @@ -1632,14 +1696,21 @@ func rebaseReviewerCohort(req Request, catalog agents.Catalog, cohort ledger.Rev
if runtimeConfig.model != member.Model || runtimeConfig.effort != member.Effort || req.ReviewerFast != member.Fast {
return freshError("saved reviewer %q runtime is incompatible with the current runtime", member.AgentID)
}
current := candidate{member: member, agent: agent}
fileGlobs, err := agents.CompileFileGlobs(agent.FileGlobs)
if err != nil {
return llm.Selection{}, nil, err
}
current := candidate{member: member, agent: agent, globs: fileGlobs}
persistedFiles := member.Files
if member.AssignmentMode == ledger.ReviewerAssignmentScoped {
if len(member.AllowedFiles) > 0 {
persistedFiles = member.AllowedFiles
}
}
for _, file := range persistedFiles {
if fileGlobs.HasExclusions() && !fileGlobs.Matches(file) {
continue
}
if changed[file] && !slices.Contains(current.files, file) {
current.files = append(current.files, file)
covered[file] = true
Expand All @@ -1653,7 +1724,8 @@ func rebaseReviewerCohort(req Request, catalog agents.Catalog, cohort ledger.Rev
}
assigned := false
for i := range candidates {
if candidates[i].member.AssignmentMode != ledger.ReviewerAssignmentBroad && !globsMatchFile(candidates[i].agent.FileGlobs, file) {
broadWithoutExclusions := candidates[i].member.AssignmentMode == ledger.ReviewerAssignmentBroad && !candidates[i].globs.HasExclusions()
if !broadWithoutExclusions && !candidates[i].globs.Matches(file) {
continue
}
candidates[i].files = append(candidates[i].files, file)
Expand All @@ -1678,14 +1750,24 @@ func rebaseReviewerCohort(req Request, catalog agents.Catalog, cohort ledger.Rev
}
Comment thread
zzwong marked this conversation as resolved.
selected := llm.SelectedAgent{AgentID: candidate.member.AgentID, Rationale: "reused reviewer cohort"}
selected.Files = append([]string(nil), candidate.files...)
if candidate.member.AssignmentMode == ledger.ReviewerAssignmentScoped {
if candidate.member.AssignmentMode == ledger.ReviewerAssignmentScoped || hasExclusionGlob(candidate.agent.FileGlobs) {
selected.AllowedFiles = append([]string(nil), candidate.files...)
}
selection.SelectedAgents = append(selection.SelectedAgents, selected)
if sessionID := strings.TrimSpace(candidate.member.ProviderSessionID); sessionID != "" {
resumes[candidate.member.AgentID] = sessionID
}
}
selection = ensureSelectedGlobCoverage(selection, catalog, changedFiles)
selectedIDs := map[string]bool{}
for _, selected := range selection.SelectedAgents {
selectedIDs[selected.AgentID] = true
}
for agentID := range resumes {
if !selectedIDs[agentID] {
delete(resumes, agentID)
}
}
return selection, resumes, nil
}

Expand Down
Loading
Loading