fix(review): address trail + CI findings on scope context and allowlist · Entire
fix(review): address trail + CI findings on scope context and allowlist
86723b0·
peyton-alt·1w ago·8 files·+75 added/-15 removed
Four review findings and one lint failure, all confirmed real:
- Commit cap kept the OLDEST commits (first lines of --reverse output) and silently dropped everything near HEAD, while the prompt forbids re-deriving scope. Cap newest-first, then reverse the kept slice for reading order — recent commits are the ones a review can least afford to lose. (Cursor Bugbot, high)
- Inline diff budget 48KiB could exceed Windows' ~32KiB CreateProcess command-line cap (prompt travels as argv and ENTIRE_REVIEW_PROMPT). Lowered to 24KiB. (Copilot)
- A diff containing \` (any markdown-touching change) closed the diff fence early, letting diff content escape into instruction position. Fence is now one backtick longer than the longest run in the diff. (Copilot)
- Bash(git branch:*) allowed mutating -d/-D/-m; removed from the read-only allowlist. (Copilot)
- guidedProfileTask's profileName parameter became unused when the built-in fallback was removed; parameter dropped. (revive)
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Changes
8
cmd/entire/cli
agent/claudecode
- Mreviewer.go+1/-1
- Mreviewer_test.go+1/-1
- review
- Mpicker.go+2/-2
- Mpicker_internal_test.go+4/-4
- Mprompt.go+26/-2
- Mprompt_test.go+22
- Mscope.go+13/-5
- Mscope_test.go+6
50 unmodified lines
Mcmd/entire/cli/agent/claudecode/reviewer.go+1/-1
437 unmodified lines
Mcmd/entire/cli/agent/claudecode/reviewer_test.go+1/-1
135 unmodified lines
Mcmd/entire/cli/review/picker.go+2/-2
31 unmodified lines
Mcmd/entire/cli/review/picker_internal_test.go+4/-4
130 unmodified lines
```
// skill-bearing workers kept receiving the maximal-audit brief forever.
func TestGuidedProfileTask_NoBuiltinFallbackPersisted(t *testing.T) {
t.Parallel()
if got := guidedProfileTask(DefaultProfileName, "", "", ""); got != "" {
t.Fatalf("guidedProfileTask with nothing user-provided = %q, want empty", got)
}
}
Mcmd/entire/cli/review/prompt.go+26/-2
315 unmodified lines
```
```
Mcmd/entire/cli/review/prompt_test.go+22
```
10 unmodified lines
```
const (
scopeMaxCommits = 50
scopeMaxFiles = 200
scopeMaxUncommitted = 100
scopeDiffInlineLimit = 48 * 1024
scopeMaxCommits = 50
scopeMaxFiles = 200
scopeMaxUncommitted = 100
// scopeDiffInlineLimit stays well under Windows' ~32KiB CreateProcess
// command-line cap (the prompt travels as one argv string and again via
// ENTIRE_REVIEW_PROMPT), leaving headroom for the rest of the prompt.
scopeDiffInlineLimit = 24 * 1024
)
// BuildScopeContext enumerates the review scope for prompt injection: the
// context
func buildScopeContextCapped(ctx context.Context, repoRoot, baseRef string, caps scopeContextCaps) (reviewtypes.ScopeContext, error) {
var sc reviewtypes.ScopeContext
commitsOut, err := runGit(ctx, repoRoot, "log", "--reverse", "--format=%h %s", baseRef+"..HEAD")
// Newest-first from git, cap (keeping the newest — the prompt declares
// this list authoritative, and recent commits are the ones a review can
// least afford to lose), then reverse for oldest-first reading order.
commitsOut, err := runGit(ctx, repoRoot, "log", "--format=%h %s", baseRef+"..HEAD")
if err != nil {
return sc, fmt.Errorf("list scope commits: %w", err)
}
sc.Commits, sc.CommitsTruncated = capScopeLines(commitsOut, caps.maxCommits)
slices.Reverse(sc.Commits)
filesOut, err := runGit(ctx, repoRoot, "diff", "--name-status", baseRef+"...HEAD")
if err != nil {
return sc, fmt.Errorf("list scope files: %w", err)
}
return sc, nil
}
```
Mcmd/entire/cli/review/scope.go+13/-5
```
587 unmodified lines
```