Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/triggers/github/check-suite-success.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/triggers/github/pr-opened.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 12 additions & 5 deletions src/triggers/github/utils.ts
Original file line number Diff line number Diff line change
@@ -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;
}

/**
Expand All @@ -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<string, unknown>,
handlerName: string,
): AuthorModeResult | null {
Expand All @@ -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 =
Expand All @@ -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 };
}

/**
Expand Down
62 changes: 62 additions & 0 deletions tests/unit/triggers/check-suite-success.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
109 changes: 109 additions & 0 deletions tests/unit/triggers/github-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
});
35 changes: 34 additions & 1 deletion tests/unit/triggers/pr-opened.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down Expand Up @@ -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');
Expand Down
Loading