mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-03 17:51:58 +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>
184 lines
13 KiB
TypeScript
184 lines
13 KiB
TypeScript
import assert from "node:assert/strict";
|
|
import { execFileSync } from "node:child_process";
|
|
import { createHash, randomUUID } from "node:crypto";
|
|
import { readFile, mkdtemp, rm, writeFile } from "node:fs/promises";
|
|
import { createRequire } from "node:module";
|
|
import { tmpdir } from "node:os";
|
|
import path from "node:path";
|
|
import { fileURLToPath, pathToFileURL } from "node:url";
|
|
import {
|
|
agents, companies, companyMemberships, createDb, principalPermissionGrants,
|
|
startEmbeddedPostgresTestDatabase, toolApplications, toolConnections, toolConnectionInstalls, type Db,
|
|
} from "../packages/db/src/index.js";
|
|
import { connectionManagementService } from "../server/src/services/connection-management.js";
|
|
import type { AuthorizationActor } from "../server/src/services/authorization.js";
|
|
|
|
// Run explicitly; this benchmark never reads DATABASE_URL or contacts a hosted DB.
|
|
// node --import ./server/node_modules/tsx/dist/esm/index.mjs scripts/benchmark-connection-install-capabilities.ts --baseline-ref=<pre-fix SHA>
|
|
const connectionPurpose = process.argv.find(arg => arg.startsWith("--purpose="))?.slice(10) ?? "tool";
|
|
assert.ok(connectionPurpose === "tool" || connectionPurpose === "ai", "Use --purpose=tool or --purpose=ai");
|
|
const baselineRef = process.argv.find(arg => arg.startsWith("--baseline-ref="))?.slice(15);
|
|
assert.ok(baselineRef, "Supply an explicit pre-fix --baseline-ref");
|
|
const repository = fileURLToPath(new URL("../", import.meta.url));
|
|
const git = (...args: string[]) => execFileSync("git", args, { cwd: repository, encoding: "utf8" }).trim();
|
|
const baselineRevision = git("rev-parse", "--verify", `${baselineRef}^{commit}`);
|
|
const treatmentRevision = git("rev-parse", "HEAD");
|
|
const servicePath = "server/src/services/connection-management.ts";
|
|
const routePath = "server/src/routes/tool-access.ts";
|
|
const runtimeChanges = git("diff", baselineRevision, "--name-only", "--", "server/src", "packages/db/src", "packages/shared/src")
|
|
.split("\n").filter(Boolean).filter(name => !name.includes("/__tests__/") && !/\.test\.[cm]?ts$/.test(name));
|
|
assert.deepEqual(runtimeChanges.filter(name => name !== servicePath && name !== routePath), [],
|
|
"Baseline imports current dependencies: isolate any additional changed runtime dependencies first");
|
|
assert.equal(git("diff", baselineRevision, "--", "pnpm-lock.yaml"), "", "Dependency lock must match the pinned baseline");
|
|
const oldSource = execFileSync("git", ["show", `${baselineRevision}:${servicePath}`], { cwd: repository, encoding: "utf8" });
|
|
const requireFromServer = createRequire(new URL("../server/package.json", import.meta.url));
|
|
const serviceUrl = new URL(`../${servicePath}`, import.meta.url);
|
|
const rewritten = oldSource.replace(/from "([^"]+)"/g, (_match, specifier: string) => {
|
|
const resolved = specifier.startsWith(".")
|
|
? fileURLToPath(new URL(specifier, serviceUrl))
|
|
: requireFromServer.resolve(specifier).replace(/\.cjs$/, ".js");
|
|
return `from ${JSON.stringify(resolved)}`;
|
|
});
|
|
const baselineDirectory = await mkdtemp(path.join(tmpdir(), "connection-install-baseline-"));
|
|
const baselineFile = path.join(baselineDirectory, "connection-management.mts");
|
|
await writeFile(baselineFile, rewritten);
|
|
const baselineModule = await import(pathToFileURL(baselineFile).href) as typeof import("../server/src/services/connection-management.js");
|
|
|
|
type Statement = { sql: string; params: unknown[]; calls: number; rows: number };
|
|
type Counts = { calls: number; rows: number; statements: Map<string, Statement> };
|
|
let activeCounts: Counts | undefined;
|
|
function instrument(db: Db): Db {
|
|
return new Proxy(db, { get(target, key, receiver) {
|
|
if (key !== "select") return Reflect.get(target, key, receiver);
|
|
return (...fields: Parameters<Db["select"]>) => {
|
|
const builder = target.select(...fields);
|
|
const from = builder.from.bind(builder);
|
|
builder.from = ((...args: Parameters<typeof from>) => {
|
|
const query = from(...args), execute = query.execute.bind(query);
|
|
query.execute = (async (...executeArgs: Parameters<typeof execute>) => {
|
|
const counts = activeCounts;
|
|
const compiled = counts ? query.toSQL() : undefined;
|
|
const result = await execute(...executeArgs);
|
|
if (counts && compiled) {
|
|
counts.calls++;
|
|
const rows = Array.isArray(result) ? result.length : 0;
|
|
counts.rows += rows;
|
|
const statement = counts.statements.get(compiled.sql) ?? { ...compiled, calls: 0, rows: 0 };
|
|
statement.calls++;
|
|
statement.rows += rows;
|
|
counts.statements.set(compiled.sql, statement);
|
|
}
|
|
return result;
|
|
}) as typeof execute;
|
|
return query;
|
|
}) as typeof builder.from;
|
|
return builder;
|
|
};
|
|
} });
|
|
}
|
|
function summary(samples: number[]) {
|
|
const sorted = [...samples].sort((a, b) => a - b);
|
|
return { samplesMs: samples, medianMs: sorted[Math.floor(sorted.length / 2)], minMs: sorted[0], maxMs: sorted.at(-1) };
|
|
}
|
|
|
|
const keepAlive = setInterval(() => {}, 1_000);
|
|
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
|
let db: Db | undefined;
|
|
try {
|
|
database = await startEmbeddedPostgresTestDatabase("connection-install-benchmark-");
|
|
db = createDb(database.connectionString);
|
|
const observedDb = instrument(db);
|
|
const old = baselineModule.connectionManagementService(observedDb);
|
|
const treatment = connectionManagementService(observedDb);
|
|
assert.equal(typeof treatment.describeAgentInstallCapabilities, "function", "Implement the production batch first");
|
|
const results = [];
|
|
for (const n of [1, 8, 32, 100]) {
|
|
for (const installs of ["company", "partial"] as const) {
|
|
const companyId = randomUUID(), connectionId = randomUUID(), applicationId = randomUUID(), userId = randomUUID();
|
|
const candidateIds = Array.from({ length: n }, () => randomUUID());
|
|
await db.insert(companies).values({ id: companyId, name: `Capability benchmark ${n} ${installs}`, issuePrefix: `B${results.length}` });
|
|
await db.insert(agents).values(candidateIds.map((id, index) => ({ id, companyId, name: `Agent ${index}`, adapterType: "process" })));
|
|
await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId, status: "active", membershipRole: "member" });
|
|
await db.insert(principalPermissionGrants).values({ companyId, principalType: "user", principalId: userId,
|
|
permissionKey: "tools:manage_profiles", scope: { targetAgentIds: candidateIds } });
|
|
await db.insert(toolApplications).values({ id: applicationId, companyId, name: "Synthetic benchmark", type: "mcp_http" });
|
|
await db.insert(toolConnections).values({ id: connectionId, applicationId, companyId, uid: connectionId,
|
|
name: "Synthetic benchmark", connectionPurpose, transport: connectionPurpose === "ai" ? "runtime_auth" : "mcp_remote", authKind: "none", config: { provider: "fixture" } });
|
|
await db.insert(toolConnectionInstalls).values(installs === "company"
|
|
? [{ companyId, connectionId, targetType: "company", targetId: companyId }]
|
|
: candidateIds.slice(0, Math.max(1, Math.floor(n / 4))).map(targetId => ({ companyId, connectionId, targetType: "agent" as const, targetId })));
|
|
await db.$client.unsafe("ANALYZE");
|
|
const actor: AuthorizationActor = { type: "board", source: "cloud_tenant", userId };
|
|
const resource = { type: "connection_management" as const, companyId, connectionId, operation: "configure", readOnly: false };
|
|
const baseline = async () => {
|
|
const capabilities = [];
|
|
for (const targetAgentId of candidateIds) capabilities.push(await old.capability(actor, "connections:profiles", { ...resource, targetAgentId }));
|
|
return capabilities;
|
|
};
|
|
const batch = () => treatment.describeAgentInstallCapabilities(actor, resource, candidateIds);
|
|
const samples = { baseline: [] as number[], treatment: [] as number[] };
|
|
const counters: Record<string, { calls: number; rows: number }> = {};
|
|
let captured: Record<string, Statement[]> = {};
|
|
for (let iteration = 0; iteration < 7; iteration++) {
|
|
const order = iteration % 2 ? ["treatment", "baseline"] as const : ["baseline", "treatment"] as const;
|
|
const outputs: Record<string, unknown> = {};
|
|
for (const name of order) {
|
|
activeCounts = { calls: 0, rows: 0, statements: new Map() };
|
|
const start = performance.now();
|
|
outputs[name] = await (name === "baseline" ? baseline() : batch());
|
|
const elapsed = performance.now() - start;
|
|
const counts = activeCounts;
|
|
activeCounts = undefined;
|
|
if (iteration >= 2) samples[name].push(elapsed);
|
|
const previous = counters[name];
|
|
const current = { calls: counts.calls, rows: counts.rows };
|
|
if (previous) assert.deepEqual(current, previous, "Stable fixture must generate stable work counts");
|
|
counters[name] = current;
|
|
if (iteration === 6) captured[name] = [...counts.statements.values()];
|
|
}
|
|
assert.deepEqual(outputs.treatment, outputs.baseline, `${installs} N=${n}: full positional capabilities`);
|
|
assert.equal((outputs.baseline as Array<{ allowed: boolean }>).length, n);
|
|
assert.ok((outputs.baseline as Array<{ allowed: boolean }>).every(capability => capability.allowed), "The benchmark must exercise every authorized retained target");
|
|
}
|
|
assert.ok(counters.treatment.calls < counters.baseline.calls || n === 1, "Batch must remove repeated SQL");
|
|
// This is a call-fan-out change. Individual statement plans are controls,
|
|
// not the explanation of the gain; capture them outside measured samples.
|
|
const plans = [];
|
|
for (const statement of captured.treatment) {
|
|
const [plan] = await db.$client.unsafe(`EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${statement.sql}`, statement.params as never[]);
|
|
plans.push({ ...statement, plan: plan["QUERY PLAN"] });
|
|
}
|
|
const before = summary(samples.baseline), after = summary(samples.treatment);
|
|
const targetLookups = (statements: Statement[]) => statements.filter(statement => statement.sql.includes('"agents"."id" =')).reduce((sum, statement) => sum + statement.calls, 0);
|
|
const expectedRetained = installs === "company" ? n : Math.max(1, Math.floor(n / 4));
|
|
assert.equal(targetLookups(captured.baseline), n * expectedRetained + n - expectedRetained);
|
|
assert.equal(targetLookups(captured.treatment), n);
|
|
const result = { n, installs, connectionPurpose, principal: "cloud noncreator member with target-scoped grant", completePositionalEquality: true,
|
|
baseline: { ...before, ...counters.baseline, targetLookups: targetLookups(captured.baseline) },
|
|
treatment: { ...after, ...counters.treatment, targetLookups: targetLookups(captured.treatment) },
|
|
speedup: before.medianMs! / after.medianMs!, statements: captured, treatmentPlans: plans };
|
|
results.push(result);
|
|
console.error(JSON.stringify({ n, installs, baselineMs: before.medianMs, treatmentMs: after.medianMs, counts: counters }));
|
|
}
|
|
}
|
|
const [version] = await db.$client.unsafe("SELECT version() AS version");
|
|
const settings = await db.$client.unsafe("SELECT name, setting, unit FROM pg_settings WHERE name IN ('shared_buffers','work_mem','effective_cache_size','jit') ORDER BY name");
|
|
console.log(JSON.stringify({ baselineRevision, treatmentRevision,
|
|
benchmarkSourceSha256: createHash("sha256").update(await readFile(fileURLToPath(import.meta.url))).digest("hex"),
|
|
treatmentSourceSha256: createHash("sha256").update(await readFile(new URL(`../${servicePath}`, import.meta.url))).digest("hex"),
|
|
uncommittedDiffSha256: createHash("sha256").update(git("diff", "HEAD")).digest("hex"),
|
|
dependencyLockfileSha256: createHash("sha256").update(await readFile(new URL("../pnpm-lock.yaml", import.meta.url))).digest("hex"),
|
|
baselineIsolation: { copiedModule: servicePath, imports: "Absolute current paths; all other server/shared/db runtime sources and lockfile verified unchanged from baseline", runtimeChanges },
|
|
node: process.version, postgres: version.version, settings, warmups: 2, samples: 5, alternatingOrder: true,
|
|
measurement: "Real service evaluation with real local PostgreSQL; select statement executions and returned Drizzle rows; no simulated transport latency",
|
|
limits: "Descriptive service batch only; no hosted latency attribution, agent fast path, route timing, concurrent revocation, or subtree hierarchy scaling claim",
|
|
results }, null, 2));
|
|
} finally {
|
|
activeCounts = undefined;
|
|
try {
|
|
await db?.$client.end({ timeout: 0 });
|
|
await database?.cleanup();
|
|
await rm(baselineDirectory, { recursive: true, force: true });
|
|
} finally { clearInterval(keepAlive); }
|
|
}
|