diff --git a/doc/connections/GITHUB.md b/doc/connections/GITHUB.md index e4a3724a34..e50df130dd 100644 --- a/doc/connections/GITHUB.md +++ b/doc/connections/GITHUB.md @@ -52,6 +52,12 @@ contains those optional agent permissions. Scheduling filters, ignored files, and inline-comment options have separate disclosures. The header shows the App identity, copyable mention, avatar download, and GitHub branding settings. +Published review summaries use the GitHub bot's display name in their heading. +The check name remains **Paperclip Review**. Multiple bots can review the same +PR through separate GitHub Apps: each keeps its own task, summary, findings, and +check. Check updates match both the App ID and the endpoint/PR/commit identity. +Repository branch rules decide separately which App's check is required to merge. + ## Self-hosted setup The setup screen states the default access in one line, with **Change** for diff --git a/maps/agent-access-map/chat-channels.md b/maps/agent-access-map/chat-channels.md index cc852db37d..383d9dbed2 100644 --- a/maps/agent-access-map/chat-channels.md +++ b/maps/agent-access-map/chat-channels.md @@ -152,3 +152,6 @@ to reply. Endpoint/destination execution defaults use the same board-only management boundary. Agents select an authorized workspace for their current task through the [workspace APIs](workspaces.md), rather than editing channel configuration. + +GitHub review summary headings use the bot display name. Checks remain +independently scoped by GitHub App and endpoint/PR/commit identity. diff --git a/maps/feature-map/chat-channels.md b/maps/feature-map/chat-channels.md index 00f0918b26..71e9cee82d 100644 --- a/maps/feature-map/chat-channels.md +++ b/maps/feature-map/chat-channels.md @@ -54,6 +54,10 @@ Automated: [channel integration](../../server/src/__tests__/chat-channels.integr Manual: Inspect the bound agent and identity, recent activity, and delivery failure details. Repair a disposable credential, retry a safe message when offered, and verify exactly one external reply. Disable the endpoint and confirm new traffic is not silently processed. +For GitHub, submit a disposable PR and verify the summary heading uses the bot's +display name and the check remains **Paperclip Review**. For two separate bots on one PR, verify +their App-scoped checks and comments remain independent. + Configure optional project and workspace defaults at the connection and destination levels. Start a new conversation and inspect its task workspace. Verify that inheritance, an explicit selection, and clearing a default remain distinct. diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index fdf06753b9..2e8456d3a8 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -4984,6 +4984,10 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { for (const comment of f.responseComments.values()) comments.set(comment.id, comment); let head = "b".repeat(40); let check = { id: 99, status: "completed", external_id: `${f.endpoint.id}:91:${head}`, app: { id: Number(f.endpoint.botExternalId) } }; + // Other bots can publish the same named check on this PR. Neither another + // App nor a different endpoint's review may be selected for our updates. + const peerCheck = { ...check, id: 9901, app: { id: Number(f.endpoint.botExternalId) + 1 } }; + const otherEndpointCheck = { ...check, id: 9902, external_id: `other-bot:91:${head}` }; const pull = () => ({ number: 91, title: "Test PR", @@ -5054,7 +5058,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }, ]); if (url.includes("/check-runs?")) - return Response.json({ check_runs: [check] }); + return Response.json({ check_runs: [peerCheck, otherEndpointCheck, check] }); return Response.json([]); }); const service = githubChatReviewService(db, f.providerFetch); @@ -5142,6 +5146,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { conclusion: "failure", details_url: `https://current-vanity.example/${company.issuePrefix}/issues/${conversation.issueId}`, }); + const postedSummary = mutations.find((m) => typeof m.body.body === "string" && m.body.body.includes("Addition subtracts")); + expect(postedSummary?.body.body).toContain(`## ${f.endpoint.botLabel} — 2/5`); + const publishedCheck = mutations.find((m) => m.body.conclusion === "failure"); + expect(publishedCheck?.url).toBe(`https://api.github.com/repos/paperclipai/paperclip/check-runs/${check.id}`); + expect(mutations.some((m) => /\/check-runs\/990[12]$/.test(m.url))).toBe(false); const publishedCount = mutations.length; const retried = await service.execute( session, diff --git a/server/src/services/chat-channels/github/review-heading.test.ts b/server/src/services/chat-channels/github/review-heading.test.ts new file mode 100644 index 0000000000..1084ab7b97 --- /dev/null +++ b/server/src/services/chat-channels/github/review-heading.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { githubReviewHeading } from "./review-heading.js"; + +describe("GitHub review heading", () => { + it("uses the GitHub bot display name independently of the assigned agent", () => { + expect(githubReviewHeading({ botDisplayName: "Ponytail Bottybois Review", botUsername: "ponytail-bottybois-review[bot]", agentName: "Ponytail", score: 3 })) + .toBe("## Ponytail Bottybois Review — 3/5"); + }); + it("falls back to the bot login, then the agent name, for older connections", () => { + expect(githubReviewHeading({ botDisplayName: null, botUsername: "ponytail[bot]", agentName: "Ponytail", score: null })) + .toBe("## ponytail — Incomplete"); + expect(githubReviewHeading({ botDisplayName: " ", botUsername: null, agentName: "Ponytail", score: 5 })) + .toBe("## Ponytail — 5/5"); + }); + it("keeps provider display names from injecting Markdown or additional lines", () => { + expect(githubReviewHeading({ botDisplayName: "[Bot](https://example.test)\n