feat(review): pass authoritative scope, time budget, and read-only tool permissions to reviewer agents by peyton-alt · Pull Request #1647 · entireio/cli

peyton-alt · 2026-07-06T11:32:12Z

Problem

entire review spawned each reviewer agent cold, unbounded, hobbled — and, it turned out, running the wrong workflow with an unrequested brief:

Cold — the parent computes the review scope for its banner, then tells the child only how to re-derive it. One live run showed a reviewer diffing a behind-main branch in the wrong direction, reporting mainline evolution as branch regressions; 2 of 4 high findings in that verdict were phantoms.
Unbounded — no deadline reached the child; a claude review ~90% complete was killed at the timeout with everything discarded.
Hobbled — headless claude -p auto-denies unapproved tools; finders bounced off denials for git diff/git log.
Wrong skill — the composed prompt led with the configured skill line, and claude -p expanded it as a slash command: it resolved to the built-in /review skill with the whole prompt as arguments, interpolating the configured skill name where a PR number belongs. The configured skill never ran.
Unrequested depth — an empty profile task was silently filled with a built-in comprehensive-audit brief (plus an output-format protocol), and setup persisted that brief into saved profiles as if user-written. Product intent: entire invokes the configured skills; it adds no prompts the user didn't configure.

Changes (one commit each)

Scope injection: RunConfig.ScopeContext carries the parent-computed commit list, three-dot file list, uncommitted lines, and the diff when under a 48KiB inline budget. The judge gets the authoritative file list + an instruction to discard out-of-scope findings.
Time budget: the effective per-reviewer timeout renders as a pacing instruction; multi-agent path now sets ReviewerTimeout per worker.
Read-only allowlist: --allowedTools with Read/Grep/Glob/Task/Skill + enumerated read-only git subcommands (not Bash(git:*)) + entire lookup commands. Never --dangerously-skip-permissions.
Slash-expansion guard: a prose preamble whenever the composed prompt starts with /, instructing skill invocation via the Skill tool — expansion is structurally impossible.
No injected task: user-configured tasks reach every worker verbatim; otherwise skill-bearing workers get an empty Task (skills define behavior — no Task also means no injected output protocol), skill-less workers keep the built-in fallback. Judge keeps a canonical task.
No persisted task: guided and first-run setup no longer write the built-in brief into saved profiles.

The resulting composed prompt is: preamble + configured skills + scope (the argument) + deadline (the harness fact) + whatever the user configured. Zero entire-authored review guidance.

Measured (same branch: 13 commits, 37 files, ~102KB diff; real claude runs)

	before	after all fixes
duration	14–15m (or killed at 10m)	9m08s running two configured skills
skill executed	built-in /review (by accident)	the configured skills, via real Skill invocations
phantom findings	2 of 4 highs in one run	none
tool denials	present	none

Testing

TDD throughout; every behavior change had a failing test first
mise run test (7,570+), test:integration (390), test:e2e:canary (117+8) — green on the final branch state
Shim-based end-to-end smoke: fake claude on PATH capturing argv/env through a real entire review run, asserting prompt & flag contents
Three real-agent runs on a live branch verifying skill routing, scope accuracy, and duration

🤖 Generated with Claude Code

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit ac36d17. Configure here.}

cursor · 2026-07-06T11:33:45Z

