From 1e48d963da4c6ff23ec01f1c7a54035e101acc82 Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Fri, 20 Feb 2026 08:05:27 +0000 Subject: [PATCH 01/10] refactor(agents): unify dual-backend gadget builders and prompt functions --- src/agents/base.ts | 147 +----------- src/agents/shared/gadgets.ts | 112 ++++++++- src/agents/shared/taskPrompts.ts | 152 ++++++++++++ src/backends/agent-profiles.ts | 229 ++++--------------- tests/unit/agents/shared/gadgets.test.ts | 206 +++++++++++++++++ tests/unit/agents/shared/taskPrompts.test.ts | 147 ++++++++++++ 6 files changed, 671 insertions(+), 322 deletions(-) create mode 100644 src/agents/shared/taskPrompts.ts create mode 100644 tests/unit/agents/shared/gadgets.test.ts create mode 100644 tests/unit/agents/shared/taskPrompts.test.ts diff --git a/src/agents/base.ts b/src/agents/base.ts index 259cee41..0c38e123 100644 --- a/src/agents/base.ts +++ b/src/agents/base.ts @@ -1,31 +1,9 @@ import type { ModelSpec, createLogger } from 'llmist'; -import { WriteFile } from '../gadgets/WriteFile.js'; import { type ProgressMonitor, createProgressMonitor } from '../backends/progress.js'; import { CUSTOM_MODELS } from '../config/customModels.js'; import { loadPartials } from '../db/repositories/partialsRepository.js'; -import { AstGrep } from '../gadgets/AstGrep.js'; -import { FileMultiEdit } from '../gadgets/FileMultiEdit.js'; -import { FileSearchAndReplace } from '../gadgets/FileSearchAndReplace.js'; -import { Finish } from '../gadgets/Finish.js'; -import { ListDirectory } from '../gadgets/ListDirectory.js'; -import { ReadFile } from '../gadgets/ReadFile.js'; -import { RipGrep } from '../gadgets/RipGrep.js'; -import { Sleep } from '../gadgets/Sleep.js'; -import { VerifyChanges } from '../gadgets/VerifyChanges.js'; -import { CreatePR } from '../gadgets/github/index.js'; -import { - AddChecklist, - CreateWorkItem, - ListWorkItems, - PMUpdateChecklistItem, - PostComment, - ReadWorkItem, - UpdateWorkItem, - formatWorkItemData, -} from '../gadgets/pm/index.js'; -import { Tmux } from '../gadgets/tmux.js'; -import { TodoDelete, TodoUpdateStatus, TodoUpsert } from '../gadgets/todo/index.js'; +import { formatWorkItemData } from '../gadgets/pm/index.js'; import { type Todo, formatTodoList, initTodoSession, saveTodos } from '../gadgets/todo/storage.js'; import { getPMProvider } from '../pm/index.js'; import type { AgentInput, AgentResult, CascadeConfig, ProjectConfig } from '../types/index.js'; @@ -33,6 +11,7 @@ import { logger } from '../utils/logging.js'; import { extractPRUrl } from '../utils/prUrl.js'; import { type BuilderType, createConfiguredBuilder } from './shared/builderFactory.js'; import { getAgentCapabilities } from './shared/capabilities.js'; +import { buildWorkItemGadgets } from './shared/gadgets.js'; import { type FileLogger, executeAgentLifecycle } from './shared/lifecycle.js'; import { resolveModelConfig } from './shared/modelResolution.js'; import { buildPromptContext } from './shared/promptContext.js'; @@ -43,6 +22,12 @@ import { injectSquintContext, injectSyntheticCall, } from './shared/syntheticCalls.js'; +import { + buildCheckFailurePrompt, + buildCommentResponsePrompt, + buildDebugPrompt, + buildWorkItemPrompt, +} from './shared/taskPrompts.js'; import type { AccumulatedLlmCall } from './utils/hooks.js'; import type { AgentLogger } from './utils/logging.js'; import type { TrackingContext } from './utils/tracking.js'; @@ -111,7 +96,7 @@ function selectPrompt( } if (prContext) return buildCheckFailurePrompt(prContext); if (debugContext) return buildDebugPrompt(debugContext); - return buildPrompt(cardId ?? ''); + return buildWorkItemPrompt(cardId ?? ''); } async function buildAgentContext( @@ -178,124 +163,12 @@ async function buildAgentContext( }; } -function buildCommentResponsePrompt( - cardId: string, - commentText: string, - commentAuthor: string, -): string { - return `A user (@${commentAuthor}) mentioned you in a comment on work item ${cardId}. - -Their comment: ---- -${commentText} ---- - -The work item data (title, description, checklists, attachments, comments) has been pre-loaded above. -Read the user's comment carefully and respond accordingly. Default to surgical, targeted updates unless they clearly ask for a full rewrite.`; -} - -function buildPrompt(cardId: string): string { - return `Analyze and process the work item with ID: ${cardId}. The work item data (title, description, checklists, attachments, comments) has been pre-loaded above. Review it and proceed with your task.`; -} - -function buildCheckFailurePrompt(prContext: { - prNumber: number; - prBranch: string; - repoFullName: string; - headSha: string; -}): string { - const [owner, repo] = prContext.repoFullName.split('/'); - - return `You are on branch \`${prContext.prBranch}\` for PR #${prContext.prNumber}. - -Your task is to fix the failing checks and push your changes. - -## Instructions - -1. **Investigate failures**: Use Tmux to run: - \`gh run list --branch ${prContext.prBranch} --limit 5 --json databaseId,conclusion,status,workflowName\` - -2. **Get failure details**: Find failed run ID and run: - \`gh run view --log-failed\` - -3. **Analyze error types**: - - Lint errors: Run \`npm run lint\` or \`pnpm run lint\` - - Type errors: Run \`npm run typecheck\` - - Test failures: Run \`npm test\` - - Build errors: Run \`npm run build\` - -4. **Fix issues**: Make targeted fixes following existing codebase patterns - -5. **Verify locally**: Run the same checks that failed in CI before pushing - -6. **Commit and push**: - \`\`\`bash - git add . - git commit -m "fix: address failing checks" - git push - \`\`\` - -The push will re-trigger checks automatically. - -## GitHub Context -Owner: ${owner} -Repo: ${repo} -PR: #${prContext.prNumber} -Branch: ${prContext.prBranch}`; -} - -function buildDebugPrompt(debugContext: { - logDir: string; - originalCardName: string; - originalCardUrl: string; - detectedAgentType: string; -}): string { - return `Analyze the ${debugContext.detectedAgentType} agent session logs in directory: ${debugContext.logDir} - -Original card: "${debugContext.originalCardName}" -Link: ${debugContext.originalCardUrl} - -Start by listing the contents of the log directory, then read and analyze the logs to identify issues.`; -} - // ============================================================================ // Agent Builder Creation // ============================================================================ function getBaseAgentGadgets(agentType: string) { - const caps = getAgentCapabilities(agentType); - - return [ - // Filesystem gadgets (read-only when canEditFiles is false) - new ListDirectory(), - new ReadFile(), - new RipGrep(), - new AstGrep(), - ...(caps.canEditFiles - ? [new FileSearchAndReplace(), new FileMultiEdit(), new WriteFile(), new VerifyChanges()] - : []), - // Shell commands via tmux (no timeout issues) - new Tmux(), - new Sleep(), - // Task tracking gadgets - new TodoUpsert(), - new TodoUpdateStatus(), - new TodoDelete(), - // GitHub gadgets (PR creation gated by capability) - ...(caps.canCreatePR ? [new CreatePR()] : []), - // PM gadgets (work items, comments, checklists — PM-agnostic) - new ReadWorkItem(), - new PostComment(), - new UpdateWorkItem(), - new CreateWorkItem(), - new ListWorkItems(), - new AddChecklist(), - // UpdateChecklistItem gated by capability — prevents planning from marking items complete - // prematurely, while respond-to-planning-comment CAN update them - ...(caps.canUpdateChecklists ? [new PMUpdateChecklistItem()] : []), - // Session control - new Finish(), - ]; + return buildWorkItemGadgets(getAgentCapabilities(agentType)); } function createAgentBuilderWithGadgets( diff --git a/src/agents/shared/gadgets.ts b/src/agents/shared/gadgets.ts index f07b4039..ea601668 100644 --- a/src/agents/shared/gadgets.ts +++ b/src/agents/shared/gadgets.ts @@ -1,12 +1,17 @@ import { AstGrep } from '../../gadgets/AstGrep.js'; +import { FileMultiEdit } from '../../gadgets/FileMultiEdit.js'; import { FileSearchAndReplace } from '../../gadgets/FileSearchAndReplace.js'; import { Finish } from '../../gadgets/Finish.js'; import { ListDirectory } from '../../gadgets/ListDirectory.js'; import { ReadFile } from '../../gadgets/ReadFile.js'; import { RipGrep } from '../../gadgets/RipGrep.js'; import { Sleep } from '../../gadgets/Sleep.js'; +import { VerifyChanges } from '../../gadgets/VerifyChanges.js'; import { WriteFile } from '../../gadgets/WriteFile.js'; import { + CreatePR, + CreatePRReview, + GetPRChecks, GetPRComments, GetPRDetails, GetPRDiff, @@ -14,20 +19,112 @@ import { ReplyToReviewComment, UpdatePRComment, } from '../../gadgets/github/index.js'; +import { + AddChecklist, + CreateWorkItem, + ListWorkItems, + PMUpdateChecklistItem, + PostComment, + ReadWorkItem, + UpdateWorkItem, +} from '../../gadgets/pm/index.js'; import { Tmux } from '../../gadgets/tmux.js'; import { TodoDelete, TodoUpdateStatus, TodoUpsert } from '../../gadgets/todo/index.js'; import type { CreateBuilderOptions } from './builderFactory.js'; +import type { AgentCapabilities } from './capabilities.js'; -export function createPRAgentGadgets(options?: { +/** + * Build the standard set of gadgets for work-item-based agents. + * + * Used by both the llmist backend (agents/base.ts) and the Claude Code backend + * (backends/agent-profiles.ts) to ensure identical gadget sets for matching + * agent types regardless of which backend runs the agent. + * + * Applies capabilities to gate optional gadgets: + * - canEditFiles: file writing tools (FileSearchAndReplace, FileMultiEdit, WriteFile, VerifyChanges) + * - canCreatePR: CreatePR + * - canUpdateChecklists: PMUpdateChecklistItem + */ +export function buildWorkItemGadgets(caps: AgentCapabilities): CreateBuilderOptions['gadgets'] { + return [ + // Filesystem gadgets (read-only when canEditFiles is false) + new ListDirectory(), + new ReadFile(), + new RipGrep(), + new AstGrep(), + ...(caps.canEditFiles + ? [new FileSearchAndReplace(), new FileMultiEdit(), new WriteFile(), new VerifyChanges()] + : []), + // Shell commands via tmux (no timeout issues) + new Tmux(), + new Sleep(), + // Task tracking gadgets + new TodoUpsert(), + new TodoUpdateStatus(), + new TodoDelete(), + // GitHub gadgets (PR creation gated by capability) + ...(caps.canCreatePR ? [new CreatePR()] : []), + // PM gadgets (work items, comments, checklists — PM-agnostic) + new ReadWorkItem(), + new PostComment(), + new UpdateWorkItem(), + new CreateWorkItem(), + new ListWorkItems(), + new AddChecklist(), + // UpdateChecklistItem gated by capability — prevents planning from marking items complete + // prematurely, while respond-to-planning-comment CAN update them + ...(caps.canUpdateChecklists ? [new PMUpdateChecklistItem()] : []), + // Session control + new Finish(), + ]; +} + +/** + * Build gadgets for the review agent (read-only, PR-focused). + * + * Used by both backends — review agent sees PR details, diff, checks, and + * can submit a review, but cannot modify files or create new PRs. + */ +export function buildReviewGadgets(): CreateBuilderOptions['gadgets'] { + return [ + new ListDirectory(), + new ReadFile(), + new Tmux(), + new Sleep(), + new TodoUpsert(), + new TodoUpdateStatus(), + new TodoDelete(), + new GetPRDetails(), + new GetPRDiff(), + new GetPRChecks(), + new CreatePRReview(), + new UpdatePRComment(), + new Finish(), + ]; +} + +/** + * Build gadgets for PR-modifying agents (respond-to-review, respond-to-ci, + * respond-to-pr-comment). + * + * Includes file editing + GitHub tools but NOT CreatePR (agents push to + * existing branches). Pass includeReviewComments=true for agents that need + * GetPRComments and ReplyToReviewComment. + * + * Used by both backends to ensure identical tool sets. + */ +export function buildPRAgentGadgets(options?: { includeReviewComments?: boolean; }): CreateBuilderOptions['gadgets'] { const gadgets: CreateBuilderOptions['gadgets'] = [ new ListDirectory(), new ReadFile(), new FileSearchAndReplace(), + new FileMultiEdit(), new WriteFile(), - new RipGrep(), + new VerifyChanges(), new AstGrep(), + new RipGrep(), new Tmux(), new Sleep(), new TodoUpsert(), @@ -35,6 +132,7 @@ export function createPRAgentGadgets(options?: { new TodoDelete(), new GetPRDetails(), new GetPRDiff(), + new GetPRChecks(), new PostPRComment(), new UpdatePRComment(), new Finish(), @@ -46,3 +144,13 @@ export function createPRAgentGadgets(options?: { return gadgets; } + +/** + * @deprecated Use buildPRAgentGadgets() instead. + * Kept for backwards compatibility with existing callers. + */ +export function createPRAgentGadgets(options?: { + includeReviewComments?: boolean; +}): CreateBuilderOptions['gadgets'] { + return buildPRAgentGadgets(options); +} diff --git a/src/agents/shared/taskPrompts.ts b/src/agents/shared/taskPrompts.ts new file mode 100644 index 00000000..4d535b15 --- /dev/null +++ b/src/agents/shared/taskPrompts.ts @@ -0,0 +1,152 @@ +/** + * Shared task prompt builders used by both backends. + * + * The llmist backend (agents/base.ts) and the Claude Code backend + * (backends/agent-profiles.ts) both need task-level prompts for each agent type. + * This module is the single source of truth so the two backends produce + * identical instructions for each agent type. + */ + +// ============================================================================ +// Work-item agents +// ============================================================================ + +/** + * Standard prompt for agents whose primary task is processing a work item + * (briefing, planning, implementation, debug). + */ +export function buildWorkItemPrompt(cardId: string): string { + return `Analyze and process the work item with ID: ${cardId}. The work item data has been pre-loaded.`; +} + +/** + * Prompt for agents responding to a PM comment mentioning them. + */ +export function buildCommentResponsePrompt( + cardId: string, + commentText: string, + commentAuthor: string, +): string { + return `A user (@${commentAuthor}) mentioned you in a comment on work item ${cardId}. + +Their comment: +--- +${commentText} +--- + +The work item data (title, description, checklists, attachments, comments) has been pre-loaded above. +Read the user's comment carefully and respond accordingly. Default to surgical, targeted updates unless they clearly ask for a full rewrite.`; +} + +// ============================================================================ +// PR agents +// ============================================================================ + +/** + * Prompt for the review agent. + */ +export function buildReviewPrompt(prNumber: number): string { + return `Review PR #${prNumber}. + +Examine the code changes carefully and submit your review using CreatePRReview.`; +} + +/** + * Prompt for the respond-to-ci agent. + */ +export function buildCIResponsePrompt(prBranch: string, prNumber: number): string { + return `You are on the branch \`${prBranch}\` for PR #${prNumber}. + +CI checks have failed. Analyze the failures and fix them.`; +} + +/** + * Prompt for PR-comment-response agents (respond-to-review, respond-to-pr-comment). + */ +export function buildPRCommentResponsePrompt( + prBranch: string, + prNumber: number, + commentBody: string, + commentPath?: string, +): string { + const pathContext = commentPath ? `\nFile: ${commentPath}` : ''; + + return `You are on the branch \`${prBranch}\` for PR #${prNumber}. + +A user commented on this PR and mentioned you. Respond to their comment. +${pathContext} + +Their comment: +--- +${commentBody} +--- + +Read the comment carefully and respond accordingly. If they ask for code changes, make the changes, commit, and push. If they ask a question, reply with a PR comment. Default to surgical, targeted changes unless they clearly ask for something broader.`; +} + +/** + * Prompt for the respond-to-ci agent (llmist backend format — includes GitHub context). + * Used by agents/base.ts when the trigger type is 'check-failure'. + */ +export function buildCheckFailurePrompt(prContext: { + prNumber: number; + prBranch: string; + repoFullName: string; + headSha: string; +}): string { + const [owner, repo] = prContext.repoFullName.split('/'); + + return `You are on branch \`${prContext.prBranch}\` for PR #${prContext.prNumber}. + +Your task is to fix the failing checks and push your changes. + +## Instructions + +1. **Investigate failures**: Use Tmux to run: + \`gh run list --branch ${prContext.prBranch} --limit 5 --json databaseId,conclusion,status,workflowName\` + +2. **Get failure details**: Find failed run ID and run: + \`gh run view --log-failed\` + +3. **Analyze error types**: + - Lint errors: Run \`npm run lint\` or \`pnpm run lint\` + - Type errors: Run \`npm run typecheck\` + - Test failures: Run \`npm test\` + - Build errors: Run \`npm run build\` + +4. **Fix issues**: Make targeted fixes following existing codebase patterns + +5. **Verify locally**: Run the same checks that failed in CI before pushing + +6. **Commit and push**: + \`\`\`bash + git add . + git commit -m "fix: address failing checks" + git push + \`\`\` + +The push will re-trigger checks automatically. + +## GitHub Context +Owner: ${owner} +Repo: ${repo} +PR: #${prContext.prNumber} +Branch: ${prContext.prBranch}`; +} + +/** + * Prompt for the debug agent analyzing session logs. + */ +export function buildDebugPrompt(debugContext: { + logDir: string; + originalCardName: string; + originalCardUrl: string; + detectedAgentType: string; +}): string { + return `Analyze the ${debugContext.detectedAgentType} agent session logs in directory: ${debugContext.logDir} + +Original card: "${debugContext.originalCardName}" +Link: ${debugContext.originalCardUrl} + +Start by listing the contents of the log directory, then read and analyze the logs to identify issues.`; +} diff --git a/src/backends/agent-profiles.ts b/src/backends/agent-profiles.ts index 6716eb08..fd1cb402 100644 --- a/src/backends/agent-profiles.ts +++ b/src/backends/agent-profiles.ts @@ -2,6 +2,11 @@ import { execFileSync } from 'node:child_process'; import { type AgentCapabilities, getAgentCapabilities } from '../agents/shared/capabilities.js'; export type { AgentCapabilities } from '../agents/shared/capabilities.js'; +import { + buildPRAgentGadgets, + buildReviewGadgets, + buildWorkItemGadgets, +} from '../agents/shared/gadgets.js'; import { formatPRComments, formatPRDetails, @@ -10,41 +15,17 @@ import { formatPRReviews, readPRFileContents, } from '../agents/shared/prFormatting.js'; +import { + buildCIResponsePrompt, + buildCommentResponsePrompt, + buildPRCommentResponsePrompt, + buildReviewPrompt, + buildWorkItemPrompt, +} from '../agents/shared/taskPrompts.js'; import type { ContextFile } from '../agents/utils/setup.js'; -import { AstGrep } from '../gadgets/AstGrep.js'; -import { FileMultiEdit } from '../gadgets/FileMultiEdit.js'; -import { FileSearchAndReplace } from '../gadgets/FileSearchAndReplace.js'; -import { Finish } from '../gadgets/Finish.js'; import { ListDirectory } from '../gadgets/ListDirectory.js'; -import { ReadFile } from '../gadgets/ReadFile.js'; -import { RipGrep } from '../gadgets/RipGrep.js'; -import { Sleep } from '../gadgets/Sleep.js'; -import { VerifyChanges } from '../gadgets/VerifyChanges.js'; -import { WriteFile } from '../gadgets/WriteFile.js'; import { formatCheckStatus } from '../gadgets/github/core/getPRChecks.js'; -import { - CreatePR, - CreatePRReview, - GetPRChecks, - GetPRComments, - GetPRDetails, - GetPRDiff, - PostPRComment, - ReplyToReviewComment, - UpdatePRComment, -} from '../gadgets/github/index.js'; import { readWorkItem } from '../gadgets/pm/core/readWorkItem.js'; -import { - AddChecklist, - CreateWorkItem, - ListWorkItems, - PMUpdateChecklistItem, - PostComment, - ReadWorkItem, - UpdateWorkItem, -} from '../gadgets/pm/index.js'; -import { Tmux } from '../gadgets/tmux.js'; -import { TodoDelete, TodoUpdateStatus, TodoUpsert } from '../gadgets/todo/index.js'; import { githubClient } from '../github/client.js'; import type { AgentInput } from '../types/index.js'; import { resolveSquintDbPath } from '../utils/squintDb.js'; @@ -138,94 +119,10 @@ export interface AgentProfile { // ============================================================================ // Llmist Gadget Builders // ============================================================================ - -/** - * Build the standard set of gadgets for work-item-based agents (briefing, planning, - * implementation, debug). Mirrors the logic in agents/base.ts getBaseAgentGadgets(). - */ -function buildWorkItemLlmistGadgets(caps: AgentCapabilities): unknown[] { - return [ - // Filesystem gadgets - new ListDirectory(), - new ReadFile(), - new RipGrep(), - new AstGrep(), - ...(caps.canEditFiles - ? [new FileSearchAndReplace(), new FileMultiEdit(), new WriteFile(), new VerifyChanges()] - : []), - // Shell commands - new Tmux(), - new Sleep(), - // Task tracking - new TodoUpsert(), - new TodoUpdateStatus(), - new TodoDelete(), - // GitHub PR creation (gated by capability) - ...(caps.canCreatePR ? [new CreatePR()] : []), - // PM gadgets - new ReadWorkItem(), - new PostComment(), - new UpdateWorkItem(), - new CreateWorkItem(), - new ListWorkItems(), - new AddChecklist(), - ...(caps.canUpdateChecklists ? [new PMUpdateChecklistItem()] : []), - // Session control - new Finish(), - ]; -} - -/** - * Build gadgets for the review agent (read-only, PR-focused). - * Mirrors the authoritative gadget list in agents/review.ts getGadgets(). - */ -function buildReviewLlmistGadgets(): unknown[] { - return [ - new ListDirectory(), - new ReadFile(), - new Tmux(), - new Sleep(), - new TodoUpsert(), - new TodoUpdateStatus(), - new TodoDelete(), - new GetPRDetails(), - new GetPRDiff(), - new GetPRChecks(), - new CreatePRReview(), - new UpdatePRComment(), - new Finish(), - ]; -} - -/** - * Build gadgets for PR-modifying agents (respond-to-review, respond-to-ci, respond-to-pr-comment). - * Includes file editing + GitHub tools but NOT CreatePR (agents push to existing branches). - * Mirrors the authoritative gadget list in agents/shared/gadgets.ts createPRAgentGadgets(). - */ -function buildPRAgentLlmistGadgets(includeReviewComments = false): unknown[] { - return [ - new ListDirectory(), - new ReadFile(), - new FileSearchAndReplace(), - new FileMultiEdit(), - new WriteFile(), - new VerifyChanges(), - new AstGrep(), - new RipGrep(), - new Tmux(), - new Sleep(), - new TodoUpsert(), - new TodoUpdateStatus(), - new TodoDelete(), - new GetPRDetails(), - new GetPRDiff(), - new GetPRChecks(), - new PostPRComment(), - new UpdatePRComment(), - ...(includeReviewComments ? [new GetPRComments(), new ReplyToReviewComment()] : []), - new Finish(), - ]; -} +// All three builder functions below delegate to the shared gadget factories in +// agents/shared/gadgets.ts, which serve as the single source of truth for tool +// sets used by both the llmist backend and the Claude Code backend. +// ============================================================================ // ============================================================================ // Context Fetching Helpers @@ -238,27 +135,22 @@ function filterToolsByNames(allTools: ToolManifest[], names: string[]): ToolMani function fetchDirectoryListing(repoDir: string): ContextInjection { const listDirGadget = new ListDirectory(); + // Pass the absolute repoDir path so ListDirectory resolves correctly + // without requiring process.chdir(), which is a dangerous side effect. const params = { comment: 'Pre-fetching codebase structure for context', - directoryPath: '.', + directoryPath: repoDir, maxDepth: 3, includeGitIgnored: false, }; - // ListDirectory uses process.cwd() — we need to be in repoDir - const originalCwd = process.cwd(); - try { - process.chdir(repoDir); - const result = listDirGadget.execute(params); - return { - toolName: 'ListDirectory', - params, - result, - description: 'Pre-fetched codebase structure', - }; - } finally { - process.chdir(originalCwd); - } + const result = listDirGadget.execute(params); + return { + toolName: 'ListDirectory', + params, + result, + description: 'Pre-fetched codebase structure', + }; } function fetchContextFileInjections(contextFiles: ContextFile[]): ContextInjection[] { @@ -519,63 +411,34 @@ async function fetchPRCommentResponseContext( } // ============================================================================ -// Task Prompt Builders +// Task Prompt Builders (thin wrappers around shared/taskPrompts.ts) // ============================================================================ function buildWorkItemTaskPrompt(input: AgentInput): string { - return `Analyze and process the work item with ID: ${input.cardId || 'unknown'}. The work item data has been pre-loaded.`; + return buildWorkItemPrompt(input.cardId || 'unknown'); } function buildCommentResponseTaskPrompt(input: AgentInput): string { const commentText = input.triggerCommentText as string; const commentAuthor = (input.triggerCommentAuthor as string) || 'unknown'; - return `A user (@${commentAuthor}) mentioned you in a comment on work item ${input.cardId || 'unknown'}. - -Their comment: ---- -${commentText} ---- - -The work item data (title, description, checklists, attachments, comments) has been pre-loaded above. -Read the user's comment carefully and respond accordingly. Default to surgical, targeted updates unless they clearly ask for a full rewrite.`; + return buildCommentResponsePrompt(input.cardId || 'unknown', commentText, commentAuthor); } function buildReviewTaskPrompt(input: AgentInput): string { - const prNumber = input.prNumber as number; - - return `Review PR #${prNumber}. - -Examine the code changes carefully and submit your review using CreatePRReview.`; + return buildReviewPrompt(input.prNumber as number); } function buildCITaskPrompt(input: AgentInput): string { - const prNumber = input.prNumber as number; - const prBranch = input.prBranch as string; - - return `You are on the branch \`${prBranch}\` for PR #${prNumber}. - -CI checks have failed. Analyze the failures and fix them.`; + return buildCIResponsePrompt(input.prBranch as string, input.prNumber as number); } function buildPRCommentResponseTaskPrompt(input: AgentInput): string { - const prNumber = input.prNumber as number; - const prBranch = input.prBranch as string; - const commentBody = input.triggerCommentBody as string; - const commentPath = input.triggerCommentPath as string; - - const pathContext = commentPath ? `\nFile: ${commentPath}` : ''; - - return `You are on the branch \`${prBranch}\` for PR #${prNumber}. - -A user commented on this PR and mentioned you. Respond to their comment. -${pathContext} - -Their comment: ---- -${commentBody} ---- - -Read the comment carefully and respond accordingly. If they ask for code changes, make the changes, commit, and push. If they ask a question, reply with a PR comment. Default to surgical, targeted changes unless they clearly ask for something broader.`; + return buildPRCommentResponsePrompt( + input.prBranch as string, + input.prNumber as number, + input.triggerCommentBody as string, + (input.triggerCommentPath as string) || undefined, + ); } // ============================================================================ @@ -591,7 +454,7 @@ const briefingProfile: AgentProfile = { fetchContext: fetchWorkItemContext, buildTaskPrompt: buildWorkItemTaskPrompt, capabilities: getAgentCapabilities('briefing'), - getLlmistGadgets: (agentType) => buildWorkItemLlmistGadgets(getAgentCapabilities(agentType)), + getLlmistGadgets: (agentType) => buildWorkItemGadgets(getAgentCapabilities(agentType)), }; const planningProfile: AgentProfile = { @@ -602,7 +465,7 @@ const planningProfile: AgentProfile = { fetchContext: fetchWorkItemContext, buildTaskPrompt: buildWorkItemTaskPrompt, capabilities: getAgentCapabilities('planning'), - getLlmistGadgets: (agentType) => buildWorkItemLlmistGadgets(getAgentCapabilities(agentType)), + getLlmistGadgets: (agentType) => buildWorkItemGadgets(getAgentCapabilities(agentType)), }; const reviewProfile: AgentProfile = { @@ -613,7 +476,7 @@ const reviewProfile: AgentProfile = { fetchContext: fetchReviewContext, buildTaskPrompt: buildReviewTaskPrompt, capabilities: getAgentCapabilities('review'), - getLlmistGadgets: (_agentType) => buildReviewLlmistGadgets(), + getLlmistGadgets: (_agentType) => buildReviewGadgets(), async preExecute({ input, logWriter }: PreExecuteParams): Promise { const repoFullName = input.repoFullName as string; @@ -634,7 +497,7 @@ const respondToPlanningCommentProfile: AgentProfile = { fetchContext: fetchWorkItemContext, buildTaskPrompt: buildCommentResponseTaskPrompt, capabilities: getAgentCapabilities('respond-to-planning-comment'), - getLlmistGadgets: (agentType) => buildWorkItemLlmistGadgets(getAgentCapabilities(agentType)), + getLlmistGadgets: (agentType) => buildWorkItemGadgets(getAgentCapabilities(agentType)), }; const respondToCIProfile: AgentProfile = { @@ -652,7 +515,7 @@ const respondToCIProfile: AgentProfile = { fetchContext: fetchCIContext, buildTaskPrompt: buildCITaskPrompt, capabilities: getAgentCapabilities('respond-to-ci'), - getLlmistGadgets: (_agentType) => buildPRAgentLlmistGadgets(), + getLlmistGadgets: (_agentType) => buildPRAgentGadgets(), async preExecute({ input, logWriter }: PreExecuteParams): Promise { const repoFullName = input.repoFullName as string; @@ -678,7 +541,7 @@ const respondToReviewProfile: AgentProfile = { fetchContext: fetchPRCommentResponseContext, buildTaskPrompt: buildPRCommentResponseTaskPrompt, capabilities: getAgentCapabilities('respond-to-review'), - getLlmistGadgets: (_agentType) => buildPRAgentLlmistGadgets(true), + getLlmistGadgets: (_agentType) => buildPRAgentGadgets({ includeReviewComments: true }), }; const respondToPRCommentProfile: AgentProfile = { @@ -690,7 +553,7 @@ const respondToPRCommentProfile: AgentProfile = { fetchContext: fetchPRCommentResponseContext, buildTaskPrompt: buildPRCommentResponseTaskPrompt, capabilities: getAgentCapabilities('respond-to-pr-comment'), - getLlmistGadgets: (_agentType) => buildPRAgentLlmistGadgets(true), + getLlmistGadgets: (_agentType) => buildPRAgentGadgets({ includeReviewComments: true }), }; const defaultProfile: AgentProfile = { @@ -701,14 +564,14 @@ const defaultProfile: AgentProfile = { fetchContext: fetchWorkItemContext, buildTaskPrompt: buildWorkItemTaskPrompt, capabilities: getAgentCapabilities('debug'), - getLlmistGadgets: (agentType) => buildWorkItemLlmistGadgets(getAgentCapabilities(agentType)), + getLlmistGadgets: (agentType) => buildWorkItemGadgets(getAgentCapabilities(agentType)), }; const implementationProfile: AgentProfile = { ...defaultProfile, needsGitHubToken: true, capabilities: getAgentCapabilities('implementation'), - getLlmistGadgets: (agentType) => buildWorkItemLlmistGadgets(getAgentCapabilities(agentType)), + getLlmistGadgets: (agentType) => buildWorkItemGadgets(getAgentCapabilities(agentType)), }; // ============================================================================ diff --git a/tests/unit/agents/shared/gadgets.test.ts b/tests/unit/agents/shared/gadgets.test.ts new file mode 100644 index 00000000..1f00d901 --- /dev/null +++ b/tests/unit/agents/shared/gadgets.test.ts @@ -0,0 +1,206 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +/** Create a mock class with the given name so constructor.name works in assertions */ +function mockClass(name: string) { + const cls = { [name]: class {} }[name]; + return vi.fn().mockImplementation(() => new cls()); +} + +vi.mock('../../../../src/gadgets/AstGrep.js', () => ({ AstGrep: mockClass('AstGrep') })); +vi.mock('../../../../src/gadgets/FileMultiEdit.js', () => ({ + FileMultiEdit: mockClass('FileMultiEdit'), +})); +vi.mock('../../../../src/gadgets/FileSearchAndReplace.js', () => ({ + FileSearchAndReplace: mockClass('FileSearchAndReplace'), +})); +vi.mock('../../../../src/gadgets/Finish.js', () => ({ Finish: mockClass('Finish') })); +vi.mock('../../../../src/gadgets/ListDirectory.js', () => ({ + ListDirectory: mockClass('ListDirectory'), +})); +vi.mock('../../../../src/gadgets/ReadFile.js', () => ({ ReadFile: mockClass('ReadFile') })); +vi.mock('../../../../src/gadgets/RipGrep.js', () => ({ RipGrep: mockClass('RipGrep') })); +vi.mock('../../../../src/gadgets/Sleep.js', () => ({ Sleep: mockClass('Sleep') })); +vi.mock('../../../../src/gadgets/VerifyChanges.js', () => ({ + VerifyChanges: mockClass('VerifyChanges'), +})); +vi.mock('../../../../src/gadgets/WriteFile.js', () => ({ WriteFile: mockClass('WriteFile') })); +vi.mock('../../../../src/gadgets/github/index.js', () => ({ + CreatePR: mockClass('CreatePR'), + CreatePRReview: mockClass('CreatePRReview'), + GetPRChecks: mockClass('GetPRChecks'), + GetPRComments: mockClass('GetPRComments'), + GetPRDetails: mockClass('GetPRDetails'), + GetPRDiff: mockClass('GetPRDiff'), + PostPRComment: mockClass('PostPRComment'), + ReplyToReviewComment: mockClass('ReplyToReviewComment'), + UpdatePRComment: mockClass('UpdatePRComment'), +})); +vi.mock('../../../../src/gadgets/pm/index.js', () => ({ + AddChecklist: mockClass('AddChecklist'), + CreateWorkItem: mockClass('CreateWorkItem'), + ListWorkItems: mockClass('ListWorkItems'), + PMUpdateChecklistItem: mockClass('PMUpdateChecklistItem'), + PostComment: mockClass('PostComment'), + ReadWorkItem: mockClass('ReadWorkItem'), + UpdateWorkItem: mockClass('UpdateWorkItem'), +})); +vi.mock('../../../../src/gadgets/tmux.js', () => ({ Tmux: mockClass('Tmux') })); +vi.mock('../../../../src/gadgets/todo/index.js', () => ({ + TodoUpsert: mockClass('TodoUpsert'), + TodoUpdateStatus: mockClass('TodoUpdateStatus'), + TodoDelete: mockClass('TodoDelete'), +})); + +import type { AgentCapabilities } from '../../../../src/agents/shared/capabilities.js'; +import { + buildPRAgentGadgets, + buildReviewGadgets, + buildWorkItemGadgets, + createPRAgentGadgets, +} from '../../../../src/agents/shared/gadgets.js'; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +function names(gadgets: unknown[]): string[] { + return gadgets.map((g) => (g as object).constructor.name); +} + +const FULL_CAPS: AgentCapabilities = { + canEditFiles: true, + canCreatePR: true, + canUpdateChecklists: true, + isReadOnly: false, +}; + +const READ_ONLY_CAPS: AgentCapabilities = { + canEditFiles: false, + canCreatePR: false, + canUpdateChecklists: false, + isReadOnly: true, +}; + +describe('buildWorkItemGadgets', () => { + it('always includes base read gadgets and session control', () => { + const gadgets = names(buildWorkItemGadgets(FULL_CAPS)); + expect(gadgets).toContain('ListDirectory'); + expect(gadgets).toContain('ReadFile'); + expect(gadgets).toContain('RipGrep'); + expect(gadgets).toContain('AstGrep'); + expect(gadgets).toContain('Tmux'); + expect(gadgets).toContain('Sleep'); + expect(gadgets).toContain('TodoUpsert'); + expect(gadgets).toContain('TodoUpdateStatus'); + expect(gadgets).toContain('TodoDelete'); + expect(gadgets).toContain('ReadWorkItem'); + expect(gadgets).toContain('PostComment'); + expect(gadgets).toContain('Finish'); + }); + + it('includes file-editing gadgets when canEditFiles is true', () => { + const gadgets = names(buildWorkItemGadgets(FULL_CAPS)); + expect(gadgets).toContain('FileSearchAndReplace'); + expect(gadgets).toContain('FileMultiEdit'); + expect(gadgets).toContain('WriteFile'); + expect(gadgets).toContain('VerifyChanges'); + }); + + it('excludes file-editing gadgets when canEditFiles is false', () => { + const gadgets = names(buildWorkItemGadgets(READ_ONLY_CAPS)); + expect(gadgets).not.toContain('FileSearchAndReplace'); + expect(gadgets).not.toContain('FileMultiEdit'); + expect(gadgets).not.toContain('WriteFile'); + expect(gadgets).not.toContain('VerifyChanges'); + }); + + it('includes CreatePR when canCreatePR is true', () => { + const gadgets = names(buildWorkItemGadgets(FULL_CAPS)); + expect(gadgets).toContain('CreatePR'); + }); + + it('excludes CreatePR when canCreatePR is false', () => { + const gadgets = names(buildWorkItemGadgets(READ_ONLY_CAPS)); + expect(gadgets).not.toContain('CreatePR'); + }); + + it('includes PMUpdateChecklistItem when canUpdateChecklists is true', () => { + const gadgets = names(buildWorkItemGadgets(FULL_CAPS)); + expect(gadgets).toContain('PMUpdateChecklistItem'); + }); + + it('excludes PMUpdateChecklistItem when canUpdateChecklists is false', () => { + const gadgets = names(buildWorkItemGadgets(READ_ONLY_CAPS)); + expect(gadgets).not.toContain('PMUpdateChecklistItem'); + }); +}); + +describe('buildReviewGadgets', () => { + it('includes PR review gadgets', () => { + const gadgets = names(buildReviewGadgets()); + expect(gadgets).toContain('GetPRDetails'); + expect(gadgets).toContain('GetPRDiff'); + expect(gadgets).toContain('GetPRChecks'); + expect(gadgets).toContain('CreatePRReview'); + expect(gadgets).toContain('UpdatePRComment'); + expect(gadgets).toContain('Finish'); + }); + + it('does not include file-editing gadgets (read-only)', () => { + const gadgets = names(buildReviewGadgets()); + expect(gadgets).not.toContain('FileSearchAndReplace'); + expect(gadgets).not.toContain('WriteFile'); + expect(gadgets).not.toContain('CreatePR'); + }); + + it('does not include PostPRComment (submits via CreatePRReview)', () => { + const gadgets = names(buildReviewGadgets()); + expect(gadgets).not.toContain('PostPRComment'); + }); +}); + +describe('buildPRAgentGadgets', () => { + it('includes file editing and GitHub PR tools', () => { + const gadgets = names(buildPRAgentGadgets()); + expect(gadgets).toContain('FileSearchAndReplace'); + expect(gadgets).toContain('FileMultiEdit'); + expect(gadgets).toContain('WriteFile'); + expect(gadgets).toContain('VerifyChanges'); + expect(gadgets).toContain('GetPRDetails'); + expect(gadgets).toContain('GetPRDiff'); + expect(gadgets).toContain('GetPRChecks'); + expect(gadgets).toContain('PostPRComment'); + expect(gadgets).toContain('Finish'); + }); + + it('does not include CreatePR (pushes to existing branch)', () => { + const gadgets = names(buildPRAgentGadgets()); + expect(gadgets).not.toContain('CreatePR'); + }); + + it('excludes review comment tools by default', () => { + const gadgets = names(buildPRAgentGadgets()); + expect(gadgets).not.toContain('GetPRComments'); + expect(gadgets).not.toContain('ReplyToReviewComment'); + }); + + it('includes review comment tools when includeReviewComments is true', () => { + const gadgets = names(buildPRAgentGadgets({ includeReviewComments: true })); + expect(gadgets).toContain('GetPRComments'); + expect(gadgets).toContain('ReplyToReviewComment'); + }); +}); + +describe('createPRAgentGadgets (deprecated alias)', () => { + it('delegates to buildPRAgentGadgets', () => { + const via_new = names(buildPRAgentGadgets()); + const via_old = names(createPRAgentGadgets()); + expect(via_old).toEqual(via_new); + }); + + it('passes includeReviewComments through', () => { + const with_reviews = names(createPRAgentGadgets({ includeReviewComments: true })); + expect(with_reviews).toContain('GetPRComments'); + expect(with_reviews).toContain('ReplyToReviewComment'); + }); +}); diff --git a/tests/unit/agents/shared/taskPrompts.test.ts b/tests/unit/agents/shared/taskPrompts.test.ts new file mode 100644 index 00000000..150b3cf4 --- /dev/null +++ b/tests/unit/agents/shared/taskPrompts.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, it } from 'vitest'; + +import { + buildCIResponsePrompt, + buildCheckFailurePrompt, + buildCommentResponsePrompt, + buildDebugPrompt, + buildPRCommentResponsePrompt, + buildReviewPrompt, + buildWorkItemPrompt, +} from '../../../../src/agents/shared/taskPrompts.js'; + +describe('buildWorkItemPrompt', () => { + it('includes the card ID', () => { + const prompt = buildWorkItemPrompt('abc123'); + expect(prompt).toContain('abc123'); + }); + + it('asks the agent to process the work item', () => { + const prompt = buildWorkItemPrompt('card-99'); + expect(prompt).toContain('work item'); + }); +}); + +describe('buildCommentResponsePrompt', () => { + it('includes card ID, comment text, and author', () => { + const prompt = buildCommentResponsePrompt('card-42', 'Please add tests', 'alice'); + expect(prompt).toContain('card-42'); + expect(prompt).toContain('Please add tests'); + expect(prompt).toContain('@alice'); + }); + + it('instructs surgical updates by default', () => { + const prompt = buildCommentResponsePrompt('card-1', 'Fix the typo', 'bob'); + expect(prompt).toContain('surgical'); + }); + + it('mentions that work item data is pre-loaded', () => { + const prompt = buildCommentResponsePrompt('card-1', 'Update docs', 'carol'); + expect(prompt).toContain('pre-loaded'); + }); +}); + +describe('buildReviewPrompt', () => { + it('includes the PR number', () => { + const prompt = buildReviewPrompt(42); + expect(prompt).toContain('PR #42'); + }); + + it('instructs to use CreatePRReview', () => { + const prompt = buildReviewPrompt(7); + expect(prompt).toContain('CreatePRReview'); + }); +}); + +describe('buildCIResponsePrompt', () => { + it('includes branch and PR number', () => { + const prompt = buildCIResponsePrompt('fix/ci-errors', 99); + expect(prompt).toContain('fix/ci-errors'); + expect(prompt).toContain('PR #99'); + }); + + it('mentions CI checks have failed', () => { + const prompt = buildCIResponsePrompt('main', 1); + expect(prompt).toContain('CI checks have failed'); + }); +}); + +describe('buildPRCommentResponsePrompt', () => { + it('includes PR number, branch, and comment body', () => { + const prompt = buildPRCommentResponsePrompt('feat/new', 55, 'Can you fix the typo?'); + expect(prompt).toContain('PR #55'); + expect(prompt).toContain('feat/new'); + expect(prompt).toContain('Can you fix the typo?'); + }); + + it('includes file path when provided', () => { + const prompt = buildPRCommentResponsePrompt('feat/new', 55, 'Fix this line', 'src/utils.ts'); + expect(prompt).toContain('src/utils.ts'); + }); + + it('omits file path when not provided', () => { + const prompt = buildPRCommentResponsePrompt('feat/new', 55, 'Looks good overall!'); + expect(prompt).not.toContain('File:'); + }); + + it('omits file path when empty string provided', () => { + const prompt = buildPRCommentResponsePrompt('feat/new', 55, 'LGTM', ''); + expect(prompt).not.toContain('File:'); + }); + + it('instructs surgical changes by default', () => { + const prompt = buildPRCommentResponsePrompt('main', 1, 'Please refactor'); + expect(prompt).toContain('surgical'); + }); +}); + +describe('buildCheckFailurePrompt', () => { + const prContext = { + prNumber: 33, + prBranch: 'fix/flaky-test', + repoFullName: 'acme/widgets', + headSha: 'abc123', + }; + + it('includes PR number and branch', () => { + const prompt = buildCheckFailurePrompt(prContext); + expect(prompt).toContain('PR #33'); + expect(prompt).toContain('fix/flaky-test'); + }); + + it('includes owner and repo from repoFullName', () => { + const prompt = buildCheckFailurePrompt(prContext); + expect(prompt).toContain('acme'); + expect(prompt).toContain('widgets'); + }); + + it('provides investigation steps', () => { + const prompt = buildCheckFailurePrompt(prContext); + expect(prompt).toContain('gh run list'); + expect(prompt).toContain('gh run view'); + }); +}); + +describe('buildDebugPrompt', () => { + const debugContext = { + logDir: '/tmp/logs/abc', + originalCardName: 'Fix the login bug', + originalCardUrl: 'https://trello.com/c/abc', + detectedAgentType: 'implementation', + }; + + it('includes the log directory', () => { + const prompt = buildDebugPrompt(debugContext); + expect(prompt).toContain('/tmp/logs/abc'); + }); + + it('includes the original card name', () => { + const prompt = buildDebugPrompt(debugContext); + expect(prompt).toContain('Fix the login bug'); + }); + + it('includes the detected agent type', () => { + const prompt = buildDebugPrompt(debugContext); + expect(prompt).toContain('implementation'); + }); +}); From 39274fe9198e42ad12ce383e5c216cf59068d76c Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Fri, 20 Feb 2026 10:14:49 +0000 Subject: [PATCH 02/10] test: add coverage for router, gadgets, agents, utils, and API clients --- .../unit/agents/shared/promptContext.test.ts | 282 +++++++++++++++ tests/unit/gadgets/fileInsertContent.test.ts | 238 +++++++++++++ tests/unit/gadgets/fileRemoveContent.test.ts | 240 +++++++++++++ .../gadgets/shared/diagnosticState.test.ts | 324 ++++++++++++++++++ tests/unit/jira/client.test.ts | 304 +++++++++++++++- tests/unit/router/config.test.ts | 221 ++++++++++++ tests/unit/router/index.test.ts | 57 +++ tests/unit/trello/client.test.ts | 258 +++++++++++++- tests/unit/utils/envScrub.test.ts | 116 +++++++ 9 files changed, 2018 insertions(+), 22 deletions(-) create mode 100644 tests/unit/agents/shared/promptContext.test.ts create mode 100644 tests/unit/gadgets/fileInsertContent.test.ts create mode 100644 tests/unit/gadgets/fileRemoveContent.test.ts create mode 100644 tests/unit/gadgets/shared/diagnosticState.test.ts create mode 100644 tests/unit/router/config.test.ts create mode 100644 tests/unit/router/index.test.ts create mode 100644 tests/unit/utils/envScrub.test.ts diff --git a/tests/unit/agents/shared/promptContext.test.ts b/tests/unit/agents/shared/promptContext.test.ts new file mode 100644 index 00000000..024767a2 --- /dev/null +++ b/tests/unit/agents/shared/promptContext.test.ts @@ -0,0 +1,282 @@ +import { describe, expect, it, vi } from 'vitest'; + +// Mock getPMProvider to control the PM type +vi.mock('../../../../src/pm/index.js', () => ({ + getPMProvider: vi.fn(), +})); + +import { buildPromptContext } from '../../../../src/agents/shared/promptContext.js'; +import { getPMProvider } from '../../../../src/pm/index.js'; +import { createMockPMProvider } from '../../../helpers/mockPMProvider.js'; + +const mockGetPMProvider = vi.mocked(getPMProvider); + +function makeProject(overrides: Record = {}) { + return { + id: 'test-project', + name: 'Test Project', + repo: 'owner/repo', + orgId: 'org1', + baseBranch: 'main', + branchPrefix: 'cascade/', + trello: { + boardId: 'board1', + lists: { + briefing: 'list1', + planning: 'list2', + todo: 'list3', + stories: 'list-stories', + debug: 'list-debug', + }, + labels: { readyToProcess: 'label1', processed: 'label2' }, + }, + ...overrides, + }; +} + +describe('buildPromptContext', () => { + describe('with Trello provider', () => { + beforeEach(() => { + const mockProvider = createMockPMProvider(); + mockProvider.type = 'trello'; + mockProvider.getWorkItemUrl = vi.fn((id: string) => `https://trello.com/c/${id}`); + mockGetPMProvider.mockReturnValue(mockProvider); + }); + + it('sets workItemNoun to "card" for Trello', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.workItemNoun).toBe('card'); + }); + + it('sets workItemNounPlural to "cards" for Trello', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.workItemNounPlural).toBe('cards'); + }); + + it('sets workItemNounCap to "Card" for Trello', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.workItemNounCap).toBe('Card'); + }); + + it('sets workItemNounPluralCap to "Cards" for Trello', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.workItemNounPluralCap).toBe('Cards'); + }); + + it('sets pmName to "Trello"', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.pmName).toBe('Trello'); + }); + + it('sets pmType to "trello"', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.pmType).toBe('trello'); + }); + + it('generates cardUrl from provider', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.cardUrl).toBe('https://trello.com/c/card123'); + }); + + it('sets cardId from parameter', () => { + const ctx = buildPromptContext('card-abc', makeProject() as never); + expect(ctx.cardId).toBe('card-abc'); + }); + + it('includes storiesListId from project trello config', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.storiesListId).toBe('list-stories'); + }); + + it('includes processedLabelId from project trello config', () => { + const ctx = buildPromptContext('card123', makeProject() as never); + expect(ctx.processedLabelId).toBe('label2'); + }); + }); + + describe('with JIRA provider', () => { + beforeEach(() => { + const mockProvider = createMockPMProvider(); + mockProvider.type = 'jira' as never; + mockProvider.getWorkItemUrl = vi.fn( + (id: string) => `https://company.atlassian.net/browse/${id}`, + ); + mockGetPMProvider.mockReturnValue(mockProvider); + }); + + it('sets workItemNoun to "issue" for JIRA', () => { + const ctx = buildPromptContext('PROJ-123', makeProject() as never); + expect(ctx.workItemNoun).toBe('issue'); + }); + + it('sets workItemNounPlural to "issues" for JIRA', () => { + const ctx = buildPromptContext('PROJ-123', makeProject() as never); + expect(ctx.workItemNounPlural).toBe('issues'); + }); + + it('sets workItemNounCap to "Issue" for JIRA', () => { + const ctx = buildPromptContext('PROJ-123', makeProject() as never); + expect(ctx.workItemNounCap).toBe('Issue'); + }); + + it('sets workItemNounPluralCap to "Issues" for JIRA', () => { + const ctx = buildPromptContext('PROJ-123', makeProject() as never); + expect(ctx.workItemNounPluralCap).toBe('Issues'); + }); + + it('sets pmName to "JIRA"', () => { + const ctx = buildPromptContext('PROJ-123', makeProject() as never); + expect(ctx.pmName).toBe('JIRA'); + }); + + it('sets pmType to "jira"', () => { + const ctx = buildPromptContext('PROJ-123', makeProject() as never); + expect(ctx.pmType).toBe('jira'); + }); + }); + + describe('with prContext', () => { + beforeEach(() => { + const mockProvider = createMockPMProvider(); + mockProvider.getWorkItemUrl = vi.fn((id: string) => `https://trello.com/c/${id}`); + mockGetPMProvider.mockReturnValue(mockProvider); + }); + + const prContext = { + prNumber: 42, + prBranch: 'feature/my-branch', + repoFullName: 'owner/repo', + headSha: 'abc123def456', + }; + + it('includes prNumber', () => { + const ctx = buildPromptContext('card1', makeProject() as never, 'check_suite', prContext); + expect(ctx.prNumber).toBe(42); + }); + + it('includes prBranch', () => { + const ctx = buildPromptContext('card1', makeProject() as never, 'check_suite', prContext); + expect(ctx.prBranch).toBe('feature/my-branch'); + }); + + it('includes repoFullName', () => { + const ctx = buildPromptContext('card1', makeProject() as never, 'check_suite', prContext); + expect(ctx.repoFullName).toBe('owner/repo'); + }); + + it('includes headSha', () => { + const ctx = buildPromptContext('card1', makeProject() as never, 'check_suite', prContext); + expect(ctx.headSha).toBe('abc123def456'); + }); + + it('includes triggerType', () => { + const ctx = buildPromptContext('card1', makeProject() as never, 'check_suite', prContext); + expect(ctx.triggerType).toBe('check_suite'); + }); + }); + + describe('with debugContext', () => { + beforeEach(() => { + const mockProvider = createMockPMProvider(); + mockProvider.getWorkItemUrl = vi.fn((id: string) => `https://trello.com/c/${id}`); + mockGetPMProvider.mockReturnValue(mockProvider); + }); + + const debugContext = { + logDir: '/tmp/logs/debug-session', + originalCardId: 'original-card-id', + originalCardName: 'My Feature Card', + originalCardUrl: 'https://trello.com/c/abc', + detectedAgentType: 'implementation', + }; + + it('includes logDir', () => { + const ctx = buildPromptContext( + undefined, + makeProject() as never, + undefined, + undefined, + debugContext, + ); + expect(ctx.logDir).toBe('/tmp/logs/debug-session'); + }); + + it('includes originalCardName', () => { + const ctx = buildPromptContext( + undefined, + makeProject() as never, + undefined, + undefined, + debugContext, + ); + expect(ctx.originalCardName).toBe('My Feature Card'); + }); + + it('includes originalCardUrl', () => { + const ctx = buildPromptContext( + undefined, + makeProject() as never, + undefined, + undefined, + debugContext, + ); + expect(ctx.originalCardUrl).toBe('https://trello.com/c/abc'); + }); + + it('includes detectedAgentType', () => { + const ctx = buildPromptContext( + undefined, + makeProject() as never, + undefined, + undefined, + debugContext, + ); + expect(ctx.detectedAgentType).toBe('implementation'); + }); + + it('includes debugListId from project trello config', () => { + const ctx = buildPromptContext( + undefined, + makeProject() as never, + undefined, + undefined, + debugContext, + ); + expect(ctx.debugListId).toBe('list-debug'); + }); + }); + + describe('without optional contexts', () => { + beforeEach(() => { + const mockProvider = createMockPMProvider(); + mockProvider.getWorkItemUrl = vi.fn(() => undefined); + mockGetPMProvider.mockReturnValue(mockProvider); + }); + + it('has undefined prNumber when no prContext', () => { + const ctx = buildPromptContext('card1', makeProject() as never); + expect(ctx.prNumber).toBeUndefined(); + }); + + it('has undefined logDir when no debugContext', () => { + const ctx = buildPromptContext('card1', makeProject() as never); + expect(ctx.logDir).toBeUndefined(); + }); + + it('handles undefined cardId', () => { + const ctx = buildPromptContext(undefined, makeProject() as never); + expect(ctx.cardId).toBeUndefined(); + expect(ctx.cardUrl).toBeUndefined(); + }); + + it('includes projectId from project', () => { + const ctx = buildPromptContext('card1', makeProject() as never); + expect(ctx.projectId).toBe('test-project'); + }); + + it('includes baseBranch from project', () => { + const ctx = buildPromptContext('card1', makeProject() as never); + expect(ctx.baseBranch).toBe('main'); + }); + }); +}); diff --git a/tests/unit/gadgets/fileInsertContent.test.ts b/tests/unit/gadgets/fileInsertContent.test.ts new file mode 100644 index 00000000..01d21bb7 --- /dev/null +++ b/tests/unit/gadgets/fileInsertContent.test.ts @@ -0,0 +1,238 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock readTracking so we don't have to pre-mark files +vi.mock('../../../src/gadgets/readTracking.js', () => ({ + assertFileRead: vi.fn(), // No-op — skip read guard + markFileRead: vi.fn(), + hasReadFile: vi.fn().mockReturnValue(true), + clearReadTracking: vi.fn(), + invalidateFileRead: vi.fn(), + hasListedDirectory: vi.fn().mockReturnValue(false), + markDirectoryListed: vi.fn(), +})); + +// Mock post-edit checks to avoid running tsc/biome +vi.mock('../../../src/gadgets/shared/postEditChecks.js', () => ({ + runPostEditChecks: vi.fn().mockReturnValue(null), +})); + +// Mock diagnosticState to avoid side effects +vi.mock('../../../src/gadgets/shared/diagnosticState.js', () => ({ + updateDiagnosticState: vi.fn(), + formatDiagnosticStatus: vi + .fn() + .mockReturnValue('## Diagnostic Status\n\n✅ All edited files pass type checking'), + runDiagnosticsWithTracking: vi.fn().mockReturnValue(null), + clearDiagnosticState: vi.fn(), + trackModifiedFile: vi.fn(), + getModifiedFiles: vi.fn().mockReturnValue([]), + clearModifiedFiles: vi.fn(), + recordEditFailure: vi.fn().mockReturnValue(1), + clearEditFailure: vi.fn(), + clearEditFailures: vi.fn(), + recordDiagnosticLoop: vi.fn().mockReturnValue(1), + clearDiagnosticLoop: vi.fn(), + getDiagnosticLoopFiles: vi.fn().mockReturnValue(new Map()), + hasAnyDiagnosticErrors: vi.fn().mockReturnValue(false), + getFilesWithErrors: vi.fn().mockReturnValue([]), +})); + +import { FileInsertContent } from '../../../src/gadgets/FileInsertContent.js'; + +let tmpDir: string; +let gadget: FileInsertContent; + +beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), 'cascade-test-insert-')); + gadget = new FileInsertContent(); +}); + +afterEach(() => { + rmSync(tmpDir, { recursive: true, force: true }); + vi.clearAllMocks(); +}); + +function createFile(name: string, content: string): string { + const filePath = join(tmpDir, name); + writeFileSync(filePath, content, 'utf-8'); + return filePath; +} + +describe('FileInsertContent', () => { + describe('insert before line', () => { + it('inserts content before line 1 (prepend)', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + line: 1, + mode: 'before', + content: 'newline', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('newline\nline1\nline2\nline3\n'); + expect(result).toContain('Inserted 1 line before line 1'); + }); + + it('inserts content before a middle line', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + gadget.execute({ + comment: 'test', + filePath, + line: 2, + mode: 'before', + content: 'inserted', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line1\ninserted\nline2\nline3\n'); + }); + + it('appends at end when line exceeds file length with mode=before', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + line: 999, + mode: 'before', + content: 'appended', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toContain('appended'); + expect(result).toContain('Appended'); + }); + + it('inserts multiline content before a line', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + gadget.execute({ + comment: 'test', + filePath, + line: 2, + mode: 'before', + content: 'newA\nnewB', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line1\nnewA\nnewB\nline2\n'); + }); + }); + + describe('insert after line', () => { + it('inserts content after line 1', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + gadget.execute({ + comment: 'test', + filePath, + line: 1, + mode: 'after', + content: 'inserted', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line1\ninserted\nline2\nline3\n'); + }); + + it('appends at end when line >= line count', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + line: 100, + mode: 'after', + content: 'appended', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toContain('appended'); + expect(result).toContain('Appended'); + }); + + it('inserts multiline content after a line', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + gadget.execute({ + comment: 'test', + filePath, + line: 1, + mode: 'after', + content: 'newA\nnewB', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line1\nnewA\nnewB\nline2\nline3\n'); + }); + + it('returns output with status=success for non-TS files', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + line: 1, + mode: 'after', + content: 'new content', + }); + + expect(result).toContain('status=success'); + }); + }); + + describe('output format', () => { + it('output includes the file path', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + line: 1, + mode: 'after', + content: 'new', + }); + + expect(result).toContain(`path=${filePath}`); + }); + + it('output contains context lines around the insertion point', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + line: 2, + mode: 'after', + content: 'inserted', + }); + + expect(result).toContain('inserted'); + }); + }); + + describe('new file creation', () => { + it('creates a new file when it does not exist', () => { + const filePath = join(tmpDir, 'newfile.txt'); + + const result = gadget.execute({ + comment: 'test', + filePath, + line: 0, + mode: 'before', + content: 'first line', + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toContain('first line'); + expect(result).toContain('status=success'); + }); + }); +}); diff --git a/tests/unit/gadgets/fileRemoveContent.test.ts b/tests/unit/gadgets/fileRemoveContent.test.ts new file mode 100644 index 00000000..e71c17f8 --- /dev/null +++ b/tests/unit/gadgets/fileRemoveContent.test.ts @@ -0,0 +1,240 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock readTracking so we don't have to pre-mark files +vi.mock('../../../src/gadgets/readTracking.js', () => ({ + assertFileRead: vi.fn(), // No-op — skip read guard + markFileRead: vi.fn(), + hasReadFile: vi.fn().mockReturnValue(true), + clearReadTracking: vi.fn(), + invalidateFileRead: vi.fn(), + hasListedDirectory: vi.fn().mockReturnValue(false), + markDirectoryListed: vi.fn(), +})); + +// Mock post-edit checks to avoid running tsc/biome +vi.mock('../../../src/gadgets/shared/postEditChecks.js', () => ({ + runPostEditChecks: vi.fn().mockReturnValue(null), +})); + +// Mock diagnosticState to avoid side effects +vi.mock('../../../src/gadgets/shared/diagnosticState.js', () => ({ + updateDiagnosticState: vi.fn(), + formatDiagnosticStatus: vi + .fn() + .mockReturnValue('## Diagnostic Status\n\n✅ All edited files pass type checking'), + runDiagnosticsWithTracking: vi.fn().mockReturnValue(null), + clearDiagnosticState: vi.fn(), + trackModifiedFile: vi.fn(), + getModifiedFiles: vi.fn().mockReturnValue([]), + clearModifiedFiles: vi.fn(), + recordEditFailure: vi.fn().mockReturnValue(1), + clearEditFailure: vi.fn(), + clearEditFailures: vi.fn(), + recordDiagnosticLoop: vi.fn().mockReturnValue(1), + clearDiagnosticLoop: vi.fn(), + getDiagnosticLoopFiles: vi.fn().mockReturnValue(new Map()), + hasAnyDiagnosticErrors: vi.fn().mockReturnValue(false), + getFilesWithErrors: vi.fn().mockReturnValue([]), +})); + +import { FileRemoveContent } from '../../../src/gadgets/FileRemoveContent.js'; + +let tmpDir: string; +let gadget: FileRemoveContent; + +beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), 'cascade-test-remove-')); + gadget = new FileRemoveContent(); +}); + +afterEach(() => { + rmSync(tmpDir, { recursive: true, force: true }); + vi.clearAllMocks(); +}); + +function createFile(name: string, content: string): string { + const filePath = join(tmpDir, name); + writeFileSync(filePath, content, 'utf-8'); + return filePath; +} + +describe('FileRemoveContent', () => { + describe('remove single line', () => { + it('removes a single line from the file', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + startLine: 2, + endLine: 2, + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line1\nline3\n'); + expect(result).toContain('Removed 1 line (line 2)'); + }); + + it('removes the first line', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + gadget.execute({ + comment: 'test', + filePath, + startLine: 1, + endLine: 1, + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line2\nline3\n'); + }); + + it('removes the last line', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3'); + + gadget.execute({ + comment: 'test', + filePath, + startLine: 3, + endLine: 3, + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line1\nline2'); + }); + }); + + describe('remove range of lines', () => { + it('removes a range of lines', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\nline4\nline5\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + startLine: 2, + endLine: 4, + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe('line1\nline5\n'); + expect(result).toContain('Removed 3 lines (lines 2-4)'); + }); + + it('removes all lines when range covers entire file', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + gadget.execute({ + comment: 'test', + filePath, + startLine: 1, + endLine: 2, + }); + + const written = readFileSync(filePath, 'utf-8'); + expect(written).toBe(''); + }); + + it('clamps endLine to file length when exceeding', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + startLine: 2, + endLine: 100, + }); + + const written = readFileSync(filePath, 'utf-8'); + // After removing lines 2+ from 'line1\nline2\nline3\n', only line1 remains + expect(written).toContain('line1'); + expect(written).not.toContain('line2'); + expect(result).toContain('Removed'); + }); + }); + + describe('output format', () => { + it('output includes BEFORE section', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + startLine: 2, + endLine: 2, + }); + + expect(result).toContain('--- BEFORE ---'); + }); + + it('output includes AFTER section', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + startLine: 2, + endLine: 2, + }); + + expect(result).toContain('--- AFTER ---'); + }); + + it('output includes file path and status', () => { + const filePath = createFile('test.txt', 'line1\nline2\nline3\n'); + + const result = gadget.execute({ + comment: 'test', + filePath, + startLine: 1, + endLine: 1, + }); + + expect(result).toContain(`path=${filePath}`); + expect(result).toContain('status=success'); + }); + }); + + describe('error cases', () => { + it('throws when startLine > endLine', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + expect(() => + gadget.execute({ + comment: 'test', + filePath, + startLine: 5, + endLine: 2, + }), + ).toThrow('Invalid line range'); + }); + + it('throws when startLine is beyond end of file', () => { + const filePath = createFile('test.txt', 'line1\nline2\n'); + + expect(() => + gadget.execute({ + comment: 'test', + filePath, + startLine: 10, + endLine: 12, + }), + ).toThrow('beyond end of file'); + }); + + it('throws when file does not exist', () => { + const filePath = join(tmpDir, 'nonexistent.txt'); + + expect(() => + gadget.execute({ + comment: 'test', + filePath, + startLine: 1, + endLine: 1, + }), + ).toThrow('File not found'); + }); + }); +}); diff --git a/tests/unit/gadgets/shared/diagnosticState.test.ts b/tests/unit/gadgets/shared/diagnosticState.test.ts new file mode 100644 index 00000000..138f66c3 --- /dev/null +++ b/tests/unit/gadgets/shared/diagnosticState.test.ts @@ -0,0 +1,324 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +// Mock the core diagnostics module to avoid running actual tsc/biome +vi.mock('../../../../src/gadgets/shared/diagnostics.js', () => ({ + runDiagnostics: vi.fn(), + shouldRunDiagnostics: vi.fn(), +})); + +import { + clearDiagnosticState, + formatDiagnosticStatus, + runDiagnosticsWithTracking, + updateDiagnosticState, +} from '../../../../src/gadgets/shared/diagnosticState.js'; +import { + runDiagnostics, + shouldRunDiagnostics, +} from '../../../../src/gadgets/shared/diagnostics.js'; + +const mockRunDiagnostics = vi.mocked(runDiagnostics); +const mockShouldRunDiagnostics = vi.mocked(shouldRunDiagnostics); + +afterEach(() => { + clearDiagnosticState(); + vi.clearAllMocks(); +}); + +describe('updateDiagnosticState', () => { + describe('TypeScript error parsing', () => { + it('parses TypeScript errors from raw output', () => { + const tsOutput = '/workspace/src/file.ts(10,5): error TS2345: Argument of type string'; + updateDiagnosticState( + '/workspace/src/file.ts', + { hasTypeErrors: true, hasParseErrors: false, hasLintErrors: false }, + { typescript: tsOutput }, + ); + + const status = formatDiagnosticStatus(); + expect(status).toContain('TS2345'); + expect(status).toContain('Argument of type string'); + }); + + it('parses line number from TypeScript output', () => { + const tsOutput = '/workspace/src/file.ts(42,3): error TS1234: Some error'; + updateDiagnosticState( + '/workspace/src/file.ts', + { hasTypeErrors: true, hasParseErrors: false, hasLintErrors: false }, + { typescript: tsOutput }, + ); + + const status = formatDiagnosticStatus(); + expect(status).toContain('line 42'); + }); + + it('falls back to generic message when no pattern matches', () => { + updateDiagnosticState( + 'src/file.ts', + { hasTypeErrors: true, hasParseErrors: false, hasLintErrors: false }, + // No raw output — should use generic fallback + ); + + const status = formatDiagnosticStatus(); + expect(status).toContain('Type errors detected'); + }); + + it('ignores TS error lines not matching the file path', () => { + const tsOutput = '/workspace/src/other.ts(10,5): error TS2345: Some error'; + updateDiagnosticState( + 'src/file.ts', + { hasTypeErrors: true, hasParseErrors: false, hasLintErrors: false }, + { typescript: tsOutput }, + ); + + const status = formatDiagnosticStatus(); + // Falls back to generic since the error is in other.ts + expect(status).toContain('Type errors detected'); + }); + }); + + describe('Biome parse error parsing', () => { + it('parses biome parse errors', () => { + const biomeOutput = 'src/file.ts:5:10 parse error something went wrong'; + updateDiagnosticState( + 'src/file.ts', + { hasTypeErrors: false, hasParseErrors: true, hasLintErrors: false }, + { biome: biomeOutput }, + ); + + const status = formatDiagnosticStatus(); + expect(status).toContain('Parse'); + expect(status).toContain('parse error'); + }); + + it('falls back to generic parse error message', () => { + updateDiagnosticState('src/file.ts', { + hasTypeErrors: false, + hasParseErrors: true, + hasLintErrors: false, + }); + + const status = formatDiagnosticStatus(); + expect(status).toContain('Parse error detected'); + }); + }); + + describe('Biome lint error parsing', () => { + it('parses lint rule names from biome output', () => { + const biomeOutput = ` +src/file.ts:3:5 lint/suspicious/noDoubleEquals + 3 │ if (x == y) {} +`; + updateDiagnosticState( + 'src/file.ts', + { hasTypeErrors: false, hasParseErrors: false, hasLintErrors: true }, + { biome: biomeOutput }, + ); + + const status = formatDiagnosticStatus(); + expect(status).toContain('noDoubleEquals'); + }); + + it('deduplicates repeated lint rules to single error entry', () => { + const biomeOutput = ` +src/file.ts:3:5 lint/suspicious/noDoubleEquals +src/file.ts:7:5 lint/suspicious/noDoubleEquals +src/file.ts:9:5 lint/suspicious/noBannedTypes +`; + updateDiagnosticState( + 'src/file.ts', + { hasTypeErrors: false, hasParseErrors: false, hasLintErrors: true }, + { biome: biomeOutput }, + ); + + const status = formatDiagnosticStatus(); + // Both distinct rule names should appear + expect(status).toContain('noDoubleEquals'); + expect(status).toContain('noBannedTypes'); + // The file should show 2 errors (one per unique rule) + expect(status).toContain('2 errors'); + }); + + it('falls back to generic lint error message when no rules found', () => { + const biomeOutput = 'src/file.ts:5:10 some non-lint error'; + updateDiagnosticState( + 'src/file.ts', + { hasTypeErrors: false, hasParseErrors: false, hasLintErrors: true }, + { biome: biomeOutput }, + ); + + const status = formatDiagnosticStatus(); + expect(status).toContain('lint error'); + }); + + it('falls back to generic lint error when no raw output', () => { + updateDiagnosticState('src/file.ts', { + hasTypeErrors: false, + hasParseErrors: false, + hasLintErrors: true, + }); + + const status = formatDiagnosticStatus(); + expect(status).toContain('Lint error(s) detected'); + }); + }); + + describe('state management', () => { + it('removes file from tracking when all errors are resolved', () => { + updateDiagnosticState('src/file.ts', { + hasTypeErrors: true, + hasParseErrors: false, + hasLintErrors: false, + }); + + let status = formatDiagnosticStatus(); + expect(status).toContain('⚠️'); + + // Fix the errors + updateDiagnosticState('src/file.ts', { + hasTypeErrors: false, + hasParseErrors: false, + hasLintErrors: false, + }); + + status = formatDiagnosticStatus(); + expect(status).toContain('✅ All edited files pass type checking'); + }); + + it('tracks multiple files independently', () => { + updateDiagnosticState('src/file1.ts', { + hasTypeErrors: true, + hasParseErrors: false, + hasLintErrors: false, + }); + updateDiagnosticState('src/file2.ts', { + hasTypeErrors: false, + hasParseErrors: true, + hasLintErrors: false, + }); + + const status = formatDiagnosticStatus(); + expect(status).toContain('src/file1.ts'); + expect(status).toContain('src/file2.ts'); + expect(status).toContain('2 file(s)'); + }); + }); +}); + +describe('formatDiagnosticStatus', () => { + it('returns success message when no errors', () => { + const status = formatDiagnosticStatus(); + expect(status).toBe('## Diagnostic Status [v2]\n\n✅ All edited files pass type checking'); + }); + + it('shows error count per file', () => { + updateDiagnosticState('src/file.ts', { + hasTypeErrors: true, + hasParseErrors: true, + hasLintErrors: false, + }); + + const status = formatDiagnosticStatus(); + expect(status).toContain('2 errors'); + }); + + it('shows singular "error" for exactly 1 error', () => { + updateDiagnosticState('src/file.ts', { + hasTypeErrors: true, + hasParseErrors: false, + hasLintErrors: false, + }); + + const status = formatDiagnosticStatus(); + expect(status).toMatch(/1 error\b/); + }); + + it('includes file path in output', () => { + updateDiagnosticState('src/myModule.ts', { + hasTypeErrors: true, + hasParseErrors: false, + hasLintErrors: false, + }); + + const status = formatDiagnosticStatus(); + expect(status).toContain('src/myModule.ts'); + }); +}); + +describe('runDiagnosticsWithTracking', () => { + it('returns null when diagnostics should not run for file type', () => { + mockShouldRunDiagnostics.mockReturnValue(false); + + const result = runDiagnosticsWithTracking('config.json', '/abs/config.json'); + expect(result).toBeNull(); + expect(mockRunDiagnostics).not.toHaveBeenCalled(); + }); + + it('returns hasErrors=false when no issues found', () => { + mockShouldRunDiagnostics.mockReturnValue(true); + mockRunDiagnostics.mockReturnValue({ + hasTypeErrors: false, + hasParseErrors: false, + hasLintErrors: false, + }); + + const result = runDiagnosticsWithTracking('src/file.ts', '/abs/src/file.ts'); + expect(result).not.toBeNull(); + expect(result?.hasErrors).toBe(false); + expect(result?.statusMessage).toBe('✓ No issues'); + }); + + it('returns hasErrors=true when TypeScript errors found', () => { + mockShouldRunDiagnostics.mockReturnValue(true); + mockRunDiagnostics.mockReturnValue({ + hasTypeErrors: true, + hasParseErrors: false, + hasLintErrors: false, + rawTypescript: '/abs/src/file.ts(5,3): error TS2345: error TS2345: Something is wrong', + }); + + const result = runDiagnosticsWithTracking('src/file.ts', '/abs/src/file.ts'); + expect(result?.hasErrors).toBe(true); + expect(result?.statusMessage).toContain('diagnostic issue'); + }); + + it('returns hasErrors=true when parse errors found', () => { + mockShouldRunDiagnostics.mockReturnValue(true); + mockRunDiagnostics.mockReturnValue({ + hasTypeErrors: false, + hasParseErrors: true, + hasLintErrors: false, + }); + + const result = runDiagnosticsWithTracking('src/file.ts', '/abs/src/file.ts'); + expect(result?.hasErrors).toBe(true); + }); + + it('updates diagnostic state after running', () => { + mockShouldRunDiagnostics.mockReturnValue(true); + mockRunDiagnostics.mockReturnValue({ + hasTypeErrors: true, + hasParseErrors: false, + hasLintErrors: false, + }); + + runDiagnosticsWithTracking('src/file.ts', '/abs/src/file.ts'); + + const status = formatDiagnosticStatus(); + expect(status).toContain('⚠️'); + expect(status).toContain('src/file.ts'); + }); + + it('calls runDiagnostics with the validated path', () => { + mockShouldRunDiagnostics.mockReturnValue(true); + mockRunDiagnostics.mockReturnValue({ + hasTypeErrors: false, + hasParseErrors: false, + hasLintErrors: false, + }); + + runDiagnosticsWithTracking('src/file.ts', '/abs/workspace/src/file.ts'); + + expect(mockRunDiagnostics).toHaveBeenCalledWith('/abs/workspace/src/file.ts'); + }); +}); diff --git a/tests/unit/jira/client.test.ts b/tests/unit/jira/client.test.ts index ec572234..d72c5aa8 100644 --- a/tests/unit/jira/client.test.ts +++ b/tests/unit/jira/client.test.ts @@ -8,12 +8,48 @@ vi.mock('../../../src/utils/logging.js', () => ({ }, })); -// Mock jira.js Version3Client (for other methods, not needed for raw fetch methods) +// Use vi.hoisted to create mock objects before vi.mock factories run +const { mockIssues, mockIssueComments, mockIssueSearch, mockIssueAttachments, mockMyself } = + vi.hoisted(() => ({ + mockIssues: { + getIssue: vi.fn(), + editIssue: vi.fn(), + createIssue: vi.fn(), + doTransition: vi.fn(), + getTransitions: vi.fn(), + }, + mockIssueComments: { + getComments: vi.fn(), + addComment: vi.fn(), + updateComment: vi.fn(), + }, + mockIssueSearch: { + searchForIssuesUsingJql: vi.fn(), + }, + mockIssueAttachments: { + addAttachment: vi.fn(), + }, + mockMyself: { + getCurrentUser: vi.fn(), + }, + })); + vi.mock('jira.js', () => ({ - Version3Client: vi.fn().mockImplementation(() => ({})), + Version3Client: vi.fn().mockImplementation(() => ({ + issues: mockIssues, + issueComments: mockIssueComments, + issueSearch: mockIssueSearch, + issueAttachments: mockIssueAttachments, + myself: mockMyself, + })), })); -import { _resetCloudIdCache, jiraClient, withJiraCredentials } from '../../../src/jira/client.js'; +import { + _resetCloudIdCache, + getJiraCredentials, + jiraClient, + withJiraCredentials, +} from '../../../src/jira/client.js'; describe('jiraClient', () => { const creds = { @@ -24,12 +60,26 @@ describe('jiraClient', () => { const expectedAuth = `Basic ${Buffer.from('bot@example.com:jira-token').toString('base64')}`; beforeEach(() => { - vi.clearAllMocks(); + // Reset only the call history of mock client methods, not their implementations + mockIssues.getIssue.mockReset(); + mockIssues.editIssue.mockReset(); + mockIssues.createIssue.mockReset(); + mockIssues.doTransition.mockReset(); + mockIssues.getTransitions.mockReset(); + mockIssueComments.getComments.mockReset(); + mockIssueComments.addComment.mockReset(); + mockIssueComments.updateComment.mockReset(); + mockIssueSearch.searchForIssuesUsingJql.mockReset(); + mockIssueAttachments.addAttachment.mockReset(); + mockMyself.getCurrentUser.mockReset(); _resetCloudIdCache(); }); afterEach(() => { - vi.restoreAllMocks(); + // Note: We don't call vi.restoreAllMocks() here because it would reset + // the Version3Client mock implementation from vi.mock(), breaking subsequent tests. + // Instead we clear only the fetch spy manually. + vi.clearAllMocks(); }); describe('getCloudId', () => { @@ -165,4 +215,248 @@ describe('jiraClient', () => { ).rejects.toThrow('No JIRA credentials in scope'); }); }); + + describe('getIssue', () => { + it('calls getIssue with the issue key and required fields', async () => { + const issueData = { key: 'TEST-1', fields: { summary: 'Test Issue' } }; + mockIssues.getIssue.mockResolvedValue(issueData); + + const result = await withJiraCredentials(creds, () => jiraClient.getIssue('TEST-1')); + + expect(result).toEqual(issueData); + expect(mockIssues.getIssue).toHaveBeenCalledWith( + expect.objectContaining({ issueIdOrKey: 'TEST-1' }), + ); + }); + + it('throws when called outside scope', async () => { + await expect(jiraClient.getIssue('TEST-1')).rejects.toThrow('No JIRA credentials in scope'); + }); + }); + + describe('updateIssue', () => { + it('calls editIssue with summary', async () => { + mockIssues.editIssue.mockResolvedValue(undefined); + + await withJiraCredentials(creds, () => + jiraClient.updateIssue('TEST-1', { summary: 'New Title' }), + ); + + expect(mockIssues.editIssue).toHaveBeenCalledWith( + expect.objectContaining({ + issueIdOrKey: 'TEST-1', + fields: expect.objectContaining({ summary: 'New Title' }), + }), + ); + }); + + it('calls editIssue with description', async () => { + mockIssues.editIssue.mockResolvedValue(undefined); + const desc = { type: 'doc', version: 1, content: [] }; + + await withJiraCredentials(creds, () => + jiraClient.updateIssue('TEST-1', { description: desc }), + ); + + expect(mockIssues.editIssue).toHaveBeenCalledWith( + expect.objectContaining({ + fields: expect.objectContaining({ description: desc }), + }), + ); + }); + }); + + describe('addComment', () => { + it('returns comment id', async () => { + mockIssueComments.addComment.mockResolvedValue({ id: 'comment-123' }); + + const id = await withJiraCredentials(creds, () => + jiraClient.addComment('TEST-1', { type: 'doc' }), + ); + + expect(id).toBe('comment-123'); + expect(mockIssueComments.addComment).toHaveBeenCalledWith( + expect.objectContaining({ issueIdOrKey: 'TEST-1' }), + ); + }); + + it('returns empty string when id is missing', async () => { + mockIssueComments.addComment.mockResolvedValue({}); + + const id = await withJiraCredentials(creds, () => + jiraClient.addComment('TEST-1', { type: 'doc' }), + ); + + expect(id).toBe(''); + }); + }); + + describe('createIssue', () => { + it('calls createIssue with the provided fields', async () => { + const newIssue = { id: '10001', key: 'TEST-2' }; + mockIssues.createIssue.mockResolvedValue(newIssue); + + const result = await withJiraCredentials(creds, () => + jiraClient.createIssue({ + project: { key: 'TEST' }, + summary: 'New Issue', + issuetype: { name: 'Task' }, + }), + ); + + expect(result).toEqual(newIssue); + expect(mockIssues.createIssue).toHaveBeenCalledWith( + expect.objectContaining({ + fields: expect.objectContaining({ project: { key: 'TEST' } }), + }), + ); + }); + }); + + describe('transitionIssue', () => { + it('calls doTransition with issue key and transition id', async () => { + mockIssues.doTransition.mockResolvedValue(undefined); + + await withJiraCredentials(creds, () => jiraClient.transitionIssue('TEST-1', 'transition-31')); + + expect(mockIssues.doTransition).toHaveBeenCalledWith({ + issueIdOrKey: 'TEST-1', + transition: { id: 'transition-31' }, + }); + }); + }); + + describe('getTransitions', () => { + it('returns transitions array', async () => { + const transitions = [ + { id: '31', name: 'Done' }, + { id: '11', name: 'In Progress' }, + ]; + mockIssues.getTransitions.mockResolvedValue({ transitions }); + + const result = await withJiraCredentials(creds, () => jiraClient.getTransitions('TEST-1')); + + expect(result).toEqual(transitions); + }); + + it('returns empty array when transitions is missing', async () => { + mockIssues.getTransitions.mockResolvedValue({}); + + const result = await withJiraCredentials(creds, () => jiraClient.getTransitions('TEST-1')); + + expect(result).toEqual([]); + }); + }); + + describe('updateLabels', () => { + it('calls editIssue with labels array', async () => { + mockIssues.editIssue.mockResolvedValue(undefined); + + await withJiraCredentials(creds, () => jiraClient.updateLabels('TEST-1', ['bug', 'urgent'])); + + expect(mockIssues.editIssue).toHaveBeenCalledWith({ + issueIdOrKey: 'TEST-1', + fields: { labels: ['bug', 'urgent'] }, + }); + }); + }); + + describe('searchIssues', () => { + it('returns issues from JQL search', async () => { + const issues = [ + { id: '1', key: 'TEST-1' }, + { id: '2', key: 'TEST-2' }, + ]; + mockIssueSearch.searchForIssuesUsingJql.mockResolvedValue({ issues }); + + const result = await withJiraCredentials(creds, () => + jiraClient.searchIssues('project = TEST AND status = "In Progress"'), + ); + + expect(result).toEqual(issues); + expect(mockIssueSearch.searchForIssuesUsingJql).toHaveBeenCalledWith( + expect.objectContaining({ + jql: 'project = TEST AND status = "In Progress"', + }), + ); + }); + + it('returns empty array when issues is missing', async () => { + mockIssueSearch.searchForIssuesUsingJql.mockResolvedValue({}); + + const result = await withJiraCredentials(creds, () => + jiraClient.searchIssues('project = TEST'), + ); + + expect(result).toEqual([]); + }); + + it('uses custom fields when provided', async () => { + mockIssueSearch.searchForIssuesUsingJql.mockResolvedValue({ issues: [] }); + + await withJiraCredentials(creds, () => + jiraClient.searchIssues('project = TEST', ['summary', 'status', 'priority']), + ); + + expect(mockIssueSearch.searchForIssuesUsingJql).toHaveBeenCalledWith( + expect.objectContaining({ + fields: ['summary', 'status', 'priority'], + }), + ); + }); + }); + + describe('addAttachmentFile', () => { + it('calls addAttachment with buffer and filename', async () => { + mockIssueAttachments.addAttachment.mockResolvedValue(undefined); + const buf = Buffer.from('file content'); + + await withJiraCredentials(creds, () => + jiraClient.addAttachmentFile('TEST-1', buf, 'session.zip'), + ); + + expect(mockIssueAttachments.addAttachment).toHaveBeenCalledWith( + expect.objectContaining({ + issueIdOrKey: 'TEST-1', + attachment: expect.objectContaining({ + filename: 'session.zip', + file: buf, + }), + }), + ); + }); + }); + + describe('getIssueComments', () => { + it('returns comments array', async () => { + const comments = [{ id: 'c1', body: 'First comment' }]; + mockIssueComments.getComments.mockResolvedValue({ comments }); + + const result = await withJiraCredentials(creds, () => jiraClient.getIssueComments('TEST-1')); + + expect(result).toEqual(comments); + }); + + it('returns empty array when comments is missing', async () => { + mockIssueComments.getComments.mockResolvedValue({}); + + const result = await withJiraCredentials(creds, () => jiraClient.getIssueComments('TEST-1')); + + expect(result).toEqual([]); + }); + }); + + describe('getJiraCredentials', () => { + it('throws when called outside scope', () => { + expect(() => getJiraCredentials()).toThrow('No JIRA credentials in scope'); + }); + + it('returns credentials when inside withJiraCredentials scope', async () => { + let captured: ReturnType | undefined; + await withJiraCredentials(creds, async () => { + captured = getJiraCredentials(); + }); + expect(captured).toEqual(creds); + }); + }); }); diff --git a/tests/unit/router/config.test.ts b/tests/unit/router/config.test.ts new file mode 100644 index 00000000..2bc1f72e --- /dev/null +++ b/tests/unit/router/config.test.ts @@ -0,0 +1,221 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock config provider to avoid DB connections +vi.mock('../../../src/config/provider.js', () => ({ + loadConfig: vi.fn(), +})); +vi.mock('../../../src/config/configCache.js', () => ({ + configCache: { + getSecrets: vi.fn().mockReturnValue(null), + getConfig: vi.fn().mockReturnValue(null), + getProjectByBoardId: vi.fn().mockReturnValue(null), + getProjectByRepo: vi.fn().mockReturnValue(null), + setConfig: vi.fn(), + setProjectByBoardId: vi.fn(), + setProjectByRepo: vi.fn(), + setSecrets: vi.fn(), + invalidate: vi.fn(), + }, +})); + +import { loadConfig } from '../../../src/config/provider.js'; +import { getProjectConfig, loadProjectConfig, routerConfig } from '../../../src/router/config.js'; + +const mockLoadConfig = vi.mocked(loadConfig); + +// Helper to reset the module-level cache between tests +async function resetProjectConfig(): Promise { + // Re-import the module fresh to reset its state + vi.resetModules(); +} + +describe('routerConfig', () => { + it('has default Redis URL', () => { + expect(routerConfig.redisUrl).toBe('redis://localhost:6379'); + }); + + it('has default maxWorkers', () => { + expect(routerConfig.maxWorkers).toBe(3); + }); + + it('has default workerMemoryMb', () => { + expect(routerConfig.workerMemoryMb).toBe(4096); + }); + + it('has default dockerNetwork', () => { + expect(routerConfig.dockerNetwork).toBe('services_default'); + }); + + it('has default workerTimeoutMs of 30 minutes', () => { + expect(routerConfig.workerTimeoutMs).toBe(30 * 60 * 1000); + }); +}); + +describe('getProjectConfig', () => { + it('throws when config has not been loaded yet', async () => { + // We need a fresh module state without cached config + // Use a dynamic import with a reset module + vi.resetModules(); + + // Re-mock after resetModules + vi.doMock('../../../src/config/provider.js', () => ({ + loadConfig: vi.fn(), + })); + vi.doMock('../../../src/config/configCache.js', () => ({ + configCache: { + getSecrets: vi.fn().mockReturnValue(null), + getConfig: vi.fn().mockReturnValue(null), + setConfig: vi.fn(), + }, + })); + + const { getProjectConfig: freshGetProjectConfig } = await import( + '../../../src/router/config.js' + ); + expect(() => freshGetProjectConfig()).toThrow( + '[Router] Config not loaded yet. Call loadProjectConfig() first.', + ); + + vi.resetModules(); + }); +}); + +describe('loadProjectConfig', () => { + beforeEach(() => { + vi.resetModules(); + vi.doMock('../../../src/config/provider.js', () => ({ + loadConfig: mockLoadConfig, + })); + vi.doMock('../../../src/config/configCache.js', () => ({ + configCache: { + getSecrets: vi.fn().mockReturnValue(null), + getConfig: vi.fn().mockReturnValue(null), + setConfig: vi.fn(), + }, + })); + }); + + it('maps trello project config correctly', async () => { + mockLoadConfig.mockResolvedValueOnce({ + projects: [ + { + id: 'p1', + name: 'Project 1', + repo: 'owner/repo', + orgId: 'org1', + baseBranch: 'main', + branchPrefix: 'cascade/', + pm: { type: 'trello' }, + trello: { + boardId: 'board1', + lists: { briefing: 'list1', planning: 'list2', todo: 'list3' }, + labels: { readyToProcess: 'label1', processed: 'label2' }, + }, + }, + ], + } as never); + + const { loadProjectConfig: freshLoad } = await import('../../../src/router/config.js'); + const result = await freshLoad(); + + expect(result.projects).toHaveLength(1); + expect(result.projects[0]).toMatchObject({ + id: 'p1', + repo: 'owner/repo', + pmType: 'trello', + trello: { + boardId: 'board1', + lists: { briefing: 'list1', planning: 'list2', todo: 'list3' }, + labels: { readyToProcess: 'label1', processed: 'label2' }, + }, + }); + }); + + it('maps jira project config correctly', async () => { + mockLoadConfig.mockResolvedValueOnce({ + projects: [ + { + id: 'p2', + name: 'JIRA Project', + repo: 'owner/jira-repo', + orgId: 'org1', + baseBranch: 'main', + branchPrefix: 'cascade/', + pm: { type: 'jira' }, + jira: { + projectKey: 'MYPROJ', + baseUrl: 'https://mycompany.atlassian.net', + }, + }, + ], + } as never); + + const { loadProjectConfig: freshLoad } = await import('../../../src/router/config.js'); + const result = await freshLoad(); + + expect(result.projects).toHaveLength(1); + expect(result.projects[0]).toMatchObject({ + id: 'p2', + repo: 'owner/jira-repo', + pmType: 'jira', + jira: { + projectKey: 'MYPROJ', + baseUrl: 'https://mycompany.atlassian.net', + }, + }); + }); + + it('defaults pmType to trello when pm.type is not set', async () => { + mockLoadConfig.mockResolvedValueOnce({ + projects: [ + { + id: 'p3', + name: 'No PM type', + repo: 'owner/repo3', + orgId: 'org1', + baseBranch: 'main', + branchPrefix: 'cascade/', + // No pm field + }, + ], + } as never); + + const { loadProjectConfig: freshLoad } = await import('../../../src/router/config.js'); + const result = await freshLoad(); + + expect(result.projects[0].pmType).toBe('trello'); + }); + + it('returns cached result on subsequent calls', async () => { + const innerMock = vi.fn().mockResolvedValue({ + projects: [ + { + id: 'p4', + name: 'Cached', + repo: 'owner/cached', + orgId: 'org1', + baseBranch: 'main', + branchPrefix: 'cascade/', + }, + ], + }); + + vi.resetModules(); + vi.doMock('../../../src/config/provider.js', () => ({ + loadConfig: innerMock, + })); + vi.doMock('../../../src/config/configCache.js', () => ({ + configCache: { + getSecrets: vi.fn().mockReturnValue(null), + getConfig: vi.fn().mockReturnValue(null), + setConfig: vi.fn(), + }, + })); + + const { loadProjectConfig: freshLoad } = await import('../../../src/router/config.js'); + await freshLoad(); + await freshLoad(); // Second call — should use cache + + expect(innerMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/unit/router/index.test.ts b/tests/unit/router/index.test.ts new file mode 100644 index 00000000..1434072a --- /dev/null +++ b/tests/unit/router/index.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it, vi } from 'vitest'; + +// Mock heavy imports that cause side effects +vi.mock('../../../src/router/queue.js', () => ({ + addJob: vi.fn(), + getQueueStats: vi.fn(), +})); +vi.mock('../../../src/router/worker-manager.js', () => ({ + getActiveWorkerCount: vi.fn(), + getActiveWorkers: vi.fn(), + startWorkerProcessor: vi.fn(), + stopWorkerProcessor: vi.fn(), +})); +vi.mock('@hono/node-server', () => ({ + serve: vi.fn(), +})); +vi.mock('../../../src/utils/webhookLogger.js', () => ({ + logWebhookCall: vi.fn(), +})); +vi.mock('../../../src/router/reactions.js', () => ({ + sendAcknowledgeReaction: vi.fn(), +})); +vi.mock('../../../src/router/pre-actions.js', () => ({ + addEyesReactionToPR: vi.fn(), +})); +vi.mock('../../../src/router/config.js', () => ({ + loadProjectConfig: vi.fn().mockResolvedValue({ projects: [] }), + getProjectConfig: vi.fn().mockReturnValue({ projects: [] }), +})); + +// Import the functions we want to test - they are module-private so we test through exports +// We'll use a re-export approach by importing the raw module +// Since these functions aren't exported, we test them via the Hono app behavior instead + +import { getProjectConfig } from '../../../src/router/config.js'; + +describe('router config integration', () => { + it('getProjectConfig returns cached projects', () => { + vi.mocked(getProjectConfig).mockReturnValue({ + projects: [ + { + id: 'p1', + repo: 'owner/repo', + pmType: 'trello', + trello: { + boardId: 'board1', + lists: { briefing: 'list1', planning: 'list2', todo: 'list3', debug: 'list4' }, + labels: { readyToProcess: 'label1' }, + }, + }, + ], + }); + const config = getProjectConfig(); + expect(config.projects).toHaveLength(1); + expect(config.projects[0].id).toBe('p1'); + }); +}); diff --git a/tests/unit/trello/client.test.ts b/tests/unit/trello/client.test.ts index 92395d4d..cf67cdcd 100644 --- a/tests/unit/trello/client.test.ts +++ b/tests/unit/trello/client.test.ts @@ -8,53 +8,74 @@ vi.mock('../../../src/utils/logging.js', () => ({ }, })); -const { mockAddCardComment } = vi.hoisted(() => ({ - mockAddCardComment: vi.fn(), +// Use vi.hoisted to create all mock objects before factory functions run +const { mockCards, mockChecklists, mockLists } = vi.hoisted(() => ({ + mockCards: { + addCardComment: vi.fn(), + getCard: vi.fn(), + getCardActions: vi.fn(), + updateCard: vi.fn(), + addCardLabel: vi.fn(), + deleteCardLabel: vi.fn(), + createCardAttachment: vi.fn(), + createCardChecklist: vi.fn(), + getCardChecklists: vi.fn(), + updateCardCheckItem: vi.fn(), + createCard: vi.fn(), + }, + mockChecklists: { + createChecklistCheckItems: vi.fn(), + }, + mockLists: { + getListCards: vi.fn(), + }, })); // Mock trello.js client vi.mock('trello.js', () => ({ TrelloClient: vi.fn().mockImplementation(() => ({ - cards: { - addCardComment: mockAddCardComment, - }, + cards: mockCards, + checklists: mockChecklists, + lists: mockLists, })), })); import { TrelloClient } from 'trello.js'; -import { trelloClient, withTrelloCredentials } from '../../../src/trello/client.js'; - -const MockedTrelloClient = vi.mocked(TrelloClient); +import { + getTrelloCredentials, + trelloClient, + withTrelloCredentials, +} from '../../../src/trello/client.js'; describe('trelloClient', () => { const creds = { apiKey: 'test-key', token: 'test-token' }; beforeEach(() => { - vi.clearAllMocks(); - // Re-initialize the TrelloClient mock implementation after clearAllMocks - MockedTrelloClient.mockImplementation( - () => ({ cards: { addCardComment: mockAddCardComment } }) as unknown as TrelloClient, - ); + // Reset individual mock functions without clearing implementations + for (const fn of Object.values(mockCards)) fn.mockReset(); + for (const fn of Object.values(mockChecklists)) fn.mockReset(); + for (const fn of Object.values(mockLists)) fn.mockReset(); }); afterEach(() => { - vi.restoreAllMocks(); + // Don't call restoreAllMocks() as it would clear the Version3Client mock impl + vi.clearAllMocks(); }); describe('addComment', () => { it('returns the comment action ID from API response', async () => { - mockAddCardComment.mockResolvedValue({ id: 'action-abc123' }); + mockCards.addCardComment.mockResolvedValue({ id: 'action-abc123' }); const id = await withTrelloCredentials(creds, () => trelloClient.addComment('card-1', 'Hello world'), ); - expect(mockAddCardComment).toHaveBeenCalledWith({ id: 'card-1', text: 'Hello world' }); + expect(mockCards.addCardComment).toHaveBeenCalledWith({ id: 'card-1', text: 'Hello world' }); expect(id).toBe('action-abc123'); }); it('returns empty string when API response has no id', async () => { - mockAddCardComment.mockResolvedValue({}); + mockCards.addCardComment.mockResolvedValue({}); const id = await withTrelloCredentials(creds, () => trelloClient.addComment('card-1', 'Hello'), @@ -139,4 +160,207 @@ describe('trelloClient', () => { ); }); }); + + describe('getTrelloCredentials', () => { + it('throws when called outside scope', () => { + expect(() => getTrelloCredentials()).toThrow('No Trello credentials in scope'); + }); + + it('returns credentials when inside scope', async () => { + let captured: ReturnType | undefined; + await withTrelloCredentials(creds, async () => { + captured = getTrelloCredentials(); + }); + expect(captured).toEqual(creds); + }); + }); + + describe('getCard', () => { + it('returns a card with normalized fields', async () => { + mockCards.getCard.mockResolvedValue({ + id: 'card-1', + name: 'My Card', + desc: 'Card description', + url: 'https://trello.com/c/abc123', + shortUrl: 'https://trello.com/c/abc', + idList: 'list-1', + labels: [{ id: 'label-1', name: 'Bug', color: 'red' }], + }); + + const result = await withTrelloCredentials(creds, () => trelloClient.getCard('card-1')); + + expect(result).toEqual({ + id: 'card-1', + name: 'My Card', + desc: 'Card description', + url: 'https://trello.com/c/abc123', + shortUrl: 'https://trello.com/c/abc', + idList: 'list-1', + labels: [{ id: 'label-1', name: 'Bug', color: 'red' }], + }); + expect(mockCards.getCard).toHaveBeenCalledWith({ id: 'card-1' }); + }); + + it('normalizes missing optional fields to empty strings', async () => { + mockCards.getCard.mockResolvedValue({ id: 'card-2' }); + + const result = await withTrelloCredentials(creds, () => trelloClient.getCard('card-2')); + + expect(result.name).toBe(''); + expect(result.desc).toBe(''); + expect(result.url).toBe(''); + expect(result.idList).toBe(''); + expect(result.labels).toEqual([]); + }); + + it('throws when called outside scope', async () => { + await expect(trelloClient.getCard('card-1')).rejects.toThrow( + 'No Trello credentials in scope', + ); + }); + }); + + describe('getCardComments', () => { + it('returns comments with mapped fields', async () => { + mockCards.getCardActions.mockResolvedValue([ + { + id: 'action-1', + date: '2026-01-01T00:00:00.000Z', + data: { text: 'Hello world' }, + memberCreator: { id: 'member-1', fullName: 'Alice', username: 'alice' }, + }, + ]); + + const result = await withTrelloCredentials(creds, () => + trelloClient.getCardComments('card-1'), + ); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + id: 'action-1', + date: '2026-01-01T00:00:00.000Z', + data: { text: 'Hello world' }, + memberCreator: { id: 'member-1', fullName: 'Alice', username: 'alice' }, + }); + }); + + it('returns empty array when no comments', async () => { + mockCards.getCardActions.mockResolvedValue([]); + + const result = await withTrelloCredentials(creds, () => + trelloClient.getCardComments('card-1'), + ); + + expect(result).toEqual([]); + }); + }); + + describe('updateCard', () => { + it('calls updateCard with name and desc', async () => { + mockCards.updateCard.mockResolvedValue({}); + + await withTrelloCredentials(creds, () => + trelloClient.updateCard('card-1', { name: 'New Title', desc: 'New desc' }), + ); + + expect(mockCards.updateCard).toHaveBeenCalledWith( + expect.objectContaining({ id: 'card-1', name: 'New Title', desc: 'New desc' }), + ); + }); + }); + + describe('createCard', () => { + it('returns a created card with normalized fields', async () => { + mockCards.createCard.mockResolvedValue({ + id: 'new-card', + name: 'New Feature', + desc: 'Description', + url: 'https://trello.com/c/new', + shortUrl: 'https://trello.com/c/new-short', + idList: 'list-todo', + labels: [], + }); + + const result = await withTrelloCredentials(creds, () => + trelloClient.createCard('list-todo', { name: 'New Feature', desc: 'Description' }), + ); + + expect(result.id).toBe('new-card'); + expect(result.name).toBe('New Feature'); + expect(mockCards.createCard).toHaveBeenCalledWith( + expect.objectContaining({ idList: 'list-todo', name: 'New Feature' }), + ); + }); + }); + + describe('getCardChecklists', () => { + it('returns checklists with check items', async () => { + mockCards.getCardChecklists.mockResolvedValue([ + { + id: 'cl-1', + name: 'Implementation Steps', + idCard: 'card-1', + checkItems: [ + { id: 'item-1', name: 'Step 1', state: 'complete' }, + { id: 'item-2', name: 'Step 2', state: 'incomplete' }, + ], + }, + ]); + + const result = await withTrelloCredentials(creds, () => + trelloClient.getCardChecklists('card-1'), + ); + + expect(result).toHaveLength(1); + expect(result[0].name).toBe('Implementation Steps'); + expect(result[0].checkItems[0].state).toBe('complete'); + expect(result[0].checkItems[1].state).toBe('incomplete'); + }); + }); + + describe('getCardAttachments', () => { + it('returns attachments via fetch', async () => { + const attachments = [ + { + id: 'att-1', + name: 'session.zip', + url: 'https://trello.com/attachments/att-1', + mimeType: 'application/zip', + bytes: 1024, + date: '2026-01-01T00:00:00.000Z', + }, + ]; + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValue(new Response(JSON.stringify(attachments), { status: 200 })); + + const result = await withTrelloCredentials(creds, () => + trelloClient.getCardAttachments('card-1'), + ); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + id: 'att-1', + name: 'session.zip', + url: 'https://trello.com/attachments/att-1', + mimeType: 'application/zip', + bytes: 1024, + date: '2026-01-01T00:00:00.000Z', + }); + const [url] = fetchSpy.mock.calls[0]; + expect(url).toContain('/1/cards/card-1/attachments'); + expect(url).toContain('key=test-key'); + expect(url).toContain('token=test-token'); + }); + + it('throws on non-OK response', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response('Unauthorized', { status: 401 }), + ); + + await expect( + withTrelloCredentials(creds, () => trelloClient.getCardAttachments('card-1')), + ).rejects.toThrow('Failed to get attachments: 401'); + }); + }); }); diff --git a/tests/unit/utils/envScrub.test.ts b/tests/unit/utils/envScrub.test.ts new file mode 100644 index 00000000..be1c1480 --- /dev/null +++ b/tests/unit/utils/envScrub.test.ts @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { scrubSensitiveEnv } from '../../../src/utils/envScrub.js'; + +describe('scrubSensitiveEnv', () => { + let savedEnv: Record; + + beforeEach(() => { + // Save original env values for restoration + savedEnv = { + CREDENTIAL_MASTER_KEY: process.env.CREDENTIAL_MASTER_KEY, + DATABASE_URL: process.env.DATABASE_URL, + DATABASE_SSL: process.env.DATABASE_SSL, + REDIS_URL: process.env.REDIS_URL, + CASCADE_CREDENTIALS: process.env.CASCADE_CREDENTIALS, + CASCADE_CREDENTIALS_PROJECT_ID: process.env.CASCADE_CREDENTIALS_PROJECT_ID, + }; + }); + + afterEach(() => { + // Restore original env values + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + it('removes CREDENTIAL_MASTER_KEY from process.env', () => { + process.env.CREDENTIAL_MASTER_KEY = 'super-secret-key-abc123'; + scrubSensitiveEnv(); + expect(process.env.CREDENTIAL_MASTER_KEY).toBeUndefined(); + }); + + it('removes DATABASE_URL from process.env', () => { + process.env.DATABASE_URL = 'postgresql://user:pass@host:5432/db'; + scrubSensitiveEnv(); + expect(process.env.DATABASE_URL).toBeUndefined(); + }); + + it('removes DATABASE_SSL from process.env', () => { + process.env.DATABASE_SSL = 'false'; + scrubSensitiveEnv(); + expect(process.env.DATABASE_SSL).toBeUndefined(); + }); + + it('removes REDIS_URL from process.env', () => { + process.env.REDIS_URL = 'redis://localhost:6379'; + scrubSensitiveEnv(); + expect(process.env.REDIS_URL).toBeUndefined(); + }); + + it('removes CASCADE_CREDENTIALS from process.env', () => { + process.env.CASCADE_CREDENTIALS = 'eyJzb21lIjoianNvbiJ9'; + scrubSensitiveEnv(); + expect(process.env.CASCADE_CREDENTIALS).toBeUndefined(); + }); + + it('removes CASCADE_CREDENTIALS_PROJECT_ID from process.env', () => { + process.env.CASCADE_CREDENTIALS_PROJECT_ID = 'my-project-id'; + scrubSensitiveEnv(); + expect(process.env.CASCADE_CREDENTIALS_PROJECT_ID).toBeUndefined(); + }); + + it('removes all sensitive keys in a single call', () => { + process.env.CREDENTIAL_MASTER_KEY = 'key1'; + process.env.DATABASE_URL = 'postgres://...'; + process.env.DATABASE_SSL = 'true'; + process.env.REDIS_URL = 'redis://...'; + process.env.CASCADE_CREDENTIALS = 'creds'; + process.env.CASCADE_CREDENTIALS_PROJECT_ID = 'proj-id'; + + scrubSensitiveEnv(); + + expect(process.env.CREDENTIAL_MASTER_KEY).toBeUndefined(); + expect(process.env.DATABASE_URL).toBeUndefined(); + expect(process.env.DATABASE_SSL).toBeUndefined(); + expect(process.env.REDIS_URL).toBeUndefined(); + expect(process.env.CASCADE_CREDENTIALS).toBeUndefined(); + expect(process.env.CASCADE_CREDENTIALS_PROJECT_ID).toBeUndefined(); + }); + + it('does not remove non-sensitive environment variables', () => { + process.env.MY_APP_API_KEY = 'should-remain'; + process.env.PORT = '3000'; + + scrubSensitiveEnv(); + + expect(process.env.MY_APP_API_KEY).toBe('should-remain'); + expect(process.env.PORT).toBe('3000'); + + // Clean up test-specific vars + process.env.MY_APP_API_KEY = undefined; + process.env.PORT = undefined; + }); + + it('handles keys that were never set (undefined)', () => { + // Ensure they are undefined to start + process.env.CREDENTIAL_MASTER_KEY = undefined; + process.env.DATABASE_URL = undefined; + + // Should not throw + expect(() => scrubSensitiveEnv()).not.toThrow(); + + expect(process.env.CREDENTIAL_MASTER_KEY).toBeUndefined(); + expect(process.env.DATABASE_URL).toBeUndefined(); + }); + + it('scrubbing is idempotent — calling twice does not throw', () => { + process.env.DATABASE_URL = 'postgres://...'; + scrubSensitiveEnv(); + expect(() => scrubSensitiveEnv()).not.toThrow(); + }); +}); From 9ef1905114fd1738205e8ef711a17b85e824945c Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Fri, 20 Feb 2026 11:23:23 +0000 Subject: [PATCH 03/10] feat(triggers): configurable trigger options + review-requested trigger --- src/config/schema.ts | 17 ++ src/config/triggerConfig.ts | 100 +++++++++ src/db/repositories/configRepository.ts | 31 ++- src/triggers/github/check-suite-failure.ts | 6 + src/triggers/github/check-suite-success.ts | 6 + src/triggers/github/pr-comment-mention.ts | 6 + src/triggers/github/pr-merged.ts | 7 + src/triggers/github/pr-opened.ts | 6 + src/triggers/github/pr-ready-to-merge.ts | 6 + src/triggers/github/pr-review-submitted.ts | 6 + src/triggers/github/review-requested.ts | 101 +++++++++ src/triggers/github/types.ts | 10 +- src/triggers/index.ts | 12 +- src/triggers/jira/comment-mention.ts | 6 + src/triggers/jira/issue-transitioned.ts | 6 + src/triggers/jira/label-added.ts | 6 + src/triggers/trello/card-moved.ts | 10 + src/triggers/trello/comment-mention.ts | 6 + src/triggers/trello/label-added.ts | 6 + tests/unit/config/triggerConfig.test.ts | 135 ++++++++++++ tests/unit/triggers/pr-opened.test.ts | 50 ++++- tests/unit/triggers/review-requested.test.ts | 199 ++++++++++++++++++ .../components/projects/integration-form.tsx | 121 +++++++++++ 23 files changed, 845 insertions(+), 14 deletions(-) create mode 100644 src/config/triggerConfig.ts create mode 100644 src/triggers/github/review-requested.ts create mode 100644 tests/unit/config/triggerConfig.test.ts create mode 100644 tests/unit/triggers/review-requested.test.ts diff --git a/src/config/schema.ts b/src/config/schema.ts index 2346c334..f4323ab8 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -1,4 +1,9 @@ import { z } from 'zod'; +import { + GitHubTriggerConfigSchema, + JiraTriggerConfigSchema, + TrelloTriggerConfigSchema, +} from './triggerConfig.js'; const AgentBackendConfigSchema = z.object({ default: z.string().default('llmist'), @@ -24,6 +29,7 @@ const JiraConfigSchema = z.object({ readyToProcess: z.string().default('cascade-ready'), }) .optional(), + triggers: JiraTriggerConfigSchema.partial().optional(), }); export const ProjectConfigSchema = z.object({ @@ -50,11 +56,22 @@ export const ProjectConfigSchema = z.object({ cost: z.string().optional(), }) .optional(), + triggers: TrelloTriggerConfigSchema.partial().optional(), }) .optional(), jira: JiraConfigSchema.optional(), + /** + * GitHub-specific configuration, including trigger toggles. + * Separate from trello/jira because GitHub integration is always present for code operations. + */ + github: z + .object({ + triggers: GitHubTriggerConfigSchema.partial().optional(), + }) + .optional(), + prompts: z.record(z.string()).optional(), model: z.string().optional(), agentModels: z.record(z.string()).optional(), diff --git a/src/config/triggerConfig.ts b/src/config/triggerConfig.ts new file mode 100644 index 00000000..989eef5f --- /dev/null +++ b/src/config/triggerConfig.ts @@ -0,0 +1,100 @@ +import { z } from 'zod'; + +// ============================================================================ +// Trigger Config Schemas +// ============================================================================ + +/** + * Trigger configuration for Trello integrations. + * All triggers default to `true` for backward compatibility. + */ +export const TrelloTriggerConfigSchema = z.object({ + cardMovedToBriefing: z.boolean().default(true), + cardMovedToPlanning: z.boolean().default(true), + cardMovedToTodo: z.boolean().default(true), + readyToProcessLabel: z.boolean().default(true), + commentMention: z.boolean().default(true), +}); + +/** + * Trigger configuration for JIRA integrations. + * All triggers default to `true` for backward compatibility. + */ +export const JiraTriggerConfigSchema = z.object({ + issueTransitioned: z.boolean().default(true), + readyToProcessLabel: z.boolean().default(true), + commentMention: z.boolean().default(true), +}); + +/** + * Trigger configuration for GitHub integrations. + * Existing triggers default to `true`; new triggers (`reviewRequested`, `prOpened`) default to `false`. + */ +export const GitHubTriggerConfigSchema = z.object({ + checkSuiteSuccess: z.boolean().default(true), + checkSuiteFailure: z.boolean().default(true), + prReviewSubmitted: z.boolean().default(true), + prCommentMention: z.boolean().default(true), + prReadyToMerge: z.boolean().default(true), + prMerged: z.boolean().default(true), + /** New trigger: fires review agent when review is requested from a CASCADE persona. Default false (opt-in). */ + reviewRequested: z.boolean().default(false), + /** PR opened trigger. Default false (disabled until reviewed). */ + prOpened: z.boolean().default(false), +}); + +export type TrelloTriggerConfig = z.infer; +export type JiraTriggerConfig = z.infer; +export type GitHubTriggerConfig = z.infer; + +// ============================================================================ +// Helpers +// ============================================================================ + +/** + * Resolve whether a Trello trigger is enabled based on project trigger config. + * Returns `true` (enabled) when no config is present (backward compatible). + */ +export function resolveTrelloTriggerEnabled( + config: Partial | undefined, + key: keyof TrelloTriggerConfig, +): boolean { + if (!config) return true; + const value = config[key]; + return value === undefined ? true : value; +} + +/** + * Resolve whether a JIRA trigger is enabled based on project trigger config. + * Returns `true` (enabled) when no config is present (backward compatible). + */ +export function resolveJiraTriggerEnabled( + config: Partial | undefined, + key: keyof JiraTriggerConfig, +): boolean { + if (!config) return true; + const value = config[key]; + return value === undefined ? true : value; +} + +/** + * Resolve whether a GitHub trigger is enabled based on project trigger config. + * For new opt-in triggers (reviewRequested, prOpened), returns `false` when no config is present. + */ +export function resolveGitHubTriggerEnabled( + config: Partial | undefined, + key: keyof GitHubTriggerConfig, +): boolean { + if (!config) { + // New triggers that are opt-in default to false even without config + if (key === 'reviewRequested' || key === 'prOpened') return false; + return true; + } + const value = config[key]; + if (value === undefined) { + // New triggers that are opt-in default to false + if (key === 'reviewRequested' || key === 'prOpened') return false; + return true; + } + return value; +} diff --git a/src/db/repositories/configRepository.ts b/src/db/repositories/configRepository.ts index 1b36abd6..2f526e99 100644 --- a/src/db/repositories/configRepository.ts +++ b/src/db/repositories/configRepository.ts @@ -9,6 +9,7 @@ interface TrelloIntegrationConfig { lists: Record; labels: Record; customFields?: { cost?: string }; + triggers?: Record; } interface JiraIntegrationConfig { @@ -18,6 +19,13 @@ interface JiraIntegrationConfig { issueTypes?: Record; customFields?: { cost?: string }; labels?: Record; + triggers?: Record; +} + +interface GitHubIntegrationConfig { + implementerCredentialId?: number; + reviewerCredentialId?: number; + triggers?: Record; } interface DefaultsRow { @@ -84,6 +92,7 @@ function mapProjectRow( projectAgentConfigs: AgentConfigRow[], trelloConfig?: TrelloIntegrationConfig, jiraConfig?: JiraIntegrationConfig, + githubConfig?: GitHubIntegrationConfig, ): Record { const { models, prompts, backends } = buildAgentMaps(projectAgentConfigs); @@ -111,6 +120,7 @@ function mapProjectRow( lists: trelloConfig.lists, labels: trelloConfig.labels, customFields: trelloConfig.customFields, + ...(trelloConfig.triggers ? { triggers: trelloConfig.triggers } : {}), }; } @@ -122,9 +132,14 @@ function mapProjectRow( issueTypes: jiraConfig.issueTypes, customFields: jiraConfig.customFields, labels: jiraConfig.labels, + ...(jiraConfig.triggers ? { triggers: jiraConfig.triggers } : {}), }; } + if (githubConfig?.triggers) { + project.github = { triggers: githubConfig.triggers }; + } + if (row.agentBackend) { project.agentBackend = { default: row.agentBackend, @@ -198,7 +213,16 @@ export async function loadConfigFromDb(): Promise { const jiraConfig = integrations.find((i) => i.type === 'jira')?.config as | JiraIntegrationConfig | undefined; - return mapProjectRow(row, projectAgentConfigsMap.get(row.id) ?? [], trelloConfig, jiraConfig); + const githubConfig = integrations.find((i) => i.type === 'github')?.config as + | GitHubIntegrationConfig + | undefined; + return mapProjectRow( + row, + projectAgentConfigsMap.get(row.id) ?? [], + trelloConfig, + jiraConfig, + githubConfig, + ); }), }; @@ -236,10 +260,13 @@ async function findProjectConfigFromDb( const jiraConfig = integrations.find((i) => i.type === 'jira')?.config as | JiraIntegrationConfig | undefined; + const githubConfig = integrations.find((i) => i.type === 'github')?.config as + | GitHubIntegrationConfig + | undefined; const rawConfig = { defaults: mapDefaultsRow(defaultsRow, [...globalAcs, ...orgAcs]), - projects: [mapProjectRow(row, projectAcs, trelloConfig, jiraConfig)], + projects: [mapProjectRow(row, projectAcs, trelloConfig, jiraConfig, githubConfig)], }; const config = validateConfig(rawConfig); return { project: config.projects[0], config }; diff --git a/src/triggers/github/check-suite-failure.ts b/src/triggers/github/check-suite-failure.ts index f5aa8473..a12d4ff1 100644 --- a/src/triggers/github/check-suite-failure.ts +++ b/src/triggers/github/check-suite-failure.ts @@ -1,3 +1,4 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -21,6 +22,11 @@ export class CheckSuiteFailureTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubCheckSuitePayload(ctx.payload)) return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'checkSuiteFailure')) { + return false; + } + const payload = ctx.payload; // Only trigger on completed check suites with failure conclusion diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index add37ded..ae2d879c 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -1,3 +1,4 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { type CheckSuiteStatus, githubClient } from '../../github/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -61,6 +62,11 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubCheckSuitePayload(ctx.payload)) return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'checkSuiteSuccess')) { + return false; + } + const payload = ctx.payload; // Only trigger on completed check suites with success conclusion diff --git a/src/triggers/github/pr-comment-mention.ts b/src/triggers/github/pr-comment-mention.ts index 1a6884e1..b1a273fd 100644 --- a/src/triggers/github/pr-comment-mention.ts +++ b/src/triggers/github/pr-comment-mention.ts @@ -1,3 +1,4 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import { isCascadeBot } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; @@ -18,6 +19,11 @@ export class PRCommentMentionTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'github') return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prCommentMention')) { + return false; + } + // Match issue_comment.created on PRs if (isGitHubIssueCommentPayload(ctx.payload)) { if (ctx.payload.action !== 'created') return false; diff --git a/src/triggers/github/pr-merged.ts b/src/triggers/github/pr-merged.ts index 5b06a26d..2e8b15e6 100644 --- a/src/triggers/github/pr-merged.ts +++ b/src/triggers/github/pr-merged.ts @@ -1,3 +1,4 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import { trelloClient } from '../../trello/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; @@ -12,6 +13,12 @@ export class PRMergedTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'github') return false; if (!isGitHubPullRequestPayload(ctx.payload)) return false; + + // Check trigger config — default enabled for backward compatibility + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prMerged')) { + return false; + } + return ctx.payload.action === 'closed'; } diff --git a/src/triggers/github/pr-opened.ts b/src/triggers/github/pr-opened.ts index fab79661..5fe88bff 100644 --- a/src/triggers/github/pr-opened.ts +++ b/src/triggers/github/pr-opened.ts @@ -1,3 +1,4 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { isGitHubPullRequestPayload } from './types.js'; @@ -15,6 +16,11 @@ export class PROpenedTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubPullRequestPayload(ctx.payload)) return false; + // Check trigger config — opt-in trigger, default disabled + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prOpened')) { + return false; + } + // Only trigger on newly opened PRs if (ctx.payload.action !== 'opened') return false; diff --git a/src/triggers/github/pr-ready-to-merge.ts b/src/triggers/github/pr-ready-to-merge.ts index 1972d839..367632c6 100644 --- a/src/triggers/github/pr-ready-to-merge.ts +++ b/src/triggers/github/pr-ready-to-merge.ts @@ -1,3 +1,4 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { githubClient } from '../../github/client.js'; import { trelloClient } from '../../trello/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; @@ -17,6 +18,11 @@ export class PRReadyToMergeTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'github') return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prReadyToMerge')) { + return false; + } + // Trigger on either check_suite completion (success) or review submission (approved) if (isGitHubCheckSuitePayload(ctx.payload)) { const payload = ctx.payload; diff --git a/src/triggers/github/pr-review-submitted.ts b/src/triggers/github/pr-review-submitted.ts index 7066bc47..cef45830 100644 --- a/src/triggers/github/pr-review-submitted.ts +++ b/src/triggers/github/pr-review-submitted.ts @@ -1,3 +1,4 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; import { getPersonaForLogin } from '../../github/personas.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -12,6 +13,11 @@ export class PRReviewSubmittedTrigger implements TriggerHandler { if (ctx.source !== 'github') return false; if (!isGitHubPullRequestReviewPayload(ctx.payload)) return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'prReviewSubmitted')) { + return false; + } + // Only trigger on submitted reviews, not edits or dismissals if (ctx.payload.action !== 'submitted') return false; diff --git a/src/triggers/github/review-requested.ts b/src/triggers/github/review-requested.ts new file mode 100644 index 00000000..4ccaf67f --- /dev/null +++ b/src/triggers/github/review-requested.ts @@ -0,0 +1,101 @@ +import { resolveGitHubTriggerEnabled } from '../../config/triggerConfig.js'; +import { isCascadeBot } from '../../github/personas.js'; +import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; +import { logger } from '../../utils/logging.js'; +import { type GitHubPullRequestPayload, isGitHubPullRequestPayload } from './types.js'; +import { extractWorkItemId } from './utils.js'; + +/** + * Trigger that fires the review agent when review is requested from a CASCADE persona account. + * + * This trigger: + * 1. Fires on `pull_request.review_requested` events + * 2. Checks if the requested reviewer is a CASCADE persona (implementer OR reviewer) + * 3. Fires the `review` agent with PR number and work item ID from PR body + * + * Default: **disabled** (opt-in via trigger config). Enable by setting + * `github.triggers.reviewRequested = true` in integration config. + * + * Registration: should be registered BEFORE CheckSuiteSuccessTrigger so that + * both triggers can independently fire review. The HEAD-SHA dedup in + * CheckSuiteSuccessTrigger prevents double-reviews. + */ +export class ReviewRequestedTrigger implements TriggerHandler { + name = 'review-requested'; + description = 'Triggers review agent when review is requested from a CASCADE persona account'; + + matches(ctx: TriggerContext): boolean { + if (ctx.source !== 'github') return false; + if (!isGitHubPullRequestPayload(ctx.payload)) return false; + + // Only trigger on review_requested events + if (ctx.payload.action !== 'review_requested') return false; + + // Check trigger config — opt-in trigger, default disabled + if (!resolveGitHubTriggerEnabled(ctx.project.github?.triggers, 'reviewRequested')) { + return false; + } + + return true; + } + + async handle(ctx: TriggerContext): Promise { + const payload = ctx.payload as GitHubPullRequestPayload; + const prNumber = payload.pull_request.number; + + // Require persona identities for bot detection + if (!ctx.personaIdentities) { + logger.warn('No persona identities available, skipping review-requested trigger', { + prNumber, + }); + return null; + } + + // Check if the requested reviewer is a CASCADE persona + const requestedReviewer = payload.requested_reviewer?.login; + if (!requestedReviewer) { + logger.debug('No requested reviewer in payload, skipping', { prNumber }); + return null; + } + + if (!isCascadeBot(requestedReviewer, ctx.personaIdentities)) { + logger.debug('Requested reviewer is not a CASCADE persona, skipping', { + prNumber, + requestedReviewer, + personas: ctx.personaIdentities, + }); + return null; + } + + const prBody = payload.pull_request.body; + const workItemId = extractWorkItemId(prBody, ctx.project); + + if (!workItemId) { + logger.info('PR does not have work item reference, skipping review-requested trigger', { + prNumber, + }); + return null; + } + + logger.info('Review requested from CASCADE persona, triggering review agent', { + prNumber, + requestedReviewer, + workItemId, + }); + + return { + agentType: 'review', + agentInput: { + prNumber, + prBranch: payload.pull_request.head.ref, + repoFullName: payload.repository.full_name, + headSha: payload.pull_request.head.sha, + triggerType: 'ci-success', + cardId: workItemId, + }, + prNumber, + cardId: workItemId, + workItemId, + }; + } +} diff --git a/src/triggers/github/types.ts b/src/triggers/github/types.ts index eb4a12e5..1d34f09c 100644 --- a/src/triggers/github/types.ts +++ b/src/triggers/github/types.ts @@ -152,7 +152,8 @@ export interface GitHubPullRequestPayload { | 'synchronize' | 'edited' | 'ready_for_review' - | 'converted_to_draft'; + | 'converted_to_draft' + | 'review_requested'; number: number; pull_request: { number: number; @@ -171,6 +172,13 @@ export interface GitHubPullRequestPayload { user: { login: string; }; + requested_reviewers?: Array<{ + login: string; + }>; + }; + /** Present on review_requested events — the reviewer just added */ + requested_reviewer?: { + login: string; }; repository: { full_name: string; // owner/repo diff --git a/src/triggers/index.ts b/src/triggers/index.ts index 5b57d980..75f311b3 100644 --- a/src/triggers/index.ts +++ b/src/triggers/index.ts @@ -1,10 +1,11 @@ import { CheckSuiteFailureTrigger } from './github/check-suite-failure.js'; import { CheckSuiteSuccessTrigger } from './github/check-suite-success.js'; import { PRCommentMentionTrigger } from './github/pr-comment-mention.js'; -// import { PROpenedTrigger } from './github/pr-opened.js'; import { PRMergedTrigger } from './github/pr-merged.js'; +import { PROpenedTrigger } from './github/pr-opened.js'; import { PRReadyToMergeTrigger } from './github/pr-ready-to-merge.js'; import { PRReviewSubmittedTrigger } from './github/pr-review-submitted.js'; +import { ReviewRequestedTrigger } from './github/review-requested.js'; import { JiraCommentMentionTrigger } from './jira/comment-mention.js'; import { JiraIssueTransitionedTrigger } from './jira/issue-transitioned.js'; import { JiraReadyToProcessLabelTrigger } from './jira/label-added.js'; @@ -53,8 +54,8 @@ export function registerBuiltInTriggers(registry: TriggerRegistry): void { registry.register(new JiraReadyToProcessLabelTrigger()); // GitHub: PR opened trigger (initial review on new PRs) - // DISABLED: Triggers respond-to-review which has file editing gadgets - needs review - // registry.register(new PROpenedTrigger()); + // Opt-in: disabled by default via trigger config (github.triggers.prOpened = false) + registry.register(new PROpenedTrigger()); // GitHub: PR comment @mention trigger (runs respond-to-pr-comment when reviewer is @mentioned) // Must be registered before other comment triggers so it can intercept mentions and fall through otherwise @@ -63,6 +64,11 @@ export function registerBuiltInTriggers(registry: TriggerRegistry): void { // GitHub: PR review submission trigger (when someone submits a review) registry.register(new PRReviewSubmittedTrigger()); + // GitHub: Review requested trigger (runs review agent when review is requested from CASCADE persona) + // Opt-in: disabled by default via trigger config (github.triggers.reviewRequested = false) + // Registered before CheckSuiteSuccessTrigger so both can independently trigger review + registry.register(new ReviewRequestedTrigger()); + // GitHub: Check suite failure trigger (runs implementation agent to fix) registry.register(new CheckSuiteFailureTrigger()); diff --git a/src/triggers/jira/comment-mention.ts b/src/triggers/jira/comment-mention.ts index 9abcbad5..8e991f10 100644 --- a/src/triggers/jira/comment-mention.ts +++ b/src/triggers/jira/comment-mention.ts @@ -5,6 +5,7 @@ * Runs the respond-to-planning-comment agent. */ +import { resolveJiraTriggerEnabled } from '../../config/triggerConfig.js'; import { jiraClient } from '../../jira/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -109,6 +110,11 @@ export class JiraCommentMentionTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'jira') return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveJiraTriggerEnabled(ctx.project.jira?.triggers, 'commentMention')) { + return false; + } + const payload = ctx.payload as JiraWebhookPayload; return payload.webhookEvent === 'comment_created' || payload.webhookEvent === 'comment_updated'; } diff --git a/src/triggers/jira/issue-transitioned.ts b/src/triggers/jira/issue-transitioned.ts index b4e2ce3c..a4531c19 100644 --- a/src/triggers/jira/issue-transitioned.ts +++ b/src/triggers/jira/issue-transitioned.ts @@ -5,6 +5,7 @@ * a CASCADE agent type (briefing, planning, implementation). */ +import { resolveJiraTriggerEnabled } from '../../config/triggerConfig.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -49,6 +50,11 @@ export class JiraIssueTransitionedTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'jira') return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveJiraTriggerEnabled(ctx.project.jira?.triggers, 'issueTransitioned')) { + return false; + } + const payload = ctx.payload as JiraWebhookPayload; if (!payload.webhookEvent?.startsWith('jira:issue_updated')) return false; diff --git a/src/triggers/jira/label-added.ts b/src/triggers/jira/label-added.ts index 1f10c693..b46efd10 100644 --- a/src/triggers/jira/label-added.ts +++ b/src/triggers/jira/label-added.ts @@ -10,6 +10,7 @@ * explicitly excludes events that also contain a status change in the changelog. */ +import { resolveJiraTriggerEnabled } from '../../config/triggerConfig.js'; import { resolveProjectPMConfig } from '../../pm/lifecycle.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -64,6 +65,11 @@ export class JiraReadyToProcessLabelTrigger implements TriggerHandler { matches(ctx: TriggerContext): boolean { if (ctx.source !== 'jira') return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveJiraTriggerEnabled(ctx.project.jira?.triggers, 'readyToProcessLabel')) { + return false; + } + const payload = ctx.payload as JiraLabelPayload; if (!payload.webhookEvent?.startsWith('jira:issue_updated')) return false; diff --git a/src/triggers/trello/card-moved.ts b/src/triggers/trello/card-moved.ts index 926204a9..18cc8d47 100644 --- a/src/triggers/trello/card-moved.ts +++ b/src/triggers/trello/card-moved.ts @@ -1,3 +1,4 @@ +import { resolveTrelloTriggerEnabled } from '../../config/triggerConfig.js'; import type { TrelloWebhookPayload, TriggerContext, @@ -15,6 +16,7 @@ interface CardMovedConfig { description: string; listKey: 'briefing' | 'planning' | 'todo'; agentType: string; + triggerConfigKey: 'cardMovedToBriefing' | 'cardMovedToPlanning' | 'cardMovedToTodo'; } function createCardMovedTrigger(config: CardMovedConfig): TriggerHandler { @@ -26,6 +28,11 @@ function createCardMovedTrigger(config: CardMovedConfig): TriggerHandler { if (ctx.source !== 'trello') return false; if (!isTrelloWebhookPayload(ctx.payload)) return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveTrelloTriggerEnabled(ctx.project.trello?.triggers, config.triggerConfigKey)) { + return false; + } + const payload = ctx.payload; const targetListId = ctx.project.trello?.lists[config.listKey]; @@ -68,6 +75,7 @@ export const CardMovedToBriefingTrigger = createCardMovedTrigger({ description: 'Triggers briefing agent when card moved to briefing list', listKey: 'briefing', agentType: 'briefing', + triggerConfigKey: 'cardMovedToBriefing', }); export const CardMovedToPlanningTrigger = createCardMovedTrigger({ @@ -75,6 +83,7 @@ export const CardMovedToPlanningTrigger = createCardMovedTrigger({ description: 'Triggers planning agent when card moved to planning list', listKey: 'planning', agentType: 'planning', + triggerConfigKey: 'cardMovedToPlanning', }); export const CardMovedToTodoTrigger = createCardMovedTrigger({ @@ -82,4 +91,5 @@ export const CardMovedToTodoTrigger = createCardMovedTrigger({ description: 'Triggers implementation agent when card moved to TODO list', listKey: 'todo', agentType: 'implementation', + triggerConfigKey: 'cardMovedToTodo', }); diff --git a/src/triggers/trello/comment-mention.ts b/src/triggers/trello/comment-mention.ts index 18c5f234..d116a0c1 100644 --- a/src/triggers/trello/comment-mention.ts +++ b/src/triggers/trello/comment-mention.ts @@ -1,3 +1,4 @@ +import { resolveTrelloTriggerEnabled } from '../../config/triggerConfig.js'; import { trelloClient } from '../../trello/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; @@ -35,6 +36,11 @@ export class TrelloCommentMentionTrigger implements TriggerHandler { if (ctx.source !== 'trello') return false; if (!isTrelloWebhookPayload(ctx.payload)) return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveTrelloTriggerEnabled(ctx.project.trello?.triggers, 'commentMention')) { + return false; + } + return ctx.payload.action.type === 'commentCard'; } diff --git a/src/triggers/trello/label-added.ts b/src/triggers/trello/label-added.ts index 8a490330..d1df5c79 100644 --- a/src/triggers/trello/label-added.ts +++ b/src/triggers/trello/label-added.ts @@ -1,3 +1,4 @@ +import { resolveTrelloTriggerEnabled } from '../../config/triggerConfig.js'; import { trelloClient } from '../../trello/client.js'; import { logger } from '../../utils/logging.js'; import type { @@ -16,6 +17,11 @@ export class ReadyToProcessLabelTrigger implements TriggerHandler { if (ctx.source !== 'trello') return false; if (!isTrelloWebhookPayload(ctx.payload)) return false; + // Check trigger config — default enabled for backward compatibility + if (!resolveTrelloTriggerEnabled(ctx.project.trello?.triggers, 'readyToProcessLabel')) { + return false; + } + const payload = ctx.payload; const readyLabelId = ctx.project.trello?.labels.readyToProcess; diff --git a/tests/unit/config/triggerConfig.test.ts b/tests/unit/config/triggerConfig.test.ts new file mode 100644 index 00000000..c2acf941 --- /dev/null +++ b/tests/unit/config/triggerConfig.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, it } from 'vitest'; +import { + GitHubTriggerConfigSchema, + JiraTriggerConfigSchema, + TrelloTriggerConfigSchema, + resolveGitHubTriggerEnabled, + resolveJiraTriggerEnabled, + resolveTrelloTriggerEnabled, +} from '../../../src/config/triggerConfig.js'; + +describe('TrelloTriggerConfigSchema', () => { + it('defaults all fields to true', () => { + const result = TrelloTriggerConfigSchema.parse({}); + expect(result).toEqual({ + cardMovedToBriefing: true, + cardMovedToPlanning: true, + cardMovedToTodo: true, + readyToProcessLabel: true, + commentMention: true, + }); + }); + + it('accepts explicit false values', () => { + const result = TrelloTriggerConfigSchema.parse({ + cardMovedToPlanning: false, + readyToProcessLabel: false, + }); + expect(result.cardMovedToPlanning).toBe(false); + expect(result.readyToProcessLabel).toBe(false); + expect(result.cardMovedToBriefing).toBe(true); // default still true + }); +}); + +describe('JiraTriggerConfigSchema', () => { + it('defaults all fields to true', () => { + const result = JiraTriggerConfigSchema.parse({}); + expect(result).toEqual({ + issueTransitioned: true, + readyToProcessLabel: true, + commentMention: true, + }); + }); +}); + +describe('GitHubTriggerConfigSchema', () => { + it('defaults existing triggers to true', () => { + const result = GitHubTriggerConfigSchema.parse({}); + expect(result.checkSuiteSuccess).toBe(true); + expect(result.checkSuiteFailure).toBe(true); + expect(result.prReviewSubmitted).toBe(true); + expect(result.prCommentMention).toBe(true); + expect(result.prReadyToMerge).toBe(true); + expect(result.prMerged).toBe(true); + }); + + it('defaults new opt-in triggers to false', () => { + const result = GitHubTriggerConfigSchema.parse({}); + expect(result.reviewRequested).toBe(false); + expect(result.prOpened).toBe(false); + }); +}); + +describe('resolveTrelloTriggerEnabled', () => { + it('returns true when config is undefined (backward compatible)', () => { + expect(resolveTrelloTriggerEnabled(undefined, 'cardMovedToBriefing')).toBe(true); + expect(resolveTrelloTriggerEnabled(undefined, 'readyToProcessLabel')).toBe(true); + expect(resolveTrelloTriggerEnabled(undefined, 'commentMention')).toBe(true); + }); + + it('returns true when key is not present in config', () => { + expect(resolveTrelloTriggerEnabled({}, 'cardMovedToBriefing')).toBe(true); + }); + + it('returns false when key is explicitly disabled', () => { + expect(resolveTrelloTriggerEnabled({ cardMovedToBriefing: false }, 'cardMovedToBriefing')).toBe( + false, + ); + }); + + it('returns true when key is explicitly enabled', () => { + expect(resolveTrelloTriggerEnabled({ cardMovedToPlanning: true }, 'cardMovedToPlanning')).toBe( + true, + ); + }); +}); + +describe('resolveJiraTriggerEnabled', () => { + it('returns true when config is undefined (backward compatible)', () => { + expect(resolveJiraTriggerEnabled(undefined, 'issueTransitioned')).toBe(true); + expect(resolveJiraTriggerEnabled(undefined, 'readyToProcessLabel')).toBe(true); + expect(resolveJiraTriggerEnabled(undefined, 'commentMention')).toBe(true); + }); + + it('returns false when key is explicitly disabled', () => { + expect(resolveJiraTriggerEnabled({ issueTransitioned: false }, 'issueTransitioned')).toBe( + false, + ); + }); + + it('returns true when config is empty (no explicit settings)', () => { + expect(resolveJiraTriggerEnabled({}, 'issueTransitioned')).toBe(true); + }); +}); + +describe('resolveGitHubTriggerEnabled', () => { + it('returns true for existing triggers when config is undefined', () => { + expect(resolveGitHubTriggerEnabled(undefined, 'checkSuiteSuccess')).toBe(true); + expect(resolveGitHubTriggerEnabled(undefined, 'checkSuiteFailure')).toBe(true); + expect(resolveGitHubTriggerEnabled(undefined, 'prReviewSubmitted')).toBe(true); + expect(resolveGitHubTriggerEnabled(undefined, 'prCommentMention')).toBe(true); + expect(resolveGitHubTriggerEnabled(undefined, 'prReadyToMerge')).toBe(true); + expect(resolveGitHubTriggerEnabled(undefined, 'prMerged')).toBe(true); + }); + + it('returns false for opt-in triggers when config is undefined', () => { + expect(resolveGitHubTriggerEnabled(undefined, 'reviewRequested')).toBe(false); + expect(resolveGitHubTriggerEnabled(undefined, 'prOpened')).toBe(false); + }); + + it('returns false for opt-in triggers when config is empty', () => { + expect(resolveGitHubTriggerEnabled({}, 'reviewRequested')).toBe(false); + expect(resolveGitHubTriggerEnabled({}, 'prOpened')).toBe(false); + }); + + it('returns true for opt-in triggers when explicitly enabled', () => { + expect(resolveGitHubTriggerEnabled({ reviewRequested: true }, 'reviewRequested')).toBe(true); + expect(resolveGitHubTriggerEnabled({ prOpened: true }, 'prOpened')).toBe(true); + }); + + it('returns false when existing trigger is explicitly disabled', () => { + expect(resolveGitHubTriggerEnabled({ checkSuiteSuccess: false }, 'checkSuiteSuccess')).toBe( + false, + ); + }); +}); diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts index 5bcdbc36..b860f03c 100644 --- a/tests/unit/triggers/pr-opened.test.ts +++ b/tests/unit/triggers/pr-opened.test.ts @@ -22,12 +22,20 @@ describe('PROpenedTrigger', () => { }, }; + /** Project with prOpened trigger explicitly enabled via github.triggers config */ + const mockProjectWithPrOpenedEnabled = { + ...mockProject, + github: { + triggers: { prOpened: true }, + }, + }; + beforeEach(() => { vi.clearAllMocks(); }); describe('matches', () => { - it('matches when action is opened and not draft', () => { + it('does not match by default (opt-in trigger, disabled without config)', () => { const ctx: TriggerContext = { project: mockProject, source: 'github', @@ -50,12 +58,38 @@ describe('PROpenedTrigger', () => { }, }; + expect(trigger.matches(ctx)).toBe(false); + }); + + it('matches when action is opened and not draft with prOpened enabled', () => { + const ctx: TriggerContext = { + project: mockProjectWithPrOpenedEnabled, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc123' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + expect(trigger.matches(ctx)).toBe(true); }); it('does not match when source is not github', () => { const ctx: TriggerContext = { - project: mockProject, + project: mockProjectWithPrOpenedEnabled, source: 'trello', payload: {}, }; @@ -65,7 +99,7 @@ describe('PROpenedTrigger', () => { it('does not match when action is not opened', () => { const ctx: TriggerContext = { - project: mockProject, + project: mockProjectWithPrOpenedEnabled, source: 'github', payload: { action: 'closed', @@ -91,7 +125,7 @@ describe('PROpenedTrigger', () => { it('does not match draft PRs', () => { const ctx: TriggerContext = { - project: mockProject, + project: mockProjectWithPrOpenedEnabled, source: 'github', payload: { action: 'opened', @@ -117,7 +151,7 @@ describe('PROpenedTrigger', () => { it('does not match non-PR payloads', () => { const ctx: TriggerContext = { - project: mockProject, + project: mockProjectWithPrOpenedEnabled, source: 'github', payload: { action: 'opened', @@ -132,7 +166,7 @@ describe('PROpenedTrigger', () => { describe('handle', () => { it('returns result when PR body has Trello URL', async () => { const ctx: TriggerContext = { - project: mockProject, + project: mockProjectWithPrOpenedEnabled, source: 'github', payload: { action: 'opened', @@ -173,7 +207,7 @@ describe('PROpenedTrigger', () => { it('returns null when PR body has no Trello URL', async () => { const ctx: TriggerContext = { - project: mockProject, + project: mockProjectWithPrOpenedEnabled, source: 'github', payload: { action: 'opened', @@ -201,7 +235,7 @@ describe('PROpenedTrigger', () => { it('handles null PR body', async () => { const ctx: TriggerContext = { - project: mockProject, + project: mockProjectWithPrOpenedEnabled, source: 'github', payload: { action: 'opened', diff --git a/tests/unit/triggers/review-requested.test.ts b/tests/unit/triggers/review-requested.test.ts new file mode 100644 index 00000000..802b3663 --- /dev/null +++ b/tests/unit/triggers/review-requested.test.ts @@ -0,0 +1,199 @@ +import { describe, expect, it } from 'vitest'; +import { ReviewRequestedTrigger } from '../../../src/triggers/github/review-requested.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +describe('ReviewRequestedTrigger', () => { + const trigger = new ReviewRequestedTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + }, + labels: {}, + }, + // Review-requested is opt-in, default disabled + }; + + /** Project with reviewRequested trigger explicitly enabled */ + const mockProjectWithReviewRequested = { + ...mockProject, + github: { + triggers: { reviewRequested: true }, + }, + }; + + const mockPersonaIdentities = { + implementer: 'cascade-impl', + reviewer: 'cascade-reviewer', + }; + + const makeReviewRequestedPayload = (reviewerLogin = 'cascade-reviewer') => ({ + action: 'review_requested', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'Implements https://trello.com/c/abc123/card-name', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc123' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + requested_reviewer: { login: reviewerLogin }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }); + + describe('matches', () => { + it('does not match by default (opt-in trigger, disabled without config)', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewRequestedPayload(), + personaIdentities: mockPersonaIdentities, + }; + expect(trigger.matches(ctx)).toBe(false); + }); + + it('matches when review_requested and trigger is enabled', () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: makeReviewRequestedPayload(), + personaIdentities: mockPersonaIdentities, + }; + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match when source is not github', () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'trello', + payload: makeReviewRequestedPayload(), + personaIdentities: mockPersonaIdentities, + }; + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match on non-review_requested actions', () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: { + ...makeReviewRequestedPayload(), + action: 'opened', + }, + personaIdentities: mockPersonaIdentities, + }; + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match when payload is not a PR payload', () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: { action: 'review_requested', something: 'else' }, + personaIdentities: mockPersonaIdentities, + }; + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('returns null when no persona identities', async () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: makeReviewRequestedPayload(), + // no personaIdentities + }; + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + }); + + it('returns null when requested reviewer is not a CASCADE persona', async () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: makeReviewRequestedPayload('human-reviewer'), + personaIdentities: mockPersonaIdentities, + }; + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + }); + + it('returns null when no requested reviewer in payload', async () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: { + ...makeReviewRequestedPayload(), + requested_reviewer: undefined, + }, + personaIdentities: mockPersonaIdentities, + }; + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + }); + + it('returns null when PR body has no Trello card URL', async () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: { + ...makeReviewRequestedPayload(), + pull_request: { + ...makeReviewRequestedPayload().pull_request, + body: 'No card URL here', + }, + }, + personaIdentities: mockPersonaIdentities, + }; + const result = await trigger.handle(ctx); + expect(result).toBeNull(); + }); + + it('triggers review agent when reviewer persona is requested', async () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: makeReviewRequestedPayload('cascade-reviewer'), + personaIdentities: mockPersonaIdentities, + }; + const result = await trigger.handle(ctx); + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('review'); + expect(result?.prNumber).toBe(42); + expect(result?.cardId).toBe('abc123'); + expect(result?.agentInput).toMatchObject({ + prNumber: 42, + repoFullName: 'owner/repo', + triggerType: 'ci-success', + cardId: 'abc123', + }); + }); + + it('triggers review agent when implementer persona is requested', async () => { + const ctx: TriggerContext = { + project: mockProjectWithReviewRequested, + source: 'github', + payload: makeReviewRequestedPayload('cascade-impl'), + personaIdentities: mockPersonaIdentities, + }; + const result = await trigger.handle(ctx); + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('review'); + }); + }); +}); diff --git a/web/src/components/projects/integration-form.tsx b/web/src/components/projects/integration-form.tsx index 3ae52db7..b443f58b 100644 --- a/web/src/components/projects/integration-form.tsx +++ b/web/src/components/projects/integration-form.tsx @@ -82,6 +82,51 @@ function fromKVPairs(pairs: KVPair[]): Record { return result; } +interface TriggerToggleItem { + key: string; + label: string; + description: string; + defaultValue: boolean; +} + +function TriggerToggles({ + title, + items, + values, + onChange, +}: { + title: string; + items: TriggerToggleItem[]; + values: Record; + onChange: (values: Record) => void; +}) { + return ( +
+ + {items.map((item) => { + const value = item.key in values ? values[item.key] : item.defaultValue; + return ( +
+ onChange({ ...values, [item.key]: e.target.checked })} + className="mt-0.5 h-4 w-4 rounded border-input" + /> +
+ +

{item.description}

+
+
+ ); + })} +
+ ); +} + type IntegrationType = 'trello' | 'jira' | 'github'; function TrelloForm({ @@ -97,6 +142,7 @@ function TrelloForm({ const [lists, setLists] = useState([]); const [labels, setLabels] = useState([]); const [costField, setCostField] = useState(''); + const [trelloTriggers, setTrelloTriggers] = useState>({}); useEffect(() => { if (initialConfig) { @@ -105,6 +151,7 @@ function TrelloForm({ setLabels(toKVPairs(initialConfig.labels as Record)); const cf = initialConfig.customFields as Record | undefined; setCostField(cf?.cost ?? ''); + setTrelloTriggers((initialConfig.triggers as Record) ?? {}); } }, [initialConfig]); @@ -118,6 +165,7 @@ function TrelloForm({ lists: fromKVPairs(lists), labels: fromKVPairs(labels), ...(costField ? { customFields: { cost: costField } } : {}), + ...(Object.keys(trelloTriggers).length > 0 ? { triggers: trelloTriggers } : {}), }, }), onSuccess: () => { @@ -152,6 +200,45 @@ function TrelloForm({ /> + +
+ +
-
- -
- - - - Env Var Key - Credential - Scope - - - - - {overrides.length === 0 && ( - - - No overrides configured — using org defaults - - - )} - {overrides.map((o) => ( - - {o.envVarKey} - {o.credentialName} - - {o.agentType ? ( - {o.agentType} - ) : ( - Project-wide - )} - - - - - - ))} - -
-
- - - - - Add Credential Override - -
{ - e.preventDefault(); - setMutation.mutate(); - }} - className="space-y-4" - > -
- - setEnvVarKey(e.target.value)} - placeholder="GITHUB_TOKEN" - required - /> -
-
- - -
-
- - setAgentType(e.target.value)} - placeholder="Leave empty for project-wide" - /> -
-
- - -
- {setMutation.isError && ( -

{setMutation.error.message}

- )} -
- - - - ); -} diff --git a/web/src/components/projects/integration-form.tsx b/web/src/components/projects/integration-form.tsx index 5a1cad3d..e3a1803f 100644 --- a/web/src/components/projects/integration-form.tsx +++ b/web/src/components/projects/integration-form.tsx @@ -127,145 +127,280 @@ function TriggerToggles({ ); } -type IntegrationType = 'trello' | 'jira' | 'github'; +type IntegrationCategory = 'pm' | 'scm'; -function TrelloForm({ - projectId, - initialConfig, +interface CredentialOption { + id: number; + name: string; + envVarKey: string; + value: string; +} + +function CredentialSelector({ + label, + description, + credentials, + selectedId, + onChange, + verifiedLogin, + onVerify, + isVerifying, + verifyError, }: { - projectId: string; - initialConfig?: Record; + label: string; + description: string; + credentials: CredentialOption[]; + selectedId: number | null; + onChange: (id: number | null) => void; + verifiedLogin?: string | null; + onVerify?: () => void; + isVerifying?: boolean; + verifyError?: string | null; }) { - const queryClient = useQueryClient(); + return ( +
+ +

{description}

+
+ + {onVerify && ( + + )} +
+ {verifiedLogin && ( +
+ + Resolved: {verifiedLogin} +
+ )} + {verifyError && ( +
+ + {verifyError} +
+ )} +
+ ); +} - const [boardId, setBoardId] = useState(''); - const [lists, setLists] = useState([]); - const [labels, setLabels] = useState([]); - const [costField, setCostField] = useState(''); - const [trelloTriggers, setTrelloTriggers] = useState>({}); +// ============================================================================ +// Provider-specific credential role definitions +// ============================================================================ - useEffect(() => { - if (initialConfig) { - setBoardId((initialConfig.boardId as string) ?? ''); - setLists(toKVPairs(initialConfig.lists as Record)); - setLabels(toKVPairs(initialConfig.labels as Record)); - const cf = initialConfig.customFields as Record | undefined; - setCostField(cf?.cost ?? ''); - setTrelloTriggers((initialConfig.triggers as Record) ?? {}); - } - }, [initialConfig]); +interface CredentialRoleDef { + role: string; + label: string; + description: string; + hasVerify?: boolean; +} - const upsertMutation = useMutation({ - mutationFn: () => - trpcClient.projects.integrations.upsert.mutate({ - projectId, - type: 'trello', - config: { - boardId, - lists: fromKVPairs(lists), - labels: fromKVPairs(labels), - ...(costField ? { customFields: { cost: costField } } : {}), - ...(Object.keys(trelloTriggers).length > 0 ? { triggers: trelloTriggers } : {}), - }, - }), - onSuccess: () => { - queryClient.invalidateQueries({ - queryKey: trpc.projects.integrations.list.queryOptions({ projectId }).queryKey, - }); +const PM_CREDENTIAL_ROLES: Record = { + trello: [ + { role: 'api_key', label: 'API Key', description: 'Trello API Key for authentication.' }, + { role: 'token', label: 'Token', description: 'Trello token for authorization.' }, + ], + jira: [ + { role: 'email', label: 'Email', description: 'JIRA account email for authentication.' }, + { role: 'api_token', label: 'API Token', description: 'JIRA API token for authorization.' }, + ], +}; + +const SCM_CREDENTIAL_ROLES: Record = { + github: [ + { + role: 'implementer_token', + label: 'Implementer Token', + description: 'GitHub PAT for the bot that writes code, creates PRs, and responds to reviews.', + hasVerify: true, }, - }); + { + role: 'reviewer_token', + label: 'Reviewer Token', + description: 'GitHub PAT for the bot that reviews PRs. Must be a different account.', + hasVerify: true, + }, + ], +}; - return ( -
-
- - setBoardId(e.target.value)} - placeholder="Trello board ID" - /> -
+// ============================================================================ +// Integration credential slot component +// ============================================================================ - - +function IntegrationCredentialSlots({ + projectId, + category, + roles, + credentials, + existingCredentials, + onCredentialsChange, +}: { + projectId: string; + category: IntegrationCategory; + roles: CredentialRoleDef[]; + credentials: CredentialOption[]; + existingCredentials: Map; + onCredentialsChange: (role: string, credentialId: number | null) => void; +}) { + const [verifiedLogins, setVerifiedLogins] = useState>({}); + const [verifyErrors, setVerifyErrors] = useState>({}); + const [verifyingRoles, setVerifyingRoles] = useState>({}); + + const handleVerify = async (role: string, credentialId: number) => { + setVerifyingRoles((prev) => ({ ...prev, [role]: true })); + try { + const result = await trpcClient.credentials.verifyGithubIdentity.mutate({ + credentialId, + }); + setVerifiedLogins((prev) => ({ ...prev, [role]: result.login })); + setVerifyErrors((prev) => ({ ...prev, [role]: null })); + } catch (err) { + setVerifiedLogins((prev) => ({ ...prev, [role]: null })); + setVerifyErrors((prev) => ({ + ...prev, + [role]: err instanceof Error ? err.message : String(err), + })); + } finally { + setVerifyingRoles((prev) => ({ ...prev, [role]: false })); + } + }; -
- - setCostField(e.target.value)} - placeholder="Custom field ID for cost tracking" + return ( +
+ + {roles.map((roleDef) => ( + { + onCredentialsChange(roleDef.role, id); + setVerifiedLogins((prev) => ({ ...prev, [roleDef.role]: null })); + setVerifyErrors((prev) => ({ ...prev, [roleDef.role]: null })); + }} + verifiedLogin={roleDef.hasVerify ? verifiedLogins[roleDef.role] : undefined} + onVerify={ + roleDef.hasVerify + ? () => { + const credId = existingCredentials.get(roleDef.role); + if (credId) handleVerify(roleDef.role, credId); + } + : undefined + } + isVerifying={roleDef.hasVerify ? verifyingRoles[roleDef.role] : undefined} + verifyError={roleDef.hasVerify ? verifyErrors[roleDef.role] : undefined} /> -
- - - -
- - {upsertMutation.isSuccess && Saved} - {upsertMutation.isError && ( - {upsertMutation.error.message} - )} -
+ ))}
); } -function JiraForm({ +// ============================================================================ +// PM Tab (Trello / JIRA) +// ============================================================================ + +const TRELLO_TRIGGERS: TriggerToggleItem[] = [ + { + key: 'cardMovedToBriefing', + label: 'Card moved to Briefing', + description: 'Trigger briefing agent when card is moved to the briefing list.', + defaultValue: true, + }, + { + key: 'cardMovedToPlanning', + label: 'Card moved to Planning', + description: 'Trigger planning agent when card is moved to the planning list.', + defaultValue: true, + }, + { + key: 'cardMovedToTodo', + label: 'Card moved to Todo', + description: 'Trigger implementation agent when card is moved to the todo list.', + defaultValue: true, + }, + { + key: 'readyToProcessLabel', + label: 'Ready to Process label', + description: 'Trigger agent when the "Ready to Process" label is added.', + defaultValue: true, + }, + { + key: 'commentMention', + label: 'Comment @mention', + description: 'Trigger respond-to-planning-comment when the bot is @mentioned in a comment.', + defaultValue: true, + }, +]; + +const JIRA_TRIGGERS: TriggerToggleItem[] = [ + { + key: 'issueTransitioned', + label: 'Issue transitioned', + description: 'Trigger agent when an issue transitions to a configured status.', + defaultValue: true, + }, + { + key: 'readyToProcessLabel', + label: 'Ready to Process label', + description: 'Trigger agent when the ready-to-process label is added.', + defaultValue: true, + }, + { + key: 'commentMention', + label: 'Comment @mention', + description: 'Trigger respond-to-planning-comment when the bot is @mentioned in a comment.', + defaultValue: true, + }, +]; + +function PMTab({ projectId, + initialProvider, initialConfig, + initialTriggers, + initialCredentials, }: { projectId: string; + initialProvider: string; initialConfig?: Record; + initialTriggers?: Record; + initialCredentials: Map; }) { const queryClient = useQueryClient(); + const credentialsQuery = useQuery(trpc.credentials.list.queryOptions()); + const orgCredentials = (credentialsQuery.data ?? []) as CredentialOption[]; + + const [provider, setProvider] = useState(initialProvider || 'trello'); + const [triggers, setTriggers] = useState>(initialTriggers ?? {}); + const [credentialMap, setCredentialMap] = useState>(initialCredentials); + + // Trello fields + const [boardId, setBoardId] = useState(''); + const [lists, setLists] = useState([]); + const [labels, setLabels] = useState([]); + const [costField, setCostField] = useState(''); + + // Jira fields const [jiraProjectKey, setJiraProjectKey] = useState(''); const [baseUrl, setBaseUrl] = useState(''); const [statuses, setStatuses] = useState([]); @@ -276,308 +411,352 @@ function JiraForm({ { key: 'error', value: 'cascade-error' }, { key: 'readyToProcess', value: 'cascade-ready' }, ]); - const [costField, setCostField] = useState(''); - const [jiraTriggers, setJiraTriggers] = useState>({}); + const [jiraCostField, setJiraCostField] = useState(''); useEffect(() => { - if (initialConfig) { + if (initialConfig && initialProvider === 'trello') { + setBoardId((initialConfig.boardId as string) ?? ''); + setLists(toKVPairs(initialConfig.lists as Record)); + setLabels(toKVPairs(initialConfig.labels as Record)); + const cf = initialConfig.customFields as Record | undefined; + setCostField(cf?.cost ?? ''); + } else if (initialConfig && initialProvider === 'jira') { setJiraProjectKey((initialConfig.projectKey as string) ?? ''); setBaseUrl((initialConfig.baseUrl as string) ?? ''); setStatuses(toKVPairs(initialConfig.statuses as Record)); setIssueTypes(toKVPairs(initialConfig.issueTypes as Record)); - const labels = initialConfig.labels as Record | undefined; - if (labels) { - setJiraLabels(toKVPairs(labels)); - } + const jl = initialConfig.labels as Record | undefined; + if (jl) setJiraLabels(toKVPairs(jl)); const cf = initialConfig.customFields as Record | undefined; - setCostField(cf?.cost ?? ''); - setJiraTriggers((initialConfig.triggers as Record) ?? {}); + setJiraCostField(cf?.cost ?? ''); } - }, [initialConfig]); + }, [initialConfig, initialProvider]); - const upsertMutation = useMutation({ - mutationFn: () => - trpcClient.projects.integrations.upsert.mutate({ - projectId, - type: 'jira', - config: { + useEffect(() => { + setTriggers(initialTriggers ?? {}); + }, [initialTriggers]); + + useEffect(() => { + setCredentialMap(initialCredentials); + }, [initialCredentials]); + + const saveMutation = useMutation({ + // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: handles multiple provider types + credential linking + mutationFn: async () => { + let config: Record; + if (provider === 'trello') { + config = { + boardId, + lists: fromKVPairs(lists), + labels: fromKVPairs(labels), + ...(costField ? { customFields: { cost: costField } } : {}), + }; + } else { + config = { projectKey: jiraProjectKey, baseUrl, statuses: fromKVPairs(statuses), ...(issueTypes.length > 0 ? { issueTypes: fromKVPairs(issueTypes) } : {}), ...(jiraLabels.length > 0 ? { labels: fromKVPairs(jiraLabels) } : {}), - ...(costField ? { customFields: { cost: costField } } : {}), - ...(Object.keys(jiraTriggers).length > 0 ? { triggers: jiraTriggers } : {}), - }, - }), + ...(jiraCostField ? { customFields: { cost: jiraCostField } } : {}), + }; + } + + const result = await trpcClient.projects.integrations.upsert.mutate({ + projectId, + category: 'pm', + provider, + config, + triggers, + }); + + // Set integration credentials + for (const [role, credentialId] of credentialMap) { + await trpcClient.projects.integrationCredentials.set.mutate({ + projectId, + category: 'pm', + role, + credentialId, + }); + } + + return result; + }, onSuccess: () => { queryClient.invalidateQueries({ queryKey: trpc.projects.integrations.list.queryOptions({ projectId }).queryKey, }); + queryClient.invalidateQueries({ + queryKey: trpc.projects.integrationCredentials.list.queryOptions({ + projectId, + category: 'pm', + }).queryKey, + }); }, }); + const credentialRoles = PM_CREDENTIAL_ROLES[provider] ?? []; + return (
- - setJiraProjectKey(e.target.value)} - placeholder="e.g., PROJ" - /> -
- -
- - setBaseUrl(e.target.value)} - placeholder="https://your-instance.atlassian.net" - /> + +
- -

- Map CASCADE statuses (briefing, planning, todo, inProgress, inReview, done, merged) to JIRA - status names. -

- - - - -

- JIRA label names used by CASCADE. Keys: processing, processed, error, readyToProcess. -

+ {provider === 'trello' && ( + <> +
+ + setBoardId(e.target.value)} + placeholder="Trello board ID" + /> +
+ + +
+ + setCostField(e.target.value)} + placeholder="Custom field ID for cost tracking" + /> +
+ + + )} -
- - setCostField(e.target.value)} - placeholder="e.g., customfield_10042" - /> -
+ {provider === 'jira' && ( + <> +
+ + setJiraProjectKey(e.target.value)} + placeholder="e.g., PROJ" + /> +
+
+ + setBaseUrl(e.target.value)} + placeholder="https://your-instance.atlassian.net" + /> +
+ +

+ Map CASCADE statuses (briefing, planning, todo, inProgress, inReview, done, merged) to + JIRA status names. +

+ + +

+ JIRA label names used by CASCADE. Keys: processing, processed, error, readyToProcess. +

+
+ + setJiraCostField(e.target.value)} + placeholder="e.g., customfield_10042" + /> +
+ + + )} - { + setCredentialMap((prev) => { + const next = new Map(prev); + if (id) { + next.set(role, id); + } else { + next.delete(role); + } + return next; + }); + }} />
- {upsertMutation.isSuccess && Saved} - {upsertMutation.isError && ( - {upsertMutation.error.message} + {saveMutation.isSuccess && Saved} + {saveMutation.isError && ( + {saveMutation.error.message} )}
); } -interface CredentialOption { - id: number; - name: string; - envVarKey: string; - value: string; -} - -function CredentialSelector({ - label, - description, - credentials, - selectedId, - onChange, - verifiedLogin, - onVerify, - isVerifying, - verifyError, -}: { - label: string; - description: string; - credentials: CredentialOption[]; - selectedId: number | null; - onChange: (id: number | null) => void; - verifiedLogin: string | null; - onVerify: () => void; - isVerifying: boolean; - verifyError: string | null; -}) { - return ( -
- -

{description}

-
- - -
- {verifiedLogin && ( -
- - Resolved: {verifiedLogin} -
- )} - {verifyError && ( -
- - {verifyError} -
- )} -
- ); -} - -function GitHubForm({ +// ============================================================================ +// SCM Tab (GitHub) +// ============================================================================ + +const GITHUB_TRIGGERS: TriggerToggleItem[] = [ + { + key: 'checkSuiteSuccess', + label: 'Check Suite Success', + description: 'Trigger review agent when all CI checks pass.', + defaultValue: true, + }, + { + key: 'checkSuiteFailure', + label: 'Check Suite Failure', + description: 'Trigger respond-to-ci agent when CI checks fail.', + defaultValue: true, + }, + { + key: 'prReviewSubmitted', + label: 'PR Review Submitted', + description: 'Trigger respond-to-review when a review with changes requested is submitted.', + defaultValue: true, + }, + { + key: 'prCommentMention', + label: 'PR Comment @mention', + description: + 'Trigger respond-to-pr-comment when the implementer bot is @mentioned in a comment.', + defaultValue: true, + }, + { + key: 'prReadyToMerge', + label: 'PR Ready to Merge', + description: 'Auto-move card to DONE when PR is approved and checks pass.', + defaultValue: true, + }, + { + key: 'prMerged', + label: 'PR Merged', + description: 'Auto-move card to MERGED when PR is merged.', + defaultValue: true, + }, + { + key: 'reviewRequested', + label: 'Review Requested (opt-in)', + description: + 'Trigger review agent when review is requested from a CASCADE persona. Default disabled.', + defaultValue: false, + }, + { + key: 'prOpened', + label: 'PR Opened (opt-in)', + description: 'Trigger respond-to-review when a new PR is opened. Default disabled.', + defaultValue: false, + }, +]; + +function SCMTab({ projectId, - initialConfig, + initialProvider, + initialTriggers, + initialCredentials, }: { projectId: string; - initialConfig?: Record; + initialProvider: string; + initialTriggers?: Record; + initialCredentials: Map; }) { const queryClient = useQueryClient(); const credentialsQuery = useQuery(trpc.credentials.list.queryOptions()); - const credentials = (credentialsQuery.data ?? []) as CredentialOption[]; - - const [implementerCredId, setImplementerCredId] = useState(null); - const [reviewerCredId, setReviewerCredId] = useState(null); - - const [implementerLogin, setImplementerLogin] = useState(null); - const [reviewerLogin, setReviewerLogin] = useState(null); - - const [implementerError, setImplementerError] = useState(null); - const [reviewerError, setReviewerError] = useState(null); + const orgCredentials = (credentialsQuery.data ?? []) as CredentialOption[]; - const [githubTriggers, setGithubTriggers] = useState>({}); + const [provider] = useState(initialProvider || 'github'); + const [triggers, setTriggers] = useState>(initialTriggers ?? {}); + const [credentialMap, setCredentialMap] = useState>(initialCredentials); useEffect(() => { - if (initialConfig) { - setImplementerCredId((initialConfig.implementerCredentialId as number) ?? null); - setReviewerCredId((initialConfig.reviewerCredentialId as number) ?? null); - setGithubTriggers((initialConfig.triggers as Record) ?? {}); - } - }, [initialConfig]); - - const verifyImplementer = useMutation({ - mutationFn: () => - trpcClient.credentials.verifyGithubIdentity.mutate({ - credentialId: implementerCredId as number, - }), - onSuccess: (data) => { - setImplementerLogin(data.login); - setImplementerError(null); - }, - onError: (err) => { - setImplementerLogin(null); - setImplementerError(err.message); - }, - }); + setTriggers(initialTriggers ?? {}); + }, [initialTriggers]); - const verifyReviewer = useMutation({ - mutationFn: () => - trpcClient.credentials.verifyGithubIdentity.mutate({ - credentialId: reviewerCredId as number, - }), - onSuccess: (data) => { - setReviewerLogin(data.login); - setReviewerError(null); - }, - onError: (err) => { - setReviewerLogin(null); - setReviewerError(err.message); - }, - }); + useEffect(() => { + setCredentialMap(initialCredentials); + }, [initialCredentials]); const saveMutation = useMutation({ mutationFn: async () => { - if (!implementerCredId || !reviewerCredId) { - throw new Error('Both persona credentials are required'); - } - - // Save as GitHub integration - await trpcClient.projects.integrations.upsert.mutate({ + const result = await trpcClient.projects.integrations.upsert.mutate({ projectId, - type: 'github', - config: { - implementerCredentialId: implementerCredId, - reviewerCredentialId: reviewerCredId, - ...(Object.keys(githubTriggers).length > 0 ? { triggers: githubTriggers } : {}), - }, + category: 'scm', + provider, + config: {}, + triggers, }); - // Set project-wide credential overrides for the persona token keys - await trpcClient.projects.credentialOverrides.set.mutate({ - projectId, - envVarKey: 'GITHUB_TOKEN_IMPLEMENTER', - credentialId: implementerCredId, - }); + // Set integration credentials + for (const [role, credentialId] of credentialMap) { + await trpcClient.projects.integrationCredentials.set.mutate({ + projectId, + category: 'scm', + role, + credentialId, + }); + } - await trpcClient.projects.credentialOverrides.set.mutate({ - projectId, - envVarKey: 'GITHUB_TOKEN_REVIEWER', - credentialId: reviewerCredId, - }); + return result; }, onSuccess: () => { queryClient.invalidateQueries({ queryKey: trpc.projects.integrations.list.queryOptions({ projectId }).queryKey, }); queryClient.invalidateQueries({ - queryKey: trpc.projects.credentialOverrides.list.queryOptions({ projectId }).queryKey, + queryKey: trpc.projects.integrationCredentials.list.queryOptions({ + projectId, + category: 'scm', + }).queryKey, }); }, }); + const credentialRoles = SCM_CREDENTIAL_ROLES[provider] ?? []; + return (

@@ -586,109 +765,37 @@ function GitHubForm({ reviews PRs and can approve or request changes.

- { - setImplementerCredId(id); - setImplementerLogin(null); - setImplementerError(null); - }} - verifiedLogin={implementerLogin} - onVerify={() => verifyImplementer.mutate()} - isVerifying={verifyImplementer.isPending} - verifyError={implementerError} - /> - - { - setReviewerCredId(id); - setReviewerLogin(null); - setReviewerError(null); + { + setCredentialMap((prev) => { + const next = new Map(prev); + if (id) { + next.set(role, id); + } else { + next.delete(role); + } + return next; + }); }} - verifiedLogin={reviewerLogin} - onVerify={() => verifyReviewer.mutate()} - isVerifying={verifyReviewer.isPending} - verifyError={reviewerError} /> - {implementerLogin && reviewerLogin && implementerLogin === reviewerLogin && ( -

- Both tokens resolve to the same GitHub user ({implementerLogin}). They must be different - accounts for loop prevention to work. -

- )} -
-
- {activeTab === 'trello' && ( - } - /> - )} - - {activeTab === 'jira' && ( - } + initialProvider={pmProvider} + initialConfig={pmIntegration?.config as Record} + initialTriggers={pmIntegration?.triggers as Record} + initialCredentials={pmCredMap} /> )} - {activeTab === 'github' && ( - } + initialProvider={scmProvider} + initialTriggers={scmIntegration?.triggers as Record} + initialCredentials={scmCredMap} /> )}
diff --git a/web/src/routes/projects/$projectId.tsx b/web/src/routes/projects/$projectId.tsx index ace51541..1926fcdd 100644 --- a/web/src/routes/projects/$projectId.tsx +++ b/web/src/routes/projects/$projectId.tsx @@ -1,4 +1,3 @@ -import { CredentialOverrides } from '@/components/projects/credential-overrides.js'; import { IntegrationForm } from '@/components/projects/integration-form.js'; import { ProjectAgentConfigs } from '@/components/projects/project-agent-configs.js'; import { ProjectGeneralForm } from '@/components/projects/project-general-form.js'; @@ -10,7 +9,7 @@ import { ArrowLeft } from 'lucide-react'; import { useState } from 'react'; import { rootRoute } from '../__root.js'; -type Tab = 'general' | 'integrations' | 'credentials' | 'agent-configs'; +type Tab = 'general' | 'integrations' | 'agent-configs'; function ProjectDetailPage() { const { projectId } = projectDetailRoute.useParams(); @@ -31,7 +30,6 @@ function ProjectDetailPage() { const tabs: { id: Tab; label: string }[] = [ { id: 'general', label: 'General' }, { id: 'integrations', label: 'Integrations' }, - { id: 'credentials', label: 'Credentials' }, { id: 'agent-configs', label: 'Agent Configs' }, ]; @@ -71,7 +69,6 @@ function ProjectDetailPage() { {activeTab === 'general' && } {activeTab === 'integrations' && } - {activeTab === 'credentials' && } {activeTab === 'agent-configs' && }
);