Skip to content

fix(triggers): widen evaluateAuthorMode to recognize reviewer persona as 'own'#1173

Merged
aaight merged 1 commit intodevfrom
fix/evaluate-author-mode-both-personas
Apr 23, 2026
Merged

fix(triggers): widen evaluateAuthorMode to recognize reviewer persona as 'own'#1173
aaight merged 1 commit intodevfrom
fix/evaluate-author-mode-both-personas

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Apr 23, 2026

Summary

  • Bug: evaluateAuthorMode() only checked the implementer persona when deciding if a PR is "own" — PRs authored by the reviewer persona were wrongly treated as "external"
  • Fix: Widened evaluateAuthorMode() to accept PersonaIdentities (both implementer + reviewer) and check both logins when computing isCascadePR
  • Scope: 1 source file (src/triggers/github/utils.ts), 2 log-field renames in callers, 3 test files updated

Card: https://trello.com/c/DCCWFftf/622-lets-make-sure-trigger-review-when-ci-checks-pass-on-a-pr-on-ci-passed-when-set-to-author-filter-own-properly-includes-both-pers

Changes

src/triggers/github/utils.ts

  • Widened personaIdentities parameter type from { implementer: string } to PersonaIdentities (imports from ../../github/personas.js)
  • Added isReviewerPR check alongside isImplementerPR
  • Combined into isCascadePR = isImplementerPR || isReviewerPR
  • Renamed AuthorModeResult.isImplementerPRAuthorModeResult.isCascadePR (diagnostic field used only for logging)

src/triggers/github/check-suite-success.ts + pr-opened.ts

  • Renamed log field isImplementerPRisCascadePR in both handle() methods

Tests

  • tests/unit/triggers/github-utils.test.ts: Added describe('evaluateAuthorMode') block with 12 test cases covering all persona × authorMode combinations
  • tests/unit/triggers/check-suite-success.test.ts: Added 2 new tests for reviewer persona behavior
  • tests/unit/triggers/pr-opened.test.ts: Fixed incorrect test that asserted reviewer-persona PRs should fire on authorMode=external (wrong behavior); added new test for authorMode=own

Key decisions

  • Renamed isImplementerPRisCascadePR on AuthorModeResult to reflect that "own" now means "authored by any CASCADE persona", aligning with the existing isCascadeBot() function
  • No migration needed — purely runtime logic change, no config/DB changes

Test plan

  • 95 unit tests passing for the 3 affected trigger test files
  • npm run lint — zero errors
  • npm run typecheck — zero errors
  • Note: tests/unit/worker-entry.test.ts has a pre-existing failure on origin/dev unrelated to this PR (confirmed: same failure exists without these changes)

🤖 Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · run details

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

Summary

LGTM! The fix correctly widens the author check to include both the implementer and reviewer personas when authorMode is own, resolving the core bug. The tests are comprehensive and the log field renaming accurately reflects the new logic.

Nitpicks

  • In src/triggers/github/utils.ts, you could potentially reuse isCascadeBot(prAuthorLogin, personaIdentities) from src/github/personas.ts instead of manually checking the logins and [bot] suffixes. However, the current explicit implementation is perfectly fine and logically identical.

🕵️ opencode · google/gemini-3.1-pro-preview · run details

@aaight aaight merged commit 0e4a08e into dev Apr 23, 2026
9 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