diff --git a/src/cli/dashboard/projects/review-trigger-set.ts b/src/cli/dashboard/projects/review-trigger-set.ts index b8e016ea..47faa2ec 100644 --- a/src/cli/dashboard/projects/review-trigger-set.ts +++ b/src/cli/dashboard/projects/review-trigger-set.ts @@ -41,6 +41,12 @@ export default class ProjectsReviewTriggerSet extends DashboardCommand { allowNo: true, default: undefined, }), + 'pr-opened': Flags.boolean({ + description: + 'Enable respond-to-review on newly opened PRs (filtered by own-prs-only / external-prs).', + allowNo: true, + default: undefined, + }), }; async run(): Promise { @@ -49,10 +55,16 @@ export default class ProjectsReviewTriggerSet extends DashboardCommand { const ownPrsOnly = flags['own-prs-only']; const externalPrs = flags['external-prs']; const onReviewRequested = flags['on-review-requested']; + const prOpened = flags['pr-opened']; - if (ownPrsOnly === undefined && externalPrs === undefined && onReviewRequested === undefined) { + if ( + ownPrsOnly === undefined && + externalPrs === undefined && + onReviewRequested === undefined && + prOpened === undefined + ) { this.error( - 'At least one flag must be provided: --own-prs-only, --external-prs, --on-review-requested (use --no- to disable).', + 'At least one flag must be provided: --own-prs-only, --external-prs, --on-review-requested, --pr-opened (use --no- to disable).', ); } @@ -62,15 +74,20 @@ export default class ProjectsReviewTriggerSet extends DashboardCommand { if (externalPrs !== undefined) reviewTrigger.externalPrs = externalPrs; if (onReviewRequested !== undefined) reviewTrigger.onReviewRequested = onReviewRequested; + // Build the top-level triggers payload + const triggers: Record> = {}; + if (Object.keys(reviewTrigger).length > 0) triggers.reviewTrigger = reviewTrigger; + if (prOpened !== undefined) triggers.prOpened = prOpened; + try { await this.client.projects.integrations.updateTriggers.mutate({ projectId: args.id, category: 'scm', - triggers: { reviewTrigger }, + triggers, }); if (flags.json) { - this.outputJson({ ok: true, reviewTrigger }); + this.outputJson({ ok: true, ...triggers }); return; } @@ -78,6 +95,7 @@ export default class ProjectsReviewTriggerSet extends DashboardCommand { if (ownPrsOnly !== undefined) lines.push(` ownPrsOnly: ${ownPrsOnly}`); if (externalPrs !== undefined) lines.push(` externalPrs: ${externalPrs}`); if (onReviewRequested !== undefined) lines.push(` onReviewRequested: ${onReviewRequested}`); + if (prOpened !== undefined) lines.push(` prOpened: ${prOpened}`); this.log(lines.join('\n')); } catch (err) { this.handleError(err); diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index c2193753..dddf4e79 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -1,5 +1,7 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; -import { isCascadeBot } from '../../github/personas.js'; +import { + resolveGitHubTriggerEnabled, + resolveReviewTriggerConfig, +} from '../../config/triggerConfig.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { isGitHubPullRequestPayload } from './types.js'; @@ -22,6 +24,12 @@ export class PROpenedTrigger implements TriggerHandler { return false; } + // Respect reviewTrigger config — at least one author mode must be active + const reviewConfig = resolveReviewTriggerConfig(ctx.project.github?.triggers); + if (!reviewConfig.ownPrsOnly && !reviewConfig.externalPrs) { + return false; + } + // Only trigger on newly opened PRs if (ctx.payload.action !== 'opened') return false; @@ -47,9 +55,24 @@ export class PROpenedTrigger implements TriggerHandler { const prNumber = payload.pull_request.number; const prAuthor = payload.pull_request.user.login; - // Skip PRs authored by CASCADE bots — nothing to "respond to" on our own PRs - if (ctx.personaIdentities && isCascadeBot(prAuthor, ctx.personaIdentities)) { - logger.info('Skipping PR opened by CASCADE bot', { prNumber, prAuthor }); + // Gate on PR author based on configured review trigger modes + if (!ctx.personaIdentities) return null; + const implLogin = ctx.personaIdentities.implementer; + const isImplementerPR = prAuthor === implLogin || prAuthor === `${implLogin}[bot]`; + + const reviewConfig = resolveReviewTriggerConfig(ctx.project.github?.triggers); + const shouldTrigger = + (reviewConfig.ownPrsOnly && isImplementerPR) || + (reviewConfig.externalPrs && !isImplementerPR); + + if (!shouldTrigger) { + logger.info('PR author does not match any enabled review trigger mode, skipping', { + prNumber, + prAuthor, + isImplementerPR, + ownPrsOnly: reviewConfig.ownPrsOnly, + externalPrs: reviewConfig.externalPrs, + }); return null; } diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index 92d9d1d3..d17e3d6b 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -27,11 +27,27 @@ describe('PROpenedTrigger', () => { }, }; - /** Project with prOpened trigger explicitly enabled via github.triggers config */ + /** Project with prOpened + externalPrs enabled (most common config for external PR review) */ const mockProjectWithPrOpenedEnabled = { ...mockProject, github: { - triggers: { prOpened: true }, + triggers: { prOpened: true, reviewTrigger: { externalPrs: true } }, + }, + }; + + /** Project with prOpened + ownPrsOnly (fires on implementer-authored PRs) */ + const mockProjectWithOwnPrsOnly = { + ...mockProject, + github: { + triggers: { prOpened: true, reviewTrigger: { ownPrsOnly: true } }, + }, + }; + + /** Project with prOpened + both modes (fires on all PRs) */ + const mockProjectWithBothModes = { + ...mockProject, + github: { + triggers: { prOpened: true, reviewTrigger: { ownPrsOnly: true, externalPrs: true } }, }, }; @@ -67,7 +83,7 @@ describe('PROpenedTrigger', () => { expect(trigger.matches(ctx)).toBe(false); }); - it('matches when action is opened and not draft with prOpened enabled', () => { + it('matches when action is opened and not draft with prOpened + externalPrs enabled', () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', @@ -93,6 +109,100 @@ describe('PROpenedTrigger', () => { expect(trigger.matches(ctx)).toBe(true); }); + it('does not match when prOpened is true but neither ownPrsOnly nor externalPrs', () => { + const project = { + ...mockProject, + github: { + triggers: { + prOpened: true, + reviewTrigger: { ownPrsOnly: false, externalPrs: false }, + }, + }, + }; + const ctx: TriggerContext = { + project, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'desc', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('matches when prOpened is true with ownPrsOnly only', () => { + const ctx: TriggerContext = { + project: mockProjectWithOwnPrsOnly, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'desc', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('matches when prOpened is true with legacy config (no reviewTrigger, checkSuiteSuccess defaults to ownPrsOnly)', () => { + const project = { + ...mockProject, + github: { + triggers: { prOpened: true }, + }, + }; + const ctx: TriggerContext = { + project, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'desc', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + // Legacy fallback: checkSuiteSuccess defaults to true → ownPrsOnly = true + expect(trigger.matches(ctx)).toBe(true); + }); + it('does not match when source is not github', () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, @@ -174,6 +284,7 @@ describe('PROpenedTrigger', () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { action: 'opened', number: 42, @@ -215,6 +326,7 @@ describe('PROpenedTrigger', () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { action: 'opened', number: 42, @@ -240,7 +352,7 @@ describe('PROpenedTrigger', () => { expect(result?.workItemId).toBeUndefined(); }); - it('returns null for PRs by implementer persona', async () => { + it('returns null for implementer PR when only externalPrs is enabled', async () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', @@ -267,9 +379,9 @@ describe('PROpenedTrigger', () => { expect(await trigger.handle(ctx)).toBeNull(); }); - it('returns null for PRs by reviewer persona', async () => { + it('fires for implementer PR when ownPrsOnly is enabled', async () => { const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProjectWithOwnPrsOnly, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -284,17 +396,46 @@ describe('PROpenedTrigger', () => { draft: false, head: { ref: 'feature/login', sha: 'abc' }, base: { ref: 'main' }, - user: { login: 'cascade-review' }, + user: { login: 'cascade-impl' }, }, repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, - sender: { login: 'cascade-review' }, + sender: { login: 'cascade-impl' }, + }, + }; + + const result = await trigger.handle(ctx); + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('respond-to-review'); + }); + + it('returns null for external PR when only ownPrsOnly is enabled', async () => { + const ctx: TriggerContext = { + project: mockProjectWithOwnPrsOnly, + source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'feat: external change', + body: 'External contribution', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/external', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'external-dev' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'external-dev' }, }, }; expect(await trigger.handle(ctx)).toBeNull(); }); - it('returns null for [bot] variant', async () => { + it('returns null for implementer [bot] variant when only externalPrs is enabled', async () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', @@ -321,7 +462,7 @@ describe('PROpenedTrigger', () => { expect(await trigger.handle(ctx)).toBeNull(); }); - it('fires normally for external PRs with personaIdentities present', async () => { + it('fires for external PR when externalPrs is enabled', async () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', @@ -350,7 +491,87 @@ describe('PROpenedTrigger', () => { expect(result?.agentType).toBe('respond-to-review'); }); - it('fires normally without personaIdentities (graceful degradation)', async () => { + it('fires for reviewer persona PR when externalPrs is enabled (reviewer is not implementer)', async () => { + const ctx: TriggerContext = { + project: mockProjectWithPrOpenedEnabled, + 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('respond-to-review'); + }); + + it('fires for both implementer and external PRs when both modes enabled', async () => { + const implCtx: TriggerContext = { + project: mockProjectWithBothModes, + source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'impl PR', + body: 'PR by implementer', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/impl', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'cascade-impl' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'cascade-impl' }, + }, + }; + + const extCtx: TriggerContext = { + project: mockProjectWithBothModes, + source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, + payload: { + action: 'opened', + number: 43, + pull_request: { + number: 43, + title: 'ext PR', + body: 'PR by external dev', + html_url: 'https://github.com/owner/repo/pull/43', + state: 'open', + draft: false, + head: { ref: 'feature/ext', sha: 'def' }, + base: { ref: 'main' }, + user: { login: 'external-dev' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'external-dev' }, + }, + }; + + expect(await trigger.handle(implCtx)).not.toBeNull(); + expect(await trigger.handle(extCtx)).not.toBeNull(); + }); + + it('returns null without personaIdentities (cannot determine author type)', async () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', @@ -374,15 +595,14 @@ describe('PROpenedTrigger', () => { }, }; - const result = await trigger.handle(ctx); - expect(result).not.toBeNull(); - expect(result?.agentType).toBe('respond-to-review'); + expect(await trigger.handle(ctx)).toBeNull(); }); it('fires with undefined workItemId for null PR body', async () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { action: 'opened', number: 42, diff --git a/web/src/lib/trigger-agent-mapping.ts b/web/src/lib/trigger-agent-mapping.ts index ac4f655a..6c21a141 100644 --- a/web/src/lib/trigger-agent-mapping.ts +++ b/web/src/lib/trigger-agent-mapping.ts @@ -179,8 +179,9 @@ export const AGENT_TRIGGER_MAP: Record = { }, { key: 'prOpened', - label: 'PR Opened (opt-in)', - description: 'Trigger respond-to-review when a new PR is opened. Default disabled.', + label: 'PR Opened', + description: + 'Trigger respond-to-review when a new PR is opened. Respects Own PRs / External PRs modes from the review agent.', defaultValue: false, scmProvider: 'github', category: 'scm',