Skip to content

ci(claude): marker-based review-comment edit-in-place; fixes mention-collision #444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
heskew merged 2 commits into from
May 1, 2026
+93 −18
Merged

ci(claude): marker-based review-comment edit-in-place; fixes mention-collision #444

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb05251
ci(claude): marker-based review-comment edit-in-place; fixes mention-…
heskew May 1, 2026
ba4f7e0
ci(claude): tighten gh api allowlist comment — repo-scoped, not PR-sc…
heskew May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 93 additions & 18 deletions .github/workflows/claude-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,29 @@ jobs:
echo "${DELIM}"
} >> "$GITHUB_OUTPUT"

- name: Find prior review comment
# `--edit-last` (the previous mechanism) filters by authenticated
# identity (`claude[bot]`) only — so after a `@claude` mention,
# the most recent claude[bot] comment is the mention response
# and `--edit-last` clobbers it. Marker-based lookup fixes that:
# every review comment starts with `<!-- claude-review:v1 -->`,
# which mention responses never carry. Mention responses are
# safe; only the marker'd review comment is editable here.
id: prior_review
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
set -uo pipefail
EXISTING_ID=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" \
--jq '[.[] | select(.user.login == "claude[bot]") | select(.body | startswith("<!-- claude-review:v1 -->"))] | last | .id // empty')
if [ -n "$EXISTING_ID" ]; then
echo "Prior review comment: $EXISTING_ID"
else
echo "No prior review comment found — agent will post fresh."
fi
echo "id=${EXISTING_ID}" >> "$GITHUB_OUTPUT"

- name: Claude review
id: claude-review
uses: anthropics/claude-code-action@c3d45e8e941e1b2ad7b278c57482d9c5bf1f35b3 # v1.0.99
Expand Down Expand Up @@ -139,10 +162,26 @@ jobs:
# path-bound is enforced via prompt; deviations are
# contained because the runner is ephemeral and has no
# write credentials beyond `gh pr comment`.
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr view:*),Read,Grep,Glob,Write,Bash(git diff:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*)"
# `Bash(gh api:*)` is needed for editing a prior review
# comment by integer database ID (the only way to target a
# specific comment instead of "the most recent claude[bot]
# comment", which `--edit-last` defaults to and which
# collides with `@claude` mention responses). The job's
# token (`pull-requests: write` / `contents: read` /
# `id-token: write`) is repo-scoped, not PR-scoped — a
# successful prompt injection could PATCH any top-level
# PR/issue comment in this repo, read repo contents, or
# fetch an OIDC token. The actual containment is the
# prompt's injection guard (the `Note:` paragraph treating
# PR-checked-out `CLAUDE.md` / `AGENTS.md` as untrusted
# for review-discipline overrides, plus the explicit tool
# discipline in `## Tools`), the runner's ephemerality,
# and branch-protection rules on protected refs.
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr view:*),Bash(gh api:*),Read,Grep,Glob,Write,Bash(git diff:*),Bash(git log:*),Bash(git blame:*),Bash(git show:*)"
prompt: |
REPO: ${{ github.repository }}
PR NUMBER: ${{ github.event.pull_request.number }}
PRIOR_REVIEW_COMMENT_ID: ${{ steps.prior_review.outputs.id }}

The PR branch is already checked out in the current working directory.

Expand Down Expand Up @@ -265,15 +304,43 @@ jobs:
fresh on each run; GitHub auto-marks superseded ones
"outdated".
- **Top-level PR comment** — a concise summary that
anchors the inline findings. Cap at ~5 lines. Edit
in place across runs so the PR never accumulates stacked
review comments:
`gh pr comment <N> --edit-last --create-if-none --body "<BODY>"`.
Use `--body-file` if the body is large enough to hit
shell argument limits. Don't wrap prior reviews in
`<details>` here — the related ai-review-log issue
anchors the inline findings. Cap at ~5 lines. **There
is at most one review comment per PR; edit the same
comment in place across runs.** Don't wrap prior reviews
in `<details>` here — the related ai-review-log issue
keeps history across runs.

