fix(desktop): event-driven tray menu + real org name#3458
Conversation
Replace the 5s tray polling interval with event-driven refresh: `mouse-enter` + host-service status-changed trigger an async rebuild. Drop the hostInfoCache Map — updateTrayMenu is now async and fetches host.info inline with a 2s AbortController timeout, so there is no stale state to manage. Unwrap the superjson envelope (`result.data.json`) that the original polling code missed, which was causing every org to render as a UUID slice (previously) or "Loading…" (after the first pass of this fix). Closes #3454
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe tray menu refresh mechanism was refactored from periodic 5-second polling to event-driven, on-demand updates. Host information is now fetched asynchronously with a 2-second timeout per active organization during menu updates, triggered by initialization, status changes, and mouse-enter events. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes a long-standing bug where the desktop tray menu always displayed a UUID prefix instead of the real organization name. The root cause was that the original code read Along with the fix, the polling architecture is replaced with an event-driven one:
Confidence Score: 4/5Safe to merge with one targeted fix: add a single-flight guard to prevent concurrent The core bug fix (superjson envelope unwrapping) is correct and well-reasoned. The architectural shift from polling to event-driven updates is cleaner and eliminates stale-cache issues. The 2 s AbortController pattern is implemented correctly with apps/desktop/src/main/lib/tray/index.ts — specifically the Important Files Changed
Sequence DiagramsequenceDiagram
participant E as Electron Events
participant T as Tray (index.ts)
participant C as HostServiceCoordinator
participant H as host-service HTTP
Note over T: initTray()
T->>T: void updateTrayMenu()
T->>C: getActiveOrganizationIds()
C-->>T: [orgId1, orgId2]
par fetchHostInfo(orgId1)
T->>H: GET /trpc/host.info (2s timeout)
H-->>T: { result.data.json: { organization, version } }
and fetchHostInfo(orgId2)
T->>H: GET /trpc/host.info (2s timeout)
H-->>T: { result.data.json: { ... } }
end
T->>T: buildHostServiceSubmenu(orgIds, infos)
T->>T: tray.setContextMenu(menu)
Note over E,T: status-changed event
E->>T: status-changed
T->>T: void updateTrayMenu() [same flow]
Note over E,T: user hovers tray icon
E->>T: mouse-enter
T->>T: void updateTrayMenu() [same flow]
Reviews (1): Last reviewed commit: "fix(desktop): drive tray menu off events..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async function updateTrayMenu(): Promise<void> { | ||
| if (!tray) return; | ||
|
|
||
| refreshHostInfo(); | ||
|
|
||
| const coordinator = getHostServiceCoordinator(); | ||
| const orgIds = coordinator.getActiveOrganizationIds(); | ||
|
|
||
| const infoEntries = await Promise.all( | ||
| orgIds.map(async (orgId) => [orgId, await fetchHostInfo(orgId)] as const), | ||
| ); | ||
| const infos = new Map<string, HostInfo>(); | ||
| for (const [orgId, info] of infoEntries) { | ||
| if (info) infos.set(orgId, info); | ||
| } | ||
|
|
||
| if (!tray) return; |
There was a problem hiding this comment.
updateTrayMenu calls
Both the status-changed listener and the tray.on("mouse-enter") handler call void updateTrayMenu() independently, and updateTrayMenu now awaits up to 2 seconds of parallel network fetches. There is no guard preventing multiple calls from being in-flight simultaneously.
A concrete race: status-changed fires and starts call #1 (awaiting 2 s). The user then hovers the tray 300 ms later, starting call #2. Call #2 finishes first and sets the menu correctly. Then call #1 finishes and overwrites the menu with data derived from the orgIds snapshot it took 300 ms earlier — potentially excluding an org that was just added, or including one that was just stopped.
A lightweight fix is a single-flight flag that drops duplicate calls while one is in-progress:
let updateInFlight = false;
async function updateTrayMenu(): Promise<void> {
if (updateInFlight) return;
updateInFlight = true;
try {
// ... existing body ...
} finally {
updateInFlight = false;
}
}Alternatively, schedule a debounced microtask so rapid-fire events coalesce into one fetch cycle.
| const status = coordinator.getProcessStatus(orgId); | ||
| const info = infos.get(orgId); | ||
| const isRunning = status === "running"; | ||
| const label = info?.organizationName ?? "Loading…"; | ||
| const versionSuffix = info?.version ? ` (v${info.version})` : ""; |
There was a problem hiding this comment.
status can be "stopped" while label shows "Loading…"
buildHostServiceSubmenu calls coordinator.getProcessStatus(orgId) after the async fetch boundary in updateTrayMenu. getActiveOrganizationIds() filters to status !== "stopped", but between the snapshot and the menu build the coordinator state can change. If a service stops in that window, getProcessStatus returns "stopped", isRunning is false, and both action items are disabled — but the org is still rendered with a "Loading…" label (because fetchHostInfo returned null for a non-running process). The menu then shows:
Loading…
stopped
Restart (disabled)
Stop (disabled)
This isn't catastrophic — the next status-changed event corrects it — but it's misleading UX. Consider re-reading orgIds after the await to filter to still-active orgs, or at least fall back to orgId.slice(0, 8) when the org is no longer in a running state.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…et-sh#3458) Replace the 5s tray polling interval with event-driven refresh: `mouse-enter` + host-service status-changed trigger an async rebuild. Drop the hostInfoCache Map — updateTrayMenu is now async and fetches host.info inline with a 2s AbortController timeout, so there is no stale state to manage. Unwrap the superjson envelope (`result.data.json`) that the original polling code missed, which was causing every org to render as a UUID slice (previously) or "Loading…" (after the first pass of this fix). Closes superset-sh#3454
chore(upstream): PR2 MCP OAuth audience + tray menu refactor (superset-sh#3459 superset-sh#3458)
-s ours merge to record that upstream commits a3e34bf through de70163 (13 commits) are semantically already present on origin/main via the PR1-6 cherry-pick series (PRs #176, #177, #178, #179, #180, #182), plus fork-adaptation fixes layered on top. This merge target is de70163 specifically (not upstream/main) so newer upstream commits (9fff075 and later) remain visible in future behind counts. Upstream commits covered by this audit merge: - a3e34bf fix(desktop): restore cmd+click requirement for v1 terminal file links (superset-sh#3457) [PR1/#176] - 57557f8 fix(desktop): gate v2 workspace children on collection readiness (superset-sh#3464) [PR1/#176] - 4ee2e61 fix(desktop): use native clipboard for copy path in v2 sidebar (superset-sh#3462) [PR1/#176] - 87d6e93 feat(desktop): close settings with Escape key (superset-sh#3466) [PR1/#176] - 9c7f5f4 chore(desktop): auto-restart host-service on bundle change in dev (superset-sh#3461) [PR1/#176] - 93140d9 fix(mcp): accept MCP resource URL as valid OAuth audience (superset-sh#3459) [PR2/#177] - be9e000 fix(desktop): drive tray menu off events, fetch real org name (superset-sh#3458) [PR2/#177] - c5f791e feat(v2): unify workspace delete through host-service (superset-sh#3443) [PR3/#178] - 2c24d93 feat(desktop): paginated branch picker with checkout + open actions (superset-sh#3397) [PR4/#179] - 2bf1049 feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator (superset-sh#3460) [PR5/#180] - 1294a7d feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav (superset-sh#3472) [PR5/#180] - de70163 feat(desktop): v2 review tab first pass — PR info, checks, comments (superset-sh#3463) [PR6/#182] Intentionally skipped (version bump, fork has independent versioning): - 1e23353 chore(desktop): bump version to 1.5.5 (superset-sh#3473) Fork-adaptation fixes layered on top of the cherry-picks: - PR1: host-service-coordinator alias import fix, settings Escape selector narrowing (role-based + popper wrapper), Escape close uses replace navigation - PR2: dual quit mode preservation (requestQuit "release"/"stop"), trayUpdateToken guard for stale async fetchHostInfo results - PR4: ChangesHeader.normalizeBranchName regex rewrite (lint false positive), worktree add uses fullRef for remote-tracking refs, syncTimedOut reset on pendingId change, GIT_REFS.md barrel example fix - PR5: migrate.ts re-sanitize of existing localStorage overrides (v2 marker bump intent), FOCUS_PANE_* enabled:isActive for KeepAliveWorkspaces, CATEGORY_ORDER merges Navigation (upstream) and Browser (fork) - PR6: normalizeThreadsToComments flattens all thread.comments (not just first), CommentPane overrides <a> (openUrl) and <img> (SafeImage), zero-badge suppression, pr-null comments gate Fork features verified intact (Explore agent audit of combined 36d4de4..35d95f3 range): - BROWSER_RELOAD / BROWSER_HARD_RELOAD hotkeys - dual quit mode menu in tray - v1 terminal cold-restore + retry reconnect (out of range but unaffected) - KeepAliveWorkspaces (FOCUS_PANE_* gated on isActive) - useCommandPalette + addMemoTab in v2 workspace - host-service-coordinator rename alias pattern
1 participant
Summary
apps/desktop/src/main/lib/tray/index.ts. The tray menu is now driven byhost-service-coordinatorstatus-changedevents,mouse-enteron the tray icon, and the initialinitTray()call.hostInfoCacheMap.updateTrayMenu()is now a single async function that fetcheshost.infofor every active org in parallel (2sAbortControllertimeout) and builds the menu directly from the results — no persistent state, no sync/async split.result.data.json) when readinghost.info. The original polling code readresult.dataand silently discarded every response, which is why the tray was falling back toorgId.slice(0, 8)(the UUID prefix) for every running service.Test plan
bun run lint:fix— cleanbun run typecheck(apps/desktop) — cleanbun test(apps/desktop) — 1660 pass, 0 failLoading…/ a UUID slicestatus-changedwithout needing to hoverCloses #3454
Summary by cubic
Make the desktop tray menu event-driven and show real organization names and versions instead of UUIDs or “Loading…”. Removes 5s polling and updates the menu on status changes and when hovering over the tray icon.
Bug Fixes
host.infofromresult.data.json, so entries display the correct organization name and version.Refactors
status-changedand traymouse-enter.hostInfoCache;updateTrayMenuis async and fetches per active org in parallel with a 2s timeout (viaAbortController).Written for commit a595c31. Summary will update on new commits.
Summary by CodeRabbit