fix(review): address dogfood-review findings — complete the no-persisted-task fix

5faec78·

peyton-alt·1w ago·11 files·+169 added/-35 removed

Ran the fixed entire review against this branch itself; its verdict identified that fd7d1cbd9 was incomplete and five smaller defects. All confirmed against the code:

• buildCrewProfile, saveReviewProfileConfig, and the flags-configure path still seeded the built-in task into saved profiles, so every interactively-configured profile reintroduced the maximal-audit brief for skill-bearing workers. The prior test was tautological (inputs production never produces); replaced with a test through the real constructor, plus an inverted stale assertion in configure_test.
• Scope-context failure degraded at Debug (invisible at default level) right after the banner claimed the scope was computed. Now Warn plus a one-line stderr notice.
• The categorical "discard out-of-scope findings" rule contradicted truncated file lists (files past the cap are in scope) and rendered nonsense for net-zero diffs (commit + revert). The rule now matches what was rendered: absent without lists, softened to verify-first when truncated.
• Rendered list bytes are charged against the inline-diff budget so the composed prompt stays bounded on ~32KiB-limited platforms.
• Allowlist comment now names the accepted --output residual instead of claiming blanket write-safety.
• Test gaps: fan-out loop now asserts per-worker ScopeContext/Task wiring; uncommitted-only scope pins Diff==""/!DiffOmitted; redundant Skill allowlist test folded into the main one.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

d4ef50cdaee1View transcript

Changes

11

• cmd/entire/cli

  • agent/claudecode

    • Mreviewer.go+7/-3
    • Mreviewer_test.go+1/-17

  • review

    • Mcmd.go+8/-7
    • Mcmd_test.go+14
    • Mconfigure_test.go+2/-2
    • Mpicker.go+2/-4
    • Mpicker_internal_test.go+14
    • Mprompt.go+14/-1
    • Mprompt_test.go+39
    • Mscope.go+19/-1
    • Mscope_test.go+49

41 unmodified lines  
42  
43  
44  
45  
45  
46  
47  
48  
49  
48  
49  
50  
51  
52  
53  
54  
55  
56  

// Deliberately narrow: read-only git subcommands are enumerated instead of
// granting Bash(git:) — git can execute arbitrary code via aliases/hooks,
// and push/commit match the blanket pattern. Nothing write-capable (Edit,
// and push/commit match the blanket pattern. No file tools that write (Edit,
// Write) and never --dangerously-skip-permissions: reviewers process
// untrusted input (the diff under review), so the child must stay unable to
// modify the repo. --allowedTools ADDS to the user's own permission config;
// it cannot revoke anything.
var reviewToolAllowlist = []string{
"Read", "Grep", "Glob", "Task", "TodoWrite", "Skill",
"Bash(git diff:)", "Bash(git log:)", "Bash(git show:)",

Mcmd/entire/cli/agent/claudecode/reviewer.go+7/-3

429 unmodified lines
430
431
432
433
433
434
435
436
47 unmodified lines

...

Mcmd/entire/cli/review/cmd.go+8/-7

775 unmodified lines

...

Mcmd/entire/cli/review/cmd_test.go+14

68 unmodified lines

...

Mcmd/entire/cli/review/configure_test.go+2/-2

433 unmodified lines

...

Mcmd/entire/cli/review/picker.go+2/-4

76 unmodified lines

...

Mcmd/entire/cli/review/picker_internal_test.go+14

98 unmodified lines

...

Mcmd/entire/cli/review/prompt.go+14/-1

291 unmodified lines

...

Mcmd/entire/cli/review/scope.go+19/-1

640 unmodified lines

...