Skip to content

fix: handle @docker-agent mentions in inline PR review comments (pull_request_review_comment)#181

Merged
derekmisler merged 3 commits into
docker:mainfrom
docker-agent:fix/reply-to-mention-pr-review-comment
May 6, 2026
Merged

fix: handle @docker-agent mentions in inline PR review comments (pull_request_review_comment)#181
derekmisler merged 3 commits into
docker:mainfrom
docker-agent:fix/reply-to-mention-pr-review-comment

Conversation

@docker-agent
Copy link
Copy Markdown
Contributor

Problem

When someone @docker-agent mentions in an inline PR review comment (a pull_request_review_comment event), the reply-to-mention job was always silently skipped and the 👀 reaction call would use the wrong GitHub API endpoint.

Three bugs combined to produce this:

  1. Workflow if: condition — the reply-to-mention job only matched issue_comment events; pull_request_review_comment events were never routed to it.

  2. parseEventContext() payload mismatchsrc/mention-reply/index.ts always read the PR number from raw.issue.number, which does not exist in pull_request_review_comment payloads (the PR lives at raw.pull_request.number there).

  3. Wrong reactions APIsrc/add-reaction/index.ts always called createForIssueComment, which 404s on review comments; createForPullRequestReviewComment is needed instead.

Changes

.github/workflows/review-pr.yml

  • reply-to-mention job if: — adds an OR branch for pull_request_review_comment events. No github.event.issue.pull_request guard is needed (review comments are always on PRs).
  • Add completion reaction step — routes to the correct gh api endpoint (pulls/comments vs issues/comments) depending on EVENT_NAME.

src/add-reaction/index.ts

  • Exports a new CommentType = 'issue' | 'pull_request_review' type.
  • Adds an optional commentType parameter (default 'issue') to addReaction().
  • When 'pull_request_review', calls octokit.rest.reactions.createForPullRequestReviewComment instead of createForIssueComment.

src/mention-reply/index.ts

  • Adds a commentType: CommentType field to EventContext.
  • parseEventContext() now checks GITHUB_EVENT_NAME; for pull_request_review_comment it reads prNumber from raw.pull_request.number and sets isPrComment = true and commentType = 'pull_request_review'.
  • The addReaction call passes ctx.commentType so the correct API endpoint is used.

Tests

  • src/add-reaction/__tests__/add-reaction.test.ts — adds beforeEach mock reset; covers the new commentType parameter for both 'issue' and 'pull_request_review'.
  • src/mention-reply/__tests__/mention-reply.test.ts — adds makePrReviewCommentEvent fixture and new describe blocks covering parseEventContext, runGuards, and run() for pull_request_review_comment events (guard paths, happy path, non-member path, correct reaction commentType). Test count goes from 25 to 36.

Testing

pnpm build    # clean
pnpm test     # 120/120 passed
biome ci .    # no errors
tsc --noEmit  # no errors
actionlint    # no errors

The reply-to-mention job was silently skipped whenever @docker-agent was
mentioned in an inline PR review comment (pull_request_review_comment
event) because:

1. The job's 'if:' condition only matched issue_comment events.
2. parseEventContext() read PR number from raw.issue.number, which
   doesn't exist in pull_request_review_comment payloads (it's at
   raw.pull_request.number).
3. addReaction() always called createForIssueComment, which 404s for
   review comments (need createForPullRequestReviewComment).

Changes:

