Skip to content

Commit 7749712

Browse files
leehanchungclaude
andauthored
ci(codex): post review as inline diff comments (#78)
## Summary Phase 2 of the codex-reviewer parity work. Stacked on #76. Until now Codex was posting a single review with all findings concatenated into the body. Claude (via the subagent architecture) anchors each finding to the specific diff line in the **Files Changed** tab. This change brings Codex to format parity: - **Prompt** now asks Codex for a single JSON object with `event`, `body`, and an array of `comments` (`path` / `line` / `side` / `body` per finding). Schema is documented inline so the model has no ambiguity. - **Submit step** parses the JSON with `jq`, builds the Reviews API payload in `jq` (so review text never round-trips through shell quoting), and posts via `gh api --input <file>`. We deliberately do NOT use `-f event=...` flags because `gh api --input` routes those into the query string, not the body — codex itself flagged that exact mistake in PR #61's review of the slash-command migration. - **Defensive cleanup**: strip leading/trailing ` ```json ` fences before parsing (codex sometimes wraps JSON in a fence despite the explicit instruction); validate the event value and fail loudly if it's outside `APPROVE` / `COMMENT` / `REQUEST_CHANGES`. Once this lands, Codex reviews show up inline in the Files Changed tab next to the relevant lines, exactly like Claude's. ## Test plan - [ ] CI on this PR posts a review with at least one inline comment anchored to a specific line in `.github/workflows/codex-code-review.yml` (or summary-only if Codex thinks the change is clean) - [ ] No 422 from the Reviews API (would surface as a workflow failure) ## Stacking note Base branch is `ci/codex-app-token` (PR #76). Once #76 merges, this PR's base auto-rebases to `main` and CI re-runs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d28dce9 commit 7749712

1 file changed

Lines changed: 88 additions & 29 deletions

File tree

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

Lines changed: 88 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -176,26 +176,53 @@ jobs:
176176
id: run_codex
177177
env:
178178
PROMPT: |
179-
This is PR #${{ github.event.pull_request.number }} for ${{ github.repository }}.
179+
You are reviewing PR #${{ github.event.pull_request.number }}
180+
for ${{ github.repository }}.
180181
181-
Review ONLY the changes introduced by the PR, so consider:
182-
git log --oneline ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }}
183-
184-
Suggest any improvements, potential bugs, or issues.
185-
Be concise and specific in your feedback.
182+
Look ONLY at the changes introduced by the PR:
183+
git diff ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }}
186184
187185
Pull request title and body:
188186
----
189187
${{ github.event.pull_request.title }}
190188
${{ github.event.pull_request.body }}
189+
----
191190
192-
IMPORTANT: End your response with exactly one of these on its
193-
own line:
194-
VERDICT: APPROVE
195-
VERDICT: COMMENT
196-
197-
Use APPROVE if the code looks good with no substantive issues.
198-
Use COMMENT if you found issues worth addressing.
191+
Output a single JSON object matching this schema, with
192+
no markdown fence and no surrounding prose:
193+
194+
{
195+
"event": "APPROVE" | "COMMENT" | "REQUEST_CHANGES",
196+
"body": "<one short paragraph summarizing the review>",
197+
"comments": [
198+
{
199+
"path": "<repo-relative file path that exists in the diff>",
200+
"line": <integer line number in the file's NEW (head) version>,
201+
"side": "RIGHT",
202+
"body": "<concise comment scoped to this single line>"
203+
}
204+
]
205+
}
206+
207+
Rules:
208+
- Use APPROVE only if there is nothing worth flagging.
209+
- Use COMMENT for advisory findings; REQUEST_CHANGES only
210+
for genuine blockers.
211+
- Each entry in `comments` MUST anchor to a line that
212+
actually appears in the diff (use `git diff` to verify
213+
before emitting). Prefer fewer, sharper comments over
214+
many noisy ones.
215+
- For comments on lines REMOVED by the PR, set "side":
216+
"LEFT" and reference the line number in the OLD version.
217+
- If you have a finding that doesn't anchor to a single
218+
line (cross-file design issue, missing test coverage,
219+
etc.), put it in the top-level `body` instead of
220+
forcing a `comments` entry.
221+
- Output an empty `comments` array if there are no
222+
line-specific findings.
223+
- The output is parsed by `jq` and posted to the GitHub
224+
Reviews API verbatim — invalid JSON or invalid
225+
path/line refs will fail the workflow loudly. Be precise.
199226
run: |
200227
set -euo pipefail
201228
@@ -436,27 +463,59 @@ jobs:
436463
exit 0
437464
fi
438465
439-
# Read directly from the file — never round-trips through
440-
# $GITHUB_OUTPUT, so no delimiter-collision risk regardless
441-
# of what Codex emits.
442-
CODEX_MESSAGE=$(cat "$CODEX_OUTPUT_FILE")
443-
444-
if echo "$CODEX_MESSAGE" | grep -q "^VERDICT: APPROVE"; then
445-
EVENT="APPROVE"
446-
else
447-
EVENT="COMMENT"
466+
# Codex now emits a JSON object with `event`, `body`, and
467+
# an optional `comments` array of {path, line, side, body}
468+
# entries — the review posts as inline diff comments,
469+
# matching how anthropics/claude-code-action surfaces.
470+
# See the prompt above for the schema.
471+
#
472+
# Defensive cleanup: in practice codex sometimes wraps the
473+
# JSON in a ```json fence despite the explicit "no markdown
474+
# fence" instruction. Strip leading/trailing fences before
475+
# parsing so we don't fail on a known model quirk.
476+
RAW=$(cat "$CODEX_OUTPUT_FILE")
477+
CLEAN=$(printf '%s' "$RAW" \
478+
| sed -E 's/^```(json)?[[:space:]]*//' \
479+
| sed -E 's/[[:space:]]*```$//')
480+
481+
if ! echo "$CLEAN" | jq empty 2>/dev/null; then
482+
echo "::error::Codex output is not valid JSON. Raw output:"
483+
printf '%s\n' "$RAW"
484+
exit 1
448485
fi
449-
BODY=$(echo "$CODEX_MESSAGE" | sed '/^VERDICT: /d')
450486
451-
# gh api accepts -F field=@file for body content; using -f
452-
# with a shell var is fine here because we control the
453-
# variable expansion via bash quoting (the var is set from
454-
# `cat` and never re-interpreted).
487+
# Build the Reviews API payload in jq so the body / comment
488+
# text never round-trips through shell quoting. `--input`
489+
# below sends the file as the request body; we do NOT pass
490+
# `-f event=...` flags because `gh api --input` puts those
491+
# in the query string instead of the body (codex itself
492+
# warned about this exact pattern in an earlier review).
493+
PAYLOAD=$(mktemp -p "$RUNNER_TEMP" review.XXXXXX.json)
494+
echo "$CLEAN" \
495+
| jq --arg sha "${{ github.event.pull_request.head.sha }}" \
496+
'{commit_id: $sha, event: .event, body: .body, comments: (.comments // [])}' \
497+
> "$PAYLOAD"
498+
499+
# Sanity-check the event value before posting — GitHub
500+
# only accepts APPROVE / REQUEST_CHANGES / COMMENT and
501+
# 422s on anything else, so fail fast with a useful
502+
# message if the model freelanced.
503+
EVENT=$(jq -r .event "$PAYLOAD")
504+
case "$EVENT" in
505+
APPROVE|REQUEST_CHANGES|COMMENT) ;;
506+
*)
507+
echo "::error::Codex returned unsupported event '$EVENT'. Expected APPROVE / REQUEST_CHANGES / COMMENT."
508+
exit 1
509+
;;
510+
esac
511+
512+
NUM_COMMENTS=$(jq '.comments | length' "$PAYLOAD")
513+
echo "Posting $EVENT review with $NUM_COMMENTS inline comment(s)"
514+
455515
gh api \
456516
"repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/reviews" \
457517
--method POST \
458-
-f event="$EVENT" \
459-
-f body="$BODY" \
518+
--input "$PAYLOAD" \
460519
--jq '.html_url'
461520
462521
- name: Resolve snapshotted prior threads

0 commit comments

Comments
 (0)