From 7494b8dd4c113a9f290e9dc5fbda4f61776f5d67 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 25 Oct 2026 00:35:07 -0700 Subject: [PATCH] perf(auth): batch current run history read decisions (#17569) Reuse the existing authorization batch owner for current run-history read decisions after company-scoped existence checks. Preserve missing-task denial, per-request freshness, and the decide-only adapter path. Validation: 97 focused tests, full build and repository typecheck. A real PostgreSQL fixture returns identical decisions with 4 queries instead of 327; the original path fails the query-bound regression. Exact-head Greptile 5/5 (medium), all CI checks green, and no unresolved comments. Co-Authored-By: Paperclip --- doc/DEVELOPING.md | 9 ++ .../heartbeat-run-privacy-routes.test.ts | 105 +++++++++++++++++- server/src/services/access.ts | 1 + server/src/services/heartbeat-run-privacy.ts | 43 ++++++- 4 files changed, 153 insertions(+), 5 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 20b6ce6563d..3cc2067d727 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -907,6 +907,15 @@ can update an entry only after the current account's summary confirms access. The first healthy live subscription reconciles domain data without re-fetching session and health; reconnects and visibility recovery retain full reconciliation. +### Run history authorization reads + +Run history reuses the authorization service's batch read API for distinct task +bindings. Existence checks remain company-scoped and use batches of at most 500 +IDs. Missing or foreign tasks stay unreadable in every privacy mode. The batch +snapshot lasts for one request only; the next request checks current grants, +membership, and run restrictions again. Adapters that expose only `decide` +retain the individual-read path with duplicate bindings shared within the request. + ### Local Linux runner qualification Before serving requests, the server reads `getconf GNU_LIBC_VERSION` once with diff --git a/server/src/__tests__/heartbeat-run-privacy-routes.test.ts b/server/src/__tests__/heartbeat-run-privacy-routes.test.ts index 40d966ed8e8..ed10d2548f2 100644 --- a/server/src/__tests__/heartbeat-run-privacy-routes.test.ts +++ b/server/src/__tests__/heartbeat-run-privacy-routes.test.ts @@ -483,6 +483,105 @@ describeEmbeddedPostgres("heartbeat run privacy routes", { concurrent: false }, process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce"; } }); + it("batches distinct issue bindings without changing current authorization decisions", async () => { + const fixture = await seedFixture(); + const ids = Array.from({ length: 64 }, () => randomUUID()); + await db.insert(issues).values(ids.map((id, index) => ({ + id, companyId: fixture.companyId, title: `History ${index}`, status: "in_progress", + visibility: index % 2 ? "private" : "open", privacyRootIssueId: index % 2 ? id : null, + assigneeAgentId: fixture.ownerAgentId, responsibleUserId: fixture.ownerUserId, + }))); + const queries: string[] = []; + const observedDb = drizzle(db.$client, { schema, logger: { logQuery(query) { queries.push(query); } } }) as typeof db; + const access = authorizationService(observedDb); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.grantedAgentId }; + const bindings = [...ids, fixture.issueId].map(issueId => ({ companyId: fixture.companyId, scopeKind: "issue" as const, issueId })); + const runs = [...bindings, ...bindings, { ...bindings[0], companyId: randomUUID() }, + { ...bindings[0], issueId: randomUUID() }, { companyId: fixture.companyId, scopeKind: "company" as const }, + { companyId: fixture.companyId, scopeKind: "issue" as const }]; + const individual = await getHeartbeatRunReadDecisions(observedDb, { decide: access.decide }, actor, runs); + const individualCount = queries.length; + queries.length = 0; + const batched = await getHeartbeatRunReadDecisions(observedDb, access, actor, runs); + expect(batched).toEqual(individual); + expect(batched.slice(0, 64)).toEqual(ids.map((_, index) => index % 2 === 0)); + expect(batched.at(64)).toBe(true); + expect(batched.slice(-4)).toEqual([false, false, true, false]); + expect(queries.length).toBeLessThan(25); + expect(queries.length).toBeLessThan(individualCount / 4); + await db.update(issueAccessGrants).set({ revokedAt: new Date() }).where(eq(issueAccessGrants.subjectId, fixture.grantedAgentId)); + expect((await getHeartbeatRunReadDecisions(observedDb, access, actor, runs)).at(64)).toBe(false); + }); + + it.each(["enforce", "shadow", "off"])("keeps missing and foreign issue bindings closed in %s mode", async mode => { + const fixture = await seedFixture(); + process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = mode; + try { + const access = authorizationService(db); + const actor = { type: "board" as const, userId: fixture.ownerUserId }; + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + const runs = [binding, { ...binding, issueId: randomUUID() }, { ...binding, companyId: randomUUID() }]; + const expected = await getHeartbeatRunReadDecisions(db, { decide: access.decide }, actor, runs); + expect(await getHeartbeatRunReadDecisions(db, access, actor, runs)).toEqual(expected); + expect(expected).toEqual([true, false, false]); + await db.delete(companyMemberships).where(eq(companyMemberships.principalId, fixture.ownerUserId)); + expect(await getHeartbeatRunReadDecisions(db, access, actor, [binding])).toEqual( + await getHeartbeatRunReadDecisions(db, { decide: access.decide }, actor, [binding]), + ); + } finally { process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce"; } + }); + + it("checks more than 500 task bindings with bounded company-scoped queries", async () => { + const fixture = await seedFixture(); + const ids = Array.from({ length: 501 }, () => randomUUID()); + await db.insert(issues).values(ids.map((id, index) => ({ + id, companyId: fixture.companyId, title: `Batch ${index}`, visibility: "open", status: "in_progress", + }))); + const queries: { query: string; params: unknown[] }[] = []; + const observedDb = drizzle(db.$client, { schema, logger: { logQuery(query, params) { queries.push({ query, params }); } } }) as typeof db; + const access = authorizationService(observedDb); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + const runs = ids.map(issueId => ({ companyId: fixture.companyId, scopeKind: "issue" as const, issueId })); + expect(await getHeartbeatRunReadDecisions(observedDb, access, actor, runs)).toEqual(ids.map(() => true)); + const existence = queries.filter(({ query }) => query.startsWith('select "id" from "issues"')); + expect(existence.map(({ params }) => params.length).sort((a, b) => a - b)).toEqual([2, 501]); + expect(existence.every(({ query, params }) => query.includes('"company_id"') && params[0] === fixture.companyId)).toBe(true); + }); + + it("captures batch inputs before awaiting reads and rejects a failed batch", async () => { + const fixture = await seedFixture(); + const access = authorizationService(db); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + const decideReads = vi.spyOn(access, "decideReads"); + const pending = getHeartbeatRunReadDecisions(db, access, actor, [binding, binding]); + binding.issueId = randomUUID(); + expect(await pending).toEqual([true, true]); + expect(decideReads).toHaveBeenCalledExactlyOnceWith(actor, [expect.objectContaining({ issueId: fixture.issueId })]); + const error = new Error("authorization unavailable"); + decideReads.mockRejectedValueOnce(error); + const current = { ...binding, issueId: fixture.issueId }; + await expect(getHeartbeatRunReadDecisions(db, access, actor, [current])).rejects.toBe(error); + const transient = Object.assign(new Error("temporary disconnect"), { code: "CONNECTION_CLOSED" }); + decideReads.mockRejectedValueOnce(transient); + expect(await getHeartbeatRunReadDecisions(db, access, actor, [current])).toEqual([true]); + }); + + it("does no database or authorization work for company-only or empty batches", async () => { + const fixture = await seedFixture(); + const queries: string[] = []; + const observedDb = drizzle(db.$client, { schema, logger: { logQuery(query) { queries.push(query); } } }) as typeof db; + const access = authorizationService(observedDb); + const decideReads = vi.spyOn(access, "decideReads"); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + expect(await getHeartbeatRunReadDecisions(observedDb, access, actor, [])).toEqual([]); + expect(await getHeartbeatRunReadDecisions(observedDb, access, actor, [ + { companyId: fixture.companyId, scopeKind: "company" }, { companyId: fixture.companyId, scopeKind: "issue" }, + ])).toEqual([true, false]); + expect(queries).toEqual([]); + expect(decideReads).not.toHaveBeenCalled(); + }); + it.each(["owner", "outsider"])("checks concurrent duplicate issue runs once for the %s", async (viewer) => { const fixture = await seedFixture(); const access = authorizationService(db); @@ -493,7 +592,7 @@ describeEmbeddedPostgres("heartbeat run privacy routes", { concurrent: false }, agentId: viewer === "owner" ? fixture.ownerAgentId : fixture.otherAgentId, }; const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; - const decisions = await getHeartbeatRunReadDecisions(db, access, actor, Array.from({ length: 200 }, () => ({ ...binding }))); + const decisions = await getHeartbeatRunReadDecisions(db, { decide: access.decide }, actor, Array.from({ length: 200 }, () => ({ ...binding }))); expect(decisions).toEqual(Array(200).fill(viewer === "owner")); expect(decide).toHaveBeenCalledTimes(1); decide.mockRestore(); @@ -517,7 +616,7 @@ describeEmbeddedPostgres("heartbeat run privacy routes", { concurrent: false }, const decide = vi.spyOn(access, "decide"); const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; - const pending = getHeartbeatRunReadDecisions(db, access, actor, [binding, binding]); + const pending = getHeartbeatRunReadDecisions(db, { decide: access.decide }, actor, [binding, binding]); binding.issueId = randomUUID(); expect(await pending).toEqual([true, true]); expect(decide).toHaveBeenCalledExactlyOnceWith(expect.objectContaining({ @@ -591,7 +690,7 @@ describeEmbeddedPostgres("heartbeat run privacy routes", { concurrent: false }, const decide = vi.spyOn(access, "decide").mockRejectedValue(error); const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; - await expect(getHeartbeatRunReadDecisions(db, access, actor, Array(20).fill(binding))).rejects.toBe(error); + await expect(getHeartbeatRunReadDecisions(db, { decide: access.decide }, actor, Array(20).fill(binding))).rejects.toBe(error); expect(decide).toHaveBeenCalledTimes(1); decide.mockRestore(); expect(await getHeartbeatRunReadDecisions(db, access, actor, [binding])).toEqual([true]); diff --git a/server/src/services/access.ts b/server/src/services/access.ts index fced6f2c698..d7ae99b09a7 100644 --- a/server/src/services/access.ts +++ b/server/src/services/access.ts @@ -1162,6 +1162,7 @@ export function accessService(db: Db) { return { isInstanceAdmin, decide, + decideReads: authorization.decideReads, projectDiscoveryCandidateIds: authorization.projectDiscoveryCandidateIds, canUser, hasPermission, diff --git a/server/src/services/heartbeat-run-privacy.ts b/server/src/services/heartbeat-run-privacy.ts index 17b61f4cbbd..d1cb41a19a2 100644 --- a/server/src/services/heartbeat-run-privacy.ts +++ b/server/src/services/heartbeat-run-privacy.ts @@ -1,7 +1,7 @@ import type { Db } from "@paperclipai/db"; import { issues, heartbeatRuns } from "@paperclipai/db"; -import { and, eq } from "drizzle-orm"; -import { canActorReadExecutionWorkspace, type AuthorizationActor, type AuthorizationDecision } from "./authorization.js"; +import { and, eq, inArray } from "drizzle-orm"; +import { canActorReadExecutionWorkspace, type AuthorizationActor, type AuthorizationDecision, type authorizationService } from "./authorization.js"; import { retryIdempotentDatabaseOperation } from "../database-retry.js"; type RunIssueBinding = { @@ -13,6 +13,7 @@ type RunIssueBinding = { export async function canActorReadHeartbeatRun( db: Db, access: { + decideReads?: ReturnType["decideReads"]; decide(input: { actor: AuthorizationActor; action: "issue:read"; @@ -66,6 +67,44 @@ export async function getHeartbeatRunReadDecisions( actor: AuthorizationActor, runs: readonly RunIssueBinding[], ): Promise { + const decideReads = access.decideReads; + if (decideReads) { + const bindings = runs.map(run => ({ companyId: run.companyId, scopeKind: run.scopeKind, issueId: run.issueId })); + const key = (companyId: string, issueId: string) => JSON.stringify([companyId, issueId]); + const issuesByCompany = new Map>(); + for (const run of bindings) { + if (run.scopeKind === "company" || !run.issueId) continue; + let ids = issuesByCompany.get(run.companyId); + if (!ids) { ids = new Set(); issuesByCompany.set(run.companyId, ids); } + ids.add(run.issueId); + } + const resources: Parameters[1] = []; + for (const [companyId, issueIds] of issuesByCompany) { + const ids = [...issueIds]; + for (let offset = 0; offset < ids.length; offset += 500) { + const chunk = ids.slice(offset, offset + 500); + const existing = await retryIdempotentDatabaseOperation(() => db + .select({ id: issues.id }) + .from(issues) + .where(and(eq(issues.companyId, companyId), inArray(issues.id, chunk)))); + for (const { id: issueId } of existing) resources.push({ + type: "issue", companyId, issueId, projectId: null, parentIssueId: null, + assigneeAgentId: null, assigneeUserId: null, + }); + } + } + const allowed = new Map(); + if (resources.length) { + const decisions = await retryIdempotentDatabaseOperation(() => decideReads(actor, resources)); + for (let index = 0; index < resources.length; index++) { + const resource = resources[index]; + if (resource.type === "issue" && resource.issueId) { + allowed.set(key(resource.companyId, resource.issueId), decisions[index]?.allowed === true); + } + } + } + return bindings.map(run => run.scopeKind === "company" || Boolean(run.issueId && allowed.get(key(run.companyId, run.issueId)))); + } const decisions = new Map>(); return Promise.all(runs.map(run => { const binding = { companyId: run.companyId, scopeKind: run.scopeKind, issueId: run.issueId };