diff --git a/CLAUDE.md b/CLAUDE.md index c91b116e..c5c17829 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -172,6 +172,28 @@ cascade projects integration-credential-set --category scm --role r - `respond-to-pr-comment` skips @mentions from **any** known persona - `check-suite-success` checks reviews from the **reviewer** persona specifically +### Review Agent Scope (`reviewScope`) + +The `reviewScope` field in the SCM integration `triggers` JSONB controls which PRs trigger the `review` agent. It is an array of modes that compose: + +| Mode | Description | +|------|-------------| +| `own` | CI passes on a PR authored by the implementer persona | +| `all` | CI passes on any PR (non-implementer PRs included); also enables PR Opened for respond-to-review | +| `reviewRequested` | Review explicitly requested from a CASCADE persona | + +**Default: `["reviewRequested"]`** — review fires only when explicitly requested. Configure via the Dashboard (Agent Configs tab) or directly in the `triggers` JSONB column. + +Examples: +```json +{ "reviewScope": ["reviewRequested"] } // default — review on explicit request only +{ "reviewScope": ["own"] } // auto-review own PRs after CI passes +{ "reviewScope": ["own", "reviewRequested"] } // both: own PRs + explicit requests +{ "reviewScope": ["all"] } // auto-review all PRs after CI passes +``` + +**Migration note**: The old `checkSuiteSuccess` boolean defaulted to `true`, meaning CI-success auto-review on implementer PRs was enabled by default. The new `reviewScope` defaults to `['reviewRequested']`, which does **not** include `'own'`. Existing projects that relied on the default auto-review-after-CI behavior will need to explicitly set `reviewScope: ['own', 'reviewRequested']` to restore that behavior. Projects that had `checkSuiteSuccess: true` stored in their triggers JSONB should be updated — the old key is ignored by the new code. + ### Integration Credential Resolution Integration credentials are resolved by `(projectId, category, role)`: diff --git a/src/api/routers/projects.ts b/src/api/routers/projects.ts index 5cc571ee..1c390ec2 100644 --- a/src/api/routers/projects.ts +++ b/src/api/routers/projects.ts @@ -124,7 +124,9 @@ export const projectsRouter = router({ category: z.enum(['pm', 'scm']), provider: z.string().min(1), config: z.record(z.unknown()), - triggers: z.record(z.boolean()).optional(), + triggers: z + .record(z.union([z.boolean(), z.record(z.boolean()), z.array(z.string())])) + .optional(), }), ) .mutation(async ({ ctx, input }) => { @@ -143,7 +145,7 @@ export const projectsRouter = router({ z.object({ projectId: z.string(), category: z.enum(['pm', 'scm']), - triggers: z.record(z.union([z.boolean(), z.record(z.boolean())])), + triggers: z.record(z.union([z.boolean(), z.record(z.boolean()), z.array(z.string())])), }), ) .mutation(async ({ ctx, input }) => { diff --git a/src/config/triggerConfig.ts b/src/config/triggerConfig.ts index b28df502..dec39e42 100644 --- a/src/config/triggerConfig.ts +++ b/src/config/triggerConfig.ts @@ -43,27 +43,48 @@ export const JiraTriggerConfigSchema = z.object({ commentMention: z.boolean().default(true), }); +/** + * Controls which PRs trigger the review agent. + * - `own`: CI passes on a PR authored by the implementer persona + * - `all`: CI passes on any PR (non-implementer PRs included) + * - `reviewRequested`: review is explicitly requested from a CASCADE persona + * Modes compose — e.g., `['own', 'reviewRequested']` enables both. + * Default: `['reviewRequested']` — review only fires when explicitly requested. + */ +export const ReviewScopeSchema = z + .array(z.enum(['own', 'all', 'reviewRequested'])) + .default(['reviewRequested']); + +export type ReviewScope = z.infer; + /** * Trigger configuration for GitHub integrations. - * Existing triggers default to `true`; new triggers (`reviewRequested`, `prOpened`) default to `false`. */ export const GitHubTriggerConfigSchema = z.object({ - checkSuiteSuccess: z.boolean().default(true), checkSuiteFailure: z.boolean().default(true), prReviewSubmitted: z.boolean().default(true), prCommentMention: z.boolean().default(true), prReadyToMerge: z.boolean().default(true), prMerged: z.boolean().default(true), - /** New trigger: fires review agent when review is requested from a CASCADE persona. Default false (opt-in). */ - reviewRequested: z.boolean().default(false), - /** PR opened trigger. Default false (disabled until reviewed). */ - prOpened: z.boolean().default(false), + /** Controls which PRs trigger the review agent. See ReviewScopeSchema for options. */ + reviewScope: ReviewScopeSchema, }); export type TrelloTriggerConfig = z.infer; export type JiraTriggerConfig = z.infer; export type GitHubTriggerConfig = z.infer; +// ============================================================================ +// Review Scope Helper +// ============================================================================ + +/** + * Returns whether the given reviewScope array includes the specified mode. + */ +export function isReviewScopeEnabled(scope: ReviewScope, mode: ReviewScope[number]): boolean { + return scope.includes(mode); +} + // ============================================================================ // Helpers // ============================================================================ @@ -132,24 +153,33 @@ export function resolveJiraTriggerEnabled( return value === undefined ? true : (value as boolean); } +/** Boolean-only keys from GitHubTriggerConfig (excludes array fields like reviewScope). */ +type GitHubBooleanTriggerKey = { + [K in keyof GitHubTriggerConfig]: GitHubTriggerConfig[K] extends boolean ? K : never; +}[keyof GitHubTriggerConfig]; + /** * Resolve whether a GitHub trigger is enabled based on project trigger config. - * For new opt-in triggers (reviewRequested, prOpened), returns `false` when no config is present. + * Returns `true` (enabled) when no config is present (backward compatible). + * Only accepts boolean trigger keys — use `resolveReviewScope` for array-typed fields. */ export function resolveGitHubTriggerEnabled( config: Partial | undefined, - key: keyof GitHubTriggerConfig, + key: GitHubBooleanTriggerKey, ): boolean { - if (!config) { - // New triggers that are opt-in default to false even without config - if (key === 'reviewRequested' || key === 'prOpened') return false; - return true; - } + if (!config) return true; const value = config[key]; - if (value === undefined) { - // New triggers that are opt-in default to false - if (key === 'reviewRequested' || key === 'prOpened') return false; - return true; - } + if (value === undefined) return true; return value; } + +/** + * Resolve the reviewScope from project GitHub trigger config. + * Returns the default scope when no config is present. + */ +export function resolveReviewScope(config: Partial | undefined): ReviewScope { + if (!config || config.reviewScope === undefined) { + return ReviewScopeSchema.parse(undefined); + } + return config.reviewScope; +} diff --git a/src/db/repositories/settingsRepository.ts b/src/db/repositories/settingsRepository.ts index 5dd49306..856c3980 100644 --- a/src/db/repositories/settingsRepository.ts +++ b/src/db/repositories/settingsRepository.ts @@ -166,14 +166,15 @@ export async function upsertProjectIntegration( category: string, provider: string, config: Record, - triggers?: Record, + triggers?: Record | string[]>, ) { const db = getDb(); // Preserve existing triggers if not provided (prevents data loss from Integration tab saves) let triggersToSave = triggers; if (triggersToSave === undefined) { const existing = await getIntegrationByProjectAndCategory(projectId, category); - triggersToSave = (existing?.triggers as Record) ?? {}; + triggersToSave = + (existing?.triggers as Record | string[]>) ?? {}; } const [row] = await db .insert(projectIntegrations) diff --git a/src/triggers/builtins.ts b/src/triggers/builtins.ts index 032e5e8c..549ac27e 100644 --- a/src/triggers/builtins.ts +++ b/src/triggers/builtins.ts @@ -55,7 +55,7 @@ export function registerBuiltInTriggers(registry: TriggerRegistry): void { registry.register(new JiraReadyToProcessLabelTrigger()); // GitHub: PR opened trigger (initial review on new PRs) - // Opt-in: disabled by default via trigger config (github.triggers.prOpened = false) + // Fires when reviewScope includes 'all' (disabled by default — default reviewScope is ['reviewRequested']) registry.register(new PROpenedTrigger()); // GitHub: PR comment @mention trigger (runs respond-to-pr-comment when reviewer is @mentioned) @@ -66,7 +66,7 @@ export function registerBuiltInTriggers(registry: TriggerRegistry): void { registry.register(new PRReviewSubmittedTrigger()); // GitHub: Review requested trigger (runs review agent when review is requested from CASCADE persona) - // Opt-in: disabled by default via trigger config (github.triggers.reviewRequested = false) + // Fires when reviewScope includes 'reviewRequested' (enabled by default — default reviewScope is ['reviewRequested']) // Registered before CheckSuiteSuccessTrigger so both can independently trigger review registry.register(new ReviewRequestedTrigger()); diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index 566825ca..beb0b8a4 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -1,4 +1,4 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; +import { isReviewScopeEnabled, resolveReviewScope } from '../../config/triggerConfig.js'; import { type CheckSuiteStatus, githubClient } from '../../github/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -66,8 +66,9 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubCheckSuitePayload(ctx.payload)) return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'checkSuiteSuccess')) { + // Check trigger config — only fire when reviewScope includes 'own' or 'all' + const reviewScope = resolveReviewScope(ctx.project.github?.triggers); + if (!isReviewScopeEnabled(reviewScope, 'own') && !isReviewScopeEnabled(reviewScope, 'all')) { return false; } @@ -99,15 +100,18 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { // Fetch PR details const prDetails = await githubClient.getPR(owner, repo, prNumber); - // Gate on PR author being the implementer persona - if (!ctx.personaIdentities) return null; - const implLogin = ctx.personaIdentities.implementer; - if (prDetails.user.login !== implLogin && prDetails.user.login !== `${implLogin}[bot]`) { - logger.info('PR not authored by implementer persona, skipping', { - prNumber, - prAuthor: prDetails.user.login, - }); - return null; + // Gate on PR author being the implementer persona — unless scope includes 'all' + const reviewScope = resolveReviewScope(ctx.project.github?.triggers); + if (!isReviewScopeEnabled(reviewScope, 'all')) { + if (!ctx.personaIdentities) return null; + const implLogin = ctx.personaIdentities.implementer; + if (prDetails.user.login !== implLogin && prDetails.user.login !== `${implLogin}[bot]`) { + logger.info('PR not authored by implementer persona, skipping', { + prNumber, + prAuthor: prDetails.user.login, + }); + return null; + } } // Only trigger for PRs targeting the project's base branch @@ -132,7 +136,7 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { const reviews = await githubClient.getPRReviews(owner, repo, prNumber); // Use persona identities to identify reviewer bot's reviews - const reviewerUsername = ctx.personaIdentities.reviewer; + const reviewerUsername = ctx.personaIdentities?.reviewer; // Only consider actual reviews (approved/changes_requested), not COMMENTED // which are reply acknowledgments posted by respond-to-review agent diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index fe315449..d0fe9463 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -1,4 +1,4 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; +import { isReviewScopeEnabled, resolveReviewScope } from '../../config/triggerConfig.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { isGitHubPullRequestPayload } from './types.js'; @@ -16,8 +16,9 @@ export class PROpenedTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubPullRequestPayload(ctx.payload)) return false; - // Check trigger config — opt-in trigger, default disabled - if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prOpened')) { + // Check trigger config — only fire when reviewScope includes 'all' + const reviewScope = resolveReviewScope(ctx.project.github?.triggers); + if (!isReviewScopeEnabled(reviewScope, 'all')) { return false; } diff --git a/src/triggers/github/review-requested.ts b/src/triggers/github/review-requested.ts index 14d4524c..5bf70000 100644 --- a/src/triggers/github/review-requested.ts +++ b/src/triggers/github/review-requested.ts @@ -1,4 +1,4 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; +import { isReviewScopeEnabled, resolveReviewScope } from '../../config/triggerConfig.js'; import { isCascadeBot } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -13,8 +13,9 @@ import { resolveWorkItemId } from './utils.js'; * 2. Checks if the requested reviewer is a CASCADE persona (implementer OR reviewer) * 3. Fires the `review` agent with PR number and work item ID from PR body * - * Default: **disabled** (opt-in via trigger config). Enable by setting - * `github.triggers.reviewRequested = true` in integration config. + * Default: **enabled** — the default `reviewScope` of `['reviewRequested']` includes + * this trigger. Disable by setting `reviewScope` to a value that excludes + * `'reviewRequested'` (e.g., `['own']`). * * Registration: should be registered BEFORE CheckSuiteSuccessTrigger so that * both triggers can independently fire review. The HEAD-SHA dedup in @@ -31,8 +32,9 @@ export class ReviewRequestedTrigger implements TriggerHandler { // Only trigger on review_requested events if (ctx.payload.action !== 'review_requested') return false; - // Check trigger config — opt-in trigger, default disabled - if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'reviewRequested')) { + // Check trigger config — only fire when reviewScope includes 'reviewRequested' + const reviewScope = resolveReviewScope(ctx.project.github?.triggers); + if (!isReviewScopeEnabled(reviewScope, 'reviewRequested')) { return false; } diff --git a/tests/unit/config/triggerConfig.test.ts b/tests/unit/config/triggerConfig.test.ts index 1da86683..abe34a18 100644 --- a/tests/unit/config/triggerConfig.test.ts +++ b/tests/unit/config/triggerConfig.test.ts @@ -3,9 +3,11 @@ import { GitHubTriggerConfigSchema, JiraTriggerConfigSchema, TrelloTriggerConfigSchema, + isReviewScopeEnabled, resolveGitHubTriggerEnabled, resolveJiraTriggerEnabled, resolveReadyToProcessEnabled, + resolveReviewScope, resolveTrelloTriggerEnabled, } from '../../../src/config/triggerConfig.js'; @@ -58,7 +60,6 @@ describe('JiraTriggerConfigSchema', () => { describe('GitHubTriggerConfigSchema', () => { it('defaults existing triggers to true', () => { const result = GitHubTriggerConfigSchema.parse({}); - expect(result.checkSuiteSuccess).toBe(true); expect(result.checkSuiteFailure).toBe(true); expect(result.prReviewSubmitted).toBe(true); expect(result.prCommentMention).toBe(true); @@ -66,10 +67,18 @@ describe('GitHubTriggerConfigSchema', () => { expect(result.prMerged).toBe(true); }); - it('defaults new opt-in triggers to false', () => { + it('defaults reviewScope to ["reviewRequested"]', () => { const result = GitHubTriggerConfigSchema.parse({}); - expect(result.reviewRequested).toBe(false); - expect(result.prOpened).toBe(false); + expect(result.reviewScope).toEqual(['reviewRequested']); + }); + + it('accepts custom reviewScope', () => { + const result = GitHubTriggerConfigSchema.parse({ reviewScope: ['own', 'all'] }); + expect(result.reviewScope).toEqual(['own', 'all']); + }); + + it('rejects invalid reviewScope values', () => { + expect(() => GitHubTriggerConfigSchema.parse({ reviewScope: ['invalid'] })).toThrow(); }); }); @@ -141,7 +150,6 @@ describe('resolveJiraTriggerEnabled', () => { describe('resolveGitHubTriggerEnabled', () => { it('returns true for existing triggers when config is undefined', () => { - expect(resolveGitHubTriggerEnabled(undefined, 'checkSuiteSuccess')).toBe(true); expect(resolveGitHubTriggerEnabled(undefined, 'checkSuiteFailure')).toBe(true); expect(resolveGitHubTriggerEnabled(undefined, 'prReviewSubmitted')).toBe(true); expect(resolveGitHubTriggerEnabled(undefined, 'prCommentMention')).toBe(true); @@ -149,25 +157,50 @@ describe('resolveGitHubTriggerEnabled', () => { expect(resolveGitHubTriggerEnabled(undefined, 'prMerged')).toBe(true); }); - it('returns false for opt-in triggers when config is undefined', () => { - expect(resolveGitHubTriggerEnabled(undefined, 'reviewRequested')).toBe(false); - expect(resolveGitHubTriggerEnabled(undefined, 'prOpened')).toBe(false); + it('returns true when config is empty (backward compatible)', () => { + expect(resolveGitHubTriggerEnabled({}, 'checkSuiteFailure')).toBe(true); }); - it('returns false for opt-in triggers when config is empty', () => { - expect(resolveGitHubTriggerEnabled({}, 'reviewRequested')).toBe(false); - expect(resolveGitHubTriggerEnabled({}, 'prOpened')).toBe(false); + it('returns false when trigger is explicitly disabled', () => { + expect(resolveGitHubTriggerEnabled({ checkSuiteFailure: false }, 'checkSuiteFailure')).toBe( + false, + ); }); +}); - it('returns true for opt-in triggers when explicitly enabled', () => { - expect(resolveGitHubTriggerEnabled({ reviewRequested: true }, 'reviewRequested')).toBe(true); - expect(resolveGitHubTriggerEnabled({ prOpened: true }, 'prOpened')).toBe(true); +describe('resolveReviewScope', () => { + it('returns default ["reviewRequested"] when config is undefined', () => { + expect(resolveReviewScope(undefined)).toEqual(['reviewRequested']); }); - it('returns false when existing trigger is explicitly disabled', () => { - expect(resolveGitHubTriggerEnabled({ checkSuiteSuccess: false }, 'checkSuiteSuccess')).toBe( - false, - ); + it('returns default ["reviewRequested"] when config has no reviewScope', () => { + expect(resolveReviewScope({})).toEqual(['reviewRequested']); + }); + + it('returns the configured scope', () => { + expect(resolveReviewScope({ reviewScope: ['own', 'all'] })).toEqual(['own', 'all']); + }); + + it('returns empty array when reviewScope is explicitly set to []', () => { + expect(resolveReviewScope({ reviewScope: [] })).toEqual([]); + }); +}); + +describe('isReviewScopeEnabled', () => { + it('returns true when mode is in scope', () => { + expect(isReviewScopeEnabled(['own', 'reviewRequested'], 'own')).toBe(true); + expect(isReviewScopeEnabled(['own', 'reviewRequested'], 'reviewRequested')).toBe(true); + }); + + it('returns false when mode is not in scope', () => { + expect(isReviewScopeEnabled(['reviewRequested'], 'own')).toBe(false); + expect(isReviewScopeEnabled(['own'], 'all')).toBe(false); + }); + + it('returns false for empty scope', () => { + expect(isReviewScopeEnabled([], 'own')).toBe(false); + expect(isReviewScopeEnabled([], 'all')).toBe(false); + expect(isReviewScopeEnabled([], 'reviewRequested')).toBe(false); }); }); diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index 02ec10d5..6933ab78 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -36,6 +36,9 @@ describe('CheckSuiteSuccessTrigger', () => { }, labels: {}, }, + github: { + triggers: { reviewScope: ['own'] as const }, + }, }; const mockPersonaIdentities = { @@ -147,6 +150,45 @@ describe('CheckSuiteSuccessTrigger', () => { expect(trigger.matches(ctx)).toBe(false); }); + + it('does not match when reviewScope does not include own or all', () => { + const ctx: TriggerContext = { + project: { + ...mockProject, + github: { triggers: { reviewScope: ['reviewRequested'] as const } }, + }, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('matches when reviewScope includes all', () => { + const ctx: TriggerContext = { + project: { + ...mockProject, + github: { triggers: { reviewScope: ['all'] as const } }, + }, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match by default (default reviewScope is ["reviewRequested"])', () => { + const ctxNoConfig: TriggerContext = { + project: { + ...mockProject, + github: undefined, + }, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + expect(trigger.matches(ctxNoConfig)).toBe(false); + }); }); describe('handle', () => { @@ -687,6 +729,69 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result?.agentInput.cardId).toBeUndefined(); }); + it('allows non-implementer PRs when reviewScope is ["all"]', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'some-human' }, + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'success' }], + }); + + const ctx: TriggerContext = { + project: { + ...mockProject, + github: { triggers: { reviewScope: ['all'] as const } }, + }, + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('review'); + }); + + it('blocks non-implementer PRs when reviewScope is ["own"]', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + htmlUrl: 'https://github.com/owner/repo/pull/42', + user: { login: 'some-human' }, + }); + + const ctx: TriggerContext = { + project: mockProject, // reviewScope: ['own'] + source: 'github', + payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); + }); + it('uses DB lookup result over PR body extraction', async () => { vi.mocked(lookupWorkItemForPR).mockResolvedValue('db-work-item'); vi.mocked(githubClient.getPR).mockResolvedValue({ diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index b44471cc..d5aeb4ac 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -27,11 +27,11 @@ describe('PROpenedTrigger', () => { }, }; - /** Project with prOpened trigger explicitly enabled via github.triggers config */ + /** Project with prOpened trigger enabled via reviewScope: ['all'] */ const mockProjectWithPrOpenedEnabled = { ...mockProject, github: { - triggers: { prOpened: true }, + triggers: { reviewScope: ['all'] as const }, }, }; @@ -41,7 +41,7 @@ describe('PROpenedTrigger', () => { }); describe('matches', () => { - it('does not match by default (opt-in trigger, disabled without config)', () => { + it('does not match by default (default reviewScope does not include "all")', () => { const ctx: TriggerContext = { project: mockProject, source: 'github', @@ -67,7 +67,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 reviewScope ["all"]', () => { const ctx: TriggerContext = { project: mockProjectWithPrOpenedEnabled, source: 'github', diff --git a/tests/unit/triggers/review-requested.test.ts b/tests/unit/triggers/review-requested.test.ts index b6156bba..a8f1ae9d 100644 --- a/tests/unit/triggers/review-requested.test.ts +++ b/tests/unit/triggers/review-requested.test.ts @@ -28,11 +28,11 @@ describe('ReviewRequestedTrigger', () => { // Review-requested is opt-in, default disabled }; - /** Project with reviewRequested trigger explicitly enabled */ + /** Project with reviewRequested trigger enabled via reviewScope */ const mockProjectWithReviewRequested = { ...mockProject, github: { - triggers: { reviewRequested: true }, + triggers: { reviewScope: ['reviewRequested'] as const }, }, }; @@ -77,13 +77,28 @@ describe('ReviewRequestedTrigger', () => { }); describe('matches', () => { - it('does not match by default (opt-in trigger, disabled without config)', () => { + it('matches by default (default reviewScope includes reviewRequested)', () => { const ctx: TriggerContext = { project: mockProject, source: 'github', payload: makeReviewRequestedPayload(), personaIdentities: mockPersonaIdentities, }; + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match when reviewScope is ["own"] (reviewRequested not included)', () => { + const ctx: TriggerContext = { + project: { + ...mockProject, + github: { + triggers: { reviewScope: ['own'] as const }, + }, + }, + source: 'github', + payload: makeReviewRequestedPayload(), + personaIdentities: mockPersonaIdentities, + }; expect(trigger.matches(ctx)).toBe(false); }); diff --git a/web/src/components/projects/project-agent-configs.tsx b/web/src/components/projects/project-agent-configs.tsx index 8939a60a..59975815 100644 --- a/web/src/components/projects/project-agent-configs.tsx +++ b/web/src/components/projects/project-agent-configs.tsx @@ -55,9 +55,26 @@ function AgentConfigBadge({ config }: { config: AgentConfig | null }) { return {parts.join(' · ')}; } +/** Extract a nested dot-notation key value from a triggers record. */ +function extractNestedTrigger( + relevant: Record, + allTriggers: Record, + key: string, + defaultValue: boolean, +): void { + const [parent, child] = key.split('.'); + if (!(parent in relevant)) relevant[parent] = {}; + const parentObj = + typeof allTriggers[parent] === 'object' && allTriggers[parent] !== null + ? allTriggers[parent] + : {}; + (relevant[parent] as Record)[child] = + ((parentObj as Record)[child] as boolean) ?? defaultValue; +} + /** * Extract only the trigger keys relevant to the given trigger definitions - * from the full triggers record. Handles nested dot-notation keys. + * from the full triggers record. Handles nested dot-notation keys and multi-select arrays. */ function extractRelevantTriggers( triggerDefs: ReturnType, @@ -65,18 +82,11 @@ function extractRelevantTriggers( ): Record { const relevant: Record = {}; for (const t of triggerDefs) { - const parts = t.key.split('.'); - if (parts.length > 1) { - const [parent, child] = parts; - if (!(parent in relevant)) { - relevant[parent] = {}; - } - const parentObj = - typeof allTriggers[parent] === 'object' && allTriggers[parent] !== null - ? allTriggers[parent] - : {}; - (relevant[parent] as Record)[child] = - ((parentObj as Record)[child] as boolean) ?? t.defaultValue; + if (t.inputType === 'multi-select') { + const val = allTriggers[t.key]; + relevant[t.key] = Array.isArray(val) ? val : (t.defaultArrayValue ?? []); + } else if (t.key.includes('.')) { + extractNestedTrigger(relevant, allTriggers, t.key, t.defaultValue); } else { relevant[t.key] = allTriggers[t.key] ?? t.defaultValue; } @@ -146,7 +156,13 @@ function AgentSection({ try { const relevant: Record = {}; for (const t of agentScmTriggers) { - relevant[t.key] = localScmTriggers[t.key] ?? t.defaultValue; + if (t.inputType === 'multi-select') { + // Preserve array values for multi-select triggers + const val = localScmTriggers[t.key]; + relevant[t.key] = Array.isArray(val) ? val : (t.defaultArrayValue ?? []); + } else { + relevant[t.key] = localScmTriggers[t.key] ?? t.defaultValue; + } } await onSaveTriggers('scm', relevant, agentType); setScmSaved(true); @@ -354,7 +370,7 @@ export function ProjectAgentConfigs({ projectId }: { projectId: string }) { trpcClient.projects.integrations.updateTriggers.mutate({ projectId, category, - triggers: triggers as Record>, + triggers: triggers as Record | string[]>, }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: integrationsQueryKey }); diff --git a/web/src/components/shared/trigger-toggles.tsx b/web/src/components/shared/trigger-toggles.tsx index dab2e6ed..435a9195 100644 --- a/web/src/components/shared/trigger-toggles.tsx +++ b/web/src/components/shared/trigger-toggles.tsx @@ -1,12 +1,27 @@ import { Label } from '@/components/ui/label.js'; -import { type TriggerDef, getTriggerValue, setTriggerValue } from '@/lib/trigger-agent-mapping.js'; +import { + type TriggerDef, + getMultiSelectValue, + getTriggerValue, + setMultiSelectValue, + setTriggerValue, +} from '@/lib/trigger-agent-mapping.js'; export type { TriggerDef }; +function optionLabel(option: string): string { + if (option === 'own') return 'Own PRs (implementer-authored)'; + if (option === 'all') return 'All PRs'; + if (option === 'reviewRequested') return 'Review Requested'; + return option; +} + /** * Renders a list of trigger toggle checkboxes. * Supports both flat keys (e.g., "cardMovedToBriefing") and nested dot-notation * keys (e.g., "readyToProcessLabel.briefing"). + * When inputType === 'multi-select', renders one checkbox per option in item.options, + * storing the value as an array in the triggers record. */ export function TriggerToggles({ title, @@ -27,8 +42,45 @@ export function TriggerToggles({
{title && } {items.map((item) => { - const value = getTriggerValue(values, item.key, item.defaultValue); const htmlId = `trigger-${idPrefix ? `${idPrefix}-` : ''}${item.key}`; + + if (item.inputType === 'multi-select' && item.options) { + // Multi-select: render one checkbox per option + const selected = getMultiSelectValue(values, item.key, item.defaultArrayValue); + return ( +
+
+ {item.label} +

{item.description}

+
+
+ {item.options.map((option) => { + const optionId = `${htmlId}-${option}`; + const checked = selected.includes(option); + return ( +
+ + onChange(setMultiSelectValue(values, item.key, option, e.target.checked)) + } + className="h-4 w-4 rounded border-input" + /> + +
+ ); + })} +
+
+ ); + } + + // Standard checkbox + const value = getTriggerValue(values, item.key, item.defaultValue); return (
= { ], review: [ { - key: 'checkSuiteSuccess', - label: 'Check Suite Success', - description: 'Trigger review agent when all CI checks pass.', - defaultValue: true, - scmProvider: 'github', - category: 'scm', - }, - { - key: 'reviewRequested', - label: 'Review Requested (opt-in)', + key: 'reviewScope', + label: 'Review Scope', description: - 'Trigger review agent when review is requested from a CASCADE persona. Default disabled.', + 'Controls which PRs trigger the review agent. Select one or more modes. "Own PRs" = CI passes on implementer-authored PRs. "All PRs" = CI passes on any PR (also enables PR Opened for respond-to-review). "Review Requested" = review explicitly requested from a CASCADE persona.', defaultValue: false, + inputType: 'multi-select' as const, + options: ['own', 'all', 'reviewRequested'], + defaultArrayValue: ['reviewRequested'], scmProvider: 'github', category: 'scm', }, @@ -149,14 +150,6 @@ export const AGENT_TRIGGER_MAP: Record = { scmProvider: 'github', category: 'scm', }, - { - key: 'prOpened', - label: 'PR Opened (opt-in)', - description: 'Trigger respond-to-review when a new PR is opened. Default disabled.', - defaultValue: false, - scmProvider: 'github', - category: 'scm', - }, ], 'respond-to-ci': [ { @@ -222,6 +215,36 @@ export function getTriggerValue( return defaultValue; } +/** + * Get a multi-select (array) value from a triggers record. + * Returns the stored array, the provided defaultValue, or an empty array if absent. + */ +export function getMultiSelectValue( + triggers: Record, + key: string, + defaultValue?: string[], +): string[] { + const val = triggers[key]; + if (Array.isArray(val)) return val as string[]; + return defaultValue ?? []; +} + +/** + * Toggle an option in a multi-select array value and return a new triggers record. + */ +export function setMultiSelectValue( + triggers: Record, + key: string, + option: string, + checked: boolean, +): Record { + const current = getMultiSelectValue(triggers, key); + const next = checked + ? [...current.filter((v) => v !== option), option] + : current.filter((v) => v !== option); + return { ...triggers, [key]: next }; +} + /** * Set a trigger value using dot-notation path, returning a new triggers record. */