mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 08:53:19 +01:00
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - An agent can review a GitHub pull request. After the agent publishes the review, the task waits in review for the next GitHub reply. A later run of the same task can inherit this wait. > - Issue lists, issue summaries and inbox attention show these waits. For each waiting task, the server first checks the requester provenance of the waiting run (`chatRunSourceMessageId`). This check is an authorization check: it walks the identity-context lineage of the run and rejects a changed requester, another task, another company or another provider. > - #20673 batched the review reads and the publication reads of one list request. It kept the provenance check as one call for each waiting run, inside `Promise.all`. > - Thus a request with N waits still built and sent at least 2N statements at the same moment. The database is fast for each statement, but the server builds each statement on its main thread. > - On a hosted instance, a CPU profile of the server showed the main thread at 96% of one core. SQL text building, per-statement timers, socket writes and garbage collection had the largest shares. Connection samples showed 235 to 292 pool connections whose last statement was the source-message link read of this check, and 53 to 112 whose last statement was its identity-context read. > - This pull request checks the provenance of all the waiting runs of one request in one batch. The runs that need the same kind of read share one statement. Each run gets the same decision as a separate call, from the same walk and the same reads. > - The benefit is that a list request with 250 waits sends 17 statements, not 658, and uses about one quarter of the CPU time. The statement count no longer follows the number of waits. ## Linked Issues or Issue Description Refs #20673 (batched the review reads and the publication reads of the same request; this change does the same for the provenance reads that #20673 kept for each run). Refs #20551 (authorizes the waits of one issue page with one read). **What happened** `readInheritedGitHubReviewWaits` called `chatRunSourceMessageId` one time for each waiting run, inside `Promise.all`. Each call sent an identity-context read and a source-message link read. A run that continues an earlier run also sent a parent-context read and a source-run read for each continuation in its lineage. `listExternalReviewWaits` calls this function for the issue list, the issue summaries and the inbox attention of a company, so each of those requests repeated the same reads for every waiting task. Evidence from a hosted instance (sanitized): - CPU profile of the server process, 30 s, 5 ms samples: main thread at 96% of one core. Building SQL text for select statements: 17.7% inclusive. The idle-timer restart that the database client does for each statement: 7.6%. Socket writes: 6.2%. Garbage collection: 13%. - Three samples of the pool connections (about 494 connections). 235 to 292 connections had the source-message link read of `chatRunSourceMessageId` as their last statement. 53 to 112 connections had its identity-context read as their last statement. - The `DISTINCT ON` review read that #20673 added ran about 29 times each minute. It runs one time for each list request, so each request started hundreds of provenance reads. **Expected behavior** One list request sends a number of provenance statements that does not follow the number of waiting tasks. The decision for each run does not change. **Steps to reproduce** 1. Put many tasks of one company in review, each with an inherited GitHub review wait. 2. Open the issue list or the inbox. 3. Count the statements that the server sends. The identity-context read and the source-message link read run one time for each waiting task. The benchmark below does these steps on a local PostgreSQL. ## What Changed - `server/src/services/chat-channels/run-origin.ts`: - The provenance decision is now one walk (`sourceMessageWalk`). The walk asks for one read at a time and decides on the returned rows only. Its checks, their order and its error messages are the checks of the previous `chatRunSourceMessageId`. - The five reads of the walk (active identity context, source-message link, board message, parent context, source run) are defined one time in `provenanceReads`. Each definition takes its values either as literals or as SQL columns. - `chatRunSourceMessageId` (single run) drives the walk with one statement for each read, as before. The joins, the company scope and the filters are the same. The identity-context reads now select the seven columns that the walk uses instead of every column. - New `chatRunSourceMessageIds(db, runs, options)` (batch). It drives one walk for each run. In each round, the runs that wait for the same kind of read share one statement: `unnest(...)` of the request values, `CROSS JOIN LATERAL` the same single-row read with the request columns in place of the literals. Thus each run gets the row that its own single read returns, with `LIMIT 1` and the company scope on each join. The result for each run is `{ messageId }` or `{ forbidden }` (the 403 that the single call throws). A database error rejects the whole batch, as `Promise.all` did. - `server/src/services/chat-channels/github/review-wait.ts`: `readInheritedGitHubReviewWaits` checks the provenance of its runs with the batch. A 403 still gives `revoked`. The single-run `readInheritedGitHubReviewWait` does not change. - `server/src/services/chat-channels/run-origin.test.ts`: two new tests (see Verification). - `server/src/__tests__/github-review-wait.test.ts`: one new regression test for the statement count. - `server/scripts/benchmark-review-wait-provenance.ts`: the reproduction and the before/after measurement. It loads the pre-fix modules from `--baseline-ref` and fails unless both versions return the same waits. - `scripts/benchmark-github-review-wait.ts`: removed an assertion that `run-origin.ts` is byte-identical to its baseline. Both sides of that benchmark import the current helper, so the comparison stays fair. Without this change, the documented command of that benchmark fails. - `doc/DATABASE.md`: describes the batch provenance check and the new benchmark command. Statement count of one batch: the walk of one run does at most 3 + 4L reads, where L is the number of continuations in its requester lineage. The batch does the same number of rounds for its longest lineage. Each round sends at most one statement for each kind of read (5 kinds), for any number of runs. A request whose waiting runs accepted their GitHub comment directly sends 2 provenance statements. ## Verification Red/green regression test (`server/src/__tests__/github-review-wait.test.ts`): - `reads the waits of forty runs with the statements of four runs`. Before the fix: `expected { few: 10, many: 82 } to deeply equal { few: 4, many: 4 }`. After the fix: passes. Equivalence tests (`server/src/services/chat-channels/run-origin.test.ts`), on real PostgreSQL: - `gives every run of a batch the decision of its own check, in every provenance state`. 33 runs, each in a different provenance state, are checked with the single function and with the batch for five option sets (no options, legacy wake allowed, GitHub, GitHub with legacy wake, Slack). The test compares the decision of each run (message, `null`, or the 403 and its message) for: the full batch, the reversed batch, and a batch of one run. The states include the negative cases: a run of another company, a message that is linked only in another company, a message that is linked to another task, a conversation that moved to another task, a continuation on another task, a changed requester (run, identity context, continuation, board message), a link of another provider, a missing link, an outbound link only, a deleted message, an agent message, a rejected or pending identity context, a missing parent, another agent and a lineage cycle. The test also states the expected GitHub decision for each state, so the comparison cannot pass on two wrong answers. - `checks the runs of a batch with statements that follow the lineage, not the number of runs`. 4 runs and 40 runs each send 2 statements. 40 separate checks send 80. One continuation in each lineage gives 4 statements for the batch. - A mutation check: when the batch gives a row to the wrong request, both tests fail. Commands: - `pnpm vitest run server/src/services/chat-channels/run-origin.test.ts server/src/__tests__/github-review-wait.test.ts` - Focused run of 15 test files that import the changed modules, their callers or the shared seed helper, including `external-chat-wait.integration.test.ts`, `chat-github-tools.test.ts`, `slack-authority.test.ts` and `issue-list-review-wait-authorization.integration.test.ts`: 482 tests passed. In `idle-completed-history.test.ts`, `afterEach` hooks (a `TRUNCATE` of the test database) timed out at 30 s for 1 test in this run and for 3 tests in a second run of the file, while other agents loaded the machine. No assertion failed. The three tests of that file that reach the changed code pass when they run alone. CI runs the full file. - `server/src/__tests__/chat-channels.integration.test.ts` also uses the single check. It was not run locally (very large file). CI runs it. - Typecheck of the changed files and of the callers of `chatRunSourceMessageId` with a focused `tsc` program: no errors in these files. The full server typecheck was not run locally (shared machine). CI supplies it. - The first CI run of this pull request failed `ci / policy`: the module boundary check rejects an insert into the agents table outside the agent lifecycle module, and the benchmark seeded its fixture agent that way. The second commit seeds that agent with SQL. `pnpm check:module-boundaries` passes locally, and the benchmark gives the same statement counts. - `pnpm verify` returns exit 2 (broader verification required) because the change adds a file under `server/scripts/`. CI supplies the broad coverage. Before/after measurement: ```sh pnpm --filter @paperclipai/server exec tsx scripts/benchmark-review-wait-provenance.ts --baseline-ref=e2cf9a670fc5d5f5058fd0972beb71650f77c554 ``` One call of `listExternalReviewWaits` (the read of one issue list request). Embedded PostgreSQL 18.1, Node v26.4.0, `VACUUM ANALYZE` after seeding. One company, one agent, one GitHub repository. Every wait has a published review. Every tenth waiting run continues three earlier runs. 3 warm-up calls, then 15 pairs in alternating order. Both versions return the same waits and the same inherited wait for each run. | Waits | Version | Statements | SQL text bytes | Wall ms, median (min–max) | CPU ms, median (min–max) | | --- | --- | --- | --- | --- | --- | | 20 | before | 60 | 31,084 | 14.1 (12.8–19.6) | 13.7 (8.7–39.3) | | 20 | after | 17 | 15,847 | 12.5 (9.4–22.7) | 8.8 (4.1–87.1) | | 250 | before | 658 | 300,611 | 63.3 (58.1–89.6) | 78.1 (64.8–103.3) | | 250 | after | 17 | 17,171 | 20.8 (18.9–29.2) | 18.7 (14.1–27.3) | | 1000 | before | 2,608 | 1,179,710 | 256.0 (221.0–363.2) | 303.3 (262.2–507.8) | | 1000 | after | 17 | 21,695 | 54.9 (46.7–81.7) | 51.0 (41.6–72.2) | Of the 658 statements before the fix at 250 waits, 650 are provenance reads: 325 identity-context reads, 250 source-message link reads and 75 source-run reads. After the fix, 9 statements do the same reads. The other 8 statements of the request do not change. Sizes are set with `WAITS=20` and `WAITS=1000`. Other agents used the same machine during the measurement, so the maximum values have noise. - The batch statements use index scans only (`run_identity_contexts_pkey`, `run_identity_contexts_company_run_idx`, `chat_message_links_company_comment_idx`, `chat_conversations_pkey`, `chat_endpoints_company_id_uq`, `heartbeat_runs_pkey`). The 9 batch statements of a request with 250 waits take about 1 ms of server execution in total. - Wall time and CPU time are almost equal before the fix. This shows that the cost is in the server process, not in the database. Not verified: - The effect on the hosted instance. This pull request does not deploy. The share of the main-thread CPU that this path uses there is not known; the profile gives the share of all statement building. ## Risks - The provenance check is an authorization check. The risk is a batch decision that differs from the single decision. Mitigation: both paths run the same walk and build each read from the same definition. The batch read is the single read inside `CROSS JOIN LATERAL`, for each request row. The equivalence test compares both paths in every state. - A database error (for example an invalid UUID in a legacy task binding) rejects the whole batch. The previous `Promise.all` also rejected the whole request. The callers of the batch pass the native task of each run. - The statement count of the batch follows the longest lineage of its runs. A batch with one very long lineage sends as many statements as that run sent before, in sequence. It does not send more. - The batch sends its statements in sequence on one connection. Before, the reads of N runs used up to N pool connections at the same moment. - The single-run callers (GitHub tools, review publication, response comments, Slack authority, the finalizer, the status committer) keep one statement for each read. Their identity-context reads select fewer columns. - No migration. No new index. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Fable 5.2 (`claude-fable-5-2`), Anthropic, used through Claude Code with tool use (code reading, editing, shell, tests). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
347 lines
26 KiB
TypeScript
347 lines
26 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { eq } from "drizzle-orm";
|
|
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
|
import { agents, chatConversations, chatEndpoints, chatMessageLinks, companies, createDb, heartbeatRuns, issueComments,
|
|
issues, runIdentityContexts } from "@paperclipai/db";
|
|
import type { ChatProvider } from "@paperclipai/shared";
|
|
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js";
|
|
import { seedGitHubReviewWait } from "../../__tests__/helpers/github-review-wait.js";
|
|
import { HttpError } from "../../errors.js";
|
|
import { boundRunIssueId, boundRunIssueIdSql } from "../run-task-binding.js";
|
|
import { chatRunSourceMessageId, chatRunSourceMessageIds } from "./run-origin.js";
|
|
|
|
const support = await getEmbeddedPostgresTestSupport();
|
|
(support.supported ? describe : describe.skip)("channel task provenance", () => {
|
|
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
let db: ReturnType<typeof createDb>;
|
|
|
|
beforeAll(async () => {
|
|
database = await startEmbeddedPostgresTestDatabase("paperclip-channel-origin-");
|
|
db = createDb(database.connectionString);
|
|
}, 30_000);
|
|
afterAll(async () => { await database?.cleanup(); }, 60_000);
|
|
|
|
async function fixture() {
|
|
const [company] = await db.insert(companies).values({ name: "Channel continuation", issuePrefix: randomUUID().slice(0, 8) }).returning();
|
|
const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Channel agent", role: "general", adapterType: "codex_local" }).returning();
|
|
const [issue] = await db.insert(issues).values({ companyId: company.id, title: "Channel task", assigneeAgentId: agent.id }).returning();
|
|
const [source, continued] = await db.insert(heartbeatRuns).values([
|
|
{ companyId: company.id, agentId: agent.id, status: "succeeded", contextSnapshot: { issueId: issue.id }, responsibleUserId: "requester" },
|
|
{ companyId: company.id, agentId: agent.id, status: "running", nativeIssueId: issue.id, contextSnapshot: {}, responsibleUserId: "requester" },
|
|
]).returning();
|
|
const messageId = randomUUID();
|
|
const [origin] = await db.insert(runIdentityContexts).values({ companyId: company.id, runId: source.id, revision: 1, responsibleUserId: "requester", messageId, cause: "instruction", correlationId: "message" }).returning();
|
|
const [context] = await db.insert(runIdentityContexts).values({ companyId: company.id, runId: continued.id, revision: 1, responsibleUserId: "requester", parentContextId: origin.id, cause: "monitor", correlationId: "dispatch" }).returning();
|
|
const [run] = await db.update(heartbeatRuns).set({ activeIdentityContextId: context.id }).where(eq(heartbeatRuns.id, continued.id)).returning();
|
|
return { company, agent, issue, source, run, origin, context, messageId };
|
|
}
|
|
|
|
it("recovers the accepted message across repeated native and legacy task continuations", async () => {
|
|
const f = await fixture();
|
|
expect(await chatRunSourceMessageId(db, f.run)).toBe(f.messageId);
|
|
const [next] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.agent.id, status: "running", issueId: f.issue.id, responsibleUserId: "requester" }).returning();
|
|
const [context] = await db.insert(runIdentityContexts).values({ companyId: f.company.id, runId: next.id, revision: 1, responsibleUserId: "requester", parentContextId: f.context.id, cause: "retry", correlationId: "dispatch" }).returning();
|
|
const [run] = await db.update(heartbeatRuns).set({ activeIdentityContextId: context.id }).where(eq(heartbeatRuns.id, next.id)).returning();
|
|
expect(await chatRunSourceMessageId(db, run)).toBe(f.messageId);
|
|
});
|
|
|
|
it("keeps valid channel authority after more than 100 continuations", async () => {
|
|
const f = await fixture();
|
|
const runs = Array.from({ length: 120 }, () => ({ id: randomUUID(), companyId: f.company.id, agentId: f.agent.id, status: "succeeded", nativeIssueId: f.issue.id, responsibleUserId: "requester" }));
|
|
await db.insert(heartbeatRuns).values(runs);
|
|
const ids = runs.map(() => randomUUID());
|
|
await db.insert(runIdentityContexts).values(runs.map((run, index) => ({ id: ids[index], companyId: f.company.id, runId: run.id, revision: 1, responsibleUserId: "requester", parentContextId: index ? ids[index - 1] : f.context.id, cause: "monitor", correlationId: "dispatch" })));
|
|
const [run] = await db.update(heartbeatRuns).set({ activeIdentityContextId: ids.at(-1)! }).where(eq(heartbeatRuns.id, runs.at(-1)!.id)).returning();
|
|
expect(await chatRunSourceMessageId(db, run)).toBe(f.messageId);
|
|
});
|
|
|
|
it("stops at a new accepted message instead of borrowing an earlier person's origin", async () => {
|
|
const f = await fixture();
|
|
const messageId = randomUUID();
|
|
await db.update(runIdentityContexts).set({ messageId, responsibleUserId: "new-requester" }).where(eq(runIdentityContexts.id, f.context.id));
|
|
const [run] = await db.update(heartbeatRuns).set({ responsibleUserId: "new-requester", contextSnapshot: { wakeCommentId: f.messageId } }).where(eq(heartbeatRuns.id, f.run.id)).returning();
|
|
expect(await chatRunSourceMessageId(db, run, { allowLegacyWakeComment: true })).toBe(messageId);
|
|
});
|
|
|
|
it("recovers a legacy wake only through a verified same-requester parent run", async () => {
|
|
const f = await fixture();
|
|
await db.update(runIdentityContexts).set({ messageId: null }).where(eq(runIdentityContexts.id, f.origin.id));
|
|
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: f.issue.id, wakeCommentId: f.messageId } }).where(eq(heartbeatRuns.id, f.source.id));
|
|
expect(await chatRunSourceMessageId(db, f.run)).toBeNull();
|
|
expect(await chatRunSourceMessageId(db, f.run, { allowLegacyWakeComment: true })).toBe(f.messageId);
|
|
await db.update(heartbeatRuns).set({ responsibleUserId: "another-person" }).where(eq(heartbeatRuns.id, f.source.id));
|
|
expect(await chatRunSourceMessageId(db, f.run, { allowLegacyWakeComment: true })).toBeNull();
|
|
});
|
|
|
|
it("does not borrow GitHub origin from an earlier requester after a board message", async () => {
|
|
const f = await fixture();
|
|
const [message] = await db.insert(issueComments).values({ companyId: f.company.id, issueId: f.issue.id, authorType: "user", authorUserId: "new-requester", body: "Continue" }).returning();
|
|
await db.update(runIdentityContexts).set({ messageId: message.id, responsibleUserId: "new-requester" }).where(eq(runIdentityContexts.id, f.context.id));
|
|
const [run] = await db.update(heartbeatRuns).set({ responsibleUserId: "new-requester", contextSnapshot: { wakeCommentId: f.messageId } }).where(eq(heartbeatRuns.id, f.run.id)).returning();
|
|
expect(await chatRunSourceMessageId(db, run, { provider: "github", allowLegacyWakeComment: true })).toBeNull();
|
|
});
|
|
|
|
it("recovers a legacy GitHub source after a verified same-user board message", async () => {
|
|
const f = await fixture();
|
|
const [message] = await db.insert(issueComments).values({ companyId: f.company.id, issueId: f.issue.id, authorType: "user", authorUserId: "requester", body: "Continue" }).returning();
|
|
await db.update(runIdentityContexts).set({ messageId: null }).where(eq(runIdentityContexts.id, f.origin.id));
|
|
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: f.issue.id, wakeCommentId: f.messageId } }).where(eq(heartbeatRuns.id, f.source.id));
|
|
await db.update(runIdentityContexts).set({ messageId: message.id }).where(eq(runIdentityContexts.id, f.context.id));
|
|
const [run] = await db.update(heartbeatRuns).set({ contextSnapshot: { wakeCommentId: message.id } }).where(eq(heartbeatRuns.id, f.run.id)).returning();
|
|
expect(await chatRunSourceMessageId(db, run, { provider: "github", allowLegacyWakeComment: true })).toBe(f.messageId);
|
|
await db.update(heartbeatRuns).set({ responsibleUserId: "another-person" }).where(eq(heartbeatRuns.id, f.source.id));
|
|
expect(await chatRunSourceMessageId(db, run, { provider: "github", allowLegacyWakeComment: true })).toBeNull();
|
|
});
|
|
|
|
async function breakInheritedAuthority(f: Awaited<ReturnType<typeof fixture>>, fault: string) {
|
|
if (fault === "task") {
|
|
const [other] = await db.insert(issues).values({ companyId: f.company.id, title: "Other task" }).returning();
|
|
const [otherRun] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.agent.id, status: "succeeded", nativeIssueId: other.id }).returning();
|
|
await db.update(runIdentityContexts).set({ runId: otherRun.id }).where(eq(runIdentityContexts.id, f.origin.id));
|
|
}
|
|
if (fault === "agent") {
|
|
const [other] = await db.insert(agents).values({ companyId: f.company.id, name: "Other agent", role: "general", adapterType: "codex_local" }).returning();
|
|
await db.update(heartbeatRuns).set({ agentId: other.id }).where(eq(heartbeatRuns.id, f.source.id));
|
|
}
|
|
if (fault === "company") {
|
|
const [other] = await db.insert(companies).values({ name: "Other company", issuePrefix: randomUUID().slice(0, 8) }).returning();
|
|
await db.update(runIdentityContexts).set({ companyId: other.id }).where(eq(runIdentityContexts.id, f.origin.id));
|
|
}
|
|
if (fault === "requester") await db.update(runIdentityContexts).set({ responsibleUserId: "another-person" }).where(eq(runIdentityContexts.id, f.origin.id));
|
|
if (fault === "missing-parent") await db.update(runIdentityContexts).set({ parentContextId: randomUUID() }).where(eq(runIdentityContexts.id, f.context.id));
|
|
if (fault === "pending-parent") await db.update(runIdentityContexts).set({ status: "pending" }).where(eq(runIdentityContexts.id, f.origin.id));
|
|
if (fault === "pending-current") await db.update(runIdentityContexts).set({ status: "pending" }).where(eq(runIdentityContexts.id, f.context.id));
|
|
if (fault === "cycle") await db.update(runIdentityContexts).set({ messageId: null, parentContextId: f.context.id }).where(eq(runIdentityContexts.id, f.origin.id));
|
|
}
|
|
|
|
it.each(["task", "agent", "company", "requester", "missing-parent", "pending-parent", "pending-current", "cycle"])(
|
|
"rejects a %s break in inherited authority", async fault => {
|
|
const f = await fixture();
|
|
await breakInheritedAuthority(f, fault);
|
|
await expect(chatRunSourceMessageId(db, f.run)).rejects.toMatchObject({ status: 403 });
|
|
},
|
|
);
|
|
|
|
type Run = Parameters<typeof chatRunSourceMessageId>[1];
|
|
type Options = { allowLegacyWakeComment?: boolean; provider?: ChatProvider };
|
|
type Seeded = Awaited<ReturnType<typeof seedGitHubReviewWait>>;
|
|
const optionSets: Options[] = [{}, { allowLegacyWakeComment: true }, { provider: "github" },
|
|
{ provider: "github", allowLegacyWakeComment: true }, { provider: "slack" }];
|
|
const FORBIDDEN = { changed: "Chat requester provenance changed", otherTask: "Chat request belongs to another task",
|
|
inherited: "Chat tools cannot be inherited by another task", unverified: "Chat source provenance could not be verified" };
|
|
|
|
/** The decision of the single check: its message, or the 403 that it throws. */
|
|
async function singleDecision(run: Run, options: Options) {
|
|
try { return { messageId: await chatRunSourceMessageId(db, run, options) }; } catch (error) {
|
|
if (!(error instanceof HttpError)) throw error;
|
|
return { status: error.status, message: error.message };
|
|
}
|
|
}
|
|
async function batchDecisions(runs: Run[], options: Options) {
|
|
return (await chatRunSourceMessageIds(db, runs, options)).map(result => "forbidden" in result
|
|
? { status: result.forbidden.status, message: result.forbidden.message } : result);
|
|
}
|
|
|
|
/** Count the statements that the client sends. */
|
|
async function countStatements<T>(read: () => Promise<T>) {
|
|
const client = db.$client as unknown as { unsafe: (query: string, ...rest: unknown[]) => unknown };
|
|
const unsafe = client.unsafe;
|
|
let statements = 0;
|
|
client.unsafe = (query: string, ...rest: unknown[]) => { statements += 1; return unsafe.call(client, query, ...rest); };
|
|
try { return { result: await read(), statements }; } finally { client.unsafe = unsafe; }
|
|
}
|
|
|
|
/** A later run of the task that continues the identity context of an earlier run. */
|
|
async function continuation(companyId: string, agentId: string, issueId: string, parentContextId: string, responsibleUserId = "operator") {
|
|
const [next] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "running", nativeIssueId: issueId, responsibleUserId }).returning();
|
|
const [context] = await db.insert(runIdentityContexts).values({ companyId, runId: next.id, revision: 1, responsibleUserId, parentContextId, cause: "monitor", correlationId: "dispatch" }).returning();
|
|
const [run] = await db.update(heartbeatRuns).set({ activeIdentityContextId: context.id }).where(eq(heartbeatRuns.id, next.id)).returning();
|
|
return run;
|
|
}
|
|
|
|
/** Runs in every provenance state, each with the decision that the GitHub
|
|
* check of a separate call must make. The states are a GitHub request with
|
|
* an inbound message link, its breaks, and the states of the tests above. */
|
|
async function provenanceStates() {
|
|
const states: Array<{ state: string; run: Run; github: string | null | { forbidden: string } }> = [];
|
|
|
|
// A GitHub request: the accepted message has an inbound link to the
|
|
// conversation of the task, on a GitHub endpoint.
|
|
const linked = await seedGitHubReviewWait(db);
|
|
states.push({ state: "linked request", run: linked.run, github: linked.commentId });
|
|
states.push({ state: "the same run again", run: linked.run, github: linked.commentId });
|
|
states.push({ state: "continuation of a linked request", github: linked.commentId,
|
|
run: await continuation(linked.companyId, linked.agentId, linked.issueId, linked.activeIdentityContextId) });
|
|
const [otherTask] = await db.insert(issues).values({ companyId: linked.companyId, title: "Other task", assigneeAgentId: linked.agentId }).returning();
|
|
states.push({ state: "continuation on another task", github: { forbidden: FORBIDDEN.inherited },
|
|
run: await continuation(linked.companyId, linked.agentId, otherTask.id, linked.activeIdentityContextId) });
|
|
states.push({ state: "continuation by another requester", github: { forbidden: FORBIDDEN.changed },
|
|
run: await continuation(linked.companyId, linked.agentId, linked.issueId, linked.activeIdentityContextId, "another-person") });
|
|
states.push({ state: "run of another requester", github: { forbidden: FORBIDDEN.changed },
|
|
run: { ...linked.run, responsibleUserId: "another-person" } });
|
|
states.push({ state: "run of another company", github: { forbidden: FORBIDDEN.changed },
|
|
run: { ...linked.run, companyId: (await seedGitHubReviewWait(db)).companyId } });
|
|
states.push({ state: "no identity context", github: null,
|
|
run: { ...linked.run, activeIdentityContextId: null, contextSnapshot: { wakeCommentId: linked.commentId } } });
|
|
|
|
// A run of another company whose accepted message is the linked message
|
|
// of the first company. The link, the conversation and the endpoint of
|
|
// another company are never read for this run.
|
|
const borrower = await seedGitHubReviewWait(db);
|
|
await db.update(runIdentityContexts).set({ messageId: linked.commentId }).where(eq(runIdentityContexts.id, borrower.activeIdentityContextId));
|
|
states.push({ state: "message linked in another company", run: borrower.run, github: { forbidden: FORBIDDEN.changed } });
|
|
|
|
// A run of another task of the same company whose accepted message is
|
|
// the linked message of the first task.
|
|
const sibling = await seedGitHubReviewWait(db, { companyId: linked.companyId, agentId: linked.agentId });
|
|
await db.update(runIdentityContexts).set({ messageId: linked.commentId }).where(eq(runIdentityContexts.id, sibling.activeIdentityContextId));
|
|
states.push({ state: "message linked to another task", run: sibling.run, github: { forbidden: FORBIDDEN.otherTask } });
|
|
|
|
const moved = await seedGitHubReviewWait(db);
|
|
const [elsewhere] = await db.insert(issues).values({ companyId: moved.companyId, title: "Other task" }).returning();
|
|
await db.update(chatConversations).set({ issueId: elsewhere.id }).where(eq(chatConversations.id, moved.conversationId));
|
|
states.push({ state: "conversation moved to another task", run: moved.run, github: { forbidden: FORBIDDEN.otherTask } });
|
|
|
|
const otherProvider = await seedGitHubReviewWait(db);
|
|
await db.update(chatEndpoints).set({ provider: "slack" }).where(eq(chatEndpoints.id, otherProvider.endpointId));
|
|
states.push({ state: "link of another provider", run: otherProvider.run, github: null });
|
|
|
|
const rejected = await seedGitHubReviewWait(db);
|
|
await db.update(runIdentityContexts).set({ status: "rejected" }).where(eq(runIdentityContexts.id, rejected.activeIdentityContextId));
|
|
states.push({ state: "rejected identity context", run: rejected.run, github: { forbidden: FORBIDDEN.changed } });
|
|
|
|
const reassigned = await seedGitHubReviewWait(db);
|
|
await db.update(runIdentityContexts).set({ responsibleUserId: "another-person" }).where(eq(runIdentityContexts.id, reassigned.activeIdentityContextId));
|
|
states.push({ state: "identity context of another requester", run: reassigned.run, github: { forbidden: FORBIDDEN.changed } });
|
|
|
|
// Without an inbound link, the accepted message must be a board message
|
|
// of the requester on the task of the run.
|
|
const unlinked = async (change: (f: Seeded) => Promise<unknown>) => {
|
|
const f = await seedGitHubReviewWait(db);
|
|
await db.delete(chatMessageLinks).where(eq(chatMessageLinks.commentId, f.commentId));
|
|
await change(f);
|
|
return f.run;
|
|
};
|
|
const message = (f: Seeded, values: Partial<typeof issueComments.$inferInsert>) =>
|
|
db.update(issueComments).set(values).where(eq(issueComments.id, f.commentId));
|
|
states.push({ state: "missing link, board message of the requester", run: await unlinked(async () => {}), github: null });
|
|
states.push({ state: "missing link, message of another person", github: { forbidden: FORBIDDEN.changed },
|
|
run: await unlinked(f => message(f, { authorUserId: "another-person" })) });
|
|
states.push({ state: "missing link, agent message", github: { forbidden: FORBIDDEN.changed },
|
|
run: await unlinked(f => message(f, { authorType: "agent" })) });
|
|
states.push({ state: "missing link, deleted message", github: { forbidden: FORBIDDEN.changed },
|
|
run: await unlinked(f => message(f, { deletedAt: new Date() })) });
|
|
states.push({ state: "missing link, message of another task", github: { forbidden: FORBIDDEN.changed },
|
|
run: await unlinked(async f => {
|
|
const [other] = await db.insert(issues).values({ companyId: f.companyId, title: "Other task" }).returning();
|
|
await message(f, { issueId: other.id });
|
|
}) });
|
|
const outbound = await seedGitHubReviewWait(db);
|
|
await db.update(chatMessageLinks).set({ direction: "outbound" }).where(eq(chatMessageLinks.commentId, outbound.commentId));
|
|
states.push({ state: "outbound link only", run: outbound.run, github: null });
|
|
|
|
// The states of the single-check tests above: a continuation whose
|
|
// accepted message has no link and is not a board message.
|
|
states.push({ state: "continuation of an unlinked message", run: (await fixture()).run, github: { forbidden: FORBIDDEN.changed } });
|
|
for (const fault of ["task", "agent", "company", "requester", "missing-parent", "pending-parent", "pending-current", "cycle"]) {
|
|
const f = await fixture();
|
|
await breakInheritedAuthority(f, fault);
|
|
states.push({ state: `${fault} break in inherited authority`, run: f.run, github: { forbidden:
|
|
fault === "task" || fault === "agent" ? FORBIDDEN.inherited : fault === "cycle" ? FORBIDDEN.unverified : FORBIDDEN.changed } });
|
|
}
|
|
|
|
// A legacy wake comment, through a verified parent run of the same
|
|
// requester, and after a board message of the same requester.
|
|
const legacy = await fixture();
|
|
await db.update(runIdentityContexts).set({ messageId: null }).where(eq(runIdentityContexts.id, legacy.origin.id));
|
|
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: legacy.issue.id, wakeCommentId: legacy.messageId } }).where(eq(heartbeatRuns.id, legacy.source.id));
|
|
states.push({ state: "legacy wake of a verified parent run", run: legacy.run, github: null });
|
|
const board = await fixture();
|
|
const [boardMessage] = await db.insert(issueComments).values({ companyId: board.company.id, issueId: board.issue.id, authorType: "user", authorUserId: "requester", body: "Continue" }).returning();
|
|
await db.update(runIdentityContexts).set({ messageId: null }).where(eq(runIdentityContexts.id, board.origin.id));
|
|
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: board.issue.id, wakeCommentId: board.messageId } }).where(eq(heartbeatRuns.id, board.source.id));
|
|
await db.update(runIdentityContexts).set({ messageId: boardMessage.id }).where(eq(runIdentityContexts.id, board.context.id));
|
|
const [boardRun] = await db.update(heartbeatRuns).set({ contextSnapshot: { wakeCommentId: boardMessage.id } }).where(eq(heartbeatRuns.id, board.run.id)).returning();
|
|
states.push({ state: "legacy wake after a board message of the requester", run: boardRun, github: null });
|
|
const later = await fixture();
|
|
const [laterMessage] = await db.insert(issueComments).values({ companyId: later.company.id, issueId: later.issue.id, authorType: "user", authorUserId: "new-requester", body: "Continue" }).returning();
|
|
await db.update(runIdentityContexts).set({ messageId: laterMessage.id, responsibleUserId: "new-requester" }).where(eq(runIdentityContexts.id, later.context.id));
|
|
const [laterRun] = await db.update(heartbeatRuns).set({ responsibleUserId: "new-requester", contextSnapshot: { wakeCommentId: later.messageId } }).where(eq(heartbeatRuns.id, later.run.id)).returning();
|
|
states.push({ state: "board message of a new requester", run: laterRun, github: null });
|
|
|
|
// A long lineage: the request is twelve continuations back.
|
|
let deep = linked.run;
|
|
for (let depth = 0; depth < 12; depth++) deep = await continuation(linked.companyId, linked.agentId, linked.issueId, deep.activeIdentityContextId!);
|
|
states.push({ state: "long lineage of a linked request", run: deep, github: linked.commentId });
|
|
return states;
|
|
}
|
|
|
|
it("gives every run of a batch the decision of its own check, in every provenance state", async () => {
|
|
const states = await provenanceStates();
|
|
const runs = states.map(state => state.run);
|
|
// The GitHub decision of each state is the documented one.
|
|
const github: Array<Awaited<ReturnType<typeof singleDecision>>> = [];
|
|
for (const run of runs) github.push(await singleDecision(run, { provider: "github" }));
|
|
expect(Object.fromEntries(states.map((state, index) => [state.state, github[index]]))).toEqual(
|
|
Object.fromEntries(states.map(state => [state.state, state.github !== null && typeof state.github === "object"
|
|
? { status: 403, message: state.github.forbidden } : { messageId: state.github }])));
|
|
for (const options of optionSets) {
|
|
const single: Array<Awaited<ReturnType<typeof singleDecision>>> = [];
|
|
for (const run of runs) single.push(await singleDecision(run, options));
|
|
// Every state is decided in one batch, beside every other state.
|
|
expect(await batchDecisions(runs, options)).toEqual(single);
|
|
// The decision of a run does not depend on its position or on its neighbors.
|
|
expect((await batchDecisions([...runs].reverse(), options)).reverse()).toEqual(single);
|
|
for (const [index, run] of runs.entries()) expect(await batchDecisions([run], options)).toEqual([single[index]]);
|
|
}
|
|
// The legacy wake is returned only when the caller allows it.
|
|
const legacyWake = states.findIndex(state => state.state === "legacy wake of a verified parent run");
|
|
expect((await batchDecisions(runs, { provider: "github", allowLegacyWakeComment: true }))[legacyWake])
|
|
.toEqual({ messageId: expect.any(String) });
|
|
expect(await chatRunSourceMessageIds(db, [])).toEqual([]);
|
|
}, 120_000);
|
|
|
|
it("checks the runs of a batch with statements that follow the lineage, not the number of runs", async () => {
|
|
const first = await seedGitHubReviewWait(db);
|
|
const few = [first.run], many = [first.run];
|
|
for (let wait = 1; wait < 40; wait++) {
|
|
const { run } = await seedGitHubReviewWait(db, { companyId: first.companyId, agentId: first.agentId });
|
|
many.push(run);
|
|
if (wait < 4) few.push(run);
|
|
}
|
|
const options = { provider: "github" as const };
|
|
const batchOfFew = await countStatements(() => chatRunSourceMessageIds(db, few, options));
|
|
const batchOfMany = await countStatements(() => chatRunSourceMessageIds(db, many, options));
|
|
// One identity-context read and one message-link read for the whole batch.
|
|
expect(batchOfFew.statements).toBe(2);
|
|
expect(batchOfMany.statements).toBe(2);
|
|
expect(new Set(batchOfMany.result.map(result => "messageId" in result ? result.messageId : null)).size).toBe(many.length);
|
|
// Separate checks send both reads for every run.
|
|
const separate = await countStatements(() => Promise.all(many.map(run => chatRunSourceMessageId(db, run, options))));
|
|
expect(separate.statements).toBe(2 * many.length);
|
|
expect(batchOfMany.result).toEqual(separate.result.map(messageId => ({ messageId })));
|
|
|
|
// One continuation in every lineage adds the two reads of that step to
|
|
// the batch, not two reads for every run.
|
|
const continued: Run[] = [];
|
|
for (const run of many) continued.push(await continuation(first.companyId, first.agentId, run.nativeIssueId!, run.activeIdentityContextId!));
|
|
const batchOfContinued = await countStatements(() => chatRunSourceMessageIds(db, continued, options));
|
|
expect(batchOfContinued.statements).toBe(4);
|
|
expect(batchOfContinued.result).toEqual(batchOfMany.result);
|
|
}, 120_000);
|
|
|
|
it.each(["native", "issue-column", "legacy-issue", "legacy-task"])("uses the same %s task binding in SQL and runtime authority", async source => {
|
|
const f = await fixture();
|
|
const values = {
|
|
nativeIssueId: source === "native" ? f.issue.id : null,
|
|
issueId: source === "issue-column" ? f.issue.id : null,
|
|
contextSnapshot: source === "legacy-task" ? { issueId: "", taskId: f.issue.id }
|
|
: { issueId: source === "native" ? randomUUID() : source === "issue-column" ? [] : f.issue.id },
|
|
};
|
|
const [run] = await db.update(heartbeatRuns).set(values).where(eq(heartbeatRuns.id, f.run.id)).returning();
|
|
const [projection] = await db.select({ issueId: boundRunIssueIdSql() }).from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id));
|
|
expect(projection.issueId).toBe(f.issue.id);
|
|
expect(boundRunIssueId(run)).toBe(projection.issueId);
|
|
});
|
|
});
|