From 39e5c8c1c32ee38d7b40d068560e63b2e93320be Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 24 Oct 2026 19:01:19 -0700 Subject: [PATCH] perf(server): narrow committed chat authorization reads (#17530) Select current chat authorization fields for committed responses under the existing scoped share lock. Preserve complete evidence for actual retries, source revocation, JSON types, and saved run data. Full build, typecheck, 113 focused tests, complete CI, and exact-head Greptile5/5 pass. Co-Authored-By: Paperclip --- doc/native-workspace-finalization-recovery.md | 6 + .../authorization-run-read.test.ts | 116 ++++++++++++++++++ .../chat-channels/authorization-run-read.ts | 46 +++++++ .../chat-channels/failed-run-retry.ts | 20 +-- 4 files changed, 174 insertions(+), 14 deletions(-) create mode 100644 server/src/services/chat-channels/authorization-run-read.test.ts create mode 100644 server/src/services/chat-channels/authorization-run-read.ts diff --git a/doc/native-workspace-finalization-recovery.md b/doc/native-workspace-finalization-recovery.md index 428c32b3fe4..c888ce1a34f 100644 --- a/doc/native-workspace-finalization-recovery.md +++ b/doc/native-workspace-finalization-recovery.md @@ -475,3 +475,9 @@ For a cancelled native run, the file-recovery form also offers an optional confirmation that no external actions were performed. Leave it unchecked unless the run history verifies that fact. The default records files without certifying action outcomes. Changing the selected source clears the evidence and confirmations. + +Committed chat response authorization reads current run identity, completion +binding, and chat source fields under the existing nonblocking share lock. +It excludes saved execution input, runner profiles, and log excerpts. Actual +retry admission still reads the complete run and its recovery evidence. These +reads preserve stored run data, source revocation checks, and JSON value types. diff --git a/server/src/services/chat-channels/authorization-run-read.test.ts b/server/src/services/chat-channels/authorization-run-read.test.ts new file mode 100644 index 00000000000..3d56b695222 --- /dev/null +++ b/server/src/services/chat-channels/authorization-run-read.test.ts @@ -0,0 +1,116 @@ +import { randomBytes, randomUUID } from "node:crypto"; +import { agents, companies, createDb, heartbeatRuns } from "@paperclipai/db"; +import { createDatabaseResponseCapture, installDatabaseResponseCapture } from "@paperclipai/db/response-capture"; +import { eq } from "drizzle-orm"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js"; +import { readChatAuthorizationRun } from "./authorization-run-read.js"; + +const support = await getEmbeddedPostgresTestSupport(); + +describe.skipIf(!support.supported)("chat authorization run reads", () => { + let database: Awaited>; + let db: ReturnType; + let companyId: string; + let agentId: string; + + beforeAll(async () => { + database = await startEmbeddedPostgresTestDatabase("chat-authorization-read-"); + db = createDb(database.connectionString); + const [company] = await db.insert(companies).values({ name: "Chat authorization fixture", issuePrefix: "CAR" }).returning(); + const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Chat agent", adapterType: "process" }).returning(); + companyId = company.id; + agentId = agent.id; + }, 60_000); + + afterAll(async () => { + await db?.$client.end(); + await database?.cleanup(); + }); + + async function seed(contextSnapshot: unknown = {}) { + const [run] = await db.insert(heartbeatRuns).values({ + companyId, agentId, contextSnapshot: contextSnapshot as Record | null, + }).returning(); + return { companyId, agentId, failedRunId: run.id }; + } + + it("reads committed authority below the byte budget and keeps complete retry evidence", async () => { + const bulk = randomBytes(128 * 1024).toString("base64"); + const context = { + source: "chat:slack", issueId: randomUUID(), interactionId: null, taskKey: "CAR-1", + wakeCommentIds: [randomUUID()], wakeCommentId: null, + chatFailedRunRetry: { version: 1, actionId: randomUUID(), failedRunId: randomUUID() }, + retryOfRunId: randomUUID(), + }; + const scope = await seed({ ...context, savedInput: bulk }); + await db.update(heartbeatRuns).set({ runnerProfileJson: { savedInput: bulk }, stdoutExcerpt: bulk, stderrExcerpt: bulk }) + .where(eq(heartbeatRuns.id, scope.failedRunId)); + const [before] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, scope.failedRunId)); + const events: Array> = []; + const now = Date.now(); + const capture = createDatabaseResponseCapture({ start: now - 100, end: now + 59_000 }, event => events.push(event)); + installDatabaseResponseCapture(db, { heartbeatRuns }, capture); + try { + const selected = await db.transaction(tx => readChatAuthorizationRun(tx, scope, true)); + expect(selected.kind).toBe("committed"); + expect(selected.run?.contextSnapshot).toEqual(context); + expect(selected.run).not.toHaveProperty("runnerProfileJson"); + expect(selected.run).not.toHaveProperty("stdoutExcerpt"); + } finally { capture.flush(); } + const summary = events.find(event => event.event === "paperclip.db_response_capture.summary.v1")!; + expect(summary.completedQueries).toBeGreaterThan(0); + expect(summary.responseValueBytes).toBeLessThan(4096); + const retry = await db.transaction(tx => readChatAuthorizationRun(tx, scope, false)); + expect(retry.kind).toBe("retry"); + expect(retry.run).toEqual(before); + const [after] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, scope.failedRunId)); + expect(after).toEqual(before); + expect(JSON.stringify(events)).not.toContain(bulk.slice(0, 64)); + }); + + it.each([{}, { wakeCommentId: null }, { wakeCommentId: false, wakeCommentIds: "invalid", interactionId: [] }])( + "preserves key presence and JSON types in %j", async context => { + const scope = await seed(context); + expect((await readChatAuthorizationRun(db, scope, true)).run?.contextSnapshot).toEqual(context); + }, + ); + + it.each([null, [], "invalid", 42])("handles non-object context %j", async context => { + const scope = await seed(context); + expect((await readChatAuthorizationRun(db, scope, true)).run?.contextSnapshot).toEqual(context === null ? null : {}); + expect((await readChatAuthorizationRun(db, scope, false)).run?.contextSnapshot).toEqual(context); + }); + + it("reads current authority and fails closed for another company or agent", async () => { + const scope = await seed({ source: "chat:slack", taskKey: "CAR-1" }); + expect((await readChatAuthorizationRun(db, scope, true)).run?.contextSnapshot).toEqual({ source: "chat:slack", taskKey: "CAR-1" }); + await db.update(heartbeatRuns).set({ contextSnapshot: { source: "chat:discord", taskKey: "CAR-2" } }) + .where(eq(heartbeatRuns.id, scope.failedRunId)); + expect((await readChatAuthorizationRun(db, scope, true)).run?.contextSnapshot).toEqual({ source: "chat:discord", taskKey: "CAR-2" }); + for (const committed of [true, false]) { + expect((await readChatAuthorizationRun(db, { ...scope, companyId: randomUUID() }, committed)).run).toBeUndefined(); + expect((await readChatAuthorizationRun(db, { ...scope, agentId: randomUUID() }, committed)).run).toBeUndefined(); + expect((await readChatAuthorizationRun(db, { ...scope, failedRunId: randomUUID() }, committed)).run).toBeUndefined(); + } + }); + + it("preserves nonblocking authorization row locks", async () => { + const scope = await seed(); + let acquired!: () => void, release!: () => void; + const ready = new Promise(resolve => { acquired = resolve; }); + const released = new Promise(resolve => { release = resolve; }); + const holder = db.transaction(async tx => { + await tx.update(heartbeatRuns).set({ status: "running" }).where(eq(heartbeatRuns.id, scope.failedRunId)); + acquired(); + await released; + }); + try { + await ready; + for (const committed of [true, false]) { + await expect(db.transaction(tx => readChatAuthorizationRun(tx, scope, committed))).rejects.toMatchObject({ cause: { code: "55P03" } }); + } + } finally { release(); await holder; } + expect((await readChatAuthorizationRun(db, scope, true)).run?.status).toBe("running"); + }); +}); diff --git a/server/src/services/chat-channels/authorization-run-read.ts b/server/src/services/chat-channels/authorization-run-read.ts new file mode 100644 index 00000000000..3390ba103b7 --- /dev/null +++ b/server/src/services/chat-channels/authorization-run-read.ts @@ -0,0 +1,46 @@ +import { heartbeatRuns, type Db } from "@paperclipai/db"; +import { and, eq, sql } from "drizzle-orm"; + +type Database = Db | Parameters[0]>[0]; +type Scope = { companyId: string; agentId: string; failedRunId: string }; + +const contextKeys = [ + "source", "issueId", "interactionId", "taskKey", "wakeCommentIds", + "wakeCommentId", "chatFailedRunRetry", "retryOfRunId", +] as const; + +const committedContext = sql | null>` + case when ${heartbeatRuns.contextSnapshot} is null then null else ( + select coalesce(jsonb_object_agg(field.key, field.value), '{}'::jsonb) + from jsonb_each(case when jsonb_typeof(${heartbeatRuns.contextSnapshot}) = 'object' + then ${heartbeatRuns.contextSnapshot} else '{}'::jsonb end) as field(key, value) + where field.key in (${sql.join(contextKeys.map(key => sql`${key}`), sql`, `)}) + ) end +`; + +export async function readChatAuthorizationRun(db: Database, scope: Scope, committed: boolean) { + const where = and( + eq(heartbeatRuns.id, scope.failedRunId), + eq(heartbeatRuns.companyId, scope.companyId), + eq(heartbeatRuns.agentId, scope.agentId), + ); + if (!committed) { + const [run] = await db.select().from(heartbeatRuns).where(where).for("share", { noWait: true }); + return { kind: "retry" as const, run }; + } + const [run] = await db.select({ + id: heartbeatRuns.id, + companyId: heartbeatRuns.companyId, + agentId: heartbeatRuns.agentId, + wakeupRequestId: heartbeatRuns.wakeupRequestId, + status: heartbeatRuns.status, + finishedAt: heartbeatRuns.finishedAt, + runtimeMode: heartbeatRuns.runtimeMode, + errorCode: heartbeatRuns.errorCode, + nativeIssueId: heartbeatRuns.nativeIssueId, + completionContractId: heartbeatRuns.completionContractId, + retryOfRunId: heartbeatRuns.retryOfRunId, + contextSnapshot: committedContext, + }).from(heartbeatRuns).where(where).for("share", { noWait: true }); + return { kind: "committed" as const, run }; +} diff --git a/server/src/services/chat-channels/failed-run-retry.ts b/server/src/services/chat-channels/failed-run-retry.ts index 2b5d2c9b026..6310d448a66 100644 --- a/server/src/services/chat-channels/failed-run-retry.ts +++ b/server/src/services/chat-channels/failed-run-retry.ts @@ -2,6 +2,7 @@ import { continuingTeamsFilePublication, type ContinuingTeamsFileTransfer } from import { readTaskChatRetryIntent, taskChatRetryCanAwaitRecovery, taskChatRetryExecutionStopped } from "./task-retry-authority.js"; import { notifyChatActionWork, notifyChatPublicationWork } from "../chat-work-notifications.js"; import { FAILED_RUN_RETRY_STALE_MS, dueChatAction } from "../chat-action-work.js"; +import { readChatAuthorizationRun } from "./authorization-run-read.js"; import { createHash, @@ -688,17 +689,8 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti ), ) .for("share", { noWait: true }); - const [failedRun] = await tx - .select() - .from(heartbeatRuns) - .where( - and( - eq(heartbeatRuns.id, input.failedRunId), - eq(heartbeatRuns.companyId, input.companyId), - eq(heartbeatRuns.agentId, input.agentId), - ), - ) - .for("share", { noWait: true }); + const evidence = await readChatAuthorizationRun(tx, input, Boolean(input.committedResponse)); + const failedRun = evidence.run; if ( !issue || issue.assigneeAgentId !== input.agentId || @@ -719,7 +711,7 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti "process_exit", "timeout", ].includes(failedRun.errorCode ?? "") || - (await isPreProviderReviewedChatFailure(tx, failedRun)) + (evidence.kind === "retry" && await isPreProviderReviewedChatFailure(tx, evidence.run)) )) || (!input.publication && ["backlog", "done", "cancelled"].includes(issue.status)) @@ -729,8 +721,8 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti companyId: input.companyId, issueId: issue.id, agentId: input.agentId, sourceRunId: failedRun.id, intentId: input.taskRetryIntentId, })) throw failedChatRetryDenied(); - if (failedRun.runtimeMode === "native" && !input.committedResponse && !input.taskRetryIntentId) - await failedNativeRetryCoordinator(tx, failedRun, issue.id); + if (failedRun.runtimeMode === "native" && evidence.kind === "retry" && !input.taskRetryIntentId) + await failedNativeRetryCoordinator(tx, evidence.run, issue.id); const context = failedRun.contextSnapshot ?? {}; const provider = CHAT_PROVIDERS.find( (value) => context.source === `chat:${value}`,