From f4b25dd2c901bf654be381eea96860a3a1e755cc Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 26 Jun 2026 23:29:44 -0700 Subject: [PATCH] FN-7096: gate approval banner to mailbox requests Limit the dashboard approval banner to real mailbox approval requests instead of task plan-approval states. - Stop awaiting-approval task updates from fabricating Open Mailbox banner candidates or refreshing mailbox counts. - Gate DashboardBanners rendering to approval: candidates while preserving pending-count race protection. - Update dashboard tests, documentation, and release notes for mailbox-only approval banners. Files changed: .changeset/fn-7096-approval-banner-mailbox.md | 7 + docs/dashboard-guide.md | 2 +- packages/dashboard/app/App.tsx | 3 +- .../app/components/__tests__/App.test.tsx | 33 ++-- .../app/components/dashboard/DashboardBanners.tsx | 9 +- .../dashboard/__tests__/DashboardBanners.test.tsx | 65 +++++++- .../hooks/__tests__/sseSplitIntegration.test.ts | 28 ++-- .../app/hooks/__tests__/useApprovalBanner.test.ts | 177 ++++++++------------- packages/dashboard/app/hooks/useApprovalBanner.ts | 34 +--- packages/dashboard/app/hooks/useMailboxUnread.ts | 5 +- 10 files changed, 185 insertions(+), 178 deletions(-) Fusion-Task-Id: FN-7096 Fusion-Task-Lineage: 40885aba-6a9f-4720-bea6-5a6f155c326d Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-7096-approval-banner-mailbox.md | 7 + docs/dashboard-guide.md | 2 +- packages/dashboard/app/App.tsx | 3 +- .../app/components/__tests__/App.test.tsx | 33 ++-- .../components/dashboard/DashboardBanners.tsx | 9 +- .../__tests__/DashboardBanners.test.tsx | 65 ++++++- .../__tests__/sseSplitIntegration.test.ts | 32 ++-- .../hooks/__tests__/useApprovalBanner.test.ts | 177 +++++++----------- .../dashboard/app/hooks/useApprovalBanner.ts | 34 +--- .../dashboard/app/hooks/useMailboxUnread.ts | 5 +- 10 files changed, 187 insertions(+), 180 deletions(-) create mode 100644 .changeset/fn-7096-approval-banner-mailbox.md diff --git a/.changeset/fn-7096-approval-banner-mailbox.md b/.changeset/fn-7096-approval-banner-mailbox.md new file mode 100644 index 0000000000..856ba4eba6 --- /dev/null +++ b/.changeset/fn-7096-approval-banner-mailbox.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop plan-approval tasks from showing an empty-mailbox approval banner. +category: fix +dev: Fixes useApprovalBanner so the Open Mailbox banner only follows real ApprovalRequest events. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index ed7993d54d..916ca2feb5 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -434,7 +434,7 @@ Mailbox view shows inbox/outbox communication threads and unread state. - in the **Agents** tab, the agent selector now includes **All agents**, which shows one combined agent-to-agent stream (with sender + recipient labels); selecting a specific agent still shows Inbox/Outbox subtabs - mailbox entry points now show unread/pending indicators: the desktop/tablet Header mailbox toggle shows a pending-approval dot first or an unread dot when unread mail exists without pending approvals, the mobile bottom-nav Mailbox tab carries the mobile badges/dots, and the compact Header actions overflow keeps a Mailbox entry only when the mobile bottom nav is disabled - approval lifecycle SSE events (`approval:requested`, `approval:updated`, `approval:decided`) trigger mailbox approvals refresh without manual reload -- when a task newly enters `awaiting-approval`, the app shows a persistent approval banner above project content with an **Open Mailbox** CTA; dismissals are remembered per approval item until that item advances or a different one arrives +- when a real pending mailbox approval request is created, the app shows a persistent approval banner above project content with an **Open Mailbox** CTA; task plan-approval states (`awaiting-approval`) remain visible on the triage board and do not create a mailbox banner - when a task first transitions into `done`, the dashboard shows a one-time **Enjoying Fusion?** GitHub star prompt in the project view after first-run setup is closed; clicking **Star on GitHub** or dismissing the card marks it shown in browser `localStorage`, so it does not reappear on reload or later task completions. The setup wizard does not add a second star prompt. - Visible message history/threading is driven by explicit `message.metadata.replyTo.messageId` links - Separate top-level messages from the same sender remain independent in the inbox and detail pane diff --git a/packages/dashboard/app/App.tsx b/packages/dashboard/app/App.tsx index 0112106a5c..51f8f45a84 100644 --- a/packages/dashboard/app/App.tsx +++ b/packages/dashboard/app/App.tsx @@ -469,7 +469,7 @@ function AppInner() { useMobileViewportRestoreReset(isMobile); // App-level mailbox/chat unread state (used for header/mobile nav badges) - const { mailboxUnreadCount, mailboxPendingApprovalCount, setMailboxUnreadCount, refresh: mailboxRefresh } = useMailboxUnread(currentProject?.id); + const { mailboxUnreadCount, mailboxPendingApprovalCount, setMailboxUnreadCount } = useMailboxUnread(currentProject?.id); const { chatHasUnreadResponse } = useChatUnreadBadge(currentProject?.id, { taskView, quickChatOpen }); const { stashOrphanCount } = useStashOrphanCount(currentProject?.id); const [showGitHubStarPrompt, setShowGitHubStarPrompt] = useState(false); @@ -480,7 +480,6 @@ function AppInner() { currentProjectId: currentProject?.id, gitHubStarPromptShown, onStarPrompt: handleStarPrompt, - onMailboxRefresh: mailboxRefresh, }); const { diff --git a/packages/dashboard/app/components/__tests__/App.test.tsx b/packages/dashboard/app/components/__tests__/App.test.tsx index 1a59edd5e3..b7b13b4fc6 100644 --- a/packages/dashboard/app/components/__tests__/App.test.tsx +++ b/packages/dashboard/app/components/__tests__/App.test.tsx @@ -1133,7 +1133,7 @@ describe("App mailbox unread count", () => { }); describe("App approval notification banner", () => { - it("shows banner when a task newly enters awaiting-approval", async () => { + it("does not show the mailbox banner when a task newly enters awaiting-approval", async () => { mockUseTasks.mockImplementation(() => ({ tasks: [{ id: "FN-1", title: "Task", description: "x", status: "in-progress", column: "in-progress", dependencies: [], steps: [], currentStep: 0, log: [], createdAt: "", updatedAt: "" }], createTask: mockCreateTask, @@ -1158,10 +1158,10 @@ describe("App approval notification banner", () => { await waitFor(() => expect(mockSubscribeSse).toHaveBeenCalled()); - const mailboxSubscriptionCall = mockSubscribeSse.mock.calls.find( + const approvalSubscriptionCall = mockSubscribeSse.mock.calls.find( ([url, sub]) => String(url).startsWith("/api/events") && typeof (sub as { events?: Record })?.events?.["task:updated"] === "function", ); - const subscriptionConfig = mailboxSubscriptionCall?.[1] as { + const subscriptionConfig = approvalSubscriptionCall?.[1] as { events: Record void>; }; @@ -1173,10 +1173,10 @@ describe("App approval notification banner", () => { ); }); - expect(screen.getByLabelText("Approval requests")).toBeInTheDocument(); + expect(screen.queryByLabelText("Approval requests")).toBeNull(); }); - it("persists dismissals and suppresses repeat alerts for the same approval item", async () => { + it("persists dismissals and suppresses repeat alerts for the same real approval request", async () => { mockUseTasks.mockImplementation(() => ({ tasks: [{ id: "FN-4", title: "Task", description: "x", status: "in-progress", column: "in-progress", dependencies: [], steps: [], currentStep: 0, log: [], createdAt: "", updatedAt: "" }], createTask: mockCreateTask, @@ -1201,17 +1201,19 @@ describe("App approval notification banner", () => { await waitFor(() => expect(mockSubscribeSse).toHaveBeenCalled()); - const mailboxSubscriptionCall = mockSubscribeSse.mock.calls.find( - ([url, sub]) => String(url).startsWith("/api/events") && typeof (sub as { events?: Record })?.events?.["task:updated"] === "function", + const approvalSubscriptionCall = mockSubscribeSse.mock.calls.find( + ([url, sub]) => String(url).startsWith("/api/events") + && typeof (sub as { events?: Record })?.events?.["approval:requested"] === "function" + && typeof (sub as { events?: Record })?.events?.["task:updated"] === "function", ); - const subscriptionConfig = mailboxSubscriptionCall?.[1] as { + const subscriptionConfig = approvalSubscriptionCall?.[1] as { events: Record void>; }; await act(async () => { - subscriptionConfig.events["task:updated"]( - new MessageEvent("task:updated", { - data: JSON.stringify({ id: "FN-4", status: "awaiting-approval", updatedAt: "2026-05-05T10:00:00.000Z" }), + subscriptionConfig.events["approval:requested"]( + new MessageEvent("approval:requested", { + data: JSON.stringify({ id: "approval-1", taskId: "FN-4", updatedAt: "2026-05-05T10:00:00.000Z" }), }), ); }); @@ -1225,15 +1227,16 @@ describe("App approval notification banner", () => { const latestSubscription = mockSubscribeSse.mock.calls .slice() .reverse() - .find(([, sub]) => typeof (sub as { events?: Record })?.events?.["task:updated"] === "function"); + .find(([, sub]) => typeof (sub as { events?: Record })?.events?.["approval:requested"] === "function" + && typeof (sub as { events?: Record })?.events?.["task:updated"] === "function"); const latestConfig = latestSubscription?.[1] as { events: Record void>; }; await act(async () => { - latestConfig.events["task:updated"]( - new MessageEvent("task:updated", { - data: JSON.stringify({ id: "FN-4", status: "awaiting-approval", updatedAt: "2026-05-05T10:00:00.000Z" }), + latestConfig.events["approval:requested"]( + new MessageEvent("approval:requested", { + data: JSON.stringify({ id: "approval-1", taskId: "FN-4", updatedAt: "2026-05-05T10:00:00.000Z" }), }), ); }); diff --git a/packages/dashboard/app/components/dashboard/DashboardBanners.tsx b/packages/dashboard/app/components/dashboard/DashboardBanners.tsx index b2d898be4e..a79c3f3a87 100644 --- a/packages/dashboard/app/components/dashboard/DashboardBanners.tsx +++ b/packages/dashboard/app/components/dashboard/DashboardBanners.tsx @@ -20,6 +20,10 @@ import { SetupWarningBanner } from "../SetupWarningBanner"; import { ApprovalNotificationBanner } from "../ApprovalNotificationBanner"; import { GitHubStarPrompt } from "../GitHubStarPrompt"; +function isMailboxApprovalCandidate(candidate: DashboardBannersProps["approvalBannerCandidate"]): boolean { + return candidate?.dedupeKey.startsWith("approval:") === true; +} + export function DashboardBanners({ viewMode, currentProject, @@ -61,6 +65,9 @@ export function DashboardBanners({ markGitHubStarPromptShown, setShowGitHubStarPrompt, }: DashboardBannersProps) { + /* FNXC:DashboardBanners 2026-06-26-00:00: The Open Mailbox approval banner is gated by an approval: candidate from a real ApprovalRequest. The count floor remains only for the approval-SSE/count-refresh race and must not fabricate a mailbox request for task awaiting-approval states. */ + const showMailboxApprovalBanner = isMailboxApprovalCandidate(approvalBannerCandidate); + return ( <> {viewMode === "project" && currentProject && ( @@ -150,7 +157,7 @@ export function DashboardBanners({ onDismiss={handleDismissSetupWarning} /> )} - {viewMode === "project" && currentProject && approvalBannerCandidate && ( + {viewMode === "project" && currentProject && approvalBannerCandidate && showMailboxApprovalBanner && ( handleTaskViewChange("mailbox")} diff --git a/packages/dashboard/app/components/dashboard/__tests__/DashboardBanners.test.tsx b/packages/dashboard/app/components/dashboard/__tests__/DashboardBanners.test.tsx index f3ad6c7cde..e13f511400 100644 --- a/packages/dashboard/app/components/dashboard/__tests__/DashboardBanners.test.tsx +++ b/packages/dashboard/app/components/dashboard/__tests__/DashboardBanners.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from "@testing-library/react"; +import { fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import type { AiSessionSummary } from "../../../api"; import type { ModalManager } from "../../../hooks/useModalManager"; @@ -16,7 +16,14 @@ vi.mock("../../MergeAdvanceNotice", () => ({ default: () => null })); vi.mock("../../TaskIdIntegrityBanner", () => ({ TaskIdIntegrityBanner: () => null })); vi.mock("../../DbCorruptionBanner", () => ({ DbCorruptionBanner: () => null })); vi.mock("../../SetupWarningBanner", () => ({ SetupWarningBanner: () => null })); -vi.mock("../../ApprovalNotificationBanner", () => ({ ApprovalNotificationBanner: () => null })); +vi.mock("../../ApprovalNotificationBanner", () => ({ + ApprovalNotificationBanner: ({ pendingCount, onOpenMailbox }: { pendingCount: number; onOpenMailbox: () => void }) => ( +
+ {pendingCount} approval {pendingCount === 1 ? "request" : "requests"} need your attention + +
+ ), +})); vi.mock("../../GitHubStarPrompt", () => ({ GitHubStarPrompt: () => null })); import { DashboardBanners } from "../DashboardBanners"; @@ -225,3 +232,57 @@ describe("DashboardBanners session notification visibility", () => { expect(querySessionBanner()).not.toBeInTheDocument(); }); }); + +describe("DashboardBanners approval notification visibility", () => { + it("does not render the mailbox approval banner without a real approval candidate", () => { + render(); + + expect(screen.queryByRole("region", { name: /approval requests/i })).not.toBeInTheDocument(); + expect(screen.queryByText(/approval request.*need your attention/i)).not.toBeInTheDocument(); + }); + + it("does not render the mailbox approval banner for a task plan-approval candidate", () => { + render( + , + ); + + expect(screen.queryByRole("region", { name: /approval requests/i })).not.toBeInTheDocument(); + }); + + it("renders a real approval candidate with the mailbox count and CTA", () => { + const handleTaskViewChange = vi.fn(); + render( + , + ); + + expect(screen.getByRole("region", { name: /approval requests/i })).toBeInTheDocument(); + expect(screen.getByText("2 approval requests need your attention")).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: /open mailbox/i })); + expect(handleTaskViewChange).toHaveBeenCalledWith("mailbox"); + }); + + it("keeps the one-request floor only for a real approval SSE/count race", () => { + render( + , + ); + + expect(screen.getByText("1 approval request need your attention")).toBeInTheDocument(); + }); +}); diff --git a/packages/dashboard/app/hooks/__tests__/sseSplitIntegration.test.ts b/packages/dashboard/app/hooks/__tests__/sseSplitIntegration.test.ts index 0e2d2897b3..76564b1475 100644 --- a/packages/dashboard/app/hooks/__tests__/sseSplitIntegration.test.ts +++ b/packages/dashboard/app/hooks/__tests__/sseSplitIntegration.test.ts @@ -40,8 +40,7 @@ describe("SSE split (KTD4): mailbox-refresh vs approval-banner", () => { fetchUnreadCount.mockResolvedValue({ unreadCount: 0 }); }); - it("co-mount keeps the awaiting-approval refresh single-fired and the banner independent", async () => { - const mailboxSpy = vi.fn(); + it("co-mount keeps mailbox counts on approval events and task plan-approval out of the banner", async () => { const tasks: Task[] = []; const onStarPrompt = vi.fn(); @@ -54,7 +53,6 @@ describe("SSE split (KTD4): mailbox-refresh vs approval-banner", () => { currentProjectId: "p1", gitHubStarPromptShown: true, onStarPrompt, - onMailboxRefresh: mailboxSpy, }), ); @@ -76,14 +74,13 @@ describe("SSE split (KTD4): mailbox-refresh vs approval-banner", () => { expect(mailboxSub!.onReconnect).toBeTruthy(); expect(approvalSub!.onReconnect).toBeUndefined(); - // (i) approval:requested sets the banner candidate but does NOT fire - // mailbox-refresh; the mailbox hook's approval:requested handler - // (count refresh) is a distinct function from the banner's. + // (i) approval:requested sets the banner candidate; the mailbox hook's + // approval:requested handler (count refresh) remains a distinct + // function from the banner's. act(() => { approvalSub!.events["approval:requested"]?.(msg({ id: "a1", updatedAt: "2026-01-01T00:00:00Z" })); }); expect(approval.result.current.candidate?.dedupeKey).toBe("approval:a1"); - expect(mailboxSpy).not.toHaveBeenCalled(); expect(mailboxSub!.events["approval:requested"]).toBeTruthy(); expect(mailboxSub!.events["approval:requested"]).not.toBe(approvalSub!.events["approval:requested"]); // (ib) … and the mailbox handler actually refreshes the count (wires to @@ -94,22 +91,19 @@ describe("SSE split (KTD4): mailbox-refresh vs approval-banner", () => { }); expect(fetchUnreadCount).toHaveBeenCalledTimes(refreshCallsBefore + 1); - // (ii) task:updated → awaiting-approval sets the candidate + fires the - // mailbox refresh exactly once. + act(() => { + approval.result.current.dismissApproval(approval.result.current.candidate!); + }); + + // (ii) task:updated → awaiting-approval is plan approval, not mailbox + // approval: no banner candidate and no mailbox-count refresh. + const refreshCallsAfterApproval = fetchUnreadCount.mock.calls.length; act(() => { approvalSub!.events["task:updated"]?.( msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-02T00:00:00Z" }), ); }); - expect(approval.result.current.candidate?.dedupeKey).toBe("task:t1"); - expect(mailboxSpy).toHaveBeenCalledTimes(1); - - // (iii) a second awaiting-approval for the same task is deduped — no second refresh. - act(() => { - approvalSub!.events["task:updated"]?.( - msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-03T00:00:00Z" }), - ); - }); - expect(mailboxSpy).toHaveBeenCalledTimes(1); + expect(approval.result.current.candidate).toBeNull(); + expect(fetchUnreadCount).toHaveBeenCalledTimes(refreshCallsAfterApproval); }); }); diff --git a/packages/dashboard/app/hooks/__tests__/useApprovalBanner.test.ts b/packages/dashboard/app/hooks/__tests__/useApprovalBanner.test.ts index 5d54569bcb..c0a321b0e8 100644 --- a/packages/dashboard/app/hooks/__tests__/useApprovalBanner.test.ts +++ b/packages/dashboard/app/hooks/__tests__/useApprovalBanner.test.ts @@ -18,43 +18,78 @@ import { msg } from "./sseTestHelpers"; const task = (id: string, status: string): Task => ({ id, status, title: id } as Task); +function renderApprovalBannerHook(overrides: Partial[0]> = {}) { + const options: Parameters[0] = { + tasks: [], + currentProjectId: "p1", + gitHubStarPromptShown: true, + onStarPrompt: vi.fn(), + ...overrides, + }; + return renderHook(() => useApprovalBanner(options)); +} + describe("useApprovalBanner", () => { beforeEach(() => { for (const key of Object.keys(handlers)) delete handlers[key]; + window.localStorage.clear(); }); - it("triggers the banner + mailbox refresh when a task enters awaiting-approval", () => { - const onMailboxRefresh = vi.fn(); - const { result } = renderHook(() => - useApprovalBanner({ - tasks: [], - currentProjectId: "p1", - gitHubStarPromptShown: true, - onStarPrompt: vi.fn(), - onMailboxRefresh, - }), - ); + it("does not trigger the mailbox banner when a task enters awaiting-approval", () => { + const { result } = renderApprovalBannerHook(); act(() => { handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-01T00:00:00Z" })); }); - expect(result.current.candidate?.dedupeKey).toBe("task:t1"); - expect(onMailboxRefresh).toHaveBeenCalledTimes(1); + expect(result.current.candidate).toBeNull(); + }); + + it("triggers the banner for a real approval:requested event", () => { + const { result } = renderApprovalBannerHook(); + + act(() => { + handlers["approval:requested"]?.(msg({ id: "a1", updatedAt: "2026-01-01T00:00:00Z" })); + }); + + expect(result.current.candidate).toEqual({ + dedupeKey: "approval:a1", + updatedAtMs: Date.parse("2026-01-01T00:00:00Z"), + }); + }); + + it("ignores approval:requested payloads without an approval request id", () => { + const { result } = renderApprovalBannerHook(); + + act(() => { + handlers["approval:requested"]?.(msg({ taskId: "t1", updatedAt: "2026-01-01T00:00:00Z" })); + }); + + expect(result.current.candidate).toBeNull(); + }); + + it("keeps task plan-approval separate when a real approval is also pending", () => { + const { result } = renderApprovalBannerHook(); + + act(() => { + handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-01T00:00:00Z" })); + }); + expect(result.current.candidate).toBeNull(); + + act(() => { + handlers["approval:requested"]?.(msg({ id: "a1", taskId: "t1", updatedAt: "2026-01-02T00:00:00Z" })); + }); + expect(result.current.candidate?.dedupeKey).toBe("approval:a1"); }); it("fires the star prompt on the first transition to done", () => { const onStarPrompt = vi.fn(); - renderHook(() => - useApprovalBanner({ - // Seed the status map so done is a transition from in-progress. - tasks: [task("t1", "in-progress")], - currentProjectId: "p1", - gitHubStarPromptShown: false, - onStarPrompt, - onMailboxRefresh: vi.fn(), - }), - ); + renderApprovalBannerHook({ + // Seed the status map so done is a transition from in-progress. + tasks: [task("t1", "in-progress")], + gitHubStarPromptShown: false, + onStarPrompt, + }); act(() => { handlers["task:updated"]?.(msg({ id: "t1", status: "done" })); @@ -65,15 +100,11 @@ describe("useApprovalBanner", () => { it("does not star-prompt again once the prompt has been shown", () => { const onStarPrompt = vi.fn(); - renderHook(() => - useApprovalBanner({ - tasks: [task("t1", "in-progress")], - currentProjectId: "p1", - gitHubStarPromptShown: true, - onStarPrompt, - onMailboxRefresh: vi.fn(), - }), - ); + renderApprovalBannerHook({ + tasks: [task("t1", "in-progress")], + gitHubStarPromptShown: true, + onStarPrompt, + }); act(() => { handlers["task:updated"]?.(msg({ id: "t1", status: "done" })); @@ -83,15 +114,7 @@ describe("useApprovalBanner", () => { }); it("dedupes a repeated approval:requested for the same key", () => { - const { result } = renderHook(() => - useApprovalBanner({ - tasks: [], - currentProjectId: "p1", - gitHubStarPromptShown: true, - onStarPrompt: vi.fn(), - onMailboxRefresh: vi.fn(), - }), - ); + const { result } = renderApprovalBannerHook(); act(() => { handlers["approval:requested"]?.(msg({ id: "a1", updatedAt: "2026-01-01T00:00:00Z" })); @@ -102,19 +125,11 @@ describe("useApprovalBanner", () => { handlers["approval:requested"]?.(msg({ id: "a1", updatedAt: "2026-01-02T00:00:00Z" })); }); // Same dedupeKey — candidate stays at the first trigger's value. - expect(result.current.candidate?.dedupeKey).toBe("approval:a1"); + expect(result.current.candidate?.updatedAtMs).toBe(Date.parse("2026-01-01T00:00:00Z")); }); it("dismiss clears the candidate and suppresses re-trigger until a newer timestamp", () => { - const { result } = renderHook(() => - useApprovalBanner({ - tasks: [], - currentProjectId: "p1", - gitHubStarPromptShown: true, - onStarPrompt: vi.fn(), - onMailboxRefresh: vi.fn(), - }), - ); + const { result } = renderApprovalBannerHook(); act(() => { handlers["approval:requested"]?.(msg({ id: "a1", updatedAt: "2026-01-01T00:00:00Z" })); @@ -133,64 +148,4 @@ describe("useApprovalBanner", () => { }); expect(result.current.candidate).toBeNull(); }); - it("re-triggers after leaving and re-entering awaiting-approval (clear-on-leave)", () => { - const onMailboxRefresh = vi.fn(); - const seedTasks: Task[] = [task("t1", "awaiting-approval")]; - const { result } = renderHook(() => - useApprovalBanner({ - tasks: seedTasks, - currentProjectId: "p1", - gitHubStarPromptShown: true, - onStarPrompt: vi.fn(), - onMailboxRefresh, - }), - ); - - // The seeded awaiting-approval task is already in the seen set, so a repeat - // event for it must NOT trigger. - act(() => { - handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-01T00:00:00Z" })); - }); - expect(result.current.candidate).toBeNull(); - expect(onMailboxRefresh).not.toHaveBeenCalled(); - - // Task leaves awaiting-approval → the seen-key for t1 is cleared. - act(() => { - handlers["task:updated"]?.(msg({ id: "t1", status: "approved", updatedAt: "2026-01-02T00:00:00Z" })); - }); - expect(result.current.candidate).toBeNull(); - - // Re-entering awaiting-approval re-triggers the candidate + mailbox refresh. - act(() => { - handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-03T00:00:00Z" })); - }); - expect(result.current.candidate?.dedupeKey).toBe("task:t1"); - expect(onMailboxRefresh).toHaveBeenCalledTimes(1); - }); - - it("dedupes mailbox refresh on a repeated awaiting-approval task:updated", () => { - const onMailboxRefresh = vi.fn(); - const tasks: Task[] = []; - const { result } = renderHook(() => - useApprovalBanner({ - tasks, - currentProjectId: "p1", - gitHubStarPromptShown: true, - onStarPrompt: vi.fn(), - onMailboxRefresh, - }), - ); - - act(() => { - handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-01T00:00:00Z" })); - }); - expect(result.current.candidate?.dedupeKey).toBe("task:t1"); - expect(onMailboxRefresh).toHaveBeenCalledTimes(1); - - // A second awaiting-approval for the same task is suppressed by seenApprovalKeys. - act(() => { - handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-04T00:00:00Z" })); - }); - expect(onMailboxRefresh).toHaveBeenCalledTimes(1); - }); }); diff --git a/packages/dashboard/app/hooks/useApprovalBanner.ts b/packages/dashboard/app/hooks/useApprovalBanner.ts index 4104c0ae65..714ab025f1 100644 --- a/packages/dashboard/app/hooks/useApprovalBanner.ts +++ b/packages/dashboard/app/hooks/useApprovalBanner.ts @@ -1,9 +1,12 @@ /* FNXC:ApprovalBanner 2026-06-24-00:00: -Approval-notification banner dedupe/dismiss state machine, driven by task:updated and approval:requested SSE events. Also fires the first-completed-task GitHub-star prompt and a mailbox-count refresh when a task enters awaiting-approval — preserving the former single-subscriber side effects via the onStarPrompt / onMailboxRefresh callbacks. Extracted from AppInner. +Approval-notification banner dedupe/dismiss state machine, driven by task:updated and approval:requested SSE events. Also fires the first-completed-task GitHub-star prompt. Extracted from AppInner. FNXC:ApprovalBanner 2026-06-24-00:00: Stale-closure / effect-identity hazard: the per-`tasks` ref-sync effect rebuilds the status + seen-key maps on every tasks change, and the dismissal-timestamp comparison (`updatedAtMs <= dismissedAt`) suppresses re-trigger. Preserve both exactly when touching this hook (see docs/solutions ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation and logic-errors/queued-chat-message-flush-trusts-stale-isgenerating). + +FNXC:ApprovalBanner 2026-06-26-00:00: +The mailbox approval banner represents only real ApprovalRequest rows delivered by approval:requested SSE with approval: dedupe keys. Task awaiting-approval is a plan-approval lifecycle state surfaced on the triage board and must not create an Open Mailbox banner or refresh mailbox counts. */ import { useCallback, useEffect, useRef, useState } from "react"; @@ -11,7 +14,6 @@ import type { Task } from "@fusion/core"; import { subscribeSse } from "../sse-bus"; import { type ApprovalBannerCandidate, - didEnterAwaitingApproval, didEnterDone, loadApprovalBannerDismissals, parseDateMs, @@ -24,8 +26,6 @@ export interface UseApprovalBannerOptions { gitHubStarPromptShown: boolean; /** Invoked when a task first transitions to done (drives the GitHub-star prompt). */ onStarPrompt: () => void; - /** Invoked when a task enters awaiting-approval (drives a mailbox-count refresh). */ - onMailboxRefresh: () => void; } export interface UseApprovalBannerResult { @@ -38,7 +38,6 @@ export function useApprovalBanner({ currentProjectId, gitHubStarPromptShown, onStarPrompt, - onMailboxRefresh, }: UseApprovalBannerOptions): UseApprovalBannerResult { const [candidate, setCandidate] = useState(null); const taskStatusByIdRef = useRef>(new Map()); @@ -50,9 +49,6 @@ export function useApprovalBanner({ const nextSeen = new Set(); for (const task of tasks) { next.set(task.id, task.status); - if (task.status === "awaiting-approval") { - nextSeen.add(`task:${task.id}`); - } } taskStatusByIdRef.current = next; seenApprovalKeysRef.current = nextSeen; @@ -83,7 +79,7 @@ export function useApprovalBanner({ updatedAt?: string; createdAt?: string; }; - const dedupeKey = payload.id ? `approval:${payload.id}` : payload.taskId ? `task:${payload.taskId}` : undefined; + const dedupeKey = payload.id ? `approval:${payload.id}` : undefined; if (!dedupeKey || seenApprovalKeysRef.current.has(dedupeKey)) { return; } @@ -102,36 +98,18 @@ export function useApprovalBanner({ if (!payload?.id) { return; } - const dedupeKey = `task:${payload.id}`; const previousStatus = taskStatusByIdRef.current.get(payload.id); taskStatusByIdRef.current.set(payload.id, payload.status); if (!gitHubStarPromptShown && didEnterDone(payload.status, previousStatus)) { onStarPrompt(); } - if (payload.status !== "awaiting-approval") { - seenApprovalKeysRef.current.delete(dedupeKey); - approvalDismissalsRef.current.delete(dedupeKey); - persistApprovalBannerDismissals(approvalDismissalsRef.current); - return; - } - if (seenApprovalKeysRef.current.has(dedupeKey)) { - return; - } - if (didEnterAwaitingApproval(payload.status, previousStatus)) { - seenApprovalKeysRef.current.add(dedupeKey); - triggerApprovalBanner({ - dedupeKey, - updatedAtMs: parseDateMs(payload.updatedAt), - }); - onMailboxRefresh(); - } } catch { // no-op } }, }, }); - }, [currentProjectId, gitHubStarPromptShown, onStarPrompt, onMailboxRefresh]); + }, [currentProjectId, gitHubStarPromptShown, onStarPrompt]); const dismissApproval = useCallback((dismissed: ApprovalBannerCandidate) => { approvalDismissalsRef.current.set( diff --git a/packages/dashboard/app/hooks/useMailboxUnread.ts b/packages/dashboard/app/hooks/useMailboxUnread.ts index b4421b7ddb..c9e938aba0 100644 --- a/packages/dashboard/app/hooks/useMailboxUnread.ts +++ b/packages/dashboard/app/hooks/useMailboxUnread.ts @@ -1,6 +1,9 @@ /* FNXC:MailboxBadge 2026-06-24-00:00: -Header/mobile-nav unread + pending-approval counts for the mailbox, refreshed on message and approval SSE events. Extracted from AppInner; exposes `refresh` (so the approval-banner hook can re-fetch counts when a task enters awaiting-approval, preserving the former single-subscriber side effect) and `setMailboxUnreadCount` (MailboxView reports its own count changes through onUnreadCountChange). +Header/mobile-nav unread + pending-approval counts for the mailbox, refreshed on message and approval SSE events. Extracted from AppInner; exposes `refresh` for reconnect/SSE-driven count refresh and `setMailboxUnreadCount` because MailboxView reports its own count changes through onUnreadCountChange. + +FNXC:MailboxBadge 2026-06-26-00:00: +Pending approval counts are mailbox-only and refresh from approval:* events backed by ApprovalRequest rows. Task awaiting-approval transitions must not refresh or inflate these mailbox counts. */ import { useCallback, useEffect, useState } from "react";