fix(macos): suppress notification storm during backend re-init loop (MCP-2328) - #659
Merged
Merged
Conversation
Quarantine and sensitive-data notifications fire on a "count went up vs the last-seen value" heuristic. When the core is stuck in a ~10s re-init / restart loop, each cycle disconnects then replays the full server/activity state, so the tracked count transiently drops and is re-established — making every cycle look like a brand-new event and producing tens of Notification Center alerts. Add ConnectionSettleGate: replay-driven notifications are suppressed until the connection has been free of instability (reconnect / relaunch / error transition / config reload) for a settle interval (12s, above the observed loop period). An active loop re-arms the gate each cycle and never settles, so no spurious alert fires; a genuinely stable connection settles and lets legitimate events through unchanged. - NotificationService: gate guards sendQuarantineAlert / sendSensitiveDataAlert - CoreProcessManager: marks the connection unsettled on transient/error state transitions and on config.reloaded - NotificationReplaySuppressionTests: unit-tests the loop-never-settles invariant against the real production type Related MCP-2328
Deploying mcpproxy-docs with
|
| Latest commit: |
f2a97f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://34806d0c.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-mcp-2328-notification-re.mcpproxy-docs.pages.dev |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27495449138 --repo smart-mcp-proxy/mcpproxy-go
|
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gatekeeper approval — Codex review verdict: ACCEPT.
This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of the Codex reviewer (verdict of record lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.
Auto-approved per Model B (MCP-1249).
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
…r server Related #786 Spec 077 US4 (MCP-2207): the security-scan notification storm came from per-scanner scan_started/progress/completed/failed SSE events multiplied by reconnect storms (prior partial fixes: #659, MCP-2223). Replace those per-scanner lifecycle emissions with a single debounced security.scan_settled event per server per scan. ## Changes - Add scanNotifyDebouncer (internal/runtime/scan_notify.go): terminal-triggered per-server debounce with a generation counter guarding the AfterFunc race; only completed/failed arm the timer, started/progress are dropped. - Add EventTypeSecurityScanSettled; route runtime EmitSecurityScan* through the debouncer (started/progress become no-ops, completed/failed record terminal state) and publish one settled event carrying the terminal findings summary. - Wire scanNotify (750ms) into newRuntime alongside the existing coalescer. - Collapse the activity log to one handleSecurityScanSettled record per scan, removing the former started/completed/failed handlers. ## Testing - scan_notify_test.go: a reconnect storm across N servers yields <= N settled events (exactly one per server) and zero per-scanner lifecycle events; settled event carries the terminal summary. Related: Spec 077 (specs/077-scanner-simplification)
Dumbris
added a commit
that referenced
this pull request
Jul 2, 2026
… event (Spec 077, MCP-2207) (#794) * feat(security): debounce scan notifications into one settled event per server Related #786 Spec 077 US4 (MCP-2207): the security-scan notification storm came from per-scanner scan_started/progress/completed/failed SSE events multiplied by reconnect storms (prior partial fixes: #659, MCP-2223). Replace those per-scanner lifecycle emissions with a single debounced security.scan_settled event per server per scan. ## Changes - Add scanNotifyDebouncer (internal/runtime/scan_notify.go): terminal-triggered per-server debounce with a generation counter guarding the AfterFunc race; only completed/failed arm the timer, started/progress are dropped. - Add EventTypeSecurityScanSettled; route runtime EmitSecurityScan* through the debouncer (started/progress become no-ops, completed/failed record terminal state) and publish one settled event carrying the terminal findings summary. - Wire scanNotify (750ms) into newRuntime alongside the existing coalescer. - Collapse the activity log to one handleSecurityScanSettled record per scan, removing the former started/completed/failed handlers. ## Testing - scan_notify_test.go: a reconnect storm across N servers yields <= N settled events (exactly one per server) and zero per-scanner lifecycle events; settled event carries the terminal summary. Related: Spec 077 (specs/077-scanner-simplification) * feat(web-ui): consume debounced scan.settled event; drop per-scanner lifecycle Related #786 Spec 077 US4 (MCP-2207): forward the new security.scan_settled SSE event from the system store as a mcpproxy:scan-settled window event, and have useSecurityScannerStatus refresh its cached scan totals off that single settled signal instead of tracking per-scanner lifecycle events. ## Changes - stores/system.ts: add a security.scan_settled SSE listener that dispatches mcpproxy:scan-settled. - composables/useSecurityScannerStatus.ts: register a module-scope mcpproxy:scan-settled listener that triggers a status refresh. ## Testing - frontend vue-tsc --noEmit clean; vite build succeeds. Related: Spec 077 (specs/077-scanner-simplification)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the macOS tray notification storm (tens of Notification Center alerts top-right) that appears while the backend core is stuck in a ~10s re-init / restart loop.
PR #656 (MCP-2215) proved the Vue Web UI emits zero toasts on SSE reconnect/replay — the frontend is exonerated. The user-visible symptom is native macOS Notification Center alerts, not browser toasts.
Root cause
sendQuarantineAlert(onservers.changed) andsendSensitiveDataAlert(onactivity) fire on a "count went up vs the last-seen value" heuristic inCoreProcessManager.handleSSEEvent. During a re-init loop each cycle disconnects then replays the full server/activity state, so the tracked count transiently drops to 0 and is re-established — making every cycle look like a brand-new event. The existing 5-min per-key rate limit doesn't fully tame it across the loop, and the alerts are spurious regardless.Fix
Add
ConnectionSettleGatetoNotificationService:CoreProcessManagermarks the connection unsettled on every transient/error state transition (.launching/.waitingForCore/.reconnecting/.error) and onconfig.reloaded..connected(steady state) never marks unsettled, so once the loop ends real notifications resume.Acceptance criteria
NotificationReplaySuppressionTestsguards the loop-never-settles invariant against the real productionConnectionSettleGatetype (deterministic injected clock).Verification
swift test --filter "NotificationReplaySuppressionTests|NotificationRateLimitTests"→ 20 passed, 0 failures.swiftc -O, exit 0).swift testrun (SSEParserTests / AutoStartTests / MergePatchEncodingTests / ModelsTests) are pre-existing on cleanorigin/main(verified by stashing this change and re-running) — unrelated to notifications.Note: the storm requires a live backend crash-loop to reproduce; behavior is timing-based and not screenshot-verifiable, so the unit test (AC-3) is the regression guard.
Related MCP-2328 · refs #656 (MCP-2215), MCP-2207