From 36c9cb995bac314807486a71c466535c0b821d5d Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Tue, 7 Apr 2026 18:36:54 +0900 Subject: [PATCH] fix(mcp-server): suppress dispatch-ready execution while gated (#1422) When executionGate.gated=true, the parse_mode response now suppresses expensive specialist dispatch metadata to prevent clients from proceeding with costly execution during clarification: - dispatchReady is fully removed (no Task-tool-ready params leak) - executionPlan is fully removed (no execution plan metadata leaks) - parallelAgentsRecommendation.dispatch is downgraded to "deferred" - Specialist names and hints are preserved for transparency Added suppressDispatchWhileGated() pure function in execution-gate.ts with 14 new tests covering gated suppression, ungated pass-through, and edge cases. --- .../src/mcp/handlers/execution-gate.spec.ts | 244 +++++++++++++++++- .../src/mcp/handlers/execution-gate.ts | 70 +++++ .../src/mcp/handlers/mode.handler.spec.ts | 16 +- .../src/mcp/handlers/mode.handler.ts | 31 ++- 4 files changed, 353 insertions(+), 8 deletions(-) diff --git a/apps/mcp-server/src/mcp/handlers/execution-gate.spec.ts b/apps/mcp-server/src/mcp/handlers/execution-gate.spec.ts index dcb4a94c..b763db2a 100644 --- a/apps/mcp-server/src/mcp/handlers/execution-gate.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/execution-gate.spec.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from 'vitest'; -import { evaluateExecutionGate, type ExecutionGateInput } from './execution-gate'; +import { + evaluateExecutionGate, + suppressDispatchWhileGated, + type ExecutionGateInput, + type ExecutionGate, + type GatedResponseFields, +} from './execution-gate'; import type { ClarificationMetadata } from './clarification-gate'; import type { PlanningStageMetadata } from './planning-stage'; @@ -273,4 +279,240 @@ describe('execution-gate', () => { }); }); }); + + // ==================================================================== + // suppressDispatchWhileGated (#1422) + // ==================================================================== + + describe('suppressDispatchWhileGated', () => { + const sampleDispatchReady = { + primaryAgent: { + name: 'software-engineer', + displayName: 'Software Engineer', + description: 'Software Engineer - PLAN mode', + dispatchParams: { + subagent_type: 'general-purpose' as const, + prompt: 'You are a software engineer...', + description: 'Software Engineer - PLAN mode', + }, + }, + parallelAgents: [ + { + name: 'security-specialist', + displayName: 'Security Specialist', + description: 'Security specialist', + dispatchParams: { + subagent_type: 'general-purpose' as const, + prompt: 'You are a security specialist...', + description: 'Security specialist', + run_in_background: true as const, + }, + }, + { + name: 'performance-specialist', + displayName: 'Performance Specialist', + description: 'Performance specialist', + dispatchParams: { + subagent_type: 'general-purpose' as const, + prompt: 'You are a performance specialist...', + description: 'Performance specialist', + run_in_background: true as const, + }, + }, + ], + }; + + const sampleParallelRecommendation = { + specialists: ['security-specialist', 'performance-specialist'], + hint: 'Dispatch specialists for review', + dispatch: 'auto' as const, + suggestedStack: 'review', + stackBased: true, + }; + + // ------------------------------------------------------------------ + // Gated: suppression + // ------------------------------------------------------------------ + + describe('when gated=true', () => { + const gatedGate: ExecutionGate = { + gated: true, + reason: 'Request is ambiguous', + unblockCondition: 'Resolve clarification', + deferredSpecialists: ['security-specialist', 'performance-specialist'], + }; + + it('removes dispatchReady entirely', () => { + const fields: GatedResponseFields = { + dispatchReady: sampleDispatchReady, + parallelAgentsRecommendation: sampleParallelRecommendation, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.dispatchReady).toBeUndefined(); + }); + + it('downgrades parallelAgentsRecommendation.dispatch to "deferred"', () => { + const fields: GatedResponseFields = { + parallelAgentsRecommendation: { ...sampleParallelRecommendation, dispatch: 'auto' }, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.parallelAgentsRecommendation?.dispatch).toBe('deferred'); + }); + + it('preserves specialist names in parallelAgentsRecommendation for transparency', () => { + const fields: GatedResponseFields = { + parallelAgentsRecommendation: sampleParallelRecommendation, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.parallelAgentsRecommendation?.specialists).toEqual([ + 'security-specialist', + 'performance-specialist', + ]); + }); + + it('preserves hint in parallelAgentsRecommendation', () => { + const fields: GatedResponseFields = { + parallelAgentsRecommendation: sampleParallelRecommendation, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.parallelAgentsRecommendation?.hint).toBe('Dispatch specialists for review'); + }); + + it('removes executionPlan when gated', () => { + const fields: GatedResponseFields = { + dispatchReady: sampleDispatchReady, + executionPlan: { strategy: 'subagent', layers: [] }, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.executionPlan).toBeUndefined(); + }); + + it('handles missing dispatchReady gracefully', () => { + const fields: GatedResponseFields = { + parallelAgentsRecommendation: sampleParallelRecommendation, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.dispatchReady).toBeUndefined(); + expect(result.parallelAgentsRecommendation?.dispatch).toBe('deferred'); + }); + + it('handles missing parallelAgentsRecommendation gracefully', () => { + const fields: GatedResponseFields = { + dispatchReady: sampleDispatchReady, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.dispatchReady).toBeUndefined(); + expect(result.parallelAgentsRecommendation).toBeUndefined(); + }); + + it('handles all fields missing gracefully', () => { + const fields: GatedResponseFields = {}; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.dispatchReady).toBeUndefined(); + expect(result.parallelAgentsRecommendation).toBeUndefined(); + expect(result.executionPlan).toBeUndefined(); + }); + + it('does not mutate the original fields object', () => { + const fields: GatedResponseFields = { + dispatchReady: sampleDispatchReady, + parallelAgentsRecommendation: { ...sampleParallelRecommendation }, + }; + const originalDispatch = fields.parallelAgentsRecommendation?.dispatch; + + suppressDispatchWhileGated(gatedGate, fields); + + // Original should be untouched + expect(fields.dispatchReady).toBe(sampleDispatchReady); + expect(fields.parallelAgentsRecommendation?.dispatch).toBe(originalDispatch); + }); + + it('downgrades "recommend" dispatch to "deferred"', () => { + const fields: GatedResponseFields = { + parallelAgentsRecommendation: { ...sampleParallelRecommendation, dispatch: 'recommend' }, + }; + + const result = suppressDispatchWhileGated(gatedGate, fields); + + expect(result.parallelAgentsRecommendation?.dispatch).toBe('deferred'); + }); + }); + + // ------------------------------------------------------------------ + // Ungated: pass-through + // ------------------------------------------------------------------ + + describe('when gated=false', () => { + const ungatedGate: ExecutionGate = { + gated: false, + reason: 'Request is clear', + }; + + it('preserves dispatchReady unchanged', () => { + const fields: GatedResponseFields = { + dispatchReady: sampleDispatchReady, + parallelAgentsRecommendation: sampleParallelRecommendation, + }; + + const result = suppressDispatchWhileGated(ungatedGate, fields); + + expect(result.dispatchReady).toBe(sampleDispatchReady); + }); + + it('preserves parallelAgentsRecommendation unchanged', () => { + const fields: GatedResponseFields = { + parallelAgentsRecommendation: sampleParallelRecommendation, + }; + + const result = suppressDispatchWhileGated(ungatedGate, fields); + + expect(result.parallelAgentsRecommendation).toBe(sampleParallelRecommendation); + }); + + it('preserves executionPlan unchanged', () => { + const executionPlan = { strategy: 'subagent', layers: [] }; + const fields: GatedResponseFields = { + executionPlan, + }; + + const result = suppressDispatchWhileGated(ungatedGate, fields); + + expect(result.executionPlan).toBe(executionPlan); + }); + }); + + // ------------------------------------------------------------------ + // Edge: undefined gate + // ------------------------------------------------------------------ + + describe('when gate is undefined', () => { + it('passes through all fields unchanged', () => { + const fields: GatedResponseFields = { + dispatchReady: sampleDispatchReady, + parallelAgentsRecommendation: sampleParallelRecommendation, + }; + + const result = suppressDispatchWhileGated(undefined, fields); + + expect(result.dispatchReady).toBe(sampleDispatchReady); + expect(result.parallelAgentsRecommendation).toBe(sampleParallelRecommendation); + }); + }); + }); }); diff --git a/apps/mcp-server/src/mcp/handlers/execution-gate.ts b/apps/mcp-server/src/mcp/handlers/execution-gate.ts index eb249a7a..3ce1a81f 100644 --- a/apps/mcp-server/src/mcp/handlers/execution-gate.ts +++ b/apps/mcp-server/src/mcp/handlers/execution-gate.ts @@ -99,6 +99,76 @@ export function evaluateExecutionGate(input: ExecutionGateInput): ExecutionGate }; } +// --------------------------------------------------------------------------- +// Response suppression (#1422) +// --------------------------------------------------------------------------- + +/** Minimal parallel-agent recommendation shape for gating. */ +interface GatedParallelRecommendation { + specialists: string[]; + hint: string; + dispatch?: string; + suggestedStack?: string; + stackBased?: boolean; +} + +/** + * Fields from the parse_mode response that may need suppression while gated. + * Uses a generic for dispatchReady and executionPlan so the caller can pass + * the concrete types from keyword.types without coupling this module to them. + */ +export interface GatedResponseFields { + dispatchReady?: D; + parallelAgentsRecommendation?: GatedParallelRecommendation; + executionPlan?: E; +} + +/** Return type mirrors the input but with suppressed fields. */ +export type SuppressedResponseFields = GatedResponseFields; + +/** + * Suppress or downgrade dispatch-ready metadata when the execution gate + * is active (#1422). + * + * When `gate.gated === true`: + * - `dispatchReady` is removed (no Task-tool-ready params leak). + * - `executionPlan` is removed (no execution plan metadata leaks). + * - `parallelAgentsRecommendation.dispatch` is downgraded to `"deferred"`. + * - Specialist names and hint are preserved for transparency. + * + * When `gate` is undefined or ungated, all fields pass through unchanged. + * + * This function is pure and does NOT mutate its inputs. + */ +export function suppressDispatchWhileGated( + gate: ExecutionGate | undefined, + fields: GatedResponseFields, +): SuppressedResponseFields { + // No gate or ungated → pass through + if (!gate || !gate.gated) { + return fields; + } + + // Gated → suppress expensive dispatch metadata + const result: SuppressedResponseFields = {}; + + // dispatchReady: fully removed (no dispatch params should leak) + // result.dispatchReady stays undefined + + // executionPlan: fully removed + // result.executionPlan stays undefined + + // parallelAgentsRecommendation: keep specialist names, downgrade dispatch + if (fields.parallelAgentsRecommendation) { + result.parallelAgentsRecommendation = { + ...fields.parallelAgentsRecommendation, + dispatch: 'deferred', + }; + } + + return result; +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- diff --git a/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts b/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts index c795a51b..4f6a825a 100644 --- a/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts @@ -326,6 +326,7 @@ describe('ModeHandler', () => { const result = await handler.handle('parse_mode', { prompt: 'PLAN test', + question_budget: 0, // bypass clarification gate (#1422) }); expect(result?.isError).toBeFalsy(); @@ -705,6 +706,7 @@ describe('ModeHandler', () => { const result = await handler.handle('parse_mode', { prompt: 'PLAN design auth feature', + question_budget: 0, // bypass clarification gate (#1422) }); expect(result?.isError).toBeFalsy(); @@ -747,6 +749,7 @@ describe('ModeHandler', () => { const result = await handler.handle('parse_mode', { prompt: 'PLAN design auth feature', verbosity: 'full', + question_budget: 0, // bypass clarification gate (#1422) }); expect(result?.isError).toBeFalsy(); @@ -806,6 +809,7 @@ describe('ModeHandler', () => { const result = await handler.handle('parse_mode', { prompt: 'PLAN design auth feature', verbosity: 'full', + question_budget: 0, // bypass clarification gate (#1422) }); // Should not fail the whole operation @@ -902,7 +906,10 @@ describe('ModeHandler', () => { }, }); - const result = await handler.handle('parse_mode', { prompt: 'PLAN design feature' }); + const result = await handler.handle('parse_mode', { + prompt: 'PLAN design feature', + question_budget: 0, // bypass clarification gate (#1422) + }); expect(result?.isError).toBeFalsy(); const parsed = JSON.parse(result!.content[0].text as string); @@ -940,7 +947,10 @@ describe('ModeHandler', () => { ai: { dispatchStrength: 'skip' }, }); - const result = await handler.handle('parse_mode', { prompt: 'PLAN design feature' }); + const result = await handler.handle('parse_mode', { + prompt: 'PLAN design feature', + question_budget: 0, // bypass clarification gate (#1422) + }); expect(result?.isError).toBeFalsy(); const parsed = JSON.parse(result!.content[0].text as string); @@ -1386,6 +1396,7 @@ describe('ModeHandler', () => { const result = await handler.handle('parse_mode', { prompt: 'PLAN design auth feature', + question_budget: 0, // bypass clarification gate (#1422) }); expect(result?.isError).toBeFalsy(); @@ -1414,6 +1425,7 @@ describe('ModeHandler', () => { const result = await handler.handle('parse_mode', { prompt: 'PLAN design auth feature', + question_budget: 0, // bypass clarification gate (#1422) }); expect(result?.isError).toBeFalsy(); diff --git a/apps/mcp-server/src/mcp/handlers/mode.handler.ts b/apps/mcp-server/src/mcp/handlers/mode.handler.ts index cc9bc2b3..dec12ba5 100644 --- a/apps/mcp-server/src/mcp/handlers/mode.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/mode.handler.ts @@ -45,7 +45,11 @@ import { ImpactEventService } from '../../impact'; import { RuleEventCollector } from '../../rules/rule-event-collector'; import { evaluateClarification, type ClarificationMetadata } from './clarification-gate'; import { resolvePlanningStage, type PlanningStageMetadata } from './planning-stage'; -import { evaluateExecutionGate, type ExecutionGate } from './execution-gate'; +import { + evaluateExecutionGate, + suppressDispatchWhileGated, + type ExecutionGate, +} from './execution-gate'; import { buildCouncilScene } from './council-scene.builder'; /** Maximum length for context title slug generation */ @@ -385,6 +389,18 @@ export class ModeHandler extends AbstractHandler { result.parallelAgentsRecommendation?.specialists, ); + // Execution Gate suppression (#1422) — when gated, suppress dispatch-ready + // metadata and downgrade parallelAgentsRecommendation so clients cannot + // proceed with expensive specialist execution while clarification is needed. + const serializedExecutionPlan = executionPlan + ? serializeExecutionPlan(executionPlan) + : undefined; + const gatedFields = suppressDispatchWhileGated(executionGate, { + dispatchReady, + parallelAgentsRecommendation: result.parallelAgentsRecommendation, + executionPlan: serializedExecutionPlan, + }); + const response = createJsonResponse({ ...result, // Clarification-first instruction override (#1423) — applied AFTER @@ -393,8 +409,13 @@ export class ModeHandler extends AbstractHandler { language, languageInstruction: languageInstructionResult.instruction, resolvedModel, - // Include dispatch-ready data when available - ...(dispatchReady && { dispatchReady }), + // Override parallelAgentsRecommendation with gated version (#1422) + // Applied AFTER spreading result so it wins over the original field. + ...(gatedFields.parallelAgentsRecommendation !== undefined && { + parallelAgentsRecommendation: gatedFields.parallelAgentsRecommendation, + }), + // Include dispatch-ready data when available (suppressed when gated #1422) + ...(gatedFields.dispatchReady && { dispatchReady: gatedFields.dispatchReady }), // Include deep thinking instructions for PLAN/AUTO modes ...(deepThinkingInstructions && { deepThinkingInstructions }), // Include plan review gate for PLAN/AUTO modes @@ -409,8 +430,8 @@ export class ModeHandler extends AbstractHandler { ...(councilPreset && { councilPreset }), // Include council scene contract for PLAN/EVAL/AUTO modes (#1366) ...(councilScene && { councilScene }), - // Include execution plan metadata when dispatch is active - ...(executionPlan && { executionPlan: serializeExecutionPlan(executionPlan) }), + // Include execution plan metadata when dispatch is active (suppressed when gated #1422) + ...(gatedFields.executionPlan && { executionPlan: gatedFields.executionPlan }), // Include Teams capability status ...(teamsCapability && { teamsCapability }), // Include Clarification Gate metadata for PLAN/AUTO modes (#1371)