codex-review-post.yml

name: Codex Review

# Stage 2 of the two-stage Codex PR review.
#
# Triggered by the completion of Stage 1 (codex-review.yml) via `workflow_run`,
# so it runs in the BASE repository context WITH access to secrets — even for
# fork PRs. It NEVER checks out PR code: it only reads the pre-computed diff
# artifact from Stage 1, so untrusted fork code can never execute next to the
# OpenAI key. That structural guarantee is what makes reviewing fork PRs safe.
#
# Residual risk: Codex still reads the diff as *data*, so a prompt-injection
# payload in the diff could in principle try to coax the key into a review
# comment (the action keeps the key behind a proxy, but read-only Codex can reach
# process memory). Use a scoped, low-limit OPENAI_API_KEY for this workflow.

on:
  workflow_run:
    workflows: ["Codex Review (collect diff)"]
    types: [completed]

permissions:
  contents: read

concurrency:
  # Serialize per PR source (repo + branch), not per SHA, so a newer push cancels
  # an in-flight older review instead of racing it to post stale findings.
  group: codex-review-post-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }}
  cancel-in-progress: true

jobs:
  review:
    if: github.event.workflow_run.conclusion == 'success'
    runs-on: ubuntu-latest
    permissions:
      contents: read
      actions: read          # download-artifact needs this to read another run's artifact
      pull-requests: write   # covers PR review comments AND the PR conversation comment
    steps:
      - name: Download Stage 1 diff artifact
        uses: actions/download-artifact@v4
        with:
          name: codex-review-payload
          path: codex-payload
          run-id: ${{ github.event.workflow_run.id }}
          github-token: ${{ github.token }}

      - name: Resolve and verify PR
        id: pr
        uses: actions/github-script@v7
        env:
          RUN_HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
        with:
          github-token: ${{ github.token }}
          script: |
            const fs = require('fs');
            const raw = fs.readFileSync('codex-payload/pr-number.txt', 'utf8').trim();
            if (!/^\d+$/.test(raw)) {
              core.setFailed(`Invalid PR number in artifact: "${raw}"`);
              return;
            }
            const pull_number = Number(raw);
            const { owner, repo } = context.repo;
            const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number });
            // Bind the artifact to a real PR: the triggering run's head_sha is set by
            // GitHub (not the fork), so requiring it to equal the PR head prevents a
            // tampered artifact from redirecting the review onto an unrelated PR.
            if (pr.head.sha !== process.env.RUN_HEAD_SHA) {
              core.setFailed(
                `PR #${pull_number} head ${pr.head.sha} != run head ${process.env.RUN_HEAD_SHA}; refusing to post.`,
              );
              return;
            }
            // Re-apply Stage 1's gate against the PR's *current* state: it may have
            // been closed or converted to draft since the diff was collected. Keeps
            // posting behavior consistent with what gets reviewed.
            if (pr.state !== 'open' || pr.draft || pr.user?.type === 'Bot') {
              core.notice(`PR #${pull_number} is not an open, ready, human PR; skipping review.`);
              core.setOutput('skip', 'true');
              return;
            }
            core.setOutput('skip', 'false');
            core.setOutput('number', String(pull_number));

      - name: Prepare working directory
        if: steps.pr.outputs.skip != 'true'
        run: |
          set -euo pipefail
          # codex-action runs `codex exec --cd <working-directory>`; give it a valid
          # (empty) git repo containing ONLY the diff — never any PR code.
          cd codex-payload
          # Allow an empty diff (e.g. a push reverted all changes) so the post step
          # still clears stale prior findings; only a missing file is an error.
          test -f pr.diff || { echo "pr.diff missing"; exit 1; }
          git init -q

      - name: Run Codex on the diff
        id: run_codex
        if: steps.pr.outputs.skip != 'true'
        # Pinned to the v1 commit SHA: this privileged job holds OPENAI_API_KEY, so a
        # moved/compromised tag must not silently change what runs here.
        uses: openai/codex-action@52fe01ec70a42f454c9d2ebd47598f9fd6893d56 # v1
        with:
          openai-api-key: ${{ secrets.OPENAI_API_KEY }}
          # This job reviews ALL PRs incl. forks; the action's default write-access
          # gate would otherwise reject fork/non-collaborator authors. Safe to open
          # here because security comes from this job never checking out PR code
          # (only the diff artifact), not from the actor allowlist.
          allow-users: "*"
          model: gpt-5.5
          effort: xhigh
          sandbox: read-only
          working-directory: codex-payload
          output-schema: |
            {
              "type": "object",
              "additionalProperties": false,
              "properties": {
                "summary": { "type": "string" },
                "findings": {
                  "type": "array",
                  "items": {
                    "type": "object",
                    "additionalProperties": false,
                    "properties": {
                      "path": { "type": "string" },
                      "line": { "type": "integer" },
                      "severity": { "type": "string", "enum": ["blocking", "consider"] },
                      "comment": { "type": "string" }
                    },
                    "required": ["path", "line", "severity", "comment"]
                  }
                }
              },
              "required": ["summary", "findings"]
            }
          prompt: |
            You are reviewing a pull request in ${{ github.repository }}.

            The complete set of changes is the unified diff in the file `pr.diff` in
            your working directory (run `cat pr.diff`). It is a `git diff` of
            base...head. Review ONLY those changes. Report high-signal findings only.

            Correctness & safety:
            - logic errors, unhandled edge cases, broken assumptions
            - security vulnerabilities
            - data loss, concurrency hazards, resource leaks

            Design & code quality:
            - the soundness of the overall approach, not just line-level bugs
            - elegance: is there a simpler, cleaner way to achieve the same result?
            - abstraction: prefer the most general clean abstraction that fits the problem,
              without over-engineering for cases that don't exist
            - redundancy: flag duplicated logic, dead code, and anything that violates DRY

            Skip pure formatting and style nits.

            Report at most the 5 most important findings. Consolidate an issue that
            recurs in several places into one finding at the most representative location.

            Output JSON matching the provided schema:
            - `summary`: one or two sentences on the PR overall. If there are no real
              issues, set summary to "No issues found." and findings to [].
            - `findings[].path`: repository-relative file path, exactly as it appears in
              the diff (the path after `+++ b/`).
            - `findings[].line`: the line number in the NEW (post-change) version of the
              file. It MUST be a line the PR adds or modifies.
            - `findings[].severity`: "blocking" or "consider".
            - `findings[].comment`: markdown review comment with a short code snippet and a
              concrete fix.

      - name: Post inline review
        if: steps.pr.outputs.skip != 'true' && steps.run_codex.outputs.final-message != ''
        uses: actions/github-script@v7
        env:
          CODEX_RESULT: ${{ steps.run_codex.outputs.final-message }}
          PR_NUMBER: ${{ steps.pr.outputs.number }}
          REVIEWED_SHA: ${{ github.event.workflow_run.head_sha }}
        with:
          github-token: ${{ github.token }}
          script: |
            const { owner, repo } = context.repo;
            const pull_number = Number(process.env.PR_NUMBER);
            const SUMMARY_MARKER = '<!-- codex-review-summary -->';
            const INLINE_MARKER = '<!-- codex-review-inline -->';
            const MAX_COMMENTS = 5;
            const BOT = 'github-actions[bot]'; // only ever touch comments we authored

            // Parse Codex JSON. With --output-schema the result is already pure JSON,
            // and its findings may contain fenced code blocks, so never grab an inner
            // fence: parse the whole string first, then a fence wrapping the whole
            // string, then fall back to the outermost braces.
            function parseResult(raw) {
              if (!raw) return null;
              const tryParse = (s) => { try { return JSON.parse(s); } catch { return null; } };
              const trimmed = raw.trim();
              let out = tryParse(trimmed);
              if (out) return out;
              const fence = trimmed.match(/^```(?:json)?\s*\n?([\s\S]*?)\n?```$/);
              if (fence) { out = tryParse(fence[1].trim()); if (out) return out; }
              const a = trimmed.indexOf('{'), b = trimmed.lastIndexOf('}');
              if (a !== -1 && b > a) return tryParse(trimmed.slice(a, b + 1));
              return null;
            }
            const result = parseResult(process.env.CODEX_RESULT);
            if (!result) { core.setFailed('Could not parse Codex output as JSON.'); return; }

            const summary = (result.summary || '').trim();
            const findings = (Array.isArray(result.findings) ? result.findings : [])
              .slice(0, MAX_COMMENTS);

            // Build the set of (path -> commentable new-file line numbers) from the diff.
            const pr = await github.rest.pulls.get({ owner, repo, pull_number });
            const headSha = pr.data.head.sha;
            // If the PR advanced past the commit we reviewed, a newer run is already
            // handling the new head — don't post findings generated from a stale diff.
            if (headSha !== process.env.REVIEWED_SHA) {
              core.notice(`PR head moved ${process.env.REVIEWED_SHA} -> ${headSha}; skipping stale review.`);
              return;
            }
            const files = await github.paginate(github.rest.pulls.listFiles, {
              owner, repo, pull_number, per_page: 100,
            });
            const commentable = new Map();
            for (const f of files) {
              if (!f.patch) continue;
              const lines = new Set();
              let newLine = 0;
              for (const ln of f.patch.split('\n')) {
                const h = ln.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
                if (h) { newLine = parseInt(h[1], 10); continue; }
                if (ln.startsWith('\\')) continue;          // "\ No newline at end of file"
                if (ln.startsWith('+')) { lines.add(newLine); newLine++; }
                else if (ln.startsWith('-')) { /* removed line, no new-side number */ }
                else { newLine++; }                          // context line
              }
              commentable.set(f.filename, lines);
            }

            // Split findings into inline-able vs. orphans (lines not in the diff).
            const inline = [], orphans = [];
            for (const fnd of findings) {
              const sev = (fnd.severity || 'consider').toUpperCase();
              const set = commentable.get(fnd.path);
              if (set && set.has(fnd.line)) {
                inline.push({
                  path: fnd.path, line: fnd.line, side: 'RIGHT',
                  body: `${INLINE_MARKER}\n**${sev}** ${fnd.comment}`,
                });
              } else {
                orphans.push({ ...fnd, sev });
              }
            }

            // Always clear our prior inline comments first, so findings resolved in a
            // later push disappear even when this run produces no inline comments.
            try {
              const prior = await github.paginate(github.rest.pulls.listReviewComments, {
                owner, repo, pull_number, per_page: 100,
              });
              for (const c of prior) {
                if (c.user?.login === BOT && c.body && c.body.includes(INLINE_MARKER)) {
                  try { await github.rest.pulls.deleteReviewComment({ owner, repo, comment_id: c.id }); }
                  catch {}
                }
              }
            } catch (e) { core.warning(`Could not clean prior inline comments: ${e.message}`); }

            // Post this run's inline comments. If it fails, fold them into the summary.
            let inlinePosted = false;
            if (inline.length) {
              try {
                await github.rest.pulls.createReview({
                  owner, repo, pull_number, commit_id: headSha,
                  event: 'COMMENT', comments: inline,
                });
                inlinePosted = true;
              } catch (err) {
                core.warning(`Inline review failed (${err.status || ''}); folding into the summary.`);
              }
            }

            // Build the rolling summary comment.
            const leftover = inlinePosted
              ? orphans
              : findings.map(f => ({ ...f, sev: (f.severity || 'consider').toUpperCase() }));
            let body = `${SUMMARY_MARKER}\n### Codex review\n\n` +
              (summary || (findings.length ? 'See inline comments.' : 'No issues found.'));
            if (leftover.length) {
              body += `\n\n**${inlinePosted ? 'Findings not on changed lines' : 'Findings'}:**\n`;
              for (const o of leftover) body += `\n- \`${o.path}:${o.line}\` **${o.sev}** ${o.comment}`;
            }

            // Upsert one rolling summary comment instead of stacking on each push.
            const comments = await github.paginate(github.rest.issues.listComments, {
              owner, repo, issue_number: pull_number, per_page: 100,
            });
            const existing = comments.find(c => c.user?.login === BOT && c.body && c.body.includes(SUMMARY_MARKER));
            if (existing) {
              await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body });
            } else {
              await github.rest.issues.createComment({ owner, repo, issue_number: pull_number, body });
            }