From dbaf62d9b3f8157ce9309b1c6581a5711ee62396 Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Thu, 23 Apr 2026 20:00:13 +0000 Subject: [PATCH] fix(triggers): widen evaluateAuthorMode to recognize reviewer persona as 'own' --- src/triggers/github/check-suite-success.ts | 2 +- src/triggers/github/pr-opened.ts | 2 +- src/triggers/github/utils.ts | 17 ++- .../unit/triggers/check-suite-success.test.ts | 62 ++++++++++ tests/unit/triggers/github-utils.test.ts | 109 ++++++++++++++++++ tests/unit/triggers/pr-opened.test.ts | 35 +++++- 6 files changed, 219 insertions(+), 8 deletions(-) diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index 2ca5e375..a77e8ffb 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -138,7 +138,7 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { handler: this.name, prNumber, prAuthor: prDetails.user.login, - isImplementerPR: authorResult.isImplementerPR, + isCascadePR: authorResult.isCascadePR, authorMode: authorResult.authorMode, }); return null; diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index ad674a01..edb90908 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -67,7 +67,7 @@ export class PROpenedTrigger implements TriggerHandler { handler: this.name, prNumber, prAuthor, - isImplementerPR: authorResult.isImplementerPR, + isCascadePR: authorResult.isCascadePR, authorMode: authorResult.authorMode, }); return null; diff --git a/src/triggers/github/utils.ts b/src/triggers/github/utils.ts index 9bcda4bb..f5492bef 100644 --- a/src/triggers/github/utils.ts +++ b/src/triggers/github/utils.ts @@ -1,11 +1,12 @@ import { lookupWorkItemForPR } from '../../db/repositories/prWorkItemsRepository.js'; +import type { PersonaIdentities } from '../../github/personas.js'; import type { ProjectConfig } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; export interface AuthorModeResult { shouldTrigger: boolean; authorMode: string; - isImplementerPR: boolean; + isCascadePR: boolean; } /** @@ -14,10 +15,13 @@ export interface AuthorModeResult { * * Returns `null` when personaIdentities is missing (caller should return null). * Validates authorMode against known values and falls back to 'own'. + * + * "own" means the PR was authored by any CASCADE persona (implementer OR reviewer). + * This aligns with `isCascadeBot()` which already checks both personas. */ export function evaluateAuthorMode( prAuthorLogin: string, - personaIdentities: { implementer: string } | undefined, + personaIdentities: PersonaIdentities | undefined, parameters: Record, handlerName: string, ): AuthorModeResult | null { @@ -26,7 +30,10 @@ export function evaluateAuthorMode( return null; } const implLogin = personaIdentities.implementer; + const reviewerLogin = personaIdentities.reviewer; const isImplementerPR = prAuthorLogin === implLogin || prAuthorLogin === `${implLogin}[bot]`; + const isReviewerPR = prAuthorLogin === reviewerLogin || prAuthorLogin === `${reviewerLogin}[bot]`; + const isCascadePR = isImplementerPR || isReviewerPR; const rawMode = parameters.authorMode; const authorMode = @@ -41,10 +48,10 @@ export function evaluateAuthorMode( const shouldTrigger = authorMode === 'all' || - (authorMode === 'own' && isImplementerPR) || - (authorMode === 'external' && !isImplementerPR); + (authorMode === 'own' && isCascadePR) || + (authorMode === 'external' && !isCascadePR); - return { shouldTrigger, authorMode, isImplementerPR }; + return { shouldTrigger, authorMode, isCascadePR }; } /** diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index b5390c5f..54e6cfdf 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -880,6 +880,68 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result).toBeNull(); }); + it('triggers when PR authored by reviewer persona and authorMode=own', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'own' }, + }); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Reviewer persona PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/reviewer-authored', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-reviewer' }, + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('review'); + }); + + it('skips reviewer persona PR when authorMode=external', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Reviewer persona PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/reviewer-authored', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'cascade-reviewer' }, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + it('triggers for both authors when authorMode=all', async () => { vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, diff --git a/tests/unit/triggers/github-utils.test.ts b/tests/unit/triggers/github-utils.test.ts index 9dfca8ca..e9568378 100644 --- a/tests/unit/triggers/github-utils.test.ts +++ b/tests/unit/triggers/github-utils.test.ts @@ -5,7 +5,9 @@ vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ })); import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; +import type { PersonaIdentities } from '../../../src/github/personas.js'; import { + evaluateAuthorMode, extractJiraIssueKey, extractTrelloCardId, extractWorkItemId, @@ -200,3 +202,110 @@ describe('parsePrNumberFromRef', () => { expect(parsePrNumberFromRef('pull/42/head')).toBeNull(); }); }); + +describe('evaluateAuthorMode', () => { + const personas: PersonaIdentities = { + implementer: 'cascade-impl', + reviewer: 'cascade-reviewer', + }; + + it('returns null when personaIdentities is undefined', () => { + const result = evaluateAuthorMode('some-user', undefined, {}, 'test-handler'); + expect(result).toBeNull(); + }); + + it('returns shouldTrigger:true + isCascadePR:true for implementer login when authorMode=own', () => { + const result = evaluateAuthorMode('cascade-impl', personas, { authorMode: 'own' }, 'handler'); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:true + isCascadePR:true for reviewer login when authorMode=own (core bug regression)', () => { + const result = evaluateAuthorMode( + 'cascade-reviewer', + personas, + { authorMode: 'own' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:true + isCascadePR:true for implementer[bot] variant when authorMode=own', () => { + const result = evaluateAuthorMode( + 'cascade-impl[bot]', + personas, + { authorMode: 'own' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:true + isCascadePR:true for reviewer[bot] variant when authorMode=own', () => { + const result = evaluateAuthorMode( + 'cascade-reviewer[bot]', + personas, + { authorMode: 'own' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'own', isCascadePR: true }); + }); + + it('returns shouldTrigger:false for external author when authorMode=own', () => { + const result = evaluateAuthorMode('external-dev', personas, { authorMode: 'own' }, 'handler'); + expect(result).toEqual({ shouldTrigger: false, authorMode: 'own', isCascadePR: false }); + }); + + it('returns shouldTrigger:true for external author when authorMode=external', () => { + const result = evaluateAuthorMode( + 'external-dev', + personas, + { authorMode: 'external' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: true, authorMode: 'external', isCascadePR: false }); + }); + + it('returns shouldTrigger:false for implementer when authorMode=external', () => { + const result = evaluateAuthorMode( + 'cascade-impl', + personas, + { authorMode: 'external' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: false, authorMode: 'external', isCascadePR: true }); + }); + + it('returns shouldTrigger:false for reviewer when authorMode=external (second regression test)', () => { + const result = evaluateAuthorMode( + 'cascade-reviewer', + personas, + { authorMode: 'external' }, + 'handler', + ); + expect(result).toEqual({ shouldTrigger: false, authorMode: 'external', isCascadePR: true }); + }); + + it('returns shouldTrigger:true for any author when authorMode=all', () => { + for (const login of ['cascade-impl', 'cascade-reviewer', 'external-dev']) { + const result = evaluateAuthorMode(login, personas, { authorMode: 'all' }, 'handler'); + expect(result?.shouldTrigger).toBe(true); + expect(result?.authorMode).toBe('all'); + } + }); + + it('falls back to "own" when authorMode is an invalid string', () => { + const result = evaluateAuthorMode( + 'cascade-impl', + personas, + { authorMode: 'invalid' }, + 'handler', + ); + expect(result?.authorMode).toBe('own'); + expect(result?.shouldTrigger).toBe(true); + }); + + it('falls back to "own" when authorMode is missing from parameters', () => { + const result = evaluateAuthorMode('cascade-impl', personas, {}, 'handler'); + expect(result?.authorMode).toBe('own'); + expect(result?.shouldTrigger).toBe(true); + }); +}); diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index 7e9dcfe0..35f67b7d 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -417,7 +417,7 @@ describe('PROpenedTrigger', () => { expect(result?.agentType).toBe('review'); }); - it('fires for reviewer persona PR when authorMode=external (reviewer is not implementer)', async () => { + it('skips reviewer persona PR when authorMode=external (reviewer is own)', async () => { vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ enabled: true, parameters: { authorMode: 'external' }, @@ -446,6 +446,39 @@ describe('PROpenedTrigger', () => { }, }; + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + }); + + it('fires for reviewer persona PR when authorMode=own', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'own' }, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'feat: add login', + body: 'Implements feature', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/login', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'cascade-review' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'cascade-review' }, + }, + }; + const result = await trigger.handle(ctx); expect(result).not.toBeNull(); expect(result?.agentType).toBe('review');