From 1305bf35dd5a3e9aee02c4e5c7f117078e3993fb Mon Sep 17 00:00:00 2001
From: Codex Review Integration <codex-review@local.invalid>
Date: Wed, 19 Aug 2026 10:26:28 -0300
Subject: [PATCH] fix(vendors): bypass the query cache when reloading after a
 conflict (SH-250)

The reload fix re-seeded the form from queryClient.fetchQuery, which inherits the
global 5-minute staleTime. A 409 never invalidates that key -
useSaveVendorCompanyRoster invalidates only in onSuccess and its onError returns
early for conflicts - and the Add flow has no mounted observer on it, so reload
returned the cached roster with the same stale rowVersion and every retry 409'd
again. Update mode was unaffected because it recovers via query.refetch(), which
bypasses staleTime.

Pass staleTime: 0 on that fetch, since its sole purpose is the freshest
rowVersion. The regression test uses a client with the app's real staleTime; the
default test client uses 0, which masked this entirely.
---
 .../use-vendor-roster-selection.ts            |  7 +++
 .../vendors/use-vendor-roster-form.test.tsx   | 46 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts b/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts
index 4bd467f1..24d51ce8 100644
--- a/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts
+++ b/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts
@@ -87,6 +87,13 @@ export function useVendorRosterSelection({
       queryClient.fetchQuery({
         queryKey: queryKeys.vendors.roster("companyId", id),
         queryFn: () => vendorCompanyRosterApi.get({ companyId: id }),
+        // SH-250: this fetch exists to obtain the freshest rowVersion, so it must not
+        // serve the global 5-minute staleTime cache. A conflict never invalidates this
+        // key (useSaveVendorCompanyRoster invalidates only on success, and its onError
+        // returns early for conflicts), and the Add flow has no mounted observer on it.
+        // Without staleTime: 0 the Reload button re-seeds the same stale rowVersion and
+        // the retry 409s again indefinitely.
+        staleTime: 0,
       }),
     [queryClient],
   );
diff --git a/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx b/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx
index 777363fd..b56aa714 100644
--- a/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx
+++ b/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx
@@ -156,6 +156,17 @@ function createClient(): QueryClient {
   return new QueryClient({ defaultOptions: { queries: { retry: false } } });
 }
 
+/**
+ * SH-250: mirrors the app's real staleTime (src/lib/query/query-client.ts). The default
+ * test client uses staleTime 0, which silently masks cache-staleness defects on the
+ * reload path — the conflict recovery must not depend on an empty cache.
+ */
+function createCachingClient(): QueryClient {
+  return new QueryClient({
+    defaultOptions: { queries: { retry: false, staleTime: 5 * 60 * 1000 } },
+  });
+}
+
 function makeWrapper(client: QueryClient) {
   return function Wrapper({ children }: { children: ReactNode }) {
     return <QueryClientProvider client={client}>{children}</QueryClientProvider>;
@@ -534,6 +545,41 @@ describe("useVendorRosterForm additive add flow (SH-250)", () => {
     await waitFor(() => expect(result.current.name.field.value).toBe("Gateway Plumbing & Drain"));
   });
 
+  it("reload bypasses the cache so the retry carries the fresh rowVersion", async () => {
+    rosterGet.mockResolvedValueOnce(roster);
+    const { result } = renderHook(
+      () => useVendorRosterForm({ mode: "create", startWithTechnician: true }),
+      { wrapper: makeWrapper(createCachingClient()) },
+    );
+
+    await act(async () => {
+      await result.current.selectCompany(company);
+    });
+    expect(rosterGet).toHaveBeenCalledTimes(1);
+
+    act(() => {
+      result.current.submit(submitValues([newTechnician]));
+    });
+    expect(saveMutate.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ rowVersion: "rv-1" }));
+
+    // The 409 never invalidates this key, and in the Add flow nothing else observes it.
+    // Without staleTime: 0 on the reload fetch, the cached rv-1 is served straight back
+    // and every retry 409s again.
+    rosterGet.mockResolvedValueOnce({ ...roster, rowVersion: "rv-2" });
+    await act(async () => {
+      result.current.reload();
+    });
+
+    await waitFor(() => expect(rosterGet).toHaveBeenCalledTimes(2));
+
+    act(() => {
+      result.current.submit(submitValues([newTechnician]));
+    });
+    expect(saveMutate.mock.calls[1]?.[0]).toEqual(
+      expect.objectContaining({ mode: "add", rowVersion: "rv-2" }),
+    );
+  });
+
   it("does not regress the Edit Vendor reconcile path (mode update)", () => {
     rosterQueryResult = {
       data: rosterWithTechnicians,