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:<id> 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) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7096-approval-banner-mailbox.md
Normal file
7
.changeset/fn-7096-approval-banner-mailbox.md
Normal file
@@ -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.
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<string, unknown> })?.events?.["task:updated"] === "function",
|
||||
);
|
||||
const subscriptionConfig = mailboxSubscriptionCall?.[1] as {
|
||||
const subscriptionConfig = approvalSubscriptionCall?.[1] as {
|
||||
events: Record<string, (event: MessageEvent) => 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<string, unknown> })?.events?.["task:updated"] === "function",
|
||||
const approvalSubscriptionCall = mockSubscribeSse.mock.calls.find(
|
||||
([url, sub]) => String(url).startsWith("/api/events")
|
||||
&& typeof (sub as { events?: Record<string, unknown> })?.events?.["approval:requested"] === "function"
|
||||
&& typeof (sub as { events?: Record<string, unknown> })?.events?.["task:updated"] === "function",
|
||||
);
|
||||
const subscriptionConfig = mailboxSubscriptionCall?.[1] as {
|
||||
const subscriptionConfig = approvalSubscriptionCall?.[1] as {
|
||||
events: Record<string, (event: MessageEvent) => 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<string, unknown> })?.events?.["task:updated"] === "function");
|
||||
.find(([, sub]) => typeof (sub as { events?: Record<string, unknown> })?.events?.["approval:requested"] === "function"
|
||||
&& typeof (sub as { events?: Record<string, unknown> })?.events?.["task:updated"] === "function");
|
||||
const latestConfig = latestSubscription?.[1] as {
|
||||
events: Record<string, (event: MessageEvent) => 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" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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:<id> 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 && (
|
||||
<ApprovalNotificationBanner
|
||||
pendingCount={Math.max(mailboxPendingApprovalCount, 1)}
|
||||
onOpenMailbox={() => handleTaskViewChange("mailbox")}
|
||||
|
||||
@@ -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 }) => (
|
||||
<section role="region" aria-label="Approval requests">
|
||||
<span>{pendingCount} approval {pendingCount === 1 ? "request" : "requests"} need your attention</span>
|
||||
<button type="button" onClick={onOpenMailbox}>Open Mailbox</button>
|
||||
</section>
|
||||
),
|
||||
}));
|
||||
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(<DashboardBanners {...buildProps({ mailboxPendingApprovalCount: 0, approvalBannerCandidate: null })} />);
|
||||
|
||||
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(
|
||||
<DashboardBanners
|
||||
{...buildProps({
|
||||
mailboxPendingApprovalCount: 0,
|
||||
approvalBannerCandidate: { dedupeKey: "task:t1", updatedAtMs: Date.parse("2026-01-01T00:00:00Z") },
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(
|
||||
<DashboardBanners
|
||||
{...buildProps({
|
||||
mailboxPendingApprovalCount: 2,
|
||||
approvalBannerCandidate: { dedupeKey: "approval:a1", updatedAtMs: Date.parse("2026-01-01T00:00:00Z") },
|
||||
handleTaskViewChange,
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(
|
||||
<DashboardBanners
|
||||
{...buildProps({
|
||||
mailboxPendingApprovalCount: 0,
|
||||
approvalBannerCandidate: { dedupeKey: "approval:a1", updatedAtMs: Date.parse("2026-01-01T00:00:00Z") },
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText("1 approval request need your attention")).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<Parameters<typeof useApprovalBanner>[0]> = {}) {
|
||||
const options: Parameters<typeof useApprovalBanner>[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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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:<id> 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<ApprovalBannerCandidate | null>(null);
|
||||
const taskStatusByIdRef = useRef<Map<string, string | undefined>>(new Map());
|
||||
@@ -50,9 +49,6 @@ export function useApprovalBanner({
|
||||
const nextSeen = new Set<string>();
|
||||
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(
|
||||
|
||||
@@ -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";
|
||||
|
||||
Reference in New Issue
Block a user