Skip to content

feat(triggers): expand respond-to-review to also fire on COMMENT-type reviews#646

Merged
zbigniewsobiecki merged 1 commit intodevfrom
feature/respond-to-review-commented-state
Mar 7, 2026
Merged

feat(triggers): expand respond-to-review to also fire on COMMENT-type reviews#646
zbigniewsobiecki merged 1 commit intodevfrom
feature/respond-to-review-commented-state

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 7, 2026

Summary

  • Expand PRReviewSubmittedTrigger.matches() to accept both changes_requested and commented review states, excluding only approved (dismissed reviews are already excluded by the action !== 'submitted' guard)
  • Update tests to flip the existing commented-state assertion from false to true, add a new matches test for dismissed state, and add two new handle tests covering commented-state triggering and its null-body fallback
  • Update respond-to-review.yaml trigger description to reflect the broader scope

Card: https://trello.com/c/69ac12414c2917eab39385fc

Changed files

File Change
src/triggers/github/pr-review-submitted.ts Replace !== 'changes_requested' check with === 'approved' check; update inline comments
tests/unit/triggers/pr-review-submitted.test.ts Flip commented state assertion; add dismissed/commented handle tests
src/agents/definitions/respond-to-review.yaml Update trigger description

Safety analysis

  • No infinite loop riskrespond-to-review runs as the implementer persona; the trigger requires the reviewer persona. The guard in handle() is unchanged.
  • check-suite-success.ts dedup logic that filters out COMMENTED reviews is unrelated and intentionally unchanged.

Test plan

  • matches() returns true for changes_requested and commented
  • matches() returns false for approved and dismissed (action-filtered)
  • handle() returns correct TriggerResult for commented state from reviewer persona
  • handle() uses Review: commented fallback when body is null
  • All 16 unit tests pass
  • Lint and type-check pass

🤖 Generated with Claude Code

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

LGTM — Clean, well-scoped change that broadens the review trigger to fire on commented-state reviews in addition to changes_requested.

Verified:

  • The matches() logic change from !== 'changes_requested'=== 'approved' correctly excludes only approvals (dismissals are already filtered by the action !== 'submitted' guard above)
  • No infinite loop risk: the handle() persona guard (line 61) ensures only reviews from the reviewer persona trigger the agent, while respond-to-review runs as implementer
  • The check-suite-success.ts dedup logic that filters out COMMENTED reviews is unaffected and intentionally separate
  • The null-body fallback (Review: ${state}) works correctly for the commented state
  • All 16 tests pass, covering both matches and handle for the new states

@zbigniewsobiecki zbigniewsobiecki merged commit f0c12f3 into dev Mar 7, 2026
6 checks passed
@zbigniewsobiecki zbigniewsobiecki deleted the feature/respond-to-review-commented-state branch March 16, 2026 16:43
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.

3 participants