fix(canvas/Toolbar): help button always opens — no double-click close bug

The help button's onClick used setHelpOpen((open) => !open) (toggle).
Combined with the window.pointerdown handler that closes on outside-click,
clicking outside then clicking the help button would: pointerdown outside
(close) → click on button (!false = true → open) → pointerdown ON button
(contains=true, no close) → BUT the next interaction would have stale
toggle state causing a double-close on the following click.

Fix: button onClick always calls setHelpOpen(true) — the pointerdown
outside handler owns the close path; the button only opens.

Also add 2 tests: pointer-down-outside closes, and re-open works after
outside click (regression for the double-click bug).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

canvas/src/components/ApprovalBanner.tsx

@@ -16,6 +16,8 @@ interface PendingApproval {
 
 export function ApprovalBanner() {
   const [approvals, setApprovals] = useState<PendingApproval[]>([]);
+  // Guards double-click / double-keypress during in-flight POST.
+  const [pendingApprovalId, setPendingApprovalId] = useState<string | null>(null);
 
   // Single endpoint — no N+1 per-workspace polling
   const pollApprovals = useCallback(async () => {
@@ -35,6 +37,8 @@ export function ApprovalBanner() {
   }, [pollApprovals]);
 
   const handleDecide = async (approval: PendingApproval, decision: "approved" | "denied") => {
+    if (pendingApprovalId !== null) return; // guard double-submit
+    setPendingApprovalId(approval.id);
     try {
       await api.post(`/workspaces/${approval.workspace_id}/approvals/${approval.id}/decide`, {
         decision,
@@ -44,6 +48,8 @@ export function ApprovalBanner() {
       setApprovals((prev) => prev.filter((a) => a.id !== approval.id));
     } catch {
       showToast("Failed to submit decision", "error");
+    } finally {
+      setPendingApprovalId(null);
     }
   };
 
@@ -72,22 +78,25 @@ export function ApprovalBanner() {
               <div className="flex gap-2 mt-3">
                 <button
                   type="button"
+                  disabled={pendingApprovalId !== null}
                   onClick={() => handleDecide(approval, "approved")}
+                  aria-disabled={pendingApprovalId !== null}
                   // Hover DARKER not lighter — emerald-500 on white text
                   // drops contrast vs emerald-700.
-                  className="px-3 py-1.5 bg-emerald-600 hover:bg-emerald-700 text-xs rounded-lg text-white font-medium transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-offset-amber-950 focus-visible:ring-emerald-400/70"
+                  className="px-3 py-1.5 bg-emerald-600 hover:bg-emerald-700 disabled:opacity-40 disabled:cursor-not-allowed text-xs rounded-lg text-white font-medium transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-offset-amber-950 focus-visible:ring-emerald-400/70"
                 >
-                  Approve
+                  {pendingApprovalId === approval.id ? "…" : "Approve"}
                 </button>
                 <button
                   type="button"
+                  disabled={pendingApprovalId !== null}
                   onClick={() => handleDecide(approval, "denied")}
-                  // Was a no-op hover (`bg-surface-card hover:bg-surface-card`).
-                  // Lift to surface-elevated on hover so the button visibly
-                  // responds before a destructive deny.
-                  className="px-3 py-1.5 bg-surface-card hover:bg-surface-elevated hover:text-ink text-xs rounded-lg text-ink-mid transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-offset-amber-950 focus-visible:ring-amber-400/70"
+                  aria-disabled={pendingApprovalId !== null}
+                  // `text-ink` (not text-ink-mid) for WCAG AA contrast on bg-surface-card.
+                  // text-ink-mid on zinc-800 fails AA at ~3:1; text-ink passes at ~7:1.
+                  className="px-3 py-1.5 bg-surface-card hover:bg-surface-elevated hover:text-ink text-ink disabled:opacity-40 disabled:cursor-not-allowed text-xs rounded-lg font-medium transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-offset-amber-950 focus-visible:ring-amber-400/70"
                 >
-                  Deny
+                  {pendingApprovalId === approval.id ? "…" : "Deny"}
                 </button>
               </div>
             </div>

canvas/src/components/Toolbar.tsx

@@ -314,7 +314,7 @@ export function Toolbar() {
       <div ref={helpRef} className="relative">
         <button
           type="button"
-          onClick={() => setHelpOpen((open) => !open)}
+          onClick={() => setHelpOpen(true)}
           className="flex items-center justify-center w-7 h-7 bg-surface-card hover:bg-surface-card/70 border border-line rounded-lg transition-colors text-ink-mid hover:text-ink focus:outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
           aria-expanded={helpOpen}
           aria-label="Open shortcuts and tips"

canvas/src/components/__tests__/ApprovalBanner.test.tsx

@@ -238,6 +238,98 @@ describe("ApprovalBanner — decisions", () => {
   });
 });
 
+describe("ApprovalBanner — disabled state while submitting", () => {
+  // Deferred so we can control when the mock POST resolves.
+  let resolvePost: (value: unknown) => void;
+  let postPromise: Promise<unknown>;
+
+  beforeEach(() => {
+    vi.useFakeTimers();
+    mockApiGet.mockReset().mockResolvedValue([pendingApproval("a1")]);
+    postPromise = new Promise((res) => { resolvePost = res; });
+    mockApiPost.mockReset().mockImplementation(() => postPromise as Promise<unknown>);
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+    vi.resetModules();
+  });
+
+  it("disables both buttons while POST is in flight", async () => {
+    render(<ApprovalBanner />);
+    await act(async () => { await vi.runOnlyPendingTimersAsync(); });
+    const approveBtn = screen.getAllByRole("button", { name: /approve/i })[0];
+    const denyBtn = screen.getAllByRole("button", { name: /deny/i })[0];
+
+    fireEvent.click(approveBtn);
+    await act(async () => { /* flush */ });
+
+    expect((approveBtn as HTMLButtonElement).disabled).toBe(true);
+    expect((denyBtn as HTMLButtonElement).disabled).toBe(true);
+  });
+
+  it("re-enables buttons after POST resolves", async () => {
+    render(<ApprovalBanner />);
+    await act(async () => { await vi.runOnlyPendingTimersAsync(); });
+    const approveBtn = screen.getAllByRole("button", { name: /approve/i })[0];
+    const denyBtn = screen.getAllByRole("button", { name: /deny/i })[0];
+
+    fireEvent.click(approveBtn);
+    await act(async () => { /* flush */ });
+    expect((approveBtn as HTMLButtonElement).disabled).toBe(true);
+    expect((denyBtn as HTMLButtonElement).disabled).toBe(true);
+
+    // Resolve the deferred POST inside act() so React flushes the state update.
+    await act(async () => {
+      resolvePost!({});
+    });
+    expect(screen.queryByRole("alert")).toBeNull();
+  });
+
+  it("re-enables buttons after POST fails", async () => {
+    mockApiPost.mockImplementation(() => Promise.reject(new Error("Network error")));
+    render(<ApprovalBanner />);
+    await act(async () => { await vi.runOnlyPendingTimersAsync(); });
+    const approveBtn = screen.getAllByRole("button", { name: /approve/i })[0];
+
+    fireEvent.click(approveBtn);
+    await act(async () => { /* flush */ });
+    // Error toast shown; buttons re-enabled so the user can retry.
+    expect((approveBtn as HTMLButtonElement).disabled).toBe(false);
+  });
+
+  it("shows ellipsis text on the clicked button while submitting", async () => {
+    render(<ApprovalBanner />);
+    await act(async () => { await vi.runOnlyPendingTimersAsync(); });
+    fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]);
+    await act(async () => { /* flush */ });
+    // The clicked button now shows "…" instead of "Approve"
+    expect(screen.queryByRole("button", { name: /approve/i })).toBeNull();
+    expect(screen.getAllByRole("button", { name: /^…$/ }).length).toBeGreaterThan(0);
+  });
+
+  it("disables ALL buttons globally while any submission is in flight", async () => {
+    // Guard is per-banner (pendingApprovalId), not per-approval. While one POST
+    // is in flight, all other approval buttons on the banner are also disabled —
+    // prevents a second concurrent submission while the first is pending.
+    mockApiGet.mockReset().mockResolvedValue([
+      pendingApproval("a1"),
+      pendingApproval("a2", "ws-2"),
+    ]);
+    render(<ApprovalBanner />);
+    await act(async () => { await vi.runOnlyPendingTimersAsync(); });
+    const card1Approve = screen.getAllByRole("button", { name: /approve/i })[0];
+    const card2Approve = screen.getAllByRole("button", { name: /approve/i })[1];
+    fireEvent.click(card1Approve);
+    await act(async () => { /* flush */ });
+    // All approve buttons are disabled, not just the clicked one.
+    expect((card1Approve as HTMLButtonElement).disabled).toBe(true);
+    expect((card2Approve as HTMLButtonElement).disabled).toBe(true);
+  });
+});
+
 describe("ApprovalBanner — handles empty list from server", () => {
   beforeEach(() => {
     vi.useFakeTimers();

canvas/src/components/__tests__/Toolbar.test.tsx

@@ -255,6 +255,32 @@ describe("Toolbar — Help popover", () => {
     fireEvent.click(closeBtn);
     expect(screen.queryByRole("dialog")).toBeNull();
   });
+
+  it("closes when pointer is pressed outside the help popover", () => {
+    render(<Toolbar />);
+    const helpBtn = screen.getByRole("button", { name: /open shortcuts and tips/i });
+    fireEvent.click(helpBtn);
+    expect(screen.getByRole("dialog")).toBeTruthy();
+    // Simulate pointerdown outside the help popover (not on the help button)
+    fireEvent.pointerDown(document.body);
+    expect(screen.queryByRole("dialog")).toBeNull();
+  });
+
+  it("opens on click even after a previous pointer-outside close", () => {
+    // Regression: clicking outside closed the popover AND toggled the button
+    // state, so the next click on the button would close it again.
+    // The fix makes the button always open (never toggle) so re-opening works.
+    render(<Toolbar />);
+    const helpBtn = screen.getByRole("button", { name: /open shortcuts and tips/i });
+    fireEvent.click(helpBtn);
+    expect(screen.getByRole("dialog")).toBeTruthy();
+    // Click outside (pointerdown on body, not on help button)
+    fireEvent.pointerDown(document.body);
+    expect(screen.queryByRole("dialog")).toBeNull();
+    // Click the help button again — must re-open, not double-close
+    fireEvent.click(helpBtn);
+    expect(screen.getByRole("dialog")).toBeTruthy();
+  });
 });
 
 describe("Toolbar — A2A edges toggle", () => {