From f9a9e66107776224718e9de1de7a4d08472db2f2 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Mon, 14 Sep 2026 20:08:31 -0400 Subject: [PATCH] fix(triage): treat user-account bot comments as innocent Bot detection relied solely on user.type === 'Bot', which GitHub only sets for app accounts. Classic bots that run on plain user accounts (e.g. withastro's astrobot-houston) are typed 'User', so their comments slipped past the router and the FSM treated them as human activity. On issue withastro/astro#17991 that made the factory retriage the issue and swap the maintainer-set 'triage: needs reproduction' label away (needs reproduction -> needs triage -> in progress -> unable to reproduce). - Add isBotAuthor() (src/github/bots.ts): flags '[bot]'-suffixed app logins and known user-account bots (astrobot-houston). - Router: drop issue_comment deliveries from bot authors before dispatch. - Triage workflow: skip 'comment' deliveries whose commentAuthor is a bot, so already-queued deliveries from before this fix cannot drop a label either (defense in depth). - fetchIssueDetails: mark known user-account bots as authorIsBot so the conversation the retriage judge and fix verifier read no longer treats them as human input. --- src/github/bots.ts | 23 +++++++++++++++++++++++ src/github/issues.ts | 4 +++- src/router.ts | 12 +++++++++--- src/triage/workflow.ts | 29 ++++++++++++++++++----------- tests/bots.test.ts | 26 ++++++++++++++++++++++++++ tests/router.test.ts | 36 ++++++++++++++++++++++++++++++++++++ 6 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 src/github/bots.ts create mode 100644 tests/bots.test.ts diff --git a/src/github/bots.ts b/src/github/bots.ts new file mode 100644 index 0000000..76d0022 --- /dev/null +++ b/src/github/bots.ts @@ -0,0 +1,23 @@ +/** + * Bot-account detection for webhook payloads. + * + * GitHub reports GitHub App accounts with `user.type === 'Bot'`, and that is + * the only signal the router needs for them. Classic bots that run on plain + * user accounts — for example withastro's astrobot-houston comment bot — are + * typed `User` and are otherwise indistinguishable from humans in a webhook, + * so they need an explicit list. + * + * The `[bot]` login suffix is GitHub's own convention for app accounts; it is + * checked here as a belt-and-braces fallback for payloads that omit the type + * field, and it costs nothing because GitHub reserves the suffix for apps. + */ + +export const KNOWN_USER_TYPE_BOT_LOGINS: ReadonlySet = new Set([ + 'astrobot-houston', +]); + +/** True when the author is a bot: a `[bot]` app account or a known bot login. */ +export function isBotAuthor(login: string | undefined): boolean { + if (!login) return false; + return login.endsWith('[bot]') || KNOWN_USER_TYPE_BOT_LOGINS.has(login); +} diff --git a/src/github/issues.ts b/src/github/issues.ts index a5ab867..534a987 100644 --- a/src/github/issues.ts +++ b/src/github/issues.ts @@ -6,6 +6,7 @@ import * as v from 'valibot'; import type { LabelAppearance } from '../triage/labels.ts'; +import { isBotAuthor } from './bots.ts'; import type { InstallationClient } from './client.ts'; import { isGitHubStatus } from './content.ts'; @@ -73,7 +74,8 @@ export async function fetchIssueDetails( createdAt: issue.data.created_at, comments: comments.map((comment) => ({ author: { login: comment.user?.login ?? '' }, - authorIsBot: comment.user?.type === 'Bot', + authorIsBot: + comment.user?.type === 'Bot' || isBotAuthor(comment.user?.login), authorAssociation: comment.author_association, body: comment.body ?? '', createdAt: comment.created_at, diff --git a/src/router.ts b/src/router.ts index 59a82e7..e1afc05 100644 --- a/src/router.ts +++ b/src/router.ts @@ -12,9 +12,14 @@ * - `issue_comment.created` → triage, unless the comment is on a * pull request or written by a bot * (bot filtering prevents self-trigger - * loops) + * loops). Bot filtering covers GitHub + * App accounts (`user.type === 'Bot'`) + * and known user-account bots such as + * astrobot-houston, so another bot's + * comment can never start a triage. */ +import { isBotAuthor } from './github/bots.ts'; import { RELEASE_SECURITY_CHECK_NAMES } from './release-security/checks.ts'; import { RELEASE_BRANCH_PREFIX, @@ -184,10 +189,11 @@ export function routeDelivery( reason: 'The comment is on a pull request, not an issue.', }; } - if (payload.comment?.user?.type === 'Bot') { + const commentAuthor = payload.comment?.user?.login; + if (payload.comment?.user?.type === 'Bot' || isBotAuthor(commentAuthor)) { return { kind: 'none', - reason: `Comment from bot (${payload.comment.user.login ?? 'unknown'}).`, + reason: `Comment from bot (${commentAuthor ?? 'unknown'}).`, }; } return { diff --git a/src/triage/workflow.ts b/src/triage/workflow.ts index 73cfafd..37ee0df 100644 --- a/src/triage/workflow.ts +++ b/src/triage/workflow.ts @@ -11,6 +11,7 @@ import { type TriageConfig, } from '../config.ts'; import type { WorkerEnv } from '../env.ts'; +import { isBotAuthor } from '../github/bots.ts'; import { createInstallationClient, createScopedInstallationToken, @@ -1538,17 +1539,23 @@ async function loadAndRoute( params.repo, params.issueNumber, ); - const action = route( - { - action: params.issueAction, - // Read from the issue rather than inferred from the action: the - // delivery only says what happened, and the issue may have moved on - // while the delivery waited its turn in the per-issue queue. - issueState: normalizeIssueState(details.state), - issueLabels: details.labels, - }, - config.triage.labels, - ); + const action = + params.issueAction === 'comment' && isBotAuthor(params.commentAuthor) + ? { + type: 'skip' as const, + reason: `Comment from bot (${params.commentAuthor}).`, + } + : route( + { + action: params.issueAction, + // Read from the issue rather than inferred from the action: the + // delivery only says what happened, and the issue may have moved on + // while the delivery waited its turn in the per-issue queue. + issueState: normalizeIssueState(details.state), + issueLabels: details.labels, + }, + config.triage.labels, + ); const conversation = details.comments .slice(-MAX_CONVERSATION_ENTRIES) diff --git a/tests/bots.test.ts b/tests/bots.test.ts new file mode 100644 index 0000000..c0d3724 --- /dev/null +++ b/tests/bots.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import { isBotAuthor, KNOWN_USER_TYPE_BOT_LOGINS } from '../src/github/bots.ts'; + +describe('isBotAuthor', () => { + it('flags GitHub App account logins by their [bot] suffix', () => { + expect(isBotAuthor('factory[bot]')).toBe(true); + expect(isBotAuthor('astro-build[bot]')).toBe(true); + expect(isBotAuthor('github-actions[bot]')).toBe(true); + }); + + it('flags known user-account bots that GitHub types as User', () => { + expect(isBotAuthor('astrobot-houston')).toBe(true); + expect(KNOWN_USER_TYPE_BOT_LOGINS.has('astrobot-houston')).toBe(true); + }); + + it('does not flag human logins', () => { + expect(isBotAuthor('matthewp')).toBe(false); + expect(isBotAuthor('ilovesusu')).toBe(false); + expect(isBotAuthor('someone[bot]ish')).toBe(false); + }); + + it('is safe for missing logins', () => { + expect(isBotAuthor(undefined)).toBe(false); + expect(isBotAuthor('')).toBe(false); + }); +}); diff --git a/tests/router.test.ts b/tests/router.test.ts index 87cd56b..3e3d120 100644 --- a/tests/router.test.ts +++ b/tests/router.test.ts @@ -229,6 +229,42 @@ describe('webhook dispatch router', () => { }); }); + it('ignores comments from user-account bots like astrobot-houston', () => { + // GitHub reports classic bots that run on user accounts as `type: + // User`, so the type check alone misses them (issue #17991: Houston's + // comment triggered a retriage that dropped "needs reproduction"). + const dispatch = routeDelivery( + 'issue_comment', + { + action: 'created', + installation, + repository, + issue: { number: 42 }, + comment: { user: { login: 'astrobot-houston', type: 'User' } }, + }, + 'delivery-5b', + ); + expect(dispatch).toEqual({ + kind: 'none', + reason: 'Comment from bot (astrobot-houston).', + }); + }); + + it('ignores [bot]-suffixed accounts even when the type field is missing', () => { + const dispatch = routeDelivery( + 'issue_comment', + { + action: 'created', + installation, + repository, + issue: { number: 42 }, + comment: { user: { login: 'some-app[bot]' } }, + }, + 'delivery-5c', + ); + expect(dispatch.kind).toBe('none'); + }); + it('routes private repositories to triage with the private flag set', () => { const dispatch = routeDelivery( 'issues',