Skip to content

fix(code-review): post the review comment from the workflow, not the model - #27

Closed
rathorevaibhav wants to merge 1 commit into
mainfrom
fix/post-review-comment-and-guidelines-dir
Closed

rathorevaibhav wants to merge 1 commit into
mainfrom
fix/post-review-comment-and-guidelines-dir

Conversation

@rathorevaibhav

Copy link
Copy Markdown
Contributor

What this fixes

Two bugs in claude-code-review.yml, both silent. The workflow goes green either way.

1. The review was often never posted

anthropics/claude-code-action auto-detects agent mode on a pull_request event. Agent mode posts nothing to the PR by itself, and use_sticky_comment does nothing there. The review reached the PR only when the model chose to run gh pr comment on its own.

It usually did not. Across the last 14 labelled runs on coloredcow-admin/coloredcow, only 4 called gh pr comment:

Run Called gh pr comment?
35698189378 (PR #1505) no
31793924039 (PR #1462) no
31252577365 (PR #1475) no
30918524478 (PR #1470) no
30995245461, 30914500300, 30696316095, 30615671614, 31151997989, 30997913919 no
31182274634, 31180400114, 31152743199, 31150723766 yes

The 10 misses all finished subtype: success with the complete review in the run's result field, visible only in the Actions job summary. PR #1470 still has no review on it. On PR #1462 a teammate copy-pasted the summary in by hand three minutes after the run.

Fix: take the model out of the delivery path. A new Post review comment step reads the action's execution_file output, pulls the closing message with jq, and posts it. The prompt now tells the model to make its final message the review and to post nothing itself, and gh pr comment is off the default tool list.

The step also makes use_sticky_comment mean something again. It finds its own earlier comment by a hidden <!-- claude-code-review --> marker plus a bot author and updates that, so re-running the label does not stack duplicates.

It posts with GITHUB_TOKEN, so comments come from github-actions[bot] rather than claude[bot]. That is deliberate: the action revokes its own app token in its last internal step, so steps.claude-review.outputs.github_token is already dead by the time a following step runs.

2. guidelines_dir never reached the model

The default prompt interpolated ${{ inputs.guidelines_dir }} inside the default value of another workflow_call input. GitHub resolves input defaults before the caller's inputs exist, so it rendered empty. Every prompt ever sent has said:

Check if the directory `` exists.

No review has read a project's guidelines. Not once.

Fix: the default prompt carries a literal {{GUIDELINES_DIR}} placeholder, and a new Build review prompt step substitutes it where inputs do resolve. Callers passing a custom prompt can use the same placeholder.

Also corrected a stale path in automated-code-review/README.md, which pointed teams at docs/code-review-guidelines.md. setup.sh writes docs/code-review/review-guidelines.md.

Testing

Replayed the real PR #1505 execution output (39 turns, rebuilt from the run log) through the new step with gh stubbed:

  • no existing sticky comment → POST, recovers the full 1999-character review
  • existing sticky comment → PATCH against the right comment id
  • missing execution file → warning, exit 0
  • run with no review text → warning, exit 0
  • sticky lookup API failure → degrades to POST instead of losing the review

Also checked the sticky lookup against a comment with a null body and against a human who pasted the marker. The old expression crashed on null; the hardened one skips both and matches only the bot's comment.

Note for reviewers

Sanitize inputs still interpolates ${{ inputs.allowed_tools }} straight into a shell script. That is a script-injection vector and predates this change. I left it alone to keep this PR to the two diagnosed bugs. Worth a follow-up.

🤖 Generated with Claude Code

…model

The action auto-detects "agent" mode on pull_request events. Agent mode posts
nothing to the PR on its own, and use_sticky_comment is inert there. The review
only reached the PR when the model happened to call `gh pr comment` itself,
which it did on 4 of the last 14 runs. The other 10 finished green with the
full review sitting in the job summary and no comment on the PR.

Take the model out of the delivery path. A new step reads the action's
execution_file output, pulls the closing message, and posts it. The prompt now
tells the model to make its final message the review and to post nothing, and
`gh pr comment` is off the default tool list.

The step also honours use_sticky_comment for real by updating its own earlier
comment, matched on a hidden marker and a bot author. It uses GITHUB_TOKEN
because the action revokes its app token in its own final step.

Also fix guidelines_dir, which has never reached the model. It was interpolated
inside the default value of another workflow_call input, where GitHub cannot
resolve it, so every prompt so far has read "Check if the directory `` exists".
The default prompt now carries a {{GUIDELINES_DIR}} placeholder that a job step
substitutes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rathorevaibhav

Copy link
Copy Markdown
Contributor Author

Closing for now. The team's call is that the missed comments are infrequent enough not to warrant this change yet.

Leaving the branch fix/post-review-comment-and-guidelines-dir in place so this can be reopened if it comes up again. Diagnosis is in the PR description above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant