From 589fe6ead52bf9747be002afbeaa1f14946b0dc7 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sat, 7 Mar 2026 07:03:24 +0000 Subject: [PATCH 1/3] =?UTF-8?q?refactor:=20clean=20up=20trigger=20config?= =?UTF-8?q?=20migration=20=E2=80=94=20deduplicate,=20validate,=20and=20har?= =?UTF-8?q?den?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `checkTriggerEnabledWithParams()` to combine enabled check + parameter fetch in a single DB call (eliminates double query in check-suite-success and pr-opened handlers) - Extract `evaluateAuthorMode()` into `src/triggers/github/utils.ts` to deduplicate ~25 lines of authorMode gating logic across two handlers, with proper validation against ['own', 'external', 'all'] and fallback - Switch pr-ready-to-merge and pr-merged from direct `isTriggerEnabled()` to shared `checkTriggerEnabled()` helper for consistent logging - Add debug log for no-@mention path in pr-comment-mention - Remove ~1900 lines of legacy trigger config schemas and tests - Add disabled-trigger + argument verification tests to all 13 handler test files for complete coverage of the DB-driven trigger config path Co-Authored-By: Claude Opus 4.6 --- src/agents/definitions/review.yaml | 19 + src/config/schema.ts | 17 - src/config/triggerConfig.ts | 390 --------- src/db/repositories/configMapper.ts | 36 +- src/db/repositories/configRepository.ts | 12 +- src/pm/config.ts | 12 - src/triggers/github/check-suite-failure.ts | 24 +- src/triggers/github/check-suite-success.ts | 57 +- src/triggers/github/pr-comment-mention.ts | 20 +- src/triggers/github/pr-merged.ts | 19 +- src/triggers/github/pr-opened.ts | 59 +- src/triggers/github/pr-ready-to-merge.ts | 14 +- src/triggers/github/pr-review-submitted.ts | 19 +- src/triggers/github/review-requested.ts | 16 +- src/triggers/jira/comment-mention.ts | 20 +- src/triggers/jira/label-added.ts | 18 +- src/triggers/jira/status-changed.ts | 18 +- src/triggers/shared/trigger-check.ts | 54 ++ src/triggers/trello/comment-mention.ts | 19 +- src/triggers/trello/label-added.ts | 16 +- src/triggers/trello/status-changed.ts | 19 +- tests/unit/config/triggerConfig.test.ts | 814 ------------------ .../unit/db/repositories/configMapper.test.ts | 45 +- .../triggers/backlog-status-changed.test.ts | 8 + .../unit/triggers/check-suite-failure.test.ts | 30 + .../unit/triggers/check-suite-success.test.ts | 120 ++- .../github-pr-comment-mention.test.ts | 26 + .../triggers/jira-comment-mention.test.ts | 26 +- tests/unit/triggers/jira-label-added.test.ts | 22 + .../unit/triggers/jira-status-changed.test.ts | 85 +- tests/unit/triggers/label-added.test.ts | 49 ++ tests/unit/triggers/pr-merged.test.ts | 81 +- tests/unit/triggers/pr-opened.test.ts | 289 +++---- tests/unit/triggers/pr-ready-to-merge.test.ts | 40 + .../unit/triggers/pr-review-submitted.test.ts | 30 + tests/unit/triggers/review-requested.test.ts | 114 +-- tests/unit/triggers/status-changed.test.ts | 41 + .../triggers/trello-comment-mention.test.ts | 23 + 38 files changed, 835 insertions(+), 1886 deletions(-) create mode 100644 src/triggers/shared/trigger-check.ts diff --git a/src/agents/definitions/review.yaml b/src/agents/definitions/review.yaml index 5a4eb1db..0164c247 100644 --- a/src/agents/definitions/review.yaml +++ b/src/agents/definitions/review.yaml @@ -48,7 +48,26 @@ triggers: description: Trigger review when a new PR is opened (without waiting for CI) defaultEnabled: false providers: [github] + parameters: + - name: authorMode + type: select + label: Author Filter + description: Filter PRs by author type + options: [own, external, all] + defaultValue: own contextPipeline: [prContext, contextFiles, squint] + - event: scm:pr-ready-to-merge + label: PR Ready to Merge + description: Move work item to DONE when PR is approved and all checks pass + defaultEnabled: true + providers: [github] + contextPipeline: [] + - event: scm:pr-merged + label: PR Merged + description: Move work item to MERGED status when PR is merged + defaultEnabled: true + providers: [github] + contextPipeline: [] strategies: {} diff --git a/src/config/schema.ts b/src/config/schema.ts index 8aa1da3b..0e11144d 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -1,9 +1,4 @@ import { z } from 'zod'; -import { - GitHubTriggerConfigSchema, - JiraTriggerConfigSchema, - TrelloTriggerConfigSchema, -} from './triggerConfig.js'; const AgentBackendConfigSchema = z.object({ default: z.string().default('llmist'), @@ -29,7 +24,6 @@ const JiraConfigSchema = z.object({ readyToProcess: z.string().default('cascade-ready'), }) .optional(), - triggers: JiraTriggerConfigSchema.partial().optional(), }); export const ProjectConfigSchema = z.object({ @@ -59,22 +53,11 @@ export const ProjectConfigSchema = z.object({ cost: z.string().optional(), }) .optional(), - triggers: TrelloTriggerConfigSchema.partial().optional(), }) .optional(), jira: JiraConfigSchema.optional(), - /** - * GitHub-specific configuration, including trigger toggles. - * Separate from trello/jira because GitHub integration is always present for code operations. - */ - github: z - .object({ - triggers: GitHubTriggerConfigSchema.partial().optional(), - }) - .optional(), - model: z.string().optional(), agentModels: z.record(z.string()).optional(), cardBudgetUsd: z.number().positive().optional(), diff --git a/src/config/triggerConfig.ts b/src/config/triggerConfig.ts index 19a9a14a..0cfe4a29 100644 --- a/src/config/triggerConfig.ts +++ b/src/config/triggerConfig.ts @@ -1,126 +1,5 @@ import { z } from 'zod'; -// ============================================================================ -// Trigger Config Schemas -// ============================================================================ - -/** - * Per-agent ready-to-process label configuration. - * Each agent type can independently toggle whether the label trigger fires for it. - */ -export const ReadyToProcessLabelSchema = z - .union([ - z.boolean(), - z.object({ - splitting: z.boolean().default(true), - planning: z.boolean().default(true), - implementation: z.boolean().default(true), - 'backlog-manager': z.boolean().default(true), - }), - ]) - .optional(); - -export type ReadyToProcessLabelConfig = z.infer; - -/** - * Per-agent status-changed configuration. - * Each agent type can independently toggle whether the status-changed trigger fires for it. - */ -export const StatusChangedSchema = z - .union([ - z.boolean(), - z.object({ - splitting: z.boolean().default(true), - planning: z.boolean().default(true), - implementation: z.boolean().default(true), - 'backlog-manager': z.boolean().default(true), - }), - ]) - .optional(); - -export type StatusChangedConfig = z.infer; - -/** - * Trigger configuration for Trello integrations. - * All triggers default to `true` for backward compatibility. - * - * `statusChanged` is the unified key replacing the legacy `cardMovedToSplitting`, - * `cardMovedToPlanning`, and `cardMovedToTodo` keys. - */ -export const TrelloTriggerConfigSchema = z.object({ - /** Unified status-changed toggle (replaces legacy cardMovedTo* keys). */ - statusChanged: StatusChangedSchema.default(true), - /** @deprecated Use `statusChanged` instead. */ - cardMovedToSplitting: z.boolean().default(true), - /** @deprecated Use `statusChanged` instead. */ - cardMovedToPlanning: z.boolean().default(true), - /** @deprecated Use `statusChanged` instead. */ - cardMovedToTodo: z.boolean().default(true), - readyToProcessLabel: ReadyToProcessLabelSchema, - commentMention: z.boolean().default(true), -}); - -/** - * Trigger configuration for JIRA integrations. - * All triggers default to `true` for backward compatibility. - * - * `statusChanged` is the unified key replacing the legacy `issueTransitioned` key. - */ -export const JiraTriggerConfigSchema = z.object({ - /** Unified status-changed toggle (replaces legacy issueTransitioned key). */ - statusChanged: StatusChangedSchema, - /** @deprecated Use `statusChanged` instead. */ - issueTransitioned: StatusChangedSchema, - readyToProcessLabel: ReadyToProcessLabelSchema, - commentMention: z.boolean().default(true), -}); - -/** - * Structured review trigger configuration with three independent modes. - * All modes default to `false` (safe default — users must explicitly opt in). - */ -export const ReviewTriggerConfigSchema = z.object({ - /** Trigger review for PRs authored by the implementer persona. */ - ownPrsOnly: z.boolean().default(false), - /** Trigger review for PRs authored by anyone (not just the implementer). */ - externalPrs: z.boolean().default(false), - /** Trigger review when a CASCADE persona is explicitly requested as reviewer. */ - onReviewRequested: z.boolean().default(false), -}); - -export type ReviewTriggerConfig = 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), - /** Legacy 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), - /** - * After a PR is merged, chain into the backlog-manager agent to pick the next card. - * Default false (opt-in) for backward compatibility. - */ - prMergedBacklogManager: z.boolean().default(false), - /** - * Structured review trigger config with three independent modes. - * When present, takes precedence over the legacy `reviewRequested` / `checkSuiteSuccess` booleans. - */ - reviewTrigger: ReviewTriggerConfigSchema.optional(), -}); - -export type TrelloTriggerConfig = z.infer; -export type JiraTriggerConfig = z.infer; -export type GitHubTriggerConfig = z.infer; - // ============================================================================ // Email Trigger Config // ============================================================================ @@ -159,272 +38,3 @@ export function parseEmailJokeTriggers(triggers: unknown): EmailJokeTriggerConfi const result = EmailJokeTriggerConfigSchema.safeParse(triggers); return result.success ? result.data : { senderEmail: undefined }; } - -// ============================================================================ -// Review Trigger Resolution -// ============================================================================ - -/** - * Resolve the structured review trigger config from GitHub trigger config. - * - * Precedence: - * 1. `reviewTrigger` object (new structured config) — wins when present - * 2. Legacy booleans: `checkSuiteSuccess` → `ownPrsOnly`, `reviewRequested` → `onReviewRequested` - * 3. Bare defaults (no config) — all modes false - * - * This helper is the single source of truth for determining which review trigger modes are active. - */ -export function resolveReviewTriggerConfig( - config: Partial | undefined, -): ReviewTriggerConfig { - // New structured config wins when present - if (config?.reviewTrigger !== undefined) { - return { - ownPrsOnly: config.reviewTrigger.ownPrsOnly ?? false, - externalPrs: config.reviewTrigger.externalPrs ?? false, - onReviewRequested: config.reviewTrigger.onReviewRequested ?? false, - }; - } - - // Legacy fallback: map old boolean flags to structured modes - const legacyOwnPrsOnly = config?.checkSuiteSuccess ?? true; // existing default was true - const legacyOnReviewRequested = config?.reviewRequested ?? false; - - return { - ownPrsOnly: legacyOwnPrsOnly, - externalPrs: false, // no legacy equivalent — always false - onReviewRequested: legacyOnReviewRequested, - }; -} - -// ============================================================================ -// Generic Helpers -// ============================================================================ - -/** Shape of a per-agent toggle object (splitting / planning / implementation / backlog-manager). */ -type PerAgentObject = { - splitting?: boolean; - planning?: boolean; - implementation?: boolean; - 'backlog-manager'?: boolean; -}; - -/** - * Generic resolver for per-agent toggles that can be: - * - `undefined` → returns `true` (backward compatible, always enabled) - * - `boolean` → applies uniformly to all agents - * - `{ splitting, planning, implementation }` → per-agent lookup - * - * This replaces the duplicated logic in `resolveReadyToProcessEnabled` and - * `resolveIssueTransitionedEnabled`. - */ -export function resolvePerAgentToggle( - value: boolean | PerAgentObject | undefined, - agentType: string, -): boolean { - if (value === undefined) return true; - if (typeof value === 'boolean') return value; - // Nested object: check per-agent toggle - if (agentType === 'splitting') return value.splitting ?? true; - if (agentType === 'planning') return value.planning ?? true; - if (agentType === 'implementation') return value.implementation ?? true; - if (agentType === 'backlog-manager') return value['backlog-manager'] ?? true; - // Unknown agent type — default to enabled - return true; -} - -/** Options for `resolveTriggerEnabled()`. */ -interface ResolveTriggerEnabledOptions { - /** - * Keys that are opt-in (default `false` even when config is absent). - * All other keys default to `true` when absent. - */ - optInKeys?: string[]; - /** - * Keys whose value is a `ReadyToProcessLabelConfig`-style object. - * When queried at the outer level (not per-agent), returns `true` if any - * agent sub-key is enabled. - */ - nestedKeys?: string[]; - /** - * Keys whose value may be a non-boolean object (e.g. `reviewTrigger`). - * When the stored value is not a boolean, returns `true` (the object being - * present signals it is active). - */ - objectKeys?: string[]; -} - -/** - * Generic resolver for a single trigger key within any platform's trigger config. - * - * Handles all the special cases across Trello, JIRA, and GitHub: - * - Missing config or missing key → `true` (or `false` for opt-in keys) - * - Boolean value → return it directly - * - Nested per-agent object (nestedKeys) → `true` if any agent sub-key is enabled - * - Non-boolean object (objectKeys) → `true` (presence indicates active) - * - * Platform-specific wrappers (`resolveTrelloTriggerEnabled`, etc.) call this - * function with the appropriate option sets, keeping all special-case knowledge - * in one place. - */ -export function resolveTriggerEnabled( - config: Record | undefined, - key: string, - options: ResolveTriggerEnabledOptions = {}, -): boolean { - const { optInKeys = [], nestedKeys = [], objectKeys = [] } = options; - const isOptIn = optInKeys.includes(key); - - if (!config) return !isOptIn; - - const value = config[key]; - - if (value === undefined) return !isOptIn; - - // Nested per-agent object (e.g. readyToProcessLabel, issueTransitioned) - if (nestedKeys.includes(key)) { - if (typeof value === 'boolean') return value; - const obj = value as PerAgentObject; - return !!(obj.splitting || obj.planning || obj.implementation || obj['backlog-manager']); - } - - // Object key (e.g. reviewTrigger) — non-boolean means the object is present/active - if (objectKeys.includes(key)) { - if (typeof value !== 'boolean') return true; - return value; - } - - return typeof value === 'boolean' ? value : true; -} - -// ============================================================================ -// Platform config constants -// ============================================================================ - -/** Keys whose values are per-agent nested objects in the Trello trigger config. */ -const TRELLO_NESTED_KEYS: string[] = ['readyToProcessLabel', 'statusChanged']; - -/** Keys whose values are per-agent nested objects in the JIRA trigger config. */ -const JIRA_NESTED_KEYS: string[] = ['readyToProcessLabel', 'statusChanged', 'issueTransitioned']; - -/** Keys that are opt-in (default false) in the GitHub trigger config. */ -const GITHUB_OPT_IN_KEYS: string[] = ['reviewRequested', 'prOpened', 'prMergedBacklogManager']; - -/** Keys whose values may be non-boolean objects in the GitHub trigger config. */ -const GITHUB_OBJECT_KEYS: string[] = ['reviewTrigger']; - -// ============================================================================ -// Platform-specific wrappers (thin, backward-compatible) -// ============================================================================ - -/** - * Resolve whether a Trello trigger is enabled based on project trigger config. - * Returns `true` (enabled) when no config is present (backward compatible). - */ -export function resolveTrelloTriggerEnabled( - config: Partial | undefined, - key: keyof TrelloTriggerConfig, -): boolean { - return resolveTriggerEnabled(config as Record | undefined, key as string, { - nestedKeys: TRELLO_NESTED_KEYS, - }); -} - -/** - * Resolve whether a JIRA trigger is enabled based on project trigger config. - * Returns `true` (enabled) when no config is present (backward compatible). - */ -export function resolveJiraTriggerEnabled( - config: Partial | undefined, - key: keyof JiraTriggerConfig, -): boolean { - return resolveTriggerEnabled(config as Record | undefined, key as string, { - nestedKeys: JIRA_NESTED_KEYS, - }); -} - -/** - * 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. - */ -export function resolveGitHubTriggerEnabled( - config: Partial | undefined, - key: keyof GitHubTriggerConfig, -): boolean { - return resolveTriggerEnabled(config as Record | undefined, key as string, { - optInKeys: GITHUB_OPT_IN_KEYS, - objectKeys: GITHUB_OBJECT_KEYS, - }); -} - -// ============================================================================ -// Per-agent trigger resolvers (thin wrappers over resolvePerAgentToggle) -// ============================================================================ - -/** - * Resolve whether the ready-to-process trigger is enabled for a specific agent type. - * Supports both the new nested object format and the legacy boolean format. - * Returns `true` when no config is present (backward compatible). - */ -export function resolveReadyToProcessEnabled( - config: Partial | Partial | undefined, - agentType: string, -): boolean { - return resolvePerAgentToggle( - config?.readyToProcessLabel as boolean | PerAgentObject | undefined, - agentType, - ); -} - -/** - * Resolve whether the status-changed trigger is enabled for a specific agent type (Jira). - * Reads from the `statusChanged` key first, falling back to the legacy `issueTransitioned` key. - * Supports both the new nested object format and the legacy boolean format. - * Returns `true` when no config is present (backward compatible). - */ -export function resolveStatusChangedEnabled( - config: Partial | undefined, - agentType: string, -): boolean { - // Prefer new `statusChanged` key, fall back to legacy `issueTransitioned` - const value = - config?.statusChanged !== undefined ? config.statusChanged : config?.issueTransitioned; - return resolvePerAgentToggle(value as boolean | PerAgentObject | undefined, agentType); -} - -/** - * Resolve whether the status-changed trigger is enabled for a specific agent type (Trello). - * Reads from the `statusChanged` key first, falling back to the legacy per-agent keys: - * - splitting → cardMovedToSplitting - * - planning → cardMovedToPlanning - * - implementation → cardMovedToTodo - * - * Supports both the new nested object format and the legacy boolean format. - * Returns `true` when no config is present (backward compatible). - */ -export function resolveTrelloStatusChangedEnabled( - config: Partial | undefined, - agentType: string, -): boolean { - // Prefer new `statusChanged` key - if (config?.statusChanged !== undefined) { - return resolvePerAgentToggle( - config.statusChanged as boolean | PerAgentObject | undefined, - agentType, - ); - } - - // Fall back to legacy per-agent keys - if (agentType === 'splitting' && config?.cardMovedToSplitting !== undefined) { - return config.cardMovedToSplitting; - } - if (agentType === 'planning' && config?.cardMovedToPlanning !== undefined) { - return config.cardMovedToPlanning; - } - if (agentType === 'implementation' && config?.cardMovedToTodo !== undefined) { - return config.cardMovedToTodo; - } - - // No config present — default enabled for backward compatibility - return true; -} diff --git a/src/db/repositories/configMapper.ts b/src/db/repositories/configMapper.ts index 9aa627ee..3ef33db4 100644 --- a/src/db/repositories/configMapper.ts +++ b/src/db/repositories/configMapper.ts @@ -57,7 +57,6 @@ export interface IntegrationRow { category: string; provider: string; config: unknown; - triggers: unknown; } // --------------------------------------------------------------------------- @@ -68,11 +67,8 @@ export interface MapProjectInput { row: ProjectRow; projectAgentConfigs: AgentConfigRow[]; trelloConfig?: TrelloIntegrationConfig; - trelloTriggers?: Record; jiraConfig?: JiraIntegrationConfig; - jiraTriggers?: Record; githubConfig?: GitHubIntegrationConfig; - githubTriggers?: Record; } // --------------------------------------------------------------------------- @@ -96,7 +92,6 @@ export interface ProjectConfigRaw { lists: Record; labels: Record; customFields?: { cost?: string }; - triggers?: Record; }; jira?: { projectKey: string; @@ -105,9 +100,7 @@ export interface ProjectConfigRaw { issueTypes?: Record; customFields?: { cost?: string }; labels?: Record; - triggers?: Record; }; - github?: { triggers: Record }; agentBackend?: { default?: string; overrides: Record; @@ -153,23 +146,16 @@ export function orUndefined>(obj: T): T | unde return Object.keys(obj).length > 0 ? obj : undefined; } -function buildTrelloConfig( - config: TrelloIntegrationConfig, - triggers?: Record, -): ProjectConfigRaw['trello'] { +function buildTrelloConfig(config: TrelloIntegrationConfig): ProjectConfigRaw['trello'] { return { boardId: config.boardId, lists: config.lists, labels: config.labels, customFields: config.customFields, - ...(triggers && Object.keys(triggers).length > 0 ? { triggers } : {}), }; } -function buildJiraConfig( - config: JiraIntegrationConfig, - triggers?: Record, -): ProjectConfigRaw['jira'] { +function buildJiraConfig(config: JiraIntegrationConfig): ProjectConfigRaw['jira'] { return { projectKey: config.projectKey, baseUrl: config.baseUrl, @@ -177,7 +163,6 @@ function buildJiraConfig( issueTypes: config.issueTypes, customFields: config.customFields, labels: config.labels, - ...(triggers && Object.keys(triggers).length > 0 ? { triggers } : {}), }; } @@ -220,11 +205,8 @@ export function mapDefaultsRow( export function extractIntegrationConfigs(integrations: IntegrationRow[]): { trelloConfig?: TrelloIntegrationConfig; - trelloTriggers?: Record; jiraConfig?: JiraIntegrationConfig; - jiraTriggers?: Record; githubConfig?: GitHubIntegrationConfig; - githubTriggers?: Record; } { const trelloRow = integrations.find((i) => i.provider === 'trello'); const jiraRow = integrations.find((i) => i.provider === 'jira'); @@ -232,11 +214,8 @@ export function extractIntegrationConfigs(integrations: IntegrationRow[]): { return { trelloConfig: trelloRow?.config as TrelloIntegrationConfig | undefined, - trelloTriggers: (trelloRow?.triggers ?? undefined) as Record | undefined, jiraConfig: jiraRow?.config as JiraIntegrationConfig | undefined, - jiraTriggers: (jiraRow?.triggers ?? undefined) as Record | undefined, githubConfig: githubRow?.config as GitHubIntegrationConfig | undefined, - githubTriggers: (githubRow?.triggers ?? undefined) as Record | undefined, }; } @@ -244,10 +223,7 @@ export function mapProjectRow({ row, projectAgentConfigs, trelloConfig, - trelloTriggers, jiraConfig, - jiraTriggers, - githubTriggers, }: MapProjectInput): ProjectConfigRaw { const { models, backends } = buildAgentMaps(projectAgentConfigs); @@ -269,15 +245,11 @@ export function mapProjectRow({ }; if (trelloConfig) { - project.trello = buildTrelloConfig(trelloConfig, trelloTriggers); + project.trello = buildTrelloConfig(trelloConfig); } if (jiraConfig) { - project.jira = buildJiraConfig(jiraConfig, jiraTriggers); - } - - if (githubTriggers && Object.keys(githubTriggers).length > 0) { - project.github = { triggers: githubTriggers }; + project.jira = buildJiraConfig(jiraConfig); } const agentBackend = buildAgentBackendConfig(row, backends); diff --git a/src/db/repositories/configRepository.ts b/src/db/repositories/configRepository.ts index 30af354b..cc34e646 100644 --- a/src/db/repositories/configRepository.ts +++ b/src/db/repositories/configRepository.ts @@ -45,23 +45,13 @@ function buildRawConfig({ defaults: mapDefaultsRow(defaultsRow, globalAgentConfigs), projects: projectRows.map((row) => { const integrations = integrationsByProject.get(row.id) ?? []; - const { - trelloConfig, - trelloTriggers, - jiraConfig, - jiraTriggers, - githubConfig, - githubTriggers, - } = extractIntegrationConfigs(integrations); + const { trelloConfig, jiraConfig, githubConfig } = extractIntegrationConfigs(integrations); return mapProjectRow({ row, projectAgentConfigs: projectAgentConfigsMap.get(row.id) ?? [], trelloConfig, - trelloTriggers, jiraConfig, - jiraTriggers, githubConfig, - githubTriggers, }); }), }; diff --git a/src/pm/config.ts b/src/pm/config.ts index 24207fae..083bd6cd 100644 --- a/src/pm/config.ts +++ b/src/pm/config.ts @@ -14,7 +14,6 @@ export interface TrelloConfig { lists: Record; labels: Record; customFields?: { cost?: string }; - triggers?: Record; } /** JIRA-specific configuration (from project_integrations JSONB) */ @@ -30,7 +29,6 @@ export interface JiraConfig { error?: string; readyToProcess?: string; }; - triggers?: Record; } /** @@ -63,13 +61,3 @@ export function getCostFieldId(project: ProjectConfig): string | undefined { } return getTrelloConfig(project)?.customFields?.cost; } - -/** - * Get PM-specific trigger config for a project. - */ -export function getPMTriggerConfig(project: ProjectConfig): Record | undefined { - if (project.pm?.type === 'jira') { - return getJiraConfig(project)?.triggers; - } - return getTrelloConfig(project)?.triggers; -} diff --git a/src/triggers/github/check-suite-failure.ts b/src/triggers/github/check-suite-failure.ts index 89a1dceb..45a5e909 100644 --- a/src/triggers/github/check-suite-failure.ts +++ b/src/triggers/github/check-suite-failure.ts @@ -1,8 +1,8 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js'; import { resolveWorkItemId } from './utils.js'; @@ -24,11 +24,6 @@ export class CheckSuiteFailureTrigger 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, 'checkSuiteFailure')) { - return false; - } - const payload = ctx.payload; // Only trigger on completed check suites with failure conclusion @@ -42,6 +37,18 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-ci', + 'scm:check-suite-failure', + this.name, + )) + ) { + return null; + } + const payload = ctx.payload as GitHubCheckSuitePayload; const { owner, repo } = parseRepoFullName(payload.repository.full_name); @@ -54,7 +61,10 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { const prDetails = await githubClient.getPR(owner, repo, prNumber); // Gate on PR author being the implementer persona - if (!ctx.personaIdentities) return null; + if (!ctx.personaIdentities) { + logger.info('No persona identities available, skipping', { handler: this.name, prNumber }); + 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 check failure trigger', { diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index 1e220082..b6fd20dc 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -1,10 +1,10 @@ -import { resolveReviewTriggerConfig } 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'; import { parseRepoFullName } from '../../utils/repo.js'; +import { checkTriggerEnabledWithParams } from '../shared/trigger-check.js'; import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js'; -import { resolveWorkItemId } from './utils.js'; +import { evaluateAuthorMode, resolveWorkItemId } from './utils.js'; const MAX_RETRIES = 5; const RETRY_DELAY_MS = 10_000; @@ -55,7 +55,7 @@ export async function waitForChecks( * * This trigger fires when: * 1. A check_suite completes with success conclusion - * 2. The PR author matches the implementer persona (or its [bot] variant) + * 2. The PR author matches the configured author mode (own/external/all) * 3. All checks are actually passing (verified via API) * * Work item resolution uses the pr_work_items DB table (with PR body extraction as fallback). @@ -72,12 +72,6 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubCheckSuitePayload(ctx.payload)) return false; - // Check trigger config — at least one CI-based review mode must be active - const reviewConfig = resolveReviewTriggerConfig(ctx.project.github?.triggers); - if (!reviewConfig.ownPrsOnly && !reviewConfig.externalPrs) { - return false; - } - const payload = ctx.payload; // Only trigger on completed check suites with success conclusion @@ -91,6 +85,17 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { + // Check trigger config + get parameters in a single DB call + const triggerConfig = await checkTriggerEnabledWithParams( + ctx.project.id, + 'review', + 'scm:check-suite-success', + this.name, + ); + if (!triggerConfig.enabled) { + return null; + } + const payload = ctx.payload as GitHubCheckSuitePayload; const { owner, repo } = parseRepoFullName(payload.repository.full_name); @@ -102,24 +107,23 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { // Fetch PR details const prDetails = await githubClient.getPR(owner, repo, prNumber); - // Gate on PR author based on configured review trigger modes - if (!ctx.personaIdentities) return null; - const implLogin = ctx.personaIdentities.implementer; - const isImplementerPR = - prDetails.user.login === implLogin || prDetails.user.login === `${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', { + // Gate on PR author based on configured authorMode parameter + const authorResult = evaluateAuthorMode( + prDetails.user.login, + ctx.personaIdentities, + triggerConfig.parameters, + this.name, + ); + if (!authorResult) { + return null; + } + if (!authorResult.shouldTrigger) { + logger.info('PR author does not match configured authorMode, skipping', { + handler: this.name, prNumber, prAuthor: prDetails.user.login, - isImplementerPR, - ownPrsOnly: reviewConfig.ownPrsOnly, - externalPrs: reviewConfig.externalPrs, + isImplementerPR: authorResult.isImplementerPR, + authorMode: authorResult.authorMode, }); return null; } @@ -146,7 +150,8 @@ 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; + // (evaluateAuthorMode above already verified personaIdentities exists) + 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-comment-mention.ts b/src/triggers/github/pr-comment-mention.ts index b6d0aa7e..60988adc 100644 --- a/src/triggers/github/pr-comment-mention.ts +++ b/src/triggers/github/pr-comment-mention.ts @@ -1,9 +1,9 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import { isCascadeBot } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { isGitHubIssueCommentPayload, isGitHubPRReviewCommentPayload } from './types.js'; import { resolveWorkItemId } from './utils.js'; @@ -20,11 +20,6 @@ export class PRCommentMentionTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'github') return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prCommentMention')) { - return false; - } - // Match issue_comment.created on PRs if (isGitHubIssueCommentPayload(ctx.payload)) { if (ctx.payload.action !== 'created') return false; @@ -40,6 +35,18 @@ export class PRCommentMentionTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-pr-comment', + 'scm:pr-comment-mention', + this.name, + )) + ) { + return null; + } + // Require persona identities for @mention detection if (!ctx.personaIdentities) { logger.warn('No persona identities available, skipping @mention trigger'); @@ -97,6 +104,7 @@ export class PRCommentMentionTrigger implements TriggerHandler { // Check for @mention of the implementer persona (case-insensitive) const mentionPattern = new RegExp(`@${mentionTarget}\\b`, 'i'); if (!mentionPattern.test(commentBody)) { + logger.debug('No @mention in comment, skipping', { prNumber, mentionTarget }); return null; } diff --git a/src/triggers/github/pr-merged.ts b/src/triggers/github/pr-merged.ts index 968d444f..4f983864 100644 --- a/src/triggers/github/pr-merged.ts +++ b/src/triggers/github/pr-merged.ts @@ -1,10 +1,10 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import { getPMProvider } from '../../pm/context.js'; import { resolveProjectPMConfig } from '../../pm/lifecycle.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; import { resolveWorkItemId } from './utils.js'; @@ -16,15 +16,15 @@ export class PRMergedTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubPullRequestPayload(ctx.payload)) return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prMerged')) { - return false; - } - return ctx.payload.action === 'closed'; } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if (!(await checkTriggerEnabled(ctx.project.id, 'review', 'scm:pr-merged', this.name))) { + return null; + } + const payload = ctx.payload as GitHubPullRequestPayload; const { owner, repo } = parseRepoFullName(payload.repository.full_name); const prNumber = payload.pull_request.number; @@ -83,12 +83,7 @@ export class PRMergedTrigger implements TriggerHandler { logger.info('Moved work item to merged status', { workItemId, prNumber }); // Optionally chain into the backlog-manager agent to pick the next card - const chainBacklogManager = resolveGitHubTriggerEnabled( - ctx.project.github?.triggers, - 'prMergedBacklogManager', - ); - - if (chainBacklogManager) { + if (await checkTriggerEnabled(ctx.project.id, 'backlog-manager', 'scm:pr-merged', this.name)) { logger.info('Chaining to backlog-manager after PR merge', { workItemId, prNumber }); return { agentType: 'backlog-manager', diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index c02f7159..6dfb515a 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -1,11 +1,8 @@ -import { - resolveGitHubTriggerEnabled, - resolveReviewTriggerConfig, -} from '../../config/triggerConfig.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabledWithParams } from '../shared/trigger-check.js'; import { isGitHubPullRequestPayload } from './types.js'; -import { resolveWorkItemId } from './utils.js'; +import { evaluateAuthorMode, resolveWorkItemId } from './utils.js'; /** * Trigger that fires the review agent when a new PR is opened. @@ -19,17 +16,6 @@ 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')) { - 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; @@ -40,6 +26,17 @@ export class PROpenedTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { + // Check trigger config + get parameters in a single DB call + const triggerConfig = await checkTriggerEnabledWithParams( + ctx.project.id, + 'review', + 'scm:pr-opened', + this.name, + ); + if (!triggerConfig.enabled) { + return null; + } + const payload = ctx.payload as { pull_request: { number: number; @@ -55,23 +52,23 @@ export class PROpenedTrigger implements TriggerHandler { const prNumber = payload.pull_request.number; const prAuthor = payload.pull_request.user.login; - // 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', { + // Gate on PR author based on configured authorMode parameter + const authorResult = evaluateAuthorMode( + prAuthor, + ctx.personaIdentities, + triggerConfig.parameters, + this.name, + ); + if (!authorResult) { + return null; + } + if (!authorResult.shouldTrigger) { + logger.info('PR author does not match configured authorMode, skipping', { + handler: this.name, prNumber, prAuthor, - isImplementerPR, - ownPrsOnly: reviewConfig.ownPrsOnly, - externalPrs: reviewConfig.externalPrs, + isImplementerPR: authorResult.isImplementerPR, + authorMode: authorResult.authorMode, }); return null; } diff --git a/src/triggers/github/pr-ready-to-merge.ts b/src/triggers/github/pr-ready-to-merge.ts index f70c31c8..3aed4621 100644 --- a/src/triggers/github/pr-ready-to-merge.ts +++ b/src/triggers/github/pr-ready-to-merge.ts @@ -1,10 +1,10 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import { getPMProvider } from '../../pm/context.js'; import { resolveProjectPMConfig } from '../../pm/lifecycle.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type GitHubCheckSuitePayload, type GitHubPullRequestReviewPayload, @@ -20,11 +20,6 @@ export class PRReadyToMergeTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'github') return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prReadyToMerge')) { - return false; - } - // Trigger on either check_suite completion (success) or review submission (approved) if (isGitHubCheckSuitePayload(ctx.payload)) { const payload = ctx.payload; @@ -47,6 +42,13 @@ export class PRReadyToMergeTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if ( + !(await checkTriggerEnabled(ctx.project.id, 'review', 'scm:pr-ready-to-merge', this.name)) + ) { + return null; + } + let prNumber: number; let headSha: string; let repoFullName: string; diff --git a/src/triggers/github/pr-review-submitted.ts b/src/triggers/github/pr-review-submitted.ts index 7a15662e..e7b3b939 100644 --- a/src/triggers/github/pr-review-submitted.ts +++ b/src/triggers/github/pr-review-submitted.ts @@ -1,7 +1,7 @@ -import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { getPersonaForLogin } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { isGitHubPullRequestReviewPayload } from './types.js'; import { resolveWorkItemId } from './utils.js'; @@ -13,11 +13,6 @@ export class PRReviewSubmittedTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubPullRequestReviewPayload(ctx.payload)) return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prReviewSubmitted')) { - return false; - } - // Only trigger on submitted reviews, not edits or dismissals if (ctx.payload.action !== 'submitted') return false; @@ -28,6 +23,18 @@ export class PRReviewSubmittedTrigger implements TriggerHandler { } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-review', + 'scm:pr-review-submitted', + this.name, + )) + ) { + return null; + } + // Type assertion since we validated in matches() const reviewPayload = ctx.payload as { pull_request: { number: number; body: string | null; head: { ref: string } }; diff --git a/src/triggers/github/review-requested.ts b/src/triggers/github/review-requested.ts index 2d1eb0a1..c62faa15 100644 --- a/src/triggers/github/review-requested.ts +++ b/src/triggers/github/review-requested.ts @@ -1,7 +1,7 @@ -import { resolveReviewTriggerConfig } 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'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; import { resolveWorkItemId } from './utils.js'; @@ -13,8 +13,7 @@ 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: **disabled** (opt-in via trigger config). * * Registration: should be registered BEFORE CheckSuiteSuccessTrigger so that * both triggers can independently fire review. The HEAD-SHA dedup in @@ -31,16 +30,15 @@ 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 - const reviewConfig = resolveReviewTriggerConfig(ctx.project.github?.triggers); - if (!reviewConfig.onReviewRequested) { - return false; - } - return true; } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if (!(await checkTriggerEnabled(ctx.project.id, 'review', 'scm:review-requested', this.name))) { + return null; + } + const payload = ctx.payload as GitHubPullRequestPayload; const prNumber = payload.pull_request.number; diff --git a/src/triggers/jira/comment-mention.ts b/src/triggers/jira/comment-mention.ts index 0ca534b4..6805eb21 100644 --- a/src/triggers/jira/comment-mention.ts +++ b/src/triggers/jira/comment-mention.ts @@ -5,11 +5,10 @@ * Runs the respond-to-planning-comment agent. */ -import { resolveJiraTriggerEnabled } from '../../config/triggerConfig.js'; import { jiraClient } from '../../jira/client.js'; -import { getJiraConfig } from '../../pm/config.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import type { JiraWebhookPayload } from './types.js'; // Cache authenticated user info to avoid repeated API calls @@ -95,16 +94,23 @@ export class JiraCommentMentionTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'jira') return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveJiraTriggerEnabled(getJiraConfig(ctx.project)?.triggers, 'commentMention')) { - return false; - } - const payload = ctx.payload as JiraWebhookPayload; return payload.webhookEvent === 'comment_created' || payload.webhookEvent === 'comment_updated'; } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-planning-comment', + 'pm:comment-mention', + this.name, + )) + ) { + return null; + } + const payload = ctx.payload as JiraWebhookPayload; const issueKey = payload.issue?.key; const commentBody = payload.comment?.body; diff --git a/src/triggers/jira/label-added.ts b/src/triggers/jira/label-added.ts index a81e2a39..15cd4f58 100644 --- a/src/triggers/jira/label-added.ts +++ b/src/triggers/jira/label-added.ts @@ -10,14 +10,11 @@ * explicitly excludes events that also contain a status change in the changelog. */ -import { - resolveJiraTriggerEnabled, - resolveReadyToProcessEnabled, -} from '../../config/triggerConfig.js'; import { getJiraConfig } from '../../pm/config.js'; import { resolveProjectPMConfig } from '../../pm/lifecycle.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type JiraWebhookPayload, STATUS_TO_AGENT } from './types.js'; /** @@ -42,11 +39,6 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'jira') return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveJiraTriggerEnabled(getJiraConfig(ctx.project)?.triggers, 'readyToProcessLabel')) { - return false; - } - const payload = ctx.payload as JiraWebhookPayload; if (!payload.webhookEvent?.startsWith('jira:issue_updated')) return false; @@ -110,12 +102,8 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { return null; } - // Check per-agent ready-to-process toggle - if (!resolveReadyToProcessEnabled(getJiraConfig(ctx.project)?.triggers, agentType)) { - logger.info('JIRA ready-to-process disabled for agent type, skipping', { - issueKey, - agentType, - }); + // Check per-agent ready-to-process toggle via new DB-driven system + if (!(await checkTriggerEnabled(ctx.project.id, agentType, 'pm:label-added', this.name))) { return null; } diff --git a/src/triggers/jira/status-changed.ts b/src/triggers/jira/status-changed.ts index c492108e..cdcfd5ac 100644 --- a/src/triggers/jira/status-changed.ts +++ b/src/triggers/jira/status-changed.ts @@ -5,13 +5,10 @@ * a CASCADE agent type (splitting, planning, implementation). */ -import { - resolveJiraTriggerEnabled, - resolveStatusChangedEnabled, -} from '../../config/triggerConfig.js'; import { getJiraConfig } from '../../pm/config.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type JiraWebhookPayload, STATUS_TO_AGENT } from './types.js'; export class JiraStatusChangedTrigger implements TriggerHandler { @@ -21,11 +18,6 @@ export class JiraStatusChangedTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'jira') return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveJiraTriggerEnabled(getJiraConfig(ctx.project)?.triggers, 'statusChanged')) { - return false; - } - const payload = ctx.payload as JiraWebhookPayload; if (!payload.webhookEvent?.startsWith('jira:issue_updated')) return false; @@ -74,12 +66,8 @@ export class JiraStatusChangedTrigger implements TriggerHandler { return null; } - // Check per-agent toggle for statusChanged - if (!resolveStatusChangedEnabled(jiraConfig?.triggers, agentType)) { - logger.debug('JIRA status-changed trigger disabled for agent', { - issueKey, - agentType, - }); + // Check per-agent toggle for statusChanged via new DB-driven system + if (!(await checkTriggerEnabled(ctx.project.id, agentType, 'pm:status-changed', this.name))) { return null; } diff --git a/src/triggers/shared/trigger-check.ts b/src/triggers/shared/trigger-check.ts new file mode 100644 index 00000000..4f0914d0 --- /dev/null +++ b/src/triggers/shared/trigger-check.ts @@ -0,0 +1,54 @@ +/** + * Shared helper for checking trigger configuration in handler `handle()` methods. + * + * Wraps `isTriggerEnabled()` / `getResolvedTriggerConfig()` from config-resolver + * with consistent logging. + */ + +import { logger } from '../../utils/logging.js'; +import { getResolvedTriggerConfig, isTriggerEnabled } from '../config-resolver.js'; + +/** + * Check whether a trigger is enabled for a project/agent/event combination. + * Logs an info message when the trigger is disabled, so every skip has a reason. + */ +export async function checkTriggerEnabled( + projectId: string, + agentType: string, + triggerEvent: string, + handlerName: string, +): Promise { + const enabled = await isTriggerEnabled(projectId, agentType, triggerEvent); + if (!enabled) { + logger.info('Trigger disabled by config, skipping', { + handler: handlerName, + agentType, + triggerEvent, + projectId, + }); + } + return enabled; +} + +/** + * Check whether a trigger is enabled AND return its parameters in a single DB call. + * Use this instead of separate `checkTriggerEnabled` + `getTriggerParameters` calls. + */ +export async function checkTriggerEnabledWithParams( + projectId: string, + agentType: string, + triggerEvent: string, + handlerName: string, +): Promise<{ enabled: boolean; parameters: Record }> { + const config = await getResolvedTriggerConfig(projectId, agentType, triggerEvent); + if (!config || !config.enabled) { + logger.info('Trigger disabled by config, skipping', { + handler: handlerName, + agentType, + triggerEvent, + projectId, + }); + return { enabled: false, parameters: {} }; + } + return { enabled: true, parameters: config.parameters }; +} diff --git a/src/triggers/trello/comment-mention.ts b/src/triggers/trello/comment-mention.ts index f49adfe7..ecf17a16 100644 --- a/src/triggers/trello/comment-mention.ts +++ b/src/triggers/trello/comment-mention.ts @@ -1,8 +1,8 @@ -import { resolveTrelloTriggerEnabled } from '../../config/triggerConfig.js'; import { getTrelloConfig } from '../../pm/config.js'; import { trelloClient } from '../../trello/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import type { TrelloWebhookPayload } from '../types.js'; import { isTrelloWebhookPayload } from '../types.js'; @@ -37,15 +37,22 @@ export class TrelloCommentMentionTrigger implements TriggerHandler { if (ctx.source !== 'trello') return false; if (!isTrelloWebhookPayload(ctx.payload)) return false; - // Check trigger config — default enabled for backward compatibility - if (!resolveTrelloTriggerEnabled(getTrelloConfig(ctx.project)?.triggers, 'commentMention')) { - return false; - } - return ctx.payload.action.type === 'commentCard'; } async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if ( + !(await checkTriggerEnabled( + ctx.project.id, + 'respond-to-planning-comment', + 'pm:comment-mention', + this.name, + )) + ) { + return null; + } + const payload = ctx.payload as TrelloWebhookPayload; const cardId = payload.action.data.card?.id; const commentText = payload.action.data.text; diff --git a/src/triggers/trello/label-added.ts b/src/triggers/trello/label-added.ts index 42c61f90..dae4558f 100644 --- a/src/triggers/trello/label-added.ts +++ b/src/triggers/trello/label-added.ts @@ -1,10 +1,7 @@ -import { - resolveReadyToProcessEnabled, - resolveTrelloTriggerEnabled, -} from '../../config/triggerConfig.js'; import { getTrelloConfig } from '../../pm/config.js'; import { trelloClient } from '../../trello/client.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import type { TrelloWebhookPayload, TriggerContext, @@ -21,13 +18,7 @@ export class ReadyToProcessLabelTrigger implements TriggerHandler { if (ctx.source !== 'trello') return false; if (!isTrelloWebhookPayload(ctx.payload)) return false; - // Check trigger config — default enabled for backward compatibility - // (checks if any agent has readyToProcessLabel enabled) const trelloConfig = getTrelloConfig(ctx.project); - if (!resolveTrelloTriggerEnabled(trelloConfig?.triggers, 'readyToProcessLabel')) { - return false; - } - const payload = ctx.payload; const readyLabelId = trelloConfig?.labels.readyToProcess; @@ -71,9 +62,8 @@ export class ReadyToProcessLabelTrigger implements TriggerHandler { logger.info('Agent type determined', { agentType, cardId, listId: currentListId }); - // Check per-agent ready-to-process toggle - if (!resolveReadyToProcessEnabled(getTrelloConfig(ctx.project)?.triggers, agentType)) { - logger.info('Ready-to-process disabled for agent type, skipping', { agentType, cardId }); + // Check per-agent ready-to-process toggle via new DB-driven system + if (!(await checkTriggerEnabled(ctx.project.id, agentType, 'pm:label-added', this.name))) { return null; } diff --git a/src/triggers/trello/status-changed.ts b/src/triggers/trello/status-changed.ts index 51a632ca..a021b2ef 100644 --- a/src/triggers/trello/status-changed.ts +++ b/src/triggers/trello/status-changed.ts @@ -1,6 +1,6 @@ -import { resolveTrelloStatusChangedEnabled } from '../../config/triggerConfig.js'; import { getTrelloConfig } from '../../pm/config.js'; import { logger } from '../../utils/logging.js'; +import { checkTriggerEnabled } from '../shared/trigger-check.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../types.js'; import { type TrelloWebhookPayload, isTrelloWebhookPayload } from './types.js'; @@ -24,12 +24,7 @@ function createStatusChangedTrigger(config: StatusChangedConfig): TriggerHandler if (ctx.source !== 'trello') return false; if (!isTrelloWebhookPayload(ctx.payload)) return false; - // Check trigger config with per-agent fallback to legacy keys const trelloConfig = getTrelloConfig(ctx.project); - if (!resolveTrelloStatusChangedEnabled(trelloConfig?.triggers, config.agentType)) { - return false; - } - const payload = ctx.payload; const targetListId = trelloConfig?.lists[config.listKey]; @@ -47,6 +42,18 @@ function createStatusChangedTrigger(config: StatusChangedConfig): TriggerHandler }, async handle(ctx: TriggerContext): Promise { + // Check trigger config via new DB-driven system + if ( + !(await checkTriggerEnabled( + ctx.project.id, + config.agentType, + 'pm:status-changed', + config.name, + )) + ) { + return null; + } + const payload = ctx.payload as TrelloWebhookPayload; const cardId = payload.action.data.card?.id; diff --git a/tests/unit/config/triggerConfig.test.ts b/tests/unit/config/triggerConfig.test.ts index ad8b9917..1aea6d51 100644 --- a/tests/unit/config/triggerConfig.test.ts +++ b/tests/unit/config/triggerConfig.test.ts @@ -1,824 +1,10 @@ import { describe, expect, it } from 'vitest'; import { EmailJokeTriggerConfigSchema, - GitHubTriggerConfigSchema, - JiraTriggerConfigSchema, - TrelloTriggerConfigSchema, parseEmailJokeTriggers, resolveEmailJokeTriggerConfig, - resolveGitHubTriggerEnabled, - resolveJiraTriggerEnabled, - resolvePerAgentToggle, - resolveReadyToProcessEnabled, - resolveReviewTriggerConfig, - resolveStatusChangedEnabled, - resolveTrelloStatusChangedEnabled, - resolveTrelloTriggerEnabled, - resolveTriggerEnabled, } from '../../../src/config/triggerConfig.js'; -describe('TrelloTriggerConfigSchema', () => { - it('defaults boolean fields to true (including new statusChanged)', () => { - const result = TrelloTriggerConfigSchema.parse({}); - expect(result.statusChanged).toBe(true); - expect(result.cardMovedToSplitting).toBe(true); - expect(result.cardMovedToPlanning).toBe(true); - expect(result.cardMovedToTodo).toBe(true); - expect(result.commentMention).toBe(true); - // readyToProcessLabel is optional — not present in default parse - expect(result.readyToProcessLabel).toBeUndefined(); - }); - - it('accepts explicit false for new statusChanged key', () => { - const result = TrelloTriggerConfigSchema.parse({ statusChanged: false }); - expect(result.statusChanged).toBe(false); - expect(result.cardMovedToSplitting).toBe(true); // legacy default still true - }); - - it('accepts explicit false values (legacy keys)', () => { - const result = TrelloTriggerConfigSchema.parse({ - cardMovedToPlanning: false, - readyToProcessLabel: false, - }); - expect(result.cardMovedToPlanning).toBe(false); - expect(result.readyToProcessLabel).toBe(false); - expect(result.cardMovedToSplitting).toBe(true); // default still true - }); - - it('accepts per-agent readyToProcessLabel object', () => { - const result = TrelloTriggerConfigSchema.parse({ - readyToProcessLabel: { splitting: true, planning: false, implementation: true }, - }); - expect(result.readyToProcessLabel).toEqual({ - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': true, - }); - }); - - it('accepts per-agent readyToProcessLabel object with backlog-manager disabled', () => { - const result = TrelloTriggerConfigSchema.parse({ - readyToProcessLabel: { - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': false, - }, - }); - expect(result.readyToProcessLabel).toEqual({ - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': false, - }); - }); -}); - -describe('JiraTriggerConfigSchema', () => { - it('defaults commentMention to true, statusChanged/issueTransitioned/readyToProcessLabel optional', () => { - const result = JiraTriggerConfigSchema.parse({}); - expect(result.commentMention).toBe(true); - expect(result.statusChanged).toBeUndefined(); - expect(result.issueTransitioned).toBeUndefined(); - expect(result.readyToProcessLabel).toBeUndefined(); - }); - - it('accepts new boolean statusChanged key', () => { - const result = JiraTriggerConfigSchema.parse({ statusChanged: false }); - expect(result.statusChanged).toBe(false); - }); - - it('accepts per-agent statusChanged object (backlog-manager defaults to true)', () => { - const result = JiraTriggerConfigSchema.parse({ - statusChanged: { splitting: true, planning: false, implementation: true }, - }); - expect(result.statusChanged).toEqual({ - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': true, - }); - }); - - it('accepts per-agent statusChanged object with backlog-manager explicitly disabled', () => { - const result = JiraTriggerConfigSchema.parse({ - statusChanged: { - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': false, - }, - }); - expect(result.statusChanged).toEqual({ - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': false, - }); - }); - - it('accepts legacy boolean issueTransitioned', () => { - const result = JiraTriggerConfigSchema.parse({ issueTransitioned: false }); - expect(result.issueTransitioned).toBe(false); - }); - - it('accepts per-agent issueTransitioned object', () => { - const result = JiraTriggerConfigSchema.parse({ - issueTransitioned: { splitting: true, planning: false, implementation: true }, - }); - expect(result.issueTransitioned).toEqual({ - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': true, - }); - }); -}); - -describe('StatusChangedSchema preserves backlog-manager key', () => { - it('does not strip backlog-manager when set to false (Zod schema includes the key)', () => { - const parsed = TrelloTriggerConfigSchema.parse({ - statusChanged: { 'backlog-manager': false }, - }); - const statusChanged = parsed.statusChanged as Record; - expect(statusChanged['backlog-manager']).toBe(false); - }); - - it('allows disabling backlog-manager via nested object config format', () => { - const parsed = TrelloTriggerConfigSchema.parse({ - statusChanged: { 'backlog-manager': false }, - }); - expect(resolveTrelloStatusChangedEnabled(parsed, 'backlog-manager')).toBe(false); - }); -}); - -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); - expect(result.prReadyToMerge).toBe(true); - expect(result.prMerged).toBe(true); - }); - - it('defaults new opt-in triggers to false', () => { - const result = GitHubTriggerConfigSchema.parse({}); - expect(result.reviewRequested).toBe(false); - expect(result.prOpened).toBe(false); - }); - - it('accepts reviewTrigger nested object', () => { - const result = GitHubTriggerConfigSchema.parse({ - reviewTrigger: { ownPrsOnly: true, externalPrs: false, onReviewRequested: true }, - }); - expect(result.reviewTrigger).toEqual({ - ownPrsOnly: true, - externalPrs: false, - onReviewRequested: true, - }); - }); - - it('reviewTrigger optional — absent by default', () => { - const result = GitHubTriggerConfigSchema.parse({}); - expect(result.reviewTrigger).toBeUndefined(); - }); -}); - -describe('resolveTrelloTriggerEnabled', () => { - it('returns true when config is undefined (backward compatible)', () => { - expect(resolveTrelloTriggerEnabled(undefined, 'cardMovedToSplitting')).toBe(true); - expect(resolveTrelloTriggerEnabled(undefined, 'readyToProcessLabel')).toBe(true); - expect(resolveTrelloTriggerEnabled(undefined, 'commentMention')).toBe(true); - }); - - it('returns true when key is not present in config', () => { - expect(resolveTrelloTriggerEnabled({}, 'cardMovedToSplitting')).toBe(true); - }); - - it('returns false when key is explicitly disabled', () => { - expect( - resolveTrelloTriggerEnabled({ cardMovedToSplitting: false }, 'cardMovedToSplitting'), - ).toBe(false); - }); - - it('returns true when key is explicitly enabled', () => { - expect(resolveTrelloTriggerEnabled({ cardMovedToPlanning: true }, 'cardMovedToPlanning')).toBe( - true, - ); - }); - - it('returns false for readyToProcessLabel when boolean false', () => { - expect(resolveTrelloTriggerEnabled({ readyToProcessLabel: false }, 'readyToProcessLabel')).toBe( - false, - ); - }); - - it('returns true for readyToProcessLabel when any agent is enabled in object form', () => { - expect( - resolveTrelloTriggerEnabled( - { readyToProcessLabel: { splitting: false, planning: true, implementation: false } }, - 'readyToProcessLabel', - ), - ).toBe(true); - }); - - it('returns false for readyToProcessLabel when all agents disabled in object form', () => { - expect( - resolveTrelloTriggerEnabled( - { readyToProcessLabel: { splitting: false, planning: false, implementation: false } }, - 'readyToProcessLabel', - ), - ).toBe(false); - }); -}); - -describe('resolveJiraTriggerEnabled', () => { - it('returns true when config is undefined (backward compatible)', () => { - expect(resolveJiraTriggerEnabled(undefined, 'statusChanged')).toBe(true); - expect(resolveJiraTriggerEnabled(undefined, 'issueTransitioned')).toBe(true); - expect(resolveJiraTriggerEnabled(undefined, 'readyToProcessLabel')).toBe(true); - expect(resolveJiraTriggerEnabled(undefined, 'commentMention')).toBe(true); - }); - - it('returns false when statusChanged is explicitly false', () => { - expect(resolveJiraTriggerEnabled({ statusChanged: false }, 'statusChanged')).toBe(false); - }); - - it('returns false when issueTransitioned is explicitly false (legacy boolean)', () => { - expect(resolveJiraTriggerEnabled({ issueTransitioned: false }, 'issueTransitioned')).toBe( - false, - ); - }); - - it('returns true when config is empty (no explicit settings)', () => { - expect(resolveJiraTriggerEnabled({}, 'statusChanged')).toBe(true); - expect(resolveJiraTriggerEnabled({}, 'issueTransitioned')).toBe(true); - }); - - it('returns true for statusChanged object when any agent is enabled', () => { - expect( - resolveJiraTriggerEnabled( - { statusChanged: { splitting: false, planning: true, implementation: false } }, - 'statusChanged', - ), - ).toBe(true); - }); - - it('returns false for statusChanged object when all agents disabled', () => { - expect( - resolveJiraTriggerEnabled( - { statusChanged: { splitting: false, planning: false, implementation: false } }, - 'statusChanged', - ), - ).toBe(false); - }); -}); - -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); - expect(resolveGitHubTriggerEnabled(undefined, 'prReadyToMerge')).toBe(true); - 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 false for opt-in triggers when config is empty', () => { - expect(resolveGitHubTriggerEnabled({}, 'reviewRequested')).toBe(false); - expect(resolveGitHubTriggerEnabled({}, 'prOpened')).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); - }); - - it('returns false when existing trigger is explicitly disabled', () => { - expect(resolveGitHubTriggerEnabled({ checkSuiteSuccess: false }, 'checkSuiteSuccess')).toBe( - false, - ); - }); - - it('returns true for reviewTrigger when config contains the object', () => { - expect( - resolveGitHubTriggerEnabled( - { reviewTrigger: { ownPrsOnly: true, externalPrs: false, onReviewRequested: false } }, - 'reviewTrigger', - ), - ).toBe(true); - }); -}); - -describe('resolveReadyToProcessEnabled', () => { - it('returns true when config is undefined (backward compatible)', () => { - expect(resolveReadyToProcessEnabled(undefined, 'splitting')).toBe(true); - expect(resolveReadyToProcessEnabled(undefined, 'planning')).toBe(true); - expect(resolveReadyToProcessEnabled(undefined, 'implementation')).toBe(true); - }); - - it('returns true when readyToProcessLabel is not set', () => { - expect(resolveReadyToProcessEnabled({}, 'splitting')).toBe(true); - }); - - it('applies legacy boolean true to all agents', () => { - const config = { readyToProcessLabel: true as const }; - expect(resolveReadyToProcessEnabled(config, 'splitting')).toBe(true); - expect(resolveReadyToProcessEnabled(config, 'planning')).toBe(true); - expect(resolveReadyToProcessEnabled(config, 'implementation')).toBe(true); - }); - - it('applies legacy boolean false to all agents', () => { - const config = { readyToProcessLabel: false as const }; - expect(resolveReadyToProcessEnabled(config, 'splitting')).toBe(false); - expect(resolveReadyToProcessEnabled(config, 'planning')).toBe(false); - expect(resolveReadyToProcessEnabled(config, 'implementation')).toBe(false); - }); - - it('returns per-agent value from nested object', () => { - const config = { - readyToProcessLabel: { splitting: true, planning: false, implementation: true }, - }; - expect(resolveReadyToProcessEnabled(config, 'splitting')).toBe(true); - expect(resolveReadyToProcessEnabled(config, 'planning')).toBe(false); - expect(resolveReadyToProcessEnabled(config, 'implementation')).toBe(true); - }); - - it('defaults to true for unknown agent types', () => { - const config = { - readyToProcessLabel: { splitting: false, planning: false, implementation: false }, - }; - expect(resolveReadyToProcessEnabled(config, 'unknown-agent')).toBe(true); - }); - - it('defaults to true for known non-toggle agents like respond-to-review', () => { - const config = { - readyToProcessLabel: { splitting: false, planning: false, implementation: false }, - }; - expect(resolveReadyToProcessEnabled(config, 'respond-to-review')).toBe(true); - expect(resolveReadyToProcessEnabled(config, 'debug')).toBe(true); - }); - - it('defaults all agents to true when nested object is empty (Zod fills defaults)', () => { - const parsed = TrelloTriggerConfigSchema.parse({ readyToProcessLabel: {} }); - expect(resolveReadyToProcessEnabled(parsed, 'splitting')).toBe(true); - expect(resolveReadyToProcessEnabled(parsed, 'planning')).toBe(true); - expect(resolveReadyToProcessEnabled(parsed, 'implementation')).toBe(true); - }); -}); - -describe('resolveStatusChangedEnabled', () => { - it('returns true when config is undefined (backward compatible)', () => { - expect(resolveStatusChangedEnabled(undefined, 'splitting')).toBe(true); - expect(resolveStatusChangedEnabled(undefined, 'planning')).toBe(true); - expect(resolveStatusChangedEnabled(undefined, 'implementation')).toBe(true); - }); - - it('returns true when no statusChanged or issueTransitioned key is set', () => { - expect(resolveStatusChangedEnabled({}, 'splitting')).toBe(true); - }); - - it('applies new statusChanged boolean true to all agents', () => { - const config = { statusChanged: true as const }; - expect(resolveStatusChangedEnabled(config, 'splitting')).toBe(true); - expect(resolveStatusChangedEnabled(config, 'planning')).toBe(true); - expect(resolveStatusChangedEnabled(config, 'implementation')).toBe(true); - }); - - it('applies new statusChanged boolean false to all agents', () => { - const config = { statusChanged: false as const }; - expect(resolveStatusChangedEnabled(config, 'splitting')).toBe(false); - expect(resolveStatusChangedEnabled(config, 'planning')).toBe(false); - expect(resolveStatusChangedEnabled(config, 'implementation')).toBe(false); - }); - - it('returns per-agent value from statusChanged nested object', () => { - const config = { - statusChanged: { splitting: true, planning: false, implementation: true }, - }; - expect(resolveStatusChangedEnabled(config, 'splitting')).toBe(true); - expect(resolveStatusChangedEnabled(config, 'planning')).toBe(false); - expect(resolveStatusChangedEnabled(config, 'implementation')).toBe(true); - }); - - it('falls back to legacy issueTransitioned when statusChanged is not set', () => { - const config = { issueTransitioned: false as const }; - expect(resolveStatusChangedEnabled(config, 'splitting')).toBe(false); - expect(resolveStatusChangedEnabled(config, 'planning')).toBe(false); - expect(resolveStatusChangedEnabled(config, 'implementation')).toBe(false); - }); - - it('prefers statusChanged over legacy issueTransitioned when both are set', () => { - const config = { - statusChanged: true as const, - issueTransitioned: false as const, - }; - expect(resolveStatusChangedEnabled(config, 'splitting')).toBe(true); - }); - - it('defaults to true for unknown agent types', () => { - const config = { - statusChanged: { splitting: false, planning: false, implementation: false }, - }; - expect(resolveStatusChangedEnabled(config, 'unknown-agent')).toBe(true); - }); - - it('defaults all agents to true when nested object is empty (Zod fills defaults)', () => { - const parsed = JiraTriggerConfigSchema.parse({ statusChanged: {} }); - expect(resolveStatusChangedEnabled(parsed, 'splitting')).toBe(true); - expect(resolveStatusChangedEnabled(parsed, 'planning')).toBe(true); - expect(resolveStatusChangedEnabled(parsed, 'implementation')).toBe(true); - }); -}); - -describe('resolveTrelloStatusChangedEnabled', () => { - it('returns true when config is undefined (backward compatible)', () => { - expect(resolveTrelloStatusChangedEnabled(undefined, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(undefined, 'planning')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(undefined, 'implementation')).toBe(true); - }); - - it('returns true when no statusChanged or legacy keys are set', () => { - expect(resolveTrelloStatusChangedEnabled({}, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled({}, 'planning')).toBe(true); - expect(resolveTrelloStatusChangedEnabled({}, 'implementation')).toBe(true); - }); - - it('applies new statusChanged boolean true to all agents', () => { - const config = { statusChanged: true as const }; - expect(resolveTrelloStatusChangedEnabled(config, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(config, 'planning')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(config, 'implementation')).toBe(true); - }); - - it('applies new statusChanged boolean false to all agents', () => { - const config = { statusChanged: false as const }; - expect(resolveTrelloStatusChangedEnabled(config, 'splitting')).toBe(false); - expect(resolveTrelloStatusChangedEnabled(config, 'planning')).toBe(false); - expect(resolveTrelloStatusChangedEnabled(config, 'implementation')).toBe(false); - }); - - it('returns per-agent value from statusChanged nested object', () => { - const config = { - statusChanged: { splitting: true, planning: false, implementation: true }, - }; - expect(resolveTrelloStatusChangedEnabled(config, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(config, 'planning')).toBe(false); - expect(resolveTrelloStatusChangedEnabled(config, 'implementation')).toBe(true); - }); - - it('falls back to cardMovedToSplitting when statusChanged is not set', () => { - const config = { cardMovedToSplitting: false }; - expect(resolveTrelloStatusChangedEnabled(config, 'splitting')).toBe(false); - }); - - it('falls back to cardMovedToPlanning when statusChanged is not set', () => { - const config = { cardMovedToPlanning: false }; - expect(resolveTrelloStatusChangedEnabled(config, 'planning')).toBe(false); - }); - - it('falls back to cardMovedToTodo when statusChanged is not set', () => { - const config = { cardMovedToTodo: false }; - expect(resolveTrelloStatusChangedEnabled(config, 'implementation')).toBe(false); - }); - - it('prefers statusChanged over legacy keys when both are set (boolean)', () => { - const config = { - statusChanged: true as const, - cardMovedToSplitting: false, - cardMovedToPlanning: false, - cardMovedToTodo: false, - }; - expect(resolveTrelloStatusChangedEnabled(config, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(config, 'planning')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(config, 'implementation')).toBe(true); - }); - - it('prefers statusChanged over legacy keys when both are set (per-agent object)', () => { - const config = { - statusChanged: { splitting: false, planning: true, implementation: false }, - cardMovedToSplitting: true, - cardMovedToPlanning: false, - cardMovedToTodo: true, - }; - expect(resolveTrelloStatusChangedEnabled(config, 'splitting')).toBe(false); - expect(resolveTrelloStatusChangedEnabled(config, 'planning')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(config, 'implementation')).toBe(false); - }); - - it('allows per-agent control via legacy keys when statusChanged is not set', () => { - const config = { - cardMovedToSplitting: true, - cardMovedToPlanning: false, - cardMovedToTodo: true, - }; - expect(resolveTrelloStatusChangedEnabled(config, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(config, 'planning')).toBe(false); - expect(resolveTrelloStatusChangedEnabled(config, 'implementation')).toBe(true); - }); - - it('defaults all agents to true when nested object is empty (Zod fills defaults)', () => { - const parsed = TrelloTriggerConfigSchema.parse({ statusChanged: {} }); - expect(resolveTrelloStatusChangedEnabled(parsed, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(parsed, 'planning')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(parsed, 'implementation')).toBe(true); - }); - - it('accepts per-agent statusChanged object in schema', () => { - const parsed = TrelloTriggerConfigSchema.parse({ - statusChanged: { splitting: true, planning: false, implementation: true }, - }); - expect(resolveTrelloStatusChangedEnabled(parsed, 'splitting')).toBe(true); - expect(resolveTrelloStatusChangedEnabled(parsed, 'planning')).toBe(false); - expect(resolveTrelloStatusChangedEnabled(parsed, 'implementation')).toBe(true); - }); -}); - -describe('resolveReviewTriggerConfig', () => { - it('maps legacy defaults when config is undefined (backward compatible)', () => { - // No config → legacy fallback: checkSuiteSuccess defaults to true → ownPrsOnly=true - // This preserves the existing behavior for projects with no trigger config - const result = resolveReviewTriggerConfig(undefined); - expect(result).toEqual({ ownPrsOnly: true, externalPrs: false, onReviewRequested: false }); - }); - - it('returns ownPrsOnly=true (legacy default) when config has no review-related keys', () => { - // checkSuiteSuccess is undefined → legacy default is true → ownPrsOnly=true - const result = resolveReviewTriggerConfig({ checkSuiteFailure: true }); - expect(result).toEqual({ ownPrsOnly: true, externalPrs: false, onReviewRequested: false }); - }); - - describe('new structured reviewTrigger config takes precedence', () => { - it('uses reviewTrigger object when present', () => { - const result = resolveReviewTriggerConfig({ - reviewTrigger: { ownPrsOnly: true, externalPrs: true, onReviewRequested: true }, - // Legacy booleans present but should be ignored - checkSuiteSuccess: false, - reviewRequested: false, - }); - expect(result).toEqual({ ownPrsOnly: true, externalPrs: true, onReviewRequested: true }); - }); - - it('uses reviewTrigger partial — missing fields default to false', () => { - const result = resolveReviewTriggerConfig({ - reviewTrigger: { ownPrsOnly: true, externalPrs: false, onReviewRequested: false }, - }); - expect(result.ownPrsOnly).toBe(true); - expect(result.externalPrs).toBe(false); - expect(result.onReviewRequested).toBe(false); - }); - - it('externalPrs can be independently enabled', () => { - const result = resolveReviewTriggerConfig({ - reviewTrigger: { ownPrsOnly: false, externalPrs: true, onReviewRequested: false }, - }); - expect(result.ownPrsOnly).toBe(false); - expect(result.externalPrs).toBe(true); - expect(result.onReviewRequested).toBe(false); - }); - }); - - describe('legacy boolean fallback', () => { - it('maps checkSuiteSuccess=true to ownPrsOnly=true (legacy default)', () => { - const result = resolveReviewTriggerConfig({ checkSuiteSuccess: true }); - expect(result.ownPrsOnly).toBe(true); - expect(result.externalPrs).toBe(false); - }); - - it('maps checkSuiteSuccess=false to ownPrsOnly=false', () => { - const result = resolveReviewTriggerConfig({ checkSuiteSuccess: false }); - expect(result.ownPrsOnly).toBe(false); - }); - - it('maps reviewRequested=true to onReviewRequested=true', () => { - const result = resolveReviewTriggerConfig({ reviewRequested: true }); - expect(result.onReviewRequested).toBe(true); - }); - - it('maps reviewRequested=false to onReviewRequested=false', () => { - const result = resolveReviewTriggerConfig({ reviewRequested: false }); - expect(result.onReviewRequested).toBe(false); - }); - - it('externalPrs is always false in legacy mode (no legacy equivalent)', () => { - const result = resolveReviewTriggerConfig({ - checkSuiteSuccess: true, - reviewRequested: true, - }); - expect(result.externalPrs).toBe(false); - }); - }); -}); - -// ============================================================================ -// Tests for new generic helpers -// ============================================================================ - -describe('resolvePerAgentToggle', () => { - describe('undefined value', () => { - it('returns true for all known agent types', () => { - expect(resolvePerAgentToggle(undefined, 'splitting')).toBe(true); - expect(resolvePerAgentToggle(undefined, 'planning')).toBe(true); - expect(resolvePerAgentToggle(undefined, 'implementation')).toBe(true); - expect(resolvePerAgentToggle(undefined, 'backlog-manager')).toBe(true); - }); - - it('returns true for unknown agent types', () => { - expect(resolvePerAgentToggle(undefined, 'review')).toBe(true); - expect(resolvePerAgentToggle(undefined, 'debug')).toBe(true); - }); - }); - - describe('boolean value', () => { - it('returns true when value is true, for all agent types', () => { - expect(resolvePerAgentToggle(true, 'splitting')).toBe(true); - expect(resolvePerAgentToggle(true, 'planning')).toBe(true); - expect(resolvePerAgentToggle(true, 'implementation')).toBe(true); - expect(resolvePerAgentToggle(true, 'unknown')).toBe(true); - }); - - it('returns false when value is false, for all agent types', () => { - expect(resolvePerAgentToggle(false, 'splitting')).toBe(false); - expect(resolvePerAgentToggle(false, 'planning')).toBe(false); - expect(resolvePerAgentToggle(false, 'implementation')).toBe(false); - expect(resolvePerAgentToggle(false, 'unknown')).toBe(false); - }); - }); - - describe('per-agent object', () => { - it('returns the correct value for each known agent type', () => { - const obj = { - splitting: true, - planning: false, - implementation: true, - 'backlog-manager': false, - }; - expect(resolvePerAgentToggle(obj, 'splitting')).toBe(true); - expect(resolvePerAgentToggle(obj, 'planning')).toBe(false); - expect(resolvePerAgentToggle(obj, 'implementation')).toBe(true); - expect(resolvePerAgentToggle(obj, 'backlog-manager')).toBe(false); - }); - - it('defaults backlog-manager to true when not present in object', () => { - const obj = { splitting: false, planning: false, implementation: false }; - expect(resolvePerAgentToggle(obj, 'backlog-manager')).toBe(true); - }); - - it('defaults to true for unknown agent types', () => { - const obj = { splitting: false, planning: false, implementation: false }; - expect(resolvePerAgentToggle(obj, 'respond-to-review')).toBe(true); - expect(resolvePerAgentToggle(obj, 'debug')).toBe(true); - expect(resolvePerAgentToggle(obj, 'anything-else')).toBe(true); - }); - - it('defaults missing fields to true', () => { - const obj = { splitting: false }; // planning and implementation are undefined - expect(resolvePerAgentToggle(obj, 'planning')).toBe(true); - expect(resolvePerAgentToggle(obj, 'implementation')).toBe(true); - }); - }); -}); - -describe('resolveTriggerEnabled', () => { - describe('no config (undefined)', () => { - it('returns true for standard keys', () => { - expect(resolveTriggerEnabled(undefined, 'someKey')).toBe(true); - }); - - it('returns false for opt-in keys', () => { - expect(resolveTriggerEnabled(undefined, 'optInKey', { optInKeys: ['optInKey'] })).toBe(false); - }); - }); - - describe('config present but key missing', () => { - it('returns true for standard keys', () => { - expect(resolveTriggerEnabled({}, 'someKey')).toBe(true); - }); - - it('returns false for opt-in keys', () => { - expect(resolveTriggerEnabled({}, 'optInKey', { optInKeys: ['optInKey'] })).toBe(false); - }); - }); - - describe('boolean values', () => { - it('returns the boolean value directly', () => { - expect(resolveTriggerEnabled({ foo: true }, 'foo')).toBe(true); - expect(resolveTriggerEnabled({ foo: false }, 'foo')).toBe(false); - }); - - it('respects opt-in keys when value is explicitly set', () => { - expect(resolveTriggerEnabled({ optKey: true }, 'optKey', { optInKeys: ['optKey'] })).toBe( - true, - ); - expect(resolveTriggerEnabled({ optKey: false }, 'optKey', { optInKeys: ['optKey'] })).toBe( - false, - ); - }); - }); - - describe('nested keys (per-agent objects)', () => { - it('returns true for boolean true nested key', () => { - expect(resolveTriggerEnabled({ rtp: true }, 'rtp', { nestedKeys: ['rtp'] })).toBe(true); - }); - - it('returns false for boolean false nested key', () => { - expect(resolveTriggerEnabled({ rtp: false }, 'rtp', { nestedKeys: ['rtp'] })).toBe(false); - }); - - it('returns true if any agent in the object is enabled', () => { - expect( - resolveTriggerEnabled( - { rtp: { splitting: false, planning: true, implementation: false } }, - 'rtp', - { nestedKeys: ['rtp'] }, - ), - ).toBe(true); - }); - - it('returns false if all agents in the object are disabled', () => { - expect( - resolveTriggerEnabled( - { rtp: { splitting: false, planning: false, implementation: false } }, - 'rtp', - { nestedKeys: ['rtp'] }, - ), - ).toBe(false); - }); - }); - - describe('object keys (non-boolean objects)', () => { - it('returns true when value is an object (non-boolean)', () => { - expect( - resolveTriggerEnabled({ rt: { ownPrsOnly: true } }, 'rt', { objectKeys: ['rt'] }), - ).toBe(true); - }); - - it('returns the boolean value when the key stores a boolean', () => { - expect(resolveTriggerEnabled({ rt: true }, 'rt', { objectKeys: ['rt'] })).toBe(true); - expect(resolveTriggerEnabled({ rt: false }, 'rt', { objectKeys: ['rt'] })).toBe(false); - }); - }); - - describe('backward-compat verification — wrapper behavior matches generic', () => { - it('resolveTrelloTriggerEnabled matches resolveTriggerEnabled for all Trello cases', () => { - const cases: [Record, string, boolean][] = [ - [{}, 'cardMovedToSplitting', true], - [{ cardMovedToSplitting: false }, 'cardMovedToSplitting', false], - [{ readyToProcessLabel: false }, 'readyToProcessLabel', false], - [{ readyToProcessLabel: true }, 'readyToProcessLabel', true], - [ - { readyToProcessLabel: { splitting: false, planning: true, implementation: false } }, - 'readyToProcessLabel', - true, - ], - [ - { readyToProcessLabel: { splitting: false, planning: false, implementation: false } }, - 'readyToProcessLabel', - false, - ], - ]; - for (const [config, key, expected] of cases) { - expect(resolveTriggerEnabled(config, key, { nestedKeys: ['readyToProcessLabel'] })).toBe( - expected, - ); - } - }); - - it('resolveGitHubTriggerEnabled matches resolveTriggerEnabled for opt-in and object keys', () => { - const optInKeys = ['reviewRequested', 'prOpened']; - const objectKeys = ['reviewTrigger']; - - // opt-in absent - expect(resolveTriggerEnabled(undefined, 'reviewRequested', { optInKeys, objectKeys })).toBe( - false, - ); - // opt-in enabled - expect( - resolveTriggerEnabled({ reviewRequested: true }, 'reviewRequested', { - optInKeys, - objectKeys, - }), - ).toBe(true); - // object key present - expect( - resolveTriggerEnabled( - { reviewTrigger: { ownPrsOnly: true, externalPrs: false, onReviewRequested: false } }, - 'reviewTrigger', - { optInKeys, objectKeys }, - ), - ).toBe(true); - }); - }); -}); - describe('EmailJokeTriggerConfigSchema', () => { it('accepts valid email address', () => { const result = EmailJokeTriggerConfigSchema.parse({ senderEmail: 'test@example.com' }); diff --git a/tests/unit/db/repositories/configMapper.test.ts b/tests/unit/db/repositories/configMapper.test.ts index 97d8064a..424fc40d 100644 --- a/tests/unit/db/repositories/configMapper.test.ts +++ b/tests/unit/db/repositories/configMapper.test.ts @@ -47,7 +47,6 @@ const trelloIntegrationRow: IntegrationRow = { category: 'pm', provider: 'trello', config: trelloConfig, - triggers: {}, }; const jiraIntegrationRow: IntegrationRow = { @@ -55,7 +54,6 @@ const jiraIntegrationRow: IntegrationRow = { category: 'pm', provider: 'jira', config: jiraConfig, - triggers: {}, }; const githubIntegrationRow: IntegrationRow = { @@ -63,7 +61,6 @@ const githubIntegrationRow: IntegrationRow = { category: 'scm', provider: 'github', config: {}, - triggers: { ownPrsOnly: true }, }; // --------------------------------------------------------------------------- @@ -214,18 +211,9 @@ describe('extractIntegrationConfigs', () => { expect(result.trelloConfig).toBeUndefined(); }); - it('extracts github triggers from integration rows', () => { + it('extracts github config from integration rows', () => { const result = extractIntegrationConfigs([githubIntegrationRow]); - expect(result.githubTriggers).toEqual({ ownPrsOnly: true }); - }); - - it('extracts trello triggers', () => { - const withTriggers: IntegrationRow = { - ...trelloIntegrationRow, - triggers: { cardMovedToTodo: true }, - }; - const result = extractIntegrationConfigs([withTriggers]); - expect(result.trelloTriggers).toEqual({ cardMovedToTodo: true }); + expect(result.githubConfig).toEqual({}); }); it('handles empty integration list', () => { @@ -239,7 +227,7 @@ describe('extractIntegrationConfigs', () => { const rows = [trelloIntegrationRow, githubIntegrationRow]; const result = extractIntegrationConfigs(rows); expect(result.trelloConfig).toEqual(trelloConfig); - expect(result.githubTriggers).toEqual({ ownPrsOnly: true }); + expect(result.githubConfig).toEqual({}); expect(result.jiraConfig).toBeUndefined(); }); }); @@ -295,16 +283,6 @@ describe('mapProjectRow', () => { expect(result.trello?.labels).toEqual({ processing: 'label-proc' }); }); - it('includes trello triggers when non-empty', () => { - const result = mapProjectRow(makeInput({ trelloTriggers: { cardMovedToTodo: true } })); - expect(result.trello?.triggers).toEqual({ cardMovedToTodo: true }); - }); - - it('omits trello triggers when empty object', () => { - const result = mapProjectRow(makeInput({ trelloTriggers: {} })); - expect(result.trello?.triggers).toBeUndefined(); - }); - it('builds jira config', () => { const result = mapProjectRow(makeInput({ trelloConfig: undefined, jiraConfig })); expect(result.jira?.projectKey).toBe('PROJ'); @@ -312,23 +290,6 @@ describe('mapProjectRow', () => { expect(result.jira?.statuses).toEqual({ splitting: 'Briefing', todo: 'To Do' }); }); - it('includes jira triggers when non-empty', () => { - const result = mapProjectRow( - makeInput({ trelloConfig: undefined, jiraConfig, jiraTriggers: { issueTransitioned: true } }), - ); - expect(result.jira?.triggers).toEqual({ issueTransitioned: true }); - }); - - it('builds github section when githubTriggers is non-empty', () => { - const result = mapProjectRow(makeInput({ githubTriggers: { ownPrsOnly: true } })); - expect(result.github?.triggers).toEqual({ ownPrsOnly: true }); - }); - - it('omits github section when githubTriggers is empty', () => { - const result = mapProjectRow(makeInput({ githubTriggers: {} })); - expect(result.github).toBeUndefined(); - }); - it('omits agentBackend when neither row.agentBackend nor agent overrides are set', () => { const result = mapProjectRow(makeInput()); expect(result.agentBackend).toBeUndefined(); diff --git a/tests/unit/triggers/backlog-status-changed.test.ts b/tests/unit/triggers/backlog-status-changed.test.ts index 20cc03fa..688525b0 100644 --- a/tests/unit/triggers/backlog-status-changed.test.ts +++ b/tests/unit/triggers/backlog-status-changed.test.ts @@ -9,6 +9,14 @@ vi.mock('../../../src/utils/logging.js', () => ({ }, })); +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + // Mocks required for PM integration registration (pm/index.js side-effect) vi.mock('../../../src/config/provider.js', () => ({ getIntegrationCredential: vi.fn(), diff --git a/tests/unit/triggers/check-suite-failure.test.ts b/tests/unit/triggers/check-suite-failure.test.ts index f2dae961..75928284 100644 --- a/tests/unit/triggers/check-suite-failure.test.ts +++ b/tests/unit/triggers/check-suite-failure.test.ts @@ -7,6 +7,15 @@ import type { TriggerContext } from '../../../src/triggers/types.js'; import { createMockProject } from '../../helpers/factories.js'; import { mockPersonaIdentities } from '../../helpers/mockPersonas.js'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + vi.mock('../../../src/github/client.js', () => ({ githubClient: { getPR: vi.fn(), @@ -21,6 +30,7 @@ vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ lookupWorkItemForPR: vi.fn(), })); import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; describe('CheckSuiteFailureTrigger', () => { const trigger = new CheckSuiteFailureTrigger(); @@ -121,6 +131,26 @@ describe('CheckSuiteFailureTrigger', () => { }); describe('handle', () => { + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test', + 'respond-to-ci', + 'scm:check-suite-failure', + 'check-suite-failure', + ); + }); + it('returns respond-to-ci result when PR has Trello URL and checks failed', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index 0a858be7..783b0615 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -7,6 +7,16 @@ import type { TriggerContext } from '../../../src/triggers/types.js'; import { createMockProject } from '../../helpers/factories.js'; import { mockPersonaIdentities } from '../../helpers/mockPersonas.js'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), + checkTriggerEnabledWithParams: vi.fn().mockResolvedValue({ enabled: true, parameters: {} }), +})); + vi.mock('../../../src/github/client.js', () => ({ githubClient: { getPR: vi.fn(), @@ -22,6 +32,7 @@ vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ })); import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; +import { checkTriggerEnabledWithParams } from '../../../src/triggers/shared/trigger-check.js'; describe('CheckSuiteSuccessTrigger', () => { const trigger = new CheckSuiteSuccessTrigger(); @@ -612,62 +623,35 @@ describe('CheckSuiteSuccessTrigger', () => { }); }); - describe('reviewTrigger mode-aware behavior', () => { - /** Project with only externalPrs enabled */ - const mockProjectExternalOnly = createMockProject({ - github: { - triggers: { - reviewTrigger: { ownPrsOnly: false, externalPrs: true, onReviewRequested: false }, - }, - }, - }); - - /** Project with both ownPrsOnly and externalPrs enabled */ - const mockProjectBothModes = createMockProject({ - github: { - triggers: { - reviewTrigger: { ownPrsOnly: true, externalPrs: true, onReviewRequested: false }, - }, - }, - }); - - /** Project with all modes disabled */ - const mockProjectNoModes = createMockProject({ - github: { - triggers: { - reviewTrigger: { ownPrsOnly: false, externalPrs: false, onReviewRequested: false }, - }, - }, - }); - - it('does not match when all modes are disabled', () => { - const ctx: TriggerContext = { - project: mockProjectNoModes, - source: 'github', - payload: makeCheckSuitePayload(), - }; - expect(trigger.matches(ctx)).toBe(false); - }); + describe('authorMode-aware behavior via trigger parameters', () => { + it('handle returns null when trigger is disabled via checkTriggerEnabledWithParams', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: false, + parameters: {}, + }); - it('matches when externalPrs is enabled', () => { const ctx: TriggerContext = { - project: mockProjectExternalOnly, + project: mockProject, source: 'github', payload: makeCheckSuitePayload(), + personaIdentities: mockPersonaIdentities, }; - expect(trigger.matches(ctx)).toBe(true); - }); - it('matches when both modes are enabled', () => { - const ctx: TriggerContext = { - project: mockProjectBothModes, - source: 'github', - payload: makeCheckSuitePayload(), - }; - expect(trigger.matches(ctx)).toBe(true); + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabledWithParams).toHaveBeenCalledWith( + 'test', + 'review', + 'scm:check-suite-success', + 'check-suite-success', + ); }); - it('triggers for external PR author when externalPrs=true', async () => { + it('triggers for external PR author when authorMode=external', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, title: 'External PR', @@ -683,7 +667,7 @@ describe('CheckSuiteSuccessTrigger', () => { vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); const ctx: TriggerContext = { - project: mockProjectExternalOnly, + project: mockProject, source: 'github', payload: makeCheckSuitePayload(), personaIdentities: mockPersonaIdentities, @@ -695,7 +679,11 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result?.agentType).toBe('review'); }); - it('skips implementer PR when only externalPrs=true', async () => { + it('skips implementer PR when authorMode=external', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, title: 'Implementer PR', @@ -710,7 +698,7 @@ describe('CheckSuiteSuccessTrigger', () => { }); const ctx: TriggerContext = { - project: mockProjectExternalOnly, + project: mockProject, source: 'github', payload: makeCheckSuitePayload(), personaIdentities: mockPersonaIdentities, @@ -721,7 +709,12 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result).toBeNull(); }); - it('triggers for both authors when ownPrsOnly=true and externalPrs=true', async () => { + it('triggers for both authors when authorMode=all', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ + enabled: true, + parameters: { authorMode: 'all' }, + }); + const setupMocks = (authorLogin: string) => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, @@ -741,7 +734,7 @@ describe('CheckSuiteSuccessTrigger', () => { // Implementer PR setupMocks('cascade-impl'); const implCtx: TriggerContext = { - project: mockProjectBothModes, + project: mockProject, source: 'github', payload: makeCheckSuitePayload(), personaIdentities: mockPersonaIdentities, @@ -754,7 +747,7 @@ describe('CheckSuiteSuccessTrigger', () => { vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); setupMocks('external-contributor'); const extCtx: TriggerContext = { - project: mockProjectBothModes, + project: mockProject, source: 'github', payload: makeCheckSuitePayload(), personaIdentities: mockPersonaIdentities, @@ -763,7 +756,11 @@ describe('CheckSuiteSuccessTrigger', () => { expect(extResult).not.toBeNull(); }); - it('backward compat: legacy checkSuiteSuccess=true still triggers for implementer PRs', async () => { + it('defaults to authorMode=own when no parameters configured', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: {}, + }); vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, title: 'Test PR', @@ -778,7 +775,6 @@ describe('CheckSuiteSuccessTrigger', () => { }); vi.mocked(githubClient.getPRReviews).mockResolvedValue([]); - // mockProject has no github triggers — resolves to legacy defaults (ownPrsOnly=true) const ctx: TriggerContext = { project: mockProject, source: 'github', @@ -790,17 +786,5 @@ describe('CheckSuiteSuccessTrigger', () => { expect(result).not.toBeNull(); expect(result?.agentType).toBe('review'); }); - - it('backward compat: legacy checkSuiteSuccess=false skips even implementer PRs', () => { - const ctx: TriggerContext = { - project: { - ...mockProject, - github: { triggers: { checkSuiteSuccess: false } }, - }, - source: 'github', - payload: makeCheckSuitePayload(), - }; - expect(trigger.matches(ctx)).toBe(false); - }); }); }); diff --git a/tests/unit/triggers/github-pr-comment-mention.test.ts b/tests/unit/triggers/github-pr-comment-mention.test.ts index 2e703052..a6bb00f1 100644 --- a/tests/unit/triggers/github-pr-comment-mention.test.ts +++ b/tests/unit/triggers/github-pr-comment-mention.test.ts @@ -6,6 +6,15 @@ const { mockGetPR, mockIsCascadeBot } = vi.hoisted(() => ({ mockIsCascadeBot: vi.fn(), })); +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + vi.mock('../../../src/github/client.js', () => ({ githubClient: { getPR: mockGetPR, @@ -31,6 +40,7 @@ vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { PRCommentMentionTrigger } from '../../../src/triggers/github/pr-comment-mention.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; import { createMockProject } from '../../helpers/factories.js'; import { @@ -150,6 +160,7 @@ describe('PRCommentMentionTrigger', () => { beforeEach(() => { vi.resetAllMocks(); trigger = new PRCommentMentionTrigger(); + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); mockIsCascadeBot.mockReturnValue(false); vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); mockGetPR.mockResolvedValue({ @@ -200,6 +211,21 @@ describe('PRCommentMentionTrigger', () => { }); }); + describe('handle — disabled trigger', () => { + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const result = await trigger.handle(buildCtx()); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test-project', + 'respond-to-pr-comment', + 'scm:pr-comment-mention', + 'pr-comment-mention', + ); + }); + }); + describe('handle — issue_comment path', () => { it('returns respond-to-pr-comment result when @mention is present', async () => { const result = await trigger.handle(buildCtx()); diff --git a/tests/unit/triggers/jira-comment-mention.test.ts b/tests/unit/triggers/jira-comment-mention.test.ts index 3356762c..b9d1d353 100644 --- a/tests/unit/triggers/jira-comment-mention.test.ts +++ b/tests/unit/triggers/jira-comment-mention.test.ts @@ -1,5 +1,4 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { TriggerContext } from '../../../src/types/index.js'; const { mockGetMyself } = vi.hoisted(() => ({ mockGetMyself: vi.fn(), @@ -11,8 +10,19 @@ vi.mock('../../../src/jira/client.js', () => ({ }, })); +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + +import type { TriggerContext } from '../../../src/types/index.js'; + // Import after vi.mock is hoisted import { JiraCommentMentionTrigger } from '../../../src/triggers/jira/comment-mention.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; const BOT_ACCOUNT_ID = 'bot-account-123'; const BOT_DISPLAY_NAME = 'CASCADE Bot'; @@ -114,6 +124,7 @@ describe('JiraCommentMentionTrigger', () => { beforeEach(() => { trigger = new JiraCommentMentionTrigger(); + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); mockGetMyself.mockResolvedValue({ accountId: BOT_ACCOUNT_ID, displayName: BOT_DISPLAY_NAME, @@ -143,6 +154,19 @@ describe('JiraCommentMentionTrigger', () => { }); describe('handle', () => { + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const result = await trigger.handle(buildCtx()); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test-project', + 'respond-to-planning-comment', + 'pm:comment-mention', + 'jira-comment-mention', + ); + }); + it('triggers agent when bot is @mentioned in comment', async () => { const ctx = buildCtx(); const result = await trigger.handle(ctx); diff --git a/tests/unit/triggers/jira-label-added.test.ts b/tests/unit/triggers/jira-label-added.test.ts index 79fbac90..94b876fb 100644 --- a/tests/unit/triggers/jira-label-added.test.ts +++ b/tests/unit/triggers/jira-label-added.test.ts @@ -1,5 +1,13 @@ import { describe, expect, it, vi } from 'vitest'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + // Mocks required for PM integration registration (pm/index.js side-effect) vi.mock('../../../src/config/provider.js', () => ({ getIntegrationCredential: vi.fn(), @@ -31,6 +39,7 @@ vi.mock('../../../src/router/reactions.js', () => ({ import '../../../src/pm/index.js'; import { JiraReadyToProcessLabelTrigger } from '../../../src/triggers/jira/label-added.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; import type { TriggerContext } from '../../../src/types/index.js'; const trigger = new JiraReadyToProcessLabelTrigger(); @@ -210,6 +219,19 @@ describe('JiraReadyToProcessLabelTrigger', () => { }); describe('handle()', () => { + it('should return null when trigger is disabled for the resolved agent', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const result = await trigger.handle(buildCtx({ statusName: 'Splitting' })); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test-project', + 'splitting', + 'pm:label-added', + 'jira-ready-to-process-label-added', + ); + }); + it('returns splitting agent for issue in Briefing status', async () => { const result = await trigger.handle(buildCtx({ statusName: 'Splitting' })); expect(result).not.toBeNull(); diff --git a/tests/unit/triggers/jira-status-changed.test.ts b/tests/unit/triggers/jira-status-changed.test.ts index 29c04fa6..9a708d03 100644 --- a/tests/unit/triggers/jira-status-changed.test.ts +++ b/tests/unit/triggers/jira-status-changed.test.ts @@ -9,7 +9,16 @@ vi.mock('../../../src/utils/logging.js', () => ({ }, })); +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + import { JiraStatusChangedTrigger } from '../../../src/triggers/jira/status-changed.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; const mockProject = { @@ -37,15 +46,9 @@ function buildCtx( issueKey?: string; statusChangeItems?: Array<{ field?: string; fromString?: string; toString?: string }>; noJiraConfig?: boolean; - triggers?: Record; } = {}, ): TriggerContext { - const baseJira = overrides.triggers - ? { ...mockProject.jira, triggers: overrides.triggers } - : mockProject.jira; - const project = overrides.noJiraConfig - ? { ...mockProject, jira: undefined } - : { ...mockProject, jira: baseJira }; + const project = overrides.noJiraConfig ? { ...mockProject, jira: undefined } : mockProject; return { project: project as TriggerContext['project'], @@ -70,6 +73,7 @@ describe('JiraStatusChangedTrigger', () => { beforeEach(() => { vi.resetAllMocks(); + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); trigger = new JiraStatusChangedTrigger(); }); @@ -209,48 +213,30 @@ describe('JiraStatusChangedTrigger', () => { expect(result).toBeNull(); }); - describe('per-agent statusChanged toggle', () => { - it('fires when statusChanged toggle is true for agent (legacy boolean)', async () => { - const ctx = buildCtx({ - statusChangeItems: [{ field: 'status', fromString: 'Backlog', toString: 'Splitting' }], - triggers: { statusChanged: true }, - }); - - const result = await trigger.handle(ctx); - - expect(result?.agentType).toBe('splitting'); - }); - - it('returns null when statusChanged disabled globally (legacy boolean false)', async () => { - const ctx = buildCtx({ - statusChangeItems: [{ field: 'status', fromString: 'Backlog', toString: 'Splitting' }], - triggers: { statusChanged: false }, - }); - - const result = await trigger.handle(ctx); - - expect(result).toBeNull(); - }); + describe('per-agent statusChanged toggle (via checkTriggerEnabled)', () => { + it('fires when trigger is enabled for agent', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); - it('fires when per-agent statusChanged.splitting is enabled', async () => { const ctx = buildCtx({ statusChangeItems: [{ field: 'status', fromString: 'Backlog', toString: 'Splitting' }], - triggers: { - statusChanged: { splitting: true, planning: false, implementation: false }, - }, }); const result = await trigger.handle(ctx); expect(result?.agentType).toBe('splitting'); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test-project', + 'splitting', + 'pm:status-changed', + 'jira-status-changed', + ); }); - it('returns null when per-agent statusChanged.splitting is disabled', async () => { + it('returns null when trigger is disabled for splitting agent', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValue(false); + const ctx = buildCtx({ statusChangeItems: [{ field: 'status', fromString: 'Backlog', toString: 'Splitting' }], - triggers: { - statusChanged: { splitting: false, planning: true, implementation: true }, - }, }); const result = await trigger.handle(ctx); @@ -258,12 +244,11 @@ describe('JiraStatusChangedTrigger', () => { expect(result).toBeNull(); }); - it('fires planning agent when statusChanged.planning is enabled', async () => { + it('fires planning agent when trigger is enabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); + const ctx = buildCtx({ statusChangeItems: [{ field: 'status', fromString: 'Splitting', toString: 'Planning' }], - triggers: { - statusChanged: { splitting: false, planning: true, implementation: false }, - }, }); const result = await trigger.handle(ctx); @@ -271,23 +256,11 @@ describe('JiraStatusChangedTrigger', () => { expect(result?.agentType).toBe('planning'); }); - it('returns null when per-agent statusChanged.implementation is disabled', async () => { - const ctx = buildCtx({ - statusChangeItems: [{ field: 'status', fromString: 'Planning', toString: 'To Do' }], - triggers: { - statusChanged: { splitting: true, planning: true, implementation: false }, - }, - }); - - const result = await trigger.handle(ctx); - - expect(result).toBeNull(); - }); + it('returns null when trigger is disabled for implementation agent', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValue(false); - it('backward compat: respects legacy issueTransitioned boolean toggle', async () => { const ctx = buildCtx({ - statusChangeItems: [{ field: 'status', fromString: 'Backlog', toString: 'Splitting' }], - triggers: { issueTransitioned: false }, + statusChangeItems: [{ field: 'status', fromString: 'Planning', toString: 'To Do' }], }); const result = await trigger.handle(ctx); diff --git a/tests/unit/triggers/label-added.test.ts b/tests/unit/triggers/label-added.test.ts index f0561bc3..3d840ce7 100644 --- a/tests/unit/triggers/label-added.test.ts +++ b/tests/unit/triggers/label-added.test.ts @@ -2,6 +2,14 @@ import { describe, expect, it, vi } from 'vitest'; import type { TriggerContext } from '../../../src/triggers/types.js'; import { createMockProject } from '../../helpers/factories.js'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + // Mocks required for PM integration registration (pm/index.js side-effect) vi.mock('../../../src/config/provider.js', () => ({ getIntegrationCredential: vi.fn(), @@ -33,6 +41,7 @@ vi.mock('../../../src/router/reactions.js', () => ({ import '../../../src/pm/index.js'; import { trelloClient } from '../../../src/trello/client.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; import { ReadyToProcessLabelTrigger } from '../../../src/triggers/trello/label-added.js'; describe('ReadyToProcessLabelTrigger', () => { @@ -131,6 +140,46 @@ describe('ReadyToProcessLabelTrigger', () => { }); describe('handle', () => { + it('should return null when trigger is disabled for the resolved agent', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + mockGetCard.mockResolvedValue({ + id: 'card123', + name: 'Test Card', + desc: '', + url: 'https://trello.com/c/abc', + shortUrl: 'https://trello.com/c/abc', + idList: 'splitting-list-id', + labels: [], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Board' }, + action: { + id: 'action1', + idMemberCreator: 'member1', + type: 'addLabelToCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Test Card', idShort: 1, shortLink: 'abc' }, + label: { id: 'ready-label-id', name: 'Ready', color: 'green' }, + }, + }, + }, + }; + + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test', + 'splitting', + 'pm:label-added', + 'ready-to-process-label-added', + ); + }); + it('returns splitting agent when card is in splitting list', async () => { mockGetCard.mockResolvedValue({ id: 'card123', diff --git a/tests/unit/triggers/pr-merged.test.ts b/tests/unit/triggers/pr-merged.test.ts index bb98e732..51c20124 100644 --- a/tests/unit/triggers/pr-merged.test.ts +++ b/tests/unit/triggers/pr-merged.test.ts @@ -1,5 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + // Mock the GitHub client vi.mock('../../../src/github/client.js', () => ({ githubClient: { @@ -56,6 +65,7 @@ import { createMockProject } from '../../helpers/factories.js'; import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { githubClient } from '../../../src/github/client.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; describe('PRMergedTrigger', () => { const trigger = new PRMergedTrigger(); @@ -130,7 +140,34 @@ describe('PRMergedTrigger', () => { }); describe('handle', () => { + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'closed', + number: 123, + pull_request: { number: 123, body: 'https://trello.com/c/abc123' }, + repository: { full_name: 'owner/repo' }, + }, + }; + + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test', + 'review', + 'scm:pr-merged', + 'pr-merged', + ); + }); + it('moves card to merged list when PR is merged', async () => { + // First call: scm:pr-merged = true; second call: backlog-manager = false + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(true).mockResolvedValueOnce(false); + vi.mocked(githubClient.getPR).mockResolvedValue({ number: 123, title: 'Test PR', @@ -299,7 +336,8 @@ describe('PRMergedTrigger', () => { }); }); - it('chains to backlog-manager when prMergedBacklogManager is enabled', async () => { + it('chains to backlog-manager when backlog-manager trigger is enabled', async () => { + // checkTriggerEnabled defaults to true from the mock, so backlog-manager will chain vi.mocked(githubClient.getPR).mockResolvedValue({ number: 123, title: 'Test PR', @@ -319,24 +357,8 @@ describe('PRMergedTrigger', () => { labels: [], }); - const projectWithBacklogManager = createMockProject({ - trello: { - boardId: 'board123', - lists: { - splitting: 'splitting-list-id', - planning: 'planning-list-id', - todo: 'todo-list-id', - merged: 'merged-list-id', - }, - labels: {}, - }, - github: { - triggers: { prMergedBacklogManager: true }, - }, - }); - const ctx: TriggerContext = { - project: projectWithBacklogManager, + project: mockProject, source: 'github', payload: { action: 'closed', @@ -361,7 +383,10 @@ describe('PRMergedTrigger', () => { }); }); - it('returns agentType null when prMergedBacklogManager is explicitly disabled', async () => { + it('returns agentType null when backlog-manager trigger is disabled', async () => { + // First call: scm:pr-merged = true; second call: backlog-manager = false + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(true).mockResolvedValueOnce(false); + vi.mocked(githubClient.getPR).mockResolvedValue({ number: 123, title: 'Test PR', @@ -381,24 +406,8 @@ describe('PRMergedTrigger', () => { labels: [], }); - const projectWithBacklogManagerDisabled = createMockProject({ - trello: { - boardId: 'board123', - lists: { - splitting: 'splitting-list-id', - planning: 'planning-list-id', - todo: 'todo-list-id', - merged: 'merged-list-id', - }, - labels: {}, - }, - github: { - triggers: { prMergedBacklogManager: false }, - }, - }); - const ctx: TriggerContext = { - project: projectWithBacklogManagerDisabled, + project: mockProject, source: 'github', payload: { action: 'closed', diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index 4ed09605..850127da 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -3,43 +3,34 @@ import { PROpenedTrigger } from '../../../src/triggers/github/pr-opened.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; import { createMockProject } from '../../helpers/factories.js'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), + checkTriggerEnabledWithParams: vi.fn().mockResolvedValue({ enabled: true, parameters: {} }), +})); + vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ lookupWorkItemForPR: vi.fn(), })); import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; +import { checkTriggerEnabledWithParams } from '../../../src/triggers/shared/trigger-check.js'; describe('PROpenedTrigger', () => { const trigger = new PROpenedTrigger(); const mockProject = createMockProject(); - /** Project with prOpened + externalPrs enabled (most common config for external PR review) */ - const mockProjectWithPrOpenedEnabled = createMockProject({ - github: { - triggers: { prOpened: true, reviewTrigger: { externalPrs: true } }, - }, - }); - - /** Project with prOpened + ownPrsOnly (fires on implementer-authored PRs) */ - const mockProjectWithOwnPrsOnly = createMockProject({ - github: { - triggers: { prOpened: true, reviewTrigger: { ownPrsOnly: true } }, - }, - }); - - /** Project with prOpened + both modes (fires on all PRs) */ - const mockProjectWithBothModes = createMockProject({ - github: { - triggers: { prOpened: true, reviewTrigger: { ownPrsOnly: true, externalPrs: true } }, - }, - }); - beforeEach(() => { vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ enabled: true, parameters: {} }); }); describe('matches', () => { - it('does not match by default (opt-in trigger, disabled without config)', () => { + it('matches when action is opened and not draft', () => { const ctx: TriggerContext = { project: mockProject, source: 'github', @@ -62,132 +53,12 @@ describe('PROpenedTrigger', () => { }, }; - expect(trigger.matches(ctx)).toBe(false); - }); - - it('matches when action is opened and not draft with prOpened + externalPrs enabled', () => { - const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, - source: 'github', - payload: { - action: 'opened', - number: 42, - pull_request: { - number: 42, - title: 'Test PR', - body: 'https://trello.com/c/abc123', - html_url: 'https://github.com/owner/repo/pull/42', - state: 'open', - draft: false, - head: { ref: 'feature/test', sha: 'abc123' }, - 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('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, + project: mockProject, source: 'trello', payload: {}, }; @@ -197,7 +68,7 @@ describe('PROpenedTrigger', () => { it('does not match when action is not opened', () => { const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', payload: { action: 'closed', @@ -223,7 +94,7 @@ describe('PROpenedTrigger', () => { it('does not match draft PRs', () => { const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', payload: { action: 'opened', @@ -249,7 +120,7 @@ describe('PROpenedTrigger', () => { it('does not match non-PR payloads', () => { const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', payload: { action: 'opened', @@ -263,8 +134,13 @@ describe('PROpenedTrigger', () => { describe('handle', () => { it('returns result when PR body has Trello URL', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'all' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -304,8 +180,13 @@ describe('PROpenedTrigger', () => { }); it('fires without work item when PR has no work item reference', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'all' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -333,9 +214,52 @@ describe('PROpenedTrigger', () => { expect(result?.workItemId).toBeUndefined(); }); - it('returns null for implementer PR when only externalPrs is enabled', async () => { + it('returns null when trigger is disabled via checkTriggerEnabledWithParams', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: false, + parameters: {}, + }); + const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, + source: 'github', + personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, + 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(await trigger.handle(ctx)).toBeNull(); + expect(checkTriggerEnabledWithParams).toHaveBeenCalledWith( + 'test', + 'review', + 'scm:pr-opened', + 'pr-opened', + ); + }); + + it('returns null for implementer PR when authorMode=external', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); + + const ctx: TriggerContext = { + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -360,9 +284,14 @@ describe('PROpenedTrigger', () => { expect(await trigger.handle(ctx)).toBeNull(); }); - it('fires for implementer PR when ownPrsOnly is enabled', async () => { + it('fires for implementer PR when authorMode=own', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'own' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithOwnPrsOnly, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -389,9 +318,14 @@ describe('PROpenedTrigger', () => { expect(result?.agentType).toBe('review'); }); - it('returns null for external PR when only ownPrsOnly is enabled', async () => { + it('returns null for external PR when authorMode=own', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'own' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithOwnPrsOnly, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -416,9 +350,14 @@ describe('PROpenedTrigger', () => { expect(await trigger.handle(ctx)).toBeNull(); }); - it('returns null for implementer [bot] variant when only externalPrs is enabled', async () => { + it('returns null for implementer [bot] variant when authorMode=external', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -443,9 +382,14 @@ describe('PROpenedTrigger', () => { expect(await trigger.handle(ctx)).toBeNull(); }); - it('fires for external PR when externalPrs is enabled', async () => { + it('fires for external PR when authorMode=external', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -472,9 +416,14 @@ describe('PROpenedTrigger', () => { expect(result?.agentType).toBe('review'); }); - it('fires for reviewer persona PR when externalPrs is enabled (reviewer is not implementer)', async () => { + it('fires for reviewer persona PR when authorMode=external (reviewer is not implementer)', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'external' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -501,9 +450,14 @@ describe('PROpenedTrigger', () => { expect(result?.agentType).toBe('review'); }); - it('fires for both implementer and external PRs when both modes enabled', async () => { + it('fires for both implementer and external PRs when authorMode=all', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValue({ + enabled: true, + parameters: { authorMode: 'all' }, + }); + const implCtx: TriggerContext = { - project: mockProjectWithBothModes, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -526,7 +480,7 @@ describe('PROpenedTrigger', () => { }; const extCtx: TriggerContext = { - project: mockProjectWithBothModes, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { @@ -554,7 +508,7 @@ describe('PROpenedTrigger', () => { it('returns null without personaIdentities (cannot determine author type)', async () => { const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', // no personaIdentities — credential resolution failed payload: { @@ -580,8 +534,13 @@ describe('PROpenedTrigger', () => { }); it('fires with undefined workItemId for null PR body', async () => { + vi.mocked(checkTriggerEnabledWithParams).mockResolvedValueOnce({ + enabled: true, + parameters: { authorMode: 'all' }, + }); + const ctx: TriggerContext = { - project: mockProjectWithPrOpenedEnabled, + project: mockProject, source: 'github', personaIdentities: { implementer: 'cascade-impl', reviewer: 'cascade-review' }, payload: { diff --git a/tests/unit/triggers/pr-ready-to-merge.test.ts b/tests/unit/triggers/pr-ready-to-merge.test.ts index 0dbc403a..8ee1a5e7 100644 --- a/tests/unit/triggers/pr-ready-to-merge.test.ts +++ b/tests/unit/triggers/pr-ready-to-merge.test.ts @@ -1,5 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + vi.mock('../../../src/github/client.js', () => ({ githubClient: { getPR: vi.fn(), @@ -57,6 +66,7 @@ import { createMockProject } from '../../helpers/factories.js'; import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { githubClient } from '../../../src/github/client.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; describe('PRReadyToMergeTrigger', () => { const trigger = new PRReadyToMergeTrigger(); @@ -238,6 +248,36 @@ describe('PRReadyToMergeTrigger', () => { }); describe('handle', () => { + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test', + 'review', + 'scm:pr-ready-to-merge', + 'pr-ready-to-merge', + ); + }); + it('moves card to DONE when check_suite triggers and all conditions met', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, diff --git a/tests/unit/triggers/pr-review-submitted.test.ts b/tests/unit/triggers/pr-review-submitted.test.ts index fb2301e6..197af803 100644 --- a/tests/unit/triggers/pr-review-submitted.test.ts +++ b/tests/unit/triggers/pr-review-submitted.test.ts @@ -4,10 +4,20 @@ import type { TriggerContext } from '../../../src/triggers/types.js'; import { createMockProject } from '../../helpers/factories.js'; import { mockPersonaIdentities } from '../../helpers/mockPersonas.js'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ lookupWorkItemForPR: vi.fn(), })); import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; describe('PRReviewSubmittedTrigger', () => { const trigger = new PRReviewSubmittedTrigger(); @@ -119,6 +129,26 @@ describe('PRReviewSubmittedTrigger', () => { }); describe('handle', () => { + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload(), + personaIdentities: mockPersonaIdentities, + }; + + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test', + 'respond-to-review', + 'scm:pr-review-submitted', + 'pr-review-submitted', + ); + }); + it('returns respond-to-review result when reviewer persona posts changes_requested', async () => { const ctx: TriggerContext = { project: mockProject, diff --git a/tests/unit/triggers/review-requested.test.ts b/tests/unit/triggers/review-requested.test.ts index 0cc1d745..e7c5bb31 100644 --- a/tests/unit/triggers/review-requested.test.ts +++ b/tests/unit/triggers/review-requested.test.ts @@ -4,34 +4,29 @@ import type { TriggerContext } from '../../../src/triggers/types.js'; import { createMockProject } from '../../helpers/factories.js'; import { mockPersonaIdentities } from '../../helpers/mockPersonas.js'; +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + vi.mock('../../../src/db/repositories/prWorkItemsRepository.js', () => ({ lookupWorkItemForPR: vi.fn(), })); import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; describe('ReviewRequestedTrigger', () => { const trigger = new ReviewRequestedTrigger(); const mockProject = createMockProject(); - /** Project with reviewRequested trigger explicitly enabled (legacy style) */ - const mockProjectWithReviewRequested = createMockProject({ - github: { - triggers: { reviewRequested: true }, - }, - }); - - /** Project with new structured reviewTrigger.onReviewRequested enabled */ - const mockProjectWithOnReviewRequested = createMockProject({ - github: { - triggers: { - reviewTrigger: { ownPrsOnly: false, externalPrs: false, onReviewRequested: true }, - }, - }, - }); - beforeEach(() => { vi.mocked(lookupWorkItemForPR).mockResolvedValue(null); + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); }); const makeReviewRequestedPayload = (reviewerLogin = 'cascade-reviewer') => ({ @@ -54,29 +49,19 @@ describe('ReviewRequestedTrigger', () => { }); describe('matches', () => { - it('does not match by default (opt-in trigger, disabled without config)', () => { + it('matches when review_requested action on a PR payload', () => { const ctx: TriggerContext = { project: mockProject, source: 'github', payload: makeReviewRequestedPayload(), personaIdentities: mockPersonaIdentities, }; - expect(trigger.matches(ctx)).toBe(false); - }); - - it('matches when review_requested and trigger is enabled', () => { - const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, - source: 'github', - payload: makeReviewRequestedPayload(), - personaIdentities: mockPersonaIdentities, - }; expect(trigger.matches(ctx)).toBe(true); }); it('does not match when source is not github', () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'trello', payload: makeReviewRequestedPayload(), personaIdentities: mockPersonaIdentities, @@ -86,7 +71,7 @@ describe('ReviewRequestedTrigger', () => { it('does not match on non-review_requested actions', () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: { ...makeReviewRequestedPayload(), @@ -99,7 +84,7 @@ describe('ReviewRequestedTrigger', () => { it('does not match when payload is not a PR payload', () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: { action: 'review_requested', something: 'else' }, personaIdentities: mockPersonaIdentities, @@ -111,7 +96,7 @@ describe('ReviewRequestedTrigger', () => { describe('handle', () => { it('returns null when no persona identities', async () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: makeReviewRequestedPayload(), // no personaIdentities @@ -122,7 +107,7 @@ describe('ReviewRequestedTrigger', () => { it('returns null when requested reviewer is not a CASCADE persona', async () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: makeReviewRequestedPayload('human-reviewer'), personaIdentities: mockPersonaIdentities, @@ -133,7 +118,7 @@ describe('ReviewRequestedTrigger', () => { it('returns null when no requested reviewer in payload', async () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: { ...makeReviewRequestedPayload(), @@ -147,7 +132,7 @@ describe('ReviewRequestedTrigger', () => { it('fires without work item when PR has no work item reference', async () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: { ...makeReviewRequestedPayload(), @@ -165,7 +150,7 @@ describe('ReviewRequestedTrigger', () => { it('triggers review agent when reviewer persona is requested', async () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: makeReviewRequestedPayload('cascade-reviewer'), personaIdentities: mockPersonaIdentities, @@ -185,7 +170,7 @@ describe('ReviewRequestedTrigger', () => { it('triggers review agent when implementer persona is requested', async () => { const ctx: TriggerContext = { - project: mockProjectWithReviewRequested, + project: mockProject, source: 'github', payload: makeReviewRequestedPayload('cascade-impl'), personaIdentities: mockPersonaIdentities, @@ -196,56 +181,31 @@ describe('ReviewRequestedTrigger', () => { }); }); - describe('new structured reviewTrigger config', () => { - it('matches when reviewTrigger.onReviewRequested=true', () => { - const ctx: TriggerContext = { - project: mockProjectWithOnReviewRequested, - source: 'github', - payload: makeReviewRequestedPayload(), - personaIdentities: mockPersonaIdentities, - }; - expect(trigger.matches(ctx)).toBe(true); - }); + describe('trigger config via checkTriggerEnabled', () => { + it('handle returns null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); - it('does not match when reviewTrigger.onReviewRequested=false', () => { const ctx: TriggerContext = { - project: { - ...mockProject, - github: { - triggers: { - reviewTrigger: { ownPrsOnly: true, externalPrs: true, onReviewRequested: false }, - }, - }, - }, + project: mockProject, source: 'github', - payload: makeReviewRequestedPayload(), + payload: makeReviewRequestedPayload('cascade-reviewer'), personaIdentities: mockPersonaIdentities, }; - expect(trigger.matches(ctx)).toBe(false); + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test', + 'review', + 'scm:review-requested', + 'review-requested', + ); }); - it('new config takes precedence over legacy reviewRequested=false', () => { - // reviewTrigger.onReviewRequested=true wins even when legacy reviewRequested=false - const ctx: TriggerContext = { - project: { - ...mockProject, - github: { - triggers: { - reviewRequested: false, // legacy says disabled - reviewTrigger: { ownPrsOnly: false, externalPrs: false, onReviewRequested: true }, - }, - }, - }, - source: 'github', - payload: makeReviewRequestedPayload(), - personaIdentities: mockPersonaIdentities, - }; - expect(trigger.matches(ctx)).toBe(true); - }); + it('triggers review agent when trigger is enabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(true); - it('triggers review agent using new config', async () => { const ctx: TriggerContext = { - project: mockProjectWithOnReviewRequested, + project: mockProject, source: 'github', payload: makeReviewRequestedPayload('cascade-reviewer'), personaIdentities: mockPersonaIdentities, diff --git a/tests/unit/triggers/status-changed.test.ts b/tests/unit/triggers/status-changed.test.ts index f9d6e493..76f13ce4 100644 --- a/tests/unit/triggers/status-changed.test.ts +++ b/tests/unit/triggers/status-changed.test.ts @@ -9,6 +9,14 @@ vi.mock('../../../src/utils/logging.js', () => ({ }, })); +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + // Mocks required for PM integration registration (pm/index.js side-effect) vi.mock('../../../src/config/provider.js', () => ({ getIntegrationCredential: vi.fn(), @@ -39,6 +47,7 @@ vi.mock('../../../src/router/reactions.js', () => ({ // Register PM integrations in the registry import '../../../src/pm/index.js'; +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; import { TrelloStatusChangedPlanningTrigger, TrelloStatusChangedSplittingTrigger, @@ -152,6 +161,38 @@ describe('TrelloStatusChangedSplittingTrigger', () => { expect(trigger.matches(ctx)).toBe(false); }); + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Board' }, + action: { + id: 'action1', + idMemberCreator: 'member1', + type: 'updateCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Test Card', idShort: 1, shortLink: 'abc' }, + listBefore: { id: 'other-list', name: 'Other' }, + listAfter: { id: 'splitting-list-id', name: 'Splitting' }, + }, + }, + }, + }; + + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test', + 'splitting', + 'pm:status-changed', + 'trello-status-changed-splitting', + ); + }); + it('handles and returns splitting agent', async () => { const ctx: TriggerContext = { project: mockProject, diff --git a/tests/unit/triggers/trello-comment-mention.test.ts b/tests/unit/triggers/trello-comment-mention.test.ts index 1eb356d1..ec3e428e 100644 --- a/tests/unit/triggers/trello-comment-mention.test.ts +++ b/tests/unit/triggers/trello-comment-mention.test.ts @@ -22,10 +22,19 @@ vi.mock('../../../src/utils/logging.js', () => ({ }, })); +vi.mock('../../../src/triggers/config-resolver.js', () => ({ + isTriggerEnabled: vi.fn().mockResolvedValue(true), + getTriggerParameters: vi.fn().mockResolvedValue({}), +})); +vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ + checkTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + // We need to reset the module-level cache between tests. // The module uses a module-level variable `cachedMemberInfo`. // We can reset it by re-importing with vi.resetModules() or by calling the exported functions. +import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; import { TrelloCommentMentionTrigger } from '../../../src/triggers/trello/comment-mention.js'; import type { TriggerContext } from '../../../src/triggers/types.js'; @@ -99,6 +108,7 @@ describe('TrelloCommentMentionTrigger', () => { beforeEach(() => { vi.resetAllMocks(); + vi.mocked(checkTriggerEnabled).mockResolvedValue(true); trigger = new TrelloCommentMentionTrigger(); // Reset the module-level member info cache by re-importing. // The cache is a module-level variable, so we set up getMe to always respond. @@ -133,6 +143,19 @@ describe('TrelloCommentMentionTrigger', () => { }); describe('handle', () => { + it('should return null when trigger is disabled', async () => { + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + + const result = await trigger.handle(buildCtx()); + expect(result).toBeNull(); + expect(checkTriggerEnabled).toHaveBeenCalledWith( + 'test-project', + 'respond-to-planning-comment', + 'pm:comment-mention', + 'trello-comment-mention', + ); + }); + it('returns respond-to-planning-comment result when @mention is present on PLANNING card', async () => { const result = await trigger.handle(buildCtx()); From 8cb83f4c996ec251a1480fa9dbe3d30afae5f589 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sat, 7 Mar 2026 07:06:02 +0000 Subject: [PATCH 2/3] fix: include evaluateAuthorMode in utils.ts (missed in prior commit) Co-Authored-By: Claude Opus 4.6 --- src/triggers/github/utils.ts | 45 ++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/triggers/github/utils.ts b/src/triggers/github/utils.ts index 61d2c9a5..86045cf4 100644 --- a/src/triggers/github/utils.ts +++ b/src/triggers/github/utils.ts @@ -2,6 +2,51 @@ import { lookupWorkItemForPR } from '../../db/repositories/prWorkItemsRepository import type { ProjectConfig } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; +export interface AuthorModeResult { + shouldTrigger: boolean; + authorMode: string; + isImplementerPR: boolean; +} + +/** + * Evaluate whether a trigger should fire based on the PR author and the + * configured `authorMode` parameter. + * + * Returns `null` when personaIdentities is missing (caller should return null). + * Validates authorMode against known values and falls back to 'own'. + */ +export function evaluateAuthorMode( + prAuthorLogin: string, + personaIdentities: { implementer: string } | undefined, + parameters: Record, + handlerName: string, +): AuthorModeResult | null { + if (!personaIdentities) { + logger.info('No persona identities available, skipping', { handler: handlerName }); + return null; + } + const implLogin = personaIdentities.implementer; + const isImplementerPR = prAuthorLogin === implLogin || prAuthorLogin === `${implLogin}[bot]`; + + const rawMode = parameters.authorMode; + const authorMode = + typeof rawMode === 'string' && ['own', 'external', 'all'].includes(rawMode) ? rawMode : 'own'; + + if (typeof rawMode === 'string' && authorMode !== rawMode) { + logger.warn('Invalid authorMode value, falling back to "own"', { + handler: handlerName, + configuredValue: rawMode, + }); + } + + const shouldTrigger = + authorMode === 'all' || + (authorMode === 'own' && isImplementerPR) || + (authorMode === 'external' && !isImplementerPR); + + return { shouldTrigger, authorMode, isImplementerPR }; +} + // Trello card URL pattern: https://trello.com/c/SHORT_ID/optional-slug const TRELLO_CARD_URL_REGEX = /https:\/\/trello\.com\/c\/([a-zA-Z0-9]+)/; From 44aa3a3800e987b12cb44c2ef583508d25c1597a Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sat, 7 Mar 2026 07:13:03 +0000 Subject: [PATCH 3/3] fix: update integration tests to use DB-driven trigger config Config toggle tests were using legacy `project_integrations.triggers` JSON which is no longer consulted. Updated to: - Seed `agent_trigger_configs` rows with `enabled: false` for disable tests - Test via `handle()` (which checks DB config) instead of `matches()` - Add `seedTriggerConfig` helper and include table in truncateAll Co-Authored-By: Claude Opus 4.6 --- tests/integration/github-personas.test.ts | 28 ++++++++++++++++------ tests/integration/helpers/db.ts | 1 + tests/integration/helpers/seed.ts | 25 +++++++++++++++++++ tests/integration/trigger-registry.test.ts | 27 +++++++++++++++------ 4 files changed, 67 insertions(+), 14 deletions(-) diff --git a/tests/integration/github-personas.test.ts b/tests/integration/github-personas.test.ts index 623d3574..4fb6d967 100644 --- a/tests/integration/github-personas.test.ts +++ b/tests/integration/github-personas.test.ts @@ -25,6 +25,7 @@ import { seedIntegrationCredential, seedOrg, seedProject, + seedTriggerConfig, } from './helpers/seed.js'; // ============================================================================ @@ -336,12 +337,11 @@ describe('GitHub Dual-Persona System (integration)', () => { // ========================================================================= describe('ReviewRequestedTrigger', () => { - it('does not match when onReviewRequested is disabled (default)', async () => { + it('returns null from handle() when scm:review-requested is disabled (default)', async () => { await seedIntegration({ category: 'scm', provider: 'github', config: {}, - // No reviewTrigger config → defaults to false }); const project = await findProjectByRepoFromDb('owner/repo'); @@ -356,15 +356,23 @@ describe('GitHub Dual-Persona System (integration)', () => { personaIdentities: TEST_PERSONAS, }; - expect(trigger.matches(ctx)).toBe(false); + // matches() checks payload shape (returns true), handle() checks DB config + // scm:review-requested has defaultEnabled: false in definition + expect(trigger.matches(ctx)).toBe(true); + const result = await trigger.handle(ctx); + expect(result).toBeNull(); }); - it('matches when onReviewRequested is enabled and persona is requested', async () => { + it('triggers review when enabled via DB and persona is requested', async () => { await seedIntegration({ category: 'scm', provider: 'github', config: {}, - triggers: { reviewTrigger: { onReviewRequested: true } }, + }); + await seedTriggerConfig({ + agentType: 'review', + triggerEvent: 'scm:review-requested', + enabled: true, }); const project = await findProjectByRepoFromDb('owner/repo'); @@ -380,14 +388,20 @@ describe('GitHub Dual-Persona System (integration)', () => { }; expect(trigger.matches(ctx)).toBe(true); + const result = await trigger.handle(ctx); + expect(result?.agentType).toBe('review'); }); - it('does not match when a non-persona reviewer is requested', async () => { + it('returns null when non-persona reviewer is requested', async () => { await seedIntegration({ category: 'scm', provider: 'github', config: {}, - triggers: { reviewTrigger: { onReviewRequested: true } }, + }); + await seedTriggerConfig({ + agentType: 'review', + triggerEvent: 'scm:review-requested', + enabled: true, }); const project = await findProjectByRepoFromDb('owner/repo'); diff --git a/tests/integration/helpers/db.ts b/tests/integration/helpers/db.ts index a91f8e9a..cb3731a3 100644 --- a/tests/integration/helpers/db.ts +++ b/tests/integration/helpers/db.ts @@ -30,6 +30,7 @@ export async function truncateAll() { pr_work_items, integration_credentials, project_integrations, + agent_trigger_configs, agent_configs, prompt_partials, sessions, diff --git a/tests/integration/helpers/seed.ts b/tests/integration/helpers/seed.ts index b007f3d5..038ef424 100644 --- a/tests/integration/helpers/seed.ts +++ b/tests/integration/helpers/seed.ts @@ -2,6 +2,7 @@ import { getDb } from '../../../src/db/client.js'; import { agentConfigs, agentRuns, + agentTriggerConfigs, cascadeDefaults, credentials, integrationCredentials, @@ -175,6 +176,30 @@ export async function seedAgentConfig( return row; } +/** + * Seeds an agent trigger config row (DB-driven trigger enable/disable). + */ +export async function seedTriggerConfig(overrides: { + projectId?: string; + agentType: string; + triggerEvent: string; + enabled?: boolean; + parameters?: Record; +}) { + const db = getDb(); + const [row] = await db + .insert(agentTriggerConfigs) + .values({ + projectId: overrides.projectId ?? 'test-project', + agentType: overrides.agentType, + triggerEvent: overrides.triggerEvent, + enabled: overrides.enabled ?? true, + parameters: overrides.parameters ?? {}, + }) + .returning(); + return row; +} + /** * Seeds an agent run row. */ diff --git a/tests/integration/trigger-registry.test.ts b/tests/integration/trigger-registry.test.ts index d9fae8e4..b1b2ee6d 100644 --- a/tests/integration/trigger-registry.test.ts +++ b/tests/integration/trigger-registry.test.ts @@ -20,7 +20,7 @@ import { import type { TriggerContext } from '../../src/types/index.js'; import { assertFound } from './helpers/assert.js'; import { truncateAll } from './helpers/db.js'; -import { seedIntegration, seedOrg, seedProject } from './helpers/seed.js'; +import { seedIntegration, seedOrg, seedProject, seedTriggerConfig } from './helpers/seed.js'; // ============================================================================ // Helpers @@ -373,7 +373,7 @@ describe('Trigger Registry (integration)', () => { // ========================================================================= describe('config toggles', () => { - it('does not fire status-changed-todo when statusChanged is disabled', async () => { + it('does not fire status-changed-todo when trigger is disabled via DB', async () => { await seedIntegration({ category: 'pm', provider: 'trello', @@ -382,7 +382,11 @@ describe('Trigger Registry (integration)', () => { lists: { todo: 'list-todo-123' }, labels: {}, }, - triggers: { statusChanged: false }, + }); + await seedTriggerConfig({ + agentType: 'implementation', + triggerEvent: 'pm:status-changed', + enabled: false, }); const project = await findProjectByBoardIdFromDb('board-123'); @@ -397,10 +401,13 @@ describe('Trigger Registry (integration)', () => { }), }; - expect(TrelloStatusChangedTodoTrigger.matches(ctx)).toBe(false); + // matches() checks payload shape (returns true), handle() checks DB config + expect(TrelloStatusChangedTodoTrigger.matches(ctx)).toBe(true); + const result = await TrelloStatusChangedTodoTrigger.handle(ctx); + expect(result).toBeNull(); }); - it('does not fire status-changed-splitting when statusChanged is disabled', async () => { + it('does not fire status-changed-splitting when trigger is disabled via DB', async () => { await seedIntegration({ category: 'pm', provider: 'trello', @@ -409,7 +416,11 @@ describe('Trigger Registry (integration)', () => { lists: { splitting: 'list-split-789' }, labels: {}, }, - triggers: { statusChanged: false }, + }); + await seedTriggerConfig({ + agentType: 'splitting', + triggerEvent: 'pm:status-changed', + enabled: false, }); const project = await findProjectByBoardIdFromDb('board-123'); @@ -424,7 +435,9 @@ describe('Trigger Registry (integration)', () => { }), }; - expect(TrelloStatusChangedSplittingTrigger.matches(ctx)).toBe(false); + expect(TrelloStatusChangedSplittingTrigger.matches(ctx)).toBe(true); + const result = await TrelloStatusChangedSplittingTrigger.handle(ctx); + expect(result).toBeNull(); }); });