mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 10:40:33 +01:00
## Thinking Path > - Paperclip manages AI agents and their access to saved connections. > - The Permissions page loads account details and permitted agent choices. > - Company-wide AI accounts repeat retained installation checks for every candidate agent. > - This makes target checks grow with the square of the roster size. > - The browser waits without a deadline, and connection names do not identify the provider login. > - This pull request batches direct AI checks and bounds loading with recovery controls. > - Account details load separately and show a provider email or an explicit profile-permission limit. ## Linked Issues or Issue Description **What happened?** Opening a saved AI account can leave Permissions on “Loading AI account…” for minutes. The account itself can be connected. The existing capability batch excludes AI connections, so a company-wide install repeats checks for every installed agent for every candidate. Saved names and Paperclip ownership also do not establish which provider login a credential uses. **Expected behavior** Direct AI accounts reuse retained-target checks within one descriptive request. The page shows loaded account controls or a bounded failure with Retry. Permissions shows the provider email and organization when the selected credential permits profile access. An unavailable profile must not block maintenance or disconnect a working inference credential. Writes still authorize current permissions. **Steps to reproduce** 1. Save an AI account in a company with several agents. 2. Install the account company-wide. 3. Open its Permissions page. 4. Delay the account-list or grants response to exercise the loading failure. The maintained benchmark reproduces database work with synthetic local data. Refs #17945, which added batching for non-AI connections. Searches found no open PR for this AI-account loading fix. ## What Changed - Include direct AI accounts in the existing board capability batch. Referenced pool members, delegated actors, complex selectors, and mutation authorization keep their existing evaluator. - Add opt-in per-caller GET deadlines to the API client. A timed-out caller detaches the stuck coalesced request so Retry starts a fresh fetch. Other callers retain their own cancellation and results. - Apply a 15-second deadline to AI account lists and grants reads. The permissions view retries transient failures once and reports other failures immediately. - Share the parent Permissions grants query with AI account details. Keep loaded controls visible after a background error and offer Retry. - Add a grant-scoped identity GET and OpenAPI contract. The service checks current membership and credential audience before it reads the selected credential. It projects only the provider email and organization; provider IDs, tokens, and raw errors stay private. - Show account details independently of maintenance controls. Anthropic profile denial has an explicit message and preserves connected status. Transient profile failures have a separate Retry action. Observations are cached for five minutes under the company, connection, grant, and grant revision. - Extend real PostgreSQL and authenticated HTTP coverage, client/component regressions, map guidance, and the existing benchmark's AI mode. - Address the Apex file-size finding by moving gallery identity lookup into the existing app display helper module. `AppDetail.tsx` is 984 lines; lookup behavior is unchanged. ## Verification Local source checkout on macOS, Node 25.6.1, pnpm 9.15.4, and disposable PostgreSQL 18.1. Provider calls use fixtures. No hosted deployment or provider qualification is claimed. - Passed `pnpm -r typecheck` and `pnpm build`. - Passed all 87 tests in `connection-management.integration.test.ts` and all 33 tests in `connection-install-capabilities.test.ts`. - Passed 196 AI connection, identity-provider, and OpenAPI tests: `ai-connections.test.ts`, `ai-account-identity.test.ts`, and `openapi-routes.test.ts`. - Passed 107 focused UI/client tests: `client.test.ts`, `ManagedAiConnectionDetails.test.tsx`, `AiConnectionAccountDetails.test.tsx`, and `AppDetail.test.tsx`. - After the Apex extraction, passed UI typecheck/build and 106 component tests across `AppDetail.test.tsx`, `AppNotConnected.test.tsx`, `ManagedAiConnectionDetails.test.tsx`, and `AiConnectionAccountDetails.test.tsx`. - The authenticated HTTP tests compare complete grants responses across human roles, for tool and AI connections. They verify a real install mutation succeeds before revocation and fails afterward. - A company-wide personal AI fixture checks 20 targets instead of 400. Pool-member permission loss still denies the complete pool description. - UI states covered: shared cached grants, stuck account/grants fetches, one automatic retry, terminal failure, manual recovery, authorization failure, background failure with retained controls, and unmount cancellation. Client tests also cover stalled response bodies and independent coalesced callers. - Identity coverage includes actual bearer-key/JWT reads, private-user and cross-company denial before provider access, revoked credentials, provider 401/403/429/500, and grant-change isolation. A real local HTTP fixture stalls the response body, verifies the shared 15-second deadline, and keeps unrelated HTTP requests responsive. The test shortens the native deadline to 100 ms. Profile permission denial leaves the saved account connected. - Passed `pnpm check:token-gates`, `node scripts/check-maps.mjs`, `node scripts/check-module-boundaries.mjs`, and `git diff --check`. - The full local `pnpm test:run` started with the loading-only change. All six chat shards passed. It recorded one failure in the unchanged `heartbeat-retry-scheduling.test.ts` quota-rejection case. That exact case passed a focused rerun on the final head. The initial failure cause remains unconfirmed and its local log is retained; this is not a full local-suite pass. The redundant local run was stopped with SIGINT after final-head CI completed successfully. - [Final-head CI](https://github.com/paperclipai/paperclip/actions/runs/41515556725) passed on `ead8d276d4a2023614fff54b3e9a858f28c3178b`: typecheck, build, all 12 server shards, all 3 chat shards, workspace suites, all 9 serialized suites, all 8 end-to-end shards, Runner checks, and the aggregate verify gate. Security checks passed. Optional Storybook checks were skipped by workflow policy. GitHub reports the branch mergeable without conflicts. - Apex reviewed head `ead8d276d4a2023614fff54b3e9a858f28c3178b` at 5/5, confirmed the extraction resolves its earlier file-size finding, and has no unresolved review threads. The existing benchmark ran for the loading-only change now recorded in commit `5f725b36fa52002545241a5a8b7a10602f0cc0ce`, with `--purpose=ai --baseline-ref=026fd9e3452`. The identity follow-up leaves that batch source unchanged. Two warmups and five alternating samples exercise each of eight fixture shapes. Complete positional capabilities, reasons, and operations match the baseline in every sample. | AI fixture | SQL calls before → after | Target reads before → after | | --- | ---: | ---: | | 1 agent, company install | 9 → 9 | 1 → 1 | | 32 agents, company install | 5,248 → 164 | 1,024 → 32 | | 100 agents, company install | 50,400 → 504 | 10,000 → 100 | | 100 agents, partial install | 13,175 → 503 | 2,575 → 100 | These counts measure the real service over local PostgreSQL. They do not predict hosted page latency. To reproduce the standalone benchmark, use a checkout of commit `5f725b36fa52002545241a5a8b7a10602f0cc0ce` and run the command below. Its isolation check intentionally rejects the later identity follow-up's additional runtime changes. ```sh node --import ./server/node_modules/tsx/dist/esm/index.mjs scripts/benchmark-connection-install-capabilities.ts --purpose=ai --baseline-ref=026fd9e3452 ``` Run the final-head regression suites from the repository root: ```sh cd server pnpm exec vitest run src/__tests__/connection-management.integration.test.ts src/__tests__/connection-install-capabilities.test.ts src/__tests__/ai-connections.test.ts src/services/ai-account-identity.test.ts src/__tests__/openapi-routes.test.ts cd ../ui pnpm exec vitest run src/api/client.test.ts src/components/ai-connections/ManagedAiConnectionDetails.test.tsx src/components/ai-connections/AiConnectionAccountDetails.test.tsx src/pages/apps/AppDetail.test.tsx ``` ## Risks Descriptive permissions reuse retained facts within one request. A concurrent change can leave a displayed choice stale; the next request and every write still authorize fresh. Pools with referenced members retain their existing cost. The account and grants clients now bound each caller's wait. A browser abort does not cancel an authorization read already admitted by the server. The independent profile request uses a fixed Anthropic origin, a bounded body, no redirects, and a 15-second deadline. Inference-only credentials can return no email; that is reported without guessing from a Paperclip owner or changing connection health. Profiles are fetched only for the caller's authorized selected grant. No ambient CLI identity, persistent identity cache, background polling, or billing-identity change is added. Other API reads keep their existing behavior. No migration is required. Staging has not been redeployed. ## Model Used OpenAI `gpt-6.1-sol` through the Codex harness in T3 Code, with xhigh reasoning, code editing, execution, and test tools. The host does not expose the context-window size. ## 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
310 lines
12 KiB
TypeScript
310 lines
12 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import {
|
|
createTenantSessionRecoveryCoordinator,
|
|
tenantSessionRecovery,
|
|
} from "@/lib/tenant-session-recovery";
|
|
import { api, detachInflightGet } from "./client";
|
|
import { toolsApi } from "./tools";
|
|
import { museInvitationsApi } from "./museInvitations";
|
|
|
|
interface Deferred<T> {
|
|
promise: Promise<T>;
|
|
resolve: (value: T) => void;
|
|
reject: (reason?: unknown) => void;
|
|
}
|
|
|
|
function deferred<T>(): Deferred<T> {
|
|
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 };
|
|
}
|
|
|
|
function jsonResponse(body: unknown) {
|
|
return { ok: true, status: 200, json: async () => body } as unknown as Response;
|
|
}
|
|
|
|
function errorResponse(body: unknown, status = 401) {
|
|
return { ok: false, status, json: async () => body } as unknown as Response;
|
|
}
|
|
|
|
const fetchMock = vi.fn();
|
|
|
|
beforeEach(() => {
|
|
fetchMock.mockReset();
|
|
vi.stubGlobal("fetch", fetchMock);
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
vi.unstubAllGlobals();
|
|
vi.useRealTimers();
|
|
});
|
|
|
|
describe("tenant-session recovery", () => {
|
|
it("loads anonymous Dot consent despite concurrent protected background probes", async () => {
|
|
const reload = vi.fn();
|
|
const requestId = `pcmcp_request_${"a".repeat(43)}`;
|
|
const recovery = createTenantSessionRecoveryCoordinator(reload, () => `/dot-connect/${requestId}`);
|
|
vi.spyOn(tenantSessionRecovery, "recoverIfNeeded").mockImplementation(recovery.recoverIfNeeded);
|
|
fetchMock.mockImplementation((url: string) => Promise.resolve(
|
|
url.includes("/dot-mcp/requests/")
|
|
? jsonResponse({ id: requestId, agentConnection: true })
|
|
: errorResponse({ error: "tenant_session_required" }),
|
|
));
|
|
|
|
const background = api.get("/health");
|
|
const consent = api.get(`/dot-mcp/requests/${requestId}`);
|
|
await expect(background).rejects.toMatchObject({ status: 401 });
|
|
await expect(consent).resolves.toEqual({ id: requestId, agentConnection: true });
|
|
expect(reload).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("keeps concurrent failures pending and schedules one top-level reload", async () => {
|
|
const reload = vi.fn();
|
|
const recovery = createTenantSessionRecoveryCoordinator(reload);
|
|
vi.spyOn(tenantSessionRecovery, "recoverIfNeeded").mockImplementation(recovery.recoverIfNeeded);
|
|
fetchMock.mockResolvedValue(errorResponse({ error: "tenant_session_required" }));
|
|
|
|
const first = api.post("/tenant-session-a", { value: 1 });
|
|
const second = api.post("/tenant-session-b", { value: 2 });
|
|
|
|
await vi.waitFor(() => {
|
|
expect(tenantSessionRecovery.recoverIfNeeded).toHaveBeenCalledTimes(2);
|
|
expect(reload).toHaveBeenCalledTimes(1);
|
|
});
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
|
|
let settled = false;
|
|
void Promise.all([first, second]).then(
|
|
() => { settled = true; },
|
|
() => { settled = true; },
|
|
);
|
|
await Promise.resolve();
|
|
expect(settled).toBe(false);
|
|
});
|
|
|
|
it("preserves ApiError handling for an unrelated 401", async () => {
|
|
fetchMock.mockResolvedValue(errorResponse({ error: "unauthorized" }));
|
|
|
|
await expect(api.get("/ordinary-401")).rejects.toMatchObject({
|
|
name: "ApiError",
|
|
status: 401,
|
|
message: "unauthorized",
|
|
});
|
|
|
|
fetchMock.mockResolvedValue(jsonResponse({ recovered: true }));
|
|
await expect(api.get("/ordinary-401")).resolves.toEqual({ recovered: true });
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
});
|
|
});
|
|
|
|
describe("in-tab GET coalescing", () => {
|
|
it("times out a caller without cancelling another and starts a fresh fetch on retry", async () => {
|
|
vi.useFakeTimers();
|
|
const first = deferred<Response>();
|
|
fetchMock.mockReturnValueOnce(first.promise);
|
|
const survivorController = new AbortController();
|
|
const survivor = api.get("/deadline-shared", { signal: survivorController.signal });
|
|
const timed = api.get("/deadline-shared", { timeoutMs: 100 }).catch(error => error);
|
|
const sharedSignal = fetchMock.mock.calls[0][1].signal as AbortSignal;
|
|
await vi.advanceTimersByTimeAsync(100);
|
|
expect(await timed).toMatchObject({ status: 408, body: { error: "TIMEOUT" } });
|
|
expect(sharedSignal.aborted).toBe(false);
|
|
fetchMock.mockResolvedValueOnce(jsonResponse({ fresh: true }));
|
|
await expect(api.get("/deadline-shared", { timeoutMs: 100 })).resolves.toEqual({ fresh: true });
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
survivorController.abort();
|
|
await expect(survivor).rejects.toMatchObject({ name: "AbortError" });
|
|
expect(sharedSignal.aborted).toBe(true);
|
|
first.resolve(jsonResponse({ stale: true }));
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(vi.getTimerCount()).toBe(0);
|
|
});
|
|
|
|
it("bounds response-body reads and aborts the last caller's fetch", async () => {
|
|
vi.useFakeTimers();
|
|
const body = deferred<unknown>();
|
|
fetchMock.mockResolvedValue({ ok: true, status: 200, json: () => body.promise });
|
|
const result = api.get("/deadline-body", { timeoutMs: 100 }).catch(error => error);
|
|
const signal = fetchMock.mock.calls[0][1].signal as AbortSignal;
|
|
await vi.advanceTimersByTimeAsync(100);
|
|
expect(await result).toMatchObject({ status: 408 });
|
|
expect(signal.aborted).toBe(true);
|
|
body.resolve({ tooLate: true });
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(vi.getTimerCount()).toBe(0);
|
|
});
|
|
|
|
it("clears a successful or cancelled caller's deadline", async () => {
|
|
vi.useFakeTimers();
|
|
fetchMock.mockResolvedValueOnce(jsonResponse({ ok: true }));
|
|
await expect(api.get("/deadline-success", { timeoutMs: 100 })).resolves.toEqual({ ok: true });
|
|
expect(vi.getTimerCount()).toBe(0);
|
|
const controller = new AbortController();
|
|
fetchMock.mockReturnValueOnce(deferred<Response>().promise);
|
|
const result = api.get("/deadline-cancel", { signal: controller.signal, timeoutMs: 100 });
|
|
controller.abort();
|
|
await expect(result).rejects.toMatchObject({ name: "AbortError" });
|
|
expect(vi.getTimerCount()).toBe(0);
|
|
});
|
|
|
|
it("shares one underlying fetch for identical in-flight GETs", async () => {
|
|
const d = deferred<Response>();
|
|
fetchMock.mockReturnValue(d.promise);
|
|
|
|
const p1 = api.get("/coalesce-a");
|
|
const p2 = api.get("/coalesce-a");
|
|
expect(fetchMock).toHaveBeenCalledTimes(1);
|
|
|
|
d.resolve(jsonResponse({ value: 1 }));
|
|
expect(await p1).toEqual({ value: 1 });
|
|
expect(await p2).toEqual({ value: 1 });
|
|
});
|
|
|
|
it("issues a fresh fetch after the previous one settles", async () => {
|
|
fetchMock.mockResolvedValue(jsonResponse({ value: "x" }));
|
|
await api.get("/coalesce-b");
|
|
await api.get("/coalesce-b");
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
});
|
|
|
|
it("does not coalesce different paths", async () => {
|
|
fetchMock.mockReturnValue(deferred<Response>().promise);
|
|
const c1 = new AbortController();
|
|
const c2 = new AbortController();
|
|
const p1 = api.get("/path-1", { signal: c1.signal });
|
|
const p2 = api.get("/path-2", { signal: c2.signal });
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
// Clean up the never-settling shared entries so the module map does not leak.
|
|
c1.abort();
|
|
c2.abort();
|
|
await expect(p1).rejects.toMatchObject({ name: "AbortError" });
|
|
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<Response>();
|
|
fetchMock.mockReturnValueOnce(first.promise);
|
|
const previousAccount = api.get("/detach-me");
|
|
|
|
detachInflightGet("/detach-me");
|
|
|
|
const second = deferred<Response>();
|
|
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"] });
|
|
});
|
|
|
|
it("never coalesces mutations", async () => {
|
|
fetchMock.mockResolvedValue(jsonResponse({ ok: true }));
|
|
await Promise.all([api.post("/mutate", { a: 1 }), api.post("/mutate", { a: 1 })]);
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
});
|
|
});
|
|
|
|
describe("per-caller abort semantics", () => {
|
|
it("aborting one caller rejects only that caller, not the shared fetch", async () => {
|
|
const d = deferred<Response>();
|
|
let sharedSignal: AbortSignal | undefined;
|
|
fetchMock.mockImplementation((_url: string, init: RequestInit) => {
|
|
sharedSignal = init.signal ?? undefined;
|
|
return d.promise;
|
|
});
|
|
|
|
const c1 = new AbortController();
|
|
const c2 = new AbortController();
|
|
const p1 = api.get("/abort-a", { signal: c1.signal });
|
|
const p2 = api.get("/abort-a", { signal: c2.signal });
|
|
|
|
c1.abort();
|
|
await expect(p1).rejects.toMatchObject({ name: "AbortError" });
|
|
// The shared fetch is still alive because caller 2 has not aborted.
|
|
expect(sharedSignal?.aborted).toBe(false);
|
|
|
|
d.resolve(jsonResponse({ value: 2 }));
|
|
expect(await p2).toEqual({ value: 2 });
|
|
});
|
|
|
|
it("aborts the shared fetch after all callers cancel and permits a fresh request", async () => {
|
|
const d = deferred<Response>();
|
|
let sharedSignal: AbortSignal | undefined;
|
|
fetchMock.mockImplementation((_url: string, init: RequestInit) => {
|
|
sharedSignal = init.signal ?? undefined;
|
|
return d.promise;
|
|
});
|
|
|
|
const c1 = new AbortController();
|
|
const c2 = new AbortController();
|
|
const p1 = api.get("/abort-b", { signal: c1.signal });
|
|
const p2 = api.get("/abort-b", { signal: c2.signal });
|
|
|
|
c1.abort();
|
|
c2.abort();
|
|
await expect(p1).rejects.toMatchObject({ name: "AbortError" });
|
|
await expect(p2).rejects.toMatchObject({ name: "AbortError" });
|
|
expect(sharedSignal?.aborted).toBe(true);
|
|
|
|
fetchMock.mockResolvedValueOnce(jsonResponse({ value: "fresh" }));
|
|
const fresh = api.get("/abort-b");
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
expect(await fresh).toEqual({ value: "fresh" });
|
|
});
|
|
|
|
it("rejects immediately if the caller signal is already aborted", async () => {
|
|
const controller = new AbortController();
|
|
controller.abort();
|
|
await expect(api.get("/already-aborted", { signal: controller.signal })).rejects.toMatchObject({
|
|
name: "AbortError",
|
|
});
|
|
expect(fetchMock).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
|
|
describe("managed-account request isolation", () => {
|
|
it("does not share a previous viewing user's pending account response", async () => {
|
|
const previous = deferred<Response>();
|
|
const current = deferred<Response>();
|
|
fetchMock.mockReturnValueOnce(previous.promise).mockReturnValueOnce(current.promise);
|
|
const previousUser = toolsApi.listAggregatorApps("shared-gateway");
|
|
const currentUser = toolsApi.listAggregatorApps("shared-gateway");
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
expect(fetchMock.mock.calls.every(([, options]) => options.cache === "no-store")).toBe(true);
|
|
current.resolve(jsonResponse({ apps: ["current-user-account"] }));
|
|
previous.resolve(jsonResponse({ apps: ["previous-user-account"] }));
|
|
expect(await currentUser).toEqual({ apps: ["current-user-account"] });
|
|
expect(await previousUser).toEqual({ apps: ["previous-user-account"] });
|
|
});
|
|
});
|
|
|
|
describe("Muse operator request isolation", () => {
|
|
it.each([
|
|
["invitation resume", () => museInvitationsApi.resume("company")],
|
|
["connection evidence", () => museInvitationsApi.connection("company", "agent")],
|
|
] as const)("does not join the previous operator's %s request", async (_name, read) => {
|
|
const previous = deferred<Response>();
|
|
const current = deferred<Response>();
|
|
fetchMock.mockReturnValueOnce(previous.promise).mockReturnValueOnce(current.promise);
|
|
const previousOperator = read();
|
|
const currentOperator = read();
|
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
|
expect(fetchMock.mock.calls.every(([, options]) => options.cache === "no-store")).toBe(true);
|
|
current.resolve(jsonResponse({ operator: "current" }));
|
|
previous.resolve(jsonResponse({ operator: "previous" }));
|
|
expect(await currentOperator).toEqual({ operator: "current" });
|
|
expect(await previousOperator).toEqual({ operator: "previous" });
|
|
});
|
|
});
|