+	}
+	lines := strings.Split(trimmed, "\n")
+	if maxLines > 0 && len(lines) > maxLines {
+		return lines[:maxLines], true


Commit cap drops newest commits

High Severity

When the commit list exceeds scopeMaxCommits, capScopeLines keeps the first lines of git log --reverse output, so the oldest commits stay in ScopeContext and everything nearer HEAD is dropped. The prompt still treats that list as authoritative and tells reviewers not to re-derive scope, so long branches can miss recent changes entirely.

^{Reviewed by Cursor Bugbot for commit ac36d17. Configure here.}

Copilot

Pull request overview

This PR improves entire review orchestration by passing parent-computed review scope into spawned reviewer agents and the consolidating judge, adding explicit per-reviewer pacing guidance from the effective timeout, and pre-approving a constrained tool allowlist for headless Claude review children so read-only inspection commands aren’t auto-denied.

Changes:

Add ScopeContext to RunConfig, build it from git (commits/files/uncommitted + optionally inline diff), and inject it into reviewer prompts.
Thread authoritative scope into the judge/synthesis prompt so it can discard out-of-scope findings.
Add reviewer time-budget hint rendering, and pass per-agent timeouts in the multi-agent path.
Add --allowedTools allowlist to Claude review child invocation.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cmd/entire/cli/review/types/reviewer.go	Introduces `ScopeContext` on `RunConfig` and defines scope payload shape/zero check.
cmd/entire/cli/review/synthesis_sink.go	Threads authoritative scope into synthesis prompt composition.
cmd/entire/cli/review/synthesis_prompt.go	Adds “scope gate” section to judge prompt to discard out-of-scope findings.
cmd/entire/cli/review/synthesis_prompt_test.go	Tests that synthesis prompt includes (or omits) the scope gate correctly.
cmd/entire/cli/review/scope.go	Builds bounded scope context (commits/files/uncommitted + inline diff budget).
cmd/entire/cli/review/scope_test.go	Adds coverage for scope context building, three-dot semantics, and caps.
cmd/entire/cli/review/prompt.go	Renders scope context + time budget into reviewer prompt; optionally inlines diff.
cmd/entire/cli/review/prompt_test.go	Tests scope injection and time budget hint behavior.
cmd/entire/cli/review/export_test.go	Updates/export helpers for new synthesis prompt signature and scoped testing.
cmd/entire/cli/review/cmd.go	Injects scope context into single/multi-agent configs; threads scope to sinks; sets per-agent timeout.
cmd/entire/cli/review/cmd_test.go	Extends tests to validate per-agent timeout threading and scope context injection.
cmd/entire/cli/agent/claudecode/reviewer.go	Adds `--allowedTools` allowlist to headless Claude review child.
cmd/entire/cli/agent/claudecode/reviewer_test.go	Tests that allowlist is present and does not enable unsafe flags.

+// scopeContextCaps bounds each ScopeContext field so the composed prompt
+// stays well under per-argument exec limits (Linux caps a single argv string
+// at 128KiB; the diff budget plus list caps keep the whole prompt far below).
+type scopeContextCaps struct {
+	maxCommits      int
+	maxFiles        int
+	maxUncommitted  int
+	diffInlineLimit int
+}
+
+const (
+	scopeMaxCommits      = 50
+	scopeMaxFiles        = 200
+	scopeMaxUncommitted  = 100
+	scopeDiffInlineLimit = 48 * 1024
+)


+	switch {
+	case sc.Diff != "":
+		b.WriteString("\n\nDiff under review:\n```diff\n")
+		b.WriteString(strings.TrimRight(sc.Diff, "\n"))


…ge prompts The parent computes the review scope (commits, files, uncommitted, diff) for its banner, then discards it and tells each child agent only how to re-derive it. That cost minutes of setup per reviewer and let one agent diff a behind-main branch in the wrong direction, reporting mainline evolution as branch regressions — two of four high findings in a real run were such phantoms, and the judge consolidated them unchecked. ComposeReviewPrompt now renders a parent-computed ScopeContext: commit list (oldest first), three-dot name-status file list, porcelain uncommitted lines, and the diff itself when it fits a 48KiB inline budget (beyond that, the exact three-dot command). The judge prompt gets the authoritative changed-file list with an instruction to discard findings outside it. Lists are capped with truncation flags so agents never mistake a bounded enumeration for an exhaustive one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: ca5fa66e3b98

A reviewer that knows its deadline paces itself; one that doesn't runs its full-depth pipeline and gets killed mid-verify with every finding discarded — observed live: a claude-code review was ~90% done when the timeout killed it and the run recorded nothing from those 10 minutes. ComposeReviewPrompt now renders the effective per-reviewer timeout as a pacing instruction. Only a positive RunConfig.ReviewerTimeout renders (zero = unresolved, negative = disabled; inventing a default in either case could contradict the orchestrator's real deadline), and the multi-agent path now sets ReviewerTimeout on each per-agent RunConfig — previously only the single-agent path carried it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 1013984a9665

Headless claude -p auto-denies any tool call that would have prompted interactively, and buildReviewCmd passed no permission config — so finder subagents bounced off denials for git diff/log/show and detoured around them (observed in a live review transcript: 'attempted tool calls, hit approval/tool restrictions, and continued'). Pass --allowedTools with the read-only surface a review needs: Read/ Grep/Glob/Task/TodoWrite, enumerated read-only git subcommands (not Bash(git:*) — aliases/hooks execute arbitrary code and push/commit match the blanket), and entire's read-only lookup commands referenced by the checkpoint context. Never --dangerously-skip-permissions: reviewers process untrusted diffs and must stay unable to write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: f0c2e8deec13

…h command The composed prompt leads with the configured skill invocation line (e.g. /pr-review-toolkit:review-pr). claude -p expands a leading slash token as a command — observed live in two runs: it resolved to the BUILT-IN /review skill with the entire composed prompt as its arguments, interpolating the configured skill name where a PR number belongs. The configured skill never ran; the built-in maximum-breadth PR-review harness ran instead (its diff-proportional line-by-line finder alone took 11.4 of the run's 15 minutes on a 102KB scope). Prefix a prose preamble whenever the composed prompt starts with "/", instructing the agent to invoke the skill lines via the Skill tool — expansion is impossible once the first byte isn't a slash. Add Skill to the reviewer allowlist so headless invocation is pre-approved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 5ffb689d299f

Product intent: entire invokes the configured skills in parallel across agents — it does not add prompts of its own unless the user configured them. Previously an empty profile task was silently filled with the built-in comprehensive-audit brief (correctness, regressions, API design, missing tests, maintainability...), and any non-empty Task also dragged in the output-format protocol. A worker configured with only "skills" therefore received a maximal-audit mandate the user never wrote — measured live as the difference between a ~14-minute entire-spawned review and the same skill invoked directly by the user. Per-worker task resolution: a user-configured task always reaches every worker verbatim; with no user task, skill-bearing workers get an empty Task (skills define behavior — and no Task means no injected output protocol either), while skill-less workers keep the built-in default as fallback so their prompt isn't empty. The judge still receives the canonical profile task for consolidation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 1e2563ad0061

Both setup paths (guided picker fallback and non-interactive first-run) wrote the built-in default brief into the saved profile's task field, making it permanently indistinguishable from a task the user wrote — which defeated the skill-workers-get-no-injected-task rule for every profile created by setup. Persist empty; the built-in brief stays a runtime fallback for skill-less workers and the judge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 53e0eeb2e399

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> Entire-Checkpoint: 320d86619050

…ted-task fix Ran the fixed `entire review` against this branch itself; its verdict identified that fd7d1cb 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> Entire-Checkpoint: d4ef50cdaee1

peyton-alt requested a review from a team as a code owner July 6, 2026 11:32

Copilot AI review requested due to automatic review settings July 6, 2026 11:32

Copilot started reviewing on behalf of peyton-alt July 6, 2026 11:32 View session

cursor Bot reviewed Jul 6, 2026

View reviewed changes

peyton-alt changed the title ~~review: give the child reviewer what the parent already knows (scope, budget, permissions)~~ feat(review): pass authoritative scope, time budget, and read-only tool permissions to reviewer agents Jul 6, 2026

Copilot AI reviewed Jul 6, 2026

View reviewed changes

peyton-alt and others added 3 commits July 6, 2026 13:41

peyton-alt force-pushed the feat/review-child-context branch from ac36d17 to 90c8d54 Compare July 6, 2026 11:42

peyton-alt force-pushed the feat/review-child-context branch from cb8a0dc to 130abe3 Compare July 6, 2026 12:21

peyton-alt and others added 4 commits July 6, 2026 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(review): pass authoritative scope, time budget, and read-only tool permissions to reviewer agents#1647

feat(review): pass authoritative scope, time budget, and read-only tool permissions to reviewer agents#1647
peyton-alt wants to merge 8 commits into
mainfrom
feat/review-child-context

peyton-alt commented Jul 6, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jul 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

peyton-alt commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes (one commit each)

Measured (same branch: 13 commits, 37 files, ~102KB diff; real claude runs)

Testing

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 6, 2026

Choose a reason for hiding this comment

Commit cap drops newest commits

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

peyton-alt commented Jul 6, 2026 •

edited

Loading