mux: dedupe and gate all grok agent-hook notifications - #7619
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds notification classification and deduplication policy code, updates the hooks CLI to use it for Grok and antigravity notifications, adds regression tests, and wires the new Swift files into the Xcode project. ChangesGrok notification classification and dedupe
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR updates Grok agent-hook notification handling to reduce duplicate alerts. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "mux: dedupe and gate all grok agent-hook..." | Re-trigger Greptile |
| let notifications = notifyCommands(in: Array(context.state.snapshot().dropFirst(fallbackStart))) | ||
| XCTAssertEqual(notifications.count, 1, "Unclassified fallback re-notification should dedupe, saw \(notifications)") | ||
| XCTAssertTrue( | ||
| notifications.first?.hasSuffix("|c=idle-reminder;p=0") == true, |
There was a problem hiding this comment.
Fallback Seed Produces Untagged Alert
The fallback test seeds the session with a permission notification, so the later unclassified payload rebuilds from a needs-input state rather than an idle state. That path emits the attention fallback without the c=idle-reminder suffix, so this test can fail even when the CLI follows the current notification contract.
| defer { context.cleanup() } | ||
|
|
||
| try runGrokNoiseHook(context, "session-start", payload: grokNoisePayload(context, event: "SessionStart")) | ||
| try runGrokNoiseHook(context, "notification", payload: grokNoisePayload(context, event: "Notification", message: "Grok needs permission to run rm")) |
There was a problem hiding this comment.
Seed idle fallback This test still seeds the fallback rebuild with a permission notification, so the saved session status is the permission or needs-input state rather than the idle-reminder state asserted below. When the later unclassified JSON has no hook event or message, the CLI rebuilds from that saved record; a correct implementation can produce the prior permission classification, or an untagged fallback, instead of
|c=idle-reminder;p=0. That keeps this regression test tied to the wrong precondition and can fail even when idle fallback reminders are correctly gateable. Seed this case with an idle or waiting record, or split the permission fallback behavior into its own assertion.
Integration coverage for #7611 driven through the spawn-the-CLI harness against a mock socket, using payload shapes captured from a live Grok Build 0.2.91 session: - repeated identical "waiting for input" Notification events must dedupe within a turn (currently every repeat delivers a fresh banner + sound) - repeated identical permission_prompt notifications must dedupe per turn: grok emits {"notificationType":"permission_prompt","message": "Tool permission requested"} for EVERY tool step, even in auto-approve mode where nothing awaits the user, so a 6-step task rings 6 times; a prompt-submit (new turn) must re-arm delivery - distinct permission prompts must each deliver (always-deliver for novel approval content is preserved) - unparseable payloads rebuilt from the stored session record must carry gateable c=idle-reminder meta and dedupe (currently untagged, so the per-category notification settings cannot silence them) - a mid-session SessionStart re-fire must not re-arm the completion dedupe (currently clearNotificationEmission re-arms the same ding) - guards: antigravity error notifications stay untagged, incidental completion keywords cannot re-ding after the real turn-complete Tests are committed first and are red on this commit by design; the fix lands in the follow-up commit (two-commit red/green policy). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes the noise paths behind grok Build notification spam (#7611): - Dedupe every notification status, not just .idle: fingerprints are now status + a stable FNV-1a body hash (cross-process safe; the session store persists between CLI invocations). .idle keeps the whole-turn "idle-turn" fingerprint so incidental completion-keyword messages cannot re-ding. - Dedupe identical permission prompts per turn: live capture from Grok Build 0.2.91 shows an identical generic {"notificationType":"permission_prompt","message":"Tool permission requested"} Notification for every tool step, even in auto-approve mode where nothing awaits the user, so long tasks ring once per step. Identical bodies now dedupe within the turn, prompt-submit re-arms delivery for the next turn, and permission prompts with novel content still always deliver. - Make every summary carry a notifyCategory: the "needs your attention" fallback, arbitrary-text attention alerts, and the stale-record rebuild path now tag c=idle-reminder, so the per-category settings from #7129 can silence them. Errors keep the explicit .other always-deliver exemption (unchanged wire behavior). - Preserve dedupe across grok's mid-session SessionStart re-fires (auto-continue/restarts) instead of re-arming the completion ding; prompt-submit clearing is unchanged. - Replace the single-slot emitted-fingerprint store with a small per-session map (60 min window, 16-entry cap, legacy back-compat) so an interleaved notification cannot evict the idle-turn fingerprint. - Recognize grok's camelCase "notificationType" payload key in the classifier signal (captured from real traffic). The classification/dedupe policy moves to a new pure file, CLI/AgentHookNotificationPolicy.swift, compiled into both cmux-cli and cmuxTests (same pattern as FeedEventClassifier), with unit coverage of the classification table, fingerprint stability, and the app-gate meta round-trip. Claude lane wire output and antigravity fullyIdle gating are byte-identical. No new user-facing strings; existing localization keys move verbatim. Closes #7611 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3d0890d to
57cf562
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 57cf562. Configure here.
| status: mapped?.lastNotificationStatus, | ||
| isFallback: false, | ||
| notifyCategory: mapped?.lastNotificationStatus == .idle ? .turnComplete : nil | ||
| notifyCategory: mapped?.lastNotificationStatus == .idle ? .turnComplete : .idleReminder |
There was a problem hiding this comment.
Stale error alerts wrongly gated
Medium Severity
When a generic fallback notification is rebuilt from the session store, every non-idle lastNotificationStatus gets notifyCategory idleReminder, including .error. Fresh errors use .other and untagged wire meta so they always deliver; rebuilt error replays get c=idle-reminder and can be silenced by “Agent Waiting for Input” settings.
Reviewed by Cursor Bugbot for commit 57cf562. Configure here.


Closes #7611
Problem
Grok Build sessions spam notification sounds. Two report waves, both root-caused:
.idlehook notifications were deduped — every repeated Waiting/Error/Attention event delivered a fresh banner + sound; unclassified fallback payloads carried noc=<category>;p=<0|1>meta so the per-category settings from Gate agent notifications on background work + per-category settings #7129 could not silence them; mid-sessionSessionStartre-fires re-armed the completion ding.GROK_HOME): grok emits an identical{"notificationType":"permission_prompt","message":"Tool permission requested"}Notification for every tool step — even in--permission-mode autowhere nothing awaits the user. A 6-step task = 6 banners + 6 sounds. Grok has no cmux Feed approval lane (PreToolUse is non-actionable telemetry), so these were classified needsPermission and deliberately exempt from dedupe.Fix (class-level)
status + FNV-1a(body)(stable across CLI processes —String.hashValueis per-process-randomized and the fingerprint persists in the session store)..idlekeeps the whole-turn"idle-turn"fingerprint so incidental completion keywords ("All done…") can't re-ding after the real turn-complete.prompt-submit(a real turn boundary) re-arms delivery; permission prompts with novel content always deliver. First prompt of a turn still alerts an away user; the per-step spam dies.notifyCategory. The "needs your attention" fallback, arbitrary-text attention alerts, and the stale-record rebuild path tagc=idle-reminder, so "Agent Waiting for Input" silences them. Errors keep the explicit.otheralways-deliver exemption (wire output unchanged)."idle-turn".notificationTypekey (captured from real traffic).The classification/dedupe policy moves to a pure new file
CLI/AgentHookNotificationPolicy.swift, compiled into bothcmux-cliandcmuxTests(same pattern asFeedEventClassifier).CLI/cmux.swiftshrinks by 119 lines; no budget TSV changes. Claude-lane wire output and antigravity fullyIdle gating are byte-identical.Two-commit red/green structure
Commit 1 adds only the integration regression tests (spawn-the-CLI harness, isolated temp HOME/GROK_HOME/state, payload shapes from live capture): repeated-waiting dedupe, per-turn permission dedupe + prompt-submit re-arm, distinct-permission always-deliver guard, gateable fallback rebuild, SessionStart re-fire, antigravity error guard, incidental-keyword guard. Commit 2 turns it green and adds pure policy unit tests. Note: the
testsgate treats assertion-only failures as "expected" ((0 unexpected)tolerance), so the commit-1 redness is visible in the shard logs (failingTest Caselines) rather than in the check status.Verification
GROK_HOME/state (no real hook configs touched), including real Grok Build 0.2.91 sessions: pre-v2, a 6-tool-step auto-mode task delivered 6 permission banners; post-v2 expectation is 1 (re-verified after the rebuild). Synthetic repro matrix: repeatedwaiting for input→ 1 banner +skipDuplicate;{"unparseable":true}after a permission seed → 1 tagged banner + dedupe; with "Agent Waiting for Input" disabled the app logssocket.notifyTargetAsync.gated category=idle-reminderand shows nothing; incidental "All done…" after a real completion →skipDuplicate idle-turn; SessionStart re-fire + same completion →skipDuplicate.python3 scripts/swift_file_length_budget.py,./scripts/lint-pbxproj-test-wiring.sh,python3 scripts/normalize-pbxproj.py --checkall pass; taggedxcodebuild build-for-testingcompiles app + test bundles (no local test execution per repo policy).Localizable.xcstringskeys move verbatim into the policy file.🤖 Generated with Claude Code
Summary by CodeRabbit
Note
Medium Risk
Changes notification delivery and dedupe for Grok/Antigravity hooks; misclassification or fingerprint bugs could suppress real alerts or reintroduce spam, though behavior is heavily regression-tested.
Overview
Cuts Grok (and shared Antigravity) agent-hook notification spam by moving classification, dedupe, and notify meta into
AgentHookNotificationPolicy.swiftand tightening hook behavior incmux.swift.Dedupe now applies to all notification statuses for eligible agents, not only idle completions: fingerprints use
idle-turnfor completions andstatus + FNV-1a(body)for everything else. The session store keeps a multi-fingerprint map (with legacy single-slot fallback) so interleaved events do not drop the completion fingerprint. Grok skips clearing dedupe on mid-sessionSessionStartre-fires;prompt-submitstill clears for a new turn.User settings can gate more alerts: summaries always carry a
notifyCategory; attention/fallback and stale-record rebuilds tagc=idle-reminder, while errors stay untagged via.other. Wire meta is built throughmetaSegment(pending:)instead of ad-hoc helpers.Adds unit (
AgentHookNotificationPolicyTests) and CLI integration (CLIGrokNotificationNoiseTests) coverage for repeated waiting, permission dedupe per turn, SessionStart re-fire, gateable fallbacks, and incidental completion cues.Reviewed by Cursor Bugbot for commit 57cf562. Bugbot is set up for automated code reviews on this repo. Configure here.