Skip to content

ci: accept exact-head Codex no-finding comments #4006

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
kojiwakayama merged 1 commit into from
Aug 22, 2026
Merged

ci: accept exact-head Codex no-finding comments #4006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 62 additions & 2 deletions scripts/ci/automated-review-gate.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ const AUTOMATED_REVIEW_LOGINS = new Set([
]);
const CODERABBIT_LOGIN = "coderabbitai[bot]";
const CODERABBIT_RECENT_REVIEW_MARKER = "<!-- recent_review_start -->";
const CODEX_LOGIN = "chatgpt-codex-connector[bot]";
const CODEX_BOT_ID = 199175422;
const CODEX_NO_FINDING_PREFIX = "Codex Review: Didn't find any major issues.";
const CODEX_REVIEWED_COMMIT_PATTERN =
/\*\*Reviewed commit:\*\*\s*`([0-9a-f]{10})`/i;
const FULL_COMMIT_PATTERN = /^[0-9a-f]{40}$/i;
/** @type {(ref: string) => Promise<string | undefined>} */
const NO_COMMIT_RESOLVER = () => Promise.resolve(undefined);
export const AUTOMATED_REVIEW_STATUS_CONTEXT = "Automated review";
const SUBMITTED_REVIEW_STATES = new Set([
"APPROVED",
Expand All @@ -12,7 +20,14 @@ const SUBMITTED_REVIEW_STATES = new Set([
]);

/** Find an actual automated review submitted against the current PR head. */
export function findAutomatedReview({ reviews, comments }, headSha) {
export async function findAutomatedReview(
{
reviews,
comments,
resolveCommit = NO_COMMIT_RESOLVER,
},
headSha,
) {
for (let index = reviews.length - 1; index >= 0; index--) {
const review = reviews[index];
const login = review?.user?.login;
Expand Down Expand Up @@ -40,6 +55,33 @@ export function findAutomatedReview({ reviews, comments }, headSha) {
const comment = comments[index];
const login = comment?.user?.login;
const body = comment?.body;
if (
typeof login === "string" &&
login.toLowerCase() === CODEX_LOGIN &&
comment?.user?.type === "Bot" &&
comment?.user?.id === CODEX_BOT_ID &&
typeof body === "string" &&
body.startsWith(CODEX_NO_FINDING_PREFIX)
) {
const reviewedCommit = body.match(CODEX_REVIEWED_COMMIT_PATTERN)?.[1];
if (typeof reviewedCommit === "string") {
const resolvedCommit = await resolveCommit(reviewedCommit);
if (
typeof resolvedCommit === "string" &&
FULL_COMMIT_PATTERN.test(resolvedCommit) &&
resolvedCommit.toLowerCase() === headSha.toLowerCase()
) {
return {
reviewer: login,
source: "summary",
state: "COMMENTED",
url: typeof comment.html_url === "string"
? comment.html_url
: undefined,
};
}
}
}
if (
typeof login !== "string" ||
login.toLowerCase() !== CODERABBIT_LOGIN ||
Expand Down Expand Up @@ -99,7 +141,25 @@ export async function publishAutomatedReviewStatus({
issue_number: pullNumber,
per_page: 100,
});
review = findAutomatedReview({ reviews, comments }, headSha);
review = await findAutomatedReview({
reviews,
comments,
resolveCommit: async (ref) => {
try {
const response = await github.rest.repos.getCommit({
owner,
repo,
ref,
});
const sha = response?.data?.sha;
return typeof sha === "string" && FULL_COMMIT_PATTERN.test(sha)
? sha
: undefined;
} catch {
return undefined;
}
},
}, headSha);
if (!review) {
failure = new Error(
`No automated review was submitted for current commit ${
Expand Down
221 changes: 213 additions & 8 deletions scripts/ci/automated-review-gate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {

const HEAD_SHA = "a".repeat(40);
const STALE_SHA = "b".repeat(40);
const CODEX_BOT_ID = 199175422;
const WORKFLOW_PATH = new URL(
"../../.github/workflows/automated-review-gate.yml",
import.meta.url,
Expand Down Expand Up @@ -43,6 +44,25 @@ function codeRabbitSummary(
};
}

function codexNoFindingComment(
overrides: Record<string, unknown> = {},
): Record<string, unknown> {
return {
user: {
login: "chatgpt-codex-connector[bot]",
type: "Bot",
id: CODEX_BOT_ID,
},
body: [
"Codex Review: Didn't find any major issues. Nice work!",
`**Reviewed commit:** \`${HEAD_SHA.slice(0, 10)}\``,
].join("\n\n"),
html_url:
"https://github.com/veryfront/veryfront-code/pull/1#issuecomment-2",
...overrides,
};
}

function record(value: unknown, label: string): Record<string, unknown> {
if (typeof value !== "object" || value === null || Array.isArray(value)) {
throw new TypeError(`${label} must be a record`);
Expand All @@ -51,38 +71,139 @@ function record(value: unknown, label: string): Record<string, unknown> {
}

describe("automated review gate", () => {
it("accepts submitted CodeRabbit and Codex reviews for the current head", () => {
it("accepts submitted CodeRabbit and Codex reviews for the current head", async () => {
assertEquals(
findAutomatedReview({ reviews: [review()], comments: [] }, HEAD_SHA)
(await findAutomatedReview(
{ reviews: [review()], comments: [] },
HEAD_SHA,
))
?.reviewer,
"coderabbitai[bot]",
);
assertEquals(
findAutomatedReview({
(await findAutomatedReview({
reviews: [
review({ user: { login: "chatgpt-codex-connector[bot]" } }),
],
comments: [],
}, HEAD_SHA)?.reviewer,
}, HEAD_SHA))?.reviewer,
"chatgpt-codex-connector[bot]",
);
assertEquals(
findAutomatedReview(
(await findAutomatedReview(
{ reviews: [], comments: [codeRabbitSummary()] },
HEAD_SHA,
)
))
?.source,
"summary",
);
});

it("rejects skipped comments, stale reviews, pending reviews, and humans", () => {
it("accepts an authenticated Codex no-finding comment for the current head", async () => {
assertEquals(
await findAutomatedReview(
{
reviews: [],
comments: [codexNoFindingComment()],
resolveCommit: () => Promise.resolve(HEAD_SHA),
},
HEAD_SHA,
),
{
reviewer: "chatgpt-codex-connector[bot]",
source: "summary",
state: "COMMENTED",
url:
"https://github.com/veryfront/veryfront-code/pull/1#issuecomment-2",
},
);
});

it("rejects a Codex comment unless it resolves to the exact full head", async () => {
for (
const resolvedCommit of [STALE_SHA, HEAD_SHA.slice(0, 39), undefined]
) {
assertEquals(
await findAutomatedReview(
{
reviews: [],
comments: [codexNoFindingComment()],
resolveCommit: () => Promise.resolve(resolvedCommit),
},
HEAD_SHA,
),
undefined,
);
}
});

it("rejects stale or unauthenticated Codex issue comments", async () => {
const currentHeadBody = [
"Codex Review: Didn't find any major issues. Nice work!",
`**Reviewed commit:** \`${HEAD_SHA.slice(0, 10)}\``,
].join("\n\n");
const rejectedComments = [
codexNoFindingComment({
body: [
"Codex Review: Didn't find any major issues. Nice work!",
`**Reviewed commit:** \`${STALE_SHA.slice(0, 10)}\``,
].join("\n\n"),
}),
codexNoFindingComment({
user: { login: "maintainer", type: "User", id: 1 },
}),
codexNoFindingComment({
user: {
login: "chatgpt-codex-connector[bot]",
type: "Bot",
id: CODEX_BOT_ID + 1,
},
}),
codexNoFindingComment({
user: {
login: "chatgpt-codex-connector[bot]",
type: "User",
id: CODEX_BOT_ID,
},
}),
codexNoFindingComment({ body: "@codex review" }),
codexNoFindingComment({
body: `Codex Review: Action not completed.\n\n${currentHeadBody}`,
}),
codexNoFindingComment({
body:
`Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** \`${
HEAD_SHA.slice(0, 9)
}\``,
}),
codexNoFindingComment({
body:
`Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** \`${
HEAD_SHA.slice(0, 11)
}\``,
}),
];
const resolveCommit = (ref: string) =>
Promise.resolve(ref === HEAD_SHA.slice(0, 10) ? HEAD_SHA : STALE_SHA);

for (const comment of rejectedComments) {
assertEquals(
await findAutomatedReview(
{ reviews: [], comments: [comment], resolveCommit },
HEAD_SHA,
),
undefined,
);
}
});

it("rejects skipped comments, stale reviews, pending reviews, and humans", async () => {
const skippedIssueComment = {
user: { login: "coderabbitai[bot]" },
body: "rate limited, review skipped",
};
assertEquals(
findAutomatedReview({
await findAutomatedReview({
reviews: [
review({ commit_id: STALE_SHA }),
review({ state: "PENDING" }),
Expand Down Expand Up @@ -150,6 +271,90 @@ describe("automated review gate", () => {
assertEquals(statuses[1]?.state, "failure");
});

it("resolves a Codex comment to the exact commit before publishing success", async () => {
const statuses: Array<Record<string, unknown>> = [];
const resolvedRefs: string[] = [];
const listReviews = () => Promise.resolve();
const listComments = () => Promise.resolve();
const github = {
paginate: (endpoint: unknown) =>
Promise.resolve(
endpoint === listComments ? [codexNoFindingComment()] : [],
),
rest: {
issues: { listComments },
pulls: { listReviews },
repos: {
createCommitStatus: (status: Record<string, unknown>) => {
statuses.push(status);
return Promise.resolve();
},
getCommit: ({ ref }: { ref: string }) => {
resolvedRefs.push(ref);
return Promise.resolve({ data: { sha: HEAD_SHA } });
},
},
},
};

const result = await publishAutomatedReviewStatus({
github,
owner: "veryfront",
repo: "veryfront-code",
pullNumber: 1,
headSha: HEAD_SHA,
pullUrl: "https://github.com/veryfront/veryfront-code/pull/1",
});

assertEquals(result.state, "success");
assertEquals(resolvedRefs, [HEAD_SHA.slice(0, 10)]);
assertEquals(statuses[0]?.state, "success");
assertEquals(
statuses[0]?.target_url,
"https://github.com/veryfront/veryfront-code/pull/1#issuecomment-2",
);
});

it("publishes failure when a Codex commit cannot be resolved exactly", async () => {
const statuses: Array<Record<string, unknown>> = [];
const listReviews = () => Promise.resolve();
const listComments = () => Promise.resolve();
const github = {
paginate: (endpoint: unknown) =>
Promise.resolve(
endpoint === listComments ? [codexNoFindingComment()] : [],
),
rest: {
issues: { listComments },
pulls: { listReviews },
repos: {
createCommitStatus: (status: Record<string, unknown>) => {
statuses.push(status);
return Promise.resolve();
},
getCommit: () =>
Promise.reject(Object.assign(new Error("ambiguous commit"), {
status: 422,
})),
},
},
};

const result = await publishAutomatedReviewStatus({
github,
owner: "veryfront",
repo: "veryfront-code",
pullNumber: 1,
headSha: HEAD_SHA,
pullUrl: "https://github.com/veryfront/veryfront-code/pull/1",
});

assertEquals(result.state, "failure");
assertEquals(result.review, undefined);
assertEquals(statuses[0]?.state, "failure");
assertEquals(statuses[0]?.sha, HEAD_SHA);
});

it("fails closed when the review lookup throws", async () => {
const statuses: Array<Record<string, unknown>> = [];
const github = {
Expand Down
Loading