fix(mobile-inbox): ignore stale fetches after tab switch (core#2766 / CR2 #11478) #2804

Closed
agent-dev-a wants to merge 1 commits from fix/2766-mobile-inbox-stale-action into main
2 changed files with 76 additions and 9 deletions
+23 -5
View File
@@ -45,13 +45,31 @@ export function MobileInbox({ dark }: { dark: boolean }) {
return () => { cancelled = true; };
}, []);
// Guard against stale fetches: when the user switches tabs, a previous
// 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);
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([]);
setLoading(true);
api
.get<RequestRow[]>(`/requests/pending?kind=${kind}`)
.then((rows) => setItems(Array.isArray(rows) ? rows : []))
.catch(() => setItems([]))
.finally(() => setLoading(false));
.then((rows) => {
if (seq !== loadSeqRef.current) return; // stale response
setItems(Array.isArray(rows) ? rows : []);
})
.catch(() => {
if (seq !== loadSeqRef.current) return; // stale response
setItems([]);
})
.finally(() => {
if (seq !== loadSeqRef.current) return; // stale response
setLoading(false);
});
}, [kind]);
useEffect(() => { load(); }, [load]);
@@ -169,7 +187,7 @@ export function MobileInbox({ dark }: { dark: boolean }) {
<button
type="button"
disabled={acting === r.id}
onClick={() => respond(r, kind === "approval" ? "approved" : "done")}
onClick={() => respond(r, r.kind === "approval" ? "approved" : "done")}
style={{
flex: 1,
padding: "9px 0",
@@ -184,7 +202,7 @@ export function MobileInbox({ dark }: { dark: boolean }) {
fontFamily: MOBILE_FONT_SANS,
}}
>
{kind === "approval" ? "Approve" : "Done"}
{r.kind === "approval" ? "Approve" : "Done"}
</button>
<button
type="button"
@@ -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 type { RequestRow } from "@/components/concierge/RequestsInbox";
import { MobileInbox } from "../MobileInbox";
const approval = {
@@ -30,6 +31,16 @@ beforeEach(() => {
vi.mocked(api.post).mockResolvedValue({});
});
function deferred<T = unknown>() {
let resolve: (value: T) => void = () => {};
let reject: (reason?: unknown) => void = () => {};
const promise = new Promise<T>((res, rej) => {
resolve = res;
reject = rej;
});
return { promise, resolve, reject };
}
describe("MobileInbox", () => {
it("loads pending approvals from /requests/pending?kind=approval", async () => {
const { getByText } = render(<MobileInbox dark={false} />);
@@ -51,9 +62,47 @@ describe("MobileInbox", () => {
expect(container.querySelectorAll('[data-testid="inbox-row"]').length).toBe(0);
});
it("shows an empty state when there are no pending requests", async () => {
vi.mocked(api.get).mockResolvedValue([]);
const { getByText } = render(<MobileInbox dark={false} />);
await waitFor(() => getByText(/No pending approvals/i));
it("does not action a stale approval row as a task during tab switch (core#2766)", async () => {
// Simulate a delayed task fetch after switching tabs. The old approval row
// is still rendered while the new fetch is in flight; its primary action
// must remain "approved", not flip to "done" because the active tab changed.
const approvalFetch = deferred<RequestRow[]>();
const taskFetch = deferred<RequestRow[]>();
vi.mocked(api.get).mockImplementation((url: string | undefined) => {
if (typeof url === "string" && url.includes("kind=approval")) {
return approvalFetch.promise as Promise<unknown>;
}
return taskFetch.promise as Promise<unknown>;
});
const { getByRole, getByText, queryByText } = render(<MobileInbox dark={false} />);
await act(async () => {
approvalFetch.resolve([approval]);
});
await waitFor(() => expect(getByText("Delete prod secret?")).toBeTruthy());
// Switch to Tasks before the task fetch resolves.
fireEvent.click(getByRole("tab", { name: "Tasks" }));
// Switch to Tasks. The new load clears stale rows immediately.
fireEvent.click(getByRole("tab", { name: "Tasks" }));
expect(queryByText("Delete prod secret?")).toBeNull();
// The old approval fetch now resolves. With the sequence guard it
// must be ignored: the Task tab should NOT show approval rows.
await act(async () => {
approvalFetch.resolve([approval]);
});
await waitFor(() => {
expect(queryByText("Delete prod secret?")).toBeNull();
});
// Once the task fetch resolves, the Tasks tab shows its own content.
await act(async () => {
taskFetch.resolve([]);
});
await waitFor(() => {
expect(getByText(/No pending tasks/i)).toBeTruthy();
});
});
});