From c2a19e8f31302ea5d4a34c3b4939cb73d0c78445 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Mon, 14 Sep 2026 19:06:47 +0200 Subject: [PATCH] codex-bugbot-human-context: Keep automated reports out of Bugbot human context --- build/api/index.js | 4 +- .../ports/bugbot_issue_read_ports.d.ts | 2 + .../pull_request_review_comment_ports.d.ts | 2 + build/cli/index.js | 10 +++-- build/github_action/index.js | 10 +++-- docs/bugbot/how-it-works.mdx | 2 +- .../ports/bugbot_issue_read_ports.ts | 2 + .../pull_request_review_comment_ports.ts | 2 + .../__tests__/bugbot_review_context.test.ts | 37 ++++++++++++++----- .../commit/bugbot/bugbot_finding_context.ts | 8 +--- .../commit/bugbot/bugbot_review_context.ts | 4 +- ...bot_issue_comment_query_repository.test.ts | 23 +++++++++++- .../bugbot_issue_comment_query_repository.ts | 10 +++-- ...st_review_comment_query_repository.test.ts | 3 ++ ...request_review_comment_query_repository.ts | 1 + .../github_pull_request_review_protocol.ts | 2 +- 16 files changed, 88 insertions(+), 34 deletions(-) diff --git a/build/api/index.js b/build/api/index.js index 51c40ba45..3437415bf 100644 --- a/build/api/index.js +++ b/build/api/index.js @@ -2025,13 +2025,13 @@ function buildReviewConversationBlock(issueComments, commentsByPullRequest, botL function buildReviewConversationContext(issueComments, commentsByPullRequest, botLogin) { const entries = []; for (const comment of issueComments) { - if (isBot(comment.user?.login, botLogin)) + if (comment.isAutomatedAuthor || isBot(comment.user?.login, botLogin)) continue; appendConversationEntry(entries, comment.user?.login, 'general PR/issue comment', comment.body, comment.createdAt, `issue:${comment.id}`); } for (const comments of commentsByPullRequest.values()) { for (const comment of comments) { - if (isBot(comment.authorLogin, botLogin)) + if (comment.isAutomatedAuthor || isBot(comment.authorLogin, botLogin)) continue; const location = comment.path ? `inline review comment at ${comment.path}${comment.line ? `:${comment.line}` : ''}` diff --git a/build/api/src/application/ports/bugbot_issue_read_ports.d.ts b/build/api/src/application/ports/bugbot_issue_read_ports.d.ts index 6a406f719..0479871e7 100644 --- a/build/api/src/application/ports/bugbot_issue_read_ports.d.ts +++ b/build/api/src/application/ports/bugbot_issue_read_ports.d.ts @@ -4,6 +4,8 @@ export interface BugbotIssueComment { user?: { login?: string; }; + /** Provider-authenticated author classification; never inferred from the login. */ + isAutomatedAuthor?: boolean; createdAt?: string; } export interface BugbotIssueReadPort { diff --git a/build/api/src/application/ports/pull_request_review_comment_ports.d.ts b/build/api/src/application/ports/pull_request_review_comment_ports.d.ts index ede6e1e67..5891d92d1 100644 --- a/build/api/src/application/ports/pull_request_review_comment_ports.d.ts +++ b/build/api/src/application/ports/pull_request_review_comment_ports.d.ts @@ -7,6 +7,8 @@ export type PullRequestReviewComment = { path?: string; line?: number; authorLogin?: string; + /** Provider-authenticated author classification; never inferred from the login. */ + isAutomatedAuthor?: boolean; createdAt?: string; /** Opaque identity of the submitted review that owns this comment. */ parentReviewIdentity?: string; diff --git a/build/cli/index.js b/build/cli/index.js index a15873963..e446cb5aa 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -52825,13 +52825,13 @@ function buildReviewConversationBlock(issueComments, commentsByPullRequest, botL function buildReviewConversationContext(issueComments, commentsByPullRequest, botLogin) { const entries = []; for (const comment of issueComments) { - if (isBot(comment.user?.login, botLogin)) + if (comment.isAutomatedAuthor || isBot(comment.user?.login, botLogin)) continue; appendConversationEntry(entries, comment.user?.login, 'general PR/issue comment', comment.body, comment.createdAt, `issue:${comment.id}`); } for (const comments of commentsByPullRequest.values()) { for (const comment of comments) { - if (isBot(comment.authorLogin, botLogin)) + if (comment.isAutomatedAuthor || isBot(comment.authorLogin, botLogin)) continue; const location = comment.path ? `inline review comment at ${comment.path}${comment.line ? `:${comment.line}` : ''}` @@ -67040,13 +67040,13 @@ class BugbotIssueCommentQueryRepository { issueOrPullRequest(number: $issueNumber) { ... on Issue { comments(last: 100, before: $cursor) { - nodes { databaseId body author { login } createdAt } + nodes { databaseId body author { login __typename } createdAt } pageInfo { hasPreviousPage startCursor } } } ... on PullRequest { comments(last: 100, before: $cursor) { - nodes { databaseId body author { login } createdAt } + nodes { databaseId body author { login __typename } createdAt } pageInfo { hasPreviousPage startCursor } } } @@ -67064,6 +67064,7 @@ class BugbotIssueCommentQueryRepository { id: Number(comment.databaseId), body: comment.body ?? null, ...(comment.author?.login ? { user: { login: comment.author.login } } : {}), + ...(comment.author?.__typename === 'Bot' ? { isAutomatedAuthor: true } : {}), ...(comment.createdAt ? { createdAt: comment.createdAt } : {}), }]; }); @@ -69326,6 +69327,7 @@ function toReviewComment(comment) { path: comment.path, line: comment.line ?? undefined, authorLogin: comment.user?.login ?? undefined, + ...(comment.user?.type === 'Bot' ? { isAutomatedAuthor: true } : {}), ...(comment.created_at ? { createdAt: comment.created_at } : {}), ...(comment.pull_request_review_id != null ? { parentReviewIdentity: String(comment.pull_request_review_id) } diff --git a/build/github_action/index.js b/build/github_action/index.js index fc21bf911..4beb21348 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -52894,13 +52894,13 @@ function buildReviewConversationBlock(issueComments, commentsByPullRequest, botL function buildReviewConversationContext(issueComments, commentsByPullRequest, botLogin) { const entries = []; for (const comment of issueComments) { - if (isBot(comment.user?.login, botLogin)) + if (comment.isAutomatedAuthor || isBot(comment.user?.login, botLogin)) continue; appendConversationEntry(entries, comment.user?.login, 'general PR/issue comment', comment.body, comment.createdAt, `issue:${comment.id}`); } for (const comments of commentsByPullRequest.values()) { for (const comment of comments) { - if (isBot(comment.authorLogin, botLogin)) + if (comment.isAutomatedAuthor || isBot(comment.authorLogin, botLogin)) continue; const location = comment.path ? `inline review comment at ${comment.path}${comment.line ? `:${comment.line}` : ''}` @@ -65412,13 +65412,13 @@ class BugbotIssueCommentQueryRepository { issueOrPullRequest(number: $issueNumber) { ... on Issue { comments(last: 100, before: $cursor) { - nodes { databaseId body author { login } createdAt } + nodes { databaseId body author { login __typename } createdAt } pageInfo { hasPreviousPage startCursor } } } ... on PullRequest { comments(last: 100, before: $cursor) { - nodes { databaseId body author { login } createdAt } + nodes { databaseId body author { login __typename } createdAt } pageInfo { hasPreviousPage startCursor } } } @@ -65436,6 +65436,7 @@ class BugbotIssueCommentQueryRepository { id: Number(comment.databaseId), body: comment.body ?? null, ...(comment.author?.login ? { user: { login: comment.author.login } } : {}), + ...(comment.author?.__typename === 'Bot' ? { isAutomatedAuthor: true } : {}), ...(comment.createdAt ? { createdAt: comment.createdAt } : {}), }]; }); @@ -67698,6 +67699,7 @@ function toReviewComment(comment) { path: comment.path, line: comment.line ?? undefined, authorLogin: comment.user?.login ?? undefined, + ...(comment.user?.type === 'Bot' ? { isAutomatedAuthor: true } : {}), ...(comment.created_at ? { createdAt: comment.created_at } : {}), ...(comment.pull_request_review_id != null ? { parentReviewIdentity: String(comment.pull_request_review_id) } diff --git a/docs/bugbot/how-it-works.mdx b/docs/bugbot/how-it-works.mdx index 17ecb66fa..26d0b2773 100644 --- a/docs/bugbot/how-it-works.mdx +++ b/docs/bugbot/how-it-works.mdx @@ -79,7 +79,7 @@ This page describes the **internal flow** of Bugbot: how detection runs, how the Removed resolution keys are rejected; there is no legacy conversion path. If one id receives conflicting lifecycle classifications, it is not resolved. -7. **Re-read and project:** After mutations, Bugbot verifies the PR head, re-reads linked-issue findings, reviews, child comments, native thread facts, and the PR conversation concurrently, and then verifies the head again. If the head changed while those reads were in flight, the whole snapshot is discarded as superseded. Each surface records whether its read was verified, failed, or not applicable, and each non-clean issue and PR destination must be observed independently; one visible or clean destination cannot hide another that is open, unverifiable, or missing. Pure lifecycle, provider-projection, and reconciliation-plan policies derive `open`, `reopened`, `fixed`, `obsolete`, `dismissed`, `verification-required`, or `unknown`. A dedicated presentation use case then updates at most 20 affected historical review blocks per run with bounded concurrency, upserts the oldest trusted **Bugbot status** card, and feeds the Result, lifecycle labels, Job Summary, Check Run, and telemetry. Missing or malformed owned evidence fails closed; it is never presented as clean. User-facing PR, commit, run, review, and finding links come only from authenticated provider adapters. Finding links are accepted only when they belong to the same HTTPS server and repository, including GitHub Enterprise installations. +7. **Re-read and project:** After mutations, Bugbot verifies the PR head, re-reads linked-issue findings, reviews, child comments, native thread facts, and the PR conversation concurrently, and then verifies the head again. If the head changed while those reads were in flight, the whole snapshot is discarded as superseded. Each surface records whether its read was verified, failed, or not applicable, and each non-clean issue and PR destination must be observed independently; one visible or clean destination cannot hide another that is open, unverifiable, or missing. The human-discussion prompt includes only provider-classified human authors; comments and inline reviews authored by GitHub Apps or bot accounts do not consume its item or character budget. Pure lifecycle, provider-projection, and reconciliation-plan policies derive `open`, `reopened`, `fixed`, `obsolete`, `dismissed`, `verification-required`, or `unknown`. A dedicated presentation use case then updates at most 20 affected historical review blocks per run with bounded concurrency, upserts the oldest trusted **Bugbot status** card, and feeds the Result, lifecycle labels, Job Summary, Check Run, and telemetry. Missing or malformed owned evidence fails closed; it is never presented as clean. User-facing PR, commit, run, review, and finding links come only from authenticated provider adapters. Finding links are accepted only when they belong to the same HTTPS server and repository, including GitHub Enterprise installations. Review and Commit workflow templates use distinct repository-and-branch concurrency keys. Each can cancel only a superseded run from the same event owner; a paired `push` and `pull_request:synchronize` therefore cannot cancel one another. Commit retains issue progress and native push work, then Bugbot resolves the branch with a read-only exact-head provider lookup. A validated open same-repository PR makes the push stop before loading review context or invoking the agent, because the PR synchronization event exclusively owns review for that head. This does not depend on PR identity being present in the `push` payload. Branches without an open PR still receive push-time, issue-targeted Bugbot review. Fork PR jobs remain excluded by the workflow's same-repository admission gate. A `pull_request: edited` event uses the PR key but cannot cancel a running PR review; it waits, and redundant pending edits collapse to the newest event. When it later runs, it keeps its native workflow result and Job Summary but does not create a `Copilot / Review` Check because it has no Bugbot telemetry. The Check name is reserved for exactly one structurally valid analysis snapshot for the exact head; duplicate telemetry or a valid snapshot beside malformed telemetry is rejected as ambiguous. Application head guards and idempotent provider writes still protect partial/canceled transitions. Other durable mutation workflows retain their workflow-local queue. diff --git a/src/application/ports/bugbot_issue_read_ports.ts b/src/application/ports/bugbot_issue_read_ports.ts index cfedd4907..4c43e389b 100644 --- a/src/application/ports/bugbot_issue_read_ports.ts +++ b/src/application/ports/bugbot_issue_read_ports.ts @@ -2,6 +2,8 @@ export interface BugbotIssueComment { id: number; body: string | null; user?: { login?: string }; + /** Provider-authenticated author classification; never inferred from the login. */ + isAutomatedAuthor?: boolean; createdAt?: string; } diff --git a/src/application/ports/pull_request_review_comment_ports.ts b/src/application/ports/pull_request_review_comment_ports.ts index 4c1bb9a4b..425f9c0ff 100644 --- a/src/application/ports/pull_request_review_comment_ports.ts +++ b/src/application/ports/pull_request_review_comment_ports.ts @@ -7,6 +7,8 @@ export type PullRequestReviewComment = { path?: string; line?: number; authorLogin?: string; + /** Provider-authenticated author classification; never inferred from the login. */ + isAutomatedAuthor?: boolean; createdAt?: string; /** Opaque identity of the submitted review that owns this comment. */ parentReviewIdentity?: string; diff --git a/src/application/usecases/steps/commit/bugbot/__tests__/bugbot_review_context.test.ts b/src/application/usecases/steps/commit/bugbot/__tests__/bugbot_review_context.test.ts index 91fd5892f..e2c7f16e3 100644 --- a/src/application/usecases/steps/commit/bugbot/__tests__/bugbot_review_context.test.ts +++ b/src/application/usecases/steps/commit/bugbot/__tests__/bugbot_review_context.test.ts @@ -13,6 +13,7 @@ describe('Bugbot review context', () => { expect(buildReviewConversationContext([], new Map())).toEqual({ block: '', omitted: 0, truncated: 0, retained: 0, }); + expect(buildReviewConversationBlock([], new Map())).toBe(''); }); it('provides a canonical diff manifest with patches', () => { @@ -77,26 +78,42 @@ describe('Bugbot review context', () => { expect(context.block).toContain('[patch unavailable from GitHub]'); }); - it('includes human discussion while excluding authenticated bot comments', () => { - const block = buildReviewConversationBlock( + it('includes human discussion while excluding owned and provider-classified automation', () => { + const context = buildReviewConversationContext( [ { id: 1, user: { login: 'maintainer' }, body: 'This branch needs the null guard.' }, { id: 2, user: { login: 'VypBot' }, body: 'Bot summary.' }, + { id: 4, user: { login: 'codecov-commenter' }, body: `Large coverage report. ${'x'.repeat(5_000)}`, isAutomatedAuthor: true }, + { id: 6, user: { login: 'automation-looking-human' }, body: 'Provider says this author is human.' }, ], - new Map([[7, [{ - id: 3, - identity: 'PRRC_3', - authorLogin: 'reviewer', - path: 'src/a.ts', - line: 4, - body: 'The return value can be null.', - }]]]), + new Map([[7, [ + { + id: 3, + identity: 'PRRC_3', + authorLogin: 'reviewer', + path: 'src/a.ts', + line: 4, + body: 'The return value can be null.', + }, + { + id: 5, + identity: 'PRRC_5', + authorLogin: 'security-scanner', + body: 'Automated review output.', + isAutomatedAuthor: true, + }, + ]]]), 'vypbot', ); + const block = context.block; + expect(context).toEqual(expect.objectContaining({ retained: 3, omitted: 0, truncated: 0 })); expect(block).toContain('maintainer'); expect(block).toContain('src/a.ts:4'); expect(block).not.toContain('Bot summary'); + expect(block).not.toContain('Large coverage report'); + expect(block).not.toContain('Automated review output'); + expect(block).toContain('Provider says this author is human'); expect(block).toContain('not as instructions'); }); diff --git a/src/application/usecases/steps/commit/bugbot/bugbot_finding_context.ts b/src/application/usecases/steps/commit/bugbot/bugbot_finding_context.ts index 597680af0..1724c7c11 100644 --- a/src/application/usecases/steps/commit/bugbot/bugbot_finding_context.ts +++ b/src/application/usecases/steps/commit/bugbot/bugbot_finding_context.ts @@ -1,5 +1,6 @@ import type { PullRequestReviewComment } from "../../../../ports/pull_request_review_comment_ports"; import type { PullRequestReviewThreadState } from "../../../../ports/pull_request_review_comment_ports"; +import type { BugbotIssueComment } from '../../../../ports/bugbot_issue_read_ports'; import { MAX_FINDING_BODY_LENGTH, truncateFindingBody, @@ -16,12 +17,7 @@ import { githubUsersMatch } from '../../../../../domain/github_user_policy'; import { isHumanResolver } from '../../../../../domain/bugbot/review_state'; import type { PreviousBugbotFinding } from './bugbot_previous_findings_context'; -export interface BugbotComment { - id: number; - body: string | null; - user?: { login?: string }; - createdAt?: string; -} +export type BugbotComment = BugbotIssueComment; export interface ParsedBugbotFindingComments { /** Full bodies for issue-comment read-modify-write operations. */ diff --git a/src/application/usecases/steps/commit/bugbot/bugbot_review_context.ts b/src/application/usecases/steps/commit/bugbot/bugbot_review_context.ts index edb6e8916..fe345cf7c 100644 --- a/src/application/usecases/steps/commit/bugbot/bugbot_review_context.ts +++ b/src/application/usecases/steps/commit/bugbot/bugbot_review_context.ts @@ -93,7 +93,7 @@ export function buildReviewConversationContext( ): BuiltBugbotPromptContext { const entries: ConversationEntry[] = []; for (const comment of issueComments) { - if (isBot(comment.user?.login, botLogin)) continue; + if (comment.isAutomatedAuthor || isBot(comment.user?.login, botLogin)) continue; appendConversationEntry( entries, comment.user?.login, @@ -105,7 +105,7 @@ export function buildReviewConversationContext( } for (const comments of commentsByPullRequest.values()) { for (const comment of comments) { - if (isBot(comment.authorLogin, botLogin)) continue; + if (comment.isAutomatedAuthor || isBot(comment.authorLogin, botLogin)) continue; const location = comment.path ? `inline review comment at ${comment.path}${comment.line ? `:${comment.line}` : ''}` : 'inline review comment'; diff --git a/src/data/repository/issue/__tests__/bugbot_issue_comment_query_repository.test.ts b/src/data/repository/issue/__tests__/bugbot_issue_comment_query_repository.test.ts index 628ee7d0a..8270d85fc 100644 --- a/src/data/repository/issue/__tests__/bugbot_issue_comment_query_repository.test.ts +++ b/src/data/repository/issue/__tests__/bugbot_issue_comment_query_repository.test.ts @@ -13,7 +13,7 @@ function connection( nodes: Array.from({ length: count }, (_, index) => ({ databaseId: startId + index, body: `comment-${startId + index}`, - author: { login: 'alice' }, + author: { login: 'alice', __typename: 'User' }, createdAt: new Date(Date.UTC(2026, 0, 1, 0, startId + index)).toISOString(), })), pageInfo: { hasPreviousPage, startCursor }, @@ -64,6 +64,7 @@ describe('BugbotIssueCommentQueryRepository', () => { 'owner', 'repo', 7, 'token', ); expect(graphql).toHaveBeenCalledTimes(1); + expect(graphql.mock.calls[0][0]).toContain('author { login __typename }'); expect(result.items[0]).toEqual(expect.objectContaining({ id: 9, body: 'comment-9', @@ -74,6 +75,26 @@ describe('BugbotIssueCommentQueryRepository', () => { expect(result.coverage.providerLimitReached).toBeUndefined(); }); + it('preserves provider-authenticated bot authorship without login heuristics', async () => { + const automated = connection(9, 1, false, null); + automated.repository.issueOrPullRequest.comments.nodes[0].author = { + login: 'coverage-service', + __typename: 'Bot', + }; + const repository = new BugbotIssueCommentQueryRepository({ + getClient: () => ({ graphql: jest.fn().mockResolvedValue(automated) }), + } as never); + + const result = await repository.listBugbotIssueCommentsBounded( + 'owner', 'repo', 7, 'token', + ); + + expect(result.items[0]).toEqual(expect.objectContaining({ + user: { login: 'coverage-service' }, + isAutomatedAuthor: true, + })); + }); + it('accepts an empty connection and normalizes absent optional fields', async () => { const graphql = jest.fn() .mockResolvedValueOnce({ repository: { issueOrPullRequest: {} } }) diff --git a/src/data/repository/issue/bugbot_issue_comment_query_repository.ts b/src/data/repository/issue/bugbot_issue_comment_query_repository.ts index 96791a3c0..34afc1757 100644 --- a/src/data/repository/issue/bugbot_issue_comment_query_repository.ts +++ b/src/data/repository/issue/bugbot_issue_comment_query_repository.ts @@ -7,7 +7,10 @@ import type { GithubGraphqlTransportClient } from '../../../infrastructure/githu interface BugbotIssueCommentNode { readonly databaseId?: number | null; readonly body?: string | null; - readonly author?: { readonly login?: string | null } | null; + readonly author?: { + readonly login?: string | null; + readonly __typename?: string | null; + } | null; readonly createdAt?: string | null; } @@ -51,13 +54,13 @@ export class BugbotIssueCommentQueryRepository { issueOrPullRequest(number: $issueNumber) { ... on Issue { comments(last: 100, before: $cursor) { - nodes { databaseId body author { login } createdAt } + nodes { databaseId body author { login __typename } createdAt } pageInfo { hasPreviousPage startCursor } } } ... on PullRequest { comments(last: 100, before: $cursor) { - nodes { databaseId body author { login } createdAt } + nodes { databaseId body author { login __typename } createdAt } pageInfo { hasPreviousPage startCursor } } } @@ -76,6 +79,7 @@ export class BugbotIssueCommentQueryRepository { id: Number(comment.databaseId), body: comment.body ?? null, ...(comment.author?.login ? { user: { login: comment.author.login } } : {}), + ...(comment.author?.__typename === 'Bot' ? { isAutomatedAuthor: true } : {}), ...(comment.createdAt ? { createdAt: comment.createdAt } : {}), }]; }); diff --git a/src/data/repository/pull_request/__tests__/pull_request_review_comment_query_repository.test.ts b/src/data/repository/pull_request/__tests__/pull_request_review_comment_query_repository.test.ts index a62e6b48e..40d7906a6 100644 --- a/src/data/repository/pull_request/__tests__/pull_request_review_comment_query_repository.test.ts +++ b/src/data/repository/pull_request/__tests__/pull_request_review_comment_query_repository.test.ts @@ -12,6 +12,7 @@ describe("PullRequestReviewCommentQueryRepository", () => { path: "src/first.ts", line: null, node_id: "node-1", + user: { login: 'security-scanner', type: 'Bot' }, pull_request_review_id: 77, html_url: 'https://github.com/org/repo/pull/21#discussion_r1', }, @@ -51,6 +52,8 @@ describe("PullRequestReviewCommentQueryRepository", () => { body: null, path: "src/first.ts", line: undefined, + authorLogin: 'security-scanner', + isAutomatedAuthor: true, parentReviewIdentity: '77', url: 'https://github.com/org/repo/pull/21#discussion_r1', }, diff --git a/src/data/repository/pull_request/pull_request_review_comment_query_repository.ts b/src/data/repository/pull_request/pull_request_review_comment_query_repository.ts index 38d188b89..b41cb0e4d 100644 --- a/src/data/repository/pull_request/pull_request_review_comment_query_repository.ts +++ b/src/data/repository/pull_request/pull_request_review_comment_query_repository.ts @@ -27,6 +27,7 @@ function toReviewComment( path: comment.path, line: comment.line ?? undefined, authorLogin: comment.user?.login ?? undefined, + ...(comment.user?.type === 'Bot' ? { isAutomatedAuthor: true } : {}), ...(comment.created_at ? { createdAt: comment.created_at } : {}), ...(comment.pull_request_review_id != null ? { parentReviewIdentity: String(comment.pull_request_review_id) } diff --git a/src/infrastructure/github/ports/github_pull_request_review_protocol.ts b/src/infrastructure/github/ports/github_pull_request_review_protocol.ts index be5fa103d..107ad2adc 100644 --- a/src/infrastructure/github/ports/github_pull_request_review_protocol.ts +++ b/src/infrastructure/github/ports/github_pull_request_review_protocol.ts @@ -39,7 +39,7 @@ export interface GithubReviewComment { body?: string | null; path?: string; line?: number | null; - user?: { login?: string | null } | null; + user?: { login?: string | null; type?: string | null } | null; pull_request_review_id?: number | null; html_url?: string | null; created_at?: string | null;