Skip to content

feat(security): US4 Collapse the scan-notification storm into one debounced settled event (MCP-2207) (Spec 077) - #787

Closed
Dumbris wants to merge 2 commits into
077-us1-baselinefrom
077-us4-notify-collapse
Closed

feat(security): US4 Collapse the scan-notification storm into one debounced settled event (MCP-2207) (Spec 077)#787
Dumbris wants to merge 2 commits into
077-us1-baselinefrom
077-us4-notify-collapse

Conversation

@Dumbris

@Dumbris Dumbris commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

Spec 077 US4 (MCP-2207). MCPProxy's security-scan notification storm came from per-scanner security.scan_started/progress/completed/failed SSE events multiplied by reconnect storms (prior partial fixes: #659, MCP-2223). This PR replaces those per-scanner lifecycle emissions with a single debounced security.scan_settled event per server per scan, satisfying FR-015 / SC-006.

Base: 077-us1-baseline (stacked on US1 #786), not main.

Per-task changes

T034 (test-first)internal/runtime/scan_notify_test.go

  • A reconnect storm across N servers (each firing the full per-scanner lifecycle several times) yields <= N settled events — exactly one per server — and zero per-scanner lifecycle events. Confirmed red before implementation, green after.
  • A second test asserts the settled event carries the terminal findings summary and server identity.

T035internal/runtime/

  • New scanNotifyDebouncer (scan_notify.go): terminal-triggered, per-server debounce. Only completed/failed arm the per-server timer; noisy started/progress are dropped. A per-server generation counter guards the classic AfterFunc race (a superseded timer becomes a no-op), so each server settles exactly once.
  • New EventTypeSecurityScanSettled. Runtime EmitSecurityScan* now route through the debouncer: started/progress become no-ops, completed/failed record terminal state; publishScanSettled emits one event carrying the terminal findings_summary.
  • Wired scanNotify (750 ms window) into newRuntime next to the existing servers-changed coalescer.
  • Activity log collapsed to one handleSecurityScanSettled record per scan; the former started/completed/failed handlers were removed.

T036frontend/src/

  • stores/system.ts: forward the security.scan_settled SSE event as a mcpproxy:scan-settled window event.
  • composables/useSecurityScannerStatus.ts: refresh cached scan totals off the single settled signal instead of per-scanner lifecycle events.

The /events SSE endpoint forwards event types generically, so the new type flows to clients automatically; no whitelist change needed.

Verification (actual output)

  • go build ./... — clean (exit 0).
  • go test ./... (full suite, with MCPPROXY_BINARY_PATH set to a freshly built mcpproxy) — 0 failures.
  • go run ./cmd/scan-eval --corpus specs/065-evaluation-foundation/datasets/detect_corpus_v1.json --gate --min-recall 0.90 --max-fp 0.05GATE PASSED: recall=1.0000 (>=0.9000), fp=0.0000 (<=0.0500).
  • golangci-lint run --config .github/.golangci.yml ./...0 issues.
  • Frontend: vue-tsc --noEmit clean; vite build succeeds.

🤖 Generated with Claude Code

Dumbris added 2 commits July 1, 2026 10:19
…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)
…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)
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying mcpproxy-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 19c49be
Status: ✅  Deploy successful!
Preview URL: https://bbd00d01.mcpproxy-docs.pages.dev
Branch Preview URL: https://077-us4-notify-collapse.mcpproxy-docs.pages.dev

View logs

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 63.07692% with 24 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/runtime/activity_service.go 0.00% 11 Missing ⚠️
internal/runtime/event_bus.go 55.55% 7 Missing and 1 partial ⚠️
internal/runtime/scan_notify.go 85.71% 3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

📦 Build Artifacts

Workflow Run: View Run
Branch: 077-us4-notify-collapse

Available Artifacts

  • archive-darwin-amd64 (28 MB)
  • archive-darwin-arm64 (25 MB)
  • archive-linux-amd64 (16 MB)
  • archive-linux-arm64 (14 MB)
  • archive-windows-amd64 (28 MB)
  • archive-windows-arm64 (25 MB)
  • frontend-dist-pr (0 MB)
  • installer-dmg-darwin-amd64 (21 MB)
  • installer-dmg-darwin-arm64 (19 MB)

How to Download

Option 1: GitHub Web UI (easiest)

  1. Go to the workflow run page linked above
  2. Scroll to the bottom "Artifacts" section
  3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 28500611147 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

@Dumbris

Dumbris commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

KimiReviewer fallback review — ACCEPT (MCP-3818)

Reviewed at head 19c49becce4f71920d879aafe21b01c095e0ad2f.

Summary: Spec 077 US4 — collapses the per-scanner scan-notification storm into a single debounced security.scan_settled event per server per scan.

What I verified:

  • scanNotifyDebouncer: terminal-triggered, generation-guarded against AfterFunc race, last-write-wins for status/summary/error — correct design.
  • Storm signals (EmitSecurityScanStarted/EmitSecurityScanProgress) correctly no-op.
  • EmitSecurityScanCompleted/EmitSecurityScanFailed route through debouncer; safe direct-publish fallback for test environments without scanNotify wired.
  • ActivityService: handler correctly maps completed→success / failed→error.
  • Frontend: SSE listener forwards security.scan_settled as mcpproxy:scan-settled; composable registers once at module scope.
  • Both unit tests pass (TestScanNotify_ReconnectStormCollapses, TestScanNotify_SettledCarriesTerminalSummary). Build clean.

Verdict: ACCEPT — no issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants