diff --git a/cmd/sgai/menubar.go b/cmd/sgai/menubar.go index 92819d9..16fb627 100644 --- a/cmd/sgai/menubar.go +++ b/cmd/sgai/menubar.go @@ -47,6 +47,15 @@ func toMenuBarItem(srv *Server, w workspaceInfo) menuBarItem { } } +func menuBarItemsFromServer(srv *Server) []menuBarItem { + workspaces := workspaceInfos(srv.doScanWorkspaceGroups()) + items := make([]menuBarItem, 0, len(workspaces)) + for _, workspace := range workspaces { + items = append(items, toMenuBarItem(srv, workspace)) + } + return items +} + func countAttention(items []menuBarItem) int { count := 0 for _, item := range items { diff --git a/cmd/sgai/menubar_darwin.go b/cmd/sgai/menubar_darwin.go index 496db3b..a1d2a72 100644 --- a/cmd/sgai/menubar_darwin.go +++ b/cmd/sgai/menubar_darwin.go @@ -125,18 +125,7 @@ func menuBarUpdateLoop(ctx context.Context, srv *Server, state *darwinMenuBarSta } func rebuildMenuFromServer(srv *Server, state *darwinMenuBarState) { - groups, errScan := srv.scanWorkspaceGroups() - if errScan != nil { - return - } - - var items []menuBarItem - for _, grp := range groups { - items = append(items, toMenuBarItem(srv, grp.Root)) - for _, fork := range grp.Forks { - items = append(items, toMenuBarItem(srv, fork)) - } - } + items := menuBarItemsFromServer(srv) state.mu.Lock() state.nextTag = 0 diff --git a/cmd/sgai/menubar_test.go b/cmd/sgai/menubar_test.go index 9585d16..1fa182e 100644 --- a/cmd/sgai/menubar_test.go +++ b/cmd/sgai/menubar_test.go @@ -457,3 +457,22 @@ func TestToMenuBarItemRepairsMissingGoalTitle(t *testing.T) { item = toMenuBarItem(server, updated(newTestWorkspaceInfo(), func(workspace *workspaceInfo) { workspace.DirName = "test-ws"; workspace.Directory = wsDir })) assert.Equal(t, "Menu Repair Title", item.title) } + +func TestMenuBarItemsFromServerUsesFreshWorkspaceState(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "live-state") + + groups, errScan := server.scanWorkspaceGroups() + require.NoError(t, errScan) + require.Len(t, groups, 1) + assert.False(t, groups[0].Root.Running) + + server.mu.Lock() + server.sessions[wsDir] = newTestServeSession(nil, true) + server.mu.Unlock() + + items := menuBarItemsFromServer(server) + require.Len(t, items, 1) + assert.True(t, items[0].running) + assert.Equal(t, 1, countRunning(items)) +} diff --git a/cmd/sgai/webapp/src/hooks/__tests__/useNotifications.test.tsx b/cmd/sgai/webapp/src/hooks/__tests__/useNotifications.test.tsx new file mode 100644 index 0000000..6ad7de3 --- /dev/null +++ b/cmd/sgai/webapp/src/hooks/__tests__/useNotifications.test.tsx @@ -0,0 +1,225 @@ +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; +import { cleanup, renderHook, waitFor } from "@testing-library/react"; +import * as factoryStateModule from "@/lib/factory-state"; +import type { ApiWorkspaceEntry } from "@/types"; +import { useNotifications } from "../useNotifications"; + +function createMockWorkspace(overrides: Partial = {}): ApiWorkspaceEntry { + return { + name: "workspace", + dir: "/workspaces/workspace", + running: false, + needsInput: false, + inProgress: false, + pinned: false, + isRoot: false, + isFork: false, + title: "", + computedTitle: "", + status: "", + badgeClass: "", + badgeText: "", + hasSgai: true, + hasEditedGoal: false, + interactiveAuto: false, + continuousMode: false, + currentAgent: "", + currentModel: "", + task: "", + goalContent: "", + rawGoalContent: "", + pmContent: "", + hasProjectMgmt: false, + svgHash: "", + totalExecTime: "", + latestProgress: "", + humanMessage: "", + agentSequence: [], + cost: { + totalCost: 0, + dollars: { + input: 0, + output: 0, + reasoning: 0, + cacheRead: 0, + cacheWrite: 0, + total: 0, + }, + totalTokens: { + input: 0, + output: 0, + reasoning: 0, + cacheRead: 0, + cacheWrite: 0, + }, + byAgent: [], + }, + events: [], + messages: [], + projectTodos: [], + agentTodos: [], + log: [], + ...overrides, + }; +} + +interface RecordedNotification { + title: string; + options: NotificationOptions | undefined; +} + +class MockNotification { + static permission: NotificationPermission = "granted"; + + onclick: (() => void) | null = null; + + constructor(title: string, options?: NotificationOptions) { + recordedNotifications.push({ title, options }); + } +} + +let mockState: factoryStateModule.FactoryStateSnapshot; +let recordedNotifications: RecordedNotification[]; + +const originalNotification = window.Notification; + +describe("useNotifications", () => { + beforeEach(() => { + recordedNotifications = []; + mockState = { + workspaces: [], + fetchStatus: "idle", + lastFetchedAt: null, + }; + + spyOn(factoryStateModule, "useFactoryState").mockImplementation(() => mockState); + + Object.defineProperty(window, "Notification", { + configurable: true, + writable: true, + value: MockNotification, + }); + }); + + afterEach(() => { + mock.restore(); + cleanup(); + + Object.defineProperty(window, "Notification", { + configurable: true, + writable: true, + value: originalNotification, + }); + }); + + it("uses the computed workspace display label in the notification body", async () => { + const { rerender } = renderHook(() => useNotifications()); + + mockState = { + ...mockState, + lastFetchedAt: Date.now(), + workspaces: [ + createMockWorkspace({ + name: "workspace-name", + dir: "/workspaces/workspace-name", + needsInput: true, + title: "Workspace Title", + computedTitle: "Computed Workspace Label", + }), + ], + }; + + rerender(); + + await waitFor(() => { + expect(recordedNotifications).toHaveLength(1); + }); + + expect(recordedNotifications[0]).toEqual({ + title: "Approval Needed", + options: { + body: "Workspace Computed Workspace Label needs your input", + tag: "/workspaces/workspace-name", + }, + }); + }); + + it("tracks duplicate-name workspaces independently by directory using the UI label", async () => { + const { rerender } = renderHook(() => useNotifications()); + + mockState = { + ...mockState, + lastFetchedAt: 1, + workspaces: [ + createMockWorkspace({ + name: "shared-workspace", + dir: "/two/shared-workspace", + needsInput: false, + title: "Shared Workspace", + }), + createMockWorkspace({ + name: "shared-workspace", + dir: "/one/shared-workspace", + needsInput: true, + title: "Shared Workspace", + }), + ], + }; + + rerender(); + + await waitFor(() => { + expect(recordedNotifications).toHaveLength(1); + }); + + expect(recordedNotifications[0]).toEqual({ + title: "Approval Needed", + options: { + body: "Workspace Shared Workspace · one needs your input", + tag: "/one/shared-workspace", + }, + }); + + mockState = { + ...mockState, + lastFetchedAt: 2, + workspaces: [ + createMockWorkspace({ + name: "shared-workspace", + dir: "/one/shared-workspace", + needsInput: false, + title: "Shared Workspace", + }), + createMockWorkspace({ + name: "shared-workspace", + dir: "/two/shared-workspace", + needsInput: true, + title: "Shared Workspace", + }), + ], + }; + + rerender(); + + await waitFor(() => { + expect(recordedNotifications).toHaveLength(2); + }); + + expect(recordedNotifications).toEqual([ + { + title: "Approval Needed", + options: { + body: "Workspace Shared Workspace · one needs your input", + tag: "/one/shared-workspace", + }, + }, + { + title: "Approval Needed", + options: { + body: "Workspace Shared Workspace · two needs your input", + tag: "/two/shared-workspace", + }, + }, + ]); + }); +}); diff --git a/cmd/sgai/webapp/src/hooks/useNotifications.ts b/cmd/sgai/webapp/src/hooks/useNotifications.ts index 9747767..9a666cb 100644 --- a/cmd/sgai/webapp/src/hooks/useNotifications.ts +++ b/cmd/sgai/webapp/src/hooks/useNotifications.ts @@ -1,26 +1,46 @@ import { useEffect, useRef } from "react"; import { useFactoryState } from "../lib/factory-state"; import type { ApiWorkspaceEntry } from "../lib/factory-state"; +import { buildWorkspaceNameDisambiguators, getWorkspaceDisplayLabel } from "../lib/workspace-identity"; -interface NeedsInputEntry { - name: string; - needsInput: boolean; - forks?: NeedsInputEntry[]; +type NotificationWorkspaceEntry = Pick; + +type NotificationWorkspaceSource = NotificationWorkspaceEntry & Pick; + +function mergeNotificationWorkspace( + current: NotificationWorkspaceEntry | undefined, + next: NotificationWorkspaceEntry, +): NotificationWorkspaceEntry { + if (!current) { + return next; + } + + return { + ...current, + ...next, + needsInput: next.needsInput, + title: next.title || current.title, + computedTitle: next.computedTitle || current.computedTitle, + }; } -function collectNeedsInput( - workspaces: NeedsInputEntry[], - out: Map, -): void { - for (const ws of workspaces) { - out.set(ws.name, ws.needsInput); - if (ws.forks) { - collectNeedsInput(ws.forks, out); +function collectNotificationWorkspaces( + workspaces: NotificationWorkspaceSource[], +): NotificationWorkspaceEntry[] { + const byDir = new Map(); + + for (const workspace of workspaces) { + byDir.set(workspace.dir, mergeNotificationWorkspace(byDir.get(workspace.dir), workspace)); + + for (const fork of workspace.forks ?? []) { + byDir.set(fork.dir, mergeNotificationWorkspace(byDir.get(fork.dir), fork)); } } + + return [...byDir.values()]; } -function fireNotification(workspaceName: string): void { +function fireNotification(workspaceLabel: string, workspaceTag: string): void { if (!("Notification" in window)) { return; } @@ -30,8 +50,8 @@ function fireNotification(workspaceName: string): void { } const notification = new Notification("Approval Needed", { - body: `Workspace ${workspaceName} needs your input`, - tag: workspaceName, + body: `Workspace ${workspaceLabel} needs your input`, + tag: workspaceTag, }); notification.onclick = () => { @@ -48,15 +68,24 @@ export function useNotifications(): void { return; } + const notificationWorkspaces = collectNotificationWorkspaces(workspaces); + const workspaceNameDisambiguators = buildWorkspaceNameDisambiguators(notificationWorkspaces); const currentState = new Map(); - collectNeedsInput(workspaces, currentState); + + for (const workspace of notificationWorkspaces) { + currentState.set(workspace.dir, workspace.needsInput); + } const previous = previousStateRef.current; - for (const [name, needsInput] of currentState) { - const wasNeedingInput = previous.get(name) ?? false; - if (!wasNeedingInput && needsInput) { - fireNotification(name); + for (const workspace of notificationWorkspaces) { + const wasNeedingInput = previous.get(workspace.dir) ?? false; + + if (!wasNeedingInput && workspace.needsInput) { + fireNotification( + getWorkspaceDisplayLabel(workspace, workspaceNameDisambiguators), + workspace.dir, + ); } }