From b31b755bbf89303159f03a36fbb899c6f7e57511 Mon Sep 17 00:00:00 2001 From: Chad Date: Tue, 7 Jul 2026 17:40:48 -0700 Subject: [PATCH] feat(caretaker-egress): implement octokit github action handler for egress service (#28303) --- .../egress-service/src/actions/github.test.ts | 165 ++++++++++++++++++ .../egress-service/src/actions/github.ts | 113 ++++++++++++ .../cloudrun/egress-service/src/app.test.ts | 37 +++- .../cloudrun/egress-service/src/app.ts | 25 +-- .../cloudrun/egress-service/src/types.ts | 80 ++++++--- 5 files changed, 380 insertions(+), 40 deletions(-) create mode 100644 tools/caretaker-agent/cloudrun/egress-service/src/actions/github.test.ts create mode 100644 tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts 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 new file mode 100644 index 0000000000..d97ea32885 --- /dev/null +++ b/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.test.ts @@ -0,0 +1,165 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +const mockCreateComment = vi.fn(); +const mockAddLabels = vi.fn(); +const mockRemoveLabel = vi.fn(); + +vi.mock('@octokit/rest', () => ({ + Octokit: vi.fn().mockImplementation(() => ({ + rest: { + issues: { + createComment: mockCreateComment, + addLabels: mockAddLabels, + removeLabel: mockRemoveLabel, + }, + }, + })), +})); + +vi.mock('@octokit/auth-app', () => ({ + createAppAuth: vi.fn(), +})); + +describe('GitHub Actions Handler', () => { + let handleEgressEvent: (typeof import('./github.js'))['handleEgressEvent']; + + beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + vi.stubEnv('GH_APP_ID', '12345'); + vi.stubEnv('GH_PRIVATE_KEY', 'test-key'); + vi.stubEnv('GH_INSTALLATION_ID', '67890'); + vi.stubEnv('ALLOWED_OWNER', 'google-gemini'); + vi.stubEnv('ALLOWED_REPO', 'gemini-cli'); + const mod = await import('./github.js'); + handleEgressEvent = mod.handleEgressEvent; + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it('should throw an error for unauthorized repository target', async () => { + await expect( + handleEgressEvent({ + action: 'COMMENT', + payload: { + owner: 'unauthorized-org', + repo: 'other-repo', + issueNumber: 1, + commentBody: 'hi', + }, + }), + ).rejects.toThrow( + /Unauthorized repository target: unauthorized-org\/other-repo/, + ); + }); + + it('should throw an error if environment variables are missing', async () => { + vi.stubEnv('GH_APP_ID', ''); + await expect( + handleEgressEvent({ + action: 'COMMENT', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 1, + commentBody: 'hi', + }, + }), + ).rejects.toThrow(/Missing required environment variable: GH_APP_ID/); + }); + + it('should throw an error if commentBody is empty or whitespace only', async () => { + await expect( + handleEgressEvent({ + action: 'COMMENT', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 1, + commentBody: ' ', + }, + }), + ).rejects.toThrow(/Missing or empty commentBody/); + }); + + it('should call createComment for COMMENT action', async () => { + mockCreateComment.mockResolvedValueOnce({}); + await handleEgressEvent({ + action: 'COMMENT', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 10, + commentBody: 'Hello world', + }, + }); + + expect(mockCreateComment).toHaveBeenCalledWith({ + owner: 'google-gemini', + repo: 'gemini-cli', + issue_number: 10, + body: 'Hello world', + }); + }); + + it('should call addLabels for LABEL action', async () => { + mockAddLabels.mockResolvedValueOnce({}); + await handleEgressEvent({ + action: 'LABEL', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 10, + labels: ['effort/small'], + }, + }); + + expect(mockAddLabels).toHaveBeenCalledWith({ + owner: 'google-gemini', + repo: 'gemini-cli', + issue_number: 10, + labels: ['effort/small'], + }); + }); + + it('should call removeLabel for UNLABEL action', async () => { + mockRemoveLabel.mockResolvedValueOnce({}); + await handleEgressEvent({ + action: 'UNLABEL', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 10, + labels: ['need-triage'], + }, + }); + + expect(mockRemoveLabel).toHaveBeenCalledWith({ + owner: 'google-gemini', + repo: 'gemini-cli', + issue_number: 10, + name: 'need-triage', + }); + }); + + it('should throw an error for unsupported PATCH action', async () => { + await expect( + handleEgressEvent({ + action: 'PATCH', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 1, + }, + }), + ).rejects.toThrow(/PATCH action is not yet implemented/); + }); +}); diff --git a/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts b/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts new file mode 100644 index 0000000000..ff833a3d97 --- /dev/null +++ b/tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts @@ -0,0 +1,113 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Octokit } from '@octokit/rest'; +import { createAppAuth } from '@octokit/auth-app'; +import type { EgressEvent } from '../types.js'; + +function getRequiredEnvVar(name: string): string { + const value = process.env[name]; + if (!value) { + throw new Error(`Missing required environment variable: ${name}`); + } + return value; +} + +let cachedOctokit: Octokit | null = null; + +function getOctokit(): Octokit { + if (!cachedOctokit) { + const appId = getRequiredEnvVar('GH_APP_ID'); + const privateKey = getRequiredEnvVar('GH_PRIVATE_KEY'); + const installationId = getRequiredEnvVar('GH_INSTALLATION_ID'); + + cachedOctokit = new Octokit({ + authStrategy: createAppAuth, + auth: { + appId: Number(appId), + privateKey: privateKey.replace(/\\n/g, '\n'), + installationId: Number(installationId), + }, + }); + } + return cachedOctokit; +} + +export async function handleEgressEvent(event: EgressEvent): Promise { + const { action, payload } = event; + const { owner, repo, issueNumber } = payload; + + const allowedOwner = getRequiredEnvVar('ALLOWED_OWNER'); + const allowedRepo = getRequiredEnvVar('ALLOWED_REPO'); + + if ( + owner.toLowerCase() !== allowedOwner.toLowerCase() || + repo.toLowerCase() !== allowedRepo.toLowerCase() + ) { + throw new Error(`Unauthorized repository target: ${owner}/${repo}`); + } + + const octokit = getOctokit(); + + switch (action) { + // Note: The Egress Service operates as a stateless execution worker ("Hands"). + // Upstream event filtering (e.g. evaluating newly created issues for NEEDS_INFO + // or verifying bot mention/author criteria) is performed in the Triage Worker + // before publishing action payloads to the egress-actions topic. + case 'COMMENT': + if (!payload.commentBody || payload.commentBody.trim() === '') { + throw new Error('Missing or empty commentBody for COMMENT action'); + } + console.log( + `[EGRESS_GITHUB] Posting comment to ${owner}/${repo}#${issueNumber}...`, + ); + await octokit.rest.issues.createComment({ + owner, + repo, + issue_number: issueNumber, + body: payload.commentBody, + }); + break; + + case 'LABEL': + if (!payload.labels || !Array.isArray(payload.labels)) { + throw new Error('Missing or invalid labels array for LABEL action'); + } + console.log( + `[EGRESS_GITHUB] Adding labels [${payload.labels.join(', ')}] to ${owner}/${repo}#${issueNumber}...`, + ); + await octokit.rest.issues.addLabels({ + owner, + repo, + issue_number: issueNumber, + labels: payload.labels, + }); + break; + + case 'UNLABEL': + if (!payload.labels || !Array.isArray(payload.labels)) { + throw new Error('Missing or invalid labels array for UNLABEL action'); + } + console.log( + `[EGRESS_GITHUB] Removing labels [${payload.labels.join(', ')}] from ${owner}/${repo}#${issueNumber}...`, + ); + for (const name of payload.labels) { + await octokit.rest.issues.removeLabel({ + owner, + repo, + issue_number: issueNumber, + name, + }); + } + break; + + case 'PATCH': + throw new Error('PATCH action is not yet implemented'); + + default: + throw new Error(`Unknown or unsupported egress action: ${action}`); + } +} diff --git a/tools/caretaker-agent/cloudrun/egress-service/src/app.test.ts b/tools/caretaker-agent/cloudrun/egress-service/src/app.test.ts index a680c7b2a1..4e8b3cb856 100644 --- a/tools/caretaker-agent/cloudrun/egress-service/src/app.test.ts +++ b/tools/caretaker-agent/cloudrun/egress-service/src/app.test.ts @@ -6,7 +6,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import request from 'supertest'; + +vi.mock('./actions/github.js', () => ({ + handleEgressEvent: vi.fn(), +})); + import { app } from './app.js'; +import { handleEgressEvent } from './actions/github.js'; /** * Helper function simulating GCP Cloud Pub/Sub HTTP Push message wrapper. @@ -63,7 +69,7 @@ describe('Egress Service App Router', () => { expect(res.text).toContain('Malformed payload'); }); - it('POST / should trigger handleEgressEvent stub and return 200 for valid payloads', async () => { + it('POST / should trigger handleEgressEvent handler and return 200 for valid payloads', async () => { const validEvent = { action: 'COMMENT', payload: { @@ -80,5 +86,34 @@ describe('Egress Service App Router', () => { expect(res.status).toBe(200); expect(res.text).toBe('OK'); + expect(handleEgressEvent).toHaveBeenCalledWith(validEvent); + }); + + it('POST / should return 500 if handleEgressEvent fails', async () => { + const validEvent = { + action: 'LABEL', + payload: { + owner: 'google-gemini', + repo: 'gemini-cli', + issueNumber: 42, + labels: ['bug'], + }, + }; + + vi.mocked(handleEgressEvent).mockRejectedValueOnce( + new Error('GitHub API Error'), + ); + + // Suppress console.error during expected failure test + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const res = await request(app) + .post('/') + .send(createPubSubPushEnvelope(validEvent)); + + expect(res.status).toBe(500); + expect(res.text).toBe('GitHub API Error'); + + consoleSpy.mockRestore(); }); }); diff --git a/tools/caretaker-agent/cloudrun/egress-service/src/app.ts b/tools/caretaker-agent/cloudrun/egress-service/src/app.ts index b7b3a04549..3b33bac6c8 100644 --- a/tools/caretaker-agent/cloudrun/egress-service/src/app.ts +++ b/tools/caretaker-agent/cloudrun/egress-service/src/app.ts @@ -6,26 +6,11 @@ import express from 'express'; import dotenv from 'dotenv'; -import { - isPubSubMessageEnvelope, - isEgressEvent, - type EgressEvent, -} from './types.js'; +import { isPubSubMessageEnvelope, isEgressEvent } from './types.js'; +import { handleEgressEvent } from './actions/github.js'; dotenv.config(); -/** - * Top-down stub handler for Egress events. - * Octokit GitHub REST API integration will be added in a follow-up PR. - * - * @param event - The validated EgressEvent object decoded from Pub/Sub push envelope. - */ -export async function handleEgressEvent(event: EgressEvent): Promise { - console.log( - `[EGRESS_STUB] Received ${event.action} event for ${event.payload.owner}/${event.payload.repo}#${event.payload.issueNumber}`, - ); -} - export const app = express(); app.use(express.json()); @@ -38,7 +23,11 @@ app.get('/', (_req, res) => { }); }); -// Pub/Sub push subscription endpoint +/** + * Pub/Sub push subscription endpoint. + * Note: Authentication is enforced by GCP Cloud Run IAM (`roles/run.invoker`) + * using GCP-managed OIDC bearer tokens on the Pub/Sub push subscription. + */ app.post('/', async (req, res) => { if (!isPubSubMessageEnvelope(req.body)) { return res.status(400).send('Invalid Pub/Sub message envelope'); diff --git a/tools/caretaker-agent/cloudrun/egress-service/src/types.ts b/tools/caretaker-agent/cloudrun/egress-service/src/types.ts index bdac528e42..db56721817 100644 --- a/tools/caretaker-agent/cloudrun/egress-service/src/types.ts +++ b/tools/caretaker-agent/cloudrun/egress-service/src/types.ts @@ -4,23 +4,47 @@ * SPDX-License-Identifier: Apache-2.0 */ -export type EgressAction = 'COMMENT' | 'LABEL' | 'PATCH'; - -export interface EgressEventPayload { +export interface BaseEgressPayload { owner: string; repo: string; issueNumber: number; - commentBody?: string; - labels?: string[]; - patchContent?: string; - branchName?: string; } -export interface EgressEvent { - action: EgressAction; - payload: EgressEventPayload; +export interface CommentEgressEvent { + action: 'COMMENT'; + payload: BaseEgressPayload & { + commentBody: string; + }; } +export interface LabelEgressEvent { + action: 'LABEL'; + payload: BaseEgressPayload & { + labels: string[]; + }; +} + +export interface UnlabelEgressEvent { + action: 'UNLABEL'; + payload: BaseEgressPayload & { + labels: string[]; + }; +} + +export interface PatchEgressEvent { + action: 'PATCH'; + payload: BaseEgressPayload & { + patchContent?: string; + branchName?: string; + }; +} + +export type EgressEvent = + | CommentEgressEvent + | LabelEgressEvent + | UnlabelEgressEvent + | PatchEgressEvent; + export interface PubSubMessage { data?: string; messageId?: string; @@ -63,22 +87,36 @@ export function isPubSubMessageEnvelope( * Type guard for EgressEvent. */ export function isEgressEvent(obj: unknown): obj is EgressEvent { - if (!isObject(obj)) { - return false; - } if ( + !isObject(obj) || typeof obj.action !== 'string' || - !['COMMENT', 'LABEL', 'PATCH'].includes(obj.action) + !isObject(obj.payload) ) { return false; } - if (!isObject(obj.payload)) { + + // Validate base target repository properties required for all actions + const payload = obj.payload; + if ( + typeof payload.owner !== 'string' || + typeof payload.repo !== 'string' || + typeof payload.issueNumber !== 'number' + ) { return false; } - const payload = obj.payload; - return ( - typeof payload.owner === 'string' && - typeof payload.repo === 'string' && - typeof payload.issueNumber === 'number' - ); + + // Validate action-specific payload requirements for discriminated union + switch (obj.action) { + case 'COMMENT': + return typeof payload.commentBody === 'string'; + case 'LABEL': + case 'UNLABEL': + return Array.isArray(payload.labels); + 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. + return true; + default: + return false; + } }