fix: add anti-loop guardrail for drafter re-reading same file#99
Conversation
There was a problem hiding this comment.
Review Summary
Assessment: 🟡 NEEDS ATTENTION
This PR adds anti-loop guardrails to prevent the drafter from re-reading diff chunk files repeatedly. The approach is sound, but one of the new instructions has a logic flaw that could cause incomplete verification.
Findings
1 MEDIUM severity issue found in changed code
The hard-stop behavior on duplicate reads could interrupt verification mid-process, leading to inconsistent review quality where some findings are verified and others are not.
Details
See inline comments below.
|
/review |
There was a problem hiding this comment.
Review Summary
Assessment: 🟢 APPROVE
Summary
No bugs found in the changed code. This PR adds anti-loop guardrails to the drafter sub-agent's instructions to prevent infinite re-reading of the same diff chunk file.
Changes reviewed:
- Added "ANTI-LOOP" section instructing the drafter to read the diff chunk exactly once
- Added guardrail #5 prohibiting duplicate file reads (max 2 reads per file, 1 for diff chunks)
Analysis:
The changes are defensive safeguards that address the root cause identified in the PR description (629 duplicate reads of the same file). The instructions are clear and well-positioned in the prompt. No logic bugs, edge cases, or unintended side effects detected.
✅ Recommendation: This change is ready to merge.
Review completed by cagent-action
Signed-off-by: Derek Misler <derek.misler@docker.com>
f3e25e8 to
3223323
Compare
Summary
Adds explicit anti-loop instructions to the drafter sub-agent to prevent it from re-reading the same diff chunk file in an infinite loop.
Root cause: In docker/pinata run 23141381851, the drafter called
read_file("/tmp/drafter_chunk_1.diff")629 times without producing any analysis, consuming the entire 40-minute timeout. Each call succeeded (the file existed), so the existing guardrails from #84 (circuit breakers on not-found, read caps on unique files) never triggered.Fix: Two additions to the drafter's prompt instructions:
This is the prompt-level mitigation. A framework-level duplicate-call detector in cagent itself would be the complementary fix.
Closes: https://github.com/docker/gordon/issues/218
Test plan
read_filecalls on the same diff chunk path