From a6f64938c8d0bbf915068869977dfe86c58f0d3f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 13 Jul 2026 15:40:20 +0000 Subject: [PATCH 1/2] fix(ci): gate Codex auto-resolve on completed review, post as non-bot user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auto-resolve request never reached Codex because it was authored by github-actions[bot], whose @codex mentions the Codex connector ignores. It also fired on the first inline review comment, before a review completed. - Trigger the request off the Codex-authored `pull_request_review` (submitted) event instead of the first inline comment, so it runs only after a code review finishes. Skip approved/dismissed reviews and reviews with no inline findings — nothing to resolve. - Post the request with a fine-grained PAT (CODEX_TRIGGER_TOKEN) so it comes from a real user identity the Codex connector will act on. - Trust the dedup marker from the trigger-token account (resolved at runtime) rather than github-actions[bot]. - Split thread resolution into its own job that keeps `pull-requests: write` and acts only on trusted disposition-marker replies. - Update AGENTS.md to match the new trigger, token, and dedup behavior. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01NrdqyvKzaovZUX9haKfgjM --- .../codex-autofix-review-comments.yml | 275 ++++++++++++------ AGENTS.md | 15 +- 2 files changed, 189 insertions(+), 101 deletions(-) diff --git a/.github/workflows/codex-autofix-review-comments.yml b/.github/workflows/codex-autofix-review-comments.yml index 74be36b8d..38d9ded7d 100644 --- a/.github/workflows/codex-autofix-review-comments.yml +++ b/.github/workflows/codex-autofix-review-comments.yml @@ -1,24 +1,28 @@ name: Codex auto-resolve review comments on: + pull_request_review: + types: [submitted] pull_request_review_comment: types: [created] permissions: contents: read - issues: write - models: read - pull-requests: write jobs: request-codex-autoresolve: name: Request Codex auto-resolve runs-on: ubuntu-24.04 + # Only run after Codex submits a completed review, never on the first inline + # comment mid-review. Approvals/dismissals are handled below (no findings). if: > + github.event_name == 'pull_request_review' && github.event.pull_request.state == 'open' && - github.event.comment.user.type == 'Bot' && - (github.event.comment.user.login == 'chatgpt-codex-connector' || - github.event.comment.user.login == 'chatgpt-codex-connector[bot]') + github.event.review.user.type == 'Bot' && + (github.event.review.user.login == 'chatgpt-codex-connector' || + github.event.review.user.login == 'chatgpt-codex-connector[bot]') + permissions: + contents: read concurrency: group: codex-autoresolve-${{ github.event.pull_request.number }} cancel-in-progress: false @@ -26,10 +30,13 @@ jobs: - name: Ask Codex to resolve review comments uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: - github-token: ${{ github.token }} + # A fine-grained PAT from a real (non-bot) account. The Codex connector + # ignores @codex commands authored by github-actions[bot], so the + # request must be posted by a user identity for Codex to act on it. + github-token: ${{ secrets.CODEX_TRIGGER_TOKEN }} script: | const pr = context.payload.pull_request; - const reviewComment = context.payload.comment; + const review = context.payload.review; const allowedCodexBotLogins = new Set([ "chatgpt-codex-connector", "chatgpt-codex-connector[bot]", @@ -38,103 +45,62 @@ jobs: const resolvedDispositionMarker = ""; if ( - reviewComment.user?.type !== "Bot" || - !allowedCodexBotLogins.has(reviewComment.user.login) + review.user?.type !== "Bot" || + !allowedCodexBotLogins.has(review.user.login) ) { - core.warning("Skipping Codex auto-resolve request because the review comment author is not the trusted Codex connector bot."); + core.warning("Skipping Codex auto-resolve request because the review author is not the trusted Codex connector bot."); return; } - if (reviewComment.in_reply_to_id) { - const replyBody = (reviewComment.body || "").trimStart(); - if (!replyBody.startsWith(resolvedDispositionMarker)) { - core.notice("Skipping review-thread reply without the trusted resolved disposition marker."); - return; - } - - const owner = context.repo.owner; - const repo = context.repo.repo; - const issue_number = pr.number; - const targetCommentIds = new Set([reviewComment.id, reviewComment.in_reply_to_id]); - const reviewThreadsQuery = ` - query ReviewThreads($owner: String!, $repo: String!, $number: Int!, $cursor: String) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $number) { - reviewThreads(first: 100, after: $cursor) { - nodes { - id - isResolved - comments(first: 100) { - nodes { databaseId } - } - } - pageInfo { - endCursor - hasNextPage - } - } - } - } - } - `; - - let cursor = null; - let matchingThread; - - try { - do { - const result = await github.graphql(reviewThreadsQuery, { - owner, - repo, - number: issue_number, - cursor, - }); - const connection = result.repository?.pullRequest?.reviewThreads; - matchingThread = connection?.nodes?.find((thread) => - thread.comments.nodes.some((comment) => targetCommentIds.has(comment.databaseId)), - ); - cursor = connection?.pageInfo?.hasNextPage ? connection.pageInfo.endCursor : null; - } while (!matchingThread && cursor); - - if (!matchingThread) { - core.setFailed("Codex auto-resolve could not find the review thread for the disposition reply."); - return; - } + // Gate: only a completed review that actually raised findings is worth + // an auto-resolve pass. An approval or dismissal has nothing to fix. + if (review.state === "approved" || review.state === "dismissed") { + core.notice(`Skipping Codex auto-resolve request because the Codex review state was '${review.state}' (no actionable findings).`); + return; + } - if (matchingThread.isResolved) { - core.notice("The Codex review thread was already resolved."); - return; - } + const owner = context.repo.owner; + const repo = context.repo.repo; + const issue_number = pr.number; - await github.graphql( - `mutation ResolveReviewThread($threadId: ID!) { - resolveReviewThread(input: { threadId: $threadId }) { - thread { id isResolved } - } - }`, - { threadId: matchingThread.id }, - ); - core.notice("Resolved the Codex review thread after its explicit disposition reply."); - } catch (error) { - const message = `Codex auto-resolve failed while closing the review thread: ${error.message || error}`; + let reviewComments; + try { + reviewComments = await github.paginate(github.rest.pulls.listCommentsForReview, { + owner, + repo, + pull_number: issue_number, + review_id: review.id, + per_page: 100, + }); + } catch (error) { + if (error.status === 403) { + const message = "Codex auto-resolve failed because the trigger token cannot read review comments."; core.warning(message); core.setFailed(message); + return; } + + throw error; + } + + if (reviewComments.length === 0) { + core.notice("Skipping Codex auto-resolve request because the completed Codex review left no inline findings to resolve."); return; } - const sourceBody = (reviewComment.body || "").trimStart(); - const hasAutoResolveMarker = - sourceBody.startsWith("`; const maxAutoResolveRequests = 1; @@ -159,8 +125,7 @@ jobs: const trustedExistingRequests = existingComments.filter( (comment) => - comment.user?.type === "Bot" && - comment.user.login === "github-actions[bot]" && + comment.user?.login === triggerLogin && (comment.body || "").trimStart().startsWith("`, - ``, + ``, `${scopedResolveCommand} using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with ${resolvedDispositionMarker} as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.`, ].join("\n\n"); @@ -193,7 +158,7 @@ jobs: }); } catch (error) { if (error.status === 403) { - const message = "Codex auto-resolve failed because this workflow token cannot comment on the pull request."; + const message = "Codex auto-resolve failed because the trigger token cannot comment on the pull request."; core.warning(message); core.setFailed(message); return; @@ -201,3 +166,125 @@ jobs: throw error; } + + resolve-codex-thread: + name: Resolve Codex review thread + runs-on: ubuntu-24.04 + # Marker-driven thread closure only. This is the narrow job that carries + # pull-requests: write, and it acts solely on trusted disposition replies. + if: > + github.event_name == 'pull_request_review_comment' && + github.event.pull_request.state == 'open' && + github.event.comment.user.type == 'Bot' && + (github.event.comment.user.login == 'chatgpt-codex-connector' || + github.event.comment.user.login == 'chatgpt-codex-connector[bot]') + permissions: + contents: read + pull-requests: write + concurrency: + group: codex-autoresolve-thread-${{ github.event.pull_request.number }} + cancel-in-progress: false + steps: + - name: Resolve Codex review thread on disposition marker + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + github-token: ${{ github.token }} + script: | + const pr = context.payload.pull_request; + const reviewComment = context.payload.comment; + const allowedCodexBotLogins = new Set([ + "chatgpt-codex-connector", + "chatgpt-codex-connector[bot]", + ]); + const resolvedDispositionMarker = ""; + + if ( + reviewComment.user?.type !== "Bot" || + !allowedCodexBotLogins.has(reviewComment.user.login) + ) { + core.warning("Skipping Codex thread resolution because the review comment author is not the trusted Codex connector bot."); + return; + } + + // Only trusted, marked replies resolve a thread. A non-reply review + // comment is a fresh finding; the request job (on the submitted + // review) owns that path, so there is nothing to do here. + if (!reviewComment.in_reply_to_id) { + core.notice("Skipping non-reply Codex review comment; the auto-resolve request is driven by the submitted review."); + return; + } + + const replyBody = (reviewComment.body || "").trimStart(); + if (!replyBody.startsWith(resolvedDispositionMarker)) { + core.notice("Skipping review-thread reply without the trusted resolved disposition marker."); + return; + } + + const owner = context.repo.owner; + const repo = context.repo.repo; + const issue_number = pr.number; + const targetCommentIds = new Set([reviewComment.id, reviewComment.in_reply_to_id]); + const reviewThreadsQuery = ` + query ReviewThreads($owner: String!, $repo: String!, $number: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $cursor) { + nodes { + id + isResolved + comments(first: 100) { + nodes { databaseId } + } + } + pageInfo { + endCursor + hasNextPage + } + } + } + } + } + `; + + let cursor = null; + let matchingThread; + + try { + do { + const result = await github.graphql(reviewThreadsQuery, { + owner, + repo, + number: issue_number, + cursor, + }); + const connection = result.repository?.pullRequest?.reviewThreads; + matchingThread = connection?.nodes?.find((thread) => + thread.comments.nodes.some((comment) => targetCommentIds.has(comment.databaseId)), + ); + cursor = connection?.pageInfo?.hasNextPage ? connection.pageInfo.endCursor : null; + } while (!matchingThread && cursor); + + if (!matchingThread) { + core.setFailed("Codex auto-resolve could not find the review thread for the disposition reply."); + return; + } + + if (matchingThread.isResolved) { + core.notice("The Codex review thread was already resolved."); + return; + } + + await github.graphql( + `mutation ResolveReviewThread($threadId: ID!) { + resolveReviewThread(input: { threadId: $threadId }) { + thread { id isResolved } + } + }`, + { threadId: matchingThread.id }, + ); + core.notice("Resolved the Codex review thread after its explicit disposition reply."); + } catch (error) { + const message = `Codex auto-resolve failed while closing the review thread: ${error.message || error}`; + core.warning(message); + core.setFailed(message); + } diff --git a/AGENTS.md b/AGENTS.md index 300d9c801..34f7d8496 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -389,19 +389,20 @@ When explicitly asked to fix or resolve review findings: ### Automatic resolve trigger -Automatic Codex review is review-only by default. This repository includes `.github/workflows/codex-autofix-review-comments.yml`, which requests the resolve task automatically after Codex posts a PR review or inline review comment. +Automatic Codex review is review-only by default. This repository includes `.github/workflows/codex-autofix-review-comments.yml`, which requests the resolve task automatically after Codex submits a completed PR review that raised findings. -- The workflow must only trigger from Codex review bot reviews or comments on open pull requests. +- The auto-resolve request must fire only from a Codex-authored `pull_request_review` **submitted** event on an open pull request — never from the first inline comment mid-review. This guarantees the request is posted only after a code review completes; without a review there are no findings and the request is pointless. +- The request job must skip reviews with no actionable findings: skip `approved`/`dismissed` reviews, and skip when the submitted review carries zero inline comments. - Match the trusted Codex connector bot by exact login and bot type; do not use substring login checks. -- Keep per-pull-request concurrency on the authorized job, not the whole workflow, so unrelated review comments cannot displace a pending Codex request. +- Keep per-pull-request concurrency on the authorized job, not the whole workflow, so unrelated events cannot displace a pending Codex request. - Pin the supported Node 24-based `actions/github-script` release to its reviewed immutable commit SHA. -- The workflow must skip auto-resolve request comments and must never turn review-thread replies into new repair requests. -- The workflow must treat unmarked review-thread replies as inert. A trusted Codex reply beginning with `` may only resolve the exact containing thread. +- Post the `@codex` resolve request with a real (non-bot) user identity — a fine-grained PAT held in the `CODEX_TRIGGER_TOKEN` secret. The Codex connector ignores commands authored by `github-actions[bot]`, so a bot-authored request is silently dropped. The token needs `pull-requests: write` (issue-comment) access and no more. +- The workflow must treat unmarked review-thread replies as inert. A trusted Codex reply beginning with `` may only resolve the exact containing thread, and a non-reply Codex review comment must never be turned into a new repair request. - The workflow must ask Codex to resolve only existing actionable Codex review findings for the triggering pull request and current head using these repository instructions; the resolve task must not perform a new review or create new findings. - The workflow may request one automatic repair pass per pull request lifetime. Later heads require an explicit human request. -- Only trust a pull-request deduplication marker when it was posted by the GitHub Actions bot. +- Only trust a pull-request deduplication marker when it was posted by the trigger-token account (the same identity that posts the request), resolved at runtime rather than hard-coded. - Permission failures while reading or creating pull-request comments must fail the workflow visibly, not return a successful soft-skip. -- Grant `pull-requests: write` only to the narrow marker-driven thread-resolution workflow; retain read-only repository contents and do not approve reviews or alter code. +- Grant `pull-requests: write` only to the narrow marker-driven thread-resolution job; the request job runs with read-only repository contents and relies on the trigger token's own scope, and neither job approves reviews or alters code. - The workflow must not run Codex directly with API credentials. - P0 and P1 findings should always be fixed. - P2 and lower findings should be fixed only when clear, scoped, low-risk, and testable; otherwise explain the decision and resolve or mark ready for human resolution. From 1f4cdd51b0dfb98495fc293bf511c167071a217c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 13 Jul 2026 16:50:17 +0000 Subject: [PATCH 2/2] fix(ci): align workflow guard/tests to split design and harden concurrency CI's Codex-autofix guard and its unit tests encoded the old single-job, issue_comment-forbidden design and blocked the new split-trigger workflow. - Update scripts/check-codex-autofix-workflow.mjs to the new contract: require the submitted-review trigger, event-name gates, review/comment identity checks for both jobs, the completed-review findings gate, the CODEX_TRIGGER_TOKEN / github.token bindings, and trigger-token-account dedup; keep forbidding issue_comment triggers, substring logins, loose marker trust, top-level concurrency, contents:write, and mutable pins. - Rewrite tests/codex-autofix-workflow.test.ts to extract and exercise both github-script blocks (request job on review payloads, thread job on comment payloads), covering identity, review-state/findings gating, dedup trust, single-pass, and visible permission failures. - Skip the request job gracefully (warning annotation) when CODEX_TRIGGER_TOKEN is unset, so the check is not permanently red before the secret is created. - Scope the thread-resolution concurrency group per comment id so a burst of disposition replies cannot evict queued thread resolutions. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01NrdqyvKzaovZUX9haKfgjM --- .../codex-autofix-review-comments.yml | 13 +- scripts/check-codex-autofix-workflow.mjs | 72 ++- tests/codex-autofix-workflow.test.ts | 464 +++++++++++------- 3 files changed, 347 insertions(+), 202 deletions(-) diff --git a/.github/workflows/codex-autofix-review-comments.yml b/.github/workflows/codex-autofix-review-comments.yml index 38d9ded7d..d643a4d23 100644 --- a/.github/workflows/codex-autofix-review-comments.yml +++ b/.github/workflows/codex-autofix-review-comments.yml @@ -26,8 +26,16 @@ jobs: concurrency: group: codex-autoresolve-${{ github.event.pull_request.number }} cancel-in-progress: false + env: + # Assigned to job env so a step-level `if` can detect an unconfigured + # secret and skip gracefully instead of hard-failing the check. + CODEX_TRIGGER_TOKEN: ${{ secrets.CODEX_TRIGGER_TOKEN }} steps: + - name: Warn when the trigger token is not configured + if: ${{ env.CODEX_TRIGGER_TOKEN == '' }} + run: echo "::warning title=Codex auto-resolve::CODEX_TRIGGER_TOKEN secret is not configured; skipping the Codex auto-resolve request. Codex will not be asked to resolve findings until this fine-grained PAT secret is set." - name: Ask Codex to resolve review comments + if: ${{ env.CODEX_TRIGGER_TOKEN != '' }} uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: # A fine-grained PAT from a real (non-bot) account. The Codex connector @@ -181,8 +189,11 @@ jobs: permissions: contents: read pull-requests: write + # Each reply resolves a distinct thread, so key concurrency per comment — + # a per-PR group would cap at one running + one pending and drop resolutions + # during a burst of disposition replies. concurrency: - group: codex-autoresolve-thread-${{ github.event.pull_request.number }} + group: codex-autoresolve-thread-${{ github.event.pull_request.number }}-${{ github.event.comment.id }} cancel-in-progress: false steps: - name: Resolve Codex review thread on disposition marker diff --git a/scripts/check-codex-autofix-workflow.mjs b/scripts/check-codex-autofix-workflow.mjs index c593b2ec5..dd881041b 100644 --- a/scripts/check-codex-autofix-workflow.mjs +++ b/scripts/check-codex-autofix-workflow.mjs @@ -18,10 +18,6 @@ const forbiddenPatterns = [ pattern: /^\s*issue_comment:/m, message: "Do not trigger Codex auto-resolve from issue_comment events.", }, - { - pattern: /^\s*pull_request_review:/m, - message: "Do not trigger Codex auto-resolve from whole pull_request_review events.", - }, { pattern: /@codex resolve all review comments/, message: "Do not use the broad Codex resolve-all command; use the scoped actionable-findings command.", @@ -29,11 +25,12 @@ const forbiddenPatterns = [ { pattern: /contains\(\s*github\.event\.comment\.user\.login/, message: - "Do not authorize the Codex connector with a substring login match; require an exact trusted bot identity.", + "Do not authorize the Codex connector with a substring comment-login match; require an exact trusted bot identity.", }, { - pattern: /sourceBody\.includes\("codex-autoresolve(?:-pr)?:"\)/, - message: "Do not treat an auto-resolve marker mentioned anywhere in a review finding as a self-triggered request.", + pattern: /contains\(\s*github\.event\.review\.user\.login/, + message: + "Do not authorize the Codex connector with a substring review-login match; require an exact trusted bot identity.", }, { pattern: /existingComments\.some\(\(comment\) => \(comment\.body \|\| ""\)\.includes\(marker\)\)/, @@ -42,7 +39,7 @@ const forbiddenPatterns = [ { pattern: /^concurrency:/m, message: - "Do not apply Codex auto-resolve concurrency to the whole workflow; unrelated review comments must not displace an authorized pending job.", + "Do not apply Codex auto-resolve concurrency to the whole workflow; unrelated events must not displace an authorized pending job.", }, { pattern: /^\s*contents:\s*write\s*$/m, @@ -97,12 +94,14 @@ for (const [path, contents, requiredCheck] of requiredInstructionChecks) { } } +// The auto-resolve request must fire only after a completed Codex review; thread +// closure is driven separately by trusted review-comment disposition replies. const requiredTriggerAndPermissionChecks = [ + " pull_request_review:", + " types: [submitted]", " pull_request_review_comment:", " types: [created]", " contents: read", - " issues: write", - " models: read", " pull-requests: write", `uses: actions/github-script@${githubScriptPin} # v9.0.0`, "github.event.pull_request.state == 'open'", @@ -114,6 +113,29 @@ for (const requiredCheck of requiredTriggerAndPermissionChecks) { } } +// The two jobs must be gated by event name so the request path and the +// thread-resolution path never handle each other's events. +const requiredEventGateChecks = [ + "github.event_name == 'pull_request_review'", + "github.event_name == 'pull_request_review_comment'", +]; + +for (const requiredCheck of requiredEventGateChecks) { + if (!workflow.includes(requiredCheck)) { + failures.push(`Codex auto-resolve workflow is missing an event-name gate: ${requiredCheck}`); + } +} + +// The request must be posted by a real (non-bot) identity the Codex connector +// will act on, and thread closure must use the workflow's own scoped token. +const requiredTokenChecks = ["github-token: ${{ secrets.CODEX_TRIGGER_TOKEN }}", "github-token: ${{ github.token }}"]; + +for (const requiredCheck of requiredTokenChecks) { + if (!workflow.includes(requiredCheck)) { + failures.push(`Codex auto-resolve workflow is missing a required github-token binding: ${requiredCheck}`); + } +} + const requiredConcurrencyChecks = [ " concurrency:", " group: codex-autoresolve-${{ github.event.pull_request.number }}", @@ -130,7 +152,14 @@ if (!workflow.includes("codex-autoresolve-pr:${pr.number}")) { failures.push("Codex auto-resolve marker must be scoped to the pull request for a single lifetime pass."); } +// Both the request path (submitted review author) and the thread path (review +// comment author) must match the trusted connector by exact login and bot type. const requiredIdentityChecks = [ + "github.event.review.user.type == 'Bot'", + "github.event.review.user.login == 'chatgpt-codex-connector'", + "github.event.review.user.login == 'chatgpt-codex-connector[bot]'", + 'review.user?.type !== "Bot"', + "!allowedCodexBotLogins.has(review.user.login)", "github.event.comment.user.type == 'Bot'", "github.event.comment.user.login == 'chatgpt-codex-connector'", "github.event.comment.user.login == 'chatgpt-codex-connector[bot]'", @@ -144,15 +173,18 @@ for (const requiredCheck of requiredIdentityChecks) { } } -const requiredSelfTriggerChecks = [ - 'sourceBody.startsWith("\n\nFixed.", + id: 99, + in_reply_to_id: 41, + user: { login: "chatgpt-codex-connector[bot]", type: "Bot" }, + ...options?.comment, + }; + + const github = { + graphql: async (query: string, variables: Record) => { + graphqlCalls.push({ query, variables }); + if (options?.graphqlError !== undefined) throw options.graphqlError; + return options?.graphqlResults?.[graphqlCalls.length - 1] ?? {}; + }, + }; + + await threadScript( + github, + { + payload: { + comment, + pull_request: { head: { sha: "head-sha-4" }, number: 42, state: "open" }, + }, + repo: { owner: "clinical-kb", repo: "database" }, + }, + { + notice: (message) => notices.push(message), + setFailed: (message) => failures.push(message), + warning: (message) => warnings.push(message), + }, + ); + + return { failures, graphqlCalls, notices, warnings }; } function runGuard(workflow: string) { @@ -180,6 +256,29 @@ describe("Codex auto-resolve workflow guard", () => { expect(result.output).toContain("Codex auto-resolve workflow guard passed."); }); + it("rejects an issue_comment trigger", () => { + const workflow = originalWorkflow.replace( + " pull_request_review:\n types: [submitted]\n", + " pull_request_review:\n types: [submitted]\n issue_comment:\n types: [created]\n", + ); + expect(workflow).not.toBe(originalWorkflow); + + const result = runGuard(workflow); + + expect(result.status).toBe(1); + expect(result.output).toContain("issue_comment"); + }); + + it("rejects removing the submitted-review trigger", () => { + const workflow = originalWorkflow.replace(" pull_request_review:\n types: [submitted]\n", ""); + expect(workflow).not.toBe(originalWorkflow); + + const result = runGuard(workflow); + + expect(result.status).toBe(1); + expect(result.output).toContain("types: [submitted]"); + }); + it("rejects substring-based connector authorization", () => { const workflow = originalWorkflow.replace( ` github.event.comment.user.type == 'Bot' && @@ -192,15 +291,40 @@ describe("Codex auto-resolve workflow guard", () => { const result = runGuard(workflow); expect(result.status).toBe(1); - expect(result.output).toContain("substring login match"); + expect(result.output).toContain("substring comment-login match"); + }); + + it("rejects a bot-authored trigger token", () => { + const workflow = originalWorkflow.replace( + " github-token: ${{ secrets.CODEX_TRIGGER_TOKEN }}", + " github-token: ${{ github.token }}", + ); + expect(workflow).not.toBe(originalWorkflow); + + const result = runGuard(workflow); + + expect(result.status).toBe(1); + expect(result.output).toContain("secrets.CODEX_TRIGGER_TOKEN"); + }); + + it("rejects dropping the completed-review findings gate", () => { + const workflow = originalWorkflow.replace( + ` if (reviewComments.length === 0) {`, + ` if (false) {`, + ); + expect(workflow).not.toBe(originalWorkflow); + + const result = runGuard(workflow); + + expect(result.status).toBe(1); + expect(result.output).toContain("completed-review findings gate"); }); - it("rejects duplicate markers from arbitrary commenters", () => { + it("rejects duplicate markers trusted from arbitrary commenters", () => { const workflow = originalWorkflow.replace( ` const trustedExistingRequests = existingComments.filter( (comment) => - comment.user?.type === "Bot" && - comment.user.login === "github-actions[bot]" && + comment.user?.login === triggerLogin && (comment.body || "").trimStart().startsWith("", @@ -312,26 +418,13 @@ describe("Codex auto-resolve workflow script", () => { expect(result.createdComments[0]?.body).toContain(""); }); - it("honors a deduplication marker posted by GitHub Actions", async () => { - const result = await runWorkflowScript({ + it("honors a deduplication marker posted by the trigger-token account", async () => { + const result = await runRequestScript({ + triggerLogin: "codex-trigger-bot", existingComments: [ { body: "", - user: { login: "github-actions[bot]", type: "Bot" }, - }, - ], - }); - - expect(result.createdComments).toHaveLength(0); - expect(result.notices).toContainEqual(expect.stringContaining("Skipping duplicate")); - }); - - it("does not create a follow-up repair pass when Codex reviews a new pull request head", async () => { - const result = await runWorkflowScript({ - existingComments: [ - { - body: "\n\n", - user: { login: "github-actions[bot]", type: "Bot" }, + user: { login: "codex-trigger-bot", type: "Bot" }, }, ], }); @@ -340,12 +433,13 @@ describe("Codex auto-resolve workflow script", () => { expect(result.notices).toContainEqual(expect.stringContaining("Skipping duplicate")); }); - it("stops automatic repair after one trusted legacy request", async () => { - const result = await runWorkflowScript({ + it("stops automatic repair after one trusted request for another head", async () => { + const result = await runRequestScript({ + triggerLogin: "codex-trigger-bot", existingComments: [ { body: "", - user: { login: "github-actions[bot]", type: "Bot" }, + user: { login: "codex-trigger-bot", type: "Bot" }, }, ], }); @@ -355,7 +449,7 @@ describe("Codex auto-resolve workflow script", () => { }); it("emits a single-pass prompt with the thread disposition marker", async () => { - const result = await runWorkflowScript(); + const result = await runRequestScript(); expect(result.createdComments).toHaveLength(1); expect(result.createdComments[0]?.body).toContain("single automatic repair pass"); @@ -363,31 +457,71 @@ describe("Codex auto-resolve workflow script", () => { expect(result.createdComments[0]?.body).toContain("do not perform a fresh review"); }); - it("does not skip a finding that merely quotes the marker and command", async () => { - const result = await runWorkflowScript({ - reviewComment: { - body: [ - "This finding quotes in the middle of a sentence.", - "It also quotes @codex resolve actionable Codex review findings for this pull request and current head.", - ].join(" "), - }, - }); + it("fails visibly when it cannot identify the trigger-token account", async () => { + const result = await runRequestScript({ getAuthenticatedError: new Error("no user") }); + + expect(result.createdComments).toHaveLength(0); + expect(result.failures).toContainEqual(expect.stringContaining("could not identify the trigger token's account")); + }); + + it("fails visibly on a permission failure while listing review comments", async () => { + const result = await runRequestScript({ reviewCommentsError: { status: 403 } }); + + expect(result.createdComments).toHaveLength(0); + expect(result.failures).toContainEqual(expect.stringContaining("cannot read review comments")); + }); + + it("fails visibly on a permission failure while listing issue comments", async () => { + const result = await runRequestScript({ existingCommentsError: { status: 403 } }); + + expect(result.createdComments).toHaveLength(0); + expect(result.failures).toContainEqual(expect.stringContaining("denied permission to list issue comments")); + expect(result.warnings).toContainEqual(expect.stringContaining("denied permission to list issue comments")); + }); + + it("fails visibly on a permission failure while creating the request", async () => { + const result = await runRequestScript({ createError: { status: 403 } }); expect(result.createdComments).toHaveLength(1); + expect(result.failures).toContainEqual(expect.stringContaining("cannot comment on the pull request")); + expect(result.warnings).toContainEqual(expect.stringContaining("cannot comment on the pull request")); + }); + + it("rethrows unexpected failures while listing issue comments", async () => { + await expect(runRequestScript({ existingCommentsError: { status: 500 } })).rejects.toEqual({ status: 500 }); }); +}); - it("ignores review-thread replies without the resolved disposition marker", async () => { - const result = await runWorkflowScript({ - reviewComment: { body: "Fixed in the latest commit.", in_reply_to_id: 41 }, +describe("Codex auto-resolve thread-resolution script", () => { + it("rejects an untrusted look-alike review-comment author", async () => { + const result = await runThreadScript({ + comment: { user: { login: "attacker-chatgpt-codex-connector", type: "User" } }, + }); + + expect(result.graphqlCalls).toHaveLength(0); + expect(result.warnings).toContainEqual(expect.stringContaining("not the trusted Codex connector bot")); + }); + + it("skips a non-reply review comment", async () => { + const result = await runThreadScript({ + comment: { body: "P1: actionable finding", in_reply_to_id: undefined }, + }); + + expect(result.graphqlCalls).toHaveLength(0); + expect(result.notices).toContainEqual(expect.stringContaining("driven by the submitted review")); + }); + + it("ignores a reply without the resolved disposition marker", async () => { + const result = await runThreadScript({ + comment: { body: "Fixed in the latest commit." }, }); expect(result.graphqlCalls).toHaveLength(0); - expect(result.createdComments).toHaveLength(0); expect(result.notices).toContainEqual(expect.stringContaining("without the trusted resolved disposition marker")); }); it("resolves the exact review thread after a trusted disposition reply", async () => { - const result = await runWorkflowScript({ + const result = await runThreadScript({ graphqlResults: [ { repository: { @@ -407,10 +541,6 @@ describe("Codex auto-resolve workflow script", () => { }, { resolveReviewThread: { thread: { id: "thread-1", isResolved: true } } }, ], - reviewComment: { - body: "\n\nFixed and verified.", - in_reply_to_id: 41, - }, }); expect(result.graphqlCalls).toHaveLength(2); @@ -420,7 +550,7 @@ describe("Codex auto-resolve workflow script", () => { }); it("fails visibly when a disposition reply cannot be mapped to a review thread", async () => { - const result = await runWorkflowScript({ + const result = await runThreadScript({ graphqlResults: [ { repository: { @@ -433,45 +563,15 @@ describe("Codex auto-resolve workflow script", () => { }, }, ], - reviewComment: { - body: "\n\nDispositioned.", - in_reply_to_id: 41, - }, }); expect(result.failures).toContainEqual(expect.stringContaining("could not find the review thread")); }); it("fails visibly when GitHub rejects direct review-thread resolution", async () => { - const result = await runWorkflowScript({ - graphqlError: new Error("permission denied"), - reviewComment: { - body: "\n\nFixed.", - in_reply_to_id: 41, - }, - }); + const result = await runThreadScript({ graphqlError: new Error("permission denied") }); expect(result.failures).toContainEqual(expect.stringContaining("permission denied")); expect(result.warnings).toContainEqual(expect.stringContaining("permission denied")); }); - - it("fails visibly on a permission failure while listing comments", async () => { - const result = await runWorkflowScript({ paginateError: { status: 403 } }); - - expect(result.createdComments).toHaveLength(0); - expect(result.failures).toContainEqual(expect.stringContaining("denied permission to list issue comments")); - expect(result.warnings).toContainEqual(expect.stringContaining("denied permission to list issue comments")); - }); - - it("fails visibly on a permission failure while creating the request", async () => { - const result = await runWorkflowScript({ createError: { status: 403 } }); - - expect(result.createdComments).toHaveLength(1); - expect(result.failures).toContainEqual(expect.stringContaining("cannot comment on the pull request")); - expect(result.warnings).toContainEqual(expect.stringContaining("cannot comment on the pull request")); - }); - - it("rethrows unexpected failures while listing comments", async () => { - await expect(runWorkflowScript({ paginateError: { status: 500 } })).rejects.toEqual({ status: 500 }); - }); });