Skip to content

post-buffered-inline-comments exits 0 after failing to post every comment (Posted 0/N goes green) #1679

Description

@meldiner

Summary

When createReviewComment fails for a buffered comment, post-buffered-inline-comments.ts swallows the error and continues. If every comment fails, the step logs Posted 0/N, exits 0, and the job goes green. A review that dropped 100% of its findings is indistinguishable from a review that found nothing.

Current code (d721746)

postComment :135-143 — failure is logged at console.log (not even ::error::) and reported as a boolean:

} catch (e) {
  console.log(`  failed ${c.path}:${c.line}: ...`);
  return false;
}

main :219-227 — the count is printed and discarded:

let posted = 0;
for (const c of toPost) {
  if (await postComment(...)) { posted++; }
}
console.log(`Posted ${posted}/${toPost.length}`);

posted is never compared to toPost.length. main() returns normally, so the always() step succeeds.

Observed

2026-08-16: 40 consecutive runs logged Posted 0/N and reported success. Every post was rejected with 422 pull_request_review_thread.path/line could not be resolved. Nothing in the check status, the PR, or the job summary indicated the reviewer had delivered nothing — we only found it by reading raw step logs.

Our 422s came from #1542 (stale cross-PR buffer entries whose paths aren't in the current diff), but the silent-success is independent of that cause. Any 422 — a line outside the diff, a stale commit_id after a force-push, an outdated headSha — produces the same green-but-empty result. It's also the failure mode most likely to persist after #1542 is fixed, since a legitimately mis-anchored comment still vanishes silently.

Contrast with the sibling failure path

#1667 covers the opposite over-reaction in the same file: one malformed buffer line throws out of main() and exits 1, discarding everything. So the module currently hard-fails on a parse error it could recover from, and silently succeeds on a delivery failure that loses user-visible output. Both paths deserve the middle ground.

Suggested fix

  1. Emit ::error:: (not console.log) per failed post, including the API error body — a 422 naming the unresolvable path is the single most useful diagnostic and it's currently buried.
  2. When posted < toPost.length, either exit non-zero or emit a ::error:: summary, so a review that dropped findings is visibly distinct from a clean one.
  3. Print the body of each dropped comment so the finding survives in the log even when it can't be anchored.

Point 2 is arguably a behaviour change for anyone relying on the step never failing, so a warning-only variant (::error:: without a non-zero exit) would still fix the invisibility. Happy to open a PR either way.

Environment

  • anthropics/claude-code-action@v1, source read at d721746d683d812e669ce117cebe55a85fbd9c3e
  • Self-hosted runners; classify_inline_comments default (on), ANTHROPIC_API_KEY unset (OAuth token auth)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingexperimental-reviewp2Non-showstopper bug or popular feature request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions