Files
PaperClipAI/ui/src/api/client.ts
T
TonioandClaude Opus 5 ac91b7f3b2 fix(ui): scope the company selection to the signed-in account (#11430)
## 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 <noreply@anthropic.com>
2026-08-16 11:51:03 -07:00

199 lines
7.0 KiB
TypeScript

import { getPageVisibility, getVisibilityHeaderValue } from "@/lib/page-visibility";
const BASE = "/api";
export class ApiError extends Error {
status: number;
body: unknown;
constructor(message: string, status: number, body: unknown) {
super(message);
this.name = "ApiError";
this.status = status;
this.body = body;
}
}
export interface RequestOptions {
/** Abort signal wired through to `fetch` and coalescing (per-caller). */
signal?: AbortSignal;
/** Extra request headers (e.g. the async-import opt-in). Mutations only. */
headers?: Record<string, string>;
}
function abortError(): DOMException {
return new DOMException("The operation was aborted.", "AbortError");
}
/**
* Non-authoritative observability hints (PAP-12556 / Phase 1). The server treats
* these as scheduling/telemetry only and never as security signals.
*/
function applyObservabilityHeaders(headers: Headers) {
if (headers.has("X-Paperclip-Tab-Visible")) return; // caller override wins
const visibility = getPageVisibility();
headers.set("X-Paperclip-Tab-Visible", getVisibilityHeaderValue(visibility));
if (typeof window !== "undefined" && window.location) {
headers.set("X-Paperclip-Route", window.location.pathname);
}
}
async function request<T>(path: string, init?: RequestInit): Promise<T> {
const headers = new Headers(init?.headers ?? undefined);
const body = init?.body;
if (!(body instanceof FormData) && !headers.has("Content-Type")) {
headers.set("Content-Type", "application/json");
}
applyObservabilityHeaders(headers);
const res = await fetch(`${BASE}${path}`, {
headers,
credentials: "include",
...init,
});
if (!res.ok) {
const errorBody = await res.json().catch(() => null);
throw new ApiError(
(errorBody as { error?: string } | null)?.error ?? `Request failed: ${res.status}`,
res.status,
errorBody,
);
}
if (res.status === 204) return undefined as T;
return res.json();
}
// --- In-tab request coalescing for identical safe GETs -----------------------
//
// Multiple callers issuing the same GET while one is in flight share a single
// underlying fetch. Each caller keeps its own abort semantics: aborting one
// caller only cancels the shared fetch when *every* caller has aborted.
// Mutations are never coalesced.
interface InflightGet {
promise: Promise<unknown>;
controller: AbortController;
refs: Set<symbol>;
}
const inflightGets = new Map<string, InflightGet>();
function coalescedGet<T>(path: string, options?: RequestOptions): Promise<T> {
const signal = options?.signal;
if (signal?.aborted) return Promise.reject(abortError());
let entry = inflightGets.get(path);
if (!entry) {
const controller = new AbortController();
const promise = request<T>(path, { method: "GET", signal: controller.signal });
const created: InflightGet = { promise, controller, refs: new Set() };
// Clear the shared entry once settled so later calls issue a fresh request.
promise.then(
() => {
if (inflightGets.get(path) === created) inflightGets.delete(path);
},
() => {
if (inflightGets.get(path) === created) inflightGets.delete(path);
},
);
inflightGets.set(path, created);
entry = created;
}
const activeEntry = entry;
const ref = Symbol("caller");
activeEntry.refs.add(ref);
const releaseRef = () => {
if (!activeEntry.refs.delete(ref)) return;
// Last caller gone before the fetch settled → abort the shared request.
if (activeEntry.refs.size === 0 && inflightGets.get(path) === activeEntry) {
inflightGets.delete(path);
activeEntry.controller.abort();
}
};
return new Promise<T>((resolve, reject) => {
const onAbort = () => {
signal?.removeEventListener("abort", onAbort);
releaseRef();
reject(abortError());
};
if (signal) signal.addEventListener("abort", onAbort);
activeEntry.promise.then(
(value) => {
signal?.removeEventListener("abort", onAbort);
activeEntry.refs.delete(ref);
resolve(value as T);
},
(err) => {
signal?.removeEventListener("abort", onAbort);
activeEntry.refs.delete(ref);
reject(err);
},
);
});
}
/**
* 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;
}
function isRequestOptions(value: unknown): value is RequestOptions {
return typeof value === "object" && value !== null && "signal" in value;
}
export const api = {
get: <T>(path: string, options?: RequestOptions) => coalescedGet<T>(path, options),
post: <T>(path: string, body: unknown, options?: RequestOptions) =>
request<T>(path, {
method: "POST",
body: JSON.stringify(body),
signal: options?.signal,
...(options?.headers ? { headers: options.headers } : {}),
}),
postForm: <T>(path: string, body: FormData, options?: RequestOptions) =>
request<T>(path, {
method: "POST",
body,
signal: options?.signal,
// Never set Content-Type here — the browser sets multipart/form-data with
// the boundary. Extra headers (e.g. an async opt-in) may still ride along.
...(options?.headers ? { headers: options.headers } : {}),
}),
put: <T>(path: string, body: unknown, options?: RequestOptions) =>
request<T>(path, { method: "PUT", body: JSON.stringify(body), signal: options?.signal }),
/** Raw binary upload (e.g. one chunked import-transfer part); the body travels as-is. */
putRaw: <T>(path: string, body: Blob, options?: RequestOptions) =>
request<T>(path, {
method: "PUT",
body,
signal: options?.signal,
headers: { "Content-Type": "application/octet-stream", ...(options?.headers ?? {}) },
}),
patch: <T>(path: string, body: unknown, options?: RequestOptions) =>
request<T>(path, { method: "PATCH", body: JSON.stringify(body), signal: options?.signal }),
delete: <T>(path: string, bodyOrOptions?: unknown, options?: RequestOptions) => {
const requestOptions = isRequestOptions(bodyOrOptions) ? bodyOrOptions : options;
const body = bodyOrOptions === undefined || isRequestOptions(bodyOrOptions) ? undefined : JSON.stringify(bodyOrOptions);
return request<T>(path, { method: "DELETE", ...(body === undefined ? {} : { body }), signal: requestOptions?.signal });
},
deleteWithBody: <T>(path: string, body: unknown, options?: RequestOptions) =>
request<T>(path, { method: "DELETE", body: JSON.stringify(body), signal: options?.signal }),
};