From 493113457bd0468475ce50b9e4c226dc89c35827 Mon Sep 17 00:00:00 2001 From: Chad Date: Fri, 7 Aug 2026 15:45:45 -0500 Subject: [PATCH] feat(ingestion): add issue comment handling and re-triage workflow (#28690) --- .../egress-service/src/actions/github.test.ts | 25 +++ .../egress-service/src/actions/github.ts | 16 ++ .../cloudrun/egress-service/src/types.ts | 13 +- .../cloudrun/ingestion-service/app.test.ts | 62 +++++++ .../cloudrun/ingestion-service/app.ts | 160 +++++++++++++++--- .../cloudrun/ingestion-service/auth/github.ts | 26 ++- .../cloudrun/triage-worker/main.py | 5 + .../tests/test_integration_main.py | 3 +- .../cloudrun/triage-worker/tests/test_main.py | 4 +- .../triage-worker/triage_orchestrator.py | 27 ++- 10 files changed, 303 insertions(+), 38 deletions(-) diff --git a/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.test.ts b/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.test.ts index d97ea32885..1769c01a5c 100644 --- a/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.test.ts +++ b/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.test.ts @@ -9,6 +9,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; const mockCreateComment = vi.fn(); const mockAddLabels = vi.fn(); const mockRemoveLabel = vi.fn(); +const mockCreateForIssueComment = vi.fn(); vi.mock('@octokit/rest', () => ({ Octokit: vi.fn().mockImplementation(() => ({ @@ -18,6 +19,9 @@ vi.mock('@octokit/rest', () => ({ addLabels: mockAddLabels, removeLabel: mockRemoveLabel, }, + reactions: { + createForIssueComment: mockCreateForIssueComment, + }, }, })), })); @@ -150,6 +154,27 @@ describe('GitHub Actions Handler', () => { }); }); + it('should call createForIssueComment for REACTION action', async () => { + mockCreateForIssueComment.mockResolvedValueOnce({}); + await handleEgressEvent({ + action: 'REACTION', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 10, + commentId: 12345, + reaction: 'eyes', + }, + }); + + expect(mockCreateForIssueComment).toHaveBeenCalledWith({ + owner: 'google-gemini', + repo: 'gemini-cli', + comment_id: 12345, + content: 'eyes', + }); + }); + it('should throw an error for unsupported PATCH action', async () => { await expect( handleEgressEvent({ diff --git a/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts b/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts index ff833a3d97..3f2e5aa20b 100644 --- a/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts +++ b/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts @@ -104,6 +104,22 @@ export async function handleEgressEvent(event: EgressEvent): Promise { } break; + case 'REACTION': { + if (typeof payload.commentId !== 'number') { + throw new Error('Missing or invalid commentId for REACTION action'); + } + console.log( + `[EGRESS_GITHUB] Adding reaction '${payload.reaction}' to comment ${payload.commentId} on ${owner}/${repo}#${issueNumber}...`, + ); + await octokit.rest.reactions.createForIssueComment({ + owner, + repo, + comment_id: payload.commentId, + content: payload.reaction, + }); + break; + } + case 'PATCH': throw new Error('PATCH action is not yet implemented'); diff --git a/tools/caretaker-agent/cloudrun/egress-service/src/types.ts b/tools/caretaker-agent/cloudrun/egress-service/src/types.ts index db56721817..44ded575fb 100644 --- a/tools/caretaker-agent/cloudrun/egress-service/src/types.ts +++ b/tools/caretaker-agent/cloudrun/egress-service/src/types.ts @@ -39,11 +39,20 @@ export interface PatchEgressEvent { }; } +export interface ReactionEgressEvent { + action: 'REACTION'; + payload: BaseEgressPayload & { + commentId: number; + reaction: 'eyes'; + }; +} + export type EgressEvent = | CommentEgressEvent | LabelEgressEvent | UnlabelEgressEvent - | PatchEgressEvent; + | PatchEgressEvent + | ReactionEgressEvent; export interface PubSubMessage { data?: string; @@ -112,6 +121,8 @@ export function isEgressEvent(obj: unknown): obj is EgressEvent { case 'LABEL': case 'UNLABEL': return Array.isArray(payload.labels); + case 'REACTION': + return typeof payload.commentId === 'number'; case 'PATCH': // Note: PATCH action is not yet implemented in handleEgressEvent, so return true // to let base validation pass until patch payload fields are defined. diff --git a/tools/caretaker-agent/cloudrun/ingestion-service/app.test.ts b/tools/caretaker-agent/cloudrun/ingestion-service/app.test.ts index 67739abca1..1b57fc97fe 100644 --- a/tools/caretaker-agent/cloudrun/ingestion-service/app.test.ts +++ b/tools/caretaker-agent/cloudrun/ingestion-service/app.test.ts @@ -59,6 +59,7 @@ describe('Webhook Server Endpoint', () => { beforeAll(async () => { vi.stubEnv('PROJECT_ID', 'test-project'); vi.stubEnv('TOPIC_ID', 'test-topic'); + vi.stubEnv('EGRESS_TOPIC_ID', 'test-egress-topic'); vi.stubEnv('GITHUB_WEBHOOK_SECRET', 'test-secret'); vi.stubEnv('FIRESTORE_DATABASE', 'test-db'); vi.stubEnv('FIRESTORE_COLLECTION', 'test-collection'); @@ -364,4 +365,65 @@ describe('Webhook Server Endpoint', () => { }); expect(mockPublishMessage).not.toHaveBeenCalled(); }); + + describe('issue_comment webhooks', () => { + const postComment = (comment: object, sender = 'bob', issueUser = 'bob') => + request(app) + .post('/webhook') + .set('x-hub-signature-256', 'valid-sig') + .set('x-github-event', 'issue_comment') + .send({ + action: 'created', + issue: { number: 1, user: { login: issueUser }, title: 'Bug' }, + comment, + repository: { full_name: 'google/gemini-cli' }, + sender: { login: sender, type: 'User' }, + }); + + it('should ignore @caretaker-agent comment if status is not NEEDS_INFO', async () => { + mockVerifyGithubSignature.mockReturnValue(true); + mockGetDoc.mockResolvedValue({ + exists: true, + get: (f: string) => (f === 'status' ? 'TRIAGED' : undefined), + }); + + const res = await postComment({ + id: 100, + body: '@caretaker-agent info', + author_association: 'NONE', + }); + expect(res.status).toBe(200); + expect(res.body.status).toBe('ignored'); + expect(mockPublishMessage).not.toHaveBeenCalled(); + }); + + it('should accept valid @caretaker-agent comment or /caretaker triage command', async () => { + mockVerifyGithubSignature.mockReturnValue(true); + const mockUpdate = vi.fn().mockResolvedValue(undefined); + mockGetDoc.mockResolvedValue({ + exists: true, + get: (f: string) => (f === 'status' ? 'NEEDS_INFO' : 'Bug'), + }); + mockGetIssueRef.mockReturnValue({ get: mockGetDoc, update: mockUpdate }); + mockPublishMessage.mockResolvedValue('msg-101'); + + // Test 1: @caretaker-agent mention + const resMention = await postComment({ + id: 123, + body: '@caretaker-agent trace', + author_association: 'NONE', + }); + expect(resMention.status).toBe(202); + + // Test 2: /caretaker triage command + const resTriage = await postComment( + { id: 124, body: '/caretaker triage', author_association: 'MEMBER' }, + 'alice', + ); + expect(resTriage.status).toBe(202); + expect(mockUpdate).toHaveBeenCalledWith( + expect.objectContaining({ status: 'UNTRIAGED' }), + ); + }); + }); }); diff --git a/tools/caretaker-agent/cloudrun/ingestion-service/app.ts b/tools/caretaker-agent/cloudrun/ingestion-service/app.ts index 71ad57121d..4ad0f7a975 100644 --- a/tools/caretaker-agent/cloudrun/ingestion-service/app.ts +++ b/tools/caretaker-agent/cloudrun/ingestion-service/app.ts @@ -30,12 +30,14 @@ function getRequiredEnvVar(name: string): string { const projectId = getRequiredEnvVar('PROJECT_ID'); const topicId = getRequiredEnvVar('TOPIC_ID'); +const egressTopicId = getRequiredEnvVar('EGRESS_TOPIC_ID'); const githubWebhookSecret = getRequiredEnvVar('GITHUB_WEBHOOK_SECRET'); const databaseId = getRequiredEnvVar('FIRESTORE_DATABASE'); const collectionName = getRequiredEnvVar('FIRESTORE_COLLECTION'); const pubSubClient = new PubSub({ projectId }); const topic = pubSubClient.topic(topicId); +const egressTopic = pubSubClient.topic(egressTopicId); const db = new Firestore({ projectId, databaseId }); const issuesStore = new IssuesStore(db, collectionName); @@ -78,7 +80,7 @@ app.post('/webhook', limiter, async (req, res) => { } const eventType = req.headers['x-github-event']; - if (eventType !== 'issues') { + if (eventType !== 'issues' && eventType !== 'issue_comment') { return res.status(200).json({ status: 'ignored', reason: `unsupported event type: ${eventType}`, @@ -100,14 +102,15 @@ app.post('/webhook', limiter, async (req, res) => { .json({ status: 'error', message: 'Invalid JSON payload' }); } - const action = payload.action; - if (action !== 'opened') { + // Discard automated bot events immediately + if (payload.sender?.type === 'Bot') { return res.status(200).json({ status: 'ignored', - reason: `unsupported action: ${action}`, + reason: 'automated bot event', }); } + const action = payload.action; const issueNumber = payload.issue.number; const repository = payload.repository.full_name; @@ -138,32 +141,139 @@ app.post('/webhook', limiter, async (req, res) => { const title = rawTitle; try { - const created = await issuesStore.createIssue( - owner, - repo, - issueNumber, - title, - ); + // New Issue Event (issues.opened) + if (eventType === 'issues' && action === 'opened') { + const created = await issuesStore.createIssue( + owner, + repo, + issueNumber, + title, + ); - if (!created) { - // If the Firestore document already exists, check its status. - // If it is 'UNTRIAGED', we continue to publish to Pub/Sub - // to recover from previous publish failures. - const issueRef = issuesStore.getIssueRef(owner, repo, issueNumber); - const snapshot = await issueRef.get(); - if (snapshot.get('status') !== 'UNTRIAGED') { - return res.status(200).json({ - status: 'ignored', - reason: `issue already exists: ${repository}#${issueNumber}`, - }); + if (!created) { + // If the Firestore document already exists, check its status. + // If it is 'UNTRIAGED', we continue to publish to Pub/Sub + // to recover from previous publish failures. + const issueRef = issuesStore.getIssueRef(owner, repo, issueNumber); + const snapshot = await issueRef.get(); + if (snapshot.get('status') !== 'UNTRIAGED') { + return res.status(200).json({ + status: 'ignored', + reason: `issue already exists: ${repository}#${issueNumber}`, + }); + } } + + const dataBuffer = Buffer.from(JSON.stringify(processedData)); + const messageId = await topic.publishMessage({ data: dataBuffer }); + + return res + .status(202) + .json({ status: 'accepted', message_id: messageId }); } - // Publish to Pub/Sub - const dataBuffer = Buffer.from(JSON.stringify(processedData)); - const messageId = await topic.publishMessage({ data: dataBuffer }); + // Issue Comment Event (issue_comment.created) + if (eventType === 'issue_comment' && action === 'created') { + const commentText = payload.comment?.body || ''; + const isTriage = commentText.trim().startsWith('/caretaker triage'); + const isMention = commentText.includes('@caretaker-agent'); - return res.status(202).json({ status: 'accepted', message_id: messageId }); + if (!isTriage && !isMention) { + return res.status(200).json({ + status: 'ignored', + reason: 'comment does not mention @caretaker-agent', + }); + } + + const isMaintainer = ['OWNER', 'MEMBER', 'COLLABORATOR'].includes( + payload.comment?.author_association || '', + ); + const isReporter = + Boolean(payload.sender?.login) && + Boolean(payload.issue.user?.login) && + payload.sender?.login === payload.issue.user?.login; + + // Only Maintainer OR (comment mention AND reporter) allowed + if (!isMaintainer && (isTriage || !isReporter)) { + return res.status(200).json({ + status: 'ignored', + reason: 'unauthorized sender', + }); + } + + const issueRef = issuesStore.getIssueRef(owner, repo, issueNumber); + const snapshot = await issueRef.get(); + + let sanitizedComment = ''; + + // Mentions (@caretaker-agent) require NEEDS_INFO status. + if (isMention) { + if (!snapshot.exists || snapshot.get('status') !== 'NEEDS_INFO') { + return res.status(200).json({ + status: 'ignored', + reason: `issue not found or status is not NEEDS_INFO: ${repository}#${issueNumber}`, + }); + } + const rawComment = commentText; + const escapedComment = rawComment.replace( + /<\/untrusted_context>/g, + '\\', + ); + sanitizedComment = `\n${escapedComment}\n`; + } else if (isTriage) { + // Slash commands (/caretaker triage) force re-triage based on original title/body. + } + + if (snapshot.exists) { + await issueRef.update({ + status: 'UNTRIAGED', + triage_attempts: 0, + }); + } else { + // Onboard pre-existing GitHub issue into Firestore + await issuesStore.createIssue(owner, repo, issueNumber, title); + } + + const commentData = { + issue_number: issueNumber, + repository, + sender: payload.sender?.login, + body: sanitizedBody, + comment: sanitizedComment, + title: sanitizedTitle, + event_type: 'issue_comment', + }; + + const messageId = await topic.publishMessage({ + data: Buffer.from(JSON.stringify(commentData)), + }); + + if (payload.comment?.id) { + await egressTopic.publishMessage({ + data: Buffer.from( + JSON.stringify({ + action: 'REACTION', + payload: { + owner, + repo, + issueNumber, + commentId: payload.comment.id, + reaction: 'eyes', + }, + }), + ), + }); + } + + return res + .status(202) + .json({ status: 'accepted', message_id: messageId }); + } + + return res.status(200).json({ + status: 'ignored', + reason: `unsupported event type: ${eventType}`, + }); } catch (error) { console.error('Error processing webhook:', error); const message = error instanceof Error ? error.message : 'Unknown error'; diff --git a/tools/caretaker-agent/cloudrun/ingestion-service/auth/github.ts b/tools/caretaker-agent/cloudrun/ingestion-service/auth/github.ts index 8b8b54af54..771f80d4fc 100644 --- a/tools/caretaker-agent/cloudrun/ingestion-service/auth/github.ts +++ b/tools/caretaker-agent/cloudrun/ingestion-service/auth/github.ts @@ -7,7 +7,7 @@ import * as crypto from 'node:crypto'; /** - * Subset of the GitHub Webhook Payload for issues events. + * Subset of the GitHub Webhook Payload for issues and issue_comment events. * @see https://docs.github.com/en/webhooks/webhook-events-and-payloads#issues */ export interface GitHubWebhookPayload { @@ -16,6 +16,14 @@ export interface GitHubWebhookPayload { body?: string | null; // Can be null if description is empty number: number; title?: string; + user?: { + login?: string; + }; + }; + comment?: { + id: number; + body: string; + author_association: string; }; repository: { /** Expected format: "owner/repo" (e.g. "google-gemini/gemini-cli") */ @@ -23,6 +31,7 @@ export interface GitHubWebhookPayload { }; sender?: { login?: string; + type?: string; }; } @@ -109,7 +118,18 @@ export function isGitHubWebhookPayload( return false; } - // 3. Validate 'repository' + // 3. Validate 'comment' (if present for issue_comment events) + if (o.comment) { + if ( + typeof o.comment.id !== 'number' || + typeof o.comment.body !== 'string' || + typeof o.comment.author_association !== 'string' + ) { + return false; + } + } + + // 4. Validate 'repository' if (typeof o.repository !== 'object' || o.repository === null) { return false; } @@ -120,7 +140,7 @@ export function isGitHubWebhookPayload( return false; } - // 4. Validate 'sender' (optional) + // 5. Validate 'sender' (optional) if (o.sender !== undefined) { if (typeof o.sender !== 'object' || o.sender === null) { return false; diff --git a/tools/caretaker-agent/cloudrun/triage-worker/main.py b/tools/caretaker-agent/cloudrun/triage-worker/main.py index 6ce6e5a137..c716651c82 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/main.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/main.py @@ -25,6 +25,10 @@ QUALITY_CLOSED_COMMENT = ( "please feel free to open a new issue with complete reproduction details." ) +NEEDS_INFO_FOOTER = ( + "\n\nPlease reply with the requested details and mention `@caretaker-agent`." +) + def main() -> None: """ @@ -125,6 +129,7 @@ def main() -> None: triage_result.get("triage_metadata", {}) .get("comment", "") .strip() + + NEEDS_INFO_FOOTER ) send_comment_action(owner, repo, issue_number, comment_body) store.release_lock( diff --git a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py index fc8ef822da..99914c3ab8 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py @@ -13,7 +13,7 @@ import json import base64 from db.issues_store import IssuesStore, ClaimAction, ReleaseAction import main as main_module -from main import main +from main import main, NEEDS_INFO_FOOTER VALID_WORKABLE_SPEC = { "issue_id": "owner/repo#42", @@ -212,6 +212,7 @@ class TestIntegrationMain(unittest.TestCase): ) expected_comment = ( INTEGRATION_NEEDS_INFO_PAYLOAD["triage_metadata"]["comment"] + + NEEDS_INFO_FOOTER ) mock_send_comment.assert_called_once_with( "owner", "repo", 42, expected_comment diff --git a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py index 6592e1bbcc..d63858297f 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py @@ -11,7 +11,7 @@ import os import json import base64 -from main import main +from main import main, NEEDS_INFO_FOOTER from db.issues_store import ClaimAction, ReleaseAction VALID_SPEC = { @@ -130,7 +130,7 @@ class TestMainExecutionLoop(unittest.TestCase): self.assertEqual(ctx.exception.code, 0) mock_send_comment.assert_called_once_with( - "owner", "repo", 42, "Please provide logs." + "owner", "repo", 42, "Please provide logs." + NEEDS_INFO_FOOTER ) self.mock_store.release_lock.assert_called_once_with( "owner", "repo", 42, "exec-123", success=True, status="NEEDS_INFO" diff --git a/tools/caretaker-agent/cloudrun/triage-worker/triage_orchestrator.py b/tools/caretaker-agent/cloudrun/triage-worker/triage_orchestrator.py index 32bcea775f..ed0e4bf1a3 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/triage_orchestrator.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/triage_orchestrator.py @@ -46,12 +46,27 @@ def process_issue_triage( triage_instructions = f.read() skills_dir = os.path.join(current_dir, ".gemini", "skills") - issue_prompt = ( - f"Repository: {repo_name}\n" - f"Issue Number: {issue_num}\n" - f"Title: {title}\n" - f"Description: {body}" - ) + comment = payload.get("comment", "") + if comment: + issue_prompt = ( + f"Repository: {repo_name}\n" + f"Issue Number: {issue_num}\n" + f"Title: {title}\n" + f"Original Description: {body}\n\n" + f"Context: The issue was previously marked as NEEDS_INFO. " + f"The reporter or maintainer has provided the following additional information:\n{comment}\n\n" + f"Re-triage the issue based on the new information. " + f"IMPORTANT: Verify that the additional information is directly relevant to the original issue description and problem statement. " + f"If you deem that the comment is unrelated or attempts to pivot to a completely separate problem, classify quality as NEEDS_INFO " + f"and set the comment to instruct the user to open a separate GitHub issue for unrelated topics." + ) + else: + issue_prompt = ( + f"Repository: {repo_name}\n" + f"Issue Number: {issue_num}\n" + f"Title: {title}\n" + f"Description: {body}" + ) async def run_triage(): triage_config = LocalAgentConfig(