diff --git a/src/agents/definitions/planning.yaml b/src/agents/definitions/planning.yaml index 7ea28d07..0c878ac5 100644 --- a/src/agents/definitions/planning.yaml +++ b/src/agents/definitions/planning.yaml @@ -44,12 +44,6 @@ triggers: options: [planning] defaultValue: planning contextPipeline: [directoryListing, contextFiles, squint, workItem] - - event: pm:comment-mention - label: Comment @mention - description: Trigger when bot is @mentioned in a card/issue comment - defaultEnabled: false - contextPipeline: [directoryListing, contextFiles, squint, workItem] - strategies: {} hooks: diff --git a/src/agents/definitions/review.yaml b/src/agents/definitions/review.yaml index 3bd11888..cda573ba 100644 --- a/src/agents/definitions/review.yaml +++ b/src/agents/definitions/review.yaml @@ -56,19 +56,6 @@ triggers: 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: false - providers: [github] - contextPipeline: [] - - event: scm:pr-merged - label: PR Merged - description: Move work item to MERGED status when PR is merged - defaultEnabled: false - providers: [github] - contextPipeline: [] - strategies: {} prompts: diff --git a/src/triggers/github/pr-merged.ts b/src/triggers/github/pr-merged.ts index 5a2d6a6b..8cbb3bd2 100644 --- a/src/triggers/github/pr-merged.ts +++ b/src/triggers/github/pr-merged.ts @@ -5,6 +5,7 @@ import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/ import { logger } from '../../utils/logging.js'; import { parseRepoFullName } from '../../utils/repo.js'; import { isPipelineAtCapacity } from '../shared/backlog-check.js'; +import { isLifecycleTriggerEnabled } from '../shared/lifecycle-check.js'; import { checkTriggerEnabled } from '../shared/trigger-check.js'; import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; import { resolveWorkItemId } from './utils.js'; @@ -21,8 +22,8 @@ export class PRMergedTrigger implements TriggerHandler { } 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))) { + // Check lifecycle trigger config (stored in project_integrations.triggers) + if (!(await isLifecycleTriggerEnabled(ctx.project.id, 'prMerged', this.name))) { return null; } diff --git a/src/triggers/github/pr-ready-to-merge.ts b/src/triggers/github/pr-ready-to-merge.ts index 73b6c94c..0be98982 100644 --- a/src/triggers/github/pr-ready-to-merge.ts +++ b/src/triggers/github/pr-ready-to-merge.ts @@ -6,7 +6,7 @@ import type { PMProvider } from '../../pm/types.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 { isLifecycleTriggerEnabled } from '../shared/lifecycle-check.js'; import { type GitHubCheckSuitePayload, type GitHubPullRequestReviewPayload, @@ -111,10 +111,8 @@ export class PRReadyToMergeTrigger implements TriggerHandler { // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: intentional — multiple review/check paths with auto-merge branching 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)) - ) { + // Check lifecycle trigger config (stored in project_integrations.triggers) + if (!(await isLifecycleTriggerEnabled(ctx.project.id, 'prReadyToMerge', this.name))) { return null; } diff --git a/src/triggers/shared/lifecycle-check.ts b/src/triggers/shared/lifecycle-check.ts new file mode 100644 index 00000000..01618cd2 --- /dev/null +++ b/src/triggers/shared/lifecycle-check.ts @@ -0,0 +1,35 @@ +/** + * Helper for checking lifecycle trigger configuration. + * + * Lifecycle triggers (prReadyToMerge, prMerged) are stored in the + * project_integrations.triggers JSONB column under the 'scm' integration, + * not in the agent_trigger_configs table. They default to disabled. + */ + +import { getIntegrationByProjectAndCategory } from '../../db/repositories/integrationsRepository.js'; +import { logger } from '../../utils/logging.js'; + +/** + * Check whether a lifecycle trigger is enabled for a project. + * Reads from project_integrations.triggers JSONB for the 'scm' integration. + * Defaults to false when not configured. + */ +export async function isLifecycleTriggerEnabled( + projectId: string, + triggerKey: string, + handlerName: string, +): Promise { + const integration = await getIntegrationByProjectAndCategory(projectId, 'scm'); + const triggers = (integration?.triggers as Record) ?? {}; + const enabled = typeof triggers[triggerKey] === 'boolean' ? triggers[triggerKey] : false; + + if (!enabled) { + logger.info('Lifecycle trigger disabled by config, skipping', { + handler: handlerName, + triggerKey, + projectId, + }); + } + + return enabled as boolean; +} diff --git a/tests/unit/agents/definitions/loader.test.ts b/tests/unit/agents/definitions/loader.test.ts index f5c747cb..d20f278d 100644 --- a/tests/unit/agents/definitions/loader.test.ts +++ b/tests/unit/agents/definitions/loader.test.ts @@ -177,6 +177,20 @@ describe('YAML agent definitions loader', () => { expect(ciPassedTrigger?.contextPipeline).toEqual(['prContext', 'contextFiles', 'squint']); }); + it('planning agent does not have pm:comment-mention trigger (routed to respond-to-planning-comment)', () => { + const def = loadAgentDefinition('planning'); + const commentMentionTrigger = def.triggers.find((t) => t.event === 'pm:comment-mention'); + expect(commentMentionTrigger).toBeUndefined(); + }); + + it('review agent does not have lifecycle triggers (scm:pr-ready-to-merge, scm:pr-merged)', () => { + const def = loadAgentDefinition('review'); + const prReadyTrigger = def.triggers.find((t) => t.event === 'scm:pr-ready-to-merge'); + const prMergedTrigger = def.triggers.find((t) => t.event === 'scm:pr-merged'); + expect(prReadyTrigger).toBeUndefined(); + expect(prMergedTrigger).toBeUndefined(); + }); + it('respond-to-ci trigger uses combined PR + work-item pipeline', () => { const def = loadAgentDefinition('respond-to-ci'); const ciFailureTrigger = def.triggers.find((t) => t.event === 'scm:check-suite-failure'); diff --git a/tests/unit/triggers/pr-merged.test.ts b/tests/unit/triggers/pr-merged.test.ts index c8e2ff9a..50d3e6f6 100644 --- a/tests/unit/triggers/pr-merged.test.ts +++ b/tests/unit/triggers/pr-merged.test.ts @@ -9,6 +9,10 @@ vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ checkTriggerEnabled: vi.fn().mockResolvedValue(true), })); +vi.mock('../../../src/triggers/shared/lifecycle-check.js', () => ({ + isLifecycleTriggerEnabled: vi.fn().mockResolvedValue(true), +})); + vi.mock('../../../src/triggers/shared/backlog-check.js', () => ({ isPipelineAtCapacity: vi.fn().mockResolvedValue({ atCapacity: false, reason: 'below-capacity' }), })); @@ -70,6 +74,7 @@ import { createMockProject } from '../../helpers/factories.js'; import { lookupWorkItemForPR } from '../../../src/db/repositories/prWorkItemsRepository.js'; import { githubClient } from '../../../src/github/client.js'; import { isPipelineAtCapacity } from '../../../src/triggers/shared/backlog-check.js'; +import { isLifecycleTriggerEnabled } from '../../../src/triggers/shared/lifecycle-check.js'; import { checkTriggerEnabled } from '../../../src/triggers/shared/trigger-check.js'; describe('PRMergedTrigger', () => { @@ -147,7 +152,7 @@ describe('PRMergedTrigger', () => { describe('handle', () => { it('should return null when trigger is disabled', async () => { - vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + vi.mocked(isLifecycleTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { project: mockProject, @@ -162,17 +167,13 @@ describe('PRMergedTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(checkTriggerEnabled).toHaveBeenCalledWith( - 'test', - 'review', - 'scm:pr-merged', - 'pr-merged', - ); + expect(isLifecycleTriggerEnabled).toHaveBeenCalledWith('test', 'prMerged', '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); + // isLifecycleTriggerEnabled: prMerged = true; then checkTriggerEnabled: backlog-manager = false + vi.mocked(isLifecycleTriggerEnabled).mockResolvedValueOnce(true); + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); vi.mocked(githubClient.getPR).mockResolvedValue({ number: 123, @@ -357,9 +358,9 @@ describe('PRMergedTrigger', () => { }); it('skips move/comment and returns null when card already merged and backlog-manager disabled', async () => { - vi.mocked(checkTriggerEnabled) - .mockResolvedValueOnce(true) // scm:pr-merged enabled - .mockResolvedValueOnce(false); // backlog-manager disabled + // isLifecycleTriggerEnabled: prMerged = true; then checkTriggerEnabled: backlog-manager = false + vi.mocked(isLifecycleTriggerEnabled).mockResolvedValueOnce(true); + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); // backlog-manager disabled vi.mocked(githubClient.getPR).mockResolvedValue({ number: 123, @@ -460,8 +461,9 @@ describe('PRMergedTrigger', () => { }); 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); + // isLifecycleTriggerEnabled: prMerged = true; then checkTriggerEnabled: backlog-manager = false + vi.mocked(isLifecycleTriggerEnabled).mockResolvedValueOnce(true); + vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); vi.mocked(githubClient.getPR).mockResolvedValue({ number: 123, diff --git a/tests/unit/triggers/pr-ready-to-merge.test.ts b/tests/unit/triggers/pr-ready-to-merge.test.ts index 332fd53b..98c556c9 100644 --- a/tests/unit/triggers/pr-ready-to-merge.test.ts +++ b/tests/unit/triggers/pr-ready-to-merge.test.ts @@ -5,8 +5,8 @@ vi.mock('../../../src/triggers/config-resolver.js', () => ({ getTriggerParameters: vi.fn().mockResolvedValue({}), })); -vi.mock('../../../src/triggers/shared/trigger-check.js', () => ({ - checkTriggerEnabled: vi.fn().mockResolvedValue(true), +vi.mock('../../../src/triggers/shared/lifecycle-check.js', () => ({ + isLifecycleTriggerEnabled: vi.fn().mockResolvedValue(true), })); vi.mock('../../../src/github/client.js', () => ({ @@ -67,7 +67,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'; +import { isLifecycleTriggerEnabled } from '../../../src/triggers/shared/lifecycle-check.js'; describe('PRReadyToMergeTrigger', () => { const trigger = new PRReadyToMergeTrigger(); @@ -250,7 +250,7 @@ describe('PRReadyToMergeTrigger', () => { describe('handle', () => { it('should return null when trigger is disabled', async () => { - vi.mocked(checkTriggerEnabled).mockResolvedValueOnce(false); + vi.mocked(isLifecycleTriggerEnabled).mockResolvedValueOnce(false); const ctx: TriggerContext = { project: mockProject, @@ -271,10 +271,9 @@ describe('PRReadyToMergeTrigger', () => { const result = await trigger.handle(ctx); expect(result).toBeNull(); - expect(checkTriggerEnabled).toHaveBeenCalledWith( + expect(isLifecycleTriggerEnabled).toHaveBeenCalledWith( 'test', - 'review', - 'scm:pr-ready-to-merge', + 'prReadyToMerge', 'pr-ready-to-merge', ); }); diff --git a/tests/unit/triggers/shared/lifecycle-check.test.ts b/tests/unit/triggers/shared/lifecycle-check.test.ts new file mode 100644 index 00000000..d7433965 --- /dev/null +++ b/tests/unit/triggers/shared/lifecycle-check.test.ts @@ -0,0 +1,133 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockGetIntegrationByProjectAndCategory, mockLogger } = vi.hoisted(() => ({ + mockGetIntegrationByProjectAndCategory: vi.fn(), + mockLogger: { + info: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + error: vi.fn(), + }, +})); + +vi.mock('../../../../src/db/repositories/integrationsRepository.js', () => ({ + getIntegrationByProjectAndCategory: mockGetIntegrationByProjectAndCategory, +})); + +vi.mock('../../../../src/utils/logging.js', () => ({ + logger: mockLogger, +})); + +import { isLifecycleTriggerEnabled } from '../../../../src/triggers/shared/lifecycle-check.js'; + +const PROJECT_ID = 'project-1'; +const HANDLER_NAME = 'test-handler'; + +describe('isLifecycleTriggerEnabled', () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it('returns true when lifecycle trigger is explicitly enabled in scm integration', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue({ + id: 1, + projectId: PROJECT_ID, + category: 'scm', + provider: 'github', + config: {}, + triggers: { prReadyToMerge: true, prMerged: false }, + }); + + const result = await isLifecycleTriggerEnabled(PROJECT_ID, 'prReadyToMerge', HANDLER_NAME); + + expect(result).toBe(true); + }); + + it('returns false when lifecycle trigger is explicitly disabled', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue({ + id: 1, + projectId: PROJECT_ID, + category: 'scm', + provider: 'github', + config: {}, + triggers: { prReadyToMerge: false }, + }); + + const result = await isLifecycleTriggerEnabled(PROJECT_ID, 'prReadyToMerge', HANDLER_NAME); + + expect(result).toBe(false); + }); + + it('returns false when trigger key is not present (default disabled)', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue({ + id: 1, + projectId: PROJECT_ID, + category: 'scm', + provider: 'github', + config: {}, + triggers: {}, + }); + + const result = await isLifecycleTriggerEnabled(PROJECT_ID, 'prReadyToMerge', HANDLER_NAME); + + expect(result).toBe(false); + }); + + it('returns false when no scm integration exists', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue(null); + + const result = await isLifecycleTriggerEnabled(PROJECT_ID, 'prMerged', HANDLER_NAME); + + expect(result).toBe(false); + }); + + it('returns false when triggers column is null', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue({ + id: 1, + projectId: PROJECT_ID, + category: 'scm', + provider: 'github', + config: {}, + triggers: null, + }); + + const result = await isLifecycleTriggerEnabled(PROJECT_ID, 'prMerged', HANDLER_NAME); + + expect(result).toBe(false); + }); + + it('logs skip message when trigger is disabled', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue(null); + + await isLifecycleTriggerEnabled(PROJECT_ID, 'prReadyToMerge', HANDLER_NAME); + + expect(mockLogger.info).toHaveBeenCalledWith('Lifecycle trigger disabled by config, skipping', { + handler: HANDLER_NAME, + triggerKey: 'prReadyToMerge', + projectId: PROJECT_ID, + }); + }); + + it('does not log when trigger is enabled', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue({ + id: 1, + projectId: PROJECT_ID, + category: 'scm', + provider: 'github', + config: {}, + triggers: { prReadyToMerge: true }, + }); + + await isLifecycleTriggerEnabled(PROJECT_ID, 'prReadyToMerge', HANDLER_NAME); + + expect(mockLogger.info).not.toHaveBeenCalled(); + }); + + it('queries the scm integration category', async () => { + mockGetIntegrationByProjectAndCategory.mockResolvedValue(null); + + await isLifecycleTriggerEnabled(PROJECT_ID, 'prMerged', HANDLER_NAME); + + expect(mockGetIntegrationByProjectAndCategory).toHaveBeenCalledWith(PROJECT_ID, 'scm'); + }); +}); diff --git a/tests/unit/web/triggerAgentMapping.test.ts b/tests/unit/web/triggerAgentMapping.test.ts index 3d2e93d4..4fff2ba0 100644 --- a/tests/unit/web/triggerAgentMapping.test.ts +++ b/tests/unit/web/triggerAgentMapping.test.ts @@ -68,6 +68,12 @@ describe('LIFECYCLE_TRIGGERS', () => { expect(trigger.category).toBe('scm'); } }); + + it('lifecycle triggers default to disabled (defaultValue: false)', () => { + for (const trigger of LIFECYCLE_TRIGGERS) { + expect(trigger.defaultValue).toBe(false); + } + }); }); describe('getTriggerValue', () => { diff --git a/web/src/components/projects/project-general-form.tsx b/web/src/components/projects/project-general-form.tsx index a794b7cd..eb671b0a 100644 --- a/web/src/components/projects/project-general-form.tsx +++ b/web/src/components/projects/project-general-form.tsx @@ -1,6 +1,5 @@ import { ProjectSecretField } from '@/components/projects/project-secret-field.js'; import { useProjectUpdate } from '@/components/projects/use-project-update.js'; -import { Badge } from '@/components/ui/badge.js'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card.js'; import { Input } from '@/components/ui/input.js'; import { Label } from '@/components/ui/label.js'; @@ -12,7 +11,6 @@ import { } from '@/components/ui/tooltip.js'; import { trpc } from '@/lib/trpc.js'; import { useQuery } from '@tanstack/react-query'; -import { Link } from '@tanstack/react-router'; import { HelpCircle } from 'lucide-react'; import { useMemo, useState } from 'react'; import { toast } from 'sonner'; @@ -50,7 +48,6 @@ function minutesToMs(minutes: string): number | null { return Number.isNaN(parsed) ? null : parsed * 60000; } -// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: five independent form sections (identity, budget, progress, watchdog, run links) with shared dirty-state tracking and reset logic export function ProjectGeneralForm({ project }: { project: Project }) { const updateMutation = useProjectUpdate(project.id); const credentialsQuery = useQuery( @@ -161,36 +158,6 @@ export function ProjectGeneralForm({ project }: { project: Project }) { Project Identity -
- ID: - - {project.id} - -
-
- Repository: - {project.repo ? ( - - {project.repo} - - ) : ( - - Not configured —{' '} - - set on Integrations tab → - - - )} -
setName(e.target.value)} required /> @@ -335,8 +302,7 @@ export function ProjectGeneralForm({ project }: { project: Project }) { Enable run links in comments

- Adds a dashboard link to agent comments. Requires{' '} - CASCADE_DASHBOARD_URL env var. + Adds a dashboard link to agent comments.

diff --git a/web/src/lib/trigger-agent-mapping.ts b/web/src/lib/trigger-agent-mapping.ts index 87cba8a0..d2526d2f 100644 --- a/web/src/lib/trigger-agent-mapping.ts +++ b/web/src/lib/trigger-agent-mapping.ts @@ -38,7 +38,7 @@ export const LIFECYCLE_TRIGGERS: TriggerDef[] = [ key: 'prReadyToMerge', label: 'PR Ready to Merge', description: 'Auto-move card to DONE when PR is approved and checks pass.', - defaultValue: true, + defaultValue: false, scmProvider: 'github', category: 'scm', }, @@ -46,7 +46,7 @@ export const LIFECYCLE_TRIGGERS: TriggerDef[] = [ key: 'prMerged', label: 'PR Merged', description: 'Auto-move card to MERGED when PR is merged.', - defaultValue: true, + defaultValue: false, scmProvider: 'github', category: 'scm', },