From 41fcdf5a380daee2aeed31f330e3e30f3c9f0545 Mon Sep 17 00:00:00 2001 From: piekstra Date: Thu, 13 Aug 2026 18:21:30 -0400 Subject: [PATCH 1/4] fix(pipeline): exempt generated lockfiles from reviewer coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reviewer that skips a dependency lockfile (Cargo.lock, package-lock.json, go.sum, …) was marked incomplete_skipped, and hasIncompleteReviewerCoverage then downgraded an otherwise-clean APPROVE to a COMMENT — permanently, since the reviewer will always skip a machine-generated lockfile. Exempt lockfiles from the coverage universe (they are reviewed, if at all, via the manifest change that produced them, never line by line) so neither a skipped nor an unassigned lockfile blocks approval. Seen in the wild: a Cargo.lock churned by a v0.4→v0.5 dependency bump was the only 'unreviewed' file on a clean PR, and cr would not approve it. --- internal/pipeline/pipeline.go | 50 +++++++++++++++++++++++++++++- internal/pipeline/pipeline_test.go | 25 +++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index cbabd0a..7b147c0 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -2437,10 +2437,56 @@ func reviewerToolEvidenceByAgent(sessions []sessionDraft) map[string]*llm.Review return out } +// generatedLockfiles are dependency lockfiles: machine-written by a package +// manager and reviewed (if at all) through the manifest change that produced +// them, never line by line. A reviewer that skips one is behaving correctly, so +// they are excluded from the coverage universe — otherwise a skipped lockfile +// marks the reviewer incomplete_skipped and blocks approval on an otherwise +// clean review. (A real PR stalled exactly this way: a Cargo.lock churned by a +// dependency bump was the only file left "unreviewed".) +var generatedLockfiles = map[string]bool{ + "Cargo.lock": true, + "package-lock.json": true, + "npm-shrinkwrap.json": true, + "yarn.lock": true, + "pnpm-lock.yaml": true, + "bun.lockb": true, + "go.sum": true, + "Gemfile.lock": true, + "poetry.lock": true, + "Pipfile.lock": true, + "composer.lock": true, + "Podfile.lock": true, + "flake.lock": true, + "mix.lock": true, +} + +// isGeneratedLockfile reports whether path is a dependency lockfile a reviewer +// is not expected to read line by line. +func isGeneratedLockfile(path string) bool { + return generatedLockfiles[filepath.Base(path)] +} + +// filterReviewableFiles drops generated lockfiles from a file list so they do +// not become a coverage obligation. +func filterReviewableFiles(files []string) []string { + out := make([]string, 0, len(files)) + for _, file := range files { + if isGeneratedLockfile(file) { + continue + } + out = append(out, file) + } + return out +} + func buildReviewerCoverage(selected []llm.SelectedAgent, results []llm.Findings, failures []ReviewerFailure, changedFiles []string, toolEvidence ...map[string]*llm.ReviewerToolEvidence) []reviewplan.ReviewerCoverageSummary { if len(selected) == 0 && len(changedFiles) == 0 { return nil } + // Generated lockfiles are not a review obligation: exclude them so neither a + // reviewer that skips one nor an unassigned lockfile blocks approval. + changedFiles = filterReviewableFiles(changedFiles) resultByAgent := make(map[string]llm.Findings, len(results)) for _, result := range results { resultByAgent[result.AgentID] = result @@ -2452,7 +2498,9 @@ func buildReviewerCoverage(selected []llm.SelectedAgent, results []llm.Findings, assigned := map[string]bool{} out := make([]reviewplan.ReviewerCoverageSummary, 0, len(selected)+1) for _, agent := range selected { - scope := reviewerAssignmentScope(agent, changedFiles) + // A lockfile explicitly assigned to an agent is exempt too — the scope + // is what the reviewer is held to, and lockfiles are not reviewable. + scope := filterReviewableFiles(reviewerAssignmentScope(agent, changedFiles)) for _, file := range scope { assigned[file] = true } diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index e682619..12c9546 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -5097,6 +5097,31 @@ func TestBuildReviewerCoverageStatuses(t *testing.T) { } } +func TestBuildReviewerCoverageExemptsGeneratedLockfiles(t *testing.T) { + // A reviewer that inspects the real change and skips only the churned + // Cargo.lock is complete, not incomplete_skipped — a lockfile is not a + // review obligation. An unassigned lockfile (yarn.lock) likewise must not + // surface as incomplete_unassigned and block approval. + selected := []llm.SelectedAgent{ + {AgentID: "rust:impl", Files: []string{"main.go", "Cargo.lock"}, AllowedFiles: []string{"main.go", "Cargo.lock"}}, + } + results := []llm.Findings{ + {AgentID: "rust:impl", InspectedFiles: []string{"main.go"}, SkippedFiles: []string{"Cargo.lock"}}, + } + got := buildReviewerCoverage(selected, results, nil, []string{"main.go", "Cargo.lock", "yarn.lock"}) + if len(got) != 1 { + t.Fatalf("coverage = %#v, want a single reviewer entry (no lockfile coverage rows)", got) + } + if got[0].AgentID != "rust:impl" || got[0].Status != reviewerCoverageCompleteConstrained { + t.Fatalf("coverage = %#v, want rust:impl complete_constrained", got) + } + if len(got[0].SkippedFiles) != 0 { + t.Fatalf("skipped files = %#v, want none (Cargo.lock is exempt from coverage)", got[0].SkippedFiles) + } + // complete_constrained is an approvable status; had Cargo.lock counted, this + // would be reviewerCoverageIncompleteSkipped, which blocks approval. +} + func TestBuildReviewerCoverageUsesTypedToolEvidenceInsteadOfModelConstraint(t *testing.T) { got := buildReviewerCoverage( []llm.SelectedAgent{{AgentID: "harness:reviewer", Files: []string{"main.go"}}}, From 6d0772e32c08e48bf1f9b389d8a4dba63b4b5231 Mon Sep 17 00:00:00 2001 From: piekstra Date: Thu, 13 Aug 2026 19:19:00 -0400 Subject: [PATCH 2/4] fix(llm): degrade over-long/malformed coverage constraints instead of failing the reviewer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reviewer whose coverage `constraints` entry exceeded 300 runes (or exceeded the count cap, or was empty/duplicate) failed DecodeFindings with 'entry length out of bounds', which surfaced downstream as 'completed without a result file' and sank the whole reviewer — blocking approval on an otherwise-clean review. A single legitimate ~300-rune note ('could not verify against source-of-truth docs not in context') did exactly that, repeatably. Coverage constraints are informational, not a contract: cap the count, truncate an over-long entry, and drop empties/duplicates rather than erroring. Tests updated to assert graceful degradation. --- internal/llm/contracts.go | 27 ++++++------- internal/llm/contracts_test.go | 69 +++++++++++++++++++++++++++------- 2 files changed, 69 insertions(+), 27 deletions(-) diff --git a/internal/llm/contracts.go b/internal/llm/contracts.go index ef70e2b..9082eb8 100644 --- a/internal/llm/contracts.go +++ b/internal/llm/contracts.go @@ -272,10 +272,7 @@ func DecodeFindings(data []byte, opts FindingsOptions) (Findings, error) { if err := validateCoverageFileDisjoint(inspected, skipped); err != nil { return Findings{}, err } - constraints, err := decodeCoverageStrings("constraints", wire.Constraints) - if err != nil { - return Findings{}, err - } + constraints := decodeCoverageStrings(wire.Constraints) result := Findings{ AgentID: wire.AgentID, @@ -352,27 +349,31 @@ func decodeCoverageFiles(name string, files []string, changedFiles map[string]bo return out, nil } -func decodeCoverageStrings(name string, values []string) ([]string, error) { +// decodeCoverageStrings cleans reviewer coverage constraints. These are +// informational notes ("couldn't verify X against source-of-truth docs"), not +// a contract, so a malformed or verbose entry is degraded — the count is +// capped, an over-long entry is truncated, and empties/duplicates are dropped — +// rather than failing the decode. Failing here sinks the whole reviewer as +// "completed without a result file" and blocks approval on an otherwise-clean +// review, which a single legitimate ~300-rune constraint once did. +func decodeCoverageStrings(values []string) []string { if len(values) > defaultMaxCoverageConstraints { - return nil, fmt.Errorf("llm: %s cap exceeded", name) + values = values[:defaultMaxCoverageConstraints] } out := make([]string, 0, len(values)) seen := map[string]bool{} for _, value := range values { if utf8.RuneCountInString(value) > defaultMaxCoverageConstraintRunes { - return nil, fmt.Errorf("llm: %s entry length out of bounds", name) + value = truncateRunes(value, defaultMaxCoverageConstraintRunes) } value = sanitize(value) - if strings.TrimSpace(value) == "" { - return nil, fmt.Errorf("llm: %s entries must be non-empty", name) - } - if seen[value] { - return nil, fmt.Errorf("llm: duplicate %s entry %q", name, value) + if strings.TrimSpace(value) == "" || seen[value] { + continue } seen[value] = true out = append(out, value) } - return out, nil + return out } func validateCoverageFileDisjoint(inspected, skipped []string) error { diff --git a/internal/llm/contracts_test.go b/internal/llm/contracts_test.go index d2e4f3a..dc7c1a5 100644 --- a/internal/llm/contracts_test.go +++ b/internal/llm/contracts_test.go @@ -155,8 +155,6 @@ func TestDecodeFindings(t *testing.T) { assertFindingsError(t, baseOpts, `{"schema_version":1,"agent_id":"agent-1","inspected_files":["main.go","main.go"],"findings":[]}`, "duplicate inspected_files") assertFindingsError(t, baseOpts, `{"schema_version":1,"agent_id":"agent-1","inspected_files":["main.go"],"skipped_files":["other.go"],"findings":[]}`, "skipped_files entry") assertFindingsError(t, baseOpts, `{"schema_version":1,"agent_id":"agent-1","inspected_files":["main.go"],"skipped_files":["main.go"],"findings":[]}`, "both inspected and skipped") - assertFindingsError(t, baseOpts, `{"schema_version":1,"agent_id":"agent-1","inspected_files":["main.go"],"constraints":[" "],"findings":[]}`, "constraints") - assertFindingsError(t, baseOpts, `{"schema_version":1,"agent_id":"agent-1","inspected_files":["main.go"],"constraints":["one","two","three","four","five","six","seven","eight","nine","ten","eleven"],"findings":[]}`, "constraints cap exceeded") assertFindingsError(t, baseOpts, findingsFixture(`"schema_version":2,"agent_id":"agent-1","findings":[]`), "schema_version") assertFindingsError(t, baseOpts, findingsFixture(`"schema_version":1,"agent_id":"agent-1","findings":[],"extra":true`), "unknown field") assertFindingsError(t, baseOpts, findingsFixture(`"schema_version":1,"agent_id":"missing","findings":[]`), "unknown findings agent") @@ -187,35 +185,78 @@ func TestDecodeFindingsConstraintRuneBoundaries(t *testing.T) { multibyteAtLimit := strings.Repeat("界", limits.MaxRunesPerEntry) for _, tt := range []struct { - name string - constraint string - wantErr string - wantClean bool + name string + constraint string + wantTruncated bool }{ - {name: "marker opening at limit", constraint: markerAtLimit, wantClean: true}, - {name: "marker opening over limit", constraint: markerAtLimit + "x", wantErr: "constraints entry length"}, + {name: "marker opening at limit", constraint: markerAtLimit}, + // Over-limit entries are truncated, not rejected: a verbose (but valid) + // coverage note must not fail the whole reviewer and block approval. + {name: "marker opening over limit", constraint: markerAtLimit + "x", wantTruncated: true}, {name: "multibyte at limit", constraint: multibyteAtLimit}, - {name: "multibyte over limit", constraint: multibyteAtLimit + "界", wantErr: "constraints entry length"}, + {name: "multibyte over limit", constraint: multibyteAtLimit + "界", wantTruncated: true}, } { t.Run(tt.name, func(t *testing.T) { got, err := decodeFindingsWithConstraint(t, tt.constraint) - if tt.wantErr != "" { - assertErrContains(t, err, tt.wantErr) - return - } if err != nil { t.Fatalf("DecodeFindings: %v", err) } if len(got.Constraints) != 1 { t.Fatalf("constraints = %#v, want one value", got.Constraints) } - if tt.wantClean && strings.Contains(got.Constraints[0], markerOpening) { + if strings.Contains(got.Constraints[0], markerOpening) { t.Fatalf("constraint = %q, want sanitized marker opening", got.Constraints[0]) } + if tt.wantTruncated && !strings.HasSuffix(got.Constraints[0], "...") { + t.Fatalf("constraint = %q, want truncated (ends with ...), not rejected", got.Constraints[0]) + } }) } } +func TestDecodeFindingsConstraintsDegradeInsteadOfFailing(t *testing.T) { + // Coverage constraints are informational: an over-count list, a + // whitespace-only entry, and a duplicate are cleaned rather than failing + // the whole reviewer (which surfaced as "completed without a result file" + // and blocked approval on an otherwise-clean review). + payload := map[string]any{ + "schema_version": 1, + "agent_id": "agent-1", + "inspected_files": []string{"main.go"}, + "constraints": []string{ + "a", " ", "b", "a", // whitespace-only and duplicate, within the cap + "c", "d", "e", "f", "g", "h", "i", "j", // pushes the list over the cap of 10 + }, + "findings": []any{}, + } + data, err := json.Marshal(payload) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + got, err := DecodeFindings(data, FindingsOptions{ + KnownAgents: map[string]bool{"agent-1": true}, + ChangedFiles: map[string]bool{"main.go": true}, + NewFindingID: newIDQueue("f-1").next, + }) + if err != nil { + t.Fatalf("DecodeFindings degraded to an error: %v", err) + } + lim := DefaultFindingsConstraintLimits() + if len(got.Constraints) > lim.MaxEntries { + t.Fatalf("constraints = %#v, want ≤ %d after capping", got.Constraints, lim.MaxEntries) + } + seen := map[string]bool{} + for _, c := range got.Constraints { + if strings.TrimSpace(c) == "" { + t.Fatalf("kept a whitespace-only constraint: %#v", got.Constraints) + } + if seen[c] { + t.Fatalf("kept a duplicate constraint: %#v", got.Constraints) + } + seen[c] = true + } +} + func decodeFindingsWithConstraint(t *testing.T, constraint string) (Findings, error) { t.Helper() payload := map[string]any{ From 7157adbdc4f504cfd1bd9c1a8ea19c7cc6ff49d7 Mon Sep 17 00:00:00 2001 From: piekstra Date: Tue, 18 Aug 2026 13:55:44 -0400 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20address=20#567=20review=20=E2=80=94?= =?UTF-8?q?=20unify=20lockfile=20exemption,=20clamp=20constraint=20length,?= =?UTF-8?q?=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ensureSelectedGlobCoverage skips generated lockfiles too, so a reviewer's prompt scope no longer lists a file the accounting layer then exempts; the coverage universe has one owner. Also filter entry.InspectedFiles so scope and coverage rows draw from the same set. - decodeCoverageConstraints (renamed from decodeCoverageStrings): sanitize then clamp, so a truncated/marker-grown entry always fits the 300-rune cap (ellipsis reserved). Test asserts the rune cap, not just the ellipsis. - DefaultFindingsConstraintLimits doc reworded (capped/truncated, not enforced by failure). - docs/checkout-native-review-contract.md documents the lockfile exemption in the readable-files / coverage-status section. --- docs/checkout-native-review-contract.md | 14 ++++++++++++-- internal/llm/contracts.go | 19 ++++++++++++------- internal/llm/contracts_test.go | 4 ++++ internal/pipeline/pipeline.go | 11 ++++++++++- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/docs/checkout-native-review-contract.md b/docs/checkout-native-review-contract.md index c6a3e09..9174140 100644 --- a/docs/checkout-native-review-contract.md +++ b/docs/checkout-native-review-contract.md @@ -336,9 +336,19 @@ silently. Coverage uses two related scopes: -- readable files: all changed files in the workbench +- readable files: all changed files in the workbench, **except generated + dependency lockfiles** (`Cargo.lock`, `package-lock.json`, `go.sum`, and the + well-known peers — see `isGeneratedLockfile`). Lockfiles are machine-written + and reviewed, if at all, through the manifest change that produced them, so + they are exempt from the coverage universe: neither a reviewer that skips one + nor an unassigned lockfile counts as incomplete coverage. The exemption is + applied in one place — the orchestrator's glob-coverage assigner does not + force-assign a lockfile, and the coverage accounting drops lockfiles from both + scope and inspected/skipped rows — so scope and coverage are drawn from the + same set. - assignment scope: `allowed_files` when present, otherwise `files` when the - orchestrator supplied them, otherwise all changed files + orchestrator supplied them, otherwise all changed files (lockfiles exempt, as + above) The coverage status values are: diff --git a/internal/llm/contracts.go b/internal/llm/contracts.go index 9082eb8..a3e9e24 100644 --- a/internal/llm/contracts.go +++ b/internal/llm/contracts.go @@ -37,8 +37,10 @@ type FindingsConstraintLimits struct { MaxRunesPerEntry int } -// DefaultFindingsConstraintLimits returns the fixed reviewer-constraint -// limits enforced by DecodeFindings. +// DefaultFindingsConstraintLimits returns the fixed reviewer-constraint limits. +// DecodeFindings applies them by capping the count and truncating over-long +// entries (see decodeCoverageConstraints), not by failing — constraints are +// informational, so a verbose one must not sink the reviewer. func DefaultFindingsConstraintLimits() FindingsConstraintLimits { return FindingsConstraintLimits{ MaxEntries: defaultMaxCoverageConstraints, @@ -272,7 +274,7 @@ func DecodeFindings(data []byte, opts FindingsOptions) (Findings, error) { if err := validateCoverageFileDisjoint(inspected, skipped); err != nil { return Findings{}, err } - constraints := decodeCoverageStrings(wire.Constraints) + constraints := decodeCoverageConstraints(wire.Constraints) result := Findings{ AgentID: wire.AgentID, @@ -349,24 +351,27 @@ func decodeCoverageFiles(name string, files []string, changedFiles map[string]bo return out, nil } -// decodeCoverageStrings cleans reviewer coverage constraints. These are +// decodeCoverageConstraints cleans reviewer coverage constraints. These are // informational notes ("couldn't verify X against source-of-truth docs"), not // a contract, so a malformed or verbose entry is degraded — the count is // capped, an over-long entry is truncated, and empties/duplicates are dropped — // rather than failing the decode. Failing here sinks the whole reviewer as // "completed without a result file" and blocks approval on an otherwise-clean // review, which a single legitimate ~300-rune constraint once did. -func decodeCoverageStrings(values []string) []string { +func decodeCoverageConstraints(values []string) []string { if len(values) > defaultMaxCoverageConstraints { values = values[:defaultMaxCoverageConstraints] } out := make([]string, 0, len(values)) seen := map[string]bool{} for _, value := range values { + // Sanitize first (it can rewrite/grow a marker), then clamp the final + // length so the result always fits the per-entry cap. truncateRunes + // appends a 3-rune "...", so reserve that width. + value = sanitize(value) if utf8.RuneCountInString(value) > defaultMaxCoverageConstraintRunes { - value = truncateRunes(value, defaultMaxCoverageConstraintRunes) + value = truncateRunes(value, defaultMaxCoverageConstraintRunes-3) } - value = sanitize(value) if strings.TrimSpace(value) == "" || seen[value] { continue } diff --git a/internal/llm/contracts_test.go b/internal/llm/contracts_test.go index dc7c1a5..0e280c2 100644 --- a/internal/llm/contracts_test.go +++ b/internal/llm/contracts_test.go @@ -210,6 +210,10 @@ func TestDecodeFindingsConstraintRuneBoundaries(t *testing.T) { if tt.wantTruncated && !strings.HasSuffix(got.Constraints[0], "...") { t.Fatalf("constraint = %q, want truncated (ends with ...), not rejected", got.Constraints[0]) } + // Truncation (ellipsis included) must stay within the documented cap. + if n := utf8.RuneCountInString(got.Constraints[0]); n > limits.MaxRunesPerEntry { + t.Fatalf("constraint = %d runes, want ≤ %d (ellipsis must fit the cap)", n, limits.MaxRunesPerEntry) + } }) } } diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index 7b147c0..2c2c386 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -1576,6 +1576,12 @@ func ensureSelectedGlobCoverage(selection llm.Selection, catalog agents.Catalog, if covered[file] { continue } + // Generated lockfiles are not a coverage obligation (see + // buildReviewerCoverage): don't force-assign one to an agent, or its + // prompt scope would list a file the accounting layer then exempts. + if isGeneratedLockfile(file) { + continue + } for i := range selection.SelectedAgents { selected := &selection.SelectedAgents[i] candidate, ok := agentByID[selected.AgentID] @@ -2521,7 +2527,10 @@ func buildReviewerCoverage(selected []llm.SelectedAgent, results []llm.Findings, out = append(out, entry) continue } - entry.InspectedFiles = copySortedStrings(result.InspectedFiles) + // Draw both scope and coverage rows from the same lockfile-exempt set, + // so a reviewer that did read a lockfile doesn't emit an inspected file + // outside its scope. + entry.InspectedFiles = filterReviewableFiles(copySortedStrings(result.InspectedFiles)) entry.SkippedFiles = sortedIntersection(result.SkippedFiles, scope) entry.Constraints = copySortedStrings(result.Constraints) if evidence := reviewerToolEvidenceForAgent(toolEvidence, agent.AgentID); evidence != nil && evidence.DiffStatus != llm.DiffToolStatusSucceeded { From 5fdbcebd8ad03e17fed62811f5f2c586326d099e Mon Sep 17 00:00:00 2001 From: piekstra Date: Tue, 18 Aug 2026 14:03:15 -0400 Subject: [PATCH 4/4] test: cover the new lockfile-exemption branches (#567 review) - TestEnsureSelectedGlobCoverageSkipsLockfiles: a lockfile matching an agent's globs stays unassigned instead of being force-assigned into scope. - Extend the coverage test with a reviewer that reports inspecting Cargo.lock, asserting it's dropped from the summary's InspectedFiles. --- internal/pipeline/pipeline_test.go | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index 12c9546..52147a1 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -5120,6 +5120,37 @@ func TestBuildReviewerCoverageExemptsGeneratedLockfiles(t *testing.T) { } // complete_constrained is an approvable status; had Cargo.lock counted, this // would be reviewerCoverageIncompleteSkipped, which blocks approval. + + // A reviewer that reports *inspecting* a lockfile: it must be dropped from + // the coverage row too, so scope and inspected files come from one set. + inspectedLock := buildReviewerCoverage( + []llm.SelectedAgent{{AgentID: "rust:impl", Files: []string{"main.go"}}}, + []llm.Findings{{AgentID: "rust:impl", InspectedFiles: []string{"Cargo.lock", "main.go"}}}, + nil, + []string{"main.go", "Cargo.lock"}, + ) + if len(inspectedLock) != 1 || !reflect.DeepEqual(inspectedLock[0].InspectedFiles, []string{"main.go"}) { + t.Fatalf("inspected files = %#v, want [main.go] with the lockfile dropped", inspectedLock) + } +} + +func TestEnsureSelectedGlobCoverageSkipsLockfiles(t *testing.T) { + // A changed lockfile that matches an agent's globs must NOT be force-assigned + // into that agent's scope — it is exempt from the coverage universe. Without + // the skip the reviewer would be told to cover Cargo.lock, which the + // accounting layer then exempts, reintroducing the split this fix removes. + catalog := agents.Catalog{Agents: []agents.Agent{ + {ID: "rust:impl", FileGlobs: []string{"**/*.lock", "**/*.rs"}}, + }} + selection := llm.Selection{SelectedAgents: []llm.SelectedAgent{ + {AgentID: "rust:impl", Files: []string{"main.rs"}}, + }} + got := ensureSelectedGlobCoverage(selection, catalog, []string{"main.rs", "Cargo.lock"}) + for _, f := range got.SelectedAgents[0].Files { + if f == "Cargo.lock" { + t.Fatalf("Cargo.lock was force-assigned into agent scope: %#v", got.SelectedAgents[0].Files) + } + } } func TestBuildReviewerCoverageUsesTypedToolEvidenceInsteadOfModelConstraint(t *testing.T) {