Files
PaperClipAI/ui/src/api/companies-query.ts
T
Devin FoleyandPaperclip e4237c45f3 fix(ui): prevent organization title flicker during plugin loading (#13854)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The sidebar identifies the current organization.
> - An optional plugin can replace this navigation surface.
> - The built-in title appears before discovery and module loading
finish.
> - This pull request reserves the trigger until its owner is known.
> - The organization name appears once, while failures retain built-in
navigation.

## Linked Issues or Issue Description

Refs #13832. Searched related pull requests and issues; no duplicate fix
found.

**What happened?**
The organization switcher renders a provisional built-in title before an
installed replacement loads. Unrelated plugin imports can also affect
its loading state.

**Expected behavior**
Reserve the trigger with a neutral placeholder, then show the resolved
navigation surface. Keep the built-in menu on failed or absent
contributions.

**Steps to reproduce**
Install an organization-switcher contribution. Delay session, company,
contribution, and module responses. Reload the page and watch the
trigger through each stage.

## What Changed

- Reserve the trigger through account, company selection, slot
discovery, and module loading.
- Distinguish failed session lookup from pending lookup so errors retain
usable navigation.
- Load and await only contributions matching the requested slots.
Observe completion of imports started by another consumer.
- Document loading behavior and add regression coverage for loading,
failures, unrelated modules, and identity transitions.

## Verification

- `pnpm -r typecheck` passed, including Rust checks.
- `pnpm build` passed.
- All 629 UI test files passed: 6,593 tests. The 42 focused
UI/API/plugin tests also passed.
- `pnpm check:token-gates` and `git diff --check` passed.
- `pnpm test:run` was also attempted. The broad local server run was
stopped after recording skill-cache/channel fixture failures outside
this diff (for example, runtime skill source status `missing` instead of
`available`). The original cause is not established. All latest-head
Linux CI gates pass; the complete UI suite and affected local checks
pass.
- Desktop (1440px) and mobile (390px) Chromium checks passed with real
host components, dynamic module loading, and the built Account bundle.
Delayed fixture responses produced exactly two title states: empty
placeholder, then the resolved name. A slow refresh preserved the title
and trigger dimensions; absent/failed plugin fallback and Escape
dismissal passed, with zero uncaught browser errors. This is browser
component integration, not a live signed-in tenant test.

## Risks

A cold load displays a neutral placeholder until discovery completes.
Absent, ambiguous, failed, and invalid contributions still use the
built-in menu. No migrations or authorization changes. Scoped module
loading changes when an unrelated contribution is imported; each surface
loads its own matching modules.

## Model Used

OpenAI Codex, GPT-6, with reasoning, code execution, and browser
verification. The exact deployment ID and context window are not exposed
in this session.

## 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 (e.g. `docs/...`, `fix/...`)
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

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-09-22 20:33:29 -07:00

152 lines
6.5 KiB
TypeScript

import { useQuery, type QueryClient } from "@tanstack/react-query";
import type { Company } from "@paperclipai/shared";
import { authApi } from "./auth";
import { companiesApi } from "./companies";
import { ApiError } from "./client";
import { queryKeys } from "../lib/queryKeys";
export type CompanyListResult = { companies: Company[]; unauthorized: boolean };
// Single source of truth for the company-list query. Every consumer reads the
// same cache entry, so they must agree on the shape — returning a bare
// `Company[]` from one and this wrapped object from another silently corrupts
// the entry and crashes whichever reads the other's shape.
//
// The entry is keyed by account. Callers therefore cannot ask for "the company
// list" without saying whose, which is the point: the previous design let a
// list fetched for one account answer a question about another, and every
// consumer had to defend against that individually.
export function companyListQueryOptions(userId: string | null) {
return {
queryKey: queryKeys.companies.list(userId),
queryFn: async (): Promise<CompanyListResult> => {
// Request coalescing keys on the path alone, so it would happily answer a
// fetch for this account with a `/companies` request issued under the
// previous one — putting the wrong account's list in an account-keyed
// entry. Coalescing has nothing to offer here anyway: React Query already
// dedupes concurrent fetches within a key, and *across* keys the accounts
// differ by construction, which is exactly when sharing is wrong.
companiesApi.detachInflightList();
try {
return { companies: await companiesApi.list(), unauthorized: false };
} catch (err) {
if (err instanceof ApiError && (err.status === 401 || err.status === 403)) {
return { companies: [], unauthorized: true };
}
throw err;
}
},
retry: false,
} as const;
}
/** The administration directory has its own account-scoped cache and request. */
export function companyDirectoryQueryOptions(userId: string | null) {
return {
queryKey: queryKeys.companies.directory(userId),
queryFn: async () => {
companiesApi.detachInflightDirectory();
return companiesApi.directory();
},
retry: false,
} as const;
}
const sessionQueryOptions = {
queryKey: queryKeys.auth.session,
queryFn: () => authApi.getSession(),
retry: false,
} as const;
/**
* The signed-in account, as the cache currently understands it.
*
* `settled` means the session query *answered*, not merely that it stopped
* being pending. The distinction is the whole point of this helper. `null` is a
* real answer — `authApi.getSession` returns it for a 401, meaning "signed out"
* — but a session request that *failed* answers nothing, and it throws rather
* than returning null. Treating that failure as `null` would key an account's
* list to `anonymous` while the request still carried their cookie, writing a
* credentialed response into the entry every signed-out reader trusts. Which is
* the original bug, rebuilt for anonymous.
*
* So: settled on success only. A failed session lookup leaves the list unfetched
* until the query recovers, which it does on the next refetch.
*/
export function useAccountIdentity(): { userId: string | null; settled: boolean; failed: boolean } {
const { data: session, isSuccess, isError } = useQuery(sessionQueryOptions);
return { userId: session?.user.id ?? null, settled: isSuccess, failed: isError };
}
/**
* Observe the company list for the account signed in now.
*
* Pass `enabled: false` to hold for a caller's own reasons; the session gate is
* applied on top of it either way.
*/
export function useCompanyListQuery(
options: { enabled?: boolean; staleTime?: number; retry?: number | boolean } = {},
) {
const { userId, settled } = useAccountIdentity();
const query = useQuery<CompanyListResult>({
...companyListQueryOptions(userId),
...options,
enabled: settled && (options.enabled ?? true),
});
// While the account is unknown this query is disabled, and a disabled query
// reports `isLoading: false` with `data: undefined` — which consumers default
// to an empty list and read as "asked, and owns nothing". That is the
// destructive reading: `shouldClearStoredCompanySelection` would throw away
// the customer's stored company on every cold boot, before the session had
// even landed.
//
// Waiting for the account is part of getting the list, so it is reported as
// part of getting the list. Consumers gate on these two, and both must mean
// "no answer yet" for the whole window in which there is no answer.
const waitingForAccount = !settled && (options.enabled ?? true);
return {
...query,
isLoading: query.isLoading || waitingForAccount,
isFetching: query.isFetching || waitingForAccount,
};
}
/**
* Resolve the account identity for an imperative path, fetching the session if
* the cache has no answer yet.
*
* Not `getQueryData` with a `?? null` fallback: an empty or errored session
* entry would read as "signed out" and key an authenticated response to
* `anonymous`. There is no safe default here — the identity is either known or
* must be obtained, and if it cannot be obtained the caller must fail rather
* than guess.
*/
export async function resolveAccountUserId(queryClient: QueryClient): Promise<string | null> {
const state = queryClient.getQueryState<Awaited<ReturnType<typeof authApi.getSession>>>(
queryKeys.auth.session,
);
// `isInvalidated` matters as much as `status` here. Invalidation is how the
// app says "this is the previous account's answer" — InviteLanding invalidates
// the session immediately after a sign-in and resolves an identity in the next
// breath. The entry is still `success`, still holding the *old* user, until a
// refetch lands; trusting it would key the new account's list to the old
// account's id, which is the misattribution this whole file exists to prevent.
if (state?.status === "success" && !state.isInvalidated) {
return state.data?.user.id ?? null;
}
const session = await queryClient.fetchQuery({ ...sessionQueryOptions, staleTime: 0 });
return session?.user.id ?? null;
}
/** Fetch the company list for the account signed in now, ignoring what is cached. */
export async function fetchCompanyListForCurrentAccount(
queryClient: QueryClient,
): Promise<CompanyListResult> {
const userId = await resolveAccountUserId(queryClient);
return queryClient.fetchQuery({
...companyListQueryOptions(userId),
staleTime: 0,
});
}