Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 24 additions & 1 deletion agent/channels/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,23 @@ async function actorPermission(ctx: GitHubInboundContext): Promise<string> {
}
}

// The comment webhook payload eve surfaces does not carry the issue/PR state,
// so a "skip closed threads" gate needs one REST lookup. Fails open: any error
// allows dispatch, so a transient lookup failure never silently drops real work.
async function conversationIsClosed(ctx: GitHubInboundContext): Promise<boolean> {
const number = ctx.conversation.pullRequestNumber ?? ctx.conversation.issueNumber;
if (number == null) return false;
try {
const response = await ctx.github.request<{ state?: string }>({
method: "GET",
path: `/repos/${encodeURIComponent(ctx.repository.owner)}/${encodeURIComponent(ctx.repository.name)}/issues/${number}`,
});
return response.body.state === "closed";
} catch {
return false;
}
}

// eve's issue.raw is the webhook payload's `issue` object only; the top-level
// `label` key describing which label was applied is not passed through, so
// dispatch is decided from the issue's current labels array instead.
Expand All @@ -49,8 +66,14 @@ export function issueHasLabel(raw: unknown, name: string): boolean {
);
}

async function onComment(ctx: GitHubInboundContext, comment: GitHubComment) {
// Exported for unit tests; not part of the channel's public behaviour.
export async function onComment(ctx: GitHubInboundContext, comment: GitHubComment) {
if (!matchesConfiguredRepository(ctx)) return null;
// Comments on a closed issue/PR are almost always housekeeping (e.g. a
// maintainer closing with a note). Dispatching there just spawns an idle
// session holding a sandbox. Skip them; open threads still dispatch, so the
// agent keeps its freedom to act on any live request.
if (await conversationIsClosed(ctx)) return null;
const permission = await actorPermission(ctx);
return {
auth: defaultGitHubAuth(ctx),
Expand Down
53 changes: 51 additions & 2 deletions tests/channels/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import type {
GitHubApiResponse,
GitHubInboundContext,
} from "eve/channels/github";
import { issueHasLabel, matchesConfiguredRepository, onIssue } from "../../agent/channels/github";
import {
issueHasLabel,
matchesConfiguredRepository,
onComment,
onIssue,
} from "../../agent/channels/github";

const ORIGINAL_EVOLVE_REPOSITORY = process.env.EVOLVE_REPOSITORY;

Expand All @@ -26,6 +31,7 @@ function createContext(
readonly repositoryFullName?: string;
readonly senderLogin?: string;
readonly permission?: string | null;
readonly state?: string;
readonly requestError?: Error;
} = {},
): { readonly ctx: GitHubInboundContext; readonly requestCalls: number[] } {
Expand All @@ -46,7 +52,10 @@ function createContext(
requestCalls.push(1);
if (overrides.requestError) throw overrides.requestError;
return {
body: { permission: overrides.permission ?? "none" } as T,
body: {
permission: overrides.permission ?? "none",
state: overrides.state ?? "open",
} as T,
ok: true,
status: 200,
};
Expand Down Expand Up @@ -201,3 +210,43 @@ for (const permission of ["admin", "maintain", "write"]) {
assert.match(result!.context![0], /#23/);
});
}

// onComment closed-thread gate

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const STUB_COMMENT = { body: "hello", id: 1, raw: {} } as any;

test("onComment dispatches on a comment in an open thread", async () => {
process.env.EVOLVE_REPOSITORY = "uk-agents/evolve";
const { ctx, requestCalls } = createContext({ permission: "admin", state: "open" });
const result = await onComment(ctx, STUB_COMMENT);
assert.notEqual(result, null);
assert.ok(result?.auth);
assert.equal(requestCalls.length, 2, "open thread should check state, then permission");
});

test("onComment suppresses a comment on a closed thread", async () => {
process.env.EVOLVE_REPOSITORY = "uk-agents/evolve";
const { ctx, requestCalls } = createContext({ permission: "admin", state: "closed" });
const result = await onComment(ctx, STUB_COMMENT);
assert.equal(result, null);
assert.equal(requestCalls.length, 1, "closed thread should short-circuit before the permission check");
});

test("onComment fails open (dispatches) when the state lookup errors", async () => {
process.env.EVOLVE_REPOSITORY = "uk-agents/evolve";
const { ctx } = createContext({ requestError: new Error("boom") });
const result = await onComment(ctx, STUB_COMMENT);
assert.notEqual(result, null, "a lookup failure must never silently drop a real comment");
});

test("onComment ignores a repository that does not match EVOLVE_REPOSITORY", async () => {
process.env.EVOLVE_REPOSITORY = "uk-agents/evolve";
const { ctx, requestCalls } = createContext({
permission: "admin",
repositoryFullName: "someone-else/other-repo",
});
const result = await onComment(ctx, STUB_COMMENT);
assert.equal(result, null);
assert.equal(requestCalls.length, 0, "no REST calls for an unconfigured repository");
});