diff --git a/doc/connections/GITHUB.md b/doc/connections/GITHUB.md index 84d952e9b00..90fa895341b 100644 --- a/doc/connections/GITHUB.md +++ b/doc/connections/GITHUB.md @@ -178,6 +178,24 @@ It retains GitHub's destructive classification. Read tools include Provider references: [MCP toolset configuration](https://github.com/github/github-mcp-server/blob/main/docs/server-configuration.md) and [workflow dispatch permissions](https://docs.github.com/en/rest/actions/workflows#create-a-workflow-dispatch-event). +GitHub response reconciliation reads run status and request identity without +loading prompts, runtime profiles, results, or log excerpts. Request ownership +checks still use current company, agent, task, and accepted message provenance. +The run context projection retains `issueId`, `taskId`, `wakeCommentId`, and +`wakeCommentIds`; larger runtime data stays in its owning reads. + +## Webhook delivery recovery + +The Cloud connector authenticates the outer event batch before it processes +individual encrypted App deliveries. If an inner delivery cannot be decrypted +or fails its route binding check, the instance reports a failed delivery and +leaves it unacknowledged in Cloud. A local warning includes the authenticated +delivery ID and a fixed failure reason, without event contents or raw errors. +Other valid events in the batch can still finish and acknowledge independently. +Failed ciphertext never reaches the +GitHub webhook handler. The broker retains failed deliveries for retry under +its normal lease and retention policy; this does not recover a lost old key. + ## Webhooks For the shared GitHub account connector, Paperclip Cloud verifies diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 7d3c18dadd4..af91a95ebb1 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -2564,6 +2564,23 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { const session = { companyId: f.companyId, agentId: f.assignedAgentId, issueId: conversation.issueId, runId: run.id }; return { ...f, conversation, run, session }; } + it("keeps GitHub response ownership checks independent of large run payloads", async () => { + const f = await toolReplyFixture(); + const responses = githubResponseCommentService(db, f.providerFetch); + const input = { ...f.session, issueId: f.conversation.issueId!, endpointId: f.endpoint.id }; + expect(await responses.hasWorkingResponseForRun(input)).toBe(true); + const unused = "unused-run-payload".repeat(32_768); + await db.update(heartbeatRuns).set({ + contextSnapshot: { ...f.run.contextSnapshot, prompt: unused, nativeExecutionInput: { prompt: unused } }, + runnerProfileJson: { unused }, resultJson: { unused }, stdoutExcerpt: unused, + }).where(eq(heartbeatRuns.id, f.run.id)); + expect(await responses.hasWorkingResponseForRun(input)).toBe(true); + await expect(responses.hasWorkingResponseForRun({ ...input, companyId: randomUUID() })) + .rejects.toMatchObject({ status: 403 }); + await expect(responses.hasWorkingResponseForRun({ ...input, issueId: randomUUID() })) + .rejects.toMatchObject({ status: 403 }); + }); + async function publicationRetryFixture() { const f = await toolReplyFixture("github:paperclipai/paperclip:418", true); const head = "b".repeat(40); diff --git a/server/src/__tests__/github-connection-events.test.ts b/server/src/__tests__/github-connection-events.test.ts index 905e6e7ea33..77d34f15e6a 100644 --- a/server/src/__tests__/github-connection-events.test.ts +++ b/server/src/__tests__/github-connection-events.test.ts @@ -13,6 +13,7 @@ import { import { eq } from "drizzle-orm"; import { githubConnectionEventService } from "../services/github-connection-events.js"; import { subscribeCompanyLiveEvents } from "../services/live-events.js"; +import { logger } from "../middleware/logger.js"; import type { PaperclipCloudConnector } from "../services/paperclip-cloud-connector.js"; import { getEmbeddedPostgresTestSupport, @@ -45,7 +46,7 @@ describeEmbeddedPostgres("GitHub connection event delivery", () => { await tempDb?.cleanup(); }); - it("applies a normalized merged PR once, updates the snapshot, and acknowledges Cloud", async () => { + it("applies a normalized merged PR once while retaining undecryptable siblings for retry", async () => { const companyId = randomUUID(); const userId = `github-owner-${randomUUID()}`; const applicationId = randomUUID(); @@ -144,13 +145,18 @@ describeEmbeddedPostgres("GitHub connection event delivery", () => { refresh: vi.fn(), revoke: vi.fn(), setWebhookBinding: vi.fn(async () => undefined), - leaseEvents: vi.fn(async () => ({ leaseId: `lease-${++poll}`, events: [leasedEvent] })), + leaseEvents: vi.fn(async () => ({ leaseId: `lease-${++poll}`, events: [leasedEvent], failedEventIds: ["undecryptable-delivery"] })), acknowledgeEvents: vi.fn(async () => 1), } as unknown as PaperclipCloudConnector; let currentTime = new Date("2026-09-04T12:00:05.000Z"); const service = githubConnectionEventService(db, { connector, now: () => currentTime }); - await expect(service.pollOnce()).resolves.toMatchObject({ leased: 1, processed: 1, duplicate: 0, failed: 0 }); + const warning = vi.spyOn(logger, "warn").mockImplementation(() => undefined); + await expect(service.pollOnce()).resolves.toMatchObject({ leased: 2, processed: 1, duplicate: 0, failed: 1 }); + expect(warning.mock.calls).toEqual([[{ + deliveryId: "undecryptable-delivery", reason: "inner_event_authentication_failed", + }, "GitHub connector event retained for retry"]]); + warning.mockRestore(); const [snapshot] = await db.select().from(externalObjects).where(eq(externalObjects.id, externalObjectId)); expect(snapshot).toMatchObject({ statusKey: "merged", @@ -196,15 +202,27 @@ describeEmbeddedPostgres("GitHub connection event delivery", () => { repositoryId: "99", }); expect(connector.acknowledgeEvents).toHaveBeenCalledTimes(1); + expect(connector.acknowledgeEvents).toHaveBeenLastCalledWith(expect.objectContaining({ + deliveryIds: [leasedEvent.id], + })); + expect(await db.select().from(connectionEventDeliveries).where(eq( + connectionEventDeliveries.providerDeliveryId, "undecryptable-delivery", + ))).toEqual([]); currentTime = new Date(currentTime.getTime() + 6_000); - await expect(service.pollOnce()).resolves.toMatchObject({ leased: 1, processed: 0, duplicate: 1, failed: 0 }); + await expect(service.pollOnce()).resolves.toMatchObject({ leased: 2, processed: 0, duplicate: 1, failed: 1 }); const [duplicateReceipt] = await db.select().from(connectionEventDeliveries).where(eq( connectionEventDeliveries.providerDeliveryId, leasedEvent.id, )); expect(duplicateReceipt?.attempts).toBe(1); expect(connector.acknowledgeEvents).toHaveBeenCalledTimes(2); + vi.mocked(connector.leaseEvents).mockResolvedValue({ + leaseId: "failed-only", events: [], failedEventIds: ["undecryptable-delivery"], + }); + currentTime = new Date(currentTime.getTime() + 6_000); + await expect(service.pollOnce()).resolves.toMatchObject({ leased: 1, processed: 0, failed: 1 }); + expect(connector.acknowledgeEvents).toHaveBeenCalledTimes(2); }); it.each([false, true])("applies installation events once without discarding newer verified access (refreshed: %s)", async (refreshed) => { diff --git a/server/src/services/chat-github-checks.ts b/server/src/services/chat-github-checks.ts index e8d51487d28..dceb5ae3df5 100644 --- a/server/src/services/chat-github-checks.ts +++ b/server/src/services/chat-github-checks.ts @@ -171,14 +171,14 @@ export function githubReviewCheckService(db: Db, fetchImpl = fetch) { let run = review?.runId ? ( await db - .select() + .select({ id: heartbeatRuns.id, status: heartbeatRuns.status }) .from(heartbeatRuns) .where(eq(heartbeatRuns.id, review.runId)) )[0] : undefined; if (review && !run) { const [sourceRun] = await db - .select({ run: heartbeatRuns }) + .select({ run: { id: heartbeatRuns.id, status: heartbeatRuns.status } }) .from(chatDeliveries) .innerJoin( chatMessageLinks, diff --git a/server/src/services/chat-github-response-comments.ts b/server/src/services/chat-github-response-comments.ts index 04216356621..8caff8894b9 100644 --- a/server/src/services/chat-github-response-comments.ts +++ b/server/src/services/chat-github-response-comments.ts @@ -24,6 +24,23 @@ type Api = { request(path: string, options?: { method?: string; body?: unknow type Lease = { commit(write: (tx: Transaction) => Promise): Promise }; type Source = GitHubResponseSource; type Comment = { id: number; body?: string; html_url: string; user?: { login?: string } }; +const responseRunColumns = { + id: heartbeatRuns.id, + companyId: heartbeatRuns.companyId, + agentId: heartbeatRuns.agentId, + responsibleUserId: heartbeatRuns.responsibleUserId, + activeIdentityContextId: heartbeatRuns.activeIdentityContextId, + nativeIssueId: heartbeatRuns.nativeIssueId, + issueId: heartbeatRuns.issueId, + status: heartbeatRuns.status, + updatedAt: heartbeatRuns.updatedAt, + contextSnapshot: sql>`jsonb_build_object( + 'issueId', ${heartbeatRuns.contextSnapshot}->'issueId', + 'taskId', ${heartbeatRuns.contextSnapshot}->'taskId', + 'wakeCommentId', ${heartbeatRuns.contextSnapshot}->'wakeCommentId', + 'wakeCommentIds', ${heartbeatRuns.contextSnapshot}->'wakeCommentIds' + )`, +}; const generation = (endpoint: Source["endpoint"]) => (endpoint.setup as { runtimeGeneration?: number }).runtimeGeneration ?? 0; const actionKey = (deliveryId: string) => `github_response:${deliveryId}`; const responseMarker = (source: Source) => isMainPullResponse(source) ? pullResponseMarker(source) : requestResponseMarker(source); @@ -189,7 +206,7 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) { let preserveFinal = false; if (finalBody === undefined) { const existing = await stageResponse(db, source); - const [requestRun] = await db.select().from(heartbeatRuns).innerJoin(chatMessageLinks, and( + const [requestRun] = await db.select({ runId: heartbeatRuns.id }).from(heartbeatRuns).innerJoin(chatMessageLinks, and( eq(chatMessageLinks.companyId, source.endpoint.companyId), eq(chatMessageLinks.endpointId, source.endpoint.id), eq(chatMessageLinks.deliveryId, deliveryId), eq(chatMessageLinks.direction, "inbound"), or(sql`${chatMessageLinks.commentId}::text = ${heartbeatRuns.contextSnapshot}->>'wakeCommentId'`, @@ -197,13 +214,13 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) { )).where(and(eq(heartbeatRuns.companyId, source.endpoint.companyId), eq(heartbeatRuns.agentId, source.endpoint.assignedAgentId), sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${source.conversation.issueId}`)) .orderBy(desc(heartbeatRuns.createdAt)).limit(1); - const [latestRun] = await db.select().from(heartbeatRuns).where(and( + const [latestRun] = await db.select(responseRunColumns).from(heartbeatRuns).where(and( eq(heartbeatRuns.companyId, source.endpoint.companyId), eq(heartbeatRuns.agentId, source.endpoint.assignedAgentId), sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${source.conversation.issueId}`, )).orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id)).limit(1); const originMessageId = latestRun?.activeIdentityContextId ? await chatRunSourceMessageId(db, latestRun, { allowLegacyWakeComment: true, provider: "github" }) : null; - const ownsLatestRequest = !!latestRun && (requestRun?.heartbeat_runs.id === latestRun.id || + const ownsLatestRequest = !!latestRun && (requestRun?.runId === latestRun.id || (!!source.sourceMessageId && originMessageId === source.sourceMessageId)); requestRunning = ownsLatestRequest && latestRun?.status === "running"; const terminal = latestRun && ["succeeded", "failed", "interrupted", "timed_out", "cancelled"].includes(latestRun.status); @@ -272,7 +289,7 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) { }); } async function requestsForRun(input: { companyId: string; endpointId: string; issueId: string; runId: string }) { - const [run] = await db.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.id, input.runId), eq(heartbeatRuns.companyId, input.companyId))); + const [run] = await db.select(responseRunColumns).from(heartbeatRuns).where(and(eq(heartbeatRuns.id, input.runId), eq(heartbeatRuns.companyId, input.companyId))); if (!run || boundRunIssueId(run) !== input.issueId) throw forbidden("GitHub run has no request binding"); const originMessageId = await chatRunSourceMessageId(db, run, { allowLegacyWakeComment: true, provider: "github" }); const ids = [...new Set([originMessageId, run.contextSnapshot?.wakeCommentId, ...(Array.isArray(run.contextSnapshot?.wakeCommentIds) ? run.contextSnapshot.wakeCommentIds : [])].filter((id): id is string => typeof id === "string"))]; @@ -345,7 +362,7 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) { if ((await responseForRequest())?.result?.final === true) continue; const assertRun = async () => { const source = await context(link.deliveryId!); - const [current] = await db.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.id, run.id), eq(heartbeatRuns.companyId, input.companyId))); + const [current] = await db.select(responseRunColumns).from(heartbeatRuns).where(and(eq(heartbeatRuns.id, run.id), eq(heartbeatRuns.companyId, input.companyId))); if (!current || current.agentId !== source.endpoint.assignedAgentId || !["succeeded", "failed", "interrupted", "timed_out", "cancelled"].includes(current.status) || source.endpoint.id !== input.endpointId || source.conversation.issueId !== input.issueId || diff --git a/server/src/services/github-connection-events.ts b/server/src/services/github-connection-events.ts index 4e345109f32..04022c647a1 100644 --- a/server/src/services/github-connection-events.ts +++ b/server/src/services/github-connection-events.ts @@ -485,11 +485,11 @@ export function githubConnectionEventService( emptyPolls = 0; nextPollAt = now().getTime() + 5_000; const result: GitHubConnectionEventPollResult = { - leased: lease.events.length, + leased: lease.events.length + (lease.failedEventIds?.length ?? 0), processed: 0, duplicate: 0, ignored: 0, - failed: 0, + failed: lease.failedEventIds?.length ?? 0, }; const acknowledge: string[] = []; for (const event of lease.events) { @@ -535,6 +535,10 @@ export function githubConnectionEventService( deliveryIds: acknowledge, }); } + for (const deliveryId of lease.failedEventIds ?? []) { + logger.warn({ deliveryId, reason: "inner_event_authentication_failed" }, + "GitHub connector event retained for retry"); + } return result; }, }; diff --git a/server/src/services/paperclip-cloud-connector.test.ts b/server/src/services/paperclip-cloud-connector.test.ts index 80ce27343e3..29229c84390 100644 --- a/server/src/services/paperclip-cloud-connector.test.ts +++ b/server/src/services/paperclip-cloud-connector.test.ts @@ -735,11 +735,42 @@ describe("dedicated App gateway delivery", () => { expect((await connector.leaseEvents({subject,companyId}))?.events[0].payload.githubApp).toEqual(packet); expect(JSON.stringify(inner)).not.toContain("private comment"); }); - it("rejects an inner envelope swapped from another opaque route", async () => { + it("retains an inner envelope swapped from another opaque route for retry", async () => { const keys = config(); const inner = seal({v:1,instanceId,environment:"staging",registrationId:"route-b"},keys.sealPublicKey,"events",keys.config,"github.bot"); const outer = seal({v:1,instanceId,environment:"staging",leaseId:"lease-a",events:[{id:"transport-a",provider:"github",event:"gateway_delivery",payload:{githubApp:{registrationId:"route-a",sealed:inner}},bindingIds:["github-app:route-a"]}]},keys.sealPublicKey,"events",keys.config,"github.code"); const connector=createPaperclipCloudConnector({config:keys.config,request:vi.fn(async()=>Response.json({leaseId:"lease-a",sealed:outer})) as typeof fetch}); - await expect(connector.leaseEvents({subject,companyId})).rejects.toBeInstanceOf(PaperclipCloudConnectorError); + await expect(connector.leaseEvents({subject,companyId})).resolves.toMatchObject({ + leaseId: "lease-a", events: [], failedEventIds: ["transport-a"], + }); + }); + + it.each(["ciphertext", "lease"])("still rejects an invalid outer %s binding", async failure => { + const keys = config(); + const outer = seal({ v: 1, instanceId, environment: "staging", leaseId: "lease-a", events: [] }, + failure === "ciphertext" ? config().sealPublicKey : keys.sealPublicKey, "events", keys.config, "github.code"); + const connector = createPaperclipCloudConnector({ config: keys.config, + request: vi.fn(async () => Response.json({ leaseId: failure === "lease" ? "other-lease" : "lease-a", sealed: outer })) as typeof fetch }); + await expect(connector.leaseEvents({ subject, companyId })).rejects.toBeInstanceOf(PaperclipCloudConnectorError); + }); + + it("isolates a stale-key event while decrypting healthy siblings and legacy events", async () => { + const keys = config(); + const staleKeys = config(); + const packet = { v: 1, instanceId, environment: "staging", registrationId: "route-a", rawBody: "e30=" }; + const event = (id: string, inner: unknown) => ({ id, provider: "github", event: "gateway_delivery", + payload: { githubApp: { registrationId: "route-a", sealed: inner } }, bindingIds: ["github-app:route-a"] }); + const outer = seal({ v: 1, instanceId, environment: "staging", leaseId: "lease-a", events: [ + event("stale", seal(packet, staleKeys.sealPublicKey, "events", keys.config, "github.bot")), + event("healthy", seal(packet, keys.sealPublicKey, "events", keys.config, "github.bot")), + { id: "legacy", provider: "github", event: "installation", payload: {}, bindingIds: [] }, + ] }, keys.sealPublicKey, "events", keys.config, "github.code"); + const connector = createPaperclipCloudConnector({ config: keys.config, + request: vi.fn(async () => Response.json({ leaseId: "lease-a", sealed: outer })) as typeof fetch }); + const result = await connector.leaseEvents({ subject, companyId }); + expect(result?.failedEventIds).toEqual(["stale"]); + expect(result?.events.map(event => event.id)).toEqual(["healthy", "legacy"]); + expect(result?.events[0]?.payload.githubApp).toEqual(packet); + expect(JSON.stringify(result)).not.toContain('"ct"'); }); }); diff --git a/server/src/services/paperclip-cloud-connector.ts b/server/src/services/paperclip-cloud-connector.ts index 774dd1f3bc8..b84c9c709ae 100644 --- a/server/src/services/paperclip-cloud-connector.ts +++ b/server/src/services/paperclip-cloud-connector.ts @@ -520,7 +520,7 @@ export function createPaperclipCloudConnector(input: { }); await call("webhook-bind", { ...values, profile: "github.code" }, { field: "binding", value: binding }); }, - async leaseEvents(values: { subject: string; companyId: string }): Promise<{ leaseId: string; events: SealedConnectorEvents["events"] } | null> { + async leaseEvents(values: { subject: string; companyId: string }): Promise<{ leaseId: string; events: SealedConnectorEvents["events"]; failedEventIds?: string[] } | null> { const response = await call("event-lease", values); if (Array.isArray(response.events) && response.events.length === 0) return null; if (typeof response.leaseId !== "string") { @@ -531,16 +531,24 @@ export function createPaperclipCloudConnector(input: { if (opened.leaseId !== response.leaseId) { throw new PaperclipCloudConnectorError("Paperclip Cloud connector event lease did not match", "CONNECTOR_BINDING_MISMATCH"); } + const events: SealedConnectorEvents["events"] = []; + const failedEventIds: string[] = []; for (const event of opened.events) { const packet = event.payload.githubApp; - if (!isRecord(packet) || packet.sealed === undefined) continue; - const sealed = parseEnvelope(packet.sealed, "events", "github", "github.bot"); - const delivery = decryptEnvelope(sealed, sealKey, config.instanceId, config.environment, "github", "github.bot", []) as Record; - if (delivery.v !== 1 || delivery.instanceId !== config.instanceId || delivery.environment !== config.environment - || delivery.registrationId !== packet.registrationId) throw badEnvelope(); - event.payload = { githubApp: delivery }; + try { + if (isRecord(packet) && packet.sealed !== undefined) { + const sealed = parseEnvelope(packet.sealed, "events", "github", "github.bot"); + const delivery = decryptEnvelope(sealed, sealKey, config.instanceId, config.environment, "github", "github.bot", []) as Record; + if (!isRecord(delivery) || delivery.v !== 1 || delivery.instanceId !== config.instanceId || delivery.environment !== config.environment + || delivery.registrationId !== packet.registrationId) throw badEnvelope(); + event.payload = { githubApp: delivery }; + } + events.push(event); + } catch { + failedEventIds.push(event.id); + } } - return { leaseId: opened.leaseId, events: opened.events }; + return { leaseId: opened.leaseId, events, ...(failedEventIds.length ? { failedEventIds } : {}) }; }, async acknowledgeEvents(values: { subject: string; companyId: string; leaseId: string; deliveryIds: string[] }): Promise { const acknowledgement = JSON.stringify({ leaseId: values.leaseId, deliveryIds: values.deliveryIds });