ci(codex-review): review all PRs incl forks via two-stage split

.github/workflows/codex-review-post.yml

@@ -0,0 +1,302 @@
+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 });
+            }