.github/workflows/review-pr.yml
- reply-to-mention job: add OR branch for pull_request_review_comment
  events (no issue.pull_request check needed — it's always a PR).
- Add completion reaction step: route to the correct gh api endpoint
  (pulls/comments vs issues/comments) based on EVENT_NAME.

src/add-reaction/index.ts
- Export new CommentType = 'issue' | 'pull_request_review' type.
- Add optional commentType parameter (default 'issue') to addReaction().
- When 'pull_request_review', call createForPullRequestReviewComment
  instead of createForIssueComment.

src/mention-reply/index.ts
- Add commentType field to EventContext interface.
- parseEventContext(): check GITHUB_EVENT_NAME; for
  pull_request_review_comment read prNumber from raw.pull_request.number
  and set isPrComment=true and commentType='pull_request_review'.
- Pass ctx.commentType to addReaction() so the correct API is used.

Tests
- src/add-reaction/__tests__: add beforeEach mock reset; cover new
  commentType parameter for both 'issue' and 'pull_request_review'.
- src/mention-reply/__tests__: add makePrReviewCommentEvent fixture;
  test parseEventContext, runGuards, and run() for
  pull_request_review_comment events including guard paths, happy path,
  non-member path, and reaction call with correct commentType.

Assisted-By: docker-agent
@docker-agent docker-agent requested a review from a team as a code owner May 6, 2026 19:15
@docker-agent docker-agent requested a review from derekmisler May 6, 2026 19:15
Users found the 😕 reaction confusing — it made them think they had
done something wrong rather than signalling an internal action failure.

In the 'Add completion reaction' step of the reply-to-mention job, only
post the 👍 reaction when the reply step succeeds; do nothing on failure.

Searched all workflow files for other confused-on-failure patterns;
this was the only occurrence.

Assisted-By: docker-agent
Copy link
Copy Markdown
Contributor

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🟡 NEEDS ATTENTION

The core fix is correct — comment events are now properly routed, the right GitHub API endpoints are used for reactions, and PR numbers are read from the correct payload field. Two issues in the workflow if: conditions were found.

github.event.comment.user.login != 'docker-agent' &&
github.event.comment.user.type != 'Bot'
github.event.comment.user.type != 'Bot') ||
(github.event_name == 'pull_request_review_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.

[MEDIUM] Double bot response when a user replies to a review comment and mentions @docker-agent

When a user replies to an existing inline review comment and includes @docker-agent in that reply, both reply-to-feedback and the newly-added reply-to-mention branch fire simultaneously on the same event:

  • reply-to-feedback triggers because: pull_request_review_comment + github.event.comment.in_reply_to_id is set (it's a reply) + user is not docker-agent
  • reply-to-mention triggers because: pull_request_review_comment + contains(@docker-agent) + user is not docker-agent

Neither condition checks for the other. The result is two concurrent agent runs posting separate responses to the same comment — a confusing double-response the user did not ask for.

This overlap was not possible before this PR because reply-to-mention only handled issue_comment events. The reply-to-feedback job (line 364) would need a !contains(github.event.comment.body, '@docker-agent') guard, or reply-to-mention would need !github.event.comment.in_reply_to_id to break the ambiguity.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the latest commit. Added !github.event.comment.in_reply_to_id as the first guard in the pull_request_review_comment branch, so reply-to-mention only fires on fresh top-level inline comments. Replies to existing threads — including those containing @docker-agent — fall through exclusively to reply-to-feedback, which has the full thread context. No more double-response.

github.event.comment.user.type != 'Bot'
github.event.comment.user.type != 'Bot') ||
(github.event_name == 'pull_request_review_comment' &&
contains(github.event.comment.body, '@docker-agent') &&
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.

[LOW] Missing /review guard in the pull_request_review_comment branch of reply-to-mention

The issue_comment branch guards against /review-prefixed comments (line 705: !startsWith(github.event.comment.body, '/review')), but the new pull_request_review_comment branch does not:

(github.event_name == 'pull_request_review_comment' &&
contains(github.event.comment.body, '@docker-agent') &&
# ← !startsWith(..., '/review') is missing here
github.event.comment.user.login != 'docker-agent' &&
github.event.comment.user.type != 'Bot')

If someone writes /review @docker-agent as an inline PR review comment, the reply-to-mention job will spin up unnecessarily. The TypeScript runGuards() does catch this and exits cleanly (so no incorrect output is produced), but runner minutes are wasted on every such event.

Suggested fix: add !startsWith(github.event.comment.body, '/review') && after the contains(...) check in this branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the same commit. Added !startsWith(github.event.comment.body, '/review') to the pull_request_review_comment branch, immediately after the @docker-agent mention check — matching the guard that already existed in the issue_comment branch.

…eview_comment

Two review-comment fixes on the reply-to-mention job's if: condition
for the pull_request_review_comment branch:

1. Add !github.event.comment.in_reply_to_id so reply-to-mention only
   fires on fresh top-level inline mentions, not on replies to existing
   threads. Replies that happen to contain @docker-agent are better
   handled by reply-to-feedback which has full thread context, avoiding
   a double-response when both jobs would otherwise fire simultaneously.

2. Add !startsWith(github.event.comment.body, '/review') to match the
   guard that already exists in the issue_comment branch, preventing the
   mention handler from running when the user is issuing a /review command.

Assisted-By: docker-agent
@derekmisler
Copy link
Copy Markdown
Contributor

/review

Copy link
Copy Markdown
Contributor

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🟢 APPROVE

The three bug fixes are correctly implemented:

  1. Workflow if: conditionreply-to-mention job now correctly matches both issue_comment and pull_request_review_comment events, with the appropriate !github.event.comment.in_reply_to_id guard to avoid double-responding on thread replies.

  2. parseEventContext() payload mismatchpull_request_review_comment events now correctly read prNumber from raw.pull_request.number and set commentType = 'pull_request_review'.

  3. Wrong reactions APIaddReaction() correctly routes to createForPullRequestReviewComment vs createForIssueComment based on commentType.

  4. Completion reaction routing — The Add completion reaction shell block correctly dispatches to pulls/comments vs issues/comments endpoints based on EVENT_NAME, and the exit 0 guard on non-success outcomes intentionally drops the old confused reaction (per the earlier bebd4e8 commit).

  5. Tests — New test fixtures and describe blocks cover the pull_request_review_comment happy path, guard paths (in-reply-to filter, bot/self-reply), non-member path, and correct commentType propagation. Test count grows from 25 → 36.

No bugs found in the changed code.

@derekmisler derekmisler merged commit fbae530 into docker:main May 6, 2026
11 checks passed
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.

2 participants