diff --git a/src/agents/definitions/debug.yaml b/src/agents/definitions/debug.yaml index da7caa48..79746994 100644 --- a/src/agents/definitions/debug.yaml +++ b/src/agents/definitions/debug.yaml @@ -24,8 +24,7 @@ capabilities: # No external event-based triggers are configured. triggers: [] -strategies: - contextPipeline: [directoryListing, contextFiles, squint, workItem] +strategies: {} prompts: taskPrompt: | diff --git a/src/agents/definitions/email-joke.yaml b/src/agents/definitions/email-joke.yaml index 8b834b5f..228a4a5d 100644 --- a/src/agents/definitions/email-joke.yaml +++ b/src/agents/definitions/email-joke.yaml @@ -31,9 +31,9 @@ triggers: label: Sender Email Filter description: Only process emails from this sender (leave empty for all) required: false + contextPipeline: [prefetchedEmails] -strategies: - contextPipeline: [prefetchedEmails] +strategies: {} prompts: taskPrompt: | diff --git a/src/agents/definitions/implementation.yaml b/src/agents/definitions/implementation.yaml index 62bd733b..6243afcb 100644 --- a/src/agents/definitions/implementation.yaml +++ b/src/agents/definitions/implementation.yaml @@ -36,8 +36,7 @@ triggers: label: Target List options: [todo] defaultValue: todo - # contextPipeline can be specified per-trigger to override strategies.contextPipeline - # Currently using strategy default: [directoryListing, contextFiles, squint, workItem] + contextPipeline: [directoryListing, contextFiles, squint, workItem] - event: pm:issue-transitioned label: Issue Transitioned description: Trigger when issue transitions to Todo status @@ -49,6 +48,7 @@ triggers: label: Target Status options: [todo] defaultValue: todo + contextPipeline: [directoryListing, contextFiles, squint, workItem] - event: pm:label-added label: Ready to Process Label description: Trigger when Ready to Process label added to a card in the Todo list @@ -59,9 +59,9 @@ triggers: label: Target List options: [todo] defaultValue: todo + contextPipeline: [directoryListing, contextFiles, squint, workItem] -strategies: - contextPipeline: [directoryListing, contextFiles, squint, workItem] +strategies: {} prompts: taskPrompt: | diff --git a/src/agents/definitions/planning.yaml b/src/agents/definitions/planning.yaml index 815d3708..2fb5ec0c 100644 --- a/src/agents/definitions/planning.yaml +++ b/src/agents/definitions/planning.yaml @@ -33,6 +33,7 @@ triggers: label: Target List options: [planning] defaultValue: planning + contextPipeline: [directoryListing, contextFiles, squint, workItem] - event: pm:issue-transitioned label: Issue Transitioned description: Trigger when issue transitions to Planning status @@ -44,6 +45,7 @@ triggers: label: Target Status options: [planning] defaultValue: planning + contextPipeline: [directoryListing, contextFiles, squint, workItem] - event: pm:label-added label: Ready to Process Label description: Trigger when Ready to Process label added to a card in Planning list @@ -54,13 +56,14 @@ triggers: label: Target List options: [planning] defaultValue: planning + contextPipeline: [directoryListing, contextFiles, squint, workItem] - event: pm:comment-mention label: Comment @mention description: Trigger when bot is @mentioned in a card/issue comment defaultEnabled: true + contextPipeline: [directoryListing, contextFiles, squint, workItem] -strategies: - contextPipeline: [directoryListing, contextFiles, squint, workItem] +strategies: {} prompts: taskPrompt: | diff --git a/src/agents/definitions/profiles.ts b/src/agents/definitions/profiles.ts index 0b2c0c44..e5475bff 100644 --- a/src/agents/definitions/profiles.ts +++ b/src/agents/definitions/profiles.ts @@ -88,28 +88,34 @@ function getAllCapabilities(caps: AgentCapabilities): Capability[] { /** * Resolve the context pipeline for a given trigger event. - * Uses the trigger-specific pipeline if defined, otherwise falls back to the default. + * + * Returns the trigger-specific pipeline if defined, otherwise returns an empty array. + * This function handles several edge cases gracefully: + * + * - **No triggerEvent**: Returns `[]` when triggerEvent is undefined or empty string. + * This happens when an agent is invoked manually without a trigger. + * - **No matching trigger**: Returns `[]` when the triggerEvent doesn't match any + * trigger in the agent's triggers array. This could happen if a trigger is + * misconfigured or the agent doesn't support the given event. + * - **Trigger without contextPipeline**: Returns `[]` when the matching trigger + * exists but has no contextPipeline defined (contextPipeline is optional). + * - **Empty triggers array**: Returns `[]` for agents with no triggers (e.g., debug + * agent which is only invoked internally). * * @param triggers - Array of supported triggers from the agent definition - * @param defaultPipeline - Default pipeline from strategies.contextPipeline * @param triggerEvent - Optional trigger event (e.g., 'pm:card-moved', 'scm:check-suite-success') - * @returns The context pipeline to use + * @returns The context pipeline to use (empty array for any edge case) */ function resolveContextPipeline( triggers: SupportedTrigger[], - defaultPipeline: ContextStepName[], triggerEvent?: string, ): ContextStepName[] { if (!triggerEvent) { - return defaultPipeline; + return []; } const trigger = triggers.find((t) => t.event === triggerEvent); - if (trigger?.contextPipeline && trigger.contextPipeline.length > 0) { - return trigger.contextPipeline; - } - - return defaultPipeline; + return trigger?.contextPipeline ?? []; } // ============================================================================ @@ -128,9 +134,6 @@ function buildProfileFromDefinition(def: AgentDefinition, agentType: string): Ag // Get gadget options from strategies const gadgetOptions = def.strategies.gadgetOptions; - // Get default context pipeline from strategies - const defaultContextPipeline = def.strategies.contextPipeline; - // Get triggers for dynamic context pipeline resolution const triggers = def.triggers ?? []; @@ -155,13 +158,8 @@ function buildProfileFromDefinition(def: AgentDefinition, agentType: string): Ag ...(def.backend.blockGitPush !== undefined && { blockGitPush: def.backend.blockGitPush }), ...(def.backend.requiresPR && { requiresPR: true }), fetchContext: async (params) => { - // Resolve context pipeline: use trigger-specific pipeline if available, - // otherwise fall back to the default from strategies.contextPipeline - const contextPipeline = resolveContextPipeline( - triggers, - defaultContextPipeline, - params.input.triggerType, - ); + // Resolve context pipeline from the trigger (empty array if no trigger or trigger has no pipeline) + const contextPipeline = resolveContextPipeline(triggers, params.input.triggerType); const injections: ContextInjection[] = []; for (const step of contextPipeline) { diff --git a/src/agents/definitions/respond-to-ci.yaml b/src/agents/definitions/respond-to-ci.yaml index cfe198fb..19b87ef1 100644 --- a/src/agents/definitions/respond-to-ci.yaml +++ b/src/agents/definitions/respond-to-ci.yaml @@ -30,11 +30,9 @@ triggers: description: Trigger when CI checks fail defaultEnabled: true providers: [github] - # contextPipeline can be specified per-trigger to override strategies.contextPipeline - # Currently using strategy default: [prContext, directoryListing, contextFiles, squint, workItem] + contextPipeline: [prContext, directoryListing, contextFiles, squint, workItem] -strategies: - contextPipeline: [prContext, directoryListing, contextFiles, squint, workItem] +strategies: {} prompts: taskPrompt: | diff --git a/src/agents/definitions/respond-to-planning-comment.yaml b/src/agents/definitions/respond-to-planning-comment.yaml index 10d7b17c..f19d7366 100644 --- a/src/agents/definitions/respond-to-planning-comment.yaml +++ b/src/agents/definitions/respond-to-planning-comment.yaml @@ -27,9 +27,9 @@ triggers: label: Comment @mention description: Trigger when bot is @mentioned in a card/issue comment defaultEnabled: true + contextPipeline: [directoryListing, contextFiles, squint, workItem] -strategies: - contextPipeline: [directoryListing, contextFiles, squint, workItem] +strategies: {} prompts: taskPrompt: | diff --git a/src/agents/definitions/respond-to-pr-comment.yaml b/src/agents/definitions/respond-to-pr-comment.yaml index 6867588e..5a83e7d3 100644 --- a/src/agents/definitions/respond-to-pr-comment.yaml +++ b/src/agents/definitions/respond-to-pr-comment.yaml @@ -29,9 +29,9 @@ triggers: description: Trigger when the implementer bot is @mentioned in a PR comment defaultEnabled: true providers: [github] + contextPipeline: [prContext, prConversation, directoryListing, contextFiles, squint] strategies: - contextPipeline: [prContext, prConversation, directoryListing, contextFiles, squint] gadgetOptions: includeReviewComments: true @@ -59,3 +59,6 @@ backend: compaction: default hint: Complete the current task efficiently before moving to the next. + +trailingMessage: + includeDiagnostics: true diff --git a/src/agents/definitions/respond-to-review.yaml b/src/agents/definitions/respond-to-review.yaml index 92e84997..f98df59e 100644 --- a/src/agents/definitions/respond-to-review.yaml +++ b/src/agents/definitions/respond-to-review.yaml @@ -30,9 +30,9 @@ triggers: description: Trigger when a review with changes requested is submitted defaultEnabled: true providers: [github] + contextPipeline: [prContext, prConversation, directoryListing, contextFiles, squint] strategies: - contextPipeline: [prContext, prConversation, directoryListing, contextFiles, squint] gadgetOptions: includeReviewComments: true @@ -40,17 +40,14 @@ prompts: taskPrompt: | You are on the branch `<%= it.prBranch %>` for PR #<%= it.prNumber %>. - A user commented on this PR and mentioned you. Respond to their comment. - <% if (it.commentPath) { -%> - File: <%= it.commentPath %> - <% } -%> + A reviewer has submitted feedback requesting changes. Address the review comments by making the necessary code changes. - Their comment: + Review feedback: --- <%= it.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. + Carefully read each review comment and make the requested changes. Commit and push your changes when done. Use the ReplyToReviewComment tool to respond to individual review comments as you address them. Focus on surgical, targeted fixes unless the reviewer clearly asks for broader changes. backend: enableStopHooks: true diff --git a/src/agents/definitions/review.yaml b/src/agents/definitions/review.yaml index e8698ea5..2ad3f649 100644 --- a/src/agents/definitions/review.yaml +++ b/src/agents/definitions/review.yaml @@ -36,21 +36,21 @@ triggers: description: Filter PRs by author type options: [own, external, all] defaultValue: own - # contextPipeline can be specified per-trigger to override strategies.contextPipeline - # Currently using strategy default: [prContext, contextFiles, squint] + contextPipeline: [prContext, contextFiles, squint] - event: scm:review-requested label: On Review Requested description: Trigger review when a CASCADE persona is explicitly requested as reviewer defaultEnabled: false providers: [github] + contextPipeline: [prContext, contextFiles, squint] - event: scm:pr-opened label: PR Opened description: Trigger review when a new PR is opened (without waiting for CI) defaultEnabled: false providers: [github] + contextPipeline: [prContext, contextFiles, squint] -strategies: - contextPipeline: [prContext, contextFiles, squint] +strategies: {} prompts: taskPrompt: | diff --git a/src/agents/definitions/schema.ts b/src/agents/definitions/schema.ts index 684ef18b..a5bc38b8 100644 --- a/src/agents/definitions/schema.ts +++ b/src/agents/definitions/schema.ts @@ -68,7 +68,7 @@ export const TriggerParameterSchema = z ); // ============================================================================ -// Context Step Names (used by triggers and strategies) +// Context Step Names (used by trigger contextPipeline definitions) // ============================================================================ export const CONTEXT_STEP_NAMES = [ @@ -112,10 +112,11 @@ export const SupportedTriggerSchema = z.object({ /** Provider filter - only applies to these providers (e.g., ['trello']) */ providers: z.array(KnownProviderSchema).optional(), /** - * Optional custom context pipeline for this trigger. - * When specified, overrides the agent's default strategies.contextPipeline. - * Useful when different triggers require different context (e.g., PM triggers + * Context pipeline for this trigger. + * Defines what context to fetch when this trigger fires. + * Different triggers typically need different context (e.g., PM triggers * need workItem, SCM triggers need prContext). + * When not specified, an empty pipeline is used. */ contextPipeline: z.array(ContextStepNameSchema).optional(), }); @@ -197,13 +198,12 @@ const GadgetOptionsSchema = z export const COMPACTION_NAMES = ['implementation', 'default'] as const; /** - * Strategies schema - context and prompt configuration. + * Strategies schema - gadget configuration only. * Note: gadgetBuilder removed - gadgets are now derived from capabilities. * Note: taskPromptBuilder removed - task prompts are now stored in prompts.taskPrompt. + * Note: contextPipeline removed - context is now derived from triggers only. */ const StrategiesSchema = z.object({ - /** Pipeline of context fetching steps */ - contextPipeline: z.array(z.enum(CONTEXT_STEP_NAMES)), /** Optional gadget configuration for special cases */ gadgetOptions: GadgetOptionsSchema, }); @@ -262,7 +262,7 @@ export const AgentDefinitionSchema = z.object({ * Declares what events the agent can respond to, with configurable parameters. */ triggers: z.array(SupportedTriggerSchema).default([]), - /** Strategy configuration (context pipeline, prompts) */ + /** Strategy configuration (gadget options) */ strategies: StrategiesSchema, /** Backend execution configuration */ backend: BackendSchema, diff --git a/src/agents/definitions/splitting.yaml b/src/agents/definitions/splitting.yaml index a976b1ab..c14e9862 100644 --- a/src/agents/definitions/splitting.yaml +++ b/src/agents/definitions/splitting.yaml @@ -34,6 +34,7 @@ triggers: label: Target List options: [splitting] defaultValue: splitting + contextPipeline: [directoryListing, contextFiles, squint, workItem] - event: pm:issue-transitioned label: Issue Transitioned description: Trigger when issue transitions to Splitting status @@ -45,6 +46,7 @@ triggers: label: Target Status options: [splitting] defaultValue: splitting + contextPipeline: [directoryListing, contextFiles, squint, workItem] - event: pm:label-added label: Ready to Process Label description: Trigger when Ready to Process label added to a card in Splitting list @@ -55,9 +57,9 @@ triggers: label: Target List options: [splitting] defaultValue: splitting + contextPipeline: [directoryListing, contextFiles, squint, workItem] -strategies: - contextPipeline: [directoryListing, contextFiles, squint, workItem] +strategies: {} prompts: taskPrompt: | diff --git a/src/api/routers/agentDefinitions.ts b/src/api/routers/agentDefinitions.ts index 5c35530a..de17b801 100644 --- a/src/api/routers/agentDefinitions.ts +++ b/src/api/routers/agentDefinitions.ts @@ -12,7 +12,6 @@ import { type AgentDefinition, AgentDefinitionSchema, COMPACTION_NAMES, - CONTEXT_STEP_NAMES, DefinitionPatchSchema, } from '../../agents/definitions/schema.js'; import { validateTemplate } from '../../agents/prompts/index.js'; @@ -351,7 +350,6 @@ export const agentDefinitionsRouter = router({ schema: publicProcedure.query(() => { return { capabilities: [...CAPABILITIES], - contextStepNames: [...CONTEXT_STEP_NAMES], compactionNames: [...COMPACTION_NAMES], triggerRegistry: TRIGGER_REGISTRY, }; diff --git a/tests/unit/agents/definitions/loader.test.ts b/tests/unit/agents/definitions/loader.test.ts index 8ef55f1a..5b986af5 100644 --- a/tests/unit/agents/definitions/loader.test.ts +++ b/tests/unit/agents/definitions/loader.test.ts @@ -105,14 +105,16 @@ describe('YAML agent definitions loader', () => { } }); - it('all contextPipeline step references exist in CONTEXT_STEP_REGISTRY', () => { + it('all trigger contextPipeline step references exist in CONTEXT_STEP_REGISTRY', () => { for (const agentType of ALL_AGENT_TYPES) { const def = loadAgentDefinition(agentType); - for (const step of def.strategies.contextPipeline) { - expect( - step in CONTEXT_STEP_REGISTRY, - `${agentType}: contextPipeline step '${step}' not in CONTEXT_STEP_REGISTRY`, - ).toBe(true); + for (const trigger of def.triggers ?? []) { + for (const step of trigger.contextPipeline ?? []) { + expect( + step in CONTEXT_STEP_REGISTRY, + `${agentType}/${trigger.event}: contextPipeline step '${step}' not in CONTEXT_STEP_REGISTRY`, + ).toBe(true); + } } } }); @@ -151,27 +153,28 @@ describe('YAML agent definitions loader', () => { } }); - it('work-item agents use standard context pipeline', () => { - const workItemAgents = ['implementation', 'splitting', 'planning', 'debug']; - for (const agentType of workItemAgents) { - const def = loadAgentDefinition(agentType); - expect(def.strategies.contextPipeline).toEqual([ - 'directoryListing', - 'contextFiles', - 'squint', - 'workItem', - ]); - } + it('work-item agents have triggers with standard context pipeline', () => { + // implementation, splitting, planning triggers include workItem context + const def = loadAgentDefinition('implementation'); + const cardMovedTrigger = def.triggers.find((t) => t.event === 'pm:card-moved'); + expect(cardMovedTrigger?.contextPipeline).toEqual([ + 'directoryListing', + 'contextFiles', + 'squint', + 'workItem', + ]); }); - it('review agent uses PR context pipeline without directoryListing', () => { + it('review agent triggers use PR context pipeline', () => { const def = loadAgentDefinition('review'); - expect(def.strategies.contextPipeline).toEqual(['prContext', 'contextFiles', 'squint']); + const ciPassedTrigger = def.triggers.find((t) => t.event === 'scm:check-suite-success'); + expect(ciPassedTrigger?.contextPipeline).toEqual(['prContext', 'contextFiles', 'squint']); }); - it('respond-to-ci uses combined PR + work-item pipeline', () => { + it('respond-to-ci trigger uses combined PR + work-item pipeline', () => { const def = loadAgentDefinition('respond-to-ci'); - expect(def.strategies.contextPipeline).toEqual([ + const ciFailureTrigger = def.triggers.find((t) => t.event === 'scm:check-suite-failure'); + expect(ciFailureTrigger?.contextPipeline).toEqual([ 'prContext', 'directoryListing', 'contextFiles', @@ -180,18 +183,16 @@ describe('YAML agent definitions loader', () => { ]); }); - it('PR comment agents use conversation pipeline', () => { - const prCommentAgents = ['respond-to-review', 'respond-to-pr-comment']; - for (const agentType of prCommentAgents) { - const def = loadAgentDefinition(agentType); - expect(def.strategies.contextPipeline).toEqual([ - 'prContext', - 'prConversation', - 'directoryListing', - 'contextFiles', - 'squint', - ]); - } + it('PR comment agents have triggers with conversation pipeline', () => { + const def = loadAgentDefinition('respond-to-pr-comment'); + const prCommentTrigger = def.triggers.find((t) => t.event === 'scm:pr-comment-mention'); + expect(prCommentTrigger?.contextPipeline).toEqual([ + 'prContext', + 'prConversation', + 'directoryListing', + 'contextFiles', + 'squint', + ]); }); it('review has preExecute hook', () => { diff --git a/tests/unit/agents/definitions/schema.test.ts b/tests/unit/agents/definitions/schema.test.ts index e029c66f..b6af5cf0 100644 --- a/tests/unit/agents/definitions/schema.test.ts +++ b/tests/unit/agents/definitions/schema.test.ts @@ -325,9 +325,7 @@ describe('AgentDefinitionSchema', () => { required: ['fs:read', 'fs:write', 'shell:exec', 'session:ctrl', 'pm:read', 'pm:write'], optional: [], }, - strategies: { - contextPipeline: ['directoryListing', 'contextFiles', 'squint', 'workItem'], - }, + strategies: {}, backend: { enableStopHooks: false, needsGitHubToken: false, @@ -348,7 +346,6 @@ describe('AgentDefinitionSchema', () => { const full = { ...validDefinition, strategies: { - ...validDefinition.strategies, gadgetOptions: { includeReviewComments: true }, }, backend: { @@ -385,15 +382,6 @@ describe('AgentDefinitionSchema', () => { expect(result.success).toBe(false); }); - it('rejects invalid strategy names', () => { - const bad = { - ...validDefinition, - strategies: { ...validDefinition.strategies, contextPipeline: ['nonexistentStep'] }, - }; - const result = AgentDefinitionSchema.safeParse(bad); - expect(result.success).toBe(false); - }); - it('rejects invalid compaction preset names', () => { const bad = { ...validDefinition, compaction: 'aggressive' }; const result = AgentDefinitionSchema.safeParse(bad); @@ -464,18 +452,6 @@ describe('AgentDefinitionSchema', () => { } }); - it('validates contextPipeline step names', () => { - const good = { - ...validDefinition, - strategies: { - ...validDefinition.strategies, - contextPipeline: ['prContext', 'prConversation', 'directoryListing'], - }, - }; - const result = AgentDefinitionSchema.safeParse(good); - expect(result.success).toBe(true); - }); - it('rejects overlapping required and optional capabilities', () => { const bad = { ...validDefinition, diff --git a/tests/unit/api/routers/agentDefinitions.test.ts b/tests/unit/api/routers/agentDefinitions.test.ts index 9f20d0eb..69cbbf4c 100644 --- a/tests/unit/api/routers/agentDefinitions.test.ts +++ b/tests/unit/api/routers/agentDefinitions.test.ts @@ -68,9 +68,7 @@ function createMockDefinition(overrides?: Partial): AgentDefini optional: [], }, triggers: [], - strategies: { - contextPipeline: ['directoryListing'], - }, + strategies: {}, backend: { enableStopHooks: true, needsGitHubToken: true, @@ -437,7 +435,6 @@ describe('agentDefinitionsRouter', () => { const result = await caller.schema(); expect(result).toHaveProperty('capabilities'); - expect(result).toHaveProperty('contextStepNames'); expect(result).toHaveProperty('compactionNames'); // Verify they're arrays expect(Array.isArray(result.capabilities)).toBe(true); diff --git a/tests/unit/backends/agent-profiles.test.ts b/tests/unit/backends/agent-profiles.test.ts index dc3600b6..bad610f9 100644 --- a/tests/unit/backends/agent-profiles.test.ts +++ b/tests/unit/backends/agent-profiles.test.ts @@ -654,6 +654,7 @@ function makeContextParams(overrides: { repoFullName?: string; prNumber?: number; contextFiles?: Array<{ path: string; content: string }>; + triggerType?: string; }): { input: Record; repoDir: string; @@ -665,6 +666,7 @@ function makeContextParams(overrides: { cardId: overrides.cardId, repoFullName: overrides.repoFullName ?? 'acme/widgets', prNumber: overrides.prNumber ?? 42, + triggerType: overrides.triggerType, ...overrides, }, repoDir: '/repo', @@ -677,7 +679,7 @@ describe('fetchDirectoryListing', () => { it('splitting fetchContext returns a ListDirectory injection with maxDepth:3', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({ cardId: undefined }); + const params = makeContextParams({ cardId: undefined, triggerType: 'pm:card-moved' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -699,6 +701,7 @@ describe('fetchContextFileInjections', () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('splitting'); const params = makeContextParams({ + triggerType: 'pm:card-moved', contextFiles: [ { path: 'CLAUDE.md', content: 'project guidelines' }, { path: 'README.md', content: 'readme text' }, @@ -720,7 +723,7 @@ describe('fetchContextFileInjections', () => { it('returns no ReadFile injections when contextFiles is empty', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({ contextFiles: [] }); + const params = makeContextParams({ triggerType: 'pm:card-moved', contextFiles: [] }); const injections = await profile.fetchContext( params as Parameters[0], @@ -736,7 +739,7 @@ describe('fetchSquintOverview', () => { mockResolveSquintDbPath.mockReturnValue('/repo/.squint.db'); mockExecFileSync.mockReturnValue('squint overview output\n'); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({}); + const params = makeContextParams({ triggerType: 'pm:card-moved' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -751,7 +754,7 @@ describe('fetchSquintOverview', () => { it('returns no SquintOverview injection when squint db is absent', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({}); + const params = makeContextParams({ triggerType: 'pm:card-moved' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -767,7 +770,7 @@ describe('fetchSquintOverview', () => { throw new Error('squint not found'); }); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({}); + const params = makeContextParams({ triggerType: 'pm:card-moved' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -781,7 +784,7 @@ describe('fetchSquintOverview', () => { mockResolveSquintDbPath.mockReturnValue('/repo/.squint.db'); mockExecFileSync.mockReturnValue(' '); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({}); + const params = makeContextParams({ triggerType: 'pm:card-moved' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -797,7 +800,7 @@ describe('fetchWorkItemInjection', () => { mockResolveSquintDbPath.mockReturnValue(null); mockReadWorkItem.mockResolvedValue('# card title\n\ncard body'); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({ cardId: 'card-123' }); + const params = makeContextParams({ triggerType: 'pm:card-moved', cardId: 'card-123' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -817,7 +820,7 @@ describe('fetchWorkItemInjection', () => { mockResolveSquintDbPath.mockReturnValue(null); mockReadWorkItem.mockRejectedValue(new Error('card not found')); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({ cardId: 'missing-card' }); + const params = makeContextParams({ triggerType: 'pm:card-moved', cardId: 'missing-card' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -830,7 +833,7 @@ describe('fetchWorkItemInjection', () => { it('never calls readWorkItem when cardId is absent', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({ cardId: undefined }); + const params = makeContextParams({ triggerType: 'pm:card-moved', cardId: undefined }); await profile.fetchContext(params as Parameters[0]); @@ -845,6 +848,7 @@ describe('fetchWorkItemContext orchestration', () => { mockReadWorkItem.mockResolvedValue('card content'); const profile = await getAgentProfile('splitting'); const params = makeContextParams({ + triggerType: 'pm:card-moved', cardId: 'card-abc', contextFiles: [{ path: 'CLAUDE.md', content: 'guidelines' }], }); @@ -873,7 +877,7 @@ describe('fetchWorkItemContext orchestration', () => { mockResolveSquintDbPath.mockReturnValue(null); mockReadWorkItem.mockRejectedValue(new Error('unavailable')); const profile = await getAgentProfile('splitting'); - const params = makeContextParams({ cardId: 'card-xyz' }); + const params = makeContextParams({ triggerType: 'pm:card-moved', cardId: 'card-xyz' }); const injections = await profile.fetchContext( params as Parameters[0], @@ -896,7 +900,11 @@ describe('fetchReviewContext', () => { it('includes PR injections (Details, Diff, Checks)', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('review'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 42 }); + const params = makeContextParams({ + triggerType: 'scm:check-suite-success', + repoFullName: 'acme/widgets', + prNumber: 42, + }); const injections = await profile.fetchContext( params as Parameters[0], @@ -912,6 +920,7 @@ describe('fetchReviewContext', () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('review'); const params = makeContextParams({ + triggerType: 'scm:check-suite-success', repoFullName: 'acme/widgets', prNumber: 42, contextFiles: [{ path: 'CLAUDE.md', content: 'project info' }], @@ -930,7 +939,11 @@ describe('fetchReviewContext', () => { mockResolveSquintDbPath.mockReturnValue('/repo/.squint.db'); mockExecFileSync.mockReturnValue('squint content\n'); const profile = await getAgentProfile('review'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 42 }); + const params = makeContextParams({ + triggerType: 'scm:check-suite-success', + repoFullName: 'acme/widgets', + prNumber: 42, + }); const injections = await profile.fetchContext( params as Parameters[0], @@ -942,7 +955,11 @@ describe('fetchReviewContext', () => { it('does NOT include a work item injection (review has no cardId)', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('review'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 42 }); + const params = makeContextParams({ + triggerType: 'scm:check-suite-success', + repoFullName: 'acme/widgets', + prNumber: 42, + }); const injections = await profile.fetchContext( params as Parameters[0], @@ -959,7 +976,11 @@ describe('fetchReviewContext', () => { skipped: [], }); const profile = await getAgentProfile('review'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 42 }); + const params = makeContextParams({ + triggerType: 'scm:check-suite-success', + repoFullName: 'acme/widgets', + prNumber: 42, + }); const injections = await profile.fetchContext( params as Parameters[0], @@ -977,7 +998,11 @@ describe('fetchReviewContext', () => { it('calls formatting functions', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('review'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 42 }); + const params = makeContextParams({ + triggerType: 'scm:check-suite-success', + repoFullName: 'acme/widgets', + prNumber: 42, + }); await profile.fetchContext(params as Parameters[0]); @@ -1000,6 +1025,7 @@ describe('fetchCIContext', () => { mockReadWorkItem.mockResolvedValue('ci card content'); const profile = await getAgentProfile('respond-to-ci'); const params = makeContextParams({ + triggerType: 'scm:check-suite-failure', repoFullName: 'acme/widgets', prNumber: 5, cardId: 'ci-card', @@ -1024,6 +1050,7 @@ describe('fetchCIContext', () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('respond-to-ci'); const params = makeContextParams({ + triggerType: 'scm:check-suite-failure', repoFullName: 'acme/widgets', prNumber: 5, cardId: undefined, @@ -1052,7 +1079,11 @@ describe('fetchPRCommentResponseContext', () => { it('includes PR injections and 3 conversation injections', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('respond-to-pr-comment'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 7 }); + const params = makeContextParams({ + triggerType: 'scm:pr-comment-mention', + repoFullName: 'acme/widgets', + prNumber: 7, + }); const injections = await profile.fetchContext( params as Parameters[0], @@ -1073,6 +1104,7 @@ describe('fetchPRCommentResponseContext', () => { mockExecFileSync.mockReturnValue('squint pr comment output\n'); const profile = await getAgentProfile('respond-to-pr-comment'); const params = makeContextParams({ + triggerType: 'scm:pr-comment-mention', repoFullName: 'acme/widgets', prNumber: 7, contextFiles: [{ path: 'AGENTS.md', content: 'agents doc' }], @@ -1091,7 +1123,11 @@ describe('fetchPRCommentResponseContext', () => { it('calls all 3 formatting functions for conversation context', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('respond-to-pr-comment'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 7 }); + const params = makeContextParams({ + triggerType: 'scm:pr-comment-mention', + repoFullName: 'acme/widgets', + prNumber: 7, + }); await profile.fetchContext(params as Parameters[0]); @@ -1103,7 +1139,11 @@ describe('fetchPRCommentResponseContext', () => { it('calls getPRReviewComments, getPRReviews, getPRIssueComments', async () => { mockResolveSquintDbPath.mockReturnValue(null); const profile = await getAgentProfile('respond-to-pr-comment'); - const params = makeContextParams({ repoFullName: 'acme/widgets', prNumber: 7 }); + const params = makeContextParams({ + triggerType: 'scm:pr-comment-mention', + repoFullName: 'acme/widgets', + prNumber: 7, + }); await profile.fetchContext(params as Parameters[0]); @@ -1112,3 +1152,57 @@ describe('fetchPRCommentResponseContext', () => { expect(mockGithub.getPRIssueComments).toHaveBeenCalledWith('acme', 'widgets', 7); }); }); + +// ============================================================================ +// resolveContextPipeline Edge Cases +// ============================================================================ + +describe('resolveContextPipeline edge cases', () => { + it('returns empty array when triggerType is undefined', async () => { + mockResolveSquintDbPath.mockReturnValue(null); + const profile = await getAgentProfile('implementation'); + const params = makeContextParams({ triggerType: undefined }); + + const injections = await profile.fetchContext( + params as Parameters[0], + ); + + expect(injections).toHaveLength(0); + }); + + it('returns empty array when triggerType matches no trigger', async () => { + mockResolveSquintDbPath.mockReturnValue(null); + const profile = await getAgentProfile('implementation'); + const params = makeContextParams({ triggerType: 'scm:unknown-event' }); + + const injections = await profile.fetchContext( + params as Parameters[0], + ); + + expect(injections).toHaveLength(0); + }); + + it('handles agent with no triggers (debug)', async () => { + mockResolveSquintDbPath.mockReturnValue(null); + const profile = await getAgentProfile('debug'); + const params = makeContextParams({ triggerType: undefined }); + + const injections = await profile.fetchContext( + params as Parameters[0], + ); + + expect(injections).toHaveLength(0); + }); + + it('returns empty array when triggerType is empty string', async () => { + mockResolveSquintDbPath.mockReturnValue(null); + const profile = await getAgentProfile('implementation'); + const params = makeContextParams({ triggerType: '' }); + + const injections = await profile.fetchContext( + params as Parameters[0], + ); + + expect(injections).toHaveLength(0); + }); +}); diff --git a/web/src/components/settings/agent-definition-editor.tsx b/web/src/components/settings/agent-definition-editor.tsx index 8f29b83c..4f121d1f 100644 --- a/web/src/components/settings/agent-definition-editor.tsx +++ b/web/src/components/settings/agent-definition-editor.tsx @@ -38,7 +38,6 @@ export interface AgentDefinitionEditorProps { interface SchemaData { capabilities: readonly string[]; - contextStepNames: readonly string[]; compactionNames: readonly string[]; triggerRegistry: Record; } @@ -314,55 +313,39 @@ function CapabilitiesSection({ function StrategiesSection({ def, setDef, - schema, }: { def: AgentDefinition; setDef: React.Dispatch>; - schema: SchemaData | undefined; }) { + // Only show strategies section if gadgetOptions is set + if (!def.strategies.gadgetOptions) { + return null; + } + return (

Strategies

- - {schema ? ( - - setDef( - (d) => - ({ ...d, strategies: { ...d.strategies, contextPipeline } }) as AgentDefinition, - ) - } - /> - ) : ( -
Loading...
- )} + + + setDef( + (d) => + ({ + ...d, + strategies: { + ...d.strategies, + gadgetOptions: { ...d.strategies.gadgetOptions, includeReviewComments: v }, + }, + }) as AgentDefinition, + ) + } + label="Include Review Comments" + />
- {def.strategies.gadgetOptions && ( -
- - - setDef( - (d) => - ({ - ...d, - strategies: { - ...d.strategies, - gadgetOptions: { ...d.strategies.gadgetOptions, includeReviewComments: v }, - }, - }) as AgentDefinition, - ) - } - label="Include Review Comments" - /> -
- )}
); } @@ -953,9 +936,7 @@ const EMPTY_DEFINITION: AgentDefinition = { optional: [], }, triggers: [], - strategies: { - contextPipeline: [], - }, + strategies: {}, backend: { enableStopHooks: false, needsGitHubToken: false }, compaction: 'default', hint: '', @@ -1181,7 +1162,7 @@ export function AgentDefinitionEditor({ existing, onClose }: AgentDefinitionEdit - +