diff --git a/package-lock.json b/package-lock.json index 726327567..b2caac049 100644 --- a/package-lock.json +++ b/package-lock.json @@ -98,9 +98,9 @@ } }, "node_modules/@anthropic-ai/claude-agent-sdk": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk/-/claude-agent-sdk-0.2.131.tgz", - "integrity": "sha512-4Xak+BlcxXuni5BvNeb0tnSapIoCBxE7cFnXvkUs0EwbY88FkmdJEtBXZbF7NRuN8bUwDeNxvy0Fs0dWnzpU+g==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk/-/claude-agent-sdk-0.2.136.tgz", + "integrity": "sha512-43zwEEyv39+jPglzSDSBu90cZvtuKB8QsVJL/PuDt6XghhPubjhXaBJ0qdFcyHSJXrKuyKms1aHXqykRwwzTFQ==", "license": "SEE LICENSE IN README.md", "dependencies": { "@anthropic-ai/sdk": "^0.81.0", @@ -110,23 +110,23 @@ "node": ">=18.0.0" }, "optionalDependencies": { - "@anthropic-ai/claude-agent-sdk-darwin-arm64": "0.2.131", - "@anthropic-ai/claude-agent-sdk-darwin-x64": "0.2.131", - "@anthropic-ai/claude-agent-sdk-linux-arm64": "0.2.131", - "@anthropic-ai/claude-agent-sdk-linux-arm64-musl": "0.2.131", - "@anthropic-ai/claude-agent-sdk-linux-x64": "0.2.131", - "@anthropic-ai/claude-agent-sdk-linux-x64-musl": "0.2.131", - "@anthropic-ai/claude-agent-sdk-win32-arm64": "0.2.131", - "@anthropic-ai/claude-agent-sdk-win32-x64": "0.2.131" + "@anthropic-ai/claude-agent-sdk-darwin-arm64": "0.2.136", + "@anthropic-ai/claude-agent-sdk-darwin-x64": "0.2.136", + "@anthropic-ai/claude-agent-sdk-linux-arm64": "0.2.136", + "@anthropic-ai/claude-agent-sdk-linux-arm64-musl": "0.2.136", + "@anthropic-ai/claude-agent-sdk-linux-x64": "0.2.136", + "@anthropic-ai/claude-agent-sdk-linux-x64-musl": "0.2.136", + "@anthropic-ai/claude-agent-sdk-win32-arm64": "0.2.136", + "@anthropic-ai/claude-agent-sdk-win32-x64": "0.2.136" }, "peerDependencies": { "zod": "^4.0.0" } }, "node_modules/@anthropic-ai/claude-agent-sdk-darwin-arm64": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-darwin-arm64/-/claude-agent-sdk-darwin-arm64-0.2.131.tgz", - "integrity": "sha512-jOGq8lAi6bakqX0MBVkJDOddC2xSYnP1XHzps2cBF696dQlHoXs4hqU+69Wt4oKScyw4tM4Pe+Mmeut9LJqbEg==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-darwin-arm64/-/claude-agent-sdk-darwin-arm64-0.2.136.tgz", + "integrity": "sha512-EmUkFbUhKEu51Nc01dS3Mwn4YTcwjUORAydoolGY0ZGPJt04WL9ske0QO5L4MU9M43L/5FAByfn9GnDEjX3ONg==", "cpu": [ "arm64" ], @@ -137,9 +137,9 @@ ] }, "node_modules/@anthropic-ai/claude-agent-sdk-darwin-x64": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-darwin-x64/-/claude-agent-sdk-darwin-x64-0.2.131.tgz", - "integrity": "sha512-IxewhApb20ucAxnpUCAwETLjO5PsQRAJIBBlDlNqPsd20LIZVVQuQ5orFf6CGEs6MfYRnWz2FYwfHhguGNPIyQ==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-darwin-x64/-/claude-agent-sdk-darwin-x64-0.2.136.tgz", + "integrity": "sha512-uSh7OGZVl7vTd+3ybCRfCqUdAMS8E8TnqCFzC7XtqHoUxbyQpeavTaW22dMP7b0a1RsDDQ1NsvEGcQIue8XYkg==", "cpu": [ "x64" ], @@ -150,15 +150,12 @@ ] }, "node_modules/@anthropic-ai/claude-agent-sdk-linux-arm64": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-arm64/-/claude-agent-sdk-linux-arm64-0.2.131.tgz", - "integrity": "sha512-GDwaga8aadtVeYq1wJM2BSWp5l/Srel7L5WRbEvkEWXeGP463S7VLJyiNVcbjbi/HLmyQigEkzFoHfZdeqKOvw==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-arm64/-/claude-agent-sdk-linux-arm64-0.2.136.tgz", + "integrity": "sha512-u3m5O35xBDX5zkAyt0hYl0E6hPOr6oviFD00LzHtVvyu7yXFBP/Aaq1u5CGgf/dmsSYqXg6dWJhNcVepIcphUQ==", "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -166,15 +163,12 @@ ] }, "node_modules/@anthropic-ai/claude-agent-sdk-linux-arm64-musl": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-arm64-musl/-/claude-agent-sdk-linux-arm64-musl-0.2.131.tgz", - "integrity": "sha512-7efL5otHqTKMeNxIztEjEGs8ktlR3hfMmVbo1HaEbs+tkJ6fvMwS3k4xnUP7Bqy+GsM+U9r9kRdNz4MVdc80hg==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-arm64-musl/-/claude-agent-sdk-linux-arm64-musl-0.2.136.tgz", + "integrity": "sha512-djd8U2UfUVzCXRqLzd3YQFnCNe+csXPGznUjlQpR3lbLkc+M9apZmTkFANFgFhK9dG/AZdHSSyLM1GVS96dm0g==", "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -182,15 +176,12 @@ ] }, "node_modules/@anthropic-ai/claude-agent-sdk-linux-x64": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-x64/-/claude-agent-sdk-linux-x64-0.2.131.tgz", - "integrity": "sha512-tJJggvCGtkK876CowajF/42AdUy0TTJk0gHeCKuDCMJF3hMs70EtYnwyM81nb10tKUFb6zYdvn6iPn6iGx7iFQ==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-x64/-/claude-agent-sdk-linux-x64-0.2.136.tgz", + "integrity": "sha512-jSjVsZT+o96pR7R2JPRMWHp3NSBmHsVwTLtjibPS8LFSLLLGfbcSWfX6Q5m+4H78fsKYnLTTYk/tXI6r8Q1GgQ==", "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -198,15 +189,12 @@ ] }, "node_modules/@anthropic-ai/claude-agent-sdk-linux-x64-musl": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-x64-musl/-/claude-agent-sdk-linux-x64-musl-0.2.131.tgz", - "integrity": "sha512-WNqUJscB1F86Igbnw5zXpndT89I7l3aIvPJQEOrSA5JaIDmfJft8QA1rrJPwf2tcxP8nNS0H3MbEFBAxq92bNw==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-x64-musl/-/claude-agent-sdk-linux-x64-musl-0.2.136.tgz", + "integrity": "sha512-2yb+jc8OVmmMvIuBnvfu5JcnkbfuBG+Se0ePLRr0JaNPJ5DJf0ZR86drpTnjANPcAryk5cm9xtzfOliNqLTIcA==", "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -214,9 +202,9 @@ ] }, "node_modules/@anthropic-ai/claude-agent-sdk-win32-arm64": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-win32-arm64/-/claude-agent-sdk-win32-arm64-0.2.131.tgz", - "integrity": "sha512-LDXYMqR3T1JtaIusmVDr6e539IhE+IULKYBiLC7+v7VvLG6niP1cC+4W/zYZRnUcbzUgcfoIi1FvrWhtF6/M+A==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-win32-arm64/-/claude-agent-sdk-win32-arm64-0.2.136.tgz", + "integrity": "sha512-TWSeel2tr+VxA3S8PlBrwN/MYU93VZM+G/qu+mriloOFXJv3fzbycDAbUf8WnvGPrAoAbKbej11vo3mmtPiDyw==", "cpu": [ "arm64" ], @@ -227,9 +215,9 @@ ] }, "node_modules/@anthropic-ai/claude-agent-sdk-win32-x64": { - "version": "0.2.131", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-win32-x64/-/claude-agent-sdk-win32-x64-0.2.131.tgz", - "integrity": "sha512-gwLUkQWtK9Un2i9mWWQgoaEk+2rzamiH3r4j7aoTyVzB4ZQgxdBBOP9ac5o9pIwQE+vflr0HvKk1O54Z320Vng==", + "version": "0.2.136", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-win32-x64/-/claude-agent-sdk-win32-x64-0.2.136.tgz", + "integrity": "sha512-2xs5a7T58bEMxcCu3cDO2yKYQdhNJDjqJc479PXjL7CqQb3g9VnDf+SXsnWTbeLwCGqxSOVulhCBkC7T9nYTrg==", "cpu": [ "x64" ], @@ -6888,12 +6876,12 @@ } }, "node_modules/express-rate-limit": { - "version": "8.5.0", - "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.5.0.tgz", - "integrity": "sha512-XKhFohWaSBdVJNTi5TaHziqnPkv04I9UQV6q1Wy7Ui6GGQZVW12ojDFwqer14EvCXxjvPG0CyWXx7cAXpALB4Q==", + "version": "8.5.1", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.5.1.tgz", + "integrity": "sha512-5O6KYmyJEpuPJV5hNTXKbAHWRqrzyu+OI3vUnSd2kXFubIVpG7ezpgxQy76Zo5GQZtrQBg86hF+CM/NX+cioiQ==", "license": "MIT", "dependencies": { - "ip-address": "10.1.0" + "ip-address": "^10.2.0" }, "engines": { "node": ">= 16" @@ -6957,7 +6945,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.0", + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", + "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", "funding": [ { "type": "github", @@ -7435,9 +7425,9 @@ } }, "node_modules/hono": { - "version": "4.12.14", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.14.tgz", - "integrity": "sha512-am5zfg3yu6sqn5yjKBNqhnTX7Cv+m00ox+7jbaKkrLMRJ4rAdldd1xPd/JzbBWspqaQv6RSTrgFN95EsfhC+7w==", + "version": "4.12.18", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.18.tgz", + "integrity": "sha512-RWzP96k/yv0PQfyXnWjs6zot20TqfpfsNXhOnev8d1InAxubW93L11/oNUc3tQqn2G0bSdAOBpX+2uDFHV7kdQ==", "license": "MIT", "engines": { "node": ">=16.9.0" @@ -7650,9 +7640,9 @@ } }, "node_modules/ip-address": { - "version": "10.1.0", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.1.0.tgz", - "integrity": "sha512-XXADHxXmvT9+CRxhXg56LJovE+bmWnEWB78LB83VZTprKTmaC5QfruXocxzTZ2Kl0DNwKuBdlIhjL8LeY8Sf8Q==", + "version": "10.2.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.2.0.tgz", + "integrity": "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA==", "license": "MIT", "engines": { "node": ">= 12" diff --git a/src/cli/session/finish.ts b/src/cli/session/finish.ts index ac90131a2..47de552ee 100644 --- a/src/cli/session/finish.ts +++ b/src/cli/session/finish.ts @@ -1,4 +1,4 @@ -import { Command, Flags } from '@oclif/core'; +import { Flags } from '@oclif/core'; import { readCompletionEvidence } from '../../backends/completion.js'; import { validateFinish, writePushedChangesSidecar } from '../../gadgets/session/core/finish.js'; import { finishDef } from '../../gadgets/session/definitions.js'; @@ -8,6 +8,7 @@ import { PUSHED_CHANGES_SIDECAR_ENV_VAR, REVIEW_SIDECAR_ENV_VAR, } from '../../gadgets/sessionState.js'; +import { CredentialScopedCommand } from '../base.js'; function readFinishHooksFromEnv(): SessionHooks { const raw = process.env.CASCADE_FINISH_HOOKS; @@ -20,7 +21,7 @@ function readFinishHooksFromEnv(): SessionHooks { } } -export default class Finish extends Command { +export default class Finish extends CredentialScopedCommand { static override description = finishDef.description; static override flags = { @@ -39,7 +40,7 @@ export default class Finish extends Command { }), }; - async run(): Promise { + async execute(): Promise { const { flags } = await this.parse(Finish); const hooks = readFinishHooksFromEnv(); const evidence = readCompletionEvidence({ diff --git a/src/triggers/README.md b/src/triggers/README.md index 78a6813bc..ed377826f 100644 --- a/src/triggers/README.md +++ b/src/triggers/README.md @@ -77,6 +77,8 @@ To reduce duplication across the three worker-side handlers, shared utilities ar | `trigger-resolution.ts` | `resolveTriggerResult()` — pre-resolved or dispatch | Sentry (GitHub and PM use inline logic) | | `credential-scope.ts` | `withPMScope()` — `withPMCredentials` + `withPMProvider` | GitHub, Sentry | | `pm-ack.ts` | `postPMAckComment()` — posts ack to Trello/JIRA | GitHub worker handler | +| `events.ts` | `TRIGGER_EVENTS` — typed catalog of canonical trigger event names | Trigger handlers and tests | +| `result-builders.ts` | Builders for dispatch, skip, no-agent, and deferred re-check `TriggerResult` shapes | Trigger handlers and tests | | `agent-execution.ts` | `runAgentExecutionPipeline()` — full agent lifecycle | All handlers (via `webhook-execution.ts`) | | `webhook-execution.ts` | `runAgentWithCredentials()` — LLM keys + credentials + pipeline | GitHub, PM | diff --git a/src/triggers/shared/agent-execution-lifecycle.ts b/src/triggers/shared/agent-execution-lifecycle.ts new file mode 100644 index 000000000..930742115 --- /dev/null +++ b/src/triggers/shared/agent-execution-lifecycle.ts @@ -0,0 +1,159 @@ +import type { LifecycleHooks } from '../../agents/definitions/schema.js'; +import type { PMLifecycleManager } from '../../pm/index.js'; +import type { AgentResult, ProjectConfig } from '../../types/index.js'; +import { logger } from '../../utils/logging.js'; +import type { TriggerResult } from '../types.js'; +import type { AgentExecutionConfig, AgentExecutionContext } from './agent-execution-types.js'; +import { handleAgentResultArtifacts } from './agent-result-handler.js'; +import { checkBudgetExceeded } from './budget.js'; +import { + formatValidationErrors, + type ValidationResult, + validateIntegrations, +} from './integration-validation.js'; + +interface ValidationLifecycleParams { + result: TriggerResult; + project: ProjectConfig; + executionConfig: AgentExecutionConfig; + agentType: string; + lifecycle: PMLifecycleManager; +} + +/** + * Notify PM and source-specific callbacks when integration validation fails before the agent runs. + */ +async function notifyValidationFailure( + result: TriggerResult, + validation: ValidationResult, + lifecycle: PMLifecycleManager, + executionConfig: AgentExecutionConfig, + agentType: string, + projectId: string, +): Promise { + const errorMessage = formatValidationErrors(validation); + logger.error('Integration validation failed', { + agentType, + projectId, + errors: validation.errors, + }); + + // Only notify via PM if PM validation passed (otherwise PM isn't configured) + const pmFailed = validation.errors.some((e) => e.category === 'pm'); + if (result.workItemId && !pmFailed) { + await lifecycle.handleFailure(result.workItemId, errorMessage); + } + + // Call onFailure callback (for GitHub PR updates) + if (executionConfig.onFailure) { + await executionConfig.onFailure(result, { success: false, output: '', error: errorMessage }); + } +} + +/** + * Run pre-flight integration validation and apply the existing failure notification semantics. + * Returns false when execution should stop before preparing or running the agent. + */ +export async function validateAgentExecutionLifecycle({ + result, + project, + executionConfig, + agentType, + lifecycle, +}: ValidationLifecycleParams): Promise { + const validation = await validateIntegrations(project.id, agentType, project); + if (validation.valid) return true; + + await notifyValidationFailure( + result, + validation, + lifecycle, + executionConfig, + agentType, + project.id, + ); + return false; +} + +/** + * Check the budget before running an agent. + * Returns the remaining budget if not exceeded, or abort=true when the agent + * must not start and the lifecycle manager has been notified. + */ +export async function checkPreRunBudget( + workItemId: string, + project: ProjectConfig, + lifecycle: PMLifecycleManager, +): Promise<{ remainingBudgetUsd: number | undefined; abort: boolean }> { + const budgetCheck = await checkBudgetExceeded(workItemId, project); + if (budgetCheck?.exceeded) { + logger.warn('Budget exceeded, agent not started', { + workItemId, + currentCost: budgetCheck.currentCost, + budget: budgetCheck.budget, + }); + await lifecycle.handleBudgetExceeded(workItemId, budgetCheck.currentCost, budgetCheck.budget); + return { remainingBudgetUsd: undefined, abort: true }; + } + return { remainingBudgetUsd: budgetCheck?.remaining, abort: false }; +} + +/** + * Run pre-agent lifecycle steps owned by the PM lifecycle manager. + */ +export async function prepareAgentExecutionLifecycle( + context: AgentExecutionContext, +): Promise { + if (context.workItemId && !context.executionConfig.skipPrepareForAgent) { + await context.lifecycle.prepareForAgent(context.workItemId, context.lifecycleHooks); + } +} + +/** + * Run post-agent lifecycle steps: artifact handling, budget warning, cleanup, success/failure. + */ +export async function runPostAgentExecutionLifecycle( + workItemId: string, + agentType: string, + agentResult: AgentResult, + project: ProjectConfig, + lifecycle: PMLifecycleManager, + lifecycleHooks: LifecycleHooks, + executionConfig: AgentExecutionConfig, +): Promise { + const { + skipPrepareForAgent = false, + skipHandleFailure = false, + handleSuccessOnlyForAgentType, + } = executionConfig; + + await handleAgentResultArtifacts(workItemId, agentType, agentResult, project); + + const postBudgetCheck = await checkBudgetExceeded(workItemId, project); + if (postBudgetCheck?.exceeded) { + await lifecycle.handleBudgetWarning( + workItemId, + postBudgetCheck.currentCost, + postBudgetCheck.budget, + ); + } + + if (!skipPrepareForAgent) { + await lifecycle.cleanupProcessing(workItemId); + } + + const shouldCallHandleSuccess = + agentResult.success && + (!handleSuccessOnlyForAgentType || agentType === handleSuccessOnlyForAgentType); + + if (shouldCallHandleSuccess) { + await lifecycle.handleSuccess( + workItemId, + lifecycleHooks, + agentResult.prUrl, + agentResult.progressCommentId, + ); + } else if (!agentResult.success && !skipHandleFailure) { + await lifecycle.handleFailure(workItemId, agentResult.error); + } +} diff --git a/src/triggers/shared/agent-execution.ts b/src/triggers/shared/agent-execution.ts index 1f09a7d89..d853c6f84 100644 --- a/src/triggers/shared/agent-execution.ts +++ b/src/triggers/shared/agent-execution.ts @@ -1,8 +1,6 @@ import { getAgentProfile } from '../../agents/definitions/profiles.js'; import type { LifecycleHooks } from '../../agents/definitions/schema.js'; import { runAgent } from '../../agents/registry.js'; -import { createWorkItem, linkPRToWorkItem } from '../../db/repositories/prWorkItemsRepository.js'; -import { updateRunPRNumber } from '../../db/repositories/runsRepository.js'; import { getPMProvider } from '../../pm/context.js'; import { createPMProvider, @@ -20,180 +18,25 @@ import { logger } from '../../utils/logging.js'; import { extractPRNumber } from '../../utils/prUrl.js'; import { parseRepoFullName } from '../../utils/repo.js'; import type { TriggerResult } from '../types.js'; +import { + checkPreRunBudget, + prepareAgentExecutionLifecycle, + runPostAgentExecutionLifecycle, + validateAgentExecutionLifecycle, +} from './agent-execution-lifecycle.js'; import type { AgentExecutionConfig, AgentExecutionContext } from './agent-execution-types.js'; -import { handleAgentResultArtifacts } from './agent-result-handler.js'; +import { + linkPRPostExecution, + persistPreRunWorkItems, + prepareAgentWorkItem, + resolveWorkItemId, +} from './agent-work-items.js'; import { isPipelineAtCapacity } from './backlog-check.js'; -import { checkBudgetExceeded } from './budget.js'; import { triggerDebugAnalysis } from './debug-runner.js'; import { shouldTriggerDebug } from './debug-trigger.js'; -import { - formatValidationErrors, - type ValidationResult, - validateIntegrations, -} from './integration-validation.js'; export type { AgentExecutionConfig } from './agent-execution-types.js'; -/** - * Check the budget before running an agent. - * Returns the remaining budget if not exceeded, or null to signal the caller - * should abort (budget exceeded and lifecycle notified). - */ -async function checkPreRunBudget( - workItemId: string, - project: ProjectConfig, - lifecycle: PMLifecycleManager, -): Promise<{ remainingBudgetUsd: number | undefined; abort: boolean }> { - const budgetCheck = await checkBudgetExceeded(workItemId, project); - if (budgetCheck?.exceeded) { - logger.warn('Budget exceeded, agent not started', { - workItemId, - currentCost: budgetCheck.currentCost, - budget: budgetCheck.budget, - }); - await lifecycle.handleBudgetExceeded(workItemId, budgetCheck.currentCost, budgetCheck.budget); - return { remainingBudgetUsd: undefined, abort: true }; - } - return { remainingBudgetUsd: budgetCheck?.remaining, abort: false }; -} - -/** - * Run post-agent lifecycle steps: artifact handling, budget warning, cleanup, success/failure. - */ -async function runPostAgentLifecycle( - workItemId: string, - agentType: string, - agentResult: AgentResult, - project: ProjectConfig, - lifecycle: PMLifecycleManager, - lifecycleHooks: LifecycleHooks, - executionConfig: AgentExecutionConfig, -): Promise { - const { - skipPrepareForAgent = false, - skipHandleFailure = false, - handleSuccessOnlyForAgentType, - } = executionConfig; - - await handleAgentResultArtifacts(workItemId, agentType, agentResult, project); - - const postBudgetCheck = await checkBudgetExceeded(workItemId, project); - if (postBudgetCheck?.exceeded) { - await lifecycle.handleBudgetWarning( - workItemId, - postBudgetCheck.currentCost, - postBudgetCheck.budget, - ); - } - - if (!skipPrepareForAgent) { - await lifecycle.cleanupProcessing(workItemId); - } - - const shouldCallHandleSuccess = - agentResult.success && - (!handleSuccessOnlyForAgentType || agentType === handleSuccessOnlyForAgentType); - - if (shouldCallHandleSuccess) { - await lifecycle.handleSuccess( - workItemId, - lifecycleHooks, - agentResult.prUrl, - agentResult.progressCommentId, - ); - } else if (!agentResult.success && !skipHandleFailure) { - await lifecycle.handleFailure(workItemId, agentResult.error); - } -} - -/** - * Notify PM and GitHub when integration validation fails before the agent runs. - */ -async function notifyValidationFailure( - result: TriggerResult, - validation: ValidationResult, - lifecycle: PMLifecycleManager, - executionConfig: AgentExecutionConfig, - agentType: string, - projectId: string, -): Promise { - const errorMessage = formatValidationErrors(validation); - logger.error('Integration validation failed', { - agentType, - projectId, - errors: validation.errors, - }); - - // Only notify via PM if PM validation passed (otherwise PM isn't configured) - const pmFailed = validation.errors.some((e) => e.category === 'pm'); - if (result.workItemId && !pmFailed) { - await lifecycle.handleFailure(result.workItemId, errorMessage); - } - - // Call onFailure callback (for GitHub PR updates) - if (executionConfig.onFailure) { - await executionConfig.onFailure(result, { success: false, output: '', error: errorMessage }); - } -} - -/** - * Link a PR to a work item and backfill the run's prNumber after agent execution. - * Extracted to reduce cognitive complexity in the main pipeline function. - */ -async function linkPRPostExecution( - agentResult: AgentResult & { prUrl: string }, - project: ProjectConfig & { repo: string }, - result: TriggerResult, - workItemId: string | undefined, -): Promise { - const prNumber = extractPRNumber(agentResult.prUrl); - if (!prNumber) return; - - // Fetch PR title from GitHub API (best-effort, resolves the prTitle gap) - let prTitle: string | undefined; - try { - const { githubClient } = await import('../../github/client.js'); - const { parseRepoFullName } = await import('../../utils/repo.js'); - const { owner, repo } = parseRepoFullName(project.repo); - const pr = await githubClient.getPR(owner, repo, prNumber); - prTitle = pr.title; - } catch (err) { - logger.warn('Failed to fetch PR title from GitHub', { - projectId: project.id, - prNumber, - error: String(err), - }); - } - - try { - await linkPRToWorkItem(project.id, project.repo, prNumber, workItemId ?? null, { - prUrl: agentResult.prUrl, - prTitle, - workItemUrl: result.workItemUrl, - workItemTitle: result.workItemTitle, - }); - } catch (err) { - logger.warn('Failed to link PR to work item post-execution', { - projectId: project.id, - prNumber, - workItemId, - error: String(err), - }); - } - - if (agentResult.runId) { - try { - await updateRunPRNumber(agentResult.runId, prNumber); - } catch (err) { - logger.warn('Failed to backfill prNumber on run', { - runId: agentResult.runId, - prNumber, - error: String(err), - }); - } - } -} - /** * Dispatch a review agent after a successful implementation run, if the PR's * CI is green and no review has been dispatched yet. @@ -366,24 +209,6 @@ async function postAgentSummaryToPM( } } -/** - * Resolve a work item ID: prefer the one from TriggerResult, fall back to DB lookup by PR number. - */ -async function resolveWorkItemId( - workItemId: string | undefined, - projectId: string, - prNumber: number | undefined, -): Promise { - if (workItemId) return workItemId; - if (!prNumber || !projectId) return undefined; - try { - const { lookupWorkItemForPR } = await import('../../db/repositories/prWorkItemsRepository.js'); - return (await lookupWorkItemForPR(projectId, prNumber)) ?? undefined; - } catch { - return undefined; // best-effort - } -} - /** * Shared agent execution pipeline. * @@ -434,21 +259,18 @@ export async function runAgentExecutionPipeline( }); } - // Pre-flight integration validation - const validation = await validateIntegrations(project.id, agentType, project); - if (!validation.valid) { - await notifyValidationFailure( - result, - validation, - lifecycle, - executionConfig, - agentType, - project.id, - ); + const canExecute = await validateAgentExecutionLifecycle({ + result, + project, + agentType, + lifecycle, + executionConfig, + }); + if (!canExecute) { return; } - const { skipPrepareForAgent = false, onSuccess, onFailure, logLabel = 'Agent' } = executionConfig; + const { onSuccess, onFailure, logLabel = 'Agent' } = executionConfig; // Re-resolve workItemId at run time. The trigger handler (e.g. PROpenedTrigger) // captures workItemId synchronously at webhook arrival, before any other @@ -456,7 +278,7 @@ export async function runAgentExecutionPipeline( // caught up — preferring the live value avoids carrying a stale `undefined` // into runAgent (and therefore agent_runs.work_item_id) and into the // post-execution linkPRToWorkItem write. - const workItemId = await resolveWorkItemId(result.workItemId, project.id, result.prNumber); + const { workItemId, agentInput } = await prepareAgentWorkItem(result, project.id); // Patch agentInput.workItemId whenever it diverges from the resolved value. // Two cases this catches: @@ -471,11 +293,6 @@ export async function runAgentExecutionPipeline( // catches this at write-time; this runtime patch is the safety net. // tryCreateRun (src/agents/shared/runTracking.ts) reads workItemId from // agentInput when persisting agent_runs.work_item_id. - const agentInput = - workItemId && (result.agentInput.workItemId as string | undefined) !== workItemId - ? { ...result.agentInput, workItemId } - : result.agentInput; - const executionContext: AgentExecutionContext = { result, project, @@ -500,51 +317,8 @@ export async function runAgentExecutionPipeline( remainingBudgetUsd = budgetResult.remainingBudgetUsd; } - // Insert a work-item-only row so PM-triggered runs show up in the dashboard - // even before a PR is created. This is idempotent — if a row already exists - // it is updated with the latest display fields. - if (executionContext.workItemId) { - try { - await createWorkItem(executionContext.project.id, executionContext.workItemId, { - workItemUrl: executionContext.result.workItemUrl, - workItemTitle: executionContext.result.workItemTitle, - }); - } catch (err) { - logger.warn('Failed to persist work-item row for PM-triggered run', { - projectId: executionContext.project.id, - workItemId: executionContext.workItemId, - error: String(err), - }); - } - } - - // Ensure a pr_work_items entry exists for PR-triggered runs (SCM webhooks). - // This covers cases where no workItemId exists (orphan PRs) — linkPRToWorkItem - // handles upsert, promotion, and orphan creation. - if (result.prNumber && project.repo) { - try { - await linkPRToWorkItem(project.id, project.repo, result.prNumber, workItemId ?? null, { - workItemUrl: result.workItemUrl, - workItemTitle: result.workItemTitle, - prUrl: result.prUrl, - prTitle: result.prTitle, - }); - } catch (err) { - logger.warn('Failed to ensure pr_work_items entry for PR-triggered run', { - projectId: project.id, - prNumber: result.prNumber, - workItemId, - error: String(err), - }); - } - } - - if (executionContext.workItemId && !skipPrepareForAgent) { - await executionContext.lifecycle.prepareForAgent( - executionContext.workItemId, - executionContext.lifecycleHooks, - ); - } + await persistPreRunWorkItems(result, project, workItemId); + await prepareAgentExecutionLifecycle(executionContext); const agentResult = await runAgent(executionContext.agentType, { ...executionContext.agentInput, @@ -567,7 +341,7 @@ export async function runAgentExecutionPipeline( await postAgentSummaryToPM(agentType, agentResult, workItemId, project.id, result.prNumber); if (workItemId) { - await runPostAgentLifecycle( + await runPostAgentExecutionLifecycle( workItemId, agentType, agentResult, diff --git a/src/triggers/shared/agent-work-items.ts b/src/triggers/shared/agent-work-items.ts new file mode 100644 index 000000000..c14b6209c --- /dev/null +++ b/src/triggers/shared/agent-work-items.ts @@ -0,0 +1,137 @@ +import { createWorkItem, linkPRToWorkItem } from '../../db/repositories/prWorkItemsRepository.js'; +import { updateRunPRNumber } from '../../db/repositories/runsRepository.js'; +import type { AgentInput, AgentResult, ProjectConfig } from '../../types/index.js'; +import { logger } from '../../utils/logging.js'; +import { extractPRNumber } from '../../utils/prUrl.js'; +import { parseRepoFullName } from '../../utils/repo.js'; +import type { TriggerResult } from '../types.js'; + +export interface ResolvedAgentWorkItem { + workItemId: string | undefined; + agentInput: AgentInput; +} + +export async function resolveWorkItemId( + workItemId: string | undefined, + projectId: string, + prNumber: number | undefined, +): Promise { + if (workItemId) return workItemId; + if (!prNumber) return undefined; + + try { + const { lookupWorkItemForPR } = await import('../../db/repositories/prWorkItemsRepository.js'); + return (await lookupWorkItemForPR(projectId, prNumber)) ?? undefined; + } catch (err) { + logger.warn('Failed to resolve workItemId for PR', { + projectId, + prNumber, + error: String(err), + }); + return undefined; + } +} + +export async function prepareAgentWorkItem( + result: TriggerResult, + projectId: string, +): Promise { + const workItemId = await resolveWorkItemId(result.workItemId, projectId, result.prNumber); + const agentInput = + workItemId && result.agentInput.workItemId !== workItemId + ? { ...result.agentInput, workItemId } + : result.agentInput; + + return { workItemId, agentInput }; +} + +export async function persistPreRunWorkItems( + result: TriggerResult, + project: ProjectConfig, + workItemId: string | undefined, +): Promise { + if (workItemId) { + try { + await createWorkItem(project.id, workItemId, { + workItemUrl: result.workItemUrl, + workItemTitle: result.workItemTitle, + }); + } catch (err) { + logger.warn('Failed to persist work-item row for PM-triggered run', { + projectId: project.id, + workItemId, + error: String(err), + }); + } + } + + if (result.prNumber && project.repo) { + try { + await linkPRToWorkItem(project.id, project.repo, result.prNumber, workItemId ?? null, { + workItemUrl: result.workItemUrl, + workItemTitle: result.workItemTitle, + prUrl: result.prUrl, + prTitle: result.prTitle, + }); + } catch (err) { + logger.warn('Failed to ensure pr_work_items entry for PR-triggered run', { + projectId: project.id, + prNumber: result.prNumber, + workItemId, + error: String(err), + }); + } + } +} + +export async function linkPRPostExecution( + agentResult: AgentResult & { prUrl: string }, + project: ProjectConfig & { repo: string }, + result: TriggerResult, + workItemId: string | undefined, +): Promise { + const prNumber = extractPRNumber(agentResult.prUrl); + if (!prNumber) return; + + let prTitle: string | undefined; + try { + const { githubClient } = await import('../../github/client.js'); + const { owner, repo } = parseRepoFullName(project.repo); + const pr = await githubClient.getPR(owner, repo, prNumber); + prTitle = pr.title; + } catch (err) { + logger.warn('Failed to fetch PR title from GitHub', { + projectId: project.id, + prNumber, + error: String(err), + }); + } + + try { + await linkPRToWorkItem(project.id, project.repo, prNumber, workItemId ?? null, { + prUrl: agentResult.prUrl, + prTitle, + workItemUrl: result.workItemUrl, + workItemTitle: result.workItemTitle, + }); + } catch (err) { + logger.warn('Failed to link PR to work item post-execution', { + projectId: project.id, + prNumber, + workItemId, + error: String(err), + }); + } + + if (agentResult.runId) { + try { + await updateRunPRNumber(agentResult.runId, prNumber); + } catch (err) { + logger.warn('Failed to backfill prNumber on run', { + runId: agentResult.runId, + prNumber, + error: String(err), + }); + } + } +} diff --git a/src/triggers/shared/events.ts b/src/triggers/shared/events.ts new file mode 100644 index 000000000..f2d421ac3 --- /dev/null +++ b/src/triggers/shared/events.ts @@ -0,0 +1,45 @@ +export const TRIGGER_EVENTS = { + PM: { + STATUS_CHANGED: 'pm:status-changed', + LABEL_ADDED: 'pm:label-added', + COMMENT_MENTION: 'pm:comment-mention', + }, + SCM: { + CHECK_SUITE_SUCCESS: 'scm:check-suite-success', + CHECK_SUITE_FAILURE: 'scm:check-suite-failure', + PR_REVIEW_SUBMITTED: 'scm:pr-review-submitted', + REVIEW_REQUESTED: 'scm:review-requested', + PR_OPENED: 'scm:pr-opened', + PR_COMMENT_MENTION: 'scm:pr-comment-mention', + PR_MERGED: 'scm:pr-merged', + PR_READY_TO_MERGE: 'scm:pr-ready-to-merge', + PR_CONFLICT_DETECTED: 'scm:pr-conflict-detected', + }, + ALERTING: { + ISSUE_ALERT: 'alerting:issue-alert', + METRIC_ALERT: 'alerting:metric-alert', + }, + INTERNAL: { + AUTO_CHAIN: 'internal:auto-chain', + }, +} as const; + +type ObjectValues = T[keyof T]; + +export type PMTriggerEvent = ObjectValues; +export type SCMTriggerEvent = ObjectValues; +export type AlertingTriggerEvent = ObjectValues; +export type InternalTriggerEvent = ObjectValues; + +export type CanonicalTriggerEvent = + | PMTriggerEvent + | SCMTriggerEvent + | AlertingTriggerEvent + | InternalTriggerEvent; + +export const ALL_TRIGGER_EVENTS = [ + ...Object.values(TRIGGER_EVENTS.PM), + ...Object.values(TRIGGER_EVENTS.SCM), + ...Object.values(TRIGGER_EVENTS.ALERTING), + ...Object.values(TRIGGER_EVENTS.INTERNAL), +] as CanonicalTriggerEvent[]; diff --git a/src/triggers/shared/result-builders.ts b/src/triggers/shared/result-builders.ts new file mode 100644 index 000000000..2ded64be9 --- /dev/null +++ b/src/triggers/shared/result-builders.ts @@ -0,0 +1,144 @@ +import type { AgentInput, TriggerResult } from '../../types/index.js'; +import type { CanonicalTriggerEvent, PMTriggerEvent, SCMTriggerEvent } from './events.js'; + +type AgentType = Exclude; + +interface BaseDispatchOptions { + agentType: AgentType; + agentInput?: AgentInput; + workItemId?: string; + workItemUrl?: string; + workItemTitle?: string; + onBlocked?: TriggerResult['onBlocked']; + coalesceKey?: string; +} + +interface PMDispatchOptions extends BaseDispatchOptions { + triggerEvent: PMTriggerEvent; + workItemId: string; +} + +interface GitHubPRDispatchOptions extends BaseDispatchOptions { + triggerEvent: SCMTriggerEvent; + prNumber: number; + prUrl?: string; + prTitle?: string; +} + +interface NoAgentOptions { + agentInput?: AgentInput; + workItemId?: string; + workItemUrl?: string; + workItemTitle?: string; + prNumber?: number; + prUrl?: string; + prTitle?: string; + lockKey?: string; + coalesceKey?: string; +} + +interface DeferredRecheckOptions { + delayMs: number; + coalesceKey: string; + agentInput?: AgentInput; +} + +function buildAgentInput( + agentInput: AgentInput | undefined, + triggerEvent: CanonicalTriggerEvent, + workItemId: string | undefined, + prNumber?: number, +): AgentInput { + return { + ...agentInput, + triggerEvent, + ...(workItemId ? { workItemId } : {}), + ...(prNumber !== undefined ? { prNumber } : {}), + }; +} + +export function buildPMDispatchResult(options: PMDispatchOptions): TriggerResult { + const { + agentType, + agentInput, + triggerEvent, + workItemId, + workItemUrl, + workItemTitle, + onBlocked, + coalesceKey, + } = options; + + return { + agentType, + agentInput: buildAgentInput(agentInput, triggerEvent, workItemId), + workItemId, + workItemUrl, + workItemTitle, + onBlocked, + coalesceKey, + }; +} + +export function buildGitHubPRDispatchResult(options: GitHubPRDispatchOptions): TriggerResult { + const { + agentType, + agentInput, + triggerEvent, + workItemId, + workItemUrl, + workItemTitle, + prNumber, + prUrl, + prTitle, + onBlocked, + coalesceKey, + } = options; + + return { + agentType, + agentInput: buildAgentInput(agentInput, triggerEvent, workItemId, prNumber), + prNumber, + prUrl, + prTitle, + workItemId, + workItemUrl, + workItemTitle, + onBlocked, + coalesceKey, + }; +} + +export function buildSkipResult(handler: string, message: string): TriggerResult { + return { + agentType: null, + agentInput: {}, + skipReason: { handler, message }, + }; +} + +export function buildNoAgentResult(options: NoAgentOptions = {}): TriggerResult { + return { + agentType: null, + agentInput: options.agentInput ?? {}, + ...(options.workItemId ? { workItemId: options.workItemId } : {}), + ...(options.workItemUrl ? { workItemUrl: options.workItemUrl } : {}), + ...(options.workItemTitle ? { workItemTitle: options.workItemTitle } : {}), + ...(options.prNumber ? { prNumber: options.prNumber } : {}), + ...(options.prUrl ? { prUrl: options.prUrl } : {}), + ...(options.prTitle ? { prTitle: options.prTitle } : {}), + ...(options.lockKey ? { lockKey: options.lockKey } : {}), + ...(options.coalesceKey ? { coalesceKey: options.coalesceKey } : {}), + }; +} + +export function buildDeferredRecheckResult(options: DeferredRecheckOptions): TriggerResult { + return { + agentType: null, + agentInput: options.agentInput ?? {}, + deferredRecheck: { + delayMs: options.delayMs, + coalesceKey: options.coalesceKey, + }, + }; +} diff --git a/src/triggers/shared/skip.ts b/src/triggers/shared/skip.ts index 34f2442ef..46205eb82 100644 --- a/src/triggers/shared/skip.ts +++ b/src/triggers/shared/skip.ts @@ -1,4 +1,5 @@ import type { TriggerResult } from '../../types/index.js'; +import { buildSkipResult } from './result-builders.js'; /** * Build a structured self-skip result so the router's webhook log @@ -13,9 +14,5 @@ import type { TriggerResult } from '../../types/index.js'; * `import { skip } from '../shared/skip.js'` and one call. */ export function skip(handler: string, message: string): TriggerResult { - return { - agentType: null, - agentInput: {}, - skipReason: { handler, message }, - }; + return buildSkipResult(handler, message); } diff --git a/src/types/index.ts b/src/types/index.ts index b3e0ddcdf..2b9c07537 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -1,6 +1,7 @@ import type { z } from 'zod'; import type { CascadeConfigSchema, ProjectConfigSchema } from '../config/schema.js'; import type { PersonaIdentities } from '../github/personas.js'; +import type { CanonicalTriggerEvent } from '../triggers/shared/events.js'; export type ProjectConfig = z.infer; export type CascadeConfig = z.infer; @@ -29,7 +30,7 @@ export interface AgentInput { | 'manual'; /** YAML-format trigger event name for context pipeline resolution (e.g. 'scm:check-suite-success') */ - triggerEvent?: string; + triggerEvent?: CanonicalTriggerEvent | (string & {}); // Debug agent fields logDir?: string; diff --git a/tests/unit/cli/session/finish-credential-scope.test.ts b/tests/unit/cli/session/finish-credential-scope.test.ts new file mode 100644 index 000000000..c464ff230 --- /dev/null +++ b/tests/unit/cli/session/finish-credential-scope.test.ts @@ -0,0 +1,126 @@ +/** + * Regression net for cascade run `06ec8a0a` (Trello d1KVMufl, PR #1274): the agent + * created the PR but the run was marked failed because `cascade-tools session finish` + * extended raw oclif `Command` and skipped `CredentialScopedCommand.run()`'s + * `withGitHubToken` scope. The fallback `findPRForCurrentBranch()` then hit + * `getClient()` against empty AsyncLocalStorage, threw, got silently swallowed, + * and reported "Cannot finish session without creating a PR" despite the PR existing. + * + * Critical: do NOT mock `src/github/client.js` here — that would short-circuit the + * exact code path the bug lived in. Only mock at the `@octokit/rest` boundary. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const pullsList = vi.fn(); +const octokitFactory = vi.fn(() => ({ pulls: { list: pullsList } })); +vi.mock('@octokit/rest', () => ({ + Octokit: vi.fn().mockImplementation((...args: unknown[]) => octokitFactory(...args)), +})); + +const mockExecSync = vi.fn(); +const mockExecFileSync = vi.fn(); +vi.mock('node:child_process', () => ({ + execSync: (...args: unknown[]) => mockExecSync(...args), + execFileSync: (...args: unknown[]) => mockExecFileSync(...args), +})); + +// PM scope is irrelevant to this test — short-circuit it so we don't need to +// bootstrap the manifest registry. Other PM credential scopes are similarly +// unused: only GITHUB_TOKEN is set in env, so withTrello/Jira/Linear are no-ops. +vi.mock('../../../../src/pm/index.js', () => ({ + createPMProvider: vi.fn().mockReturnValue({}), + withPMProvider: vi.fn((_p: unknown, fn: () => Promise) => fn()), +})); + +import FinishCommand from '../../../../src/cli/session/finish.js'; + +describe('session finish CLI — GitHub credential scope', () => { + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + process.env.CASCADE_FINISH_HOOKS = JSON.stringify({ requiresPR: true }); + process.env.GITHUB_TOKEN = 'ghp_test_token'; + process.env.CASCADE_AGENT_TYPE = 'implementation'; + delete process.env.CASCADE_PR_SIDECAR_PATH; + delete process.env.CASCADE_REVIEW_SIDECAR_PATH; + delete process.env.CASCADE_PUSHED_CHANGES_SIDECAR_PATH; + delete process.env.CASCADE_PR_BRANCH; + delete process.env.CASCADE_INITIAL_HEAD_SHA; + + mockExecSync.mockImplementation((cmd: string) => { + if (cmd === 'git rev-parse --abbrev-ref HEAD') return 'feature/trigger-enable-gate-helper\n'; + if (cmd === 'git remote get-url origin') + return 'https://github.com/mongrel-intelligence/cascade.git\n'; + return ''; + }); + pullsList.mockReset(); + }); + + afterEach(() => { + process.env = originalEnv; + // Note: do NOT call vi.restoreAllMocks() — it would also restore the + // Octokit factory's mockImplementation set at module load, breaking the + // next test in this file. mockReset() above on the per-test spies (and + // resetting via beforeEach) is sufficient. + }); + + function buildCommand() { + const cmd = new FinishCommand([], {} as never); + cmd.log = vi.fn(); + cmd.exit = vi.fn((code?: number) => { + throw new Error(`exit:${code}`); + }) as never; + cmd.parse = vi.fn().mockResolvedValue({ + flags: { + comment: 'shipped', + 'pr-created': false, + 'review-submitted': false, + }, + args: {}, + argv: [], + raw: [], + metadata: {}, + nonExistentFlags: {}, + } as never); + return cmd; + } + + it('succeeds when sidecar is absent but findPRForCurrentBranch resolves a PR via the scoped GitHub client', async () => { + // Mocked Octokit returns one open PR — i.e. the agent did create the PR. + pullsList.mockResolvedValue({ + data: [{ html_url: 'https://github.com/mongrel-intelligence/cascade/pull/1274' }], + }); + + const cmd = buildCommand(); + await cmd.run(); + + expect(pullsList).toHaveBeenCalledWith({ + owner: 'mongrel-intelligence', + repo: 'cascade', + head: 'mongrel-intelligence:feature/trigger-enable-gate-helper', + state: 'open', + per_page: 1, + }); + expect(cmd.log).toHaveBeenCalledWith( + JSON.stringify({ success: true, data: 'Session ended: shipped' }), + ); + }); + + it('still fails (with the legitimate error) when sidecar is absent and the GitHub API returns no PR', async () => { + pullsList.mockResolvedValue({ data: [] }); + + const cmd = buildCommand(); + await expect(cmd.run()).rejects.toThrow('exit:1'); + + expect(pullsList).toHaveBeenCalled(); + expect(cmd.log).toHaveBeenCalledWith( + JSON.stringify({ + success: false, + error: + 'Cannot finish session without creating a PR. ' + + 'You must call CreatePR to submit your changes before calling Finish.', + }), + ); + }); +}); diff --git a/tests/unit/cli/session/finish.test.ts b/tests/unit/cli/session/finish.test.ts index d010038ed..b886de370 100644 --- a/tests/unit/cli/session/finish.test.ts +++ b/tests/unit/cli/session/finish.test.ts @@ -15,6 +15,14 @@ vi.mock('../../../../src/github/client.js', () => ({ githubClient: { getOpenPRByBranch: vi.fn(), }, + withGitHubToken: vi.fn((_token: string, fn: () => Promise) => fn()), +})); + +// PM and other-credential scopes are irrelevant to these tests (only pushed-changes +// validation is exercised). Short-circuit them so we don't need the manifest registry. +vi.mock('../../../../src/pm/index.js', () => ({ + createPMProvider: vi.fn().mockReturnValue({}), + withPMProvider: vi.fn((_p: unknown, fn: () => Promise) => fn()), })); import FinishCommand from '../../../../src/cli/session/finish.js'; diff --git a/tests/unit/triggers/builtins-uniqueness.test.ts b/tests/unit/triggers/builtins-uniqueness.test.ts new file mode 100644 index 000000000..8015187ed --- /dev/null +++ b/tests/unit/triggers/builtins-uniqueness.test.ts @@ -0,0 +1,57 @@ +/** + * Regression: built-in trigger handler name uniqueness with real implementations. + * + * Unlike builtins.test.ts — which mocks every trigger module and + * listPMProviders() so it can assert on ordering/counts — this file uses the + * real production PM manifests (Trello, JIRA, Linear) and real GitHub/Sentry + * trigger classes. That means a duplicate name introduced anywhere in + * src/triggers/ or in a PM manifest's triggerHandlers array will fail this + * test at CI time, not just when the mock is also updated. + */ + +import { describe, expect, it } from 'vitest'; + +// Side-effect imports: register the real production PM providers so +// listPMProviders() (called inside registerBuiltInTriggers) returns the +// actual Trello, JIRA, and Linear manifests with their real handler names. +import '../../../src/integrations/pm/trello/index.js'; +import '../../../src/integrations/pm/jira/index.js'; +import '../../../src/integrations/pm/linear/index.js'; + +import { registerBuiltInTriggers } from '../../../src/triggers/builtins.js'; +import type { TriggerRegistry } from '../../../src/triggers/registry.js'; + +function createCollectingRegistry(): { + register: (handler: unknown) => void; + handlers: unknown[]; +} { + const handlers: unknown[] = []; + return { + register: (handler: unknown) => handlers.push(handler), + handlers, + }; +} + +describe('registerBuiltInTriggers — real handler name uniqueness', () => { + it('registers each built-in trigger handler name exactly once across PM, SCM, and alerting', () => { + const registry = createCollectingRegistry(); + + registerBuiltInTriggers(registry as unknown as TriggerRegistry); + + const registeredNames = registry.handlers.map((h) => (h as { name: string }).name); + const countsByName = new Map(); + for (const name of registeredNames) { + countsByName.set(name, (countsByName.get(name) ?? 0) + 1); + } + + const duplicates = [...countsByName.entries()] + .filter(([, count]) => count > 1) + .map(([name, count]) => `${name} (${count} registrations)`); + + expect( + duplicates, + `Built-in trigger handler names must be globally unique across PM, SCM, alerting, and internal registrations.\n` + + `Duplicate names:\n${duplicates.map((d) => ` - ${d}`).join('\n')}`, + ).toEqual([]); + }); +}); diff --git a/tests/unit/triggers/shared/agent-execution-lifecycle.test.ts b/tests/unit/triggers/shared/agent-execution-lifecycle.test.ts new file mode 100644 index 000000000..f2dd7278b --- /dev/null +++ b/tests/unit/triggers/shared/agent-execution-lifecycle.test.ts @@ -0,0 +1,283 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + mockValidateIntegrations, + mockFormatValidationErrors, + mockCheckBudgetExceeded, + mockHandleAgentResultArtifacts, + mockLogger, +} = vi.hoisted(() => ({ + mockValidateIntegrations: vi.fn(), + mockFormatValidationErrors: vi.fn(), + mockCheckBudgetExceeded: vi.fn(), + mockHandleAgentResultArtifacts: vi.fn(), + mockLogger: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + }, +})); + +vi.mock('../../../../src/triggers/shared/integration-validation.js', () => ({ + validateIntegrations: mockValidateIntegrations, + formatValidationErrors: mockFormatValidationErrors, +})); + +vi.mock('../../../../src/triggers/shared/budget.js', () => ({ + checkBudgetExceeded: mockCheckBudgetExceeded, +})); + +vi.mock('../../../../src/triggers/shared/agent-result-handler.js', () => ({ + handleAgentResultArtifacts: mockHandleAgentResultArtifacts, +})); + +vi.mock('../../../../src/utils/logging.js', () => ({ + logger: mockLogger, +})); + +import type { PMLifecycleManager } from '../../../../src/pm/index.js'; +import { + checkPreRunBudget, + prepareAgentExecutionLifecycle, + runPostAgentExecutionLifecycle, + validateAgentExecutionLifecycle, +} from '../../../../src/triggers/shared/agent-execution-lifecycle.js'; +import type { AgentExecutionContext } from '../../../../src/triggers/shared/agent-execution-types.js'; +import type { TriggerResult } from '../../../../src/triggers/types.js'; +import type { AgentResult, CascadeConfig, ProjectConfig } from '../../../../src/types/index.js'; + +const PROJECT = { + id: 'project-1', + pm: { type: 'trello' }, +} as ProjectConfig; + +const CONFIG = { projects: [PROJECT] } as CascadeConfig; + +const RESULT: TriggerResult = { + agentType: 'implementation', + workItemId: 'card-1', + agentInput: {}, +}; + +function createLifecycle() { + return { + prepareForAgent: vi.fn().mockResolvedValue(undefined), + handleSuccess: vi.fn().mockResolvedValue(undefined), + handleFailure: vi.fn().mockResolvedValue(undefined), + handleBudgetExceeded: vi.fn().mockResolvedValue(undefined), + handleBudgetWarning: vi.fn().mockResolvedValue(undefined), + cleanupProcessing: vi.fn().mockResolvedValue(undefined), + } as unknown as PMLifecycleManager & Record>; +} + +function createContext( + lifecycle: PMLifecycleManager, + overrides: Partial = {}, +): AgentExecutionContext { + return { + result: RESULT, + project: PROJECT, + config: CONFIG, + executionConfig: {}, + agentType: 'implementation', + logLabel: 'Agent', + lifecycle, + lifecycleHooks: {}, + workItemId: 'card-1', + agentInput: {}, + ...overrides, + }; +} + +describe('agent execution lifecycle helper', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockValidateIntegrations.mockResolvedValue({ valid: true, errors: [] }); + mockFormatValidationErrors.mockReturnValue('validation error'); + mockCheckBudgetExceeded.mockResolvedValue(null); + mockHandleAgentResultArtifacts.mockResolvedValue(undefined); + }); + + it('allows execution when integration validation succeeds', async () => { + const lifecycle = createLifecycle(); + + await expect( + validateAgentExecutionLifecycle({ + result: RESULT, + project: PROJECT, + executionConfig: {}, + agentType: 'implementation', + lifecycle, + }), + ).resolves.toBe(true); + + expect(mockValidateIntegrations).toHaveBeenCalledWith('project-1', 'implementation', PROJECT); + expect(lifecycle.handleFailure).not.toHaveBeenCalled(); + }); + + it('formats, logs, notifies PM, and invokes onFailure when non-PM validation fails', async () => { + const lifecycle = createLifecycle(); + const onFailure = vi.fn().mockResolvedValue(undefined); + const validation = { + valid: false, + errors: [{ category: 'scm' as const, message: 'GitHub token missing' }], + }; + mockValidateIntegrations.mockResolvedValueOnce(validation); + + await expect( + validateAgentExecutionLifecycle({ + result: RESULT, + project: PROJECT, + executionConfig: { onFailure }, + agentType: 'implementation', + lifecycle, + }), + ).resolves.toBe(false); + + expect(mockFormatValidationErrors).toHaveBeenCalledWith(validation); + expect(mockLogger.error).toHaveBeenCalledWith('Integration validation failed', { + agentType: 'implementation', + projectId: 'project-1', + errors: validation.errors, + }); + expect(lifecycle.handleFailure).toHaveBeenCalledWith('card-1', 'validation error'); + expect(onFailure).toHaveBeenCalledWith(RESULT, { + success: false, + output: '', + error: 'validation error', + }); + }); + + it('does not notify PM when PM validation failed', async () => { + const lifecycle = createLifecycle(); + mockValidateIntegrations.mockResolvedValueOnce({ + valid: false, + errors: [{ category: 'pm' as const, message: 'Trello missing' }], + }); + + await validateAgentExecutionLifecycle({ + result: RESULT, + project: PROJECT, + executionConfig: {}, + agentType: 'implementation', + lifecycle, + }); + + expect(lifecycle.handleFailure).not.toHaveBeenCalled(); + }); + + it('handles pre-run budget exceedance and tells callers to abort', async () => { + const lifecycle = createLifecycle(); + mockCheckBudgetExceeded.mockResolvedValueOnce({ + exceeded: true, + currentCost: 6, + budget: 5, + remaining: 0, + }); + + const result = await checkPreRunBudget('card-1', PROJECT, lifecycle); + + expect(result).toEqual({ remainingBudgetUsd: undefined, abort: true }); + expect(lifecycle.handleBudgetExceeded).toHaveBeenCalledWith('card-1', 6, 5); + }); + + it('returns remaining budget when pre-run budget is not exceeded', async () => { + const lifecycle = createLifecycle(); + mockCheckBudgetExceeded.mockResolvedValueOnce({ + exceeded: false, + currentCost: 2, + budget: 5, + remaining: 3, + }); + + await expect(checkPreRunBudget('card-1', PROJECT, lifecycle)).resolves.toEqual({ + remainingBudgetUsd: 3, + abort: false, + }); + expect(lifecycle.handleBudgetExceeded).not.toHaveBeenCalled(); + }); + + it('runs prepareForAgent unless skipPrepareForAgent is set', async () => { + const lifecycle = createLifecycle(); + await prepareAgentExecutionLifecycle(createContext(lifecycle)); + await prepareAgentExecutionLifecycle( + createContext(lifecycle, { executionConfig: { skipPrepareForAgent: true } }), + ); + + expect(lifecycle.prepareForAgent).toHaveBeenCalledTimes(1); + expect(lifecycle.prepareForAgent).toHaveBeenCalledWith('card-1', {}); + }); + + it('runs post-agent artifacts, budget warning, cleanup, and success in order', async () => { + const lifecycle = createLifecycle(); + const agentResult: AgentResult = { + success: true, + output: '', + prUrl: 'https://github.com/acme/myapp/pull/1', + progressCommentId: 'progress-1', + }; + mockCheckBudgetExceeded.mockResolvedValueOnce({ + exceeded: true, + currentCost: 5.5, + budget: 5, + remaining: 0, + }); + + await runPostAgentExecutionLifecycle( + 'card-1', + 'implementation', + agentResult, + PROJECT, + lifecycle, + {}, + {}, + ); + + expect(mockHandleAgentResultArtifacts).toHaveBeenCalledWith( + 'card-1', + 'implementation', + agentResult, + PROJECT, + ); + expect(lifecycle.handleBudgetWarning).toHaveBeenCalledWith('card-1', 5.5, 5); + expect(lifecycle.cleanupProcessing).toHaveBeenCalledWith('card-1'); + expect(lifecycle.handleSuccess).toHaveBeenCalledWith( + 'card-1', + {}, + 'https://github.com/acme/myapp/pull/1', + 'progress-1', + ); + expect(mockHandleAgentResultArtifacts.mock.invocationCallOrder[0]).toBeLessThan( + lifecycle.handleBudgetWarning.mock.invocationCallOrder[0], + ); + expect(lifecycle.handleBudgetWarning.mock.invocationCallOrder[0]).toBeLessThan( + lifecycle.cleanupProcessing.mock.invocationCallOrder[0], + ); + expect(lifecycle.cleanupProcessing.mock.invocationCallOrder[0]).toBeLessThan( + lifecycle.handleSuccess.mock.invocationCallOrder[0], + ); + }); + + it('preserves GitHub-style post-run skip options', async () => { + const lifecycle = createLifecycle(); + + await runPostAgentExecutionLifecycle( + 'card-1', + 'review', + { success: false, output: '', error: 'review failed' }, + PROJECT, + lifecycle, + {}, + { + skipPrepareForAgent: true, + skipHandleFailure: true, + handleSuccessOnlyForAgentType: 'implementation', + }, + ); + + expect(lifecycle.cleanupProcessing).not.toHaveBeenCalled(); + expect(lifecycle.handleSuccess).not.toHaveBeenCalled(); + expect(lifecycle.handleFailure).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/triggers/shared/agent-work-items.test.ts b/tests/unit/triggers/shared/agent-work-items.test.ts new file mode 100644 index 000000000..40bc2dbe1 --- /dev/null +++ b/tests/unit/triggers/shared/agent-work-items.test.ts @@ -0,0 +1,283 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + mockCreateWorkItem, + mockLinkPRToWorkItem, + mockLookupWorkItemForPR, + mockUpdateRunPRNumber, + mockGithubClient, + mockParseRepoFullName, + mockLogger, +} = vi.hoisted(() => ({ + mockCreateWorkItem: vi.fn().mockResolvedValue(undefined), + mockLinkPRToWorkItem: vi.fn().mockResolvedValue(undefined), + mockLookupWorkItemForPR: vi.fn().mockResolvedValue(null), + mockUpdateRunPRNumber: vi.fn().mockResolvedValue(undefined), + mockGithubClient: { + getPR: vi.fn().mockResolvedValue({ title: 'feat: linked PR' }), + }, + mockParseRepoFullName: vi.fn().mockReturnValue({ owner: 'acme', repo: 'myapp' }), + mockLogger: { + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + error: vi.fn(), + }, +})); + +vi.mock('../../../../src/db/repositories/prWorkItemsRepository.js', () => ({ + createWorkItem: mockCreateWorkItem, + linkPRToWorkItem: mockLinkPRToWorkItem, + lookupWorkItemForPR: mockLookupWorkItemForPR, +})); + +vi.mock('../../../../src/db/repositories/runsRepository.js', () => ({ + updateRunPRNumber: mockUpdateRunPRNumber, +})); + +vi.mock('../../../../src/github/client.js', () => ({ + githubClient: mockGithubClient, +})); + +vi.mock('../../../../src/utils/repo.js', () => ({ + parseRepoFullName: mockParseRepoFullName, +})); + +vi.mock('../../../../src/utils/logging.js', () => ({ + logger: mockLogger, +})); + +import { + linkPRPostExecution, + persistPreRunWorkItems, + prepareAgentWorkItem, + resolveWorkItemId, +} from '../../../../src/triggers/shared/agent-work-items.js'; +import type { TriggerResult } from '../../../../src/triggers/types.js'; +import type { ProjectConfig } from '../../../../src/types/index.js'; + +const PROJECT = { + id: 'project-1', + repo: 'acme/myapp', + pm: { type: 'trello' }, +} as unknown as ProjectConfig; + +describe('agent-work-items', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockCreateWorkItem.mockResolvedValue(undefined); + mockLinkPRToWorkItem.mockResolvedValue(undefined); + mockLookupWorkItemForPR.mockResolvedValue(null); + mockUpdateRunPRNumber.mockResolvedValue(undefined); + mockGithubClient.getPR.mockResolvedValue({ title: 'feat: linked PR' }); + mockParseRepoFullName.mockReturnValue({ owner: 'acme', repo: 'myapp' }); + }); + + describe('resolveWorkItemId', () => { + it('prefers the trigger-supplied workItemId', async () => { + const result = await resolveWorkItemId('card-from-trigger', 'project-1', 42); + + expect(result).toBe('card-from-trigger'); + expect(mockLookupWorkItemForPR).not.toHaveBeenCalled(); + }); + + it('falls back to PR lookup when no trigger workItemId is present', async () => { + mockLookupWorkItemForPR.mockResolvedValueOnce('card-from-db'); + + const result = await resolveWorkItemId(undefined, 'project-1', 42); + + expect(result).toBe('card-from-db'); + expect(mockLookupWorkItemForPR).toHaveBeenCalledWith('project-1', 42); + }); + + it('warns and returns undefined when PR lookup fails', async () => { + mockLookupWorkItemForPR.mockRejectedValueOnce(new Error('db unavailable')); + + const result = await resolveWorkItemId(undefined, 'project-1', 42); + + expect(result).toBeUndefined(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Failed to resolve workItemId for PR', + expect.objectContaining({ projectId: 'project-1', prNumber: 42 }), + ); + }); + }); + + describe('prepareAgentWorkItem', () => { + it('patches agentInput.workItemId only when a work item is resolved', async () => { + mockLookupWorkItemForPR.mockResolvedValueOnce('card-from-db'); + const result = await prepareAgentWorkItem( + { + agentType: 'review', + agentInput: { prNumber: 42 }, + prNumber: 42, + }, + 'project-1', + ); + + expect(result).toEqual({ + workItemId: 'card-from-db', + agentInput: { prNumber: 42, workItemId: 'card-from-db' }, + }); + }); + + it('leaves agentInput untouched when no work item is resolved', async () => { + const agentInput = { prNumber: 42 }; + const result = await prepareAgentWorkItem( + { agentType: 'review', agentInput, prNumber: 42 }, + 'project-1', + ); + + expect(result.workItemId).toBeUndefined(); + expect(result.agentInput).toBe(agentInput); + }); + }); + + describe('persistPreRunWorkItems', () => { + it('persists PM work-item display data and links the PR before the run', async () => { + const result: TriggerResult = { + agentType: 'review', + agentInput: {}, + workItemId: 'card-1', + workItemUrl: 'https://trello.com/c/card-1', + workItemTitle: 'Build the thing', + prNumber: 42, + prUrl: 'https://github.com/acme/myapp/pull/42', + prTitle: 'Test PR', + }; + + await persistPreRunWorkItems(result, PROJECT, 'card-1'); + + expect(mockCreateWorkItem).toHaveBeenCalledWith('project-1', 'card-1', { + workItemUrl: 'https://trello.com/c/card-1', + workItemTitle: 'Build the thing', + }); + expect(mockLinkPRToWorkItem).toHaveBeenCalledWith( + 'project-1', + 'acme/myapp', + 42, + 'card-1', + expect.objectContaining({ + prUrl: 'https://github.com/acme/myapp/pull/42', + prTitle: 'Test PR', + }), + ); + }); + + it('creates an orphan PR link when no workItemId is resolved', async () => { + await persistPreRunWorkItems( + { + agentType: 'review', + agentInput: {}, + prNumber: 42, + prUrl: 'https://github.com/acme/myapp/pull/42', + }, + PROJECT, + undefined, + ); + + expect(mockCreateWorkItem).not.toHaveBeenCalled(); + expect(mockLinkPRToWorkItem).toHaveBeenCalledWith( + 'project-1', + 'acme/myapp', + 42, + null, + expect.objectContaining({ prUrl: 'https://github.com/acme/myapp/pull/42' }), + ); + }); + + it('logs persistence and link failures without throwing', async () => { + mockCreateWorkItem.mockRejectedValueOnce(new Error('create failed')); + mockLinkPRToWorkItem.mockRejectedValueOnce(new Error('link failed')); + + await expect( + persistPreRunWorkItems( + { agentType: 'review', agentInput: {}, prNumber: 42 }, + PROJECT, + 'card-1', + ), + ).resolves.toBeUndefined(); + + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Failed to persist work-item row for PM-triggered run', + expect.objectContaining({ workItemId: 'card-1' }), + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Failed to ensure pr_work_items entry for PR-triggered run', + expect.objectContaining({ prNumber: 42, workItemId: 'card-1' }), + ); + }); + }); + + describe('linkPRPostExecution', () => { + const agentResult = { + success: true, + output: '', + runId: 'run-1', + prUrl: 'https://github.com/acme/myapp/pull/42', + }; + + it('fetches the PR title, links the PR, and backfills run prNumber', async () => { + await linkPRPostExecution( + agentResult, + PROJECT as ProjectConfig & { repo: string }, + { + agentType: 'implementation', + agentInput: {}, + workItemUrl: 'https://trello.com/c/card-1', + workItemTitle: 'Build the thing', + }, + 'card-1', + ); + + expect(mockGithubClient.getPR).toHaveBeenCalledWith('acme', 'myapp', 42); + expect(mockLinkPRToWorkItem).toHaveBeenCalledWith( + 'project-1', + 'acme/myapp', + 42, + 'card-1', + expect.objectContaining({ + prUrl: 'https://github.com/acme/myapp/pull/42', + prTitle: 'feat: linked PR', + workItemTitle: 'Build the thing', + }), + ); + expect(mockUpdateRunPRNumber).toHaveBeenCalledWith('run-1', 42); + }); + + it('continues when PR title fetch, link, and run backfill fail', async () => { + mockGithubClient.getPR.mockRejectedValueOnce(new Error('github failed')); + mockLinkPRToWorkItem.mockRejectedValueOnce(new Error('link failed')); + mockUpdateRunPRNumber.mockRejectedValueOnce(new Error('run failed')); + + await expect( + linkPRPostExecution( + agentResult, + PROJECT as ProjectConfig & { repo: string }, + { agentType: 'implementation', agentInput: {} }, + 'card-1', + ), + ).resolves.toBeUndefined(); + + expect(mockLinkPRToWorkItem).toHaveBeenCalledWith( + 'project-1', + 'acme/myapp', + 42, + 'card-1', + expect.objectContaining({ prTitle: undefined }), + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Failed to fetch PR title from GitHub', + expect.objectContaining({ prNumber: 42 }), + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Failed to link PR to work item post-execution', + expect.objectContaining({ prNumber: 42, workItemId: 'card-1' }), + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Failed to backfill prNumber on run', + expect.objectContaining({ runId: 'run-1', prNumber: 42 }), + ); + }); + }); +}); diff --git a/tests/unit/triggers/shared/events.test.ts b/tests/unit/triggers/shared/events.test.ts new file mode 100644 index 000000000..2c5723e1e --- /dev/null +++ b/tests/unit/triggers/shared/events.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest'; +import { + ALL_TRIGGER_EVENTS, + type AlertingTriggerEvent, + type InternalTriggerEvent, + type PMTriggerEvent, + type SCMTriggerEvent, + TRIGGER_EVENTS, +} from '../../../../src/triggers/shared/events.js'; + +describe('TRIGGER_EVENTS', () => { + it('catalogs every current trigger event', () => { + expect(ALL_TRIGGER_EVENTS).toEqual([ + 'pm:status-changed', + 'pm:label-added', + 'pm:comment-mention', + 'scm:check-suite-success', + 'scm:check-suite-failure', + 'scm:pr-review-submitted', + 'scm:review-requested', + 'scm:pr-opened', + 'scm:pr-comment-mention', + 'scm:pr-merged', + 'scm:pr-ready-to-merge', + 'scm:pr-conflict-detected', + 'alerting:issue-alert', + 'alerting:metric-alert', + 'internal:auto-chain', + ]); + }); + + it('keeps event values available as TypeScript literals', () => { + const pmEvent = TRIGGER_EVENTS.PM.STATUS_CHANGED satisfies 'pm:status-changed'; + const scmEvent = TRIGGER_EVENTS.SCM.PR_OPENED satisfies 'scm:pr-opened'; + const alertingEvent = TRIGGER_EVENTS.ALERTING.ISSUE_ALERT satisfies 'alerting:issue-alert'; + const internalEvent = TRIGGER_EVENTS.INTERNAL.AUTO_CHAIN satisfies 'internal:auto-chain'; + + const pmTyped: PMTriggerEvent = pmEvent; + const scmTyped: SCMTriggerEvent = scmEvent; + const alertingTyped: AlertingTriggerEvent = alertingEvent; + const internalTyped: InternalTriggerEvent = internalEvent; + + expect([pmTyped, scmTyped, alertingTyped, internalTyped]).toEqual([ + 'pm:status-changed', + 'scm:pr-opened', + 'alerting:issue-alert', + 'internal:auto-chain', + ]); + }); +}); diff --git a/tests/unit/triggers/shared/result-builders.test.ts b/tests/unit/triggers/shared/result-builders.test.ts new file mode 100644 index 000000000..196d4a420 --- /dev/null +++ b/tests/unit/triggers/shared/result-builders.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, it, vi } from 'vitest'; +import { TRIGGER_EVENTS } from '../../../../src/triggers/shared/events.js'; +import { + buildDeferredRecheckResult, + buildGitHubPRDispatchResult, + buildNoAgentResult, + buildPMDispatchResult, + buildSkipResult, +} from '../../../../src/triggers/shared/result-builders.js'; + +describe('trigger result builders', () => { + it('builds PM dispatch results and mirrors workItemId into agentInput', () => { + const result = buildPMDispatchResult({ + agentType: 'implementation', + triggerEvent: TRIGGER_EVENTS.PM.STATUS_CHANGED, + workItemId: 'card-123', + workItemUrl: 'https://trello.com/c/abc', + workItemTitle: 'Implement feature', + agentInput: { + workItemId: 'stale-id', + modelOverride: 'gpt-test', + }, + }); + + expect(result).toEqual({ + agentType: 'implementation', + agentInput: { + workItemId: 'card-123', + modelOverride: 'gpt-test', + triggerEvent: 'pm:status-changed', + }, + workItemId: 'card-123', + workItemUrl: 'https://trello.com/c/abc', + workItemTitle: 'Implement feature', + onBlocked: undefined, + coalesceKey: undefined, + }); + }); + + it('builds GitHub PR dispatch results and mirrors a linked work item into agentInput', () => { + const onBlocked = vi.fn(); + + const result = buildGitHubPRDispatchResult({ + agentType: 'review', + triggerEvent: TRIGGER_EVENTS.SCM.PR_OPENED, + prNumber: 42, + prUrl: 'https://github.com/acme/repo/pull/42', + prTitle: 'feat: add thing', + workItemId: 'card-456', + agentInput: { + prNumber: 42, + repoFullName: 'acme/repo', + }, + onBlocked, + }); + + expect(result).toEqual({ + agentType: 'review', + agentInput: { + prNumber: 42, + repoFullName: 'acme/repo', + triggerEvent: 'scm:pr-opened', + workItemId: 'card-456', + }, + prNumber: 42, + prUrl: 'https://github.com/acme/repo/pull/42', + prTitle: 'feat: add thing', + workItemId: 'card-456', + workItemUrl: undefined, + workItemTitle: undefined, + onBlocked, + coalesceKey: undefined, + }); + }); + + it('mirrors prNumber into agentInput from options even when not present in agentInput', () => { + const result = buildGitHubPRDispatchResult({ + agentType: 'review', + triggerEvent: TRIGGER_EVENTS.SCM.CHECK_SUITE_SUCCESS, + prNumber: 42, + agentInput: { + repoFullName: 'acme/repo', + }, + }); + + expect(result.agentInput).toEqual({ + prNumber: 42, + repoFullName: 'acme/repo', + triggerEvent: 'scm:check-suite-success', + }); + expect(result.workItemId).toBeUndefined(); + }); + + it('mirrors prNumber into agentInput with the option value winning over a stale agentInput.prNumber', () => { + const result = buildGitHubPRDispatchResult({ + agentType: 'review', + triggerEvent: TRIGGER_EVENTS.SCM.PR_REVIEW_SUBMITTED, + prNumber: 42, + agentInput: { + prNumber: 99, // stale value — should be overridden + repoFullName: 'acme/repo', + }, + }); + + expect(result.agentInput?.prNumber).toBe(42); + expect(result.prNumber).toBe(42); + }); + + it('builds structured skip results with the existing skip payload shape', () => { + expect(buildSkipResult('check-suite-success', 'not all checks complete')).toEqual({ + agentType: null, + agentInput: {}, + skipReason: { + handler: 'check-suite-success', + message: 'not all checks complete', + }, + }); + }); + + it('builds no-agent operation results', () => { + expect( + buildNoAgentResult({ lockKey: 'sentry:issue-1', coalesceKey: 'p1:sentry:issue-1' }), + ).toEqual({ + agentType: null, + agentInput: {}, + lockKey: 'sentry:issue-1', + coalesceKey: 'p1:sentry:issue-1', + }); + }); + + it('builds deferred re-check results with scheduler metadata', () => { + expect( + buildDeferredRecheckResult({ + delayMs: 45_000, + coalesceKey: 'p1:pr-conflict-recheck:42', + }), + ).toEqual({ + agentType: null, + agentInput: {}, + deferredRecheck: { + delayMs: 45_000, + coalesceKey: 'p1:pr-conflict-recheck:42', + }, + }); + }); +}); diff --git a/tests/unit/triggers/trigger-event-consistency.test.ts b/tests/unit/triggers/trigger-event-consistency.test.ts index d078fb0f0..ed1c925da 100644 --- a/tests/unit/triggers/trigger-event-consistency.test.ts +++ b/tests/unit/triggers/trigger-event-consistency.test.ts @@ -28,6 +28,29 @@ import { describe, expect, it } from 'vitest'; const TRIGGERS_ROOT = join(__dirname, '..', '..', '..', 'src', 'triggers'); +const BUILT_IN_TRIGGER_FILES = [ + 'github/check-suite-failure.ts', + 'github/check-suite-success.ts', + 'github/pr-comment-mention.ts', + 'github/pr-conflict-detected.ts', + 'github/pr-merged.ts', + 'github/pr-opened.ts', + 'github/pr-ready-to-merge.ts', + 'github/pr-review-submitted.ts', + 'github/review-requested.ts', + 'jira/comment-mention.ts', + 'jira/label-added.ts', + 'jira/status-changed.ts', + 'linear/comment-mention.ts', + 'linear/label-added.ts', + 'linear/status-changed.ts', + 'sentry/alerting-issue.ts', + 'sentry/alerting-metric.ts', + 'trello/comment-mention.ts', + 'trello/label-added.ts', + 'trello/status-changed.ts', +].map((path) => join(TRIGGERS_ROOT, path)); + // Handlers that legitimately gate on one event without emitting it as a // `triggerEvent`. Add an entry ONLY when there's a real reason — every // exemption silently weakens the guard. @@ -101,6 +124,19 @@ describe('trigger-event-string consistency (static guard)', () => { expect(scans.length).toBeGreaterThan(10); }); + it('covers every current built-in trigger source file', () => { + const scannedFiles = new Set(allFiles); + const missingFiles = BUILT_IN_TRIGGER_FILES.filter((file) => !scannedFiles.has(file)); + + expect( + missingFiles.map((file) => file.replace(`${TRIGGERS_ROOT}/`, 'src/triggers/')), + `trigger-event-string consistency must scan every built-in trigger file registered by PM manifests, SCM, and alerting. ` + + `Missing files:\n${missingFiles + .map((file) => ` - ${file.replace(`${TRIGGERS_ROOT}/`, 'src/triggers/')}`) + .join('\n')}`, + ).toEqual([]); + }); + for (const scan of scans) { const relPath = scan.file.replace(`${TRIGGERS_ROOT}/`, 'src/triggers/'); diff --git a/tests/unit/triggers/trigger-work-item-id-consistency.test.ts b/tests/unit/triggers/trigger-work-item-id-consistency.test.ts index 178185713..a8f166376 100644 --- a/tests/unit/triggers/trigger-work-item-id-consistency.test.ts +++ b/tests/unit/triggers/trigger-work-item-id-consistency.test.ts @@ -34,6 +34,29 @@ import { describe, expect, it } from 'vitest'; const TRIGGERS_ROOT = join(__dirname, '..', '..', '..', 'src', 'triggers'); +const BUILT_IN_TRIGGER_FILES = [ + 'github/check-suite-failure.ts', + 'github/check-suite-success.ts', + 'github/pr-comment-mention.ts', + 'github/pr-conflict-detected.ts', + 'github/pr-merged.ts', + 'github/pr-opened.ts', + 'github/pr-ready-to-merge.ts', + 'github/pr-review-submitted.ts', + 'github/review-requested.ts', + 'jira/comment-mention.ts', + 'jira/label-added.ts', + 'jira/status-changed.ts', + 'linear/comment-mention.ts', + 'linear/label-added.ts', + 'linear/status-changed.ts', + 'sentry/alerting-issue.ts', + 'sentry/alerting-metric.ts', + 'trello/comment-mention.ts', + 'trello/label-added.ts', + 'trello/status-changed.ts', +].map((path) => join(TRIGGERS_ROOT, path)); + interface Violation { file: string; line: number; @@ -137,6 +160,19 @@ describe('trigger workItemId consistency (static guard)', () => { expect(allFiles.length).toBeGreaterThan(10); }); + it('covers every current built-in trigger source file', () => { + const scannedFiles = new Set(allFiles); + const missingFiles = BUILT_IN_TRIGGER_FILES.filter((file) => !scannedFiles.has(file)); + + expect( + missingFiles.map((file) => file.replace(`${TRIGGERS_ROOT}/`, 'src/triggers/')), + `trigger workItemId consistency must scan every built-in trigger file registered by PM manifests, SCM, and alerting. ` + + `Missing files:\n${missingFiles + .map((file) => ` - ${file.replace(`${TRIGGERS_ROOT}/`, 'src/triggers/')}`) + .join('\n')}`, + ).toEqual([]); + }); + const allViolations = allFiles.flatMap(scanFile); it('every TriggerResult with a top-level workItemId also carries it inside agentInput', () => {