mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 00:05:49 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
433d94ee0b
commit
7eea9d8214
7 files changed
+61
-2
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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<script>", botUsername: null, agentName: "Fallback", score: 4 }))
|
||||
.toBe("## \\[Bot\\](https://example.test) \\<script\\> — 4/5");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,12 @@
|
||||
/** GitHub owns the bot identity; render its display name as plain heading text. */
|
||||
export function githubReviewHeading(input: {
|
||||
botDisplayName: string | null;
|
||||
botUsername: string | null;
|
||||
agentName: string;
|
||||
score: number | null;
|
||||
}) {
|
||||
const name = (input.botDisplayName?.trim() || input.botUsername?.replace(/\[bot\]$/, "").trim() || input.agentName)
|
||||
.replace(/[\r\n]+/g, " ")
|
||||
.replace(/[\\`*_[\]<>]/g, "\\$&");
|
||||
return `## ${name} — ${input.score === null ? "Incomplete" : `${input.score}/5`}`;
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
import { runtimePublicOrigin } from "./cloud-runtime-identity.js";
|
||||
import { projectSafeChatPublicationText } from "./chat-publication-projection.js";
|
||||
import { githubReviewCheckService } from "./chat-github-checks.js";
|
||||
import { githubReviewHeading } from "./chat-channels/github/review-heading.js";
|
||||
import { createHash } from "node:crypto";
|
||||
import { githubResponseCommentService, writeGitHubResponseComment } from "./chat-github-response-comments.js";
|
||||
import { and, desc, eq, gt, isNull, or, sql } from "drizzle-orm";
|
||||
@@ -1159,7 +1160,12 @@ export function githubChatReviewService(db: Db, fetchImpl = fetch) {
|
||||
? ` · [Review history](${board}/apps/chat/${source.endpoint.id}/reviews)`
|
||||
: "";
|
||||
const summary = projectSafeChatPublicationText(
|
||||
`## Paperclip Review — ${assessment.complete ? `${assessment.score}/5` : "Incomplete"}\n\n${assessment.summary}\n\n${assessment.rationale}\n\nReviewed commit: \`${review.headSha}\`\n\nCoverage: ${assessment.coverage.reviewedPaths.length} files.\n${assessment.coverage.limitations.join("\n")}\n\nTask: ${taskLink} · ${runLink}${historyLink}`,
|
||||
`${githubReviewHeading({
|
||||
botDisplayName: source.endpoint.botDisplayName,
|
||||
botUsername: source.endpoint.botUsername,
|
||||
agentName: source.agent.name,
|
||||
score: assessment.complete ? assessment.score : null,
|
||||
})}\n\n${assessment.summary}\n\n${assessment.rationale}\n\nReviewed commit: \`${review.headSha}\`\n\nCoverage: ${assessment.coverage.reviewedPaths.length} files.\n${assessment.coverage.limitations.join("\n")}\n\nTask: ${taskLink} · ${runLink}${historyLink}`,
|
||||
);
|
||||
const summaryMarker = marker(
|
||||
"review",
|
||||
|
||||
Reference in new issue
Block a user