diff --git a/services/githubbot/src/pr-manager.ts b/services/githubbot/src/pr-manager.ts index 18ac3ef3b..aabac9655 100644 --- a/services/githubbot/src/pr-manager.ts +++ b/services/githubbot/src/pr-manager.ts @@ -1,6 +1,7 @@ import type { GitHubAdapter } from "@chat-adapter/github"; import type { StateAdapter } from "chat"; import { backgroundWaitUntil } from "./context"; +import { reactWorkingOnSubject, settleSubjectReaction } from "./reactions"; import { runTurnStream } from "./turn"; import type { ForwardSessionInput, @@ -719,19 +720,40 @@ function fireManagementTurn( pr: `${owner}/${repo}#${pr.number}`, work: message.label, }); + // Management turns have no triggering comment to react to, so ack on the PR + // itself — instant 👀, settled to 🚀/😕 when the turn finishes (same lifecycle + // as review-request and issue-work turns). Not awaited: the ack must not delay + // the turn, and a failed reaction is only a missing ack. + void reactWorkingOnSubject(ctx.octokit, owner, repo, pr.number, logger(ctx)); backgroundWaitUntil( runTurnStream(ctx.options, forwardInput) - .then((result) => { + .then(async (result) => { traceLog(ctx.options, "githubbot_management_turn_complete", trace, { failed: result.failed, work: message.label, }); + await settleSubjectReaction( + ctx.octokit, + owner, + repo, + pr.number, + result.failed, + logger(ctx), + ); }) - .catch((error) => { + .catch(async (error) => { logger(ctx).warn("githubbot_management_turn_failed", { error: errorMessage(error), work: message.label, }); + await settleSubjectReaction( + ctx.octokit, + owner, + repo, + pr.number, + true, + logger(ctx), + ); }), ); } diff --git a/services/githubbot/test/pr-manager.test.ts b/services/githubbot/test/pr-manager.test.ts index 02fa3786d..c3b282368 100644 --- a/services/githubbot/test/pr-manager.test.ts +++ b/services/githubbot/test/pr-manager.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { drainBackgroundWork } from "../src/context"; import { decideMerge, evaluateCi, @@ -405,3 +406,66 @@ describe("CI fix counter and escalation", () => { }); }); }); + +describe("management turn reaction ack", () => { + const submittedReview = (state: string) => + JSON.stringify({ + action: "submitted", + repository: { full_name: "base/repo" }, + pull_request: { number: 7 }, + review: { id: 55, state, user: { login: "reviewer" } }, + }); + + function reviewCtx(reactions: { issue_number: number; content: string }[]) { + return { + octokit: { + rest: { + pulls: { + get: async () => ({ + data: prPayload({ headRepoFullName: "base/repo" }), + }), + merge: async () => ({ data: {} }), + }, + git: { deleteRef: async () => ({ data: {} }) }, + reactions: { + createForIssue: async (input: { + issue_number: number; + content: string; + }) => { + reactions.push({ + issue_number: input.issue_number, + content: input.content, + }); + return { data: {} }; + }, + }, + }, + }, + options: { + apiUrl: "http://localhost", + deleteBranchOnMerge: false, + logger: quietLogger, + // Non-retryable so the backgrounded turn settles off the network. + fetch: () => Promise.resolve(new Response("no", { status: 400 })), + }, + state: makeState(), + userName: "centaur-bot", + } as unknown as PrManagerContext; + } + + test("acks a changes-requested review with eyes immediately, settling when the turn fails", async () => { + const reactions: { issue_number: number; content: string }[] = []; + await handleReviewEvent(reviewCtx(reactions), submittedReview("changes_requested")); + // The working ack lands before the management turn runs. + expect(reactions).toContainEqual({ issue_number: 7, content: "eyes" }); + await drainBackgroundWork(5_000); + expect(reactions).toContainEqual({ issue_number: 7, content: "confused" }); + }); + + test("does not react on an approved review (deterministic merge, no work turn)", async () => { + const reactions: { issue_number: number; content: string }[] = []; + await handleReviewEvent(reviewCtx(reactions), submittedReview("approved")); + await drainBackgroundWork(5_000); + expect(reactions).toEqual([]); + }); +});