mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 10:36:26 +02:00
## Thinking Path > - Paperclip manages AI agents and their work. > - Before an agent starts, repository admission checks access to retained workspace files. > - Admission holds a company provenance lock while it checks each source and reader. > - The retained-task query cast the indexed task ID to text. PostgreSQL could evaluate privacy across company history before it matched one source ID. > - Repeated slow checks kept the lock for hours and blocked other agent launches. > - This change looks up the canonical source UUID through the existing primary key before it checks privacy. > - The query does less work and keeps the same access decisions. ## Linked Issues or Issue Description **What happened?** Nine runs appeared active but had no provider process or log file. Read-only database inspection found a transaction holding the company repository-provenance lock for more than ten hours. Eight other transactions waited for that lock. The owner repeatedly ran workspace authorization queries. The company had about 13,400 tasks and 8,500 workspaces. **Expected behavior** A retained-source check should find the referenced task before it evaluates that task's privacy. Unrelated task history should not multiply the privacy work for every source and reader. **Steps to reproduce** 1. Run the new `workspace-authorization-query.test.ts` against authorization code from `5c7a025032`. 2. The real PostgreSQL fixture has 1,200 unrelated tasks and one retained source. The regression fails with 2,402 recursive privacy traversals, above the bound of 100. The other five access tests pass. 3. Apply this change and run the same test. All six tests pass. Related: Refs #16330 adds workspace timing evidence. This PR fixes a measured SQL plan. It does not change those tracing boundaries. ## What Changed - Compare the UUID task column with a guarded canonical UUID from the retained-source JSON key. - Reuse `canonicalExecutionUuid`. Invalid and noncanonical keys still match nothing and cannot cause a UUID parse error. - Add real PostgreSQL coverage for query work, missing and foreign sources, malformed and uppercase IDs, both agent and sponsor grants, and grant revocation. - Add a reproducible baseline comparison and document its scope in `doc/DATABASE.md`. ## Verification - Red: the unchanged query fails the new work-bound regression. Five access cases pass. - Green: 101 tests pass across the new query suite, workspace binding, and the production privacy review suite. - Full `pnpm -r typecheck` and `pnpm build`: pass. Dependencies use the repository's pinned pnpm 9.15.4. - All current-head CI checks pass, including the full server, chat, workspace, serialized, Runner, and eight E2E test lanes. Build, typecheck, canary, and security checks pass. Head: `ad907f6e14c2d773fa8c20692e5ccc10215c9297`. - Greptile: 5/5 on that head, with no actionable findings or unresolved review threads. The branch is mergeable. - The local full `pnpm test:run` was started, then stopped after a concurrent disposable-database check hit the macOS shared-memory limit. This is not counted as a local full-suite pass. The benchmark passed when rerun alone; the complete suite passed in CI. - `git diff --check`: pass. The branch has no lockfile or workflow changes. Synthetic SQL comparison on PostgreSQL 18.1, with 13,429 tasks and one retained source: | Query | Median | Five measured samples | Recursive privacy traversals | | --- | --- | --- | --- | | Baseline `5c7a025032` | 42.57 ms | 42.57, 47.71, 44.51, 41.29, 41.44 ms | 26,860 | | Fixed | 2.37 ms | 2.76, 2.35, 2.37, 2.11, 3.33 ms | 4 | Both versions return identical complete rows. The benchmark alternates variants after two warmup rounds. It loads the baseline authorization module with current dependencies; the UUID helper is unchanged. The measured gain is about 18x for this SQL fixture. The final committed benchmark also passed a second isolated run: 55.44 ms baseline and 3.26 ms fixed, with identical complete rows (about 17x). Two read-only queries on the affected database also returned identical complete rows. One changed from 129 ms to 7 ms; a workspace with 49 retained sources changed from 576 ms to 11 ms. These are single paired measurements, not endpoint or startup benchmarks. Reproduce: ```sh pnpm exec vitest run server/src/__tests__/workspace-authorization-query.test.ts server/src/__tests__/task-workspace-binding.test.ts server/src/__tests__/privacy-production-review.test.ts pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-workspace-authorization.ts --baseline-ref=5c7a025032 pnpm -r typecheck pnpm test:run pnpm build ``` ## Risks - A direct UUID cast could broaden or break access for malformed keys. The existing canonical guard and regression cases preserve the former text-match behavior. - Planner choices can differ across PostgreSQL versions and data sets. The regression bounds real privacy work rather than elapsed time or one exact plan shape. - Admission still checks every retained source and reader under its existing lock. This removes a measured query cost; it does not claim a bound for the full admission process. - No schema migration, permission change, lock removal, or live deployment is included. Recovery of the affected runs remains unverified until rollout. ## Model Used OpenAI GPT-6 through Codex, with tool use and code execution. The exact served model variant and context-window size are not exposed to this task. ## 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 #` 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>
75 lines
5.0 KiB
TypeScript
75 lines
5.0 KiB
TypeScript
import assert from "node:assert/strict";
|
|
import { randomUUID } from "node:crypto";
|
|
import { execFileSync } from "node:child_process";
|
|
import fs from "node:fs/promises";
|
|
import path from "node:path";
|
|
import { fileURLToPath, pathToFileURL } from "node:url";
|
|
import { createRequire } from "node:module";
|
|
const { and, eq } = createRequire(new URL("../server/package.json", import.meta.url))("drizzle-orm");
|
|
import { agents, authUsers, companies, companyMemberships, createDb, executionWorkspaces, issues,
|
|
startEmbeddedPostgresTestDatabase, type Db } from "../packages/db/src/index.js";
|
|
import * as authorization from "../server/src/services/authorization.js";
|
|
|
|
const baselineRef = process.argv.find(arg => arg.startsWith("--baseline-ref="))?.slice(15) ?? "HEAD";
|
|
const baselineRevision = execFileSync("git", ["rev-parse", `${baselineRef}^{commit}`], { encoding: "utf8" }).trim();
|
|
process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce";
|
|
const keepAlive = setInterval(() => {}, 1000);
|
|
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
|
let db: Db | undefined;
|
|
let baselineDir: string | undefined;
|
|
try {
|
|
database = await startEmbeddedPostgresTestDatabase("workspace-authorization-benchmark-");
|
|
db = createDb(database.connectionString);
|
|
baselineDir = await fs.mkdtemp(fileURLToPath(new URL("../server/scripts/.workspace-auth-baseline-", import.meta.url)));
|
|
const original = new URL("../server/src/services/authorization.ts", import.meta.url);
|
|
const source = execFileSync("git", ["show", `${baselineRevision}:server/src/services/authorization.ts`], { encoding: "utf8" });
|
|
const rewritten = source.replace(/from "(\.{1,2}\/[^\"]+)"/g,
|
|
(_match, specifier: string) => `from ${JSON.stringify(fileURLToPath(new URL(specifier, original)))}`);
|
|
const baselinePath = path.join(baselineDir, "authorization.ts");
|
|
await fs.writeFile(baselinePath, rewritten);
|
|
const baseline = await import(pathToFileURL(baselinePath).href) as typeof authorization;
|
|
const companyId = randomUUID(), agentId = randomUUID(), userId = randomUUID(), workspaceId = randomUUID();
|
|
await db.insert(companies).values({ id: companyId, name: "Synthetic workspace authorization", issuePrefix: "BENCH" });
|
|
await db.insert(authUsers).values({ id: userId, name: "Sponsor", email: `${userId}@example.test`, emailVerified: true, createdAt: new Date(), updatedAt: new Date() });
|
|
await db.insert(agents).values({ id: agentId, companyId, name: "Worker", adapterType: "process" });
|
|
await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId, status: "active", membershipRole: "operator" });
|
|
await db.insert(executionWorkspaces).values({ id: workspaceId, companyId, name: "Shared source", mode: "shared_workspace", strategyType: "operator_directory" });
|
|
await db.$client.unsafe(`insert into issues (company_id, title) select $1, 'Historical task ' || n from generate_series(1, 13428) n`, [companyId]);
|
|
const [sourceIssue] = await db.insert(issues).values({ companyId, title: "Retained source after unrelated history" }).returning({ id: issues.id });
|
|
await db.update(executionWorkspaces).set({ sourceIssueId: sourceIssue.id }).where(eq(executionWorkspaces.id, workspaceId));
|
|
await db.$client.unsafe("ANALYZE");
|
|
const actor: authorization.AuthorizationActor = { type: "agent", source: "agent_jwt", agentId, companyId, onBehalfOfUserId: userId };
|
|
const queries = await Promise.all([baseline, authorization].map(async module => db!.select().from(executionWorkspaces).where(and(
|
|
eq(executionWorkspaces.id, workspaceId), eq(executionWorkspaces.companyId, companyId),
|
|
await module.executionWorkspaceReadSqlCondition(db!, actor),
|
|
)).toSQL()));
|
|
const samples: number[][] = [[], []];
|
|
let expected: unknown;
|
|
for (let n = 0; n < 7; n++) {
|
|
for (const variant of n % 2 ? [1, 0] : [0, 1]) {
|
|
const query = queries[variant];
|
|
const start = performance.now();
|
|
const rows = await db.$client.unsafe(query.sql, query.params as never[]).values();
|
|
if (n >= 2) samples[variant].push(performance.now() - start);
|
|
assert.equal(rows.length, 1);
|
|
if (expected === undefined) expected = rows;
|
|
else assert.deepEqual(rows, expected);
|
|
}
|
|
}
|
|
const results = [];
|
|
for (const [variant, query] of queries.entries()) {
|
|
const [plan] = await db.$client.unsafe(`EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${query.sql}`, query.params as never[]);
|
|
results.push({ variant: variant ? "treatment" : "baseline", samplesMs: samples[variant],
|
|
medianMs: [...samples[variant]].sort((a, b) => a - b)[2], sql: query.sql, params: query.params, plan: plan["QUERY PLAN"] });
|
|
}
|
|
console.log(JSON.stringify({ baselineRevision, tasks: 13429, retainedSources: 1, equalCompleteRows: true,
|
|
fixtureIds: { companyId, agentId, userId, workspaceId },
|
|
postgres: (await db.$client.unsafe("select version()"))[0].version, results }, null, 2));
|
|
} finally {
|
|
try {
|
|
await db?.$client.end({ timeout: 0 });
|
|
await database?.cleanup();
|
|
if (baselineDir) await fs.rm(baselineDir, { recursive: true, force: true });
|
|
} finally { clearInterval(keepAlive); }
|
|
}
|