From 7eea9d8214fec00dfde32bda7fcf9b20a428e330 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:33:05 -0500 Subject: [PATCH] fix(github): use bot names in review summaries (#16212) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A GitHub bot can send a pull request to an agent and publish its review. > - Each bot has its own GitHub App identity, but every summary currently starts with "Paperclip Review". > - A generic heading makes it harder to identify reviewers when several bots review the same pull request. > - This pull request uses the bot display name in review summary headings. > - Users can identify each reviewer while required checks keep their existing name. ## Linked Issues or Issue Description **What existing behavior does this improve?** The heading of a GitHub bot's published pull request review summary. **Current behavior** Every summary uses "Paperclip Review", regardless of the bot's name. The check also uses "Paperclip Review" and must keep that name for existing branch rules. **Proposed behavior** Complete and incomplete review summary headings use the GitHub bot display name. The heading falls back to the bot login, then the assigned agent name, when old connections have no display name. Separate GitHub Apps keep independent checks with the same name. **Reason and benefit** Readers can identify each bot's review. A regression test confirms that publishing a review does not update a check owned by another App or endpoint. **Breaking changes** The summary heading changes. Consumers that parse the old heading must accept bot names. No API, prompt configuration, database schema, or check name changes are included. No duplicate PR was found in the GitHub bot title search. ROADMAP.md does not contain a duplicate of this change. ## What Changed - Use the bot display name in complete and incomplete review headings. - Escape Markdown in names and fall back to the bot login or agent name. - Verify the heading through the publication workflow and keep check updates scoped to the App and endpoint. - Document bot-named summaries and independent checks. Update both access maps. ## Verification - Focused policy, heading, and existing UI tests: 67 passed. - Database-backed regression cases: 2 passed, including bot-name publication and independent checks. - Map checks, token gates, and git diff --check passed. - Typecheck and build passed on the final rebased branch. The final focused run passed 44 tests; the two database-backed regression cases also passed in a separate run. All final-head CI checks passed, including full test suites, build, typecheck, E2E, and security scans. Greptile reviewed the final commit at 5/5 with no actionable findings. No review threads remain unresolved. ## Risks - Consumers that parse the old summary heading must accept bot names. - The GitHub check remains Paperclip Review. Multiple-bot check isolation was tested with provider fixtures; a live two-bot GitHub test was not run for this change. ## Model Used OpenAI GPT-6 through Codex. The exact backend model ID and context window are not exposed in this session. Used reasoning, repository tools, and code execution. ## 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 --------- Co-authored-by: Paperclip --- doc/connections/GITHUB.md | 6 ++++++ maps/agent-access-map/chat-channels.md | 3 +++ maps/feature-map/chat-channels.md | 4 ++++ .../chat-channels.integration.test.ts | 11 ++++++++++- .../github/review-heading.test.ts | 19 +++++++++++++++++++ .../chat-channels/github/review-heading.ts | 12 ++++++++++++ server/src/services/chat-github-reviews.ts | 8 +++++++- 7 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 server/src/services/chat-channels/github/review-heading.test.ts create mode 100644 server/src/services/chat-channels/github/review-heading.ts 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