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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/agents/definitions/planning.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 0 additions & 13 deletions src/agents/definitions/review.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions src/triggers/github/pr-merged.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -21,8 +22,8 @@ export class PRMergedTrigger implements TriggerHandler {
}

async handle(ctx: TriggerContext): Promise<TriggerResult | null> {
// 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;
}

Expand Down
8 changes: 3 additions & 5 deletions src/triggers/github/pr-ready-to-merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<TriggerResult | null> {
// 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;
}

Expand Down
35 changes: 35 additions & 0 deletions src/triggers/shared/lifecycle-check.ts
Original file line number Diff line number Diff line change
@@ -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<boolean> {
const integration = await getIntegrationByProjectAndCategory(projectId, 'scm');
const triggers = (integration?.triggers as Record<string, unknown>) ?? {};
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;
}
14 changes: 14 additions & 0 deletions tests/unit/agents/definitions/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
30 changes: 16 additions & 14 deletions tests/unit/triggers/pr-merged.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }),
}));
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 6 additions & 7 deletions tests/unit/triggers/pr-ready-to-merge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -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',
);
});
Expand Down
133 changes: 133 additions & 0 deletions tests/unit/triggers/shared/lifecycle-check.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
6 changes: 6 additions & 0 deletions tests/unit/web/triggerAgentMapping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading
Loading