From d6a95677e33e8835f90dea8c6e22df247a02d32d Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sun, 23 Aug 2026 01:03:21 +0200 Subject: [PATCH] Accept verified Codex no-finding reviews without weakening head binding Codex emits zero-finding results as issue comments with a ten-character commit token. Resolve that token through GitHub and require the returned full SHA to equal the pull request head before publishing success. Constraint: Codex zero-finding comments expose only a 10-hex commit token Rejected: Direct prefix comparison | permits a chosen-prefix replay against another head Rejected: Status timestamp freshness | delayed review results can race a force-push Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep comment evidence bound through GitHub commit resolution; do not replace this with prefix-only matching Tested: Focused gate tests, format, lint, typecheck, diff check, live comment and commit payload, independent security review Not-tested: Full test:scripts is blocked by unrelated generated-artifact and documentation-source failures reproduced outside this diff --- scripts/ci/automated-review-gate.mjs | 64 ++++++- scripts/ci/automated-review-gate.test.ts | 221 ++++++++++++++++++++++- 2 files changed, 275 insertions(+), 10 deletions(-) diff --git a/scripts/ci/automated-review-gate.mjs b/scripts/ci/automated-review-gate.mjs index 7d60f5856e..08576c01d6 100644 --- a/scripts/ci/automated-review-gate.mjs +++ b/scripts/ci/automated-review-gate.mjs @@ -4,6 +4,14 @@ const AUTOMATED_REVIEW_LOGINS = new Set([ ]); const CODERABBIT_LOGIN = "coderabbitai[bot]"; const CODERABBIT_RECENT_REVIEW_MARKER = ""; +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} */ +const NO_COMMIT_RESOLVER = () => Promise.resolve(undefined); export const AUTOMATED_REVIEW_STATUS_CONTEXT = "Automated review"; const SUBMITTED_REVIEW_STATES = new Set([ "APPROVED", @@ -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; @@ -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 || @@ -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 ${ diff --git a/scripts/ci/automated-review-gate.test.ts b/scripts/ci/automated-review-gate.test.ts index 312b6df5e5..013da9b557 100644 --- a/scripts/ci/automated-review-gate.test.ts +++ b/scripts/ci/automated-review-gate.test.ts @@ -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, @@ -43,6 +44,25 @@ function codeRabbitSummary( }; } +function codexNoFindingComment( + overrides: Record = {}, +): Record { + 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 { if (typeof value !== "object" || value === null || Array.isArray(value)) { throw new TypeError(`${label} must be a record`); @@ -51,38 +71,139 @@ function record(value: unknown, label: string): Record { } 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" }), @@ -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> = []; + 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) => { + 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> = []; + 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) => { + 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> = []; const github = {