Skip to content

fix(#250): remove unexecutable gh api fallback from review sub-agent prompts - #253

Merged
rh-hemartin merged 1 commit into
mainfrom
agent/250-fix-subagent-file-fetch-fallback
Jul 20, 2026
Merged

fix(#250): remove unexecutable gh api fallback from review sub-agent prompts#253
rh-hemartin merged 1 commit into
mainfrom
agent/250-fix-subagent-file-fetch-fallback

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Remove unexecutable gh api fallback instructions from review sub-agent prompt templates in skills/pr-review/SKILL.md
  • All 7 review sub-agents only have Read, Grep, Glob tools — none has Bash access, making the gh api file-fetch instruction impossible to execute
  • Replace with honest guidance: omitted files should be treated as unavailable for PR-head verification, and findings about those files should note the limitation

Fixes #250

Test plan

  • Verified all 7 sub-agent definitions confirm tools: Read, Grep, Glob (no Bash)
  • Confirmed no remaining gh api instructions directed at sub-agents in SKILL.md
  • Verified orchestrator's own gh api calls in step 2b bash script are unchanged (orchestrator has Bash)
  • Secret scan passes
  • Pre-existing test suite failure (prerequisites-creates-allowed-issue) is unrelated — same failure on main

🤖 Generated with Claude Code


Closes #250

Post-script verification

  • Branch is not main/master (agent/250-fix-subagent-file-fetch-fallback)
  • Secret scan passed (gitleaks — 6565264dd3ce65cbf5361202aadab0047b71f015..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

…prompts

All 7 review sub-agents declare only Read, Grep, Glob as allowed
tools — none has Bash access. The gh api fallback instruction added
in PR #172 for large-PR file fetching was therefore unexecutable,
creating a contradiction with the meta-prompt constraint that forbids
reading from disk.

Replace the gh api fallback instructions in steps 2b, 3d, and 4 with
honest guidance: omitted files should be treated as unavailable for
PR-head verification, and findings about those files should note the
limitation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 17, 2026 11:30
@ben-alkov ben-alkov added bug Something isn't working review-agent labels Jul 17, 2026
@ben-alkov

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:21 PM UTC · Completed 12:24 PM UTC
Commit: cc7a526 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review — approve

PR: fix(#250): remove unexecutable gh api fallback from review sub-agent prompts
Scope: skills/pr-review/SKILL.md — 3 instruction blocks rewritten (steps 2b size-guard, 3d context package, and 4 sub-agent template)

Analysis

This PR correctly implements Option A from issue #250. The change is narrow, well-scoped, and addresses a real inconsistency: all 7 review sub-agents (correctness, security, intent-coherence, style-conventions, docs-currency, cross-repo-contracts, challenger) declare tools: Read, Grep, Glob — none has Bash access — yet the prior SKILL.md text instructed them to run gh api commands to fetch omitted files on large PRs.

Verification performed:

  1. Sub-agent tool declarations confirmed: All 7 sub-agent .md files declare tools: Read, Grep, Glob only. No sub-agent has Bash.
  2. All sub-agent-directed gh api instructions removed: The three modified sections (size-guard paragraph at ~line 192, context package fields at ~line 367, and template note at ~line 461) no longer instruct sub-agents to use gh api.
  3. Orchestrator gh api calls preserved: The orchestrator's own bash scripts in steps 1, 2b (file fetching), and 2a (compare) are unchanged and correct — the orchestrator has Bash access.
  4. Remaining gh api references are orchestrator-scoped: Lines 559 and 1043 reference gh api in the context of "don't re-execute subagent commands" guidance directed at the orchestrator, not sub-agents.
  5. head_sha and repo_full_name repurposed consistently: These fields remain in the context package but their descriptions now accurately say "included for reference in sub-agent findings" rather than claiming they enable API calls.
  6. No sub-agent definition references the removed capability: Grep of all sub-agent .md files for head_sha, repo_full_name, contents API, and fetch returned no matches.
  7. Replacement guidance is honest and actionable: Sub-agents are told to treat omitted files as unavailable and note the limitation in findings — a clear, executable instruction that matches their actual capabilities.

Verdict

The change is correct, minimal, and internally consistent. The commit message follows Conventional Commits (fix(#250): with descriptive body). The PR correctly links to issue #250. No behavioral regressions — this is a documentation-only fix to an instruction that was already unexecutable.


Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

  • skills/pr-review/SKILL.md

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 17, 2026
@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 20, 2026
Merged via the queue into main with commit c65eba2 Jul 20, 2026
31 checks passed
@rh-hemartin
rh-hemartin deleted the agent/250-fix-subagent-file-fetch-fallback branch July 20, 2026 11:39
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 11:41 AM UTC · Completed 11:58 AM UTC
Commit: dd0f0c1 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #253 — Remove unexecutable gh api fallback from review sub-agent prompts

PR #253 fixed issue #250, which identified that skills/pr-review/SKILL.md instructed review sub-agents to use gh api for fetching omitted files on large PRs, but none of the 7 sub-agents have Bash access. The fix replaced unexecutable instructions with honest guidance that omitted files should be treated as unavailable.

Pipeline quality: strong

  • Issue quality (retro agent): Excellent. Issue Review sub-agents cannot execute large-PR file-fetch fallback (no Bash tool) #250 was filed by the retro agent from PR perf(#1924): pass source file contents to review sub-agents #172 analysis with verified root cause, two proposed fix options, specific target files, and validation criteria.
  • Triage: Fast and accurate (5 min). Verified all 7 sub-agent tool declarations, confirmed the bug, recommended Option A.
  • Code agent: Efficient (6 min). Chose the simpler Option A correctly. Produced a minimal 1-file diff (+17/−16 lines) that was well-scoped and internally consistent.
  • Review agent: Thorough. Verified all 7 sub-agents lack Bash, confirmed orchestrator gh api calls preserved, checked remaining gh api references are orchestrator-scoped, verified head_sha/repo_full_name fields repurposed consistently. Correctly flagged the protected path requiring human approval.
  • Human review: Approved and merged 3 days later (July 20). No additional concerns raised beyond what the review agent covered.

Findings (all covered by existing issues)

1. Auto-review dispatch silently skipped for bot-authored PR. The dispatch shim (run 29577076948) at 11:30 UTC silently skipped review dispatch because the auth check cannot resolve permissions for GitHub App bot accounts (fullsend-ai-coder[bot]). A human had to manually trigger /fs-review ~50 minutes later. This is covered by recently-closed fullsend#5238 ("Dispatch auth check silently rejects code-agent bot PRs from review," closed 2026-07-17). PR #253 provides evidence the fix was not yet deployed at 11:30 UTC on July 17.

2. Redundant review dispatches. 6 review-related workflow runs were triggered for this 1-file PR: 1 cancelled (debounce), 1 silently skipped (auth), 1 meaningful review, and 3 cascade dispatches (review comment posting, human approval events). Well-covered by existing issues: fullsend#893, fullsend#2994, fullsend#3673; agents#108, agents#204.

3. Prompt-to-tool consistency gap. The original bug (SKILL.md referencing tools sub-agents don't have) was introduced in PR #172 and caught by the retro agent post-merge. A lint or eval check could catch this class of bug at PR time. Covered by agents#188 ("Evaluate skillsaw for linting agent instructions").

No new proposals — all identified improvements are covered by existing open or recently-closed issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working requires-manual-review Review requires human judgment review-agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review sub-agents cannot execute large-PR file-fetch fallback (no Bash tool)

2 participants