Skip to content

feat(caretaker-egress): implement octokit github action handler for egress service #28303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chadd28 merged 6 commits into from
Jul 8, 2026
Merged

feat(caretaker-egress): implement octokit github action handler for egress service #28303

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3e2e46
feat(caretaker-egress): implement octokit github action handler for e…
chadd28 Jul 6, 2026
dcb2fbc
test(caretaker-egress): add HTTP 500 error handling test case
chadd28 Jul 7, 2026
5178842
fix(caretaker-egress): address code review feedback on action errors,…
chadd28 Jul 7, 2026
18f12bb
style(caretaker-egress): format github.test.ts with prettier
chadd28 Jul 7, 2026
c8bf116
feat(caretaker-egress): add UNLABEL action handler and document upstr…
chadd28 Jul 7, 2026
ae5d7e4
refactor(caretaker-egress): use discriminated union for EgressEvent a…
chadd28 Jul 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions tools/caretaker-agent/cloudrun/egress-service/src/actions/github.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* @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();

vi.mock('@octokit/rest', () => {
return {
Octokit: vi.fn().mockImplementation(() => ({
rest: {
issues: {
createComment: mockCreateComment,
addLabels: mockAddLabels,
},
},
})),
};
});

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');
const mod = await import('./github.js');
handleEgressEvent = mod.handleEgressEvent;
});

afterEach(() => {
vi.unstubAllEnvs();
});

it('should throw an error if environment variables are missing', async () => {
vi.stubEnv('GH_APP_ID', '');
await expect(
handleEgressEvent({
action: 'COMMENT',
payload: { owner: 'o', repo: 'r', 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: 'o', repo: 'r', 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',
repo: 'cli',
issueNumber: 10,
commentBody: 'Hello world',
},
});

expect(mockCreateComment).toHaveBeenCalledWith({
owner: 'google',
repo: 'cli',
issue_number: 10,
body: 'Hello world',
});
});

it('should call addLabels for LABEL action', async () => {
mockAddLabels.mockResolvedValueOnce({});
await handleEgressEvent({
action: 'LABEL',
payload: {
owner: 'google',
repo: 'cli',
issueNumber: 10,
labels: ['effort/small'],
},
});

expect(mockAddLabels).toHaveBeenCalledWith({
owner: 'google',
repo: 'cli',
issue_number: 10,
labels: ['effort/small'],
});
});
});
83 changes: 83 additions & 0 deletions tools/caretaker-agent/cloudrun/egress-service/src/actions/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import { Octokit } from '@octokit/rest';
import { createAppAuth } from '@octokit/auth-app';
import { 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<void> {
const octokit = getOctokit();
const { action, payload } = event;
const { owner, repo, issueNumber } = payload;
Comment thread
chadd28 marked this conversation as resolved.

switch (action) {
case 'COMMENT':
if (!payload.commentBody || payload.commentBody.trim() === '') {

@gundermanc gundermanc Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: I notice that you have action as a discriminated union of strings and then separately have a payload with the union of all of the supported properties.

TypeScript has this nifty feature where you can instead do a discriminated union of the request type. Something like:

type CommentAction = {
  action: 'comment';
  owner: string;
  repo: string;
  issueNumber: number;
  commentBody?: string;
  labels?: string[];
}

type LabelAction = {
  action: 'comment';
  owner: string;
  repo: string;
  issueNumber: number;
  labels?: string[];
}

type EgressAction = CommentAction | LabelAction;

The nice thing about this is that when you check for request.action == 'comment' the compiler automatically narrows the type to just the supported properties. It also makes it easy to add another new type to the union with its own unique properties.

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({

@gundermanc gundermanc Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my prototype, we enforce that the agent can only comment on issues in which it was mentioned or its own issues or PRs, to avoid the possibility of it pinging random contributors.

Do we do that here?

@chadd28 chadd28 Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the design, the Egress Service acts purely as a stateless execution worker (the 'Hands'). Upstream event filtering (e.g. evaluating newly created issues for NEEDS_INFO or checking bot mention/author criteria) is going to be performed in the Triage Worker before dispatches reach the egress-actions topic.

I've also added inline documentation above case 'COMMENT' in github.ts to explicitly document this separation of responsibilities.

Let me know what you think about this or if we should implement filtering in github.ts.

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({

@gundermanc gundermanc Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have or need the ability to remove labels? e.g.: I think untriaged items start with a need-triage label.

@chadd28 chadd28 Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not currently used in my initial implementation, but it's definitely good to have as we build out the full workflow. I've added support for the UNLABEL action in github.ts using octokit.rest.issues.removeLabel(...) .

owner,
repo,
issue_number: issueNumber,
labels: payload.labels,
});
break;

case 'PATCH':
console.log('[EGRESS] Patching action triggered (not yet implemented).');
break;
Comment thread
chadd28 marked this conversation as resolved.
Outdated

default:
console.log(`[EGRESS] Unknown action: ${action}`);
break;
}
}
37 changes: 36 additions & 1 deletion tools/caretaker-agent/cloudrun/egress-service/src/app.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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: {
Expand All @@ -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();
});
});
19 changes: 2 additions & 17 deletions tools/caretaker-agent/cloudrun/egress-service/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Comment thread
chadd28 marked this conversation as resolved.

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<void> {
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());

Expand Down