From ca87234bf3baf900b579a846765f835dbd986f43 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Tue, 7 Apr 2026 19:08:51 +0900 Subject: [PATCH] =?UTF-8?q?feat(mcp-server):=20make=20Discover=E2=86=92Des?= =?UTF-8?q?ign=E2=86=92Plan=20a=20real=20staged=20default=20(#1420)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change default routing so clear prompts start at discover instead of skipping directly to plan. Users advance through stages by passing the planning_stage parameter (discover → design → plan). - resolveStage() now defaults to 'discover' for all non-hinted calls - stageHint override preserved: callers can still force any stage - Added stageProgression metadata (completedStages, currentStage, remainingStages) - Updated unit tests for new default behavior (28 pass) - Updated mode.handler integration tests (122 pass) - Full test suite: 6190 pass, 0 fail --- .../src/mcp/handlers/mode.handler.spec.ts | 23 +++-- .../src/mcp/handlers/planning-stage.spec.ts | 93 ++++++++++++++++--- .../src/mcp/handlers/planning-stage.ts | 48 ++++++++-- 3 files changed, 133 insertions(+), 31 deletions(-) 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 4f6a825a..277ffd94 100644 --- a/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts @@ -1817,7 +1817,7 @@ describe('ModeHandler', () => { expect(parsed.planningStage.recommendedSkill).toBe('brainstorming'); }); - it('includes planningStage with plan for clear PLAN prompt', async () => { + it('includes planningStage with discover for clear PLAN prompt (staged default)', async () => { mockKeywordService.parseMode = vi.fn().mockResolvedValue({ ...mockParseModeResult, mode: 'PLAN', @@ -1831,11 +1831,16 @@ describe('ModeHandler', () => { expect(result?.isError).toBeFalsy(); const parsed = JSON.parse((result?.content[0] as { text: string }).text); expect(parsed.planningStage).toBeDefined(); - expect(parsed.planningStage.currentStage).toBe('plan'); - expect(parsed.planningStage.nextStage).toBeUndefined(); - expect(parsed.planningStage.stageTransitionHint).toBeUndefined(); - expect(parsed.planningStage.recommendedAgent).toBe('technical-planner'); - expect(parsed.planningStage.recommendedSkill).toBe('writing-plans'); + expect(parsed.planningStage.currentStage).toBe('discover'); + expect(parsed.planningStage.nextStage).toBe('design'); + expect(parsed.planningStage.stageTransitionHint).toBeTruthy(); + expect(parsed.planningStage.recommendedAgent).toBe('solution-architect'); + expect(parsed.planningStage.recommendedSkill).toBe('brainstorming'); + expect(parsed.planningStage.stageProgression).toEqual({ + completedStages: [], + currentStage: 'discover', + remainingStages: ['design', 'plan'], + }); }); it('honors explicit planning_stage=design hint', async () => { @@ -1924,7 +1929,7 @@ describe('ModeHandler', () => { expect(parsed.planningStage.currentStage).toBe('discover'); }); - it('routes budget-exhausted to plan stage', async () => { + it('routes budget-exhausted to discover stage (staged default)', async () => { mockKeywordService.parseMode = vi.fn().mockResolvedValue({ ...mockParseModeResult, mode: 'PLAN', @@ -1938,8 +1943,8 @@ describe('ModeHandler', () => { expect(result?.isError).toBeFalsy(); const parsed = JSON.parse((result?.content[0] as { text: string }).text); - expect(parsed.planningStage.currentStage).toBe('plan'); - expect(parsed.planningStage.recommendedAgent).toBe('technical-planner'); + expect(parsed.planningStage.currentStage).toBe('discover'); + expect(parsed.planningStage.recommendedAgent).toBe('solution-architect'); }); }); diff --git a/apps/mcp-server/src/mcp/handlers/planning-stage.spec.ts b/apps/mcp-server/src/mcp/handlers/planning-stage.spec.ts index 1b6d3e57..ce2cff9f 100644 --- a/apps/mcp-server/src/mcp/handlers/planning-stage.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/planning-stage.spec.ts @@ -54,16 +54,16 @@ describe('planning-stage', () => { expect(result.currentStage).toBe('discover'); }); - it('routes clear prompt directly to plan (skip discover + design)', () => { + it('routes clear prompt to discover (staged default, no skip)', () => { const result = resolvePlanningStage(clearClarification()); - expect(result.currentStage).toBe('plan'); + expect(result.currentStage).toBe('discover'); }); - it('routes budget-exhausted prompt to plan', () => { + it('routes budget-exhausted prompt to discover (staged default)', () => { const result = resolvePlanningStage(budgetExhaustedClarification()); - expect(result.currentStage).toBe('plan'); + expect(result.currentStage).toBe('discover'); }); it('routes explicit stageHint=design to design', () => { @@ -113,7 +113,9 @@ describe('planning-stage', () => { }); it('has a description for plan stage', () => { - const result = resolvePlanningStage(clearClarification()); + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'plan', + }); expect(result.stageDescription).toContain('Plan'); expect(result.stageDescription).toContain('implementation'); @@ -140,7 +142,9 @@ describe('planning-stage', () => { }); it('plan → nextStage is undefined (terminal)', () => { - const result = resolvePlanningStage(clearClarification()); + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'plan', + }); expect(result.nextStage).toBeUndefined(); }); @@ -168,7 +172,9 @@ describe('planning-stage', () => { }); it('has no transition hint for plan stage (terminal)', () => { - const result = resolvePlanningStage(clearClarification()); + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'plan', + }); expect(result.stageTransitionHint).toBeUndefined(); }); @@ -194,7 +200,9 @@ describe('planning-stage', () => { }); it('recommends technical-planner for plan', () => { - const result = resolvePlanningStage(clearClarification()); + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'plan', + }); expect(result.recommendedAgent).toBe('technical-planner'); }); @@ -220,26 +228,47 @@ describe('planning-stage', () => { }); it('recommends writing-plans skill for plan', () => { - const result = resolvePlanningStage(clearClarification()); + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'plan', + }); expect(result.recommendedSkill).toBe('writing-plans'); }); }); // ------------------------------------------------------------------ - // Backward compatibility + // Staged default behavior // ------------------------------------------------------------------ - describe('backward compatibility', () => { - it('clear prompt produces planReady + plan stage with no intermediate steps', () => { + describe('staged default behavior', () => { + it('clear prompt starts at discover with design as next stage', () => { const clarification = clearClarification(); const result = resolvePlanningStage(clarification); + expect(result.currentStage).toBe('discover'); + expect(result.nextStage).toBe('design'); + expect(result.stageTransitionHint).toBeTruthy(); + }); + + it('stageHint=plan still jumps directly to plan (caller override)', () => { + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'plan', + }); + expect(result.currentStage).toBe('plan'); expect(result.nextStage).toBeUndefined(); expect(result.stageTransitionHint).toBeUndefined(); }); + it('stageHint=design advances to design (caller override)', () => { + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'design', + }); + + expect(result.currentStage).toBe('design'); + expect(result.nextStage).toBe('plan'); + }); + it('all fields are present in the return type', () => { const result: PlanningStageMetadata = resolvePlanningStage(ambiguousClarification()); @@ -251,5 +280,45 @@ describe('planning-stage', () => { expect(result).toHaveProperty('recommendedSkill'); }); }); + + // ------------------------------------------------------------------ + // Stage progression metadata + // ------------------------------------------------------------------ + + describe('stageProgression', () => { + it('discover stage shows no completed, discover current, design+plan remaining', () => { + const result = resolvePlanningStage(ambiguousClarification()); + + expect(result.stageProgression).toEqual({ + completedStages: [], + currentStage: 'discover', + remainingStages: ['design', 'plan'], + }); + }); + + it('design stage shows discover completed, design current, plan remaining', () => { + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'design', + }); + + expect(result.stageProgression).toEqual({ + completedStages: ['discover'], + currentStage: 'design', + remainingStages: ['plan'], + }); + }); + + it('plan stage shows discover+design completed, plan current, no remaining', () => { + const result = resolvePlanningStage(clearClarification(), { + stageHint: 'plan', + }); + + expect(result.stageProgression).toEqual({ + completedStages: ['discover', 'design'], + currentStage: 'plan', + remainingStages: [], + }); + }); + }); }); }); diff --git a/apps/mcp-server/src/mcp/handlers/planning-stage.ts b/apps/mcp-server/src/mcp/handlers/planning-stage.ts index 25edf37d..c7227a33 100644 --- a/apps/mcp-server/src/mcp/handlers/planning-stage.ts +++ b/apps/mcp-server/src/mcp/handlers/planning-stage.ts @@ -25,6 +25,16 @@ import type { ClarificationMetadata } from './clarification-gate'; /** The three stages of the planning flow. */ export type PlanningStage = 'discover' | 'design' | 'plan'; +/** Progression tracking across the three planning stages. */ +export interface StageProgression { + /** Stages already completed before the current one. */ + completedStages: PlanningStage[]; + /** The stage currently active. */ + currentStage: PlanningStage; + /** Stages remaining after the current one. */ + remainingStages: PlanningStage[]; +} + /** Metadata emitted alongside the clarification fields in the PLAN response. */ export interface PlanningStageMetadata { /** Current planning stage. */ @@ -39,6 +49,8 @@ export interface PlanningStageMetadata { recommendedAgent?: string; /** Recommended supporting skill for this stage. */ recommendedSkill?: string; + /** Progression metadata showing completed / current / remaining stages. */ + stageProgression?: StageProgression; } /** Options to override automatic stage resolution. */ @@ -90,9 +102,12 @@ const STAGE_SKILLS: Record = { * Routing rules (evaluated in order): * 1. If `stageHint` is provided → use it directly (caller knows best). * 2. If `clarification.clarificationNeeded === true` → `discover`. - * 3. If `clarification.planReady === true` and no stageHint → `plan` - * (clear prompt — skip discover and design). - * 4. Otherwise → `design` (clarification resolved, approach not yet confirmed). + * 3. Default → `discover` (staged default — always start at discover). + * + * The user advances through stages by passing `planning_stage` parameter: + * First call (no hint) → discover + * User confirms direction → passes `planning_stage: "design"` → design + * User confirms approach → passes `planning_stage: "plan"` → plan */ export function resolvePlanningStage( clarification: ClarificationMetadata, @@ -109,6 +124,7 @@ export function resolvePlanningStage( }), recommendedAgent: STAGE_AGENTS[stage], ...(STAGE_SKILLS[stage] && { recommendedSkill: STAGE_SKILLS[stage] }), + stageProgression: buildStageProgression(stage), }; } @@ -130,15 +146,27 @@ function resolveStage( return 'discover'; } - // 3. Clear / plan-ready → plan (skip discover+design) - if (clarification.planReady) { - return 'plan'; - } - - // 4. Fallback: clarification resolved but approach not confirmed → design - return 'design'; + // 3. Staged default — always start at discover regardless of planReady. + // The user advances through stages via the planning_stage parameter. + return 'discover'; } function getNextStage(stage: 'discover' | 'design'): 'design' | 'plan' { return stage === 'discover' ? 'design' : 'plan'; } + +/** Canonical ordered list of all planning stages. */ +const ALL_STAGES: readonly PlanningStage[] = ['discover', 'design', 'plan']; + +/** + * Build stage progression metadata showing which stages are completed, + * which is current, and which remain. + */ +function buildStageProgression(currentStage: PlanningStage): StageProgression { + const currentIndex = ALL_STAGES.indexOf(currentStage); + return { + completedStages: ALL_STAGES.slice(0, currentIndex) as PlanningStage[], + currentStage, + remainingStages: ALL_STAGES.slice(currentIndex + 1) as PlanningStage[], + }; +}