code-review.md states (line 144) "Skip SUMMARY/git scoping entirely when --files is provided." Tier 2 honors this via `if [ -z "$FILES_OVERRIDE" ]`, but Tier 3's #2666 SUMMARY/diff cross-check had no FILES_OVERRIDE reference at all -- reached via `elif [ -n "$DIFF_BASE" ]` whenever REVIEW_FILES was already non-empty (true under --files, since Tier 1 fills it), so it silently appended the whole phase's changed files onto an explicit user-supplied file list. --files is documented as the highest-precedence scoping tier (D-08) and is the flag Tier 3's own fail-closed path recommends when no reliable diff base is found; a user narrowing a review to two files silently got the whole phase instead, and the reviewer agent spent its budget on files nobody asked about. Gated the elif on the same condition Tier 2 already uses: elif [ -z "$FILES_OVERRIDE" ] && [ -n "$DIFF_BASE" ]; then The issue's own narrowest suggested form, reasoned through against two alternatives (wrapping the whole Tier-3 fence, or changing the stated invariant instead) -- both explicitly rejected there for good reasons concurred with after reading the surrounding code. Added tests/code-review-tier3-files-override-scoping.test.cjs, mirroring the issue's own verified reproduction methodology: extracts the Tier 1/2/3 fences VERBATIM from code-review.md (never reimplemented) and runs them against a real constructed git fixture matching the issue's own scenario exactly (5 files, a SUMMARY listing only 1). Confirms --files stays scoped to exactly the requested file, and separately confirms the #2666 cross-check still widens a genuinely partial SUMMARY scope when --files is absent (proving this is a gate, not a blanket disable). Emitted-Drift-Ack-Growth: code-review.md — #4460 gates the Tier-3 #2666 cross-check on FILES_OVERRIDE, matching Tier 2's own guard, net +453 bytes Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -292,7 +292,13 @@ if [ ${#REVIEW_FILES[@]} -eq 0 ]; then
|
||||
echo "Warning: No phase commits found for '${PADDED_PHASE}'. Cannot determine reliable diff scope."
|
||||
echo "Use --files flag to specify files explicitly: /gsd:code-review ${PHASE_ARG} --files=file1,file2,..."
|
||||
fi
|
||||
elif [ -n "$DIFF_BASE" ]; then
|
||||
elif [ -z "$FILES_OVERRIDE" ] && [ -n "$DIFF_BASE" ]; then
|
||||
# #4460: gated on FILES_OVERRIDE being unset — without this, REVIEW_FILES is
|
||||
# already non-empty under --files (Tier 1 filled it), so this elif was
|
||||
# reached anyway and the #2666 cross-check below appended the whole phase
|
||||
# diff onto an explicit user-supplied file list, contradicting line 144's
|
||||
# "Skip SUMMARY/git scoping entirely when --files is provided" and Tier 2's
|
||||
# own --files guard immediately above.
|
||||
# #2666 cross-check: SUMMARY yielded a non-empty (possibly partial) scope.
|
||||
# Warn about — and add — any changed files the SUMMARY extractor did not surface,
|
||||
# so a partial result can no longer silently ship an incomplete review scope.
|
||||
|
||||
Reference in New Issue
Block a user