**Posting mechanism (read carefully).** `--edit-last`
is NOT safe — it would clobber a `@claude` mention
response that happens to be the most recent
`claude[bot]` comment. Instead, use the marker-based
flow:

1. **Every review comment body MUST begin with the
literal first line `<!-- claude-review:v1 -->`,
followed by a newline, followed by your review
content.** This sentinel is what distinguishes the
review comment from mention responses (which lack
it). Future runs find and edit this comment via the
marker.

2. **If `PRIOR_REVIEW_COMMENT_ID` (set above) is
non-empty**, edit that exact comment by integer
database ID:
```
gh api -X PATCH "repos/${{ github.repository }}/issues/comments/$PRIOR_REVIEW_COMMENT_ID" -f body="$BODY"
```
(where `$BODY` starts with the marker line).

3. **If empty**, post fresh:
```
gh pr comment ${{ github.event.pull_request.number }} --body "$BODY"
```
(where `$BODY` starts with the marker line).

Use `--body-file` (or `gh api … -F body=@<file>`) for
large bodies that hit shell argument limits.

**For "no blockers" runs, the PR comment is one sentence.**
This overrides the universal scope's
`summary is welcome during calibration` allowance — Harper
Expand Down Expand Up @@ -373,32 +440,40 @@ jobs:
fi

# When this workflow job started. Used to filter out stale Claude
# comments from previous runs so a cancelled in-flight run (e.g.
# from a force-push) doesn't re-log the prior run's comment as a
# fresh finding.
# review comments from previous runs so a cancelled in-flight
# run (e.g. from a force-push) doesn't re-log a prior run's
# content as a fresh finding.
JOB_STARTED=$(gh api "repos/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" --jq '.run_started_at // empty')

# Fetch Claude's latest comment and its createdAt timestamp.
CLAUDE_JSON=$(gh pr view "$PR_NUMBER" --json comments \
--jq '[.comments[] | select(.author.login == "claude")] | last // empty')
# Fetch the marker'd review comment via raw API. We can't use
# `gh pr view --json comments` because (a) it doesn't expose
# `updated_at` (which we need below for the staleness guard
# now that comments are edited in place), and (b) we need the
# marker filter to ignore `@claude` mention responses that
# share the `claude[bot]` identity.
CLAUDE_JSON=$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" \
--jq '[.[] | select(.user.login == "claude[bot]") | select(.body | startswith("<!-- claude-review:v1 -->"))] | last // empty')

if [ -z "$CLAUDE_JSON" ] || [ "$CLAUDE_JSON" = "null" ]; then
echo "No Claude comment found on PR #$PR_NUMBER (review_status=$REVIEW_STATUS); skipping log."
echo "No marker'd Claude review comment found on PR #$PR_NUMBER (review_status=$REVIEW_STATUS); skipping log."
exit 0
fi

CLAUDE_BODY=$(printf '%s' "$CLAUDE_JSON" | jq -r '.body // empty')
CLAUDE_AT=$(printf '%s' "$CLAUDE_JSON" | jq -r '.createdAt // empty')
# Prefer updated_at (reflects the most recent edit) over
# created_at (frozen at original post time) — comments are
# now edited in place across runs.
CLAUDE_AT=$(printf '%s' "$CLAUDE_JSON" | jq -r '.updated_at // .created_at // empty')

if [ -z "$CLAUDE_BODY" ]; then
echo "Claude comment had empty body; skipping log."
echo "Claude review comment had empty body; skipping log."
exit 0
fi

# ISO-8601 lexicographic compare — both are UTC timestamps in the
# same shape, so string comparison is sound.
if [ -n "$JOB_STARTED" ] && [ -n "$CLAUDE_AT" ] && [ "$CLAUDE_AT" \< "$JOB_STARTED" ]; then
echo "::notice::Latest Claude comment ($CLAUDE_AT) predates this job's start ($JOB_STARTED); skipping to avoid re-logging a stale comment."
echo "::notice::Latest Claude review comment update ($CLAUDE_AT) predates this job's start ($JOB_STARTED); skipping to avoid re-logging stale content."
exit 0
fi

Expand Down
Loading