Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion native/macos/MCPProxy/MCPProxy/Core/CoreProcessManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
72 changes: 72 additions & 0 deletions native/macos/MCPProxy/MCPProxy/Services/NotificationService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()

Expand All @@ -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 }

Expand All @@ -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 }

Expand Down
Original file line number Diff line number Diff line change
@@ -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)))
}
}
Loading