diff --git a/doc/plans/2026-10-07-github-app-live-test-drive.md b/doc/plans/2026-10-07-github-app-live-test-drive.md index 019355a60cc..439a398bca9 100644 --- a/doc/plans/2026-10-07-github-app-live-test-drive.md +++ b/doc/plans/2026-10-07-github-app-live-test-drive.md @@ -407,3 +407,65 @@ No new connection or App was created during this correction. The delayed-ping waiting and same-draft recovery fixes remain in place. All 30 focused setup and management UI tests, UI typecheck/build, Storybook build and token gates pass. The earlier full-suite and current-head CI/review limits still apply. + +### Explicit mentions in descriptions and comments (October 8) + +The user's issue #5 contained `@banana-bot-man` in its description. Its signed +`issues` delivery reached the stack at 11:24:21 UTC and was acknowledged, but +the handler used automatic issue policy without looking for an explicit +mention. Mentions-only correctly disabled automatic work, so no task existed. +The repository and configuring user's verified identity were already allowed. + +Explicit mentions now enter the ordinary manual-message admission path for +issue and PR descriptions, conversation comments, inline review comments, and +submitted review summaries. Body edits trigger only when they add a mention; +an existing mention does not replay on pushes, title edits, reopening, or an +unrelated body edit. The authenticated editor is the initiating principal. +Original authors cannot lend their access to an unauthorized editor. The +existing repository, identity, membership, sponsor, runtime and low-trust gates +remain authoritative, including current-state checks before task mutation and +publication. Description requests do not automatically create review checks: +the agent must use its normal review tools when a review is requested. + +Created comments preserve the native adapter's message and thread identities; +edited requests have distinct delivery-deduplicated identities. Comment edits +also retain their original lifecycle update. An explicit dependency processes +the new mention before an orphan edit receipt can wait for the old unmentioned +comment. Source URLs stay correct through both live and recovered admission. +Automatic policy skips appear in Activity without retaining provider content. + +New App manifests subscribe to `pull_request_review` as well as the existing +four events. It uses the existing Pull requests permission, with no new +permission scope. Older Apps remain compatible; review-summary mentions need +that event enabled in their App settings. The gateway still transports the +original signed bytes, and the stack owns parsing and admission. + +Live qualification used the existing Banana Bot Man App, company, agent, +installation, vault and single smoke repository, with revision 1 unchanged: +mentions-only, automatic events empty, and automatic issue runs off. + +| Real GitHub source | Paperclip outcome | Provider outcome | +| --- | --- | --- | +| Issue #6 description | GIT-7 completed | One animal response from `banana-bot-man[bot]` | +| PR #7 description | GIT-8 completed; current head `7f456f37881bf4bfe39692f897d60e1625521298` | 5/5 review with animal; completed successful Paperclip Review check | +| Issue #8 without mention | No task | Activity explains mentions-only skip | +| Issue #8 comment edited to add mention | GIT-9 completed | Animal response from the same App | +| PR #7 inline comment | GIT-10 completed in the exact inline thread | Animal reply under that comment | + +The original issue #5 was left intact. Review-summary delivery is covered by +signed integration tests; the existing App's subscription was not changed for +live qualification. All 51 GitHub workflow integration tests, 125 parsing, +policy and webhook configuration tests, 33 catalog tests, and seven native +GitHub adapter/provider regressions pass. The final five signed-source cases +also pass with mixed-case repository names and repository-specific guidance. +Workspace typecheck and build pass; server typecheck and build were repeated +after the final routing correction. + +A fresh full `pnpm test:run` reported one failure in the unchanged +`workspace-runtime.test.ts` bounded-conflict case and was interrupted after +that failure while other server suites were still running. That exact test +passes in isolation. There is no verified baseline or green full-suite claim. +Logs: `/private/tmp/github-mentions-repo-tests.log` and +`/private/tmp/github-mentions-workspace-runtime-failure.log`. + +Live review evidence: `/private/tmp/paperclip-github-e2e-evidence/github-description-mention-review.png`. diff --git a/packages/shared/src/app-definitions/github-code-review-bot.json b/packages/shared/src/app-definitions/github-code-review-bot.json index a75572b72f1..c5897ebcbbb 100644 --- a/packages/shared/src/app-definitions/github-code-review-bot.json +++ b/packages/shared/src/app-definitions/github-code-review-bot.json @@ -42,7 +42,7 @@ "secret": true } ], - "guidanceMd": "Generate the webhook secret in Paperclip, then create one private GitHub App with active SSL-verified webhooks, Issues and Pull requests read/write permission, and the selectable issue_comment and pull_request_review_comment events. GitHub sends installation and installation_repositories automatically. Install the App only on repositories where people may mention the agent.", + "guidanceMd": "Generate the webhook secret in Paperclip, then create one private GitHub App with active SSL-verified webhooks and Issues and Pull requests read/write permission. Subscribe to issue_comment and pull_request_review_comment for comment mentions; add issues and pull_request for description mentions, and pull_request_review for review-summary mentions. The wizard selects all five. GitHub sends installation and installation_repositories automatically. Install the App only on repositories where people may mention the agent.", "consoleLinks": { "register": "https://github.com/settings/apps/new", "docs": "https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/registering-a-github-app" diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 7ab2dcbca9c..ddce930660f 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -3816,6 +3816,113 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { ), ).rejects.toThrow("no longer authorized"); }); + it.each(["issues", "pull_request", "issue_comment", "pull_request_review_comment", "pull_request_review"])( + "admits signed %s mentions with automation off, deduplicates and retains manual task authority", async (event) => { + const f = await reviewBotFixture(); + const saved = await f.management.configuration(f.endpoint.id, "owner-user"); + await f.management.saveConfiguration(f.endpoint.id, { expectedRevision: saved.revision, configuration: { ...saved.configuration, defaults: { ...saved.configuration.defaults, invocation: "mentions_only", events: [], issueOpened: false }, repositories: { ...saved.configuration.repositories, "97531": { instructions: "Use repository-specific guidance" } } } }, "owner-user"); + const body = `@${f.endpoint.botUsername!.replace(/\[bot\]$/, "")} tell me a joke`; + const target = { number: 83, title: "Say hello", body, user: { id: 42, login: "octocat", type: "User" }, labels: [] }; + const comment = { id: 83001, body, user: target.user, created_at: new Date().toISOString(), updated_at: new Date().toISOString(), in_reply_to_id: 83000 }; + const payload = { + action: event.includes("comment") ? "created" : event === "pull_request_review" ? "submitted" : "opened", + installation: { id: 2468 }, + repository: { id: 97531, full_name: "PaperclipAI/Paperclip", name: "paperclip", owner: { id: 1357, login: "paperclipai" } }, + sender: target.user, issue: target, pull_request: target, comment, review: comment, + }; + const delivery = randomUUID(); + const send = () => f.service.handleWebhook(f.endpoint.publicId, "github", signedGitHubWebhookRequest({ event, delivery, payload, webhookSecret: f.webhookSecret })); + expect((await send()).status).toBeLessThan(300); + await expect.poll(async () => (await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id))).length).toBe(1); + await send(); + const [conversation] = await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id)); + expect(conversation.externalThreadId).toBe(`github:PaperclipAI/Paperclip:${event === "issues" || event === "issue_comment" ? "issue:" : ""}83${event === "pull_request_review_comment" ? ":rc:83000" : ""}`); + expect(conversation.providerUrl).not.toContain("mention-event"); + if (event === "issues") expect(conversation.providerUrl).toBe("https://github.com/paperclipai/paperclip/issues/83"); + const [task] = await db.select().from(issues).where(eq(issues.id, conversation.issueId!)); + expect(task).toMatchObject({ assigneeAgentId: f.assignedAgentId, responsibleUserId: "owner-user" }); + const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, task.id)); + expect(comments).toHaveLength(1); + expect(comments[0].body).toContain("tell me a joke"); + const [request] = await db.select().from(chatDeliveries).where(and(eq(chatDeliveries.endpointId, f.endpoint.id), eq(chatDeliveries.eventKind, "mention"))); + expect(request.normalizedEvent).toHaveProperty("githubManual.event", "mention"); + expect(request.normalizedEvent).toHaveProperty("githubManual.policy.instructions", "Use repository-specific guidance"); + expect(request.normalizedEvent).not.toHaveProperty("githubAutomatic"); + expect(request.normalizedEvent).not.toHaveProperty("githubIssue"); + expect(await db.select().from(chatGitHubReviews).where(eq(chatGitHubReviews.endpointId, f.endpoint.id))).toHaveLength(0); + expect(await db.select().from(chatActions).where(and(eq(chatActions.endpointId, f.endpoint.id), eq(chatActions.kind, "github_review_check")))).toHaveLength(0); + expect((await f.management.configuration(f.endpoint.id, "owner-user")).configuration.defaults.invocation).toBe("mentions_only"); + }, + ); + it("authorizes description edits as the editor, never inheriting the original author's access", async () => { + const f = await reviewBotFixture(); + const body = `@${f.endpoint.botUsername!} help`; + const payload = { + action: "edited", installation: { id: 2468 }, + repository: { id: 97531, full_name: "paperclipai/paperclip", name: "paperclip", owner: { id: 1357, login: "paperclipai" } }, + sender: { id: 77, login: "unlinked-editor", type: "User" }, + issue: { number: 84, title: "Say hello", body, user: { id: 42, login: "octocat", type: "User" }, labels: [] }, + changes: { body: { from: "No mention" } }, + }; + const send = (content = payload) => f.service.handleWebhook(f.endpoint.publicId, "github", signedGitHubWebhookRequest({ event: "issues", delivery: randomUUID(), payload: content, webhookSecret: f.webhookSecret })); + await send(); + expect(await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id))).toHaveLength(0); + expect(f.wakeup).not.toHaveBeenCalled(); + const allowed = { ...payload, sender: { id: 42, login: "octocat", type: "User" }, issue: { ...payload.issue, user: payload.sender } }; + await send(allowed); + await expect.poll(async () => (await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id))).length).toBe(1); + const [conversation] = await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id)); + const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, conversation.issueId!)); + expect(comments).toHaveLength(1); + await send({ ...allowed, changes: { body: { from: body } } }); + expect(await db.select().from(issueComments).where(eq(issueComments.issueId, conversation.issueId!))).toHaveLength(1); + await db.update(chatIdentityLinks).set({ status: "revoked" }).where(eq(chatIdentityLinks.principalId, f.principal.id)); + await send({ ...allowed, issue: { ...allowed.issue, number: 85 } }); + expect(await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id))).toHaveLength(1); + expect(await f.service.listActivity(f.endpoint.id)).toEqual(expect.arrayContaining([expect.objectContaining({ status: "filtered" })])); + }); + it.each(["issue_comment", "pull_request_review_comment"])("admits a newly mentioned %s edit while preserving its lifecycle record", async (event) => { + const f = await reviewBotFixture(); + const target = { number: 83, title: "Say hello", body: "Description", user: { id: 42, login: "octocat", type: "User" } }; + const payload = { + action: "edited", installation: { id: 2468 }, + repository: { id: 97531, full_name: "paperclipai/paperclip", name: "paperclip", owner: { id: 1357, login: "paperclipai" } }, + sender: target.user, issue: target, pull_request: target, + comment: { id: 83001, body: `@${f.endpoint.botUsername!} help`, user: target.user, updated_at: new Date().toISOString() }, + changes: { body: { from: "No mention" } }, + }; + await f.service.handleWebhook(f.endpoint.publicId, "github", signedGitHubWebhookRequest({ event, delivery: randomUUID(), payload, webhookSecret: f.webhookSecret })); + await expect.poll(async () => (await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id))).length).toBe(1); + const deliveries = await db.select().from(chatDeliveries).where(eq(chatDeliveries.endpointId, f.endpoint.id)); + expect(deliveries).toEqual(expect.arrayContaining([ + expect.objectContaining({ eventKind: "message_updated" }), + expect.objectContaining({ eventKind: "mention", normalizedEvent: expect.objectContaining({ githubManual: expect.objectContaining({ event: "mention" }) }) }), + ])); + }); + it("records automatic policy skips without retaining provider content, and refuses mentions in disabled repositories", async () => { + const f = await reviewBotFixture(); + const saved = await f.management.configuration(f.endpoint.id, "owner-user"); + await f.management.saveConfiguration(f.endpoint.id, { expectedRevision: saved.revision, configuration: { ...saved.configuration, defaults: { ...saved.configuration.defaults, invocation: "mentions_only" } } }, "owner-user"); + const payload = { + action: "opened", installation: { id: 2468 }, + repository: { id: 97531, full_name: "paperclipai/paperclip", name: "paperclip", owner: { id: 1357, login: "paperclipai" } }, + sender: { id: 42, login: "octocat" }, + issue: { number: 86, title: "Private title", body: "Private body", user: { id: 42, login: "octocat", type: "User" }, labels: [] }, + }; + const delivery = randomUUID(); + const send = (content = payload, guid = delivery) => f.service.handleWebhook(f.endpoint.publicId, "github", signedGitHubWebhookRequest({ event: "issues", delivery: guid, payload: content, webhookSecret: f.webhookSecret })); + await send(); await send(); + const activity = await f.service.listActivity(f.endpoint.id); + expect(activity.filter(row => row.status === "filtered")).toHaveLength(1); + expect(activity).toEqual(expect.arrayContaining([expect.objectContaining({ detail: expect.stringContaining("runs only when mentioned") })])); + const [ignored] = await db.select().from(chatDeliveries).where(eq(chatDeliveries.endpointId, f.endpoint.id)); + expect(JSON.stringify(ignored.normalizedEvent)).not.toContain("Private"); + expect(ignored.normalizedEvent).toHaveProperty("filtering.contentRetained", false); + await db.update(chatEndpointResources).set({ enabled: false }).where(eq(chatEndpointResources.endpointId, f.endpoint.id)); + await send({ ...payload, issue: { ...payload.issue, body: `@${f.endpoint.botUsername!} help` } }, randomUUID()); + expect(await db.select().from(chatConversations).where(eq(chatConversations.endpointId, f.endpoint.id))).toHaveLength(0); + expect(f.wakeup).not.toHaveBeenCalled(); + }); it("admits opt-in signed issue events without PR checks, deduplicates them and rechecks revoked policy", async () => { const f = await reviewBotFixture(); const payload = { diff --git a/server/src/services/chat-channels.ts b/server/src/services/chat-channels.ts index 2ef62a0fd2d..2d3fe81747d 100644 --- a/server/src/services/chat-channels.ts +++ b/server/src/services/chat-channels.ts @@ -14,7 +14,7 @@ import { buildChatCommunicationGuidance } from "./chat-communication-guidance.js function githubPolicyRecord(value: unknown): Record { return value && typeof value === "object" && !Array.isArray(value) ? value as Record : {}; } import { githubChatManagementService } from "./chat-github-management.js"; import { githubReviewCheckService } from "./chat-github-checks.js"; -import { githubAutomaticReviewEvent, githubAutomaticIssueEvent, githubAutomaticAdmission, githubPreviousAssessment } from "./chat-github-events.js"; +import { githubAutomaticReviewEvent, githubAutomaticIssueEvent, githubAutomaticAdmission, githubPreviousAssessment, githubBodyMentionsBot, githubExplicitMentionEvent } from "./chat-github-events.js"; import { githubReviewPrompt } from "./chat-github-review-policy.js"; import { chatGitHubConfigurations, chatGitHubRegistrations, chatGitHubReviews } from "@paperclipai/db"; import type { GitHubReviewEventContext, GitHubIssueEventContext, GitHubAutomaticEventContext, GitHubReviewPolicy } from "@paperclipai/shared"; @@ -812,6 +812,7 @@ const UNAVOIDABLE_GITHUB_EVENTS = [ const SUPPORTED_GITHUB_WEBHOOK_EVENTS = new Set([ "issues", "pull_request", + "pull_request_review", "ping", ...REQUIRED_GITHUB_EVENTS, ...UNAVOIDABLE_GITHUB_EVENTS, @@ -1098,6 +1099,7 @@ type LiveInboundMessage = { endpoint: EndpointRow; message: Message; receiptReactionSupported: boolean; + providerUrl: string | null; runtimeContext?: InboundRuntimeContext; thread: Thread; trigger: ChatSdkMessageCallbackEvent["trigger"]; @@ -6315,6 +6317,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { ...REQUIRED_GITHUB_EVENTS, ...UNAVOIDABLE_GITHUB_EVENTS, "pull_request", + "pull_request_review", "issues", ]); const excessiveEvents = [...configuredEvents].filter( @@ -14537,7 +14540,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { if (endpoint.provider === "github" && !githubAutomatic && !githubIssue && !githubManual) { const [config] = await db.select().from(chatGitHubConfigurations).where(eq(chatGitHubConfigurations.endpointId, endpoint.id)); if (config) { - const repository = thread.channelId.replace(/^github:/, ""); + const repository = thread.channelId.replace(/^github:/, "").toLowerCase(); const [repo] = await db.select().from(chatEndpointResources).where(and(eq(chatEndpointResources.endpointId, endpoint.id), eq(chatEndpointResources.providerResourceId, repository))); const repositoryId = String(repo?.metadata?.providerRepositoryId ?? ""); githubManual = { policy: { ...config.configuration.defaults, ...config.configuration.repositories[repositoryId] }, revision: config.revision, event: trigger === "mention" || message.isMention ? "mention" : "comment" }; @@ -14632,6 +14635,10 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { (githubAutomatic ? `https://github.com/${githubAutomatic.context.repository}/pull/${githubAutomatic.context.pullNumber}` : githubIssue ? `https://github.com/${githubIssue.context.repository}/issues/${githubIssue.context.issueNumber}` : null) ?? + // Description/review requests and edits have synthetic ledger IDs, not + // native comment IDs. Their verified source URL must not acquire a fake + // issuecomment anchor from that ledger identity. + (endpoint.provider === "github" && githubManual && !receiptReactionSupported ? recoveredProviderUrl : null) ?? chatProviderConversationUrl({ provider: endpoint.provider, providerAccountId: endpoint.providerAccountId, @@ -15277,6 +15284,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { message, trigger, receiptReactionSupported, + providerUrl, runtimeContext, }); if (!deferDrainUntilFollowup) { @@ -16853,8 +16861,12 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { ) return null; const normalized = delivery.normalizedEvent as { - message?: { targetProviderEventId?: unknown }; + message?: { targetProviderEventId?: unknown; admissionDependencyEventId?: unknown }; }; + // An edit can also introduce a new addressed request. Process that durable + // admission before waiting for the old, potentially unadmitted source. + if (typeof normalized.message?.admissionDependencyEventId === "string") + return normalized.message.admissionDependencyEventId; return typeof normalized.message?.targetProviderEventId === "string" ? normalized.message.targetProviderEventId : null; @@ -17458,7 +17470,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { live.message, live.trigger, false, - null, + live.providerUrl, live.runtimeContext, undefined, delivery.id, @@ -17688,6 +17700,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { providerUpdateId?: number | null; raw?: unknown; revision?: string | null; + admissionDependencyEventId?: string; }, runtimeContext?: RuntimeContext, ) { @@ -17751,6 +17764,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { message: { providerMessageId: input.messageId, targetProviderEventId, + ...(input.admissionDependencyEventId ? { admissionDependencyEventId: input.admissionDependencyEventId } : {}), ...(endpoint.provider === "github" && input.isBotMessage === true ? { isBotMessage: true } : {}), @@ -24715,6 +24729,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { eventType === "issue_comment" || eventType === "issues" || eventType === "pull_request" || + eventType === "pull_request_review" || eventType === "pull_request_review_comment" ); } @@ -24728,15 +24743,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { if (!comment || typeof comment !== "object") return false; const body = (comment as { body?: unknown }).body; if (typeof body !== "string") return false; - const identities = new Set([ - botUsername, - botUsername.replace(/\[bot\]$/i, ""), - ]); - return [...identities].some((identity) => { - if (!identity) return false; - const escaped = identity.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - return new RegExp(`(? = { + mentions_only: "This bot runs only when mentioned. No new mention was found in this event.", + event_disabled: "Automatic runs are off for this GitHub event.", + person_not_authorized: "The GitHub author is not authorized to start automatic work.", + identity_revoked: "The GitHub author's identity link was revoked.", + automatic_reviews_disabled_for_person: "Automatic runs are off for this GitHub author.", + guest_automatic_reviews_disabled: "Automatic runs are off for external contributors.", + responsible_user_unavailable: "The member responsible for automatic tasks is unavailable.", + repository_disabled: "Destination is not enabled in Paperclip", + bot_tools_disabled: "This bot's GitHub tools are disabled.", + }; + await db.transaction(async (tx) => { + const current = await runtimeCallbackEndpoint(tx, endpoint.id, runtimeContext, ["verifying", "active"]); + if (!current) return; + const providerEventId = `github:policy_ingress:${createHash("sha256").update(deliveryId).digest("hex")}`; + await tx.insert(chatDeliveries).values({ + companyId: current.companyId, endpointId: current.id, + providerEventId, deduplicationKey: providerEventId, eventKind: "message", state: "filtered", processedAt: new Date(), + redactedError: `GitHub ${event} ignored. ${details[reason ?? ""] ?? "Automatic runs are restricted by this bot's review policy."}`, + normalizedEvent: { providerEventId, kind: "message", filtering: { contentRetained: false, reason: reason ?? "automatic_policy_unavailable" } }, + }).onConflictDoNothing(); + }); + } + async function listActivity(endpointId: string, page?: { limit: number; before?: { createdAt: string; id: string } }) { const limit = page ? page.limit + 1 : 100; // Match the millisecond precision of the public timestamp and use a UUID diff --git a/server/src/services/chat-github-events.test.ts b/server/src/services/chat-github-events.test.ts index ba205b3d76b..f572fcbbb22 100644 --- a/server/src/services/chat-github-events.test.ts +++ b/server/src/services/chat-github-events.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { githubAutomaticIssueEvent } from "./chat-github-events.js"; +import { githubAutomaticIssueEvent, githubBodyMentionsBot, githubExplicitMentionEvent } from "./chat-github-events.js"; function payload() { return { @@ -26,3 +26,52 @@ describe("GitHub issue event parsing", () => { expect(githubAutomaticIssueEvent({ ...input, issue: { ...input.issue, number: 0 } }, "delivery")).toBeNull(); }); }); + +describe("explicit GitHub mentions", () => { + it.each(["@maya help", "@MAYA[bot] help", "Hi, @maya."])("recognizes the bot's exact bare and bot identity: %s", (body) => { + expect(githubBodyMentionsBot(body, "maya[bot]")).toBe(true); + }); + it.each(["@maya-other", "@mayabot", "x@maya", "@someone"])("does not match a different identity: %s", (body) => { + expect(githubBodyMentionsBot(body, "maya[bot]")).toBe(false); + }); + it.each(["issues", "pull_request", "issue_comment", "pull_request_review_comment", "pull_request_review"])("normalizes a %s request into the matching issue or PR thread", (event) => { + const issue = { ...payload().issue, body: "@maya help" }; + const comment = { id: 123, body: "@maya[bot] help", in_reply_to_id: 99 }; + const input = { + ...payload(), action: event === "pull_request_review" ? "submitted" : event.includes("comment") ? "created" : "opened", + issue, pull_request: issue, comment, review: comment, + }; + const mention = githubExplicitMentionEvent(event, input, "delivery", "maya[bot]"); + expect(mention).toMatchObject({ sender: { id: "77", login: "sender" }, edited: false }); + expect(mention!.threadId).toBe(`github:Test/Repo:${event === "issues" || event === "issue_comment" ? "issue:" : ""}2${event === "pull_request_review_comment" ? ":rc:99" : ""}`); + }); + it("puts PR conversation comments in the PR thread, and preserves the SDK comment identity", () => { + const input = { ...payload(), action: "created", issue: { ...payload().issue, pull_request: {} }, comment: { id: 123, body: "@maya help" } }; + expect(githubExplicitMentionEvent("issue_comment", input, "delivery", "maya")).toMatchObject({ threadId: "github:Test/Repo:2", messageId: "123", url: "https://github.com/test/repo/pull/2#issuecomment-123" }); + }); + it.each(["issues", "pull_request", "issue_comment", "pull_request_review_comment", "pull_request_review"])("recognizes a newly added mention in an edited %s body", (event) => { + const target = { ...payload().issue, body: "@maya help" }; + const content = { id: 123, body: "@maya help" }; + const input = { ...payload(), action: "edited", issue: target, pull_request: target, comment: content, review: content, changes: { body: { from: null } } }; + expect(githubExplicitMentionEvent(event, input, "edit", "maya")).toMatchObject({ edited: true, messageId: "mention-event:edit", receiptReactionSupported: false }); + }); + it("admits an edit adding a mention under the editor's identity, but not a persistent mention", () => { + const input = { ...payload(), action: "edited", issue: { ...payload().issue, body: "@maya help" }, changes: { body: { from: "Old description" } } }; + expect(githubExplicitMentionEvent("issues", input, "edit-delivery", "maya")).toMatchObject({ edited: true, messageId: "mention-event:edit-delivery", sender: { id: "77" } }); + expect(githubExplicitMentionEvent("issues", { ...input, changes: { body: { from: "@maya[bot] old request" } } }, "delivery", "maya")).toBeNull(); + expect(githubExplicitMentionEvent("issues", { ...input, changes: { title: { from: "Old title" } } }, "delivery", "maya")).toBeNull(); + expect(githubExplicitMentionEvent("issues", { ...input, changes: undefined }, "delivery", "maya")).toBeNull(); + }); + it.each(["synchronize", "reopened", "ready_for_review", "closed", "deleted"])("does not replay a description mention on %s", (action) => { + const input = { ...payload(), action, pull_request: { ...payload().issue, body: "@maya help" } }; + expect(githubExplicitMentionEvent("pull_request", input, "delivery", "maya")).toBeNull(); + }); + it("ignores bot senders, title-only mentions, malformed payloads and unsafe IDs", () => { + const input = { ...payload(), issue: { ...payload().issue, body: "@maya help" } }; + expect(githubExplicitMentionEvent("issues", { ...input, sender: { ...input.sender, type: "Bot" } }, "delivery", "maya")).toBeNull(); + expect(githubExplicitMentionEvent("issues", { ...input, issue: { ...input.issue, body: null, title: "@maya" } }, "delivery", "maya")).toBeNull(); + expect(githubExplicitMentionEvent("issues", { ...input, repository: { id: 1, full_name: "wrong/repo/extra" } }, "delivery", "maya")).toBeNull(); + expect(githubExplicitMentionEvent("issues", { ...input, sender: { id: Number.MAX_SAFE_INTEGER + 1, login: "sender" } }, "delivery", "maya")).toBeNull(); + expect(githubExplicitMentionEvent("issues", input, "", "maya")).toBeNull(); + }); +}); diff --git a/server/src/services/chat-github-events.ts b/server/src/services/chat-github-events.ts index 6deba72e4a3..41bf9895c14 100644 --- a/server/src/services/chat-github-events.ts +++ b/server/src/services/chat-github-events.ts @@ -31,6 +31,87 @@ const person = z.object({ login: z.string().min(1).max(100), type: z.string().optional(), }); + +export function githubBodyMentionsBot(body: string, username: string | null): boolean { + if (!username) return false; + const name = username.replace(/\[bot\]$/i, ""); + if (!name) return false; + const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`(?; + const isIssue = eventType === "issues" || eventType === "issue_comment"; + const target = descriptionSchema.safeParse(source[isIssue ? "issue" : "pull_request"]); + if (!target.success) return null; + const description = eventType === "issues" || eventType === "pull_request"; + const review = eventType === "pull_request_review"; + if (!description && !review && !["issue_comment", "pull_request_review_comment"].includes(eventType)) return null; + if (eventType === "issues" && target.data.pull_request !== undefined) return null; + const content = description ? target : commentSchema.safeParse(source[review ? "review" : "comment"]); + if (!content.success) return null; + const body = content.data.body ?? ""; + if (!githubBodyMentionsBot(body, botUsername)) return null; + const edited = envelope.action === "edited"; + if (edited) { + // A title/label edit, push, or unrelated edit retaining an old mention + // must not rerun the request. Missing previous content is not evidence + // that the editor added a mention. + if (!envelope.changes?.body || githubBodyMentionsBot(envelope.changes.body.from ?? "", botUsername)) return null; + } else if (envelope.action !== (description ? "opened" : review ? "submitted" : "created")) return null; + if (envelope.sender.type === "Bot") return null; + const repository = envelope.repository.full_name.toLowerCase(); + const number = target.data.number; + const issueThread = isIssue && target.data.pull_request === undefined; + const comment = !description && !review ? commentSchema.parse(source.comment) : null; + const reviewId = review ? commentSchema.parse(source.review).id : null; + // Match the native adapter's repository casing so addressed descriptions + // and ordinary follow-up comments share the same existing conversation. + const threadId = `github:${envelope.repository.full_name}:${issueThread ? "issue:" : ""}${number}${eventType === "pull_request_review_comment" ? `:rc:${comment!.in_reply_to_id ?? comment!.id}` : ""}`; + return { + threadId, + // Created comments retain the SDK's identity so alternate delivery paths + // cannot duplicate a turn. Edits are separate, delivery-deduplicated asks. + messageId: edited || description ? `mention-event:${deliveryId}` : review ? `review:${reviewId}` : comment!.id, + body: body.slice(0, 65536), + sender: envelope.sender, + edited, + receiptReactionSupported: !description && !review && !edited, + url: `https://github.com/${repository}/${issueThread ? "issues" : "pull"}/${number}${description ? "" : review ? `#pullrequestreview-${reviewId}` : eventType === "pull_request_review_comment" ? `#discussion_r${comment!.id}` : `#issuecomment-${comment!.id}`}`, + raw: comment ? { + type: eventType === "pull_request_review_comment" ? "review_comment" : "issue_comment", + comment: source.comment, repository: source.repository, prNumber: number, threadType: issueThread ? "issue" : "pr", + } : {}, + }; +} const payloadSchema = z.object({ action: z.enum(["opened", "synchronize", "reopened", "ready_for_review"]), repository: z.object({ diff --git a/server/src/services/chat-github-registration.ts b/server/src/services/chat-github-registration.ts index 5329c33143d..ae5019e17b4 100644 --- a/server/src/services/chat-github-registration.ts +++ b/server/src/services/chat-github-registration.ts @@ -228,6 +228,7 @@ export function githubChatRegistrationService( "issues", "issue_comment", "pull_request_review_comment", + "pull_request_review", "pull_request", ], }, diff --git a/server/src/services/chat-github-wizard.ts b/server/src/services/chat-github-wizard.ts index e8cd68bbfca..c1d73d32705 100644 --- a/server/src/services/chat-github-wizard.ts +++ b/server/src/services/chat-github-wizard.ts @@ -393,6 +393,7 @@ export function githubChatWizardService( "issues", "issue_comment", "pull_request_review_comment", + "pull_request_review", "pull_request", ], },