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
1 change: 1 addition & 0 deletions src/handlers/mention.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,7 @@ describe("createMentionHandler rereview command", () => {
base: { ref: "main" },
},
}),
listRequestedReviewers: async () => ({ data: { users: [], teams: [] } }),
requestReviewers: async (params: { team_reviewers: string[] }) => {
requestedTeams = params.team_reviewers;
return { data: {} };
Expand Down
25 changes: 10 additions & 15 deletions src/handlers/mention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { buildMentionContext } from "../execution/mention-context.ts";
import { buildMentionPrompt } from "../execution/mention-prompt.ts";
import { classifyError, formatErrorComment, postOrUpdateErrorComment } from "../lib/errors.ts";
import { wrapInDetails } from "../lib/formatting.ts";
import { requestRereviewTeamBestEffort } from "./rereview-team.ts";

/**
* Create the mention handler and register it with the event router.
Expand Down Expand Up @@ -580,21 +581,15 @@ export function createMentionHandler(deps: {
mention.prNumber !== undefined &&
(normalizedQuestion === "review" || normalizedQuestion === "recheck")
) {
const rereviewTeam = (config.review.uiRereviewTeam ?? "").trim() || "aireview";

try {
await octokit.rest.pulls.requestReviewers({
owner: mention.owner,
repo: mention.repo,
pull_number: mention.prNumber,
team_reviewers: [rereviewTeam],
});
} catch (err) {
logger.warn(
{ err, prNumber: mention.prNumber, rereviewTeam },
"Failed to request ui rereview team for rereview (best-effort)",
);
}
const configuredTeam = (config.review.uiRereviewTeam ?? "").trim() || "aireview";
await requestRereviewTeamBestEffort({
octokit,
owner: mention.owner,
repo: mention.repo,
prNumber: mention.prNumber,
configuredTeam,
logger,
});

return;
}
Expand Down
86 changes: 86 additions & 0 deletions src/handlers/rereview-team.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { describe, expect, test } from "bun:test";
import type { Logger } from "pino";
import {
buildRereviewTeamCandidates,
requestRereviewTeamBestEffort,
} from "./rereview-team.ts";

function createNoopLogger(): Logger {
return {
fatal: () => undefined,
error: () => undefined,
warn: () => undefined,
info: () => undefined,
debug: () => undefined,
trace: () => undefined,
silent: () => undefined,
} as unknown as Logger;
}

describe("rereview-team helpers", () => {
test("buildRereviewTeamCandidates normalizes owner/team input and adds alias", () => {
expect(buildRereviewTeamCandidates("xbmc/aireview")).toEqual(["aireview", "ai-review"]);
expect(buildRereviewTeamCandidates("ai-review")).toEqual(["ai-review", "aireview"]);
});

test("requestRereviewTeamBestEffort skips request when team already present", async () => {
let requestCalls = 0;
const result = await requestRereviewTeamBestEffort({
octokit: {
rest: {
pulls: {
listRequestedReviewers: async () => ({
data: { users: [], teams: [{ slug: "aireview", name: "aireview" }] },
}),
requestReviewers: async () => {
requestCalls += 1;
return { data: {} };
},
},
},
},
owner: "xbmc",
repo: "kodiai",
prNumber: 1,
configuredTeam: "aireview",
logger: createNoopLogger(),
});

expect(result.alreadyRequested).toBe(true);
expect(result.requestedTeam).toBeUndefined();
expect(requestCalls).toBe(0);
});

test("requestRereviewTeamBestEffort falls back from aireview to ai-review on 422", async () => {
const attempted: string[] = [];

const result = await requestRereviewTeamBestEffort({
octokit: {
rest: {
pulls: {
listRequestedReviewers: async () => ({ data: { users: [], teams: [] } }),
requestReviewers: async (params: { team_reviewers: string[] }) => {
const slug = params.team_reviewers[0] ?? "";
attempted.push(slug);
if (slug === "aireview") {
const err = new Error("Validation failed") as Error & { status: number };
err.status = 422;
throw err;
}
return { data: {} };
},
},
},
},
owner: "xbmc",
repo: "kodiai",
prNumber: 2,
configuredTeam: "aireview",
logger: createNoopLogger(),
});

expect(attempted).toEqual(["aireview", "ai-review"]);
expect(result.requestedTeam).toBe("ai-review");
expect(result.alreadyRequested).toBe(false);
});
});
104 changes: 104 additions & 0 deletions src/handlers/rereview-team.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import type { Logger } from "pino";

function normalizeConfiguredTeam(team: string): string {
const trimmed = team.trim().toLowerCase();
if (trimmed.length === 0) return "";
const lastSlash = trimmed.lastIndexOf("/");
return lastSlash >= 0 ? trimmed.slice(lastSlash + 1) : trimmed;
}

export function buildRereviewTeamCandidates(team: string): string[] {
const primary = normalizeConfiguredTeam(team);
if (primary.length === 0) return [];

const candidates = new Set<string>([primary]);
if (primary === "ai-review") candidates.add("aireview");
if (primary === "aireview") candidates.add("ai-review");
return Array.from(candidates);
}

function getStatusCode(err: unknown): number | undefined {
if (typeof err !== "object" || err === null) return undefined;
const value = (err as { status?: unknown }).status;
return typeof value === "number" ? value : undefined;
}

export async function requestRereviewTeamBestEffort(options: {
octokit: {
rest: {
pulls: {
listRequestedReviewers: (params: {
owner: string;
repo: string;
pull_number: number;
}) => Promise<{ data: { teams?: Array<{ slug?: string | null; name?: string | null }> } }>;
requestReviewers: (params: {
owner: string;
repo: string;
pull_number: number;
team_reviewers: string[];
}) => Promise<unknown>;
};
};
};
owner: string;
repo: string;
prNumber: number;
configuredTeam: string;
logger: Logger;
}): Promise<{ requestedTeam?: string; alreadyRequested: boolean }> {
const { octokit, owner, repo, prNumber, configuredTeam, logger } = options;
const candidates = buildRereviewTeamCandidates(configuredTeam);
if (candidates.length === 0) return { alreadyRequested: false };

try {
const requested = await octokit.rest.pulls.listRequestedReviewers({
owner,
repo,
pull_number: prNumber,
});

const already = (requested.data.teams ?? []).some((team) => {
const slug = (team.slug ?? "").trim().toLowerCase();
const name = (team.name ?? "").trim().toLowerCase();
return candidates.includes(slug) || candidates.includes(name);
});
if (already) return { alreadyRequested: true };
} catch (err) {
logger.warn(
{ err, owner, repo, prNumber, configuredTeam },
"Failed to list requested reviewers; attempting rereview request anyway",
);
}

for (const candidate of candidates) {
try {
await octokit.rest.pulls.requestReviewers({
owner,
repo,
pull_number: prNumber,
team_reviewers: [candidate],
});
return { requestedTeam: candidate, alreadyRequested: false };
} catch (err) {
const status = getStatusCode(err);
const canFallback = status === 422 && candidate !== candidates[candidates.length - 1];
logger.warn(
{
err,
owner,
repo,
prNumber,
configuredTeam,
candidate,
status,
fallbackRemaining: canFallback,
},
"Failed to request rereview team candidate",
);
if (!canFallback) break;
}
}

return { alreadyRequested: false };
}
49 changes: 3 additions & 46 deletions src/handlers/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
buildReviewOutputKey,
ensureReviewOutputNotPublished,
} from "./review-idempotency.ts";
import { requestRereviewTeamBestEffort } from "./rereview-team.ts";
import { $ } from "bun";
import { fetchAndCheckoutPullRequestHeadRef } from "../jobs/workspace.ts";

Expand Down Expand Up @@ -61,50 +62,6 @@ export function createReviewHandler(deps: {

const rereviewTeamSlugs = new Set(["ai-review", "aireview"]);

async function ensureUiRereviewTeamRequested(options: {
octokit: Awaited<ReturnType<GitHubApp["getInstallationOctokit"]>>;
owner: string;
repo: string;
prNumber: number;
team: string;
logger: Logger;
}): Promise<void> {
const { octokit, owner, repo, prNumber, team } = options;
const normalized = team.trim().toLowerCase();
if (normalized.length === 0) return;

try {
const requested = await octokit.rest.pulls.listRequestedReviewers({
owner,
repo,
pull_number: prNumber,
});
const already =
(requested.data.teams ?? []).some((t) => (t.slug ?? "").trim().toLowerCase() === normalized) ||
(requested.data.teams ?? []).some((t) => (t.name ?? "").trim().toLowerCase() === normalized);
if (already) return;
} catch (err) {
logger.warn(
{ err, owner, repo, prNumber, team },
"Failed to list requested reviewers; attempting to request ui rereview team anyway",
);
}

try {
await octokit.rest.pulls.requestReviewers({
owner,
repo,
pull_number: prNumber,
team_reviewers: [team],
});
} catch (err) {
logger.warn(
{ err, owner, repo, prNumber, team },
"Failed to request ui rereview team (best-effort)",
);
}
}

async function handleReview(event: WebhookEvent): Promise<void> {
const payload = event.payload as unknown as
| PullRequestOpenedEvent
Expand Down Expand Up @@ -333,12 +290,12 @@ export function createReviewHandler(deps: {
(action === "opened" || action === "ready_for_review")
) {
const octokit = await githubApp.getInstallationOctokit(event.installationId);
await ensureUiRereviewTeamRequested({
await requestRereviewTeamBestEffort({
octokit,
owner: apiOwner,
repo: apiRepo,
prNumber: pr.number,
team: config.review.uiRereviewTeam,
configuredTeam: config.review.uiRereviewTeam,
logger,
});
}
Expand Down
Loading