Skip to content

ci(review): advisory review posts top-level comments only (fixes the thread deadlock at the source) - #292

Merged
blove merged 1 commit into
mainfrom
fix/review-top-level-comments
Jul 6, 2026
Merged

ci(review): advisory review posts top-level comments only (fixes the thread deadlock at the source)#292
blove merged 1 commit into
mainfrom
fix/review-top-level-comments

Conversation

@blove

@blove blove commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Problem

Two compounding issues with the advisory Claude review:

  1. Its inline review comments create threads, and the branch rule "Require conversation resolution before merging" blocks auto-merge on any unresolved thread — regardless of the posting check being non-required.
  2. The auto-resolve step (ci: auto-resolve advisory Claude-review threads so they don't block merge #262) is broken: the workflow GITHUB_TOKEN gets FORBIDDEN: Resource not accessible by integration on the resolveReviewThread mutation. It was never exercised until #289 (its own PR had no inline comments), where every review round re-deadlocked the merge — 3 rounds, 11 threads resolved by hand.

Fix — kill the deadlock at the source

The bot now posts one top-level comment per review (gh pr comment), never inline review threads:

  • findings reference path/file.ts:123 in the comment body (still easy to locate),
  • ordered most-severe first; re-reviews cover only what changed,
  • the inline-comment MCP tool is removed from allowedTools, and the dead auto-resolve step is deleted.

No threads → nothing to resolve → conversation-resolution can never deadlock on the bot. Human review threads are unaffected (and still gate merge, as intended).

Review quality is unchanged — on #289 it caught two real bugs (a production shutdown container leak and a Docker writeFile parent-dir gap). Only the delivery mechanism changes.

Self-test

pull_request runs use the PR head's workflow, so this PR's own review should arrive as a single top-level comment with zero inline threads.

🤖 Generated with Claude Code

…auto-resolve

The advisory review's inline comments create review threads that trip the
"Require conversation resolution" branch rule and deadlock auto-merge. The
auto-resolve step added in #262 turned out to be broken: the workflow
GITHUB_TOKEN gets FORBIDDEN on the resolveReviewThread mutation (first
exercised on #289 — three review rounds, 11 threads hand-resolved).

Fix at the source: the bot now posts ONE top-level comment (file:line
references inline in the body), never inline review threads — nothing to
resolve, nothing to deadlock. Drop the inline-comment MCP tool from
allowedTools and delete the dead auto-resolve step. Review quality stays
(it caught two real bugs on #289); only the delivery mechanism changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@blove
blove enabled auto-merge (squash) July 6, 2026 14:36
@vercel

vercel Bot commented Jul 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dawnai Ready Ready Preview, Comment Jul 6, 2026 2:38pm

Request Review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval: this PR received an intelligent (AI) code review. See the review comments on this PR.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 38s —— View job


Review

This is a clean, well-reasoned fix. The diff removes the broken auto-resolve step and the inline-comment MCP tool, and updates the prompt to instruct the bot to post a single top-level comment. No correctness bugs, security issues, or type-safety problems found.

One note worth validating:

.github/workflows/claude-review.yml:71

--allowedTools "Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"

The prompt now tells the bot to call gh pr comment (write a new comment each run) rather than edit an existing one. The instruction "if a prior review comment of yours exists … post a fresh comment covering only what changed" will accumulate one new comment per review round. That's intentional per the PR description, but worth confirming it doesn't produce noisy comment threads on long-running PRs. Not a bug — just a UX trade-off to be aware of.

Everything else looks good:

  • Removal of the dead resolveReviewThread step is correct; the GITHUB_TOKEN genuinely cannot call that mutation.
  • The mcp__github_inline_comment__create_inline_comment tool is properly absent from allowedTools, so inline thread creation is blocked at the tool level rather than relying solely on prompt instructions.
  • The pull_request trigger (not pull_request_target) is preserved, keeping ANTHROPIC_API_KEY safe from fork PRs.

Looks good to merge.


@blove
blove merged commit 266990c into main Jul 6, 2026
9 checks passed
@blove
blove deleted the fix/review-top-level-comments branch July 6, 2026 14:47
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