feat(tray): glance section — recent calls, connected clients, 24h histogram - #930
Merged
Conversation
…acOS menu Design for a compact glance section at the top of the macOS tray menu: five most recent tool calls, active MCP clients, and a calls-per-hour histogram for the last 24h behind a submenu. Key decisions and their reasons: - NSMenu with SwiftUI views in NSMenuItem.view (CodexBar's approach), not a popover — a popover penalises the common quick-status case. - Histogram plots calls per hour, not tokens: no hourly token series exists (usage buckets carry calls/errors/resp_bytes; token_source is "bytes"), and real tokenisation only accumulates per session. - Fixes two feed bugs in scope: CoreProcessManager matches a literal 'activity' SSE event the core never emits (activity is 30s-polled today), and APIClient decodes last_active where the API emits last_activity. - Glance items and their hosting views are cached and re-inserted on rebuild, since rebuildMenu() calls removeAllItems() without dismissing the menu; extracted into Menu/Glance/ rather than growing the 1120-line MCPProxyApp.swift. Menu open stays network-free (spec 048 invariant); the histogram fetch happens on submenu open only.
Codex reviewed the design against the actual code and found nine issues; all were verified against the repo before acting on them. Substantive changes: - Rows are plain NSMenuItems, not hosted SwiftUI views. Apple documents that custom menu-item views get mouse but not keyboard events, so SwiftUI rows would not be keyboard-selectable and would need hand-rolled accessibility. Only the histogram stays a hosted view, in its submenu. This also removes the view-caching scheme, which was unnecessary for plain items and would not have prevented churn anyway. - Activity selection now keeps failed call_tool_* wrapper records. The backend intentionally retains them because a failed wrapper has no corresponding upstream tool_call record; the previous built-in allowlist would have discarded the only trace of a failed call. Page size raised 10 -> 25 so noise cannot starve the five rows. - Header count now derives from /activity/usage (in-memory snapshot), not /activity/summary, which sets Limit=0 and scans every record in the window and counts all activity types, not just calls. Same fetch feeds the histogram, so the submenu needs no fetch of its own. - SSE fix consumes activity.* event payloads directly instead of calling refreshActivity(), which would have fired a REST GET per event. - Sessions page size raised to 25 before the active filter, since closed sessions can crowd out older still-active ones. - Deep links use ?session=, the query the Activity view actually reads; request_id would have required a Web UI change, an explicit non-goal. - Chart stacks calls-errors plus errors (calls already includes errors) and synthesises zero buckets for missing hours. - Added an honest scope inventory and the protocol extraction the zero-network test requires.
Six more findings, all verified against the code first: - Do not narrow AppState.recentActivity: the native Dashboard renders the full activity log from it on purpose (security scans, quarantine, OAuth) so a quiet proxy still shows something (DashboardView.swift:572). The glance section gets its own filtered feed instead. - Header now reads 'calls this hour' and selects the bucket matching the current UTC hour, showing zero when absent. Buckets are UTC-hour aligned and sparse (usage_aggregate.go:229), so 'the most recent bucket' could have displayed an hours-old count as current. - Selection rules were self-contradictory (include all failed internal calls vs exclude management built-ins). Precedence is now explicit: management names are excluded first, then tool_call, then discovery built-ins or non-success internal calls. - Sessions page raised to the API maximum: storage truncates newest-first by START time before the runtime sorts by last activity (manager.go:1355), so a long-lived active session falls outside any small page. Server-side status filter recorded as a follow-up. - Deep links go through an app-delegate action, not a URL built inside the component: a first-time browser session needs the API key that openWebUI() fetches from the core manager (MCPProxyApp.swift:985). - Usage state fields are optional so 'not loaded' stays distinguishable from 'loaded and idle' — a valid usage response can be empty.
No P1s this round; three P2s and two P3s, all verified against the code: - rebuildMenu() gains a tracking guard. Live SSE rows make the debounced objectWillChange -> rebuildMenu() sink fire during active traffic, and rebuildMenu() calls removeAllItems() without dismissing the menu — so a busy agent could restructure the menu under the user's cursor and disturb an open histogram submenu. While tracking, rows update titles in place and structural changes are deferred to menuDidClose. - Glance state is explicitly cleared on disconnect. Optional fields only make a cleared state expressible; without an actual reset the menu can show the previous core's numbers, since the connection path flips to .connected before the first refresh completes. - Glance page size 25 -> 50. Management built-ins match the server-side type filter and are excluded client-side, and storage filters before truncating, so a burst of upstream_servers calls could fill a small page and starve real tool calls. - Mockup label corrected to 'calls this hour', matching the load-bearing bucket semantics. - SSE payload field is error_message, not error; reading the wrong key would drop failure text from optimistic rows until reconciliation.
No P1s; three P2s and one P3, all verified against the code: - Specify the SSE payload adapter. The wire format is an envelope with a Unix-seconds timestamp, and internal events send internal_tool_name plus optional target_server rather than tool_name/server_name (event_bus.go:552). Feeding raw payloads into the selection rules would have silently dropped every internal row. - Fix the sessions gap properly instead of papering over it. Storage truncates newest-by-start-time before the sort, and the handler parses offset without using it, so no page size guarantees active clients are found; a 100-record page was still a page. Adds a status filter to GET /api/v1/sessions applied during the cursor walk — a small Go change now in scope, because the alternative is a section that reports 'no clients' while a client is working. - In-place row updates must rewrite the whole row identity — title, image, tooltip, accessibility label, and the representedObject that carries the session for the click action. Updating only the title would leave a row whose click opens the previous record's session. - Selection test now pins the 50-record page the production feed uses.
No P1s; three P2s, all verified against the code: - Add a deduplication rule. A failed upstream call emits BOTH activity.tool_call.completed and activity.internal_tool_call.completed under one request id and both are persisted (mcp.go:2142,2146), so the previous rules would render one failure as two rows. Records sharing a request id now collapse to the tool_call one, which names the real server:tool; wrappers that failed before dispatch have no paired record and still render. - Provisional SSE ids are request_id + event type. ActivityEntry derives identity from id alone (Models.swift:536) and the paired events share a request id, so a bare request_id would collide two distinct records before the dedup rule could choose between them. - Give the glance section its own glanceSessions feed. recentSessions is shared: ActivityView resolves session ids to client names through it and DashboardView falls back to it for Recent Sessions, so pointing that feed at status=active would silently narrow both. Same mistake this design already avoided for recentActivity — now stated as a rule: a shared AppState feed the glance wants differently gets its own field, never a narrowed shared one.
…lders) Produced by a 21-agent workflow: four recon agents mapped the exact Swift and Go surfaces, eight drafting agents wrote one task each with real code, and each task was then adversarially verified against the repo by its own agent (all eight came back corrected, not clean). Tasks: (1) Go status filter on GET /api/v1/sessions applied during the cursor walk; (2) Swift API client methods, usage models, last_activity coding-key fix, glance data-source protocol; (3) AppState glance fields, refresh wiring, disconnect reset; (4) SSE payload adapter replacing the dead activity case; (5) GlanceSelection + GlanceFormatting pure logic; (6) GlanceSection plain NSMenuItems with whole-identity in-place updates; (7) MCPProxyApp integration, open-menu rebuild guard, session deep link, TrayMenu.swift deletion; (8) SwiftUI Charts histogram in the submenu. 156 checkbox steps, each with real code or a real command and expected output. Self-review passed: no placeholders, no Claude attribution in the commit templates, symbol names consistent across tasks, and every load-bearing spec detail present (request_id collapse, current-UTC-hour bucket, calls-includes-errors stacking, representedObject rewrites, shared feeds never narrowed).
Storage walked the sessions bucket newest-first by START time and truncated to limit before anyone could filter, so a session that began hours ago but is still active fell outside every page once enough newer sessions existed. The macOS tray's client glance would then show "No connected clients" while a client was actively calling tools, and offset cannot recover it (the handler parses offset but never passes it down). Push the filter into the cursor walk so it runs before truncation: - storage.Manager.GetRecentSessions(limit, status) filters on SessionRecord .Status during the walk. When filtering, it walks the whole bucket (capped at 100 keys by session retention) so `total` counts matching records. - runtime.Runtime / server.Server / httpapi.ServerController thread status through unchanged. - The handler validates status against the closed domain (active|closed) and returns 400 on anything else rather than silently returning unfiltered sessions the caller believes are filtered. Regenerates the OpenAPI artifacts and documents the parameter.
The status-filter suite never made `limit` binding on the filtered path: limit=2 had one match and limit=10 had four, so deleting the `if len(sessions) < limit` guard in GetRecentSessions still passed the whole suite. The filtered walk could have silently ignored limit. Add GetRecentSessions(2, "closed") over four closed sessions, asserting the page truncates to 2 while total still reports 4, and pinning the two returned IDs so the newest-first order of the filtered walk is covered too. Verified by removing the guard: only this sub-test fails.
GET /api/v1/activity/usage?window=24h&top=1 feeds the tray glance header and 24h histogram. UsageBucket parses the RFC 3339 bucket start itself so the shared fetchWrapped decoder needs no date strategy. Adds the first APIClient unit tests, via a URLProtocol stub on the existing init(session:) seam.
GET /api/v1/activity?type=tool_call,internal_tool_call&limit=50 is a separate feed from recentActivity(), which the native Dashboard uses to render the full log. The 50-record page is oversized because management built-ins are filtered client-side.
MCPSession declared last_active, but the API emits last_activity, so the field was nil for every session — DashboardView was sorting and labelling Recent Sessions off an always-empty string. Adds activeSessions(limit:), which asks the server for status=active rather than filtering a truncated page client-side.
The glance section depends on a three-method protocol instead of the concrete APIClient actor, so a counting stub can be injected and the spec-048 invariant (menu open issues zero requests) becomes assertable. APIClient's conformance is declaration-only — an actor's isolated methods already satisfy async protocol requirements.
The stub's statusCode knob existed but nothing drove it, leaving the error path of all three new calls unasserted. Pins that a non-2xx surfaces as APIClientError.httpError keyed off the status code — the 400 case is what the live core returns for ?status=bogus, and its error body carries a request_id that must not disturb message extraction. Verified by mutation: neutralising the 200...299 guard in performRequest fails all three, the 503 case by degrading to a decodingError.
When the body is enveloped and a model's decoder throws, the bare-decode fallback re-fails on the top-level shape and that second error was the one reported — so UsageBucket's "Not an RFC 3339 timestamp" surfaced as "no value associated with key window", naming the wrong cause. Both decode errors are now kept and the informative one is chosen by probing the body for the envelope. Unwrapped endpoints keep reporting the bare error exactly as before; success paths are untouched.
Adds glanceActivity/glanceSessions/usageTimeline/callsThisHour alongside the shared recentActivity/recentSessions feeds, which stay broad because the native Dashboard renders the full activity log and ActivityView resolves session ids through recentSessions. callsThisHour matches the bucket whose start equals the current UTC hour and reports 0 when that hour is absent; usage buckets are sparse, so taking the newest bucket would present an hours-old count as live.
The connect path transitions to .connected before the first refresh completes, so hiding the glance block is not enough — without a reset the menu would briefly render the previous core's activity, clients and call count as live. The reset hangs off coreState's didSet rather than transition(to:) because CoreProcessManager.awaitExternalCore and MCPProxyApp.stopCore assign coreState directly and would otherwise bypass it. updateGlanceActivity fingerprints id+status+hasSensitiveData rather than id alone: ActivityEntry's Equatable is id-only, so an id guard would discard the reconciling poll's late status and sensitive-data corrections.
…he refresh loop Three additional fetches on the existing 30s CoreProcessManager cycle, each failing soft like the surrounding refreshers. No new timer and no menu-open network call: the tray menu still renders entirely from AppState (spec 048).
clearGlanceState() alone left a window: a fetch already past its `guard let apiClient` when the core leaves .connected resolves after the reset and writes the dead core's data back over the cleared fields. shutdown() transitions to .shuttingDown before it cancels refreshTask, and cancelling would not close the window anyway — a suspended fetch still resumes and still runs the update that follows it. These are the one field set whose stated purpose is never showing a previous core's numbers as live, so the three updaters now drop writes that arrive while disconnected.
Status symbol, server:tool row label, middle truncation and a compact locale-free relative age, salvaged from the dead Menu/TrayMenu.swift. Eleven unit tests; no AppKit, no networking.
Management built-ins (upstream_servers, quarantine_security) are excluded whatever their status; every tool_call qualifies; internal_tool_call qualifies for the three discovery/execution built-ins or on any failure, so a pre-dispatch call_tool_* failure still gets a row.
A failed upstream call persists both a tool_call and a call_tool_* wrapper under one request id; rule 4 keeps the tool_call one because it names the real server:tool. Adds activityRows() (rules 1-4 then first five) and activeClients() (status==active, first five). Nine more unit tests.
A failed call_tool_* wrapper is persisted with both a tool_name and a server_name, so dropping the type check would render it as server:call_tool_read instead of the bare built-in name. Adds that case plus the at-limit and zero-limit truncation boundaries. Also documents what would invalidate rule 4: nested code_execution calls all share one request id, and are invisible here only because the nested caller writes to the legacy history without emitting an activity event.
The legend was the one user-visible thing on the chart that no test asserted the presence of: flipping .chartLegend(.visible) to .hidden left the suite green while silently reclaiming the 20pt the frame grew to fit it — and testRealChartItemIsSizedAndLabelled would then have failed on the size, inviting whoever reclaimed the space to "fix" that test. Measured rather than asserted from the outside: on an all-zero axis the chart draws no error segments, so the only red the view can contain is the legend's "Errors" swatch. Probed both ways before writing the assertion — visible: 56 red samples, all in the bottom third; hidden: zero, everywhere. The sampler walks the rep's pixel extent so "the bottom 24 points" means the same thing at 1x and 2x, which the existing pixels(of:) helper does not do. The decode test now also asserts `type` and `status`. They are the two fields the entire feature branches on — type drives rules 1-3 and rowLabel's server:tool decision, status drives the symbol, tint, VoiceOver phrasing and error clause — and nothing else sees the wire format for them.
…p one loop
Five small things the whole-branch review called out:
* CoreProcessManager's periodic-refresh comment read "SSE doesn't emit
'activity' events", thirty lines below a switch that now consumes two
activity SSE events. Literally still true, but a reader would take it
as "SSE carries no activity" and delete the activityVersion bump on
its strength.
* AppState's prepend comment described updateGlanceSessions as carrying
an id-list guard; that became a whole-value Equatable guard for a
load-bearing reason (an id-only comparison froze the live call count).
* EnvelopeProbe's residual: it answers "does this body carry a success
field", so a bare payload with its own success field probes as
enveloped. Reachable, harmless, and previously documented as the
unreachable shape.
* sendSensitiveDataAlert has no production callers and never has had
any; the note names sensitive_data.detected as the intended trigger
so the next reader does not assume it is live.
* updateInPlace indexed `entries` by `activityRows.indices` while the
adjacent loop used zip. Now both use zip: one of the two could read
out of bounds if the count guard above were weakened, and it was the
only one that could.
…orrectly Two corrections to the fix wave. mergeGlanceActivity discarded every retained row when the polled page was empty — finding 2's own bug shape surviving in the branch that handles "no newest record". An empty page contradicts nothing: it says the server had recorded no matching calls when it answered, which is silence about a call it had not written yet, so an SSE row prepended while that poll was in flight was erased anyway. The doc comment reasoned about unparsable timestamps and never noticed the empty page takes the same branch; both cases are now enumerated, and they mean opposite things — a page WITH records is the server's account of the feed and still wins. The previous commit's corrected comment also invented the event names. `activity.upstream_completed` / `activity.internal_completed` exist nowhere in the repo; the wire names, grepped from internal/runtime/events.go:29,41 and matched by GlanceEvent, are `activity.tool_call.completed` and `activity.internal_tool_call.completed`.
Deploying mcpproxy-docs with
|
| Latest commit: |
99c8130
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1d90afbb.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://feat-tray-glance.mcpproxy-docs.pages.dev |
The empty-page guard I added to keep a racing SSE row on screen returned the whole existing list, so it also preserved canonical rows the server had legitimately dropped — and on an idle core nothing ever reached back past them, so they stayed forever while the successful polls kept clearing the staleness marker that would have hinted at trouble. The same hole existed before that guard, for any deleted record whose timestamp was newer than the remaining page's maximum. "Absent from the page" was never the right predicate: retention is for rows the server has not written yet, which is exactly the rows this tray prepended from SSE and no poll has carried back. AppState now tracks those keys, and the merge retains nothing else — a row the page carries becomes the server's record and leaves the set, a row that is dropped leaves it too. My own test passed the broken behaviour because it started from an optimistic row and polled empty once. The three new cases are the ones that could not: a canonical row deleted server-side, a deleted row newer than everything the page still carries, and a live row confirmed by a poll and deleted afterwards.
The REST refreshes check the generation; the SSE path checked only `.connected`, and the "it is one MainActor hop" argument for leaving it that way was wrong — executors guarantee no ordering between that hop and the reconnect work, so an event read from a dead core's stream can resume after `.connected` has been restored and prepend that core's call to the new core's feed. The existing SSE test covers the disconnected window, where the plain guard already worked. prependGlanceActivity now takes the generation and checks isCurrentConnection, the same shape as the three fetches. The caller captures it where the STREAM is opened rather than at event arrival: a stream belongs to exactly one connection, whereas a generation read at arrival is the post-reconnect one and would pass the guard. Pinned in both directions — an event from the previous connection is dropped, one from the live connection still publishes.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…he glance ask Measured against the real activity log on this machine (2585 records, newest 100 matching the glance's type filter): 848 KB of JSON whole, 30 KB in summary form. Per record, p50 is 1.3 KB but p90 is 22 KB and p99 is 166 KB — `response` is truncated at 64 KB, `arguments` and `metadata` are not truncated at all. The tray repeats this request every 30 seconds and renders none of those three fields. So the earlier rationale for the deeper page was materially incomplete: it compared bucket scans and struct copies, which is the server's cost, and ignored the wire. 50 -> 100 was not "50 more struct copies", it was another ~420 KB per poll of serialisation, transfer and decoding. GET /api/v1/activity now takes exclude_payloads=true, which drops arguments, response and metadata from the response. It is a projection, not a filter — applied at serialisation, so it changes nothing about which records match — and has_sensitive_data is still derived from metadata before metadata is dropped, since the row icon keys off it. Default is unchanged, pinned by a test, so no other client is affected; recentActivity() deliberately does not set it, because the Dashboard renders exactly those fields. This makes the 100-record page ~28x cheaper than a 100-record page was, and ~14x cheaper than the 50-record page this branch started from.
Rule 4 collapses every record sharing a request_id into one row, so five records that pass rules 1-3 can be three rows — a failed call contributes a wrapper and an upstream record under one id. The guarantee restated with the deeper page said "five qualify", which is true of records and not of rows. The depth tests all use unique or absent request ids, so nothing would have caught the discrepancy; the new test is a wrapper/upstream page where six qualifying records make three rows.
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 30521454890 --repo smart-mcp-proxy/mcpproxy-go
|
… run in CI Both encoding tests failed on the GitHub macOS runner. Two causes, and neither was flakiness. NSHostingView.cacheDisplay needs a display context the runner does not have: it painted 13 sample points there against hundreds locally. So the tests were developer-machine-only coverage that happened to look green because they never ran — the `swift-test` job is gated on native/ paths and this is the first branch to touch them. SwiftUI's ImageRenderer draws offscreen through Core Graphics with no window server involved. Measured both ways before switching: identical red and chroma counts, so it is the same raster, not an approximation. The runner's 13 chromatic pixels were real and meaningful — the legend's red "Errors" swatch, which every render carries, success-only included. Asserting no chroma over the whole image was therefore asserting something false; it passed locally only because the old sampler strode `rep.size`, which is the POINT size at 2x and the PIXEL size at 1x, so it covered a quarter of the image on this machine and all of it on the runner. That test is now scoped to the plot area, which is what it always meant, and `scale` is pinned so the pixel grid belongs to the test rather than to the display. The alpha cut-off drops from 0.5 to 0.1: Color.secondary resolves to ~0.5 alpha, so the old threshold excluded the entire success series against a transparent backing and read as a blank chart. No skip was reintroduced. Each assertion is positive or guarded by a drawn-pixel floor, so a blank raster fails loudly instead of passing.
The previous commit said the hosting-view path "needs a display context the runner does not provide". The CI log does not support that: the success-only fixture drew 181 samples there, 13 of them chromatic, and 13 is exactly the legend's red swatch at 1x (54 at 2x here). So the runner renders solid shapes fine — it is axis TEXT that did not come through, at 1x backing scale. The two failures were caused by the sampler, not by a missing raster: `rep.size` is points at 2x and pixels at 1x, so the old stride covered a quarter of the image here and all of it on the runner (hence a legend swatch that was invisible locally and fatal there); and the `> 0.5` alpha cut-off dropped the entire Color.secondary success series, which is what made 181 samples look like a blank chart. ImageRenderer is still the right call — explicit scale, offscreen, and nothing below depends on text rendering — but the reason is host variance, not an absent display context.
prependGlanceActivity added a key for every SSE event and trimmed the feed to the cap without trimming the set, so keys outlived the rows they described. The poll is the only other thing that prunes the set — which means the set grew without bound exactly while polls were failing and SSE stayed healthy, i.e. during the scenario the staleness marker exists to report, for as long as the core stayed up. The cap now trims both together: after the feed is truncated the set is intersected with the keys still in it.
… the guard The round-2 tests hand prependGlanceActivity a generation the test picked, so they verify the final guard and nothing before it — they pass unchanged against the implementation that reads the generation when the event ARRIVES, which after a reconnect is the new generation and sails through. That is the regression the fix was written for, and no test could see it. The stream loop moves into SSEStreamSession, whose whole purpose is to own the rule: capture once, before the stream is opened, and hand that value to every event the stream delivers. startSSEStream is now a single call to it. Three stream-level tests drive it: a reconnect between the stream opening and the event arriving (dropped), an uninterrupted stream (published), and a reconnect during the connect itself (dropped, because the capture precedes open). Verified by reverting to the arrival-time read — the two negative tests fail on their assertions, while every test in GlanceReconnectGenerationTests stays green, which is the point. The handshake is on `open`, not on `captureGeneration`: `open` is called at the same point by both implementations, so the test's sequencing does not depend on the thing under test. Hanging it off the capture made the wrong implementation fail by 5s timeout instead of by assertion.
…aster Two holes in the same file, both found by mutation rather than by reading. `drawn` counts axes, grid lines and tick labels, so no assertion here proved the success bars were painted at all: setting the success fill to Color.clear left all three encoding tests green. The no-chroma test was the worst of them — with no bars it passes more easily, since what it measures is the absence of colour. The new test compares like for like, the same single-hour axis with ten successes and with none, so the difference is the bar and nothing else. And `measure` still turned "no raster at all" into an XCTSkip. That is the third narrowing of one hole: a failed threshold used to skip, then a blank raster did, and each time the surviving shape was "the environment where these tests assert nothing reports success". It is now XCTUnwrap. There is no environment where a macOS build can legitimately fail to produce a CGImage here and the suite should still pass — the tray draws this chart with the same machinery, so a host that cannot rasterise it cannot run the feature either.
The disclosed residual turned out to be reachable: with SSEStreamSession
left correct, restoring the arrival-time generation read inside
startSSEStream passed all 13 tests across both generation test files.
Keeping the body to a single call makes the omission visible on sight,
and visible on sight is not a test.
Three small things make the wiring drivable:
* SSEStreaming — the two members (connect/disconnect) the manager
actually consumes, so a stub can stand in for SSEClient.
* installSSEClient + an internal startSSEStream, the seam the test
needs. Production wiring is unchanged; connectToCore still builds
the real client.
* NotificationService's UNUserNotificationCenter is now resolved
lazily. An eager `let` traps with "bundleProxyForCurrentProcess is
nil" in an unbundled process, i.e. in any XCTest runner, which is
why nothing holding one had ever been constructed in a test — the
rate-limit tests mirror the logic in a private struct to get around
exactly this. Safe because the type is an actor, so the lazy
initialisation is isolated.
The negative assertion is deterministic rather than timed: a `status`
event follows the glance event, its handling is not generation-guarded,
and events are handled in stream order — so once the status event's
effect is visible, the glance event has already been processed.
Verified with the mutation from the report: only the new wiring test
fails, on its assertion.
The debounced objectWillChange -> rebuildMenu() sink scheduled on RunLoop.main. Combine's RunLoop scheduler installs its timers in the run loop's .default mode, but while an NSMenu is on screen the main run loop runs in NSEventTrackingRunLoopMode — so the sink was not serviced until the menu closed. That made the whole open-menu update path unreachable in production. GlanceSection.updateInPlace and MenuRebuildGuard's updateInPlace / deferUntilClose branches only ever run if something calls rebuildMenu() while the menu is up, and this sink is the only caller that does. The glance was therefore a snapshot frozen at menu-open time: live QA held the menu open for 45s and watched a 45-second-old call stay captioned "3s", the header counts stay put while the REST API moved on, two captures 2s apart come out pixel-identical, and a call made while the menu was open never appear at all. Schedule on the main dispatch queue instead, which is drained in every run-loop mode. Same reason the timers alongside it publish in .common. The 500ms window and the icon updates are untouched, and the menu is still never restructured under the cursor: with the guard armed, rebuildMenu() returns before touching menu structure on both branches. The scheduler is named rather than written inline so a test can bind to the very value production uses.
Every existing glance test calls GlanceSection.updateInPlace directly, so the whole suite stayed green while the one production caller that reaches it during menu tracking was never serviced. These cover the delivery layer underneath: they pump the main run loop in NSEventTrackingRunLoopMode ONLY, the mode it runs in while an NSMenu is up, and never in .default. The end-to-end case arms a MenuRebuildGuard, mutates AppState, and asserts the refresh reaches the section on the updateInPlace branch and rewrites the row already on screen. It binds to AppController.menuRefreshScheduler, so reverting that to RunLoop.main fails it — verified by doing exactly that: the header stays at "12 calls this hour", which is the QA symptom. A control case pins that RunLoop.main delivers nothing under the same pump but is released by one turn of .default, proving the pump is selective rather than draining every mode, and two more pin that the 500ms window still coalesces a burst into one main-thread repaint when nothing is tracking. The tests initialise NSApplication: .eventTracking only reaches the main dispatch queue once AppKit has added it to the run loop's common modes, which holds for the real tray but not for a bare xctest bundle. Without it every case here would pass by measuring a process where no scheduler at all is serviced during tracking.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Arming auto-merge: live QA on a real tray. Glance counts matched the REST API exactly on every comparison, and SSE delivery was verified (a call appeared with no intervening HTTP request). The one medium defect found — the menu froze while open because the debounced sink was scheduled on RunLoop.main, whose .default mode is not serviced during NSMenu tracking, making the updateInPlace/deferUntilClose machinery unreachable — was fixed and re-verified live: rows and header now update under an open menu across a poll boundary, a call made while open appears without reopening, ages tick, and the menu is not disrupted (highlight and open submenu survive, identical highlight-pixel counts across an 8-frame burst). 473 Swift tests, 0 failures. Cosmetics tracked in #934.
Conflict in NotificationService.swift: both sides made the center lazy to unblock unit construction. main's version (from #927) is a superset — it also adds the deliveryEnabled flag and its guards — so it is taken whole. The glance branch's other change to the file, the doc comment on sendSensitiveDataAlert, merged cleanly and is preserved.
There was a problem hiding this comment.
Arming auto-merge: re-verified live under the plain documented setup after the main merge. Menu held open ~50s now updates in place across a poll boundary (all four capture digests differ; previously pixel-identical), a call made while the menu is open appears without reopening, ages tick, and the menu is not disrupted (identical highlight-pixel signature across frames, submenu stays open). Merged with main — the NotificationService conflict took #927's superset, preserving both changes. 502 Swift tests, 0 failures. Cosmetics tracked in #934.
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 30, 2026
… gained a status filter (#939) Semantic merge conflict, not a logic bug: #928 added sessions_retention_test.go calling GetRecentSessions(limit) while #930 added a status parameter for the tray glance's client filtering. Both PRs were green independently and broke main only once combined, because branch protection does not require a PR to be up to date with main before merging. Pass "" — the documented no-filter value — at the four call sites. The retention tests want every surviving record, so unfiltered is the correct argument, not merely the compiling one.
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.
Implements
docs/superpowers/specs/2026-07-29-tray-glance-design.mdviadocs/superpowers/plans/2026-07-29-tray-glance.md.What it adds
A compact block at the top of the macOS tray menu: the five most recent tool calls, the connected MCP clients, and a 24-hour calls-per-hour histogram behind a submenu. Plus one Go change — a
statusfilter onGET /api/v1/sessions, applied during the storage cursor walk before truncation.Rows are plain
NSMenuItems, never hosted SwiftUI views: custom menu-item views receive mouse but not keyboard events, so SwiftUI rows would not be keyboard-selectable. Only the histogram is a hosted view, and it lives in a submenu.Load-bearing behaviours, each verified by mutation
menuDidClose. This is load-bearing, not a nicety:glanceSessionsrepublishes on nearly every 30s poll for an active session.representedObject— keyed onrequestId, not theid, which turns over on every reconcile. A title-only refresh would leave a row whose click opens the previous record's session.callsincludes itserrors, so the chart stackscalls - errorsanderrors. Verified on the producer, not just the consumer.AppStatefeeds are never narrowed —recentActivitybacks the Dashboard's full log,recentSessionsbacks session-name lookup; the glance has its own feeds.Testing
289 → 455 tests, 0 failures. All gates green at HEAD:
swift buildclean (no new warnings across ~1,900 new Swift lines),swift test455/455,go build ./...and-tags server,go teston 7 packages,make swagger-verify,golangci-lintv2 strict 0 issues.Eight tasks, each independently reviewed and fix-looped clean, then a whole-branch review, then a cross-model (Codex) review, then one fix wave and a scoped re-review. Roughly 90 mutants killed across the reviews.
The histogram is the first
NSHostingViewin this menu and has never been seen on screen. Unverified: whether the submenu opens populated, whether 24 bars read at ~10pt each, legend legibility at 288pt, a 132pt menu row height,Color.secondaryagainst menu vibrancy in both appearances, and whether the chart's accessibility label is announced once or twice. Please do not merge before that pass.Depends on #927
The glance clears its state on a
coreStatetransition out of.connected. In external-attach mode that transition never fires (#927), so without it the block would keep presenting a dead core's data as a live, ticking display. The branch's logic is correct; its input is broken. Merge #927 first.