diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index beb2c0b436a..f844a18bce3 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -5,6 +5,8 @@ import { createSystemRecovery } from "../services/system-recovery-state.js"; import { remoteTerminationReceipt } from "../services/remote-execution-termination.js"; import { stageReceiptReactionRemovals } from "../services/chat-receipt-reactions.js"; import type { Db } from "@paperclipai/db"; +import { createDatabaseResponseCapture, installDatabaseResponseCapture } from "@paperclipai/db/response-capture"; +import { githubResponseAuthorityFields } from "../services/chat-channels/github/response-authority-read.js"; import * as activityLogModule from "../services/activity-log.js"; import * as connectionCredentialBindings from "../services/connection-credential-bindings.js"; import { chatSlackManagerGrants, chatSlackRegistrations, toolOauthStates } from "@paperclipai/db"; @@ -2585,6 +2587,81 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { .rejects.toMatchObject({ status: 403 }); }); + it("bounds GitHub response authorization reads with large saved chat content", async () => { + const f = await toolReplyFixture(); + const [delivery] = await db.select().from(chatDeliveries).where(eq(chatDeliveries.endpointId, f.endpoint.id)); + const [resource] = await db.select().from(chatEndpointResources).where(eq(chatEndpointResources.id, f.conversation.resourceId!)); + const [configuration] = await db.select().from(chatGitHubConfigurations).where(eq(chatGitHubConfigurations.endpointId, f.endpoint.id)); + const bulk = "unused-content-for-response-authorization".repeat(8_192); + const setup = { ...f.endpoint.setup, unusedContent: bulk }; + await db.update(chatEndpoints).set({ communicationInstructions: bulk, setup }).where(eq(chatEndpoints.id, f.endpoint.id)); + await db.update(chatConversations).set({ communicationGuidance: bulk }).where(eq(chatConversations.id, f.conversation.id)); + await db.update(chatEndpointResources).set({ metadata: { ...resource.metadata, unusedContent: bulk } }) + .where(eq(chatEndpointResources.id, resource.id)); + await db.update(chatGitHubConfigurations).set({ configuration: { ...configuration.configuration, + defaults: { ...configuration.configuration.defaults, instructions: bulk } } }) + .where(eq(chatGitHubConfigurations.endpointId, f.endpoint.id)); + const normalizedEvent = { ...delivery.normalizedEvent, unusedContent: bulk }; + await db.update(chatDeliveries).set({ normalizedEvent }).where(eq(chatDeliveries.id, delivery.id)); + const events: Array> = [], now = Date.now(); + const capture = createDatabaseResponseCapture({ start: now - 100, end: now + 59_000 }, event => events.push(event)); + installDatabaseResponseCapture(db, { chatMessageLinks, chatDeliveries, chatEndpoints, chatConversations, + chatEndpointResources, chatGitHubConfigurations, chatExternalPrincipals, chatIdentityLinks, + companyMemberships, chatActions, heartbeatRuns }, capture); + try { + const receipt = await githubResponseCommentService(db, f.providerFetch).tryAcknowledge(delivery.id); + expect(receipt?.id).toBeTruthy(); + } finally { capture.flush(); } + expect(Number(events.at(-1)?.responseValueBytes)).toBeLessThan(24_000); + expect(events.filter(event => event.table === "chat_message_links").length).toBeGreaterThan(0); + expect((await db.select({ normalizedEvent: chatDeliveries.normalizedEvent }).from(chatDeliveries) + .where(eq(chatDeliveries.id, delivery.id)))[0].normalizedEvent).toEqual(normalizedEvent); + expect((await db.select({ setup: chatEndpoints.setup }).from(chatEndpoints) + .where(eq(chatEndpoints.id, f.endpoint.id)))[0].setup).toEqual(setup); + }); + + it.each(["githubAutomatic", "githubIssue"])("preserves %s policy context while omitting body content", async key => { + const f = await toolReplyFixture(); + const [delivery] = await db.select().from(chatDeliveries).where(eq(chatDeliveries.endpointId, f.endpoint.id)); + const context = { event: key === "githubIssue" ? "issue_opened" : "opened", repositoryId: "418", repository: "acme/repo", + author: { id: "42", login: "octocat", isBot: false }, sender: { id: "42", login: "octocat" }, + labels: ["review"], draft: false, baseBranch: "master", headSha: "a".repeat(40), + body: "unused-body".repeat(32_768), title: "unused-title".repeat(8_192) }; + const normalizedEvent = { text: "unrelated message text", [key]: { context, revision: 42, + policy: { instructions: "unused-policy-instructions".repeat(32_768), + prompts: { opened: "unused-policy-prompt".repeat(32_768) } }, + } }; + await db.update(chatDeliveries).set({ normalizedEvent }).where(eq(chatDeliveries.id, delivery.id)); + const events: Array> = [], now = Date.now(); + const capture = createDatabaseResponseCapture({ start: now - 100, end: now + 59_000 }, event => events.push(event)); + installDatabaseResponseCapture(db, { chatDeliveries }, capture); + let projected: { event: Record }; + try { + [projected] = await db.select({ event: githubResponseAuthorityFields.delivery.normalizedEvent }) + .from(chatDeliveries).where(eq(chatDeliveries.id, delivery.id)); + } finally { capture.flush(); } + const { body: _body, title: _title, ...policy } = context; + expect(Number(events.at(-1)?.responseValueBytes)).toBeLessThan(2_048); + expect(projected.event).toEqual({ [key]: { context: policy } }); + expect((await db.select({ event: chatDeliveries.normalizedEvent }).from(chatDeliveries) + .where(eq(chatDeliveries.id, delivery.id)))[0].event).toEqual(normalizedEvent); + }); + + it.each([null, false, 7, "invalid-event", []])("preserves malformed GitHub authority input %j", async value => { + const f = await toolReplyFixture(); + const [delivery] = await db.select().from(chatDeliveries).where(eq(chatDeliveries.endpointId, f.endpoint.id)); + await db.update(chatDeliveries).set({ normalizedEvent: sql`${JSON.stringify(value)}::jsonb` }) + .where(eq(chatDeliveries.id, delivery.id)); + await db.update(chatEndpoints).set({ setup: sql`${JSON.stringify(value)}::jsonb` }) + .where(eq(chatEndpoints.id, f.endpoint.id)); + const [projected] = await db.select({ event: githubResponseAuthorityFields.delivery.normalizedEvent }) + .from(chatDeliveries).where(eq(chatDeliveries.id, delivery.id)); + const [endpoint] = await db.select({ setup: githubResponseAuthorityFields.endpoint.setup }) + .from(chatEndpoints).where(eq(chatEndpoints.id, f.endpoint.id)); + expect(projected.event).toEqual(value); + expect(endpoint.setup).toEqual(value); + }); + async function publicationRetryFixture() { const f = await toolReplyFixture("github:paperclipai/paperclip:418", true); const head = "b".repeat(40); diff --git a/server/src/services/chat-channels/github/response-authority-read.ts b/server/src/services/chat-channels/github/response-authority-read.ts new file mode 100644 index 00000000000..e5637bfbba5 --- /dev/null +++ b/server/src/services/chat-channels/github/response-authority-read.ts @@ -0,0 +1,50 @@ +import { chatConversations, chatDeliveries, chatEndpointResources, chatEndpoints, chatGitHubConfigurations } from "@paperclipai/db"; +import { sql } from "drizzle-orm"; + +const authorizationEvent = sql>`case + when jsonb_typeof(${chatDeliveries.normalizedEvent}) = 'object' then ( + select coalesce(jsonb_object_agg(field.key, case + when jsonb_typeof(field.value->'context') = 'object' then + jsonb_build_object('context', (field.value->'context') - 'body' - 'title') + else field.value end), '{}'::jsonb) + from jsonb_each(${chatDeliveries.normalizedEvent}) as field(key, value) + where field.key in ('githubAutomatic', 'githubIssue') + ) else ${chatDeliveries.normalizedEvent} end`; + +export const githubResponseAuthorityFields = { + endpoint: { + id: chatEndpoints.id, + companyId: chatEndpoints.companyId, + assignedAgentId: chatEndpoints.assignedAgentId, + botExternalId: chatEndpoints.botExternalId, + botUsername: chatEndpoints.botUsername, + provider: chatEndpoints.provider, + status: chatEndpoints.status, + allowUnlinkedPeople: chatEndpoints.allowUnlinkedPeople, + setup: sql`case when jsonb_typeof(${chatEndpoints.setup}) = 'object' + then jsonb_build_object('runtimeGeneration', ${chatEndpoints.setup}->'runtimeGeneration') + else ${chatEndpoints.setup} end`, + }, + conversation: { + id: chatConversations.id, + issueId: chatConversations.issueId, + externalThreadId: chatConversations.externalThreadId, + }, + delivery: { + id: chatDeliveries.id, + principalId: chatDeliveries.principalId, + state: chatDeliveries.state, + createdAt: chatDeliveries.createdAt, + normalizedEvent: authorizationEvent, + }, + resource: { + enabled: chatEndpointResources.enabled, + availability: chatEndpointResources.availability, + providerResourceId: chatEndpointResources.providerResourceId, + metadata: sql`case + when jsonb_typeof(${chatEndpointResources.metadata}) = 'object' + then jsonb_build_object('providerRepositoryId', ${chatEndpointResources.metadata}->'providerRepositoryId') + else ${chatEndpointResources.metadata} end`, + }, + toolsEnabled: sql`${chatGitHubConfigurations.configuration}->'toolsEnabled'`, +}; diff --git a/server/src/services/chat-channels/github/response-target.ts b/server/src/services/chat-channels/github/response-target.ts index 1e955b14482..b45ef3c8d81 100644 --- a/server/src/services/chat-channels/github/response-target.ts +++ b/server/src/services/chat-channels/github/response-target.ts @@ -4,9 +4,11 @@ import { chatActions, chatConversations, chatDeliveries, chatEndpoints, type Db import { forbidden, HttpError } from "../../../errors.js"; export type GitHubResponseSource = { - endpoint: typeof chatEndpoints.$inferSelect; - conversation: typeof chatConversations.$inferSelect; - delivery: typeof chatDeliveries.$inferSelect; + endpoint: Pick & { setup: unknown }; + conversation: Pick; + delivery: Pick; number: number; replyId: string | null; repositoryId?: string; diff --git a/server/src/services/chat-github-access.ts b/server/src/services/chat-github-access.ts index c906d75a479..22ba2c44c5b 100644 --- a/server/src/services/chat-github-access.ts +++ b/server/src/services/chat-github-access.ts @@ -1,4 +1,4 @@ -import { and, eq } from "drizzle-orm"; +import { and, eq, sql } from "drizzle-orm"; import { chatEndpoints, chatExternalPrincipals, @@ -13,12 +13,15 @@ type Database = Db | Parameters[0]>[0]; /** Current authority shared by task admission and every later publication. */ export async function githubChatPrincipalAccess( db: Database, - endpoint: typeof chatEndpoints.$inferSelect, + endpoint: Pick, principalId: string, ) { if (endpoint.provider !== "github") return null; const [saved] = await db - .select() + .select({ configuration: sql>`jsonb_build_object( + 'people', ${chatGitHubConfigurations.configuration}->'people', + 'memberAccess', ${chatGitHubConfigurations.configuration}->'memberAccess' + )` }) .from(chatGitHubConfigurations) .where( and( @@ -29,7 +32,7 @@ export async function githubChatPrincipalAccess( .for("share"); if (!saved) return null; // Preserve the existing connector until explicitly upgraded. const [principal] = await db - .select() + .select({ externalId: chatExternalPrincipals.externalId }) .from(chatExternalPrincipals) .where( and( @@ -50,7 +53,7 @@ export async function githubChatPrincipalAccess( (p) => p.githubUserId === principal.externalId, ); const [link] = await db - .select() + .select({ status: chatIdentityLinks.status, paperclipUserId: chatIdentityLinks.paperclipUserId }) .from(chatIdentityLinks) .where( and( @@ -62,7 +65,7 @@ export async function githubChatPrincipalAccess( .for("share"); const activeMember = async (userId: string) => { const [membership] = await db - .select() + .select({ membershipRole: companyMemberships.membershipRole }) .from(companyMemberships) .where( and( diff --git a/server/src/services/chat-github-events.ts b/server/src/services/chat-github-events.ts index 4766843513a..c2656f04b73 100644 --- a/server/src/services/chat-github-events.ts +++ b/server/src/services/chat-github-events.ts @@ -20,6 +20,7 @@ import { import { effectiveGitHubReviewPolicy, githubReviewSchedulingDecision, + type GitHubAutomaticPolicyContext, } from "./chat-github-review-policy.js"; const id = z.union([ @@ -226,8 +227,8 @@ export function githubAutomaticReviewEvent( export async function githubAutomaticAdmission( db: Db | Parameters[0]>[0], - endpoint: typeof chatEndpoints.$inferSelect, - context: GitHubAutomaticEventContext, + endpoint: Pick, + context: GitHubAutomaticPolicyContext, ) { const [saved] = await db .select() diff --git a/server/src/services/chat-github-response-comments.ts b/server/src/services/chat-github-response-comments.ts index 8caff8894b9..c032ade25ba 100644 --- a/server/src/services/chat-github-response-comments.ts +++ b/server/src/services/chat-github-response-comments.ts @@ -11,7 +11,8 @@ import { githubAutomaticAdmission } from "./chat-github-events.js"; import { githubBotRepositoryToken, githubBotRequest } from "./chat-github-client.js"; import { withGitHubPublicationLease } from "./chat-github-publication-lease.js"; import { projectSafeChatPublicationText } from "./chat-publication-projection.js"; -import type { GitHubAutomaticEventContext } from "@paperclipai/shared"; +import type { GitHubAutomaticPolicyContext } from "./chat-github-review-policy.js"; +import { githubResponseAuthorityFields } from "./chat-channels/github/response-authority-read.js"; import { githubRunReplyState } from "./chat-run-publications.js"; import { logActivity } from "./activity-log.js"; import { boundRunIssueId } from "./run-task-binding.js"; @@ -159,8 +160,7 @@ export async function writeGitHubResponseComment( export function githubResponseCommentService(db: Db, fetchImpl = fetch) { async function context(deliveryId: string) { - const [row] = await db.select({ endpoint: chatEndpoints, conversation: chatConversations, - delivery: chatDeliveries, resource: chatEndpointResources, config: chatGitHubConfigurations, + const [row] = await db.select({ ...githubResponseAuthorityFields, assignee: issues.assigneeAgentId, issueStatus: issues.status, sourceMessageId: chatMessageLinks.commentId }).from(chatMessageLinks) .innerJoin(chatDeliveries, and(eq(chatDeliveries.id, chatMessageLinks.deliveryId), eq(chatDeliveries.companyId, chatMessageLinks.companyId))) .innerJoin(chatConversations, and(eq(chatConversations.id, chatMessageLinks.conversationId), eq(chatConversations.companyId, chatMessageLinks.companyId))) @@ -170,13 +170,13 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) { .innerJoin(issues, and(eq(issues.id, chatConversations.issueId), eq(issues.companyId, chatEndpoints.companyId))) .where(and(eq(chatMessageLinks.deliveryId, deliveryId), eq(chatMessageLinks.direction, "inbound"), eq(chatEndpoints.provider, "github"))); if (!row || !row.delivery.principalId || !["processing", "processed"].includes(row.delivery.state) || - !["active", "verifying"].includes(row.endpoint.status) || !row.config.configuration.toolsEnabled || + !["active", "verifying"].includes(row.endpoint.status) || !row.toolsEnabled || !row.resource.enabled || row.resource.availability !== "available" || row.assignee !== row.endpoint.assignedAgentId) throw forbidden("GitHub request is no longer authorized"); const access = await githubChatPrincipalAccess(db, row.endpoint, row.delivery.principalId); if (!access?.allowed) throw forbidden("The initiating GitHub person is no longer authorized"); const automatic = (row.delivery.normalizedEvent.githubAutomatic ?? row.delivery.normalizedEvent.githubIssue) as - { context: GitHubAutomaticEventContext } | undefined; + { context: GitHubAutomaticPolicyContext } | undefined; if (automatic && !(await githubAutomaticAdmission(db, row.endpoint, automatic.context))?.allowed) throw forbidden("Automatic GitHub task authority changed"); const match = /^github:([^:]+):(?:issue:)?([1-9][0-9]*)(?::rc:([1-9][0-9]*))?$/.exec(row.conversation.externalThreadId); diff --git a/server/src/services/chat-github-review-policy.ts b/server/src/services/chat-github-review-policy.ts index cfb805e1be2..f014e4ae066 100644 --- a/server/src/services/chat-github-review-policy.ts +++ b/server/src/services/chat-github-review-policy.ts @@ -8,9 +8,14 @@ import { type GitHubReviewConclusion, type GitHubReviewEventContext, type GitHubAutomaticEventContext, + type GitHubIssueEventContext, type GitHubReviewPolicy, } from "@paperclipai/shared"; +export type GitHubAutomaticPolicyContext = + | Pick + | Pick; + export function effectiveGitHubReviewPolicy( configuration: GitHubChatConfiguration, repositoryId: string, @@ -85,7 +90,7 @@ export function githubReviewLineIsInPatch( export function githubReviewSchedulingDecision(input: { configuration: GitHubChatConfiguration; - context: GitHubAutomaticEventContext; + context: GitHubAutomaticPolicyContext; repositoryEnabled: boolean; /** Resolved against current company membership and the user's confirmed link. */ linkedMemberUserId: string | null; diff --git a/tests/e2e/agent-chat-projects.spec.ts b/tests/e2e/agent-chat-projects.spec.ts index c8fcdc2345e..a631d3ea928 100644 --- a/tests/e2e/agent-chat-projects.spec.ts +++ b/tests/e2e/agent-chat-projects.spec.ts @@ -1,10 +1,48 @@ -import { expect, test } from "@playwright/test"; +import { expect, test, type APIRequestContext, type APIResponse } from "@playwright/test"; import { idle, json, send, setup } from "./agent-chat.shared"; test.use({ trace: "retain-on-failure" }); test.setTimeout(120_000); +test("cleanup restores agent chat experiments when cancellation fails", async ({ request }) => { + const original = await json(await request.get("/api/instance/settings/experimental")); + const response = (status: number, body: unknown) => ({ + ok: () => status >= 200 && status < 300, + status: () => status, + text: async () => JSON.stringify(body), + json: async () => body, + }) as APIResponse; + const failingRequest = new Proxy(request, { + get(target, property) { + if (property === "get") return async (...args: Parameters) => + args[0].endsWith("/live-runs") + ? response(200, [{ id: "cleanup-cancellation-fixture" }]) + : target.get(...args); + if (property === "post") return async (...args: Parameters) => + args[0].endsWith("/heartbeat-runs/cleanup-cancellation-fixture/cancel") + ? response(500, { error: "simulated cancellation failure" }) + : target.post(...args); + const value = Reflect.get(target, property, target); + return typeof value === "function" ? value.bind(target) : value; + }, + }); + try { + const f = await setup(failingRequest); + await expect(f.restore()).rejects.toThrow("500"); + const restored = await json(await request.get("/api/instance/settings/experimental")); + expect(restored).toMatchObject({ + enableAgentChat: original.enableAgentChat, + enableClassicTaskInterface: original.enableClassicTaskInterface, + }); + } finally { + await json(await request.patch("/api/instance/settings/experimental", { data: { + enableAgentChat: original.enableAgentChat, + enableClassicTaskInterface: original.enableClassicTaskInterface, + } })); + } +}); + /** * Agent chat project and content coverage: project cards, split handoff, * uploads, Ask/Plan modes, repository selection, plan approval, and shared diff --git a/tests/e2e/agent-chat.shared.ts b/tests/e2e/agent-chat.shared.ts index ab235494eb5..592af7641de 100644 --- a/tests/e2e/agent-chat.shared.ts +++ b/tests/e2e/agent-chat.shared.ts @@ -113,21 +113,24 @@ export async function setup(request: APIRequestContext) { ), }); } finally { - const runs = await json( - await request.get(`/api/companies/${company.id}/live-runs`), - ); - for (const run of runs) - await json( - await request.post(`/api/heartbeat-runs/${run.id}/cancel`), + try { + const runs = await json( + await request.get(`/api/companies/${company.id}/live-runs`), ); - await json( - await request.patch("/api/instance/settings/experimental", { - data: { - enableAgentChat: original.enableAgentChat, - enableClassicTaskInterface: original.enableClassicTaskInterface, - }, - }), - ); + for (const run of runs) + await json( + await request.post(`/api/heartbeat-runs/${run.id}/cancel`), + ); + } finally { + await json( + await request.patch("/api/instance/settings/experimental", { + data: { + enableAgentChat: original.enableAgentChat, + enableClassicTaskInterface: original.enableClassicTaskInterface, + }, + }), + ); + } } }, };