|
| 1 | +name: Codex PR Review |
| 2 | + |
| 3 | +on: |
| 4 | + pull_request: |
| 5 | + types: [opened, synchronize, reopened] |
| 6 | + issue_comment: |
| 7 | + types: [created] |
| 8 | + |
| 9 | +concurrency: |
| 10 | + group: codex-review-${{ github.event.pull_request.number || github.event.issue.number }} |
| 11 | + cancel-in-progress: true |
| 12 | + |
| 13 | +jobs: |
| 14 | + authorize: |
| 15 | + # Trigger gate: PR events always, comment events only for "@codex review" from a |
| 16 | + # trusted commenter. The step then restricts to same-repo (non-fork) PRs so a |
| 17 | + # public fork can never reach the privileged checkout below. |
| 18 | + if: > |
| 19 | + github.event_name == 'pull_request' || |
| 20 | + (github.event.issue.pull_request != null && |
| 21 | + contains(github.event.comment.body, '@codex review') && |
| 22 | + contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) |
| 23 | + runs-on: ubuntu-latest |
| 24 | + permissions: |
| 25 | + pull-requests: read |
| 26 | + outputs: |
| 27 | + ok: ${{ steps.gate.outputs.ok }} |
| 28 | + pr: ${{ steps.gate.outputs.pr }} |
| 29 | + steps: |
| 30 | + - id: gate |
| 31 | + uses: actions/github-script@v7 |
| 32 | + with: |
| 33 | + github-token: ${{ github.token }} |
| 34 | + script: | |
| 35 | + const prNum = context.payload.pull_request?.number ?? context.payload.issue?.number; |
| 36 | + if (!prNum) { core.setOutput('ok', 'false'); return; } |
| 37 | + const { data: pr } = await github.rest.pulls.get({ |
| 38 | + owner: context.repo.owner, repo: context.repo.repo, pull_number: prNum, |
| 39 | + }); |
| 40 | + const sameRepo = !!pr.head.repo && pr.head.repo.full_name === pr.base.repo.full_name; |
| 41 | + core.setOutput('pr', String(prNum)); |
| 42 | + core.setOutput('ok', sameRepo ? 'true' : 'false'); |
| 43 | + if (!sameRepo) { |
| 44 | + core.notice(`Skipping Codex review: PR #${prNum} is from a fork; only same-repo branches are reviewed.`); |
| 45 | + } |
| 46 | +
|
| 47 | + review: |
| 48 | + needs: authorize |
| 49 | + if: needs.authorize.outputs.ok == 'true' |
| 50 | + runs-on: ubuntu-latest |
| 51 | + permissions: |
| 52 | + contents: read |
| 53 | + outputs: |
| 54 | + result: ${{ steps.run_codex.outputs.final-message }} |
| 55 | + steps: |
| 56 | + - uses: actions/checkout@v4 |
| 57 | + with: |
| 58 | + ref: refs/pull/${{ needs.authorize.outputs.pr }}/merge |
| 59 | + fetch-depth: 0 |
| 60 | + persist-credentials: false |
| 61 | + |
| 62 | + - name: Run Codex |
| 63 | + id: run_codex |
| 64 | + uses: openai/codex-action@v1 |
| 65 | + with: |
| 66 | + openai-api-key: ${{ secrets.OPENAI_API_KEY }} |
| 67 | + model: gpt-5.5 |
| 68 | + effort: xhigh |
| 69 | + sandbox: read-only |
| 70 | + output-schema: | |
| 71 | + { |
| 72 | + "type": "object", |
| 73 | + "additionalProperties": false, |
| 74 | + "properties": { |
| 75 | + "summary": { "type": "string" }, |
| 76 | + "findings": { |
| 77 | + "type": "array", |
| 78 | + "items": { |
| 79 | + "type": "object", |
| 80 | + "additionalProperties": false, |
| 81 | + "properties": { |
| 82 | + "path": { "type": "string" }, |
| 83 | + "line": { "type": "integer" }, |
| 84 | + "severity": { "type": "string", "enum": ["blocking", "consider"] }, |
| 85 | + "comment": { "type": "string" } |
| 86 | + }, |
| 87 | + "required": ["path", "line", "severity", "comment"] |
| 88 | + } |
| 89 | + } |
| 90 | + }, |
| 91 | + "required": ["summary", "findings"] |
| 92 | + } |
| 93 | + prompt: | |
| 94 | + You are reviewing pull request #${{ needs.authorize.outputs.pr }} in ${{ github.repository }}. |
| 95 | +
|
| 96 | + The PR's changes are exactly the diff between the merge commit's two parents. |
| 97 | + Run `git diff HEAD^1 HEAD^2` to see everything that changed, and |
| 98 | + `git diff HEAD^1 HEAD^2 -- <path>` to focus on a single file. |
| 99 | +
|
| 100 | + Review ONLY those changes. Report high-signal findings only. |
| 101 | +
|
| 102 | + Correctness & safety: |
| 103 | + - logic errors, unhandled edge cases, broken assumptions |
| 104 | + - security vulnerabilities |
| 105 | + - data loss, concurrency hazards, resource leaks |
| 106 | +
|
| 107 | + Design & code quality: |
| 108 | + - the soundness of the overall approach, not just line-level bugs |
| 109 | + - elegance: is there a simpler, cleaner way to achieve the same result? |
| 110 | + - abstraction: prefer the most general clean abstraction that fits the problem, |
| 111 | + without over-engineering for cases that don't exist |
| 112 | + - redundancy: flag duplicated logic, dead code, and anything that violates DRY |
| 113 | +
|
| 114 | + Skip pure formatting and style nits. |
| 115 | +
|
| 116 | + Report at most the 5 most important findings. Consolidate an issue that |
| 117 | + recurs in several places into one finding at the most representative location. |
| 118 | +
|
| 119 | + Output JSON matching the provided schema: |
| 120 | + - `summary`: one or two sentences on the PR overall. If there are no real |
| 121 | + issues, set summary to "No issues found." and findings to []. |
| 122 | + - `findings[].path`: repository-relative file path, exactly as git reports it. |
| 123 | + - `findings[].line`: the line number in the NEW (post-change) version of the |
| 124 | + file. It MUST be a line the PR adds or modifies. |
| 125 | + - `findings[].severity`: "blocking" or "consider". |
| 126 | + - `findings[].comment`: markdown review comment with a short code snippet and a |
| 127 | + concrete fix. |
| 128 | +
|
| 129 | + post_review: |
| 130 | + needs: [authorize, review] |
| 131 | + if: needs.review.outputs.result != '' |
| 132 | + runs-on: ubuntu-latest |
| 133 | + permissions: |
| 134 | + issues: write |
| 135 | + pull-requests: write |
| 136 | + steps: |
| 137 | + - name: Post inline review |
| 138 | + uses: actions/github-script@v7 |
| 139 | + env: |
| 140 | + CODEX_RESULT: ${{ needs.review.outputs.result }} |
| 141 | + PR_NUMBER: ${{ needs.authorize.outputs.pr }} |
| 142 | + with: |
| 143 | + github-token: ${{ github.token }} |
| 144 | + script: | |
| 145 | + const { owner, repo } = context.repo; |
| 146 | + const pull_number = Number(process.env.PR_NUMBER); |
| 147 | + const SUMMARY_MARKER = '<!-- codex-review-summary -->'; |
| 148 | + const INLINE_MARKER = '<!-- codex-review-inline -->'; |
| 149 | + const MAX_COMMENTS = 5; |
| 150 | +
|
| 151 | + // Parse Codex JSON. With --output-schema the result is already pure JSON, |
| 152 | + // and its findings may contain fenced code blocks, so never grab an inner |
| 153 | + // fence: parse the whole string first, then a fence wrapping the whole |
| 154 | + // string, then fall back to the outermost braces. |
| 155 | + function parseResult(raw) { |
| 156 | + if (!raw) return null; |
| 157 | + const tryParse = (s) => { try { return JSON.parse(s); } catch { return null; } }; |
| 158 | + const trimmed = raw.trim(); |
| 159 | + let out = tryParse(trimmed); |
| 160 | + if (out) return out; |
| 161 | + const fence = trimmed.match(/^```(?:json)?\s*\n?([\s\S]*?)\n?```$/); |
| 162 | + if (fence) { out = tryParse(fence[1].trim()); if (out) return out; } |
| 163 | + const a = trimmed.indexOf('{'), b = trimmed.lastIndexOf('}'); |
| 164 | + if (a !== -1 && b > a) return tryParse(trimmed.slice(a, b + 1)); |
| 165 | + return null; |
| 166 | + } |
| 167 | + const result = parseResult(process.env.CODEX_RESULT); |
| 168 | + if (!result) { core.setFailed('Could not parse Codex output as JSON.'); return; } |
| 169 | +
|
| 170 | + const summary = (result.summary || '').trim(); |
| 171 | + const findings = (Array.isArray(result.findings) ? result.findings : []) |
| 172 | + .slice(0, MAX_COMMENTS); |
| 173 | +
|
| 174 | + // Build the set of (path -> commentable new-file line numbers) from the diff. |
| 175 | + const pr = await github.rest.pulls.get({ owner, repo, pull_number }); |
| 176 | + const headSha = pr.data.head.sha; |
| 177 | + const files = await github.paginate(github.rest.pulls.listFiles, { |
| 178 | + owner, repo, pull_number, per_page: 100, |
| 179 | + }); |
| 180 | + const commentable = new Map(); |
| 181 | + for (const f of files) { |
| 182 | + if (!f.patch) continue; |
| 183 | + const lines = new Set(); |
| 184 | + let newLine = 0; |
| 185 | + for (const ln of f.patch.split('\n')) { |
| 186 | + const h = ln.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/); |
| 187 | + if (h) { newLine = parseInt(h[1], 10); continue; } |
| 188 | + if (ln.startsWith('\\')) continue; // "\ No newline at end of file" |
| 189 | + if (ln.startsWith('+')) { lines.add(newLine); newLine++; } |
| 190 | + else if (ln.startsWith('-')) { /* removed line, no new-side number */ } |
| 191 | + else { newLine++; } // context line |
| 192 | + } |
| 193 | + commentable.set(f.filename, lines); |
| 194 | + } |
| 195 | +
|
| 196 | + // Split findings into inline-able vs. orphans (lines not in the diff). |
| 197 | + const inline = [], orphans = []; |
| 198 | + for (const fnd of findings) { |
| 199 | + const sev = (fnd.severity || 'consider').toUpperCase(); |
| 200 | + const set = commentable.get(fnd.path); |
| 201 | + if (set && set.has(fnd.line)) { |
| 202 | + inline.push({ |
| 203 | + path: fnd.path, line: fnd.line, side: 'RIGHT', |
| 204 | + body: `${INLINE_MARKER}\n**${sev}** ${fnd.comment}`, |
| 205 | + }); |
| 206 | + } else { |
| 207 | + orphans.push({ ...fnd, sev }); |
| 208 | + } |
| 209 | + } |
| 210 | +
|
| 211 | + // Always clear our prior inline comments first, so findings resolved in a |
| 212 | + // later push disappear even when this run produces no inline comments. |
| 213 | + try { |
| 214 | + const prior = await github.paginate(github.rest.pulls.listReviewComments, { |
| 215 | + owner, repo, pull_number, per_page: 100, |
| 216 | + }); |
| 217 | + for (const c of prior) { |
| 218 | + if (c.body && c.body.includes(INLINE_MARKER)) { |
| 219 | + try { await github.rest.pulls.deleteReviewComment({ owner, repo, comment_id: c.id }); } |
| 220 | + catch {} |
| 221 | + } |
| 222 | + } |
| 223 | + } catch (e) { core.warning(`Could not clean prior inline comments: ${e.message}`); } |
| 224 | +
|
| 225 | + // Post this run's inline comments. If it fails, fold them into the summary. |
| 226 | + let inlinePosted = false; |
| 227 | + if (inline.length) { |
| 228 | + try { |
| 229 | + await github.rest.pulls.createReview({ |
| 230 | + owner, repo, pull_number, commit_id: headSha, |
| 231 | + event: 'COMMENT', comments: inline, |
| 232 | + }); |
| 233 | + inlinePosted = true; |
| 234 | + } catch (err) { |
| 235 | + core.warning(`Inline review failed (${err.status || ''}); folding into the summary.`); |
| 236 | + } |
| 237 | + } |
| 238 | +
|
| 239 | + // Build the rolling summary comment. |
| 240 | + const leftover = inlinePosted |
| 241 | + ? orphans |
| 242 | + : findings.map(f => ({ ...f, sev: (f.severity || 'consider').toUpperCase() })); |
| 243 | + let body = `${SUMMARY_MARKER}\n### Codex review\n\n` + |
| 244 | + (summary || (findings.length ? 'See inline comments.' : 'No issues found.')); |
| 245 | + if (leftover.length) { |
| 246 | + body += `\n\n**${inlinePosted ? 'Findings not on changed lines' : 'Findings'}:**\n`; |
| 247 | + for (const o of leftover) body += `\n- \`${o.path}:${o.line}\` **${o.sev}** ${o.comment}`; |
| 248 | + } |
| 249 | +
|
| 250 | + // Upsert one rolling summary comment instead of stacking on each push. |
| 251 | + const comments = await github.paginate(github.rest.issues.listComments, { |
| 252 | + owner, repo, issue_number: pull_number, per_page: 100, |
| 253 | + }); |
| 254 | + const existing = comments.find(c => c.body && c.body.includes(SUMMARY_MARKER)); |
| 255 | + if (existing) { |
| 256 | + await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }); |
| 257 | + } else { |
| 258 | + await github.rest.issues.createComment({ owner, repo, issue_number: pull_number, body }); |
| 259 | + } |
0 commit comments