mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
perf(chat): reuse bound-run authorization query templates (#18966)
This PR depends on #18969. Merge that fixture-only prerequisite first, then retarget this PR to master. No merge or deployment is requested in this work. Its base is `codex/fix-recovery-published-workspace-fixture`; the six-line recovery fixture repair is excluded from this diff. ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - External chat authorization binds each run to current messages and conversation policy. > - Review wait and attention-retention paths repeat the same four-table read. > - Each call rebuilt the Drizzle query even when its SQL shape stayed the same. > - This pull request reuses the compiled template for the exact database handle and lock mode. > - Fresh parameters and current database reads preserve authorization while reducing Node CPU work. ## Linked Issues or Issue Description **What existing behavior does this improve?** The `authorizeChatConversationForBoundRun` links read used by external chat waits and attachment reuse. **Current behavior** At baseline `e54f9fbf828e06e385e0c62dc381ccb46190aa30`, each call rebuilt and serialized a four-table query. The observed staging shape accounted for 16.2% of sampled SQL serialization CPU across 32 samples. This is a share of query compilation CPU, not total server CPU. **Proposed behavior** Compile each read, blocking-lock or NOWAIT variant once per exact live database or transaction object. Bind new company, issue, agent, provider and comment IDs on every execution. Continue all current database reads and authorization checks. **Reason and benefit** Repeated authorized service calls used 26.91% less Node process CPU in the matched local fixture. All seven CPU pairs improved. This reduces repeated compilation on the shared service path. Related: #18896 retains task Retry intent through identity contention. It changes a different workflow and does not reuse this query template. A public issue and PR search found no duplicate template change. ROADMAP.md has no conflicting planned feature. ## What Changed - Keep compiled queries in a `WeakMap` keyed by the exact supplied database or transaction object. Create only the requested lock variant. Never cache a result or authority decision. - Bind the comment list as `ANY(uuid[])`. This supports changing list sizes with only three templates per live handle. Keep all joins, selected fields, policy checks and lock clauses. - Add real PostgreSQL regression tests, a reusable fixture, an actual service benchmark and a dated performance note. ## Verification - Passed 17 real PostgreSQL tests: `node node_modules/vitest/vitest.mjs run --project @paperclipai/server server/src/__tests__/prepared-chat-authorization.integration.test.ts server/src/services/native-runtime/chat-attachment-reuse.test.ts`. - After rebasing, seven selected existing external-chat-wait integration cases passed (266 deselected). They cover small current review waits, answered-chat identity advisory locks, connection-disable serialization and contended endpoint proof retry. - New tests cover changed parameters and list sizes, duplicate/empty inputs, invalid UUID errors, company/agent/provider binding, current setup generation and credential fingerprints, root/transaction/savepoint isolation, rollback, read mode during a held row lock, blocking timeout and NOWAIT contention. - Actual service benchmark: `node --import ./server/node_modules/tsx/dist/loader.mjs server/scripts/benchmark-bound-chat-authorization.ts --baseline-ref=e54f9fbf828e06e385e0c62dc381ccb46190aa30`. Node 24.22.2, macOS arm64, real PostgreSQL, 500 warmup calls per implementation, seven alternating pairs of 500 calls. Every result matched. - Pinned measured treatment `8386eb78d118faaf6c2fd7f7471b38c0f4b2ef54`. Its source is unchanged after rebasing onto master. Median Node CPU per 500 calls: 379.033 ms to 277.048 ms, down 26.91%. Every CPU pair improved by 13.54–33.52%. Median wall time: 742.109 ms to 648.005 ms. One wall pair increased by 0.51%. - A separate diagnostic after timing found five SQL compilations per baseline call and four per warm treatment call. All five database reads still run. - The final four-test PostgreSQL authorization suite passed again after rebasing and strengthening the lock proof. It holds only a message-link row, so principal advisory contention cannot mask a missing joined-query lock. - Independent source review found no correctness issue in exact-handle ownership, fresh binding or the three lock variants. - Preserve setup failures: the first test attempt lacked plugin SDK build output. Shared and SDK builds fixed it. Later attempts could not start embedded PostgreSQL because its local dependency directory lacked bundled native-library aliases. `DYLD_LIBRARY_PATH` did not fix it. Restoring package-local aliases fixed the environment; the final 17 tests then passed. No dependency or lockfile change is included. - The first full typecheck found new test-only errors: PostgreSQL transaction handles lack the root database `$client` type, and runtime setup test data includes fields outside the saved setup interface. Commit `40a4645750` uses the existing explicit transaction test cast and SQL JSON fixtures. No production contract changed. The full check also found local plugin SDK link prerequisites, which the coordinator is repairing separately. - Preserve the first Linux CI failure: [server shard 9](https://github.com/paperclipai/paperclip/actions/runs/41837775564/job/126602280304) failed the new setup-fence fixture's allowed case. The fixture supplied a client `processedAt` while PostgreSQL supplied default `receivedAt`; this could violate the required processing order. The test now uses explicit ordered timestamps and no wall-clock window. The four-test PostgreSQL suite passed again in 19.93 seconds. Production guards were not weakened. - [Server shard 2](https://github.com/paperclipai/paperclip/actions/runs/41837775564/job/126602280291) also failed five workspace-recovery cases with `remote_workspace_published_path_invalid`. Those failures reproduced on both pure master and the exact CI merge with the chat optimization removed. #18969 fixes the canonical path in that separate fixture, with five failing cases before and five passing after. This PR is stacked on that independent repair for final CI. - The additional Paperclip review reported a node-postgres statement-name collision and requested explicit pool cleanup. This repository uses postgres-js; locked Drizzle 0.45.2 accepts the Drizzle label but does not use or forward it. [The review response](https://github.com/paperclipai/paperclip/pull/18966#issuecomment-6632666599) links primary driver source and the database factory. Commit `c5d0d84f36` adds explicit pool teardown in `try/finally` and a real same-transaction `read → blocking → nonblocking → read` case. All five authorization tests passed in 6.47 seconds. The original separate row-lock test remains. Production SQL and code did not change. - Final combined local validation at `4ab58ff70b` passed full `pnpm -r typecheck` (39.864 seconds) and 35 real PostgreSQL cases across the chat, GitHub and shared prepared-query suites (11.378 seconds). Full `pnpm build` passed earlier with byte-identical production source; later changes affect tests only. The production file SHA matches the benchmark and this PR. The original full `pnpm test:run` was interrupted after all six chat shards passed and general tests had partly run, so it is not a full local pass. Final current-head CI provides the broader coverage. A sandbox retry failed with EPERM before assertions; the identical escalated commands then passed. Final head passed CI in [workflow 41843806060](https://github.com/paperclipai/paperclip/actions/runs/41843806060), including all build, typecheck, general, serialized and e2e test gates. The previous head `40a464575009dfc3cce7afa1eb4fffa5540dcaf6` passed Greptile 5/5, security review, typecheck, build and canary validation. The prior stacked head `74cb36370162750f8e135ed091603d3629d23b0d` passed full CI, Greptile 5/5, policy and security checks. Final head `c5d0d84f369d897a66d9b6441bab10cb79f593b9` passed a fresh Greptile 5/5 review with no actionable findings, security and PR policy checks. Its complete [current-head CI workflow passed](https://github.com/paperclipai/paperclip/actions/runs/41843806060). All 54 required CI, Greptile, policy and security checks passed. The optional Paperclip Review check is queued for this head; no current-head score is claimed. Production source SHA-256 remains `72ce9d2317e5c97b36e299e24e30fc1aa63020d8c266e495fc45dbfdd5331e93`, identical to the measured code. ## Risks - `ANY(uuid[])` relies on postgres-js array binding and UUID casting. Real PostgreSQL tests cover single and multiple valid values and invalid UUID errors. - A new transaction or savepoint pays its first compilation cost. The weak cache cannot reuse a root session for it. This benchmark covers repeated calls on one root handle. - The team paused other work during the benchmark. Unrelated Rust compilation was active elsewhere on the host. Process CPU excludes those processes, but host contention can affect caches, scheduling and wall time. This source-mode local result does not prove production request latency or whole-server CPU savings. - No migration or API contract changes. No deployment or merge is included. Monitor authorization failures and lock-contention behavior after an authorized rollout; revert the narrow service change if those regress. ## Model Used OpenAI `gpt-6.1-sol` through Codex, with reasoning, repository editing, tool use and test execution. The host did not expose a context-window size or deployment snapshot ID. ## 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 (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>
This commit is contained in:
DottaandPaperclip authored and GitHub committed
1 parent
2cbfc35a07
commit
5d226ca08a
5 files changed
+543
-34
No files matched your search
@@ -0,0 +1,79 @@
|
||||
# Bound-chat authorization query compilation
|
||||
|
||||
`authorizeChatConversationForBoundRun` reads four joined tables to establish the
|
||||
current endpoint, conversation, delivery and comment binding. It runs in shared
|
||||
external-review wait and attention-retention paths. In the November 3 staging
|
||||
sample, this SQL shape accounted for 16.2% of sampled SQL serialization CPU
|
||||
(32 builds, mean 478.5 microseconds). That share describes SQL serialization,
|
||||
not total server CPU or request latency.
|
||||
|
||||
The service now retains its Drizzle compiled query in a `WeakMap` keyed by the
|
||||
exact supplied database or transaction object. It creates only the requested
|
||||
read, `FOR UPDATE`, or `FOR UPDATE NOWAIT` variant. Each execution supplies fresh
|
||||
company, issue, agent, provider and comment IDs. An `ANY(uuid[])` placeholder
|
||||
keeps the template count at three per live handle for every comment-list size.
|
||||
It retains no result or authorization decision. Transactions and nested
|
||||
savepoints cannot borrow a root query session. Setup, credential, resource and
|
||||
principal checks still read current state.
|
||||
|
||||
## Reproduce
|
||||
|
||||
From the source checkout with dependencies and the plugin SDK built:
|
||||
|
||||
```sh
|
||||
node --import ./server/node_modules/tsx/dist/loader.mjs \
|
||||
server/scripts/benchmark-bound-chat-authorization.ts \
|
||||
--baseline-ref=e54f9fbf828e06e385e0c62dc381ccb46190aa30
|
||||
```
|
||||
|
||||
Use Node 24.22.2 to match this measurement. The command seeds an isolated real
|
||||
PostgreSQL database and invokes the actual service function. It imports the
|
||||
baseline module from the supplied Git revision with its relative imports
|
||||
resolved against the same checkout. It uses one active Slack conversation,
|
||||
one processed message, a linked member and an enabled destination. It warms
|
||||
each implementation with 500 successful calls, then alternates order across
|
||||
seven pairs of 500 successful calls per implementation. Each result is checked.
|
||||
Query compilation counts are measured separately after all timed rounds.
|
||||
|
||||
## Recorded result
|
||||
|
||||
Baseline: `e54f9fbf828e06e385e0c62dc381ccb46190aa30`.
|
||||
Treatment: `8386eb78d118faaf6c2fd7f7471b38c0f4b2ef54`.
|
||||
Node 24.22.2, macOS arm64, source execution through tsx.
|
||||
|
||||
| Metric per 500 calls | Baseline median (range), ms | Treatment median (range), ms | Median change |
|
||||
| --- | --- | --- | --- |
|
||||
| Node process CPU | 379.033 (286.092–424.702) | 277.048 (247.341–332.123) | -26.91% |
|
||||
| Wall time | 742.109 (557.337–901.323) | 648.005 (560.198–662.265) | -12.68% |
|
||||
|
||||
CPU improved in all seven matched pairs, by 13.54–33.52%. One wall-time pair
|
||||
increased by 0.51%. Warm-call SQL compilations fell from five to four: the
|
||||
joined authorization query no longer recompiles. All database reads still run.
|
||||
|
||||
The owning team stopped other builds and tests during this measurement.
|
||||
Unrelated Rust compilation was observed on the host. Process CPU excludes those
|
||||
other processes, but shared host contention can affect scheduling, caches and
|
||||
wall time. This local source-mode fixture does not establish a production
|
||||
latency improvement, a whole-server CPU reduction, or a deployed result. It
|
||||
measures repeated calls on one live root handle. A new transaction still pays
|
||||
its first compilation cost. An alternating measurement does not remove every
|
||||
host-load effect.
|
||||
|
||||
## Correctness and retained setup failures
|
||||
|
||||
The focused real PostgreSQL suite passed 17 tests: four new authorization tests
|
||||
and 13 existing attachment-reuse tests. The new cases exercise changed binding
|
||||
values and comment arities, duplicate and empty inputs, invalid UUID errors,
|
||||
company/agent/provider isolation, current setup generation and credential
|
||||
fingerprints, transaction and savepoint rollback, read mode during a held row
|
||||
lock, blocking lock timeout and NOWAIT contention.
|
||||
|
||||
The first attempt failed before test collection because plugin SDK build output
|
||||
was absent. Building shared and plugin SDK packages corrected that prerequisite.
|
||||
Subsequent attempts failed before assertions because the installed embedded
|
||||
PostgreSQL package lacked version aliases for bundled native libraries. Setting
|
||||
`DYLD_LIBRARY_PATH` did not fix that child-process setup. Restoring the missing
|
||||
aliases inside the isolated worktree's dependency directory corrected it; no
|
||||
repository dependency, lockfile or production source workaround was added.
|
||||
Those setup failures are not passing behavior evidence. The final 17-test run
|
||||
passed after the local repair. The benchmark then completed with empty stderr.
|
||||
@@ -0,0 +1,78 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { SQL } from "drizzle-orm";
|
||||
import { createDb } from "@paperclipai/db";
|
||||
import { seedBoundChatAuthorization } from "../src/__tests__/helpers/bound-chat-authorization.js";
|
||||
import { startEmbeddedPostgresTestDatabase } from "../src/__tests__/helpers/embedded-postgres.js";
|
||||
import { authorizeChatConversationForBoundRun as treatment } from "../src/services/native-runtime/chat-attachment-reuse.js";
|
||||
|
||||
const baselineRef = process.argv.find((arg) => arg.startsWith("--baseline-ref="))?.slice(15);
|
||||
assert.ok(baselineRef, "Pass --baseline-ref=<pre-fix commit>");
|
||||
const root = fileURLToPath(new URL("../../", import.meta.url));
|
||||
const baselineRevision = execFileSync("git", ["rev-parse", `${baselineRef}^{commit}`], { cwd: root, encoding: "utf8" }).trim();
|
||||
const treatmentRevision = execFileSync("git", ["rev-parse", "HEAD"], { cwd: root, encoding: "utf8" }).trim();
|
||||
const modulePath = "server/src/services/native-runtime/chat-attachment-reuse.ts";
|
||||
const source = execFileSync("git", ["show", `${baselineRevision}:${modulePath}`], { cwd: root, encoding: "utf8" });
|
||||
const currentSource = await readFile(path.join(root, modulePath), "utf8");
|
||||
assert.notEqual(source, currentSource, "Baseline must precede the optimization");
|
||||
const baselineDir = await mkdtemp(path.join(root, "server/scripts/.bound-chat-baseline-"));
|
||||
const baselineFile = path.join(baselineDir, "chat-attachment-reuse.ts");
|
||||
const moduleUrl = pathToFileURL(path.join(root, modulePath));
|
||||
await writeFile(baselineFile, source.replace(/from "(\.{1,2}\/[^\"]+)"/g,
|
||||
(_match, specifier: string) => `from ${JSON.stringify(fileURLToPath(new URL(specifier, moduleUrl)))}`));
|
||||
const keepAlive = setInterval(() => {}, 1_000);
|
||||
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
||||
let db: ReturnType<typeof createDb> | undefined;
|
||||
try {
|
||||
const { authorizeChatConversationForBoundRun: baseline } = await import(pathToFileURL(baselineFile).href) as
|
||||
typeof import("../src/services/native-runtime/chat-attachment-reuse.js");
|
||||
temporary = await startEmbeddedPostgresTestDatabase("bound-chat-benchmark-");
|
||||
db = createDb(temporary.connectionString);
|
||||
const fixture = await seedBoundChatAuthorization(db);
|
||||
const readers = { baseline, treatment };
|
||||
const expected = { endpointId: fixture.endpointId, conversationId: fixture.conversationId };
|
||||
for (const reader of Object.values(readers)) {
|
||||
for (let n = 0; n < 500; n++) assert.deepEqual(await reader(db, fixture.binding, fixture.context, "read"), expected);
|
||||
}
|
||||
const iterations = 500;
|
||||
const samples = [];
|
||||
for (let round = 0; round < 7; round++) {
|
||||
for (const label of round % 2 ? ["treatment", "baseline"] as const : ["baseline", "treatment"] as const) {
|
||||
const cpuStart = process.cpuUsage(), start = performance.now();
|
||||
for (let n = 0; n < iterations; n++) {
|
||||
assert.deepEqual(await readers[label](db, fixture.binding, fixture.context, "read"), expected);
|
||||
}
|
||||
const wallMs = performance.now() - start, cpu = process.cpuUsage(cpuStart);
|
||||
const sample = { round, label, iterations, cpuMs: (cpu.user + cpu.system) / 1_000, wallMs };
|
||||
samples.push(sample);
|
||||
process.stdout.write(JSON.stringify(sample) + "\n");
|
||||
}
|
||||
}
|
||||
const compilationCounts: Record<string, number> = {};
|
||||
const original = SQL.prototype.toQuery;
|
||||
for (const label of ["baseline", "treatment"] as const) {
|
||||
let compilations = 0;
|
||||
SQL.prototype.toQuery = function (...args) {
|
||||
compilations++;
|
||||
return Reflect.apply(original, this, args);
|
||||
};
|
||||
try {
|
||||
assert.deepEqual(await readers[label](db, fixture.binding, fixture.context, "read"), expected);
|
||||
} finally { SQL.prototype.toQuery = original; }
|
||||
compilationCounts[label] = compilations;
|
||||
}
|
||||
assert.equal(compilationCounts.baseline - compilationCounts.treatment, 1);
|
||||
process.stdout.write(JSON.stringify({ baselineRevision, treatmentRevision, node: process.version,
|
||||
platform: process.platform, arch: process.arch, iterations, rounds: 7, warmups: 500,
|
||||
data: "one active Slack conversation, processed inbound message, linked member, enabled destination",
|
||||
surface: "authorizeChatConversationForBoundRun, real PostgreSQL, read mode, warmed exact root handle",
|
||||
compilationCounts, samples }) + "\n");
|
||||
} finally {
|
||||
await db?.$client.end({ timeout: 1 });
|
||||
await temporary?.cleanup();
|
||||
await rm(baselineDir, { recursive: true, force: true });
|
||||
clearInterval(keepAlive);
|
||||
}
|
||||
@@ -0,0 +1,184 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { agents, companies, createDb, issues, issueComments, toolApplications,
|
||||
toolConnections, chatEndpoints, chatEndpointResources, chatConversations,
|
||||
chatExternalPrincipals, chatIdentityLinks, companyMemberships, chatDeliveries,
|
||||
chatMessageLinks, type Db } from "@paperclipai/db";
|
||||
export async function seedBoundChatAuthorization(db: Db) {
|
||||
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID(),
|
||||
otherIssueId = randomUUID(), runId = randomUUID(), endpointId = randomUUID(),
|
||||
resourceId = randomUUID(), conversationId = randomUUID(), principalId = randomUUID(),
|
||||
commentId = randomUUID(), deliveryId = randomUUID();
|
||||
const provider = "slack", userId = "prepared-chat-user";
|
||||
const binding = { companyId, agentId, issueId, runId };
|
||||
const context = { source: "chat:slack", paperclipHarnessCheckedOut: true,
|
||||
wakeCommentIds: [commentId] };
|
||||
await db
|
||||
.insert(companies)
|
||||
.values({
|
||||
id: companyId,
|
||||
name: "Reviewed chat",
|
||||
issuePrefix: "RCB",
|
||||
issueCounter: 1,
|
||||
});
|
||||
await db
|
||||
.insert(agents)
|
||||
.values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "Chat runner",
|
||||
adapterType: "paperclip_runner",
|
||||
adapterConfig: { provider: "codex" },
|
||||
status: "active",
|
||||
});
|
||||
await db
|
||||
.insert(issues)
|
||||
.values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
title: "Reviewed conversation",
|
||||
issueNumber: 1,
|
||||
identifier: "RCB-1",
|
||||
status: "in_review",
|
||||
workMode: "standard",
|
||||
assigneeAgentId: agentId,
|
||||
});
|
||||
await db.insert(issues).values({
|
||||
id: otherIssueId,
|
||||
companyId,
|
||||
title: "Unrelated conversation",
|
||||
issueNumber: 2,
|
||||
identifier: "RCB-2",
|
||||
status: "in_review",
|
||||
assigneeAgentId: agentId,
|
||||
});
|
||||
const applicationId = randomUUID(),
|
||||
connectionId = randomUUID();
|
||||
await db
|
||||
.insert(toolApplications)
|
||||
.values({
|
||||
id: applicationId,
|
||||
companyId,
|
||||
applicationKey: `chat:${provider}:${endpointId}`,
|
||||
name: "Discord",
|
||||
type: "chat",
|
||||
status: "active",
|
||||
});
|
||||
await db
|
||||
.insert(toolConnections)
|
||||
.values({
|
||||
id: connectionId,
|
||||
companyId,
|
||||
applicationId,
|
||||
name: "Discord",
|
||||
uid: `chat-${provider}-${endpointId}`,
|
||||
connectionPurpose: "channel",
|
||||
transport: "chat_sdk",
|
||||
status: "active",
|
||||
enabled: true,
|
||||
});
|
||||
await db
|
||||
.insert(chatEndpoints)
|
||||
.values({
|
||||
id: endpointId,
|
||||
companyId,
|
||||
connectionId,
|
||||
provider,
|
||||
publicId: randomUUID(),
|
||||
assignedAgentId: agentId,
|
||||
status: "active",
|
||||
providerAccountId: "guild-1",
|
||||
allowUnlinkedPeople: false,
|
||||
});
|
||||
await db
|
||||
.insert(chatEndpointResources)
|
||||
.values({
|
||||
id: resourceId,
|
||||
companyId,
|
||||
endpointId,
|
||||
type: "channel",
|
||||
providerResourceId: "channel-1",
|
||||
label: "#review",
|
||||
availability: "available",
|
||||
enabled: true,
|
||||
});
|
||||
await db
|
||||
.insert(chatConversations)
|
||||
.values({
|
||||
id: conversationId,
|
||||
companyId,
|
||||
endpointId,
|
||||
resourceId,
|
||||
issueId,
|
||||
externalConversationId: "channel-1",
|
||||
externalThreadId: "thread-1",
|
||||
externalLabel: "Review thread",
|
||||
state: "active",
|
||||
});
|
||||
await db
|
||||
.insert(chatExternalPrincipals)
|
||||
.values({
|
||||
id: principalId,
|
||||
companyId,
|
||||
provider,
|
||||
providerAccountId: "guild-1",
|
||||
externalId: "user-1",
|
||||
kind: "user",
|
||||
});
|
||||
await db
|
||||
.insert(chatIdentityLinks)
|
||||
.values({
|
||||
companyId,
|
||||
endpointId,
|
||||
principalId,
|
||||
paperclipUserId: userId,
|
||||
status: "linked",
|
||||
});
|
||||
await db
|
||||
.insert(companyMemberships)
|
||||
.values({
|
||||
companyId,
|
||||
principalType: "user",
|
||||
principalId: userId,
|
||||
status: "active",
|
||||
membershipRole: "member",
|
||||
});
|
||||
await db
|
||||
.insert(issueComments)
|
||||
.values({
|
||||
id: commentId,
|
||||
companyId,
|
||||
issueId,
|
||||
authorType: "user",
|
||||
authorUserId: userId,
|
||||
body: "Please inspect the earlier photo, without approving completion.",
|
||||
});
|
||||
await db
|
||||
.insert(chatDeliveries)
|
||||
.values({
|
||||
id: deliveryId,
|
||||
companyId,
|
||||
endpointId,
|
||||
conversationId,
|
||||
principalId,
|
||||
providerEventId: "review-followup",
|
||||
deduplicationKey: "review-followup",
|
||||
eventKind: "message",
|
||||
normalizedEvent: {},
|
||||
state: "processed",
|
||||
attempts: 1,
|
||||
processedAt: new Date(),
|
||||
});
|
||||
await db
|
||||
.insert(chatMessageLinks)
|
||||
.values({
|
||||
companyId,
|
||||
endpointId,
|
||||
conversationId,
|
||||
deliveryId,
|
||||
commentId,
|
||||
providerMessageId: "review-message",
|
||||
direction: "inbound",
|
||||
});
|
||||
return { binding, context, companyId, agentId, issueId, otherIssueId, runId,
|
||||
endpointId, resourceId, conversationId, principalId, commentId, deliveryId, userId };
|
||||
}
|
||||
@@ -0,0 +1,147 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq, sql } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { createDb, issueComments, chatEndpoints, chatDeliveries, chatMessageLinks, type Db } from "@paperclipai/db";
|
||||
import { seedBoundChatAuthorization } from "./helpers/bound-chat-authorization.js";
|
||||
import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
import { authorizeChatConversationForBoundRun, isExternalChatWaitAuthorizationContention }
|
||||
from "../services/native-runtime/chat-attachment-reuse.js";
|
||||
|
||||
describe("prepared bound-chat authorization", () => {
|
||||
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
let db: Db;
|
||||
let fixture: Awaited<ReturnType<typeof seedBoundChatAuthorization>>;
|
||||
let binding: typeof fixture.binding, context: typeof fixture.context;
|
||||
let companyId: string, issueId: string, otherIssueId: string, endpointId: string,
|
||||
conversationId: string, principalId: string, commentId: string, deliveryId: string, userId: string;
|
||||
const authorize = (handle: Db = db, ids = [commentId]) =>
|
||||
authorizeChatConversationForBoundRun(handle, binding, { ...context, wakeCommentIds: ids }, "read");
|
||||
const denied = "paperclip_runner_chat_attachment_binding_denied";
|
||||
beforeAll(async () => {
|
||||
temporary = await startEmbeddedPostgresTestDatabase("prepared-chat-");
|
||||
db = createDb(temporary.connectionString);
|
||||
fixture = await seedBoundChatAuthorization(db);
|
||||
({ binding, context, companyId, issueId, otherIssueId, endpointId, conversationId,
|
||||
principalId, commentId, deliveryId, userId } = fixture);
|
||||
});
|
||||
afterAll(async () => {
|
||||
try { await db?.$client.end({ timeout: 1 }); }
|
||||
finally { await temporary?.cleanup(); }
|
||||
});
|
||||
|
||||
it("binds changed values and array arities while reading current authorization", async () => {
|
||||
await expect(authorize()).resolves.toEqual({ endpointId, conversationId });
|
||||
await expect(authorize(db, [commentId, commentId])).resolves.toEqual({ endpointId, conversationId });
|
||||
const secondCommentId = randomUUID(), secondDeliveryId = randomUUID();
|
||||
await db.insert(issueComments).values({ id: secondCommentId, companyId, issueId,
|
||||
authorType: "user", authorUserId: userId, body: "Another message" });
|
||||
await db.insert(chatDeliveries).values({ id: secondDeliveryId, companyId, endpointId,
|
||||
conversationId, principalId, providerEventId: secondDeliveryId, deduplicationKey: secondDeliveryId,
|
||||
eventKind: "message", normalizedEvent: {}, state: "processed", processedAt: new Date() });
|
||||
await db.insert(chatMessageLinks).values({ companyId, endpointId, conversationId,
|
||||
deliveryId: secondDeliveryId, commentId: secondCommentId, providerMessageId: secondCommentId, direction: "inbound" });
|
||||
await expect(authorize(db, [secondCommentId])).resolves.toEqual({ endpointId, conversationId });
|
||||
await expect(authorize(db, [secondCommentId, commentId])).resolves.toEqual({ endpointId, conversationId });
|
||||
await expect(authorize(db, [commentId, randomUUID()])).rejects.toThrow(denied);
|
||||
await expect(authorize(db, [])).rejects.toThrow(denied);
|
||||
for (const changed of [{ companyId: randomUUID() }, { issueId: otherIssueId },
|
||||
{ agentId: randomUUID() }]) {
|
||||
await expect(authorizeChatConversationForBoundRun(db, { ...binding, ...changed }, context, "read"))
|
||||
.rejects.toThrow(denied);
|
||||
}
|
||||
await expect(authorizeChatConversationForBoundRun(db, binding, { ...context, source: "chat:discord" }, "read"))
|
||||
.rejects.toThrow(denied);
|
||||
await expect(authorize(db, ["invalid-uuid"])).rejects.toThrow();
|
||||
await db.update(chatEndpoints).set({ status: "paused" }).where(eq(chatEndpoints.id, endpointId));
|
||||
await expect(authorize()).rejects.toThrow(denied);
|
||||
await db.update(chatEndpoints).set({ status: "active" }).where(eq(chatEndpoints.id, endpointId));
|
||||
await expect(authorize()).resolves.toEqual({ endpointId, conversationId });
|
||||
});
|
||||
|
||||
it("rechecks setup generation and credential fences on a reused template", async () => {
|
||||
const startedAt = new Date("2026-11-01T00:00:00.000Z");
|
||||
await db.update(chatEndpoints).set({ status: "verifying", setup: sql`${JSON.stringify({
|
||||
step: "test", testStartedAt: startedAt.toISOString(), runtimeGeneration: 7,
|
||||
})}::jsonb` }).where(eq(chatEndpoints.id, endpointId));
|
||||
try {
|
||||
await expect(authorize()).rejects.toThrow(denied);
|
||||
await db.update(chatDeliveries).set({
|
||||
receivedAt: new Date(startedAt.getTime() + 1_000),
|
||||
processedAt: new Date(startedAt.getTime() + 2_000),
|
||||
normalizedEvent: { runtimeContext: {
|
||||
generation: 7, credentialFingerprint: "a".repeat(64),
|
||||
} },
|
||||
}).where(eq(chatDeliveries.id, deliveryId));
|
||||
await expect(authorize()).resolves.toEqual({ endpointId, conversationId });
|
||||
await db.update(chatDeliveries).set({ normalizedEvent: { runtimeContext: {
|
||||
generation: 8, credentialFingerprint: "a".repeat(64),
|
||||
} } }).where(eq(chatDeliveries.id, deliveryId));
|
||||
await expect(authorize()).rejects.toThrow(denied);
|
||||
await db.update(chatDeliveries).set({ normalizedEvent: { runtimeContext: {
|
||||
generation: 7, credentialFingerprint: "malformed",
|
||||
} } }).where(eq(chatDeliveries.id, deliveryId));
|
||||
await expect(authorize()).rejects.toThrow(denied);
|
||||
} finally {
|
||||
await db.update(chatEndpoints).set({ status: "active", setup: sql`'{}'::jsonb` }).where(eq(chatEndpoints.id, endpointId));
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps transaction and savepoint sessions separate from a warmed root template", async () => {
|
||||
await authorize();
|
||||
await expect(db.transaction(async (tx) => {
|
||||
await tx.update(chatEndpoints).set({ status: "paused" }).where(eq(chatEndpoints.id, endpointId));
|
||||
await expect(authorize(tx as unknown as Db)).rejects.toThrow(denied);
|
||||
await expect(authorize()).resolves.toEqual({ endpointId, conversationId });
|
||||
await expect(tx.transaction(async (savepoint) => {
|
||||
await savepoint.update(chatEndpoints).set({ status: "active" }).where(eq(chatEndpoints.id, endpointId));
|
||||
await expect(authorize(savepoint as unknown as Db)).resolves.toEqual({ endpointId, conversationId });
|
||||
throw new Error("rollback-savepoint");
|
||||
})).rejects.toThrow("rollback-savepoint");
|
||||
await expect(authorize(tx as unknown as Db)).rejects.toThrow(denied);
|
||||
throw new Error("rollback-outer");
|
||||
})).rejects.toThrow("rollback-outer");
|
||||
await expect(authorize()).resolves.toEqual({ endpointId, conversationId });
|
||||
await db.transaction(async (tx) => {
|
||||
await expect(authorize(tx as unknown as Db)).resolves.toEqual({ endpointId, conversationId });
|
||||
});
|
||||
});
|
||||
|
||||
it("executes every lock mode on the same transaction handle and PostgreSQL session", async () => {
|
||||
await db.transaction(async (tx) => {
|
||||
for (const mode of ["read", "blocking", "nonblocking", "read"] as const) {
|
||||
await expect(authorizeChatConversationForBoundRun(tx as unknown as Db, binding, context, mode))
|
||||
.resolves.toEqual({ endpointId, conversationId });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves blocking row locks and NOWAIT contention after read templates are warm", async () => {
|
||||
await authorize();
|
||||
let release!: () => void;
|
||||
let acquired!: () => void;
|
||||
const acquiredPromise = new Promise<void>((resolve) => { acquired = resolve; });
|
||||
const held = new Promise<void>((resolve) => { release = resolve; });
|
||||
const locker = db.transaction(async (tx) => {
|
||||
await tx.select({ id: chatMessageLinks.id }).from(chatMessageLinks)
|
||||
.where(eq(chatMessageLinks.commentId, commentId)).for("update");
|
||||
acquired();
|
||||
await held;
|
||||
});
|
||||
try {
|
||||
await acquiredPromise;
|
||||
await expect(authorize()).resolves.toEqual({ endpointId, conversationId });
|
||||
let contention: unknown;
|
||||
try { await db.transaction((tx) => authorizeChatConversationForBoundRun(tx as unknown as Db, binding, context, "nonblocking")); }
|
||||
catch (error) { contention = error; }
|
||||
expect(isExternalChatWaitAuthorizationContention(contention)).toBe(true);
|
||||
await expect(db.transaction(async (tx) => {
|
||||
await tx.execute(sql`set local lock_timeout = '100ms'`);
|
||||
await authorizeChatConversationForBoundRun(tx as unknown as Db, binding, context, "blocking");
|
||||
})).rejects.toThrow();
|
||||
} finally { release(); await locker; }
|
||||
await db.transaction(async (tx) => {
|
||||
await expect(authorizeChatConversationForBoundRun(tx as unknown as Db, binding, context, "nonblocking"))
|
||||
.resolves.toEqual({ endpointId, conversationId });
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -456,30 +456,7 @@ async function principalAuthorized(
|
||||
return endpoint.sponsorUserId ? activeMember(endpoint.sponsorUserId) : true;
|
||||
}
|
||||
|
||||
/** Caller must independently verify the current run/issue execution owner. */
|
||||
export async function authorizeChatConversationForBoundRun(
|
||||
tx: Db,
|
||||
binding: ChatReuseBinding,
|
||||
contextSnapshot: unknown,
|
||||
lockMode: AuthorizationLockMode = "blocking",
|
||||
): Promise<AuthorizedConversation> {
|
||||
let context = record(contextSnapshot);
|
||||
if (context.source === "issue.interaction.respond") {
|
||||
const answer = await resolveExternalChatQuestionResponse(tx, binding, context, lockMode);
|
||||
if (!answer) throw new Error("paperclip_runner_chat_attachment_binding_denied");
|
||||
context = answer.authorizationContext;
|
||||
}
|
||||
const source = typeof context.source === "string" ? context.source : "";
|
||||
const provider = boundExternalChatProvider(source);
|
||||
const commentIds = wakeCommentIds(context);
|
||||
if (
|
||||
!provider ||
|
||||
(context.paperclipHarnessCheckedOut !== true &&
|
||||
context.paperclipExternalChatExecutionBound !== true) ||
|
||||
commentIds.length === 0
|
||||
) {
|
||||
throw new Error("paperclip_runner_chat_attachment_binding_denied");
|
||||
}
|
||||
function prepareBoundRunLinks(tx: Db, lockMode: AuthorizationLockMode) {
|
||||
const linksQuery = tx
|
||||
.select({
|
||||
commentId: chatMessageLinks.commentId,
|
||||
@@ -539,25 +516,69 @@ export async function authorizeChatConversationForBoundRun(
|
||||
)
|
||||
.where(
|
||||
and(
|
||||
eq(chatMessageLinks.companyId, binding.companyId),
|
||||
eq(chatMessageLinks.companyId, sql.placeholder("companyId")),
|
||||
eq(chatMessageLinks.direction, "inbound"),
|
||||
inArray(chatMessageLinks.commentId, commentIds),
|
||||
sql`${chatMessageLinks.commentId} = any(${sql.placeholder("commentIds")}::uuid[])`,
|
||||
eq(chatDeliveries.state, "processed"),
|
||||
eq(chatConversations.issueId, binding.issueId),
|
||||
eq(chatConversations.issueId, sql.placeholder("issueId")),
|
||||
inArray(chatConversations.state, ["active", "waiting"]),
|
||||
eq(
|
||||
chatEndpoints.provider,
|
||||
provider as typeof chatEndpoints.$inferSelect.provider,
|
||||
),
|
||||
eq(chatEndpoints.assignedAgentId, binding.agentId),
|
||||
eq(chatEndpoints.provider, sql.placeholder("provider")),
|
||||
eq(chatEndpoints.assignedAgentId, sql.placeholder("agentId")),
|
||||
inArray(chatEndpoints.status, ["active", "verifying"]),
|
||||
),
|
||||
);
|
||||
const links = await (lockMode === "read"
|
||||
return (lockMode === "read"
|
||||
? linksQuery
|
||||
: lockMode === "nonblocking"
|
||||
? linksQuery.for("update", { noWait: true })
|
||||
: linksQuery.for("update"));
|
||||
: linksQuery.for("update")).prepare("chat_bound_run_links");
|
||||
}
|
||||
|
||||
const boundRunLinksTemplates = new WeakMap<
|
||||
Db,
|
||||
Partial<Record<AuthorizationLockMode, ReturnType<typeof prepareBoundRunLinks>>>
|
||||
>();
|
||||
|
||||
function boundRunLinksTemplate(tx: Db, lockMode: AuthorizationLockMode) {
|
||||
let templates = boundRunLinksTemplates.get(tx);
|
||||
if (!templates) {
|
||||
templates = {};
|
||||
boundRunLinksTemplates.set(tx, templates);
|
||||
}
|
||||
return templates[lockMode] ??= prepareBoundRunLinks(tx, lockMode);
|
||||
}
|
||||
|
||||
/** Caller must independently verify the current run/issue execution owner. */
|
||||
export async function authorizeChatConversationForBoundRun(
|
||||
tx: Db,
|
||||
binding: ChatReuseBinding,
|
||||
contextSnapshot: unknown,
|
||||
lockMode: AuthorizationLockMode = "blocking",
|
||||
): Promise<AuthorizedConversation> {
|
||||
let context = record(contextSnapshot);
|
||||
if (context.source === "issue.interaction.respond") {
|
||||
const answer = await resolveExternalChatQuestionResponse(tx, binding, context, lockMode);
|
||||
if (!answer) throw new Error("paperclip_runner_chat_attachment_binding_denied");
|
||||
context = answer.authorizationContext;
|
||||
}
|
||||
const source = typeof context.source === "string" ? context.source : "";
|
||||
const provider = boundExternalChatProvider(source);
|
||||
const commentIds = wakeCommentIds(context);
|
||||
if (
|
||||
!provider ||
|
||||
(context.paperclipHarnessCheckedOut !== true &&
|
||||
context.paperclipExternalChatExecutionBound !== true) ||
|
||||
commentIds.length === 0
|
||||
) {
|
||||
throw new Error("paperclip_runner_chat_attachment_binding_denied");
|
||||
}
|
||||
const links = await boundRunLinksTemplate(tx, lockMode).execute({
|
||||
companyId: binding.companyId,
|
||||
issueId: binding.issueId,
|
||||
agentId: binding.agentId,
|
||||
provider,
|
||||
commentIds,
|
||||
});
|
||||
const linkedCommentIds = new Set(links.map((row) => row.commentId));
|
||||
const conversationIds = new Set(links.map((row) => row.conversation.id));
|
||||
const endpointIds = new Set(links.map((row) => row.endpoint.id));
|
||||
|
||||
Reference in new issue
Block a user