ci: post Claude reviews from execution_file; fix general_review filter#145
Conversation
…lter Two fixes after observing PR #144 CI run (run 25278577961): 1. Claude review jobs ran successfully but never posted PR comments. In claude-code-action v1, `prompt:` automation mode does not auto-post a comment the way the beta `direct_prompt:` did — that behavior is gone. Replace the comment-polling step with capture-extract-post-check using the action's `execution_file` output (Claude Code stream-JSON), mirroring the existing Codex flow. Removes 6×10s sleep loop and 4-fallback gh-api queries; review now appears immediately and the blocking-findings check greps a local file. 2. `claude-review-general` and `codex-review-general` triggered on PR #144 despite all changed files being inside excluded scopes. dorny/paths-filter v3 OR-list semantics don't reliably honor `!` negations against a leading `**` glob. Replace the negation-based filter with a shell step that computes `general_review` from `gh pr diff --name-only`, true iff at least one file lives outside contracts/, client/, indexer/, api/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughGitHub Actions workflow for AI-based code reviews is refactored. The ChangesAI Review Workflow Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
🚅 Deployed to the summit-pr-145 environment in Summit 3 services not affected by this PR
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 310433e28e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ -z "$EXECUTION_FILE" ] || [ ! -f "$EXECUTION_FILE" ]; then | ||
| echo "Claude action did not produce an execution file." > /tmp/review.txt | ||
| exit 0 |
There was a problem hiding this comment.
If steps.claude.outputs.execution_file is missing or unreadable, this step writes a placeholder message to /tmp/review.txt and exits successfully, and the later gate only checks that the file is non-empty. In that scenario the Claude review job can pass without any real model output, so PRs may merge with review enforcement silently bypassed across all claude-review-* jobs that use this pattern.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accepted — real bug. The placeholder strings made /tmp/review.txt non-empty, so the [ ! -s … ] blocking-check passed and the job could merge with no real review.
Fixed in c3a2acb: both fallback branches (missing/unreadable execution_file, empty extracted text) now emit ::error::, leave /tmp/review.txt empty, and exit 1. The Post review comment step (if: always()) still runs so reviewers see the "No review output was produced." fallback comment on the PR, and the blocking-check then fails on the empty file.
There was a problem hiding this comment.
Pull request overview
Updates the PR CI workflow to (1) reliably post Claude review comments by extracting text from the Claude action execution_file, and (2) fix general_review detection so “general” AI review jobs don’t run on scoped-only changes.
Changes:
- Replace Claude review “poll for comment” logic with an extract → post-comment → local blocking-grep flow.
- Compute
general_reviewexplicitly viagh pr diff --name-onlyinstead ofpaths-filternegations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | jq -r --arg run_token "run=${{ github.run_id }}" --arg sha_token "sha=${{ github.event.pull_request.head.sha }}" \ | ||
| '[.[] | select((.user.login // "") == "claude" and ((.body // "") | contains($run_token) and contains($sha_token) and contains("scope=indexer-api")))] | last | .body // ""') | ||
| fi | ||
| REVIEW=$(jq -r 'select(.type == "result") | .result // empty' "$EXECUTION_FILE" 2>/dev/null | tail -1) |
There was a problem hiding this comment.
Going to pass on this one — the comment Copilot is paraphrasing isn't actually in the file. The block comment didn't say result.result, and it was removed in f09ca84 anyway.
On the substance: Claude Code stream-JSON's terminal message is {"type":"result","is_error":false,"result":"<final assistant text>",...} where .result is a string holding the model's final output. Verified empirically against PR #144 run 25278577961. result.result would resolve to null. The fallback over type:"assistant" text content already covers any future envelope-shape change, and the silent-pass fix in c3a2acb ensures unparseable output now fails the job rather than passing it.
| echo "Claude action did not produce an execution file." > /tmp/review.txt | ||
| exit 0 |
There was a problem hiding this comment.
Accepted — real bug. The placeholder strings made /tmp/review.txt non-empty, so the [ ! -s … ] blocking-check passed and the job could merge with no real review.
Fixed in c3a2acb: both fallback branches (missing/unreadable execution_file, empty extracted text) now emit ::error::, leave /tmp/review.txt empty, and exit 1. The Post review comment step (if: always()) still runs so reviewers see the "No review output was produced." fallback comment on the PR, and the blocking-check then fails on the empty file.
| echo "Found Claude review comment on attempt $i" | ||
| break | ||
| fi | ||
| REVIEW=$(jq -r 'select(.type == "result") | .result // empty' "$EXECUTION_FILE" 2>/dev/null | tail -1) |
There was a problem hiding this comment.
Going to pass on this one — the comment Copilot is paraphrasing isn't actually in the file. The block comment didn't say result.result, and it was removed in f09ca84 anyway.
On the substance: Claude Code stream-JSON's terminal message is {"type":"result","is_error":false,"result":"<final assistant text>",...} where .result is a string holding the model's final output. Verified empirically against PR #144 run 25278577961. result.result would resolve to null. The fallback over type:"assistant" text content already covers any future envelope-shape change, and the silent-pass fix in c3a2acb ensures unparseable output now fails the job rather than passing it.
| echo "Claude action did not produce an execution file." > /tmp/review.txt | ||
| exit 0 |
There was a problem hiding this comment.
Accepted — real bug. The placeholder strings made /tmp/review.txt non-empty, so the [ ! -s … ] blocking-check passed and the job could merge with no real review.
Fixed in c3a2acb: both fallback branches (missing/unreadable execution_file, empty extracted text) now emit ::error::, leave /tmp/review.txt empty, and exit 1. The Post review comment step (if: always()) still runs so reviewers see the "No review output was produced." fallback comment on the PR, and the blocking-check then fails on the empty file.
| echo "Found Claude review comment on attempt $i" | ||
| break | ||
| fi | ||
| REVIEW=$(jq -r 'select(.type == "result") | .result // empty' "$EXECUTION_FILE" 2>/dev/null | tail -1) |
There was a problem hiding this comment.
Going to pass on this one — the comment Copilot is paraphrasing isn't actually in the file. The block comment didn't say result.result, and it was removed in f09ca84 anyway.
On the substance: Claude Code stream-JSON's terminal message is {"type":"result","is_error":false,"result":"<final assistant text>",...} where .result is a string holding the model's final output. Verified empirically against PR #144 run 25278577961. result.result would resolve to null. The fallback over type:"assistant" text content already covers any future envelope-shape change, and the silent-pass fix in c3a2acb ensures unparseable output now fails the job rather than passing it.
| echo "Claude action did not produce an execution file." > /tmp/review.txt | ||
| exit 0 |
There was a problem hiding this comment.
Accepted — real bug. The placeholder strings made /tmp/review.txt non-empty, so the [ ! -s … ] blocking-check passed and the job could merge with no real review.
Fixed in c3a2acb: both fallback branches (missing/unreadable execution_file, empty extracted text) now emit ::error::, leave /tmp/review.txt empty, and exit 1. The Post review comment step (if: always()) still runs so reviewers see the "No review output was produced." fallback comment on the PR, and the blocking-check then fails on the empty file.
| echo "Found Claude review comment on attempt $i" | ||
| break | ||
| fi | ||
| REVIEW=$(jq -r 'select(.type == "result") | .result // empty' "$EXECUTION_FILE" 2>/dev/null | tail -1) |
There was a problem hiding this comment.
Going to pass on this one — the comment Copilot is paraphrasing isn't actually in the file. The block comment didn't say result.result, and it was removed in f09ca84 anyway.
On the substance: Claude Code stream-JSON's terminal message is {"type":"result","is_error":false,"result":"<final assistant text>",...} where .result is a string holding the model's final output. Verified empirically against PR #144 run 25278577961. result.result would resolve to null. The fallback over type:"assistant" text content already covers any future envelope-shape change, and the silent-pass fix in c3a2acb ensures unparseable output now fails the job rather than passing it.
| echo "Claude action did not produce an execution file." > /tmp/review.txt | ||
| exit 0 |
There was a problem hiding this comment.
Accepted — real bug. The placeholder strings made /tmp/review.txt non-empty, so the [ ! -s … ] blocking-check passed and the job could merge with no real review.
Fixed in c3a2acb: both fallback branches (missing/unreadable execution_file, empty extracted text) now emit ::error::, leave /tmp/review.txt empty, and exit 1. The Post review comment step (if: always()) still runs so reviewers see the "No review output was produced." fallback comment on the PR, and the blocking-check then fails on the empty file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the Extract review output step wrote a non-empty placeholder
("Claude action did not produce an execution file." / "Claude review did
not produce output.") to /tmp/review.txt and exited 0 when the action
produced no usable output. The downstream Check for blocking findings
gate uses `[ ! -s /tmp/review.txt ]`, so the placeholder satisfied it
and the job could pass without a real review — silently bypassing the
[CRITICAL]/[HIGH] enforcement that's the whole point of the gate.
Both branches now emit `::error::`, leave /tmp/review.txt empty, and
exit 1. The Post review comment step (if: always()) still runs and
posts the existing "No review output was produced." fallback so
reviewers see the failure on the PR.
Addresses Codex P1 + Copilot review findings on PR #145.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Two CI workflow fixes uncovered while testing PR #144 (run 25278577961).
Fix 1 — Claude jobs ran but never posted comments
In
claude-code-actionv1,prompt:(automation) mode does not auto-post a PR comment the way the betadirect_prompt:did. Each of our four Claude review jobs ran cleanly (is_error: false,--model claude-opus-4-7confirmed), produced a review, then the polling step timed out for 60s and failed the job because no comment ever appeared.Replace the comment-polling pattern (
gh api … | jqwith 6×10s retries and 2–4 fallback queries) with the capture-extract-post-check sequence already used by the Codex jobs:id: claudeon the action step exposesexecution_file(Claude Code stream-JSON).Extract review outputstep pulls the finalresult.resulttext (or falls back to assistant text) into/tmp/review.txt.Post review commentstep writes## Claude Review - …+ body to the PR viagh pr comment.Check for blocking findingsgreps the local file for[CRITICAL]/[HIGH].Net: review comments appear immediately, blocking-check is deterministic, and the 4 Claude jobs now mirror the 4 Codex jobs structurally.
Fix 2 —
general_reviewtriggered on scoped-only changesPR #144 changed only
api/README.md,client/README.md,contracts/README.md— all inside excluded scopes — yetclaude-review-generalandcodex-review-generalran. dorny/paths-filter v3's OR-list semantics don't reliably honor!negations against a leading**glob.Replace:
with a shell step that computes
general_reviewfromgh pr diff --name-only— true iff the PR touches a file outsidecontracts/,client/,indexer/, orapi/. The four scoped filters (contracts_review,client_review,indexer_api_review) continue to use paths-filter (no negations there).Test plan
pr-ci.yml, so AI reviews are gated off viareview_automation_modified— only the deterministic jobs run)*-generaljobs run,pr-cipasses.🤖 Generated with Claude Code
Summary by CodeRabbit
Note: This is an internal infrastructure change with no direct impact on end-user functionality.