-
Notifications
You must be signed in to change notification settings - Fork 897
fix(hooks): prevent notifications triggering loops #531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Background task completion messages were injected as new user prompts and matched keyword-detector search/analyze triggers (e.g., "Explore"), causing recursive background fan-out and repeated repo cloning. This is a problem because it creates runaway subagent sessions, saturates tool usage, and overwhelms the system with redundant clones. The fix tags completion prompts with origin metadata and centralizes origin detection so keyword injection/detection is skipped for `background-notification` parts while still allowing immediate continuation. Added tests for origin detection and keyword-detector behavior.
|
All contributors have signed the CLA. Thank you! ✅ |
|
I am still waiting for the my claude subscription to refill, so I've only tested this on GPT 5.2 Codex at the moment. |
|
I have read the CLA Document and I hereby sign the CLA |
|
@sisyphus-dev-ai review and also test the actual behaviour |
Code Review SummaryI've reviewed and tested PR #531. Here's my detailed analysis: ✅ Problem IdentificationThe PR correctly identifies and solves a critical issue: background task completion messages containing keywords like "Explore" or "analyze" were triggering keyword-detector hooks, causing:
✅ Solution ApproachThe fix implements a clean metadata-based tagging system:
✅ Code QualityStrengths:
Implementation Details:
✅ Test CoverageAdded comprehensive tests:
✅ Verification ResultsType Safety: ✅ Pass Test Suite: ✅ Pass 📋 Behavioral AnalysisThe fix correctly handles the message flow:
🎯 RecommendationsReady to Merge - This PR is production-ready with the following strengths:
🔍 Edge Cases ConsideredThe implementation handles:
📊 Impact AssessmentRisk Level: Low
Affected Components:
All components properly integrated and tested. Verdict: APPROVED ✅ The implementation is clean, well-tested, and solves the recursive loop problem effectively. @jkoelker great work on identifying and fixing this critical issue! Note: Contributor mentioned testing on GPT 5.2 Codex and waiting for Claude subscription. The fix is model-agnostic and will work identically across all providers since it operates at the message metadata level. |
|
While it may be a bit better, the same prompt (resuming the session and jumping back in context) still caused cloneing of alot of repos and the librarian + oracle managed to take the rest of the weekly sonnet usage and 31% of the 5 hour window. From codex's analysis:
If you want, I can add a broader guard next (e.g., skip keyword detection/injection for background sessions entirely, I think what is really needed is co-ordination between agents, so instead of each agent getting its own data, they ask another agent, who will either hand them a path, or deny the request if the "gatekeeper" agent doesn't think it really need the data. I'll mark this ready since it did help a little bit, but since there's an underlying issue still somewhere, feel free to just close it if you don't think the benefit is enough at this time. |
Greptile SummaryThis PR fixes a critical bug where background task completion messages were triggering keyword detectors (like "ultrawork", "search", "analyze"), causing recursive subagent spawning and repository cloning loops. Key Changes:
Quality:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant BG as BackgroundManager
participant Session as Parent Session
participant KD as KeywordDetector Hook
participant CC as ClaudeCode Hook
Note over BG: Background task completes
BG->>BG: Format completion message
BG->>Session: session.prompt() with<br/>metadata: {origin: "background-notification"}
Note over Session,CC: Message enters hook pipeline
Session->>KD: chat.message event
KD->>KD: hasPartOrigin(parts, "background-notification")
alt Has background-notification origin
KD->>KD: Early return (skip keyword detection)
Note over KD: Prevents "ultrawork"/"search"<br/>from triggering recursion
else Regular user message
KD->>KD: detectKeywordsWithType()
KD->>KD: Apply ultrawork/search behavior
end
Session->>CC: chat.message event
CC->>CC: hasPartOrigin(textParts, "background-notification")
alt Has background-notification origin
CC->>CC: Set detectedKeywords = []
Note over CC: Skip keyword message injection
else Regular user message
CC->>CC: detectKeywordsWithType()
CC->>CC: Inject keyword messages
end
Note over Session: Message processed without<br/>triggering recursive loops
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 7 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Summary
Background task completion messages were injected as new user prompts and matched keyword-detector search/analyze triggers (e.g., "Explore"), causing recursive background fan-out and repeated repo cloning. This is a problem because it creates runaway subagent sessions, saturates tool usage, and
overwhelms the system with redundant clones.
Changes
The fix tags completion prompts with origin metadata and centralizes origin detection so keyword injection/detection is skipped for
background-notificationparts while still allowing immediate continuation. Added tests for origin detection and keyword-detector behavior.Testing
bun run typecheck bun testSummary by cubic
Prevents background notification messages from triggering keyword actions, which caused recursive subagent loops and repeated repo cloning. Messages are now tagged with origin metadata, and hooks skip detection for those parts.
Written for commit a202b31. Summary will update on new commits.