From ac91b7f3b25cb889195490385ebd5c0855bc603e Mon Sep 17 00:00:00 2001 From: Tonio Date: Sun, 16 Aug 2026 11:51:03 -0700 Subject: [PATCH] fix(ui): scope the company selection to the signed-in account (#11430) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every company-scoped screen reads the active company from `CompanyProvider`, which picks one from the `["companies"]` list and remembers it in localStorage > - That cache entry is shared app-wide and carries no account identity, so it survives a change of account in the tab > - The provider therefore auto-selects from whatever list is cached, which can belong to the account that just went away > - This pull request makes the provider watch the account and refuse to derive a selection from a list fetched for a different one > - The benefit is that the app stops pointing at a company the signed-in account may not be able to see ## Linked Issues or Issue Description No public issue exists. Refs #11380, #11382, #11417. The problem follows. **What happened?** `CompanyProvider` auto-selects a company from the shared `["companies"]` cache entry and writes that id to `localStorage`. Nothing ties that entry to an account. When the account changes in the tab, the previous account's list is still served, so the provider can select — and persist — a company belonging to the account that just went away. Company-scoped screens then render against a company the current account may not be able to see. Signing in through `Auth.tsx` invalidates the entry, so the in-app sign-in path is covered. Two paths are not: a session that lapses server-side, and a second account signing in on another tab. The sign-out sweep in #11380 does not cover them either, because neither presses the sign-out button. **Expected behavior** The company selection is derived only from a company list fetched for the account that is signed in now. **Steps to reproduce** 1. On a self-hosted instance in `authenticated` mode, sign in as account A, which belongs to company X. 2. In a second tab, sign in as account B, which does not belong to company X. 3. Return to the first tab. The session query refetches and reports account B, while the company list is still account A's. 4. The provider keeps company X selected and leaves its id in `localStorage`. **Paperclip version or commit** `master` at `6542ad1f4`. ## What Changed - `ui/src/context/CompanyContext.tsx` — the provider observes `queryKeys.auth.session`. On a change of session user it clears the live selection, removes the shared company list, and holds auto-select until a list fetched for the new account lands. The stored id is left alone on purpose: `resolveBootstrapCompanySelection` re-validates it, so an account signing back in keeps its company while an unrelated account cannot inherit it. - `ui/src/context/CompanyContext.tsx` — an errored list is treated as undecided rather than as "no companies". With `retry: false` a single network blip sticks, and the empty-list branch read it as proof the account owns nothing and cleared the stored selection. - `ui/src/api/client.ts` — new `detachInflightGet(path)`. GET coalescing keys on the request path alone, so a `/companies` request issued under the previous session could be joined by the replacement fetch and answer it with the previous account's companies. Detaching leaves that request to settle for its own callers and makes the next call issue a fresh one. - `ui/src/api/companies.ts` — `companiesApi.detachInflightList()` wraps that for the list path. - `ui/src/context/CompanyContext.tsx` — `companyListUnavailable` separates "no usable list because a request failed" from "this account owns nothing", and `retryCompanies` gives a recovery action that fetches. Both are derived from the query rather than tracked beside it; a second copy of "did the last attempt succeed" drifted out of step during review, reporting a failure over a later empty list that was simply the truth. - `ui/src/components/SidebarCompanyMenu.tsx` — renders "Couldn't load companies" and a Try again item in place of "No companies", which is a claim about the account that a failed request cannot support. This is the menu `Sidebar` mounts, so it is the only place a customer can act on the failure. - `ui/src/components/CompanySwitcher.tsx` — the same treatment. The application does not render this component (its only mount is a Storybook story), so it is kept in step rather than relied on. - `ui/src/context/CompanyContext.test.tsx`, `ui/src/components/SidebarCompanyMenu.test.tsx`, `ui/src/api/client.test.ts` — coverage for the account switch, a same-account re-observation not churning, the detached GET, the failed replacement and its recovery, a single blip self-healing, unavailability not outliving the failure, and the sidebar rendering the recovery action for a failure but plain "No companies" for an account that owns nothing. ### No `retry` override on the replacement fetch The obvious fix for a failed replacement is a retry, and it is not load-bearing here. A transient failure already gets a second attempt: the observer rebinds to a fresh query on the render those state updates schedule, and issues its own request — measured as two attempts with or without the option. Retries would only add failed round trips before a real outage is reported, and the outage is what needs a way out, which is what `companyListUnavailable` and `retryCompanies` provide. ### Why `removeQueries` here, and why that does not generalise Removal notifies no observer. What rebinds them at this call site is the render the surrounding state updates schedule; every observer re-binds to a fresh query on the next render. A caller without that guarantee would leave mounted observers serving the previous account's value, so this is not a pattern to lift elsewhere — the sign-out sweep in #11380 must use `resetQueries` instead, and its measurements are at [#11380](https://github.com/paperclipai/paperclip/pull/11380#issuecomment-5300984911). The inverse caveat holds for a local reset under an observer that stays mounted, which is why #11417 and #11382 avoid `resetQueries`. ## Verification - `pnpm vitest run` in `ui`: **4014 passed, 1 failed**. - `pnpm tsc -b` in `ui`: clean. - `CompanyContext.test.tsx`: 16 passed. `SidebarCompanyMenu.test.tsx`: 15 passed. `client.test.ts`: 9 passed. The failure is pre-existing and unrelated: `IssueProperties.test.tsx` expects `4:08 PM` and gets `9:08 AM`, a timezone-dependent assertion. It reproduces on a tree without this change, and #11478 fixes it. Each new test was confirmed to fail against the implementation it covers, by reverting that change and re-running rather than by assuming. The account-switch test fails without the fix (the selection stays on the previous account's company and no refetch is issued); the flag-clearing test fails without its clause (an empty list keeps reading as "couldn't load"). **Not done:** no manual two-account run in a browser. The path needs two accounts on an `authenticated` instance, which a local dev instance cannot exercise. ## Risks Low. The failure direction is a company selection withheld for one extra round trip, which resolves when the list arrives. The direction it removes is one account's company selected and persisted for another. It adds one company-list request per account change, on a query key the app already uses. It adds no request at boot: the session query it observes is already fetched app-wide. **This does not close the class.** Company-scoped entries other than the list — `["companies", id]`, stats, and the rest of the per-account cache — still survive an account change. That is the cache-lifetime work in #11380, not this provider's. ## Model Used Claude Opus 5 (`claude-opus-5`), through Claude Code. Extended thinking enabled. Tool use enabled: file read and edit, shell for typecheck and test runs, and a scratch vitest harness to measure `removeQueries` and `resetQueries` notification behaviour against the installed `@tanstack/query-core` 5.101.4. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 --- ui/src/api/client.test.ts | 26 ++- ui/src/api/client.ts | 13 ++ ui/src/api/companies.ts | 12 +- ui/src/components/CompanySwitcher.tsx | 26 ++- ui/src/components/SidebarCompanyMenu.test.tsx | 63 +++++- ui/src/components/SidebarCompanyMenu.tsx | 26 ++- ui/src/context/CompanyContext.test.tsx | 206 +++++++++++++++++- ui/src/context/CompanyContext.tsx | 128 ++++++++++- 8 files changed, 489 insertions(+), 11 deletions(-) diff --git a/ui/src/api/client.test.ts b/ui/src/api/client.test.ts index d21ec06181..9c8efe55ab 100644 --- a/ui/src/api/client.test.ts +++ b/ui/src/api/client.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { __inflightGetCount, api } from "./client"; +import { __inflightGetCount, api, detachInflightGet } from "./client"; interface Deferred { promise: Promise; @@ -70,6 +70,30 @@ describe("in-tab GET coalescing", () => { await expect(p2).rejects.toMatchObject({ name: "AbortError" }); }); + it("stops later callers joining a detached in-flight GET", async () => { + // A GET issued under one account's session must not answer a caller that + // runs after the account changed. + const first = deferred(); + fetchMock.mockReturnValueOnce(first.promise); + const previousAccount = api.get("/detach-me"); + expect(__inflightGetCount()).toBe(1); + + detachInflightGet("/detach-me"); + expect(__inflightGetCount()).toBe(0); + + const second = deferred(); + fetchMock.mockReturnValueOnce(second.promise); + const currentAccount = api.get("/detach-me"); + expect(fetchMock).toHaveBeenCalledTimes(2); + + // Each caller gets its own response, not the other's. + first.resolve(jsonResponse({ companies: ["previous"] })); + second.resolve(jsonResponse({ companies: ["current"] })); + expect(await previousAccount).toEqual({ companies: ["previous"] }); + expect(await currentAccount).toEqual({ companies: ["current"] }); + expect(__inflightGetCount()).toBe(0); + }); + it("never coalesces mutations", async () => { fetchMock.mockResolvedValue(jsonResponse({ ok: true })); await Promise.all([api.post("/mutate", { a: 1 }), api.post("/mutate", { a: 1 })]); diff --git a/ui/src/api/client.ts b/ui/src/api/client.ts index ced7cc2612..923c7f5c15 100644 --- a/ui/src/api/client.ts +++ b/ui/src/api/client.ts @@ -136,6 +136,19 @@ function coalescedGet(path: string, options?: RequestOptions): Promise { }); } +/** + * Stop later callers from joining the in-flight GET for `path`. + * + * Coalescing keys on the path alone, so a GET issued under one account's session + * can be joined by a caller that runs after the account changed — and handed the + * previous account's response. Detaching leaves that request to settle for the + * callers that asked for it, and makes the next call issue a fresh one. It does + * not abort, because those callers still want what they asked for. + */ +export function detachInflightGet(path: string): void { + inflightGets.delete(path); +} + /** Test-only: number of in-flight coalesced GET keys. */ export function __inflightGetCount(): number { return inflightGets.size; diff --git a/ui/src/api/companies.ts b/ui/src/api/companies.ts index 72296538a7..39671839f5 100644 --- a/ui/src/api/companies.ts +++ b/ui/src/api/companies.ts @@ -21,7 +21,9 @@ import { type CompanyImportTransferPartUploadResult, type CompanyImportTransferStatus, } from "@paperclipai/shared/company-import-transfer"; -import { api } from "./client"; +import { api, detachInflightGet } from "./client"; + +const COMPANIES_LIST_PATH = "/companies"; export type CompanyStats = Record; @@ -75,7 +77,13 @@ export interface CompanyImportJobStatus { } export const companiesApi = { - list: () => api.get("/companies"), + list: () => api.get(COMPANIES_LIST_PATH), + /** + * Call before re-reading the list for a different account: an in-flight + * `/companies` GET issued under the previous session would otherwise be + * coalesced into, and answer with that account's companies. + */ + detachInflightList: () => detachInflightGet(COMPANIES_LIST_PATH), get: (companyId: string) => api.get(`/companies/${companyId}`), stats: () => api.get("/companies/stats"), create: (data: { diff --git a/ui/src/components/CompanySwitcher.tsx b/ui/src/components/CompanySwitcher.tsx index 4c38e66d5a..46067c0149 100644 --- a/ui/src/components/CompanySwitcher.tsx +++ b/ui/src/components/CompanySwitcher.tsx @@ -1,4 +1,4 @@ -import { ChevronsUpDown, Plus, Settings } from "lucide-react"; +import { ChevronsUpDown, Plus, RefreshCw, Settings } from "lucide-react"; import { Link } from "@/lib/router"; import { useCompany } from "../context/CompanyContext"; import { @@ -32,7 +32,8 @@ interface CompanySwitcherProps { export function CompanySwitcher({ open: controlledOpen, onOpenChange }: CompanySwitcherProps = {}) { const [internalOpen, setInternalOpen] = useState(false); - const { companies, selectedCompany, setSelectedCompanyId } = useCompany(); + const { companies, selectedCompany, setSelectedCompanyId, companyListUnavailable, retryCompanies } = + useCompany(); const sidebarCompanies = companies.filter((company) => company.status !== "archived"); const open = controlledOpen ?? internalOpen; const setOpen = onOpenChange ?? setInternalOpen; @@ -69,7 +70,26 @@ export function CompanySwitcher({ open: controlledOpen, onOpenChange }: CompanyS ))} {sidebarCompanies.length === 0 && ( - No companies + // "No companies" is a claim about the account, and after a failed list + // request it is one we cannot make — say what actually happened and + // give the customer the way out, since nothing else in the app does. + companyListUnavailable ? ( + <> + Couldn't load companies + { + // Keep the menu open so the result of the retry is visible. + event.preventDefault(); + void retryCompanies?.(); + }} + > + + Try again + + + ) : ( + No companies + ) )} diff --git a/ui/src/components/SidebarCompanyMenu.test.tsx b/ui/src/components/SidebarCompanyMenu.test.tsx index e9779022f2..4f57fae617 100644 --- a/ui/src/components/SidebarCompanyMenu.test.tsx +++ b/ui/src/components/SidebarCompanyMenu.test.tsx @@ -54,9 +54,19 @@ vi.mock("@/lib/router", () => ({ useNavigate: () => mockNavigate, })); +// Overridable so the list-unavailable branch can be exercised; null means "use +// the default three companies below". +const mockCompanyState = vi.hoisted(() => ({ + companies: null as unknown[] | null, + companyListUnavailable: false, + retryCompanies: vi.fn(), +})); + vi.mock("@/context/CompanyContext", () => ({ useCompany: () => ({ - companies: [ + companyListUnavailable: mockCompanyState.companyListUnavailable, + retryCompanies: mockCompanyState.retryCompanies, + companies: mockCompanyState.companies ?? [ { id: "company-1", issuePrefix: "PAP", @@ -180,6 +190,8 @@ describe("SidebarCompanyMenu", () => { updatedAt: null, }); mockLocation.pathname = "/PAP/dashboard"; + mockCompanyState.companies = null; + mockCompanyState.companyListUnavailable = false; }); afterEach(() => { @@ -218,6 +230,55 @@ describe("SidebarCompanyMenu", () => { await flushReact(); } + // This menu is the one the app renders, so it is the only place a customer can + // act on a failed company list. Saying "No companies" there states something + // about the account that a failed request cannot support, and leaves the tab + // with no way back short of a browser reload. + it("offers a way back when the company list could not be loaded", async () => { + mockCompanyState.companies = []; + mockCompanyState.companyListUnavailable = true; + + const { root } = renderMenu(); + await flushReact(); + await openMenu("Open Acme Labs company switcher"); + + expect(document.body.textContent).toContain("Couldn't load companies"); + expect(document.body.textContent).not.toContain("No companies"); + + const retryItem = Array.from(document.body.querySelectorAll('[role="menuitem"]')).find( + (item) => item.textContent?.includes("Try again"), + ); + expect(retryItem).not.toBeUndefined(); + + act(() => { + retryItem?.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, button: 0 })); + retryItem?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(mockCompanyState.retryCompanies).toHaveBeenCalled(); + + act(() => { + root.unmount(); + }); + }); + + it("still reports an account that owns no companies as empty, not broken", async () => { + mockCompanyState.companies = []; + mockCompanyState.companyListUnavailable = false; + + const { root } = renderMenu(); + await flushReact(); + await openMenu("Open Acme Labs company switcher"); + + expect(document.body.textContent).toContain("No companies"); + expect(document.body.textContent).not.toContain("Couldn't load companies"); + + act(() => { + root.unmount(); + }); + }); + it("uses company-centric create copy without the chat flag", async () => { const root = createRoot(container); const queryClient = new QueryClient({ diff --git a/ui/src/components/SidebarCompanyMenu.tsx b/ui/src/components/SidebarCompanyMenu.tsx index 7ae9899159..5b193a214c 100644 --- a/ui/src/components/SidebarCompanyMenu.tsx +++ b/ui/src/components/SidebarCompanyMenu.tsx @@ -6,6 +6,7 @@ import { GripVertical, LogOut, Plus, + RefreshCw, UserPlus, } from "lucide-react"; import { @@ -198,7 +199,8 @@ function SortableCompanyItem({ export function SidebarCompanyMenu({ open: controlledOpen, onOpenChange }: SidebarCompanyMenuProps = {}) { const [internalOpen, setInternalOpen] = useState(false); const [isEditingOrder, setIsEditingOrder] = useState(false); - const { companies, selectedCompany, setSelectedCompanyId } = useCompany(); + const { companies, selectedCompany, setSelectedCompanyId, companyListUnavailable, retryCompanies } = + useCompany(); const { openOnboarding } = useDialogActions(); const { isMobile, setSidebarOpen, collapsed, peeking } = useSidebar(); const rail = collapsed && !peeking; @@ -437,7 +439,27 @@ export function SidebarCompanyMenu({ open: controlledOpen, onOpenChange }: Sideb {orderedCompanies.length === 0 ? ( - No companies + // "No companies" is a claim about the account. After a failed + // list request it is one we cannot make, and this menu is the + // only place the customer can act on it — say what happened and + // offer the way back. + companyListUnavailable ? ( + <> + Couldn't load companies + { + // Keep the menu open so the result of the retry is visible. + event.preventDefault(); + void retryCompanies(); + }} + > + + Try again + + + ) : ( + No companies + ) ) : null} )} diff --git a/ui/src/context/CompanyContext.test.tsx b/ui/src/context/CompanyContext.test.tsx index cd45f94531..b84be34bc7 100644 --- a/ui/src/context/CompanyContext.test.tsx +++ b/ui/src/context/CompanyContext.test.tsx @@ -16,12 +16,28 @@ import { const mockCompaniesApi = vi.hoisted(() => ({ list: vi.fn(), create: vi.fn(), + detachInflightList: vi.fn(), })); vi.mock("../api/companies", () => ({ companiesApi: mockCompaniesApi, })); +const mockAuthApi = vi.hoisted(() => ({ + getSession: vi.fn(), +})); + +vi.mock("../api/auth", () => ({ + authApi: mockAuthApi, +})); + +function sessionFor(userId: string) { + return { + session: { id: `session-${userId}`, userId }, + user: { id: userId, name: "Example", email: `${userId}@example.com`, image: null }, + }; +} + const activeCompany = { id: "company-1" }; const secondActiveCompany = { id: "company-2" }; const archivedCompany = { id: "archived-company" }; @@ -54,8 +70,18 @@ function makeCompany(id: string): Company { }; } +async function flushReact() { + await act(async () => { + await new Promise((resolve) => window.setTimeout(resolve, 0)); + }); +} + +let captured: ReturnType | null = null; + function Probe({ onSelectedCompanyId }: { onSelectedCompanyId: (companyId: string | null) => void }) { - const { selectedCompanyId } = useCompany(); + const company = useCompany(); + captured = company; + const { selectedCompanyId } = company; useEffect(() => { onSelectedCompanyId(selectedCompanyId); }, [onSelectedCompanyId, selectedCompanyId]); @@ -172,6 +198,7 @@ describe("CompanyProvider", () => { beforeEach(() => { (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + captured = null; localStorage.clear(); container = document.createElement("div"); document.body.appendChild(container); @@ -181,6 +208,7 @@ describe("CompanyProvider", () => { queries: { retry: false }, }, }); + mockAuthApi.getSession.mockResolvedValue(null); }); afterEach(async () => { @@ -256,4 +284,180 @@ describe("CompanyProvider", () => { expect(seen).toEqual([null, "company-1"]); expect(localStorage.getItem("paperclip.selectedCompanyId")).toBe("company-1"); }); + + // The `["companies"]` cache entry is shared app-wide and carries no account + // identity, so it survives a change of account in this tab. Cached data is + // served whether or not it is stale, so freshness cannot stand in for + // "belongs to the account signed in now". + describe("when the account changes in this tab", () => { + async function bootWithFirstAccount(seen: Array) { + mockAuthApi.getSession.mockResolvedValue(sessionFor("user-1")); + queryClient.setQueryData(queryKeys.companies.all, { + companies: [makeCompany("company-1")], + unauthorized: false, + }); + mockCompaniesApi.list.mockImplementation(() => new Promise(() => {})); + + await act(async () => { + root.render( + + + seen.push(companyId)} /> + + , + ); + }); + await flushReact(); + + expect(seen).toEqual([null, "company-1"]); + expect(localStorage.getItem("paperclip.selectedCompanyId")).toBe("company-1"); + } + + it("drops the previous account's selection and re-reads the list", async () => { + const seen: Array = []; + await bootWithFirstAccount(seen); + + let resolveSecondList: ((companies: Company[]) => void) | null = null; + mockCompaniesApi.list.mockImplementation( + () => + new Promise((resolve) => { + resolveSecondList = resolve; + }), + ); + + await act(async () => { + queryClient.setQueryData(queryKeys.auth.session, sessionFor("user-2")); + }); + await flushReact(); + + // Nothing from the previous account is exposed while the new list loads. + expect(seen).toEqual([null, "company-1", null]); + expect(queryClient.getQueryData(queryKeys.companies.all)).toBeUndefined(); + expect(resolveSecondList).not.toBeNull(); + // The replacement fetch must not be coalesced into a `/companies` request + // issued under the previous session. + expect(mockCompaniesApi.detachInflightList).toHaveBeenCalled(); + + await act(async () => { + resolveSecondList?.([makeCompany("company-2")]); + await Promise.resolve(); + }); + await flushReact(); + + expect(seen).toEqual([null, "company-1", null, "company-2"]); + expect(localStorage.getItem("paperclip.selectedCompanyId")).toBe("company-2"); + }); + + // The replacement fetch is the only thing standing between the account + // change and a usable app: the cache entry has already been removed, so a + // failure here leaves the tab with no list and no selection. It must not + // also leave it with no way out. + it("recovers the list when the replacement fetch fails and is retried", async () => { + const seen: Array = []; + await bootWithFirstAccount(seen); + + mockCompaniesApi.list.mockRejectedValue(new Error("network down")); + + await act(async () => { + queryClient.setQueryData(queryKeys.auth.session, sessionFor("user-2")); + }); + await flushReact(); + // Past the bounded retries, so the failure has actually settled. + await act(async () => { + await new Promise((resolve) => window.setTimeout(resolve, 1200)); + }); + await flushReact(); + + // The previous account's company is gone and nothing took its place. + expect(seen).toEqual([null, "company-1", null]); + // Reported as unavailable, not as an account that owns nothing. Without + // this the switcher renders "No companies", which is a false claim. + expect(captured?.companyListUnavailable).toBe(true); + expect(captured?.companies).toEqual([]); + + // The recovery path actually recovers. + mockCompaniesApi.list.mockResolvedValue([makeCompany("company-2")]); + await act(async () => { + await captured?.retryCompanies(); + }); + await flushReact(); + + expect(captured?.companyListUnavailable).toBe(false); + expect(seen).toEqual([null, "company-1", null, "company-2"]); + expect(localStorage.getItem("paperclip.selectedCompanyId")).toBe("company-2"); + }); + + // A transient blip must not need the customer to notice and click anything. + // Nothing configures a retry — the second attempt is the observer's own, + // issued when it rebinds to a fresh query after the removal above. This + // pins that, so the recovery affordance stays reserved for real outages + // rather than becoming the only way back from a one-off blip. + it("rides out a single failed replacement request without help", async () => { + const seen: Array = []; + await bootWithFirstAccount(seen); + + mockCompaniesApi.list + .mockRejectedValueOnce(new Error("blip")) + .mockResolvedValue([makeCompany("company-2")]); + + await act(async () => { + queryClient.setQueryData(queryKeys.auth.session, sessionFor("user-2")); + }); + await flushReact(); + await act(async () => { + await new Promise((resolve) => window.setTimeout(resolve, 1200)); + }); + await flushReact(); + + expect(captured?.companyListUnavailable).toBe(false); + expect(seen).toEqual([null, "company-1", null, "company-2"]); + }); + + // The failure flag must not outlive the failure. A later success — a focus + // refetch, an invalidation from anywhere — settles the question, including + // when the honest answer is an empty list. Otherwise an account that owns + // nothing reads as "couldn't load", behind a retry that cannot change it. + it("stops reporting the list as unavailable once any fetch succeeds", async () => { + const seen: Array = []; + await bootWithFirstAccount(seen); + + mockCompaniesApi.list.mockRejectedValue(new Error("network down")); + await act(async () => { + queryClient.setQueryData(queryKeys.auth.session, sessionFor("user-2")); + }); + await flushReact(); + await act(async () => { + await new Promise((resolve) => window.setTimeout(resolve, 1200)); + }); + await flushReact(); + + expect(captured?.companyListUnavailable).toBe(true); + + // This account genuinely owns no companies, and the request says so. + mockCompaniesApi.list.mockResolvedValue([]); + await act(async () => { + await queryClient.refetchQueries({ queryKey: queryKeys.companies.all }); + }); + await flushReact(); + + expect(captured?.companies).toEqual([]); + expect(captured?.companyListUnavailable).toBe(false); + }); + + it("leaves the selection alone when the same account is observed again", async () => { + const seen: Array = []; + await bootWithFirstAccount(seen); + const listCallsAfterBoot = mockCompaniesApi.list.mock.calls.length; + + // A session refetch returns an equal-but-new object for the same account. + await act(async () => { + queryClient.setQueryData(queryKeys.auth.session, sessionFor("user-1")); + }); + await flushReact(); + + expect(seen).toEqual([null, "company-1"]); + expect(localStorage.getItem("paperclip.selectedCompanyId")).toBe("company-1"); + expect(mockCompaniesApi.list.mock.calls.length).toBe(listCallsAfterBoot); + }); + }); }); diff --git a/ui/src/context/CompanyContext.tsx b/ui/src/context/CompanyContext.tsx index 5a0a3f7d8f..8cc5534b65 100644 --- a/ui/src/context/CompanyContext.tsx +++ b/ui/src/context/CompanyContext.tsx @@ -4,11 +4,13 @@ import { useContext, useEffect, useMemo, + useRef, useState, type ReactNode, } from "react"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import type { Company } from "@paperclipai/shared"; +import { authApi } from "../api/auth"; import { companiesApi } from "../api/companies"; import { companiesListQueryOptions, type CompanyListResult } from "../api/companies-query"; import { queryKeys } from "../lib/queryKeys"; @@ -22,6 +24,14 @@ interface CompanyContextValue { selectionSource: CompanySelectionSource; loading: boolean; error: Error | null; + /** + * There is no usable company list *and* the reason is a failed request rather + * than an account that owns nothing. Consumers need the two apart: an empty + * list is a fact to render, this is a dead end to offer a way out of. + */ + companyListUnavailable: boolean; + /** Re-fetches the list for the account signed in now. Pairs with the flag above. */ + retryCompanies: () => Promise; setSelectedCompanyId: (companyId: string, options?: CompanySelectionOptions) => void; reloadCompanies: () => Promise; createCompany: (data: { @@ -102,9 +112,91 @@ export function CompanyProvider({ children }: { children: ReactNode }) { [companies], ); + // The `["companies"]` entry is shared app-wide and carries no account identity, + // so it outlives a change of account in this tab: the previous account's list + // keeps being served, and the effect below would auto-select from it and write + // that company id to localStorage. Signing in through the app drops the entry, + // but nothing does when the session lapses server-side or a second account + // signs in on another tab — so watch the account itself. + const { data: session, isPending: isSessionPending } = useQuery({ + queryKey: queryKeys.auth.session, + queryFn: () => authApi.getSession(), + retry: false, + }); + const sessionUserId = session?.user.id ?? null; + const observedUserIdRef = useRef(undefined); + const [awaitingAccountScopedList, setAwaitingAccountScopedList] = useState(false); + + useEffect(() => { + // Until the session settles the account is unknown, not changed. + if (isSessionPending) return; + const previousUserId = observedUserIdRef.current; + observedUserIdRef.current = sessionUserId; + // First settled observation is this tab's boot: no previous account to leave. + if (previousUserId === undefined || previousUserId === sessionUserId) return; + + // The live selection belongs to the account that just went away. The stored + // one deliberately survives: resolveBootstrapCompanySelection re-validates it + // against the incoming list, so an account signing back in keeps its company + // while an unrelated account cannot inherit it. + setSelectedCompanyIdState(null); + setSelectionSource("bootstrap"); + setAwaitingAccountScopedList(true); + // `removeQueries`, not `resetQueries`: reset rewinds the update counters + // `isFetchedAfterMount` is derived from while mounted observers keep their + // pre-reset baseline, which strands consumers gating on that flag (see + // AppsConnect.tsx). Removal is what fits *here*, for a reason worth saying + // out loud: removal notifies nobody, so on its own it would leave mounted + // observers serving the previous account. What rebinds them is the render + // the state updates above just scheduled — every observer re-binds to a + // fresh query on the next render. Do not lift this call anywhere that lacks + // that guarantee; the sign-out sweep is exactly such a place. + queryClient.removeQueries({ queryKey: queryKeys.companies.all, exact: true }); + + // Coalescing keys on the request path alone, so without this the fetch below + // can join a `/companies` request issued under the previous session and be + // answered with that account's companies. + companiesApi.detachInflightList(); + + // Drive the replacement fetch here rather than leaning on observer refetch + // semantics, so the gate below lifts exactly when a list for this account + // has landed. + // + // No `retry` override, though `companiesListQueryOptions` sets + // `retry: false`. A transient blip already gets a second attempt: the + // observer above rebinds to a fresh query on the render these state updates + // schedule and issues its own request, so a single failure self-heals + // (measured: two attempts either way). Adding retries here only buys extra + // failed round trips before a real outage is reported, and the outage is + // what needs a way out — see `companyListUnavailable` below. + // The rejection is caught only to keep it from going unhandled — it is not + // lost. `fetchQuery` records it on the query itself, so it arrives as + // `error` below, which is where `companyListUnavailable` reads it from. + // Carrying a second copy in component state is what let the two fall out of + // step: the copy outlived the failure and kept reporting "couldn't load" + // over a later, honest empty list. + let cancelled = false; + void queryClient + .fetchQuery({ ...companiesListQueryOptions, staleTime: 0 }) + .catch(() => undefined) + .finally(() => { + if (!cancelled) setAwaitingAccountScopedList(false); + }); + return () => { + cancelled = true; + }; + }, [isSessionPending, queryClient, sessionUserId]); + // Auto-select first company when list loads useEffect(() => { + // Nothing may be derived from the list until one has been fetched for the + // account signed in now. + if (awaitingAccountScopedList) return; if (isLoading) return; + // An errored list says nothing about which companies this account has, and + // `retry: false` makes a single network blip stick. Treat it as undecided + // rather than as "no companies", which would clear the stored selection. + if (error) return; if (companies.length === 0) { if (shouldClearStoredCompanySelection({ companies, @@ -130,7 +222,15 @@ export function CompanyProvider({ children }: { children: ReactNode }) { setSelectedCompanyIdState(next); setSelectionSource("bootstrap"); localStorage.setItem(STORAGE_KEY, next); - }, [companies, companyListUnauthorized, error, isLoading, selectedCompanyId, sidebarCompanies]); + }, [ + awaitingAccountScopedList, + companies, + companyListUnauthorized, + error, + isLoading, + selectedCompanyId, + sidebarCompanies, + ]); const setSelectedCompanyId = useCallback((companyId: string, options?: CompanySelectionOptions) => { setSelectedCompanyIdState(companyId); @@ -142,6 +242,28 @@ export function CompanyProvider({ children }: { children: ReactNode }) { await queryClient.invalidateQueries({ queryKey: queryKeys.companies.all }); }, [queryClient]); + // The way out of the dead end. Not `reloadCompanies`: invalidation refetches + // only through a mounted observer, and it leaves an errored query reporting + // its old error, so the recovery affordance would keep telling the customer + // the list is unavailable after a retry had already succeeded. + const retryCompanies = useCallback(async () => { + setAwaitingAccountScopedList(true); + try { + await queryClient.fetchQuery({ ...companiesListQueryOptions, staleTime: 0 }); + } catch { + // Recorded on the query, same as the replacement fetch above. + } finally { + setAwaitingAccountScopedList(false); + } + }, [queryClient]); + + // Empty because we could not find out, as opposed to empty because the account + // owns nothing. Derived from the query rather than tracked alongside it: the + // query is the only thing that knows whether the last attempt succeeded, and a + // second copy of that answer drifts — it did, reporting a failure over a later + // empty list that was simply the truth. + const companyListUnavailable = companies.length === 0 && Boolean(error); + const createMutation = useMutation({ mutationFn: (data: { name: string; @@ -179,6 +301,8 @@ export function CompanyProvider({ children }: { children: ReactNode }) { selectionSource, loading: isLoading, error: error as Error | null, + companyListUnavailable, + retryCompanies, setSelectedCompanyId, reloadCompanies, createCompany, @@ -190,6 +314,8 @@ export function CompanyProvider({ children }: { children: ReactNode }) { selectionSource, isLoading, error, + companyListUnavailable, + retryCompanies, setSelectedCompanyId, reloadCompanies, createCompany,