canvas/src/components/mobile/MobileInbox.tsx

@@ -33,6 +33,11 @@ export function MobileInbox({ dark }: { dark: boolean }) {
   const [items, setItems] = useState<RequestRow[]>([]);
   const [loading, setLoading] = useState(true);
   const [acting, setActing] = useState<string | null>(null);
+  // Audit F1: track backend fetch failure distinctly so the list can render
+  // a retry affordance instead of the silent "No pending approvals" empty
+  // state. A genuine backend outage during a destructive-approvals review
+  // must NOT look like a clean inbox.
+  const [loadError, setLoadError] = useState<string | null>(null);
 
   // Responder identity — the same resolution RequestsInbox uses (session
   // user_id, "admin" placeholder when unauthenticated).
@@ -49,22 +54,42 @@ export function MobileInbox({ dark }: { dark: boolean }) {
   // in-flight fetch for the old kind must not overwrite the list after the
   // new tab's load clears it (CR2 #11478).
   const loadSeqRef = useRef(0);
+  // Audit F3: track rows the user has just optimistically acted on. A
+  // REQUEST_RESPONDED WS event fired by OUR server (in response to our own
+  // POST) can race with the optimistic removal — the WS-triggered load()
+  // may re-fetch a list that still contains the row (the POST hasn't
+  // landed server-side yet), and a naive setItems would briefly re-render
+  // the just-approved row (the "flicker" report). Filtering the just-acted
+  // set on load() return closes the race; the set is cleared on POST
+  // completion (success drops it because the server no longer returns the
+  // row; failure drops it because the catch re-loads from server truth).
+  const justActedRef = useRef<Set<string>>(new Set());
   const load = useCallback(() => {
     const seq = ++loadSeqRef.current;
     // core#2766 / CR2 #11478: clear stale rows the moment the kind changes
     // so the user never sees approval cards under the Tasks tab (or
     // vice-versa) while the new list is still fetching.
     setItems([]);
+    setLoadError(null);
     setLoading(true);
     api
       .get<RequestRow[]>(`/requests/pending?kind=${kind}`)
       .then((rows) => {
         if (seq !== loadSeqRef.current) return; // stale response
-        setItems(Array.isArray(rows) ? rows : []);
+        const arr = Array.isArray(rows) ? rows : [];
+        // F3: filter out rows the user has just acted on so a racing
+        // WS-triggered load doesn't re-surface them in the brief window
+        // between optimistic-removal and POST completion.
+        setItems(arr.filter((r) => !justActedRef.current.has(r.id)));
       })
       .catch(() => {
         if (seq !== loadSeqRef.current) return; // stale response
         setItems([]);
+        // F1: surface the failure distinctly. The render path below
+        // shows a retry affordance + role="alert" instead of the
+        // "No pending approvals" copy that would falsely suggest a
+        // clean inbox during a backend outage.
+        setLoadError("Could not load pending requests.");
       })
       .finally(() => {
         if (seq !== loadSeqRef.current) return; // stale response
@@ -82,8 +107,11 @@ export function MobileInbox({ dark }: { dark: boolean }) {
     async (r: RequestRow, action: "done" | "rejected" | "approved") => {
       if (acting) return;
       setActing(r.id);
-      // Optimistic: drop the row immediately; restore on failure.
-      const prev = items;
+      // Optimistic: drop the row immediately. The server is the source of
+      // truth, so on failure we re-load() rather than restoring a stale
+      // `items` snapshot (Audit F2: a `setItems(prev)` snapshot would wipe
+      // any rows that arrived via WS during the in-flight POST).
+      justActedRef.current.add(r.id);
       setItems((cur) => cur.filter((x) => x.id !== r.id));
       try {
         await api.post(`/requests/${r.id}/respond`, {
@@ -91,13 +119,22 @@ export function MobileInbox({ dark }: { dark: boolean }) {
           responder_type: "user",
           responder_id: responderIdRef.current,
         });
+        // F3: POST succeeded — the server now treats r as responded, so
+        // future loads won't return it. Drop from justActedRef.
+        justActedRef.current.delete(r.id);
       } catch {
-        setItems(prev); // restore on failure
+        // F2: re-fetch from server truth (preserves any rows that arrived
+        // via WS during the in-flight POST — restoring a stale `items`
+        // snapshot here would wipe them). Also drops the row from
+        // justActedRef so the server's re-fetched list (with the row
+        // still pending) is rendered as-is.
+        justActedRef.current.delete(r.id);
+        load();
       } finally {
         setActing(null);
       }
     },
-    [acting, items],
+    [acting, load],
   );
 
   const subTabs: { id: InboxKind; label: string }[] = useMemo(
@@ -154,6 +191,44 @@ export function MobileInbox({ dark }: { dark: boolean }) {
       <div style={{ flex: 1, overflowY: "auto", padding: "12px 16px", display: "flex", flexDirection: "column", gap: 10 }}>
         {loading && items.length === 0 ? (
           <div style={{ color: p.text3, fontSize: 13, textAlign: "center", marginTop: 40 }}>Loading…</div>
+        ) : loadError && items.length === 0 ? (
+          // F1: backend fetch failure renders a distinct error/retry state
+          // (NOT the silent "No pending approvals" empty copy). Critical for
+          // destructive-approvals review — a clean-looking inbox during a
+          // backend outage would let destructive actions go unreviewed.
+          <div
+            role="alert"
+            data-testid="inbox-load-error"
+            style={{
+              display: "flex",
+              flexDirection: "column",
+              alignItems: "center",
+              gap: 10,
+              marginTop: 40,
+              padding: "0 16px",
+            }}
+          >
+            <div style={{ color: p.failed, fontSize: 13, textAlign: "center" }}>{loadError}</div>
+            <button
+              type="button"
+              onClick={load}
+              aria-label="Retry loading pending requests"
+              data-testid="inbox-retry"
+              style={{
+                padding: "6px 14px",
+                borderRadius: 14,
+                border: `0.5px solid ${p.failed}`,
+                background: "transparent",
+                color: p.failed,
+                fontSize: 12,
+                fontWeight: 600,
+                cursor: "pointer",
+                fontFamily: MOBILE_FONT_SANS,
+              }}
+            >
+              Retry
+            </button>
+          </div>
         ) : items.length === 0 ? (
           <div style={{ color: p.text3, fontSize: 13, textAlign: "center", marginTop: 40 }}>
             {kind === "approval"

canvas/src/components/mobile/__tests__/MobileInbox.test.tsx

@@ -15,6 +15,7 @@ vi.mock("@/lib/auth", () => ({ fetchSession: vi.fn().mockResolvedValue({ user_id
 vi.mock("@/hooks/useSocketEvent", () => ({ useSocketEvent: vi.fn() }));
 
 import { api } from "@/lib/api";
+import { useSocketEvent } from "@/hooks/useSocketEvent";
 import type { RequestRow } from "@/components/concierge/RequestsInbox";
 import { MobileInbox } from "../MobileInbox";
 
@@ -105,4 +106,129 @@ describe("MobileInbox", () => {
       expect(getByText(/No pending tasks/i)).toBeTruthy();
     });
   });
+
+  // ─────────────────────────────────────────────────────────────────────
+  // Audit F1 (HIGH): MobileInbox.tsx:155-162 — backend fetch FAILURE must
+  // render a distinct error/retry state, NOT the silent "No pending
+  // approvals" empty copy. Critical for destructive-approvals review:
+  // a clean-looking inbox during a backend outage would let destructive
+  // actions go unreviewed.
+  // ─────────────────────────────────────────────────────────────────────
+  it("F1: backend fetch failure renders error/retry, not silent 'No pending approvals'", async () => {
+    vi.mocked(api.get).mockRejectedValue(new Error("network down"));
+    const { getByTestId, queryByText } = render(<MobileInbox dark={false} />);
+
+    // The error state + retry button must render.
+    await waitFor(() => {
+      expect(getByTestId("inbox-load-error")).toBeTruthy();
+    });
+    expect(getByTestId("inbox-retry")).toBeTruthy();
+
+    // The silent "No pending approvals" copy MUST NOT render during a
+    // backend fetch failure — that's the bug F1 fixes.
+    expect(queryByText(/No pending approvals/i)).toBeNull();
+
+    // The retry button must call load() — second GET also rejects, but
+    // we assert the click is wired (no throw, error state persists).
+    await act(async () => {
+      fireEvent.click(getByTestId("inbox-retry"));
+    });
+    await waitFor(() => {
+      expect(getByTestId("inbox-load-error")).toBeTruthy();
+    });
+  });
+
+  // ─────────────────────────────────────────────────────────────────────
+  // Audit F2 (HIGH): MobileInbox.tsx:81-101 — `respond` prev-closure must
+  // NOT wipe a row that arrived via WS during the in-flight POST on
+  // failure. The server is the source of truth, so the fix re-loads from
+  // the server rather than restoring a stale `items` snapshot.
+  // ─────────────────────────────────────────────────────────────────────
+  it("F2: POST failure re-loads from server (preserves a row that arrived via WS mid-POST)", async () => {
+    const rowA: RequestRow = { ...approval, id: "req-A", title: "Row A: delete prod secret?" };
+    const rowB: RequestRow = { ...approval, id: "req-B", title: "Row B: rotate prod key?" };
+
+    // First GET (initial load): server returns [A]. The user clicks
+    // Approve on A; the POST fails. The post-failure re-load (the
+    // second GET) simulates the server returning [A, B] — A is still
+    // pending (POST failed), and B arrived via WS during the in-flight
+    // POST. The OLD snapshot-restore would wipe B; the new load()
+    // re-fetch must preserve it.
+    let getCallCount = 0;
+    vi.mocked(api.get).mockImplementation(() => {
+      getCallCount++;
+      if (getCallCount === 1) return Promise.resolve([rowA]);
+      return Promise.resolve([rowA, rowB]);
+    });
+    vi.mocked(api.post).mockRejectedValue(new Error("server down"));
+
+    const { getByText } = render(<MobileInbox dark={false} />);
+    await waitFor(() => expect(getByText("Row A: delete prod secret?")).toBeTruthy());
+
+    // User clicks Approve on A → optimistic removal + POST in flight.
+    await act(async () => {
+      fireEvent.click(getByText("Approve"));
+    });
+
+    // After the post-failure re-load, BOTH A (server still has it) AND
+    // B (WS-arrived during the in-flight POST) must be visible. B
+    // surviving is the load-bearing assertion for F2.
+    await waitFor(() => {
+      expect(getByText("Row B: rotate prod key?")).toBeTruthy();
+    });
+    expect(getByText("Row A: delete prod secret?")).toBeTruthy();
+  });
+
+  // ─────────────────────────────────────────────────────────────────────
+  // Audit F3 (HIGH): MobileInbox.tsx:77-79 — WS REQUEST_RESPONDED fired
+  // during the optimistic-removal / in-flight-POST window must NOT cause
+  // the just-acted row to briefly reappear (flicker). The fix filters
+  // justActedRef out of the load() response.
+  // ─────────────────────────────────────────────────────────────────────
+  it("F3: WS REQUEST_RESPONDED during optimistic removal does not flicker the row", async () => {
+    // Capture the useSocketEvent callback so we can fire a synthetic
+    // WS event from the test.
+    let socketCb: ((msg: { event?: string }) => void) | null = null;
+    vi.mocked(useSocketEvent).mockImplementation((cb) => {
+      socketCb = cb as typeof socketCb;
+    });
+
+    const rowA: RequestRow = { ...approval, id: "req-A", title: "Row A: delete prod secret?" };
+    vi.mocked(api.get).mockResolvedValue([rowA]);
+    // Hold the POST open so the WS race has a window to fire in.
+    const postDeferred = deferred<unknown>();
+    vi.mocked(api.post).mockReturnValue(postDeferred.promise as Promise<unknown>);
+
+    const { getByText, queryByText } = render(<MobileInbox dark={false} />);
+    await waitFor(() => expect(getByText("Row A: delete prod secret?")).toBeTruthy());
+
+    // User clicks Approve → optimistic removal; POST is in flight.
+    await act(async () => {
+      fireEvent.click(getByText("Approve"));
+    });
+    await waitFor(() => expect(queryByText("Row A: delete prod secret?")).toBeNull());
+
+    // Race: a WS REQUEST_RESPONDED fires before the POST lands. The
+    // load() triggered by it must re-fetch (the server still has A as
+    // pending because the POST hasn't returned). The OLD code would
+    // re-surface A → flicker. The NEW code filters justActedRef so A
+    // stays gone.
+    vi.mocked(api.get).mockResolvedValueOnce([rowA]);
+    await act(async () => {
+      socketCb?.({ event: "REQUEST_RESPONDED" });
+    });
+    // Give the re-fetch + setItems microtask queue a tick to settle.
+    await act(async () => {
+      await Promise.resolve();
+    });
+
+    // Critical: A must NOT have reappeared. Flicker prevented.
+    expect(queryByText("Row A: delete prod secret?")).toBeNull();
+
+    // Now resolve the POST (success) — A is permanently responded.
+    await act(async () => {
+      postDeferred.resolve({});
+    });
+    expect(queryByText("Row A: delete prod secret?")).toBeNull();
+  });
 });