From f2a97f3445c1f96723ad732ffdbc8012f6bbc021 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Sun, 14 Jun 2026 12:47:23 +0300 Subject: [PATCH] fix(macos): suppress notification storm during backend re-init loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../MCPProxy/Core/CoreProcessManager.swift | 20 +++- .../Services/NotificationService.swift | 72 ++++++++++++++ .../NotificationReplaySuppressionTests.swift | 96 +++++++++++++++++++ 3 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 native/macos/MCPProxy/MCPProxyTests/NotificationReplaySuppressionTests.swift diff --git a/native/macos/MCPProxy/MCPProxy/Core/CoreProcessManager.swift b/native/macos/MCPProxy/MCPProxy/Core/CoreProcessManager.swift index 452dc20aa..cc44dc436 100644 --- a/native/macos/MCPProxy/MCPProxy/Core/CoreProcessManager.swift +++ b/native/macos/MCPProxy/MCPProxy/Core/CoreProcessManager.swift @@ -543,7 +543,12 @@ actor CoreProcessManager { } case "config.reloaded": - // Configuration reloaded; refresh everything once + // Configuration reloaded; refresh everything once. + // A re-init loop re-emits config.reloaded each cycle even when the + // SSE connection stays up, so treat it as an instability signal: + // this re-arms the settle gate and suppresses the replay-driven + // quarantine/sensitive notifications for the duration (MCP-2328). + await notificationService.markConnectionUnsettled() await refreshState() await MainActor.run { appState.serversVersion += 1 @@ -803,6 +808,19 @@ actor CoreProcessManager { /// Transition the core state via the main actor. private func transitionState(to newState: CoreState) async { await appState.transition(to: newState) + + // Signal connection instability so replay-driven notifications + // (quarantine, sensitive-data) are suppressed until the connection + // settles. Every reconnect / relaunch / crash funnels through here, + // so during a backend re-init loop the gate is re-armed each cycle and + // never settles — breaking the notification storm (MCP-2328). + // `.connected` is the steady state and is intentionally NOT marked. + switch newState { + case .launching, .waitingForCore, .reconnecting, .error: + await notificationService.markConnectionUnsettled() + case .idle, .connected, .shuttingDown: + break + } } // MARK: - Private: API Key Generation diff --git a/native/macos/MCPProxy/MCPProxy/Services/NotificationService.swift b/native/macos/MCPProxy/MCPProxy/Services/NotificationService.swift index 037a595b0..65a93d34d 100644 --- a/native/macos/MCPProxy/MCPProxy/Services/NotificationService.swift +++ b/native/macos/MCPProxy/MCPProxy/Services/NotificationService.swift @@ -28,6 +28,52 @@ enum NotificationAction: String { case update = "UPDATE" } +// MARK: - Connection Settle Gate + +/// Decides whether SSE-replay-driven notifications (quarantine, sensitive-data) +/// may fire, based on how recently the core connection was unsettled. +/// +/// These notifications use a "count went up vs the last-seen value" heuristic. +/// When the core is stuck in a re-init / restart loop (observed ~10s cadence), +/// 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 a notification storm (MCP-2328). +/// +/// The gate requires the connection to have been free of instability for +/// `settleInterval` before such notifications are allowed again. An active loop +/// re-marks the connection unsettled faster than it can settle (the loop period +/// is shorter than `settleInterval`), so nothing fires for its duration; a +/// genuinely stable connection settles and lets legitimate events through. +struct ConnectionSettleGate { + + /// How long the connection must be free of instability before + /// replay-driven notifications are allowed again. Chosen above the + /// observed ~10s re-init loop period so an active loop never appears + /// settled. + let settleInterval: TimeInterval + + /// Timestamp of the most recent instability signal, or `nil` if the + /// connection has never been marked unsettled (steady since launch). + private var lastUnsettledAt: Date? + + init(settleInterval: TimeInterval = 12) { + self.settleInterval = settleInterval + } + + /// Record an instability signal: a reconnect, relaunch, error transition, + /// or config reload. Resets the settle window. + mutating func markUnsettled(now: Date = Date()) { + lastUnsettledAt = now + } + + /// Whether replay-driven notifications may fire at `now`. True when no + /// instability has been seen within `settleInterval`. + func isSettled(now: Date = Date()) -> Bool { + guard let last = lastUnsettledAt else { return true } + return now.timeIntervalSince(last) >= settleInterval + } +} + // MARK: - Notification Service /// Actor that manages macOS notification delivery with rate limiting. @@ -43,6 +89,11 @@ actor NotificationService { /// Minimum interval between repeated notifications of the same kind. private let rateLimitInterval: TimeInterval = 300 // 5 minutes + /// Gate that suppresses replay-driven notifications while the core + /// connection is unsettled (e.g. during a backend re-init loop). See + /// `ConnectionSettleGate` and `markConnectionUnsettled()`. + private var settleGate = ConnectionSettleGate() + /// The shared notification center. private let center = UNUserNotificationCenter.current() @@ -68,10 +119,27 @@ actor NotificationService { center.setNotificationCategories(categories) } + // MARK: - Connection State + + /// Record that the core connection just became unsettled — a reconnect, + /// relaunch, error transition, or config reload. Replay-driven + /// notifications (quarantine, sensitive-data) are suppressed until the + /// connection has been continuously settled for the gate's interval. This + /// is what breaks the notification storm during a backend re-init loop + /// (MCP-2328): each loop cycle re-marks the connection unsettled before it + /// can settle, so no spurious replay alert ever fires. + func markConnectionUnsettled() { + settleGate.markUnsettled() + } + // MARK: - Notification Senders /// Notify about sensitive data detected in a tool call. func sendSensitiveDataAlert(server: String, tool: String, category: String) async { + // Suppress while the connection is unsettled: a re-init loop replays + // the full activity list each cycle, which the count-delta heuristic + // upstream would otherwise read as a fresh detection. + guard settleGate.isSettled() else { return } let key = "sensitive:\(server):\(tool)" guard shouldDeliver(key: key) else { return } @@ -93,6 +161,10 @@ actor NotificationService { /// Notify about a server entering quarantine (new or changed tools detected). func sendQuarantineAlert(server: String, toolCount: Int) async { + // Suppress while the connection is unsettled: a re-init loop replays + // the full server list each cycle, which the count-delta heuristic + // upstream would otherwise read as a fresh quarantine event. + guard settleGate.isSettled() else { return } let key = "quarantine:\(server)" guard shouldDeliver(key: key) else { return } diff --git a/native/macos/MCPProxy/MCPProxyTests/NotificationReplaySuppressionTests.swift b/native/macos/MCPProxy/MCPProxyTests/NotificationReplaySuppressionTests.swift new file mode 100644 index 000000000..a74b80584 --- /dev/null +++ b/native/macos/MCPProxy/MCPProxyTests/NotificationReplaySuppressionTests.swift @@ -0,0 +1,96 @@ +import XCTest +@testable import MCPProxy + +/// Tests the connection-settle gate that suppresses SSE-replay-driven +/// notifications (quarantine, sensitive-data) during a backend re-init / +/// restart loop. +/// +/// Background (MCP-2328): quarantine/sensitive notifications fire on a +/// "count went up vs the last-seen value" heuristic. When the core is stuck +/// in a ~10s re-init loop, each cycle disconnects then replays the full +/// server/activity state, so the count transiently drops and is then +/// re-established — making every cycle look like a brand-new event and +/// producing a notification storm. +/// +/// `ConnectionSettleGate` requires the connection to have been free of +/// instability (reconnect / relaunch / config reload) for `settleInterval` +/// before such notifications are allowed again. An active loop keeps marking +/// the connection unsettled faster than it can settle, so nothing fires; a +/// genuinely stable connection settles and lets real events through. +final class NotificationReplaySuppressionTests: XCTestCase { + + // MARK: - Default settled state + + func testFreshGateIsSettled() { + // With no instability ever recorded, replay-driven notifications are + // allowed (e.g. a long-running, stable session). + let gate = ConnectionSettleGate(settleInterval: 12) + XCTAssertTrue(gate.isSettled(now: Date())) + } + + // MARK: - Suppression while unsettled + + func testNotSettledImmediatelyAfterInstability() { + var gate = ConnectionSettleGate(settleInterval: 12) + let t0 = Date() + gate.markUnsettled(now: t0) + // Right after a reconnect/relaunch the connection is unsettled. + XCTAssertFalse(gate.isSettled(now: t0)) + // 5s later — still inside the settle window. + XCTAssertFalse(gate.isSettled(now: t0.addingTimeInterval(5))) + // 11s later — still inside. + XCTAssertFalse(gate.isSettled(now: t0.addingTimeInterval(11))) + } + + func testSettledAtAndAfterInterval() { + var gate = ConnectionSettleGate(settleInterval: 12) + let t0 = Date() + gate.markUnsettled(now: t0) + // Exactly at the boundary — settled. + XCTAssertTrue(gate.isSettled(now: t0.addingTimeInterval(12))) + // Comfortably past — settled. + XCTAssertTrue(gate.isSettled(now: t0.addingTimeInterval(30))) + } + + // MARK: - The restart-loop invariant (AC-1) + + /// A ~10s re-init loop marks the connection unsettled every cycle. Because + /// the cycle period (10s) is shorter than the settle interval (12s), the + /// gate NEVER reports settled for the duration of the loop — so no + /// replay-driven notification is ever allowed to fire. + func testRestartLoopNeverSettles() { + var gate = ConnectionSettleGate(settleInterval: 12) + let start = Date() + let cyclePeriod: TimeInterval = 10 + for cycle in 0..<30 { // ~5 minutes of looping + let t = start.addingTimeInterval(cyclePeriod * Double(cycle)) + gate.markUnsettled(now: t) + // At the moment of each re-init, and just before the next one, + // the gate must report unsettled. + XCTAssertFalse(gate.isSettled(now: t), "cycle \(cycle): should be unsettled at re-init") + XCTAssertFalse( + gate.isSettled(now: t.addingTimeInterval(cyclePeriod - 1)), + "cycle \(cycle): should still be unsettled 9s after re-init (< 12s window)" + ) + } + } + + // MARK: - Recovery after the loop ends (AC-2) + + /// Once the loop ends and the connection stays stable past the settle + /// interval, the gate reports settled again so legitimate notifications + /// resume. + func testSettlesOnceLoopEnds() { + var gate = ConnectionSettleGate(settleInterval: 12) + let start = Date() + // Three unstable cycles... + for cycle in 0..<3 { + gate.markUnsettled(now: start.addingTimeInterval(10 * Double(cycle))) + } + let lastInstability = start.addingTimeInterval(20) + // 11s after the last re-init — not yet settled. + XCTAssertFalse(gate.isSettled(now: lastInstability.addingTimeInterval(11))) + // 13s after the last re-init — settled, real events allowed again. + XCTAssertTrue(gate.isSettled(now: lastInstability.addingTimeInterval(13))) + } +}