Skip to content

Commit 728d82a

Browse files
committed
feat(048): eliminate remaining tray /api/v1/servers refetches
Spec 047 (PR #450, v0.29.4) eliminated the burst-storm refetch path (servers.changed -> refetch). This PR addresses the residual: 5 other call sites in the Swift tray that still hit /api/v1/servers, totaling ~8 GETs per 60 s at idle on a 30-server config. All 5 sites now read SSE-driven appState.servers in place of fetching: - CoreProcessManager case "status": no longer refetches on connected_count change. Inline stats are merged unconditionally; per-server state arrives via the spec 047 servers.changed payload within ~50 ms of any actual transition. - refreshState (30 s periodic) drops the refreshServers() call. refreshActivity / refreshSessions / refreshTokenMetrics / refreshSecurityStatus stay (SSE doesn't cover those domains). - refreshSecurityStatus's Docker fallback now reads appState.servers synchronously instead of issuing another GET. - MCPProxyApp's 10 s server-refresh Timer is removed entirely. - menuWillOpen no longer refetches on every tray click. The menu rebuilds from in-memory state only. In their place: a single 5-minute Combine timer in MCPProxyApp.swift calls a new CoreProcessManager.refreshServersForSafetyNet() — pure defense-in-depth in case SSE drops events past the 256-event buffer. Verification (specs/048-tray-refetch-elimination/verification/report.md) on the same MCPProxy.app + 30-server scenario as spec 047: - 0 GETs / 60 s at idle (was ~8) - context7 disable propagates to tray menu in 1 s - context7 enable propagates in 2 s - 0 GETs during the entire toggle window Spec, plan, research, data-model, quickstart, tasks all under specs/048-tray-refetch-elimination/. No core / Go changes. Backward-compatible with older cores (spec 047 fallback path retained). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent eae45ef commit 728d82a

10 files changed

Lines changed: 651 additions & 37 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,8 @@ See `docs/prerelease-builds.md` for download instructions.
768768
- Paperclip's embedded Postgres (existing, port 54329); Synapbus DB (existing); no new storage in mcpproxy-go (045-paperclip-cockpit)
769769
- Go 1.24 (toolchain go1.24.10); Swift 5.9 (macOS 13+); TypeScript 5.9 / Vue 3.5 (frontend) + `go.etcd.io/bbolt` (existing), `go.uber.org/zap` (existing), `github.com/mark3labs/mcp-go` (existing). No new deps. (047-cpu-hotpath-fix)
770770
- BBolt (`~/.mcpproxy/config.db`) — read-only on the hot path; no schema change. (047-cpu-hotpath-fix)
771+
- Swift 5.9 (macOS 13+); Go 1.24 only for the verification harness, no Go changes in scope. + SwiftUI/AppKit (existing), Combine (existing for the periodic timer pattern). No new deps. (048-tray-refetch-elimination)
772+
- None. Pure in-memory state. (048-tray-refetch-elimination)
771773
772774
## Recent Changes
773775
- 001-update-version-display: Added Go 1.24 (toolchain go1.24.10)

native/macos/MCPProxy/MCPProxy/Core/CoreProcessManager.swift

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -500,24 +500,19 @@ actor CoreProcessManager {
500500
private func handleSSEEvent(_ event: SSEEvent) async {
501501
switch event.event {
502502
case "status":
503-
// Status events contain inline stats.
504-
// When connected count changes, re-fetch the full server list
505-
// to get accurate counts (SSE stats can lag behind actual state).
503+
// Status events contain inline stats. Spec 048: any change that
504+
// would also flip connected_count emits a `servers.changed`
505+
// event (delivered within ~50 ms by spec 047's coalescer) which
506+
// already updates the per-server appState. Stat aggregates are
507+
// updated unconditionally from the inline payload — no refetch.
506508
if let data = event.data.data(using: .utf8),
507509
let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
508510
let stats = json["upstream_stats"] as? [String: Any] {
509-
let connected = stats["connected_servers"] as? Int ?? 0
510511
let total = stats["total_servers"] as? Int ?? 0
511512
let tools = stats["total_tools"] as? Int ?? 0
512-
let oldConnected = await MainActor.run { appState.connectedCount }
513-
// If counts changed, do a full server refresh for accuracy
514-
if connected != oldConnected {
515-
await refreshServers()
516-
} else {
517-
await MainActor.run {
518-
if appState.totalServers != total { appState.totalServers = total }
519-
if appState.totalTools != tools { appState.totalTools = tools }
520-
}
513+
await MainActor.run {
514+
if appState.totalServers != total { appState.totalServers = total }
515+
if appState.totalTools != tools { appState.totalTools = tools }
521516
}
522517
}
523518

@@ -613,8 +608,10 @@ actor CoreProcessManager {
613608
}
614609

615610
/// Fetch full state from the core and update appState.
611+
/// Spec 048: dropped the per-tick refreshServers() call. The server list
612+
/// is now SSE-driven (spec 047 servers.changed payload). MCPProxyApp
613+
/// installs a separate 5-min safety-net timer for missed-event recovery.
616614
private func refreshState() async {
617-
await refreshServers()
618615
await refreshActivity()
619616
await refreshSessions()
620617
await refreshTokenMetrics()
@@ -635,10 +632,12 @@ actor CoreProcessManager {
635632
// if docker_isolation.enabled is true in the running config, treat as available.
636633
let configEnabled = await MainActor.run { appState.totalServers > 0 }
637634
if configEnabled {
638-
// Servers are connected — Docker must be working if isolation is enabled
639-
// Check via the status endpoint which shows connected servers in containers
640-
let servers = try? await apiClient.servers()
641-
let hasStdioServers = servers?.contains(where: { $0.connected && $0.protocol == "stdio" }) ?? false
635+
// Servers are connected — Docker must be working if isolation is enabled.
636+
// Spec 048: appState.servers is SSE-fed, so read it directly instead
637+
// of issuing another GET /api/v1/servers on every periodic refresh.
638+
let hasStdioServers = await MainActor.run {
639+
appState.servers.contains(where: { $0.connected && $0.protocol == "stdio" })
640+
}
642641
await MainActor.run {
643642
appState.dockerAvailable = hasStdioServers || dockerOK
644643
}
@@ -678,6 +677,14 @@ actor CoreProcessManager {
678677
}
679678
}
680679

680+
/// Spec 048: long-cadence safety-net wrapper around `refreshServers`.
681+
/// Called by a 5-minute Combine timer in `MCPProxyApp` to guard against
682+
/// missed `servers.changed` SSE events. Separate name documents intent —
683+
/// this is *not* the on-demand refresh path (that's been retired).
684+
func refreshServersForSafetyNet() async {
685+
await refreshServers()
686+
}
687+
681688
/// Fetch recent activity and update appState.
682689
private func refreshActivity() async {
683690
guard let apiClient else { return }

native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,15 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS
9797
}
9898
.store(in: &cancellables)
9999

100-
// Periodic server refresh every 10s to keep health/action data current
101-
Timer.publish(every: 10, on: .main, in: .common)
100+
// Spec 048: dropped the 10 s server-refresh timer. The server list is
101+
// now SSE-driven via the spec 047 `servers.changed` payload — appState
102+
// updates within ~50 ms of any state transition with zero round trip.
103+
// The safety-net below covers the rare case where SSE drops events.
104+
Timer.publish(every: 300, on: .main, in: .common) // 5 minutes
102105
.autoconnect()
103106
.sink { [weak self] _ in
104-
guard let self, let client = self.appState.apiClient else { return }
105-
Task {
106-
if let servers = try? await client.servers() {
107-
await self.appState.updateServers(servers)
108-
}
109-
}
107+
guard let self, let core = self.coreManager else { return }
108+
Task { await core.refreshServersForSafetyNet() }
110109
}
111110
.store(in: &cancellables)
112111

@@ -166,17 +165,9 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS
166165
// MARK: - NSMenuDelegate
167166

168167
func menuWillOpen(_ menu: NSMenu) {
169-
// Fetch fresh server data before building the menu
170-
// This ensures health.action (login/restart) is current
171-
if let client = appState.apiClient {
172-
Task {
173-
if let servers = try? await client.servers() {
174-
await appState.updateServers(servers)
175-
await MainActor.run { rebuildMenu() }
176-
}
177-
}
178-
}
179-
// Build with current data immediately (async fetch updates it shortly after)
168+
// Spec 048: dropped the per-click `client.servers()` fetch. appState
169+
// is fed by SSE (spec 047), so it's already current within ~50 ms of
170+
// the last upstream state change. Rebuild from in-memory state only.
180171
rebuildMenu()
181172
}
182173

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# Data Model — Tray Refetch Elimination
2+
3+
No persistent storage changes. No new `appState` fields. No new types. Pure refactor of existing methods.
4+
5+
## Existing types touched
6+
7+
| Type | File | Change |
8+
|---|---|---|
9+
| `CoreProcessManager` | `Core/CoreProcessManager.swift` | Modify `handleSSEEvent` (case `"status"`), `refreshState`, `refreshSecurityStatus`. Add nothing. |
10+
| `AppDelegate` (or whatever holds the Combine cancellables in `MCPProxyApp.swift`) | `MCPProxyApp.swift` | Drop the 10 s server-refresh timer. Drop the `menuWillOpen` server fetch. Add a 5 min safety-net timer. |
11+
| `AppState` | `State/AppState.swift` | No change. `appState.servers` is already the canonical state and is fed by SSE. |
12+
13+
## New: safety-net timer
14+
15+
Lives on the same `cancellables: Set<AnyCancellable>` set already used by the app-level coordinator (`MCPProxyApp.swift`):
16+
17+
```swift
18+
// MCPProxyApp.swift — replaces the existing 10 s timer block
19+
//
20+
// Spec 048: SSE-driven appState.servers is authoritative. We keep one
21+
// long-cadence safety-net refresh in case SSE drops events past the
22+
// 256-event buffer or the runtime momentarily forgets to emit.
23+
Timer.publish(every: 300, on: .main, in: .common) // 5 minutes
24+
.autoconnect()
25+
.sink { [weak self] _ in
26+
guard let self, let core = self.coreManager else { return }
27+
Task { await core.refreshServersForSafetyNet() }
28+
}
29+
.store(in: &cancellables)
30+
```
31+
32+
`refreshServersForSafetyNet()` is a tiny new method on `CoreProcessManager` that does exactly what the existing private `refreshServers()` does — separate name purely so future readers don't confuse the safety-net path with the on-demand path. Internally it just calls `refreshServers()`.
33+
34+
## Removed lifecycle hooks
35+
36+
- `MCPProxyApp.swift:101-110` — the 10 s `Timer.publish` that called `client.servers()`. Replaced by the safety-net above.
37+
- `MCPProxyApp.swift:172-178` — the inline `client.servers()` fetch inside `menuWillOpen`. The function reduces to `rebuildMenu()`.
38+
39+
## Modified lifecycle hooks
40+
41+
- `CoreProcessManager.swift:514-515``case "status":` no longer calls `refreshServers()`. The branch becomes a stat-update only.
42+
- `CoreProcessManager.swift:617``refreshState()` drops the `await refreshServers()` call. The other periodic refreshes stay.
43+
- `CoreProcessManager.swift:640``refreshSecurityStatus()` reads `appState.servers` instead of fetching.
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# Implementation Plan: Eliminate Remaining `/api/v1/servers` Refetches in macOS Tray
2+
3+
**Branch**: `048-tray-refetch-elimination` | **Date**: 2026-05-08 | **Spec**: [`spec.md`](./spec.md)
4+
**Input**: [`specs/048-tray-refetch-elimination/spec.md`](./spec.md)
5+
6+
## Summary
7+
8+
Remove the 5 remaining `/api/v1/servers` refetch sites in the macOS tray. SSE-driven `appState.servers` (delivered by spec 047) is already authoritative; replace each refetch with an in-memory read or drop it entirely. Add one long-cadence (5 min) safety-net timer for missed-event resilience. Net result: tray-driven `/api/v1/servers` GETs at idle drop from ~8 / 60 s to ≤ 1 / 60 s.
9+
10+
## Technical Context
11+
12+
**Language/Version**: Swift 5.9 (macOS 13+); Go 1.24 only for the verification harness, no Go changes in scope.
13+
**Primary Dependencies**: SwiftUI/AppKit (existing), Combine (existing for the periodic timer pattern). No new deps.
14+
**Storage**: None. Pure in-memory state.
15+
**Testing**: XCTest (Swift). Live reproduction harness from spec 047 (`/Applications/MCPProxy.app` swap-in).
16+
**Target Platform**: macOS 13+ (Personal edition).
17+
**Project Type**: Native macOS tray app subtree of the multi-target repo.
18+
**Performance Goals**: ≤ 1 `/api/v1/servers` GET per 60 s wall at idle (down from ~8). UI reactivity unchanged (≤ 50 ms from SSE event to visible update).
19+
**Constraints**: No core / Go changes. No SSE contract change. No user-visible behavior change. Must remain backward-compatible with older cores (notify-only `servers.changed` fallback already handled by spec 047).
20+
**Scale/Scope**: 5 call sites across 2 Swift files (`CoreProcessManager.swift`, `MCPProxyApp.swift`). One new Combine timer.
21+
22+
## Constitution Check
23+
24+
| Principle | Status | Notes |
25+
|---|---|---|
26+
| **I. Performance at Scale** | Reinforced | Drops residual idle CPU drag from the macOS client side; complements spec 047 server-side wins. |
27+
| **II. Actor-Based Concurrency** | Aligned | All refactors stay within existing `MainActor` / `Task` patterns. No new shared mutable state. |
28+
| **III. Configuration-Driven Architecture** | Aligned | The 5-minute safety-net cadence can be hard-coded; if user feedback ever asks for tuning, surface as a config key in a follow-up. |
29+
| **IV. Security by Default** | No regression | No auth surface change. No new permissions. No data crossing trust boundaries. |
30+
| **V. TDD** | Required | One failing XCTest per site change; see `tasks.md`. |
31+
| **VI. Documentation Hygiene** | Aligned | Spec, plan, tasks, research, quickstart, verification all committed under `specs/048-tray-refetch-elimination/`. |
32+
33+
No violations. Complexity Tracking is empty.
34+
35+
## Project Structure
36+
37+
### Documentation (this feature)
38+
39+
```text
40+
specs/048-tray-refetch-elimination/
41+
├── spec.md
42+
├── plan.md ← this file
43+
├── research.md ← Phase 0
44+
├── data-model.md ← Phase 1
45+
├── quickstart.md ← Phase 1
46+
├── tasks.md ← Phase 2 (speckit.tasks)
47+
└── verification/
48+
├── http_log_idle.txt ← /api/v1/servers GETs over 60 s idle
49+
└── report.md
50+
```
51+
52+
(No `contracts/` directory — this is a client-only refactor with no API change.)
53+
54+
### Source Code
55+
56+
```text
57+
native/macos/MCPProxy/MCPProxy/
58+
├── Core/CoreProcessManager.swift ← sites 1, 2, 3 + the safety-net hook
59+
└── MCPProxyApp.swift ← sites 4, 5
60+
61+
native/macos/MCPProxy/MCPProxyTests/
62+
└── SSEHandlerTests.swift ← extend with 6 new tests (one per site + safety-net)
63+
64+
specs/048-tray-refetch-elimination/
65+
└── verification/ ← post-fix http.log GET counts
66+
```
67+
68+
**Structure Decision**: Pure Swift refactor; the file layout above is the entirety of the change set.
69+
70+
## Phase 0: Research (`research.md`)
71+
72+
All decisions resolved during spec drafting on 2026-05-08. Key calls captured in `research.md`:
73+
- 5-minute safety-net interval (chosen over 1 / 10 / 30 minutes).
74+
- Approach for `refreshSecurityStatus` Docker fallback (read `appState.servers` synchronously).
75+
- `menuWillOpen` strategy (drop refetch entirely; rely on appState).
76+
- How to handle "tray just opened, appState empty" race (existing initial fetch on `connect` covers it).
77+
78+
## Phase 1: Design & Contracts
79+
80+
- **`data-model.md`** — declares only the new safety-net timer reference held on the app-level coordinator. No persistent storage.
81+
- **No `contracts/` directory** — this PR doesn't change any API.
82+
- **`quickstart.md`** — exact reproduction recipe for the live verification (build tray, swap into app bundle, launch, watch `http.log`).
83+
- **Agent context update** — runs at the end via `.specify/scripts/bash/update-agent-context.sh claude`.
84+
85+
## Phase 2: Tasks (`tasks.md`)
86+
87+
Generated by `/speckit.tasks` from this plan. Each site change is preceded by a failing XCTest.
88+
89+
## Risks (mirrored from spec)
90+
91+
See [`spec.md`](./spec.md) "Risks & Mitigations". No new risks identified during planning.
92+
93+
## Out of Scope
94+
95+
- Replacing `refreshActivity` / `refreshTokenMetrics` / `refreshSessions` periodics (separate spec; needs SSE design for those domains).
96+
- Investigating whether `refreshSecurityStatus`'s Docker check could itself become SSE-driven (separate spec).
97+
- Adding a config key for the safety-net cadence (surface only if user feedback asks).
98+
- Web UI changes (already covered by spec 047).
99+
100+
## Complexity Tracking
101+
102+
(empty — no Constitution gate violations)
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Quickstart — Tray Refetch Elimination
2+
3+
## Build
4+
5+
```bash
6+
cd native/macos/MCPProxy
7+
SDK=$(xcrun --sdk macosx --show-sdk-path)
8+
swiftc -target arm64-apple-macosx13.0 -sdk "$SDK" -module-name MCPProxy -emit-executable -O \
9+
-o /tmp/MCPProxy-048 \
10+
$(find MCPProxy -name "*.swift" -not -path "*/Tests/*" | sort | tr '\n' ' ')
11+
```
12+
13+
## Run XCTests
14+
15+
```bash
16+
# From the MCPProxy package root
17+
swift test --target MCPProxyTests --filter "SSEHandler|SafetyNet"
18+
```
19+
20+
## Live verification — count `/api/v1/servers` GETs at idle
21+
22+
Reproduces the spec 047 harness with the swap-in bundle. Replace the tray binary inside a clone of `/Applications/MCPProxy.app`, launch, watch `http.log`.
23+
24+
```bash
25+
pkill -f "MCPProxy-048" 2>/dev/null
26+
rm -rf /tmp/MCPProxy-048.app
27+
cp -R /Applications/MCPProxy.app /tmp/MCPProxy-048.app
28+
cp /tmp/MCPProxy-048 /tmp/MCPProxy-048.app/Contents/MacOS/MCPProxy
29+
codesign --force --deep --sign - /tmp/MCPProxy-048.app
30+
open /tmp/MCPProxy-048.app
31+
32+
# Wait for steady state
33+
until curl -sf -H "X-API-Key: $(jq -r .api_key ~/.mcpproxy/mcp_config.json)" http://127.0.0.1:8080/api/v1/status >/dev/null; do sleep 1; done
34+
sleep 30 # let the tray finish startup fetches
35+
36+
# Sample 60 s of idle traffic
37+
START=$(date +%s)
38+
NOW=$START
39+
while [ $((NOW-START)) -lt 60 ]; do sleep 5; NOW=$(date +%s); done
40+
41+
LOG=~/Library/Logs/mcpproxy/http.log
42+
COUNT=$(grep '"path": "/api/v1/servers"' "$LOG" | awk -F'|' '{print $1}' | awk '{print $1}' | awk -v START=$(date -v-60S +%FT%T) '$0 >= START' | wc -l | tr -d ' ')
43+
echo "GET /api/v1/servers in last 60 s of idle: $COUNT"
44+
```
45+
46+
**Acceptance**: `$COUNT` should be `0` or `1` (the latter only if the test happened to span a 5-minute safety-net boundary). Before this PR: ~8.
47+
48+
## Live verification — UI reactivity unchanged
49+
50+
Use the same `mcpproxy-ui-test` MCP harness from spec 047:
51+
52+
```bash
53+
BUNDLE=com.smartmcpproxy.mcpproxy
54+
K=$(jq -r .api_key ~/.mcpproxy/mcp_config.json)
55+
56+
# Toggle a server, observe the tray reacting via SSE
57+
curl -H "X-API-Key: $K" -X POST http://127.0.0.1:8080/api/v1/servers/context7/disable
58+
sleep 2
59+
echo '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"list_menu_items","arguments":{"path":["Servers (30)"]}}}' \
60+
| /tmp/mcpproxy-ui-test --bundle-id "$BUNDLE" 2>/dev/null \
61+
| grep '^{' | head -1 \
62+
| jq -r '.result.content[0].text' \
63+
| jq '.items[] | select(.title | test("context7"; "i")) | .children[0:3]'
64+
```
65+
66+
Expected: `Disabled / Enable` within 2 s. Re-enable → `Connected (2 tools) / Disable` within 5 s.
67+
68+
Same as spec 047, but the verification now also asserts that no `/api/v1/servers` GET appeared in `http.log` during the toggle (only the SSE-driven update path is exercised).

0 commit comments

Comments
 (0)