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 f644726c..c795a51b 100644 --- a/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts @@ -1648,6 +1648,138 @@ describe('ModeHandler', () => { }); }); + // ========================================================================== + // Clarification-First Directive (#1423) + // ========================================================================== + describe('parse_mode Clarification-First Directive (#1423)', () => { + it('overrides instructions with clarification-first directive for ambiguous PLAN prompt', async () => { + mockKeywordService.parseMode = vi.fn().mockResolvedValue({ + ...mockParseModeResult, + mode: 'PLAN', + originalPrompt: '개선해줘', + }); + + const result = await handler.handle('parse_mode', { + prompt: 'PLAN 개선해줘', + }); + + expect(result?.isError).toBeFalsy(); + const parsed = JSON.parse((result?.content[0] as { text: string }).text); + expect(parsed.clarificationNeeded).toBe(true); + expect(parsed.instructions).toContain('CLARIFICATION REQUIRED'); + expect(parsed.instructions).toContain('DO NOT PLAN'); + expect(parsed.instructions).toContain('Ask EXACTLY the question below and STOP'); + expect(parsed.instructions).not.toContain('CONTEXT: Check contextDocument'); + }); + + it('includes nextQuestion text in overridden instructions', async () => { + mockKeywordService.parseMode = vi.fn().mockResolvedValue({ + ...mockParseModeResult, + mode: 'PLAN', + originalPrompt: 'improve it', + }); + + const result = await handler.handle('parse_mode', { + prompt: 'PLAN improve it', + }); + + expect(result?.isError).toBeFalsy(); + const parsed = JSON.parse((result?.content[0] as { text: string }).text); + expect(parsed.clarificationNeeded).toBe(true); + // The nextQuestion should appear in the instructions + expect(parsed.instructions).toContain(parsed.nextQuestion); + }); + + it('includes remaining questionBudget in overridden instructions', async () => { + mockKeywordService.parseMode = vi.fn().mockResolvedValue({ + ...mockParseModeResult, + mode: 'PLAN', + originalPrompt: 'improve it', + }); + + const result = await handler.handle('parse_mode', { + prompt: 'PLAN improve it', + question_budget: 2, + }); + + expect(result?.isError).toBeFalsy(); + const parsed = JSON.parse((result?.content[0] as { text: string }).text); + expect(parsed.clarificationNeeded).toBe(true); + expect(parsed.instructions).toContain('question_budget=1'); + }); + + it('does NOT override instructions when planReady=true', async () => { + mockKeywordService.parseMode = vi.fn().mockResolvedValue({ + ...mockParseModeResult, + mode: 'PLAN', + originalPrompt: 'add tests to src/auth.ts', + }); + + const result = await handler.handle('parse_mode', { + prompt: 'PLAN add tests to src/auth.ts', + }); + + expect(result?.isError).toBeFalsy(); + const parsed = JSON.parse((result?.content[0] as { text: string }).text); + expect(parsed.planReady).toBe(true); + expect(parsed.instructions).not.toContain('CLARIFICATION REQUIRED'); + }); + + it('applies directive in AUTO mode for ambiguous prompt', async () => { + mockKeywordService.parseMode = vi.fn().mockResolvedValue({ + ...mockParseModeResult, + mode: 'AUTO', + originalPrompt: '개선', + }); + + const result = await handler.handle('parse_mode', { + prompt: 'AUTO 개선', + }); + + expect(result?.isError).toBeFalsy(); + const parsed = JSON.parse((result?.content[0] as { text: string }).text); + expect(parsed.clarificationNeeded).toBe(true); + expect(parsed.instructions).toContain('CLARIFICATION REQUIRED'); + expect(parsed.instructions).toContain('DO NOT PLAN'); + }); + + it('does NOT apply directive when budget is exhausted', async () => { + mockKeywordService.parseMode = vi.fn().mockResolvedValue({ + ...mockParseModeResult, + mode: 'PLAN', + originalPrompt: 'improve it', + }); + + const result = await handler.handle('parse_mode', { + prompt: 'PLAN improve it', + question_budget: 0, + }); + + expect(result?.isError).toBeFalsy(); + const parsed = JSON.parse((result?.content[0] as { text: string }).text); + expect(parsed.clarificationNeeded).toBe(false); + expect(parsed.planReady).toBe(true); + expect(parsed.instructions).not.toContain('CLARIFICATION REQUIRED'); + }); + + it('does NOT apply directive when override phrase is present', async () => { + mockKeywordService.parseMode = vi.fn().mockResolvedValue({ + ...mockParseModeResult, + mode: 'PLAN', + originalPrompt: 'improve it, just do it', + }); + + const result = await handler.handle('parse_mode', { + prompt: 'PLAN improve it, just do it', + }); + + expect(result?.isError).toBeFalsy(); + const parsed = JSON.parse((result?.content[0] as { text: string }).text); + expect(parsed.clarificationNeeded).toBe(false); + expect(parsed.instructions).not.toContain('CLARIFICATION REQUIRED'); + }); + }); + // ========================================================================== // Planning Stage Router (#1372) // ========================================================================== diff --git a/apps/mcp-server/src/mcp/handlers/mode.handler.ts b/apps/mcp-server/src/mcp/handlers/mode.handler.ts index 06ba7462..cc9bc2b3 100644 --- a/apps/mcp-server/src/mcp/handlers/mode.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/mode.handler.ts @@ -361,6 +361,13 @@ export class ModeHandler extends AbstractHandler { questionBudget, ); + // Clarification-first directive (#1423): when the gate determines the + // request is ambiguous, build an instruction override so the AI client + // asks the next question and STOPS before generating any plan. + // We intentionally do NOT mutate `result` — the override is applied + // later in the response spread to avoid side-effects on shared objects. + const clarificationInstructions = this.buildClarificationFirstInstructions(clarification); + // Planning Stage Router (#1372) — maps clarification output to // discover / design / plan stage. Only for PLAN/AUTO modes. const planningStage = this.buildPlanningStageMetadata( @@ -380,6 +387,9 @@ export class ModeHandler extends AbstractHandler { const response = createJsonResponse({ ...result, + // Clarification-first instruction override (#1423) — applied AFTER + // spreading result so it wins over the original instructions field. + ...(clarificationInstructions !== undefined && { instructions: clarificationInstructions }), language, languageInstruction: languageInstructionResult.instruction, resolvedModel, @@ -696,6 +706,39 @@ export class ModeHandler extends AbstractHandler { }); } + /** + * Build a clarification-first instruction string (#1423). + * + * Returns the directive text when the Clarification Gate determines the + * request is ambiguous, or `undefined` when the request is clear. + * The caller spreads the result into the response so the original + * `result` object is never mutated (avoids shared-reference side-effects). + */ + private buildClarificationFirstInstructions( + clarification: ClarificationMetadata | undefined, + ): string | undefined { + if (!clarification?.clarificationNeeded) { + return undefined; + } + + const question = + clarification.nextQuestion ?? + 'Can you clarify the scope and success criteria of this request?'; + const budget = clarification.questionBudget ?? 0; + + return ( + '🔴 CLARIFICATION REQUIRED — DO NOT PLAN.\n\n' + + 'The request is ambiguous. You MUST:\n' + + '1. Ask EXACTLY the question below and STOP.\n' + + '2. Do NOT output any implementation plan, architecture, or code.\n' + + "3. Wait for the user's response before continuing.\n\n" + + `❓ Ask this: "${question}"\n\n` + + `Remaining question budget: ${budget}\n\n` + + 'After the user answers, call parse_mode again with the clarified prompt ' + + `and question_budget=${budget} to continue.` + ); + } + /** * Build Planning Stage metadata for PLAN/AUTO modes (#1372). * Returns undefined for ACT/EVAL modes. diff --git a/packages/claude-code-plugin/hooks/lib/mode_engine.py b/packages/claude-code-plugin/hooks/lib/mode_engine.py index 32ff11dc..7af10e8d 100644 --- a/packages/claude-code-plugin/hooks/lib/mode_engine.py +++ b/packages/claude-code-plugin/hooks/lib/mode_engine.py @@ -108,6 +108,132 @@ } +# --------------------------------------------------------------------------- +# Standalone Clarification Gate (#1423) +# Mirrors MCP server clarification-gate.ts heuristics so standalone mode +# also asks before planning when the request is ambiguous. +# --------------------------------------------------------------------------- + +MIN_PROMPT_LENGTH = 20 + +_OVERRIDE_PATTERNS = [ + re.compile(r"\bjust\s+do\s+it\b", re.I), + re.compile(r"\buse\s+your\s+(?:judg(?:e)?ment|best\s+guess|discretion)\b", re.I), + re.compile(r"\bgo\s+ahead\b", re.I), + re.compile(r"\bmake\s+assumptions?\b", re.I), + re.compile(r"\bassume\s+(?:whatever|defaults?|reasonable)\b", re.I), + re.compile(r"알아서\s*(?:해|진행|처리)"), + re.compile(r"그냥\s*(?:해|진행)"), + re.compile(r"임의로\s*(?:해|진행)"), +] + +_VAGUE_INTENT_PATTERNS = [ + re.compile(r"\bimprove\b", re.I), + re.compile(r"\b(?:make\s+it\s+)?better\b", re.I), + re.compile(r"\benhance\b", re.I), + re.compile(r"\boptimi[sz]e\b", re.I), + re.compile(r"\brefactor\b", re.I), + re.compile(r"\bclean\s*up\b", re.I), + re.compile(r"\btweak\b", re.I), + re.compile(r"\bfix\s+(?:stuff|things|issues?)\b", re.I), + re.compile(r"개선"), + re.compile(r"향상"), + re.compile(r"최적화"), + re.compile(r"정리"), + re.compile(r"개량"), +] + +_TECH_REFERENCE_PATTERNS = [ + re.compile( + r"\.(?:ts|tsx|js|jsx|py|go|rs|java|kt|swift|rb|php|c|cpp|cs|md" + r"|json|ya?ml|toml|sql|sh)\b", + re.I, + ), + re.compile(r"(?:^|[\s([`\"'])[\w.-]+/[\w.\-/]+"), + re.compile(r"\b[a-zA-Z_][\w]*\("), + re.compile(r"\b[A-Z][a-zA-Z0-9]*[A-Z][a-zA-Z0-9]+\b"), + re.compile(r"\b[a-z][a-z0-9]+[A-Z][a-zA-Z0-9]+\b"), + re.compile(r"\b[a-z]{3,}_[a-z]{3,}[a-z0-9_]*\b"), + re.compile(r"`[^`]+`"), +] + +_MODE_KEYWORD_RE = re.compile( + r"^(PLAN|ACT|EVAL|AUTO|계획|실행|평가|자동|計画|実行|評価|自動" + r"|计划|执行|评估|自动|PLANIFICAR|ACTUAR|EVALUAR|AUTOMÁTICO)\s*[:\s]*", + re.I, +) + + +DEFAULT_QUESTION_BUDGET = 3 + + +def evaluate_clarification_standalone( + prompt: str, question_budget: Optional[int] = None +) -> Optional[str]: + """ + Standalone clarification gate (#1423). + + Returns a clarification-first directive string when the prompt is + ambiguous, or ``None`` when the request is clear enough to plan. + + Args: + prompt: Raw user prompt including mode keyword. + question_budget: Remaining clarification rounds. Defaults to + ``DEFAULT_QUESTION_BUDGET``. When 0 the gate falls back + to planning with explicit assumptions. + """ + budget = question_budget if question_budget is not None else DEFAULT_QUESTION_BUDGET + + trimmed = prompt.strip() + if not trimmed: + return None + + # Budget exhausted — proceed with explicit assumptions. + if budget <= 0: + return None + + # Strip mode keyword prefix before evaluating content + stripped = _MODE_KEYWORD_RE.sub("", trimmed).strip() + if not stripped: + return None + + if any(p.search(stripped) for p in _OVERRIDE_PATTERNS): + return None + + if any(p.search(stripped) for p in _TECH_REFERENCE_PATTERNS): + return None + + is_vague = any(p.search(stripped) for p in _VAGUE_INTENT_PATTERNS) + is_short = 0 < len(stripped) < MIN_PROMPT_LENGTH + + if not is_vague and not is_short: + return None + + remaining = budget - 1 + + if is_vague: + question = ( + "What concrete change are you targeting — " + "which behavior, file, or metric should differ after this task?" + ) + else: + question = ( + "Can you describe the goal, inputs, and expected outcome " + "in a bit more detail?" + ) + + return ( + "🔴 CLARIFICATION REQUIRED — DO NOT PLAN.\n\n" + "The request is ambiguous. You MUST:\n" + "1. Ask EXACTLY the question below and STOP.\n" + "2. Do NOT output any implementation plan, architecture, or code.\n" + "3. Wait for the user's response before continuing.\n\n" + f'❓ Ask this: "{question}"\n\n' + f"Remaining question budget: {remaining}\n\n" + "After the user answers, re-invoke the mode with the clarified prompt." + ) + + def _resolve_rules_dir(cwd: Optional[str] = None) -> Optional[str]: """ Resolve path to .ai-rules/ directory. @@ -290,7 +416,12 @@ def build_council_scene(self, mode: str) -> Optional[dict]: "format": "tiny-actor-grid", } - def build_instructions(self, mode: str) -> str: + def build_instructions( + self, + mode: str, + prompt: Optional[str] = None, + question_budget: Optional[int] = None, + ) -> str: """ Build complete mode instructions for hook output. @@ -298,13 +429,26 @@ def build_instructions(self, mode: str) -> str: available and enforces the ``CHAR_LIMIT`` (2000) ceiling. Falls back to the minimal template when ``.ai-rules/`` is absent. + When *prompt* is provided for PLAN/AUTO modes the standalone + Clarification Gate (#1423) is evaluated first. If the request is + ambiguous a clarification-first directive is returned instead of + the normal planning instructions. + Args: mode: Mode name (PLAN, ACT, EVAL, AUTO) + prompt: Optional raw user prompt for clarification evaluation. Returns: Complete mode instructions string (≤ 2000 chars). """ mode_upper = mode.upper() + + # Clarification gate for PLAN/AUTO modes (#1423) + if prompt and mode_upper in ("PLAN", "AUTO"): + directive = evaluate_clarification_standalone(prompt, question_budget) + if directive: + return directive + agent = self.get_default_agent(mode_upper) template = MODE_TEMPLATES.get(mode_upper, MODE_TEMPLATES["ACT"]) diff --git a/packages/claude-code-plugin/hooks/lib/test_mode_engine.py b/packages/claude-code-plugin/hooks/lib/test_mode_engine.py index 33b8685f..17eb46cd 100644 --- a/packages/claude-code-plugin/hooks/lib/test_mode_engine.py +++ b/packages/claude-code-plugin/hooks/lib/test_mode_engine.py @@ -462,5 +462,79 @@ def test_all_modes_work_without_rules_data(self): self.assertTrue(len(result) > 0, f"{mode} produced empty output") +class TestStandaloneClarificationGate(unittest.TestCase): + """Test standalone clarification gate (#1423).""" + + def setUp(self): + with tempfile.TemporaryDirectory() as tmpdir: + self.engine = ModeEngine(rules_dir=tmpdir) + + def test_ambiguous_plan_returns_clarification_directive(self): + result = self.engine.build_instructions("PLAN", prompt="PLAN improve the UI") + self.assertIn("CLARIFICATION REQUIRED", result) + self.assertIn("DO NOT PLAN", result) + self.assertIn("Ask EXACTLY the question below and STOP", result) + + def test_ambiguous_auto_returns_clarification_directive(self): + result = self.engine.build_instructions("AUTO", prompt="AUTO 개선해줘") + self.assertIn("CLARIFICATION REQUIRED", result) + + def test_clear_plan_with_file_path_returns_normal_instructions(self): + result = self.engine.build_instructions( + "PLAN", prompt="PLAN add tests to src/auth.ts" + ) + self.assertNotIn("CLARIFICATION REQUIRED", result) + self.assertIn("# Mode: PLAN", result) + + def test_clear_plan_with_function_ref_returns_normal_instructions(self): + result = self.engine.build_instructions( + "PLAN", prompt="PLAN refactor parseMode() to handle localized keywords" + ) + self.assertNotIn("CLARIFICATION REQUIRED", result) + + def test_override_phrase_bypasses_gate(self): + result = self.engine.build_instructions( + "PLAN", prompt="PLAN improve it, just do it" + ) + self.assertNotIn("CLARIFICATION REQUIRED", result) + self.assertIn("# Mode: PLAN", result) + + def test_korean_override_phrase_bypasses_gate(self): + result = self.engine.build_instructions( + "PLAN", prompt="PLAN 개선해줘 알아서 해" + ) + self.assertNotIn("CLARIFICATION REQUIRED", result) + + def test_short_prompt_without_tech_ref_triggers_gate(self): + result = self.engine.build_instructions("PLAN", prompt="PLAN fix it") + self.assertIn("CLARIFICATION REQUIRED", result) + + def test_act_mode_skips_clarification(self): + result = self.engine.build_instructions("ACT", prompt="ACT improve it") + self.assertNotIn("CLARIFICATION REQUIRED", result) + self.assertIn("# Mode: ACT", result) + + def test_eval_mode_skips_clarification(self): + result = self.engine.build_instructions("EVAL", prompt="EVAL improve it") + self.assertNotIn("CLARIFICATION REQUIRED", result) + + def test_no_prompt_skips_clarification(self): + result = self.engine.build_instructions("PLAN") + self.assertNotIn("CLARIFICATION REQUIRED", result) + self.assertIn("# Mode: PLAN", result) + + def test_well_specified_long_prompt_skips_gate(self): + result = self.engine.build_instructions( + "PLAN", + prompt="PLAN implement password reset endpoint that sends an email with a reset link", + ) + self.assertNotIn("CLARIFICATION REQUIRED", result) + + def test_vague_intent_question_content(self): + result = self.engine.build_instructions("PLAN", prompt="PLAN refactor this") + self.assertIn("CLARIFICATION REQUIRED", result) + self.assertIn("concrete change", result) + + if __name__ == "__main__": unittest.main() diff --git a/packages/claude-code-plugin/hooks/user-prompt-submit.py b/packages/claude-code-plugin/hooks/user-prompt-submit.py index 142a2238..97a5ac8e 100644 --- a/packages/claude-code-plugin/hooks/user-prompt-submit.py +++ b/packages/claude-code-plugin/hooks/user-prompt-submit.py @@ -90,7 +90,9 @@ def main(): # is required for mode handling. print("# Backend: standalone (self-contained, no MCP required)") engine = ModeEngine(cwd=project_dir) - instructions = engine.build_instructions(detected_mode) + instructions = engine.build_instructions( + detected_mode, prompt=prompt + ) print(instructions) # Standalone council preset from Tiny Actor presets (#1361) try: diff --git a/tests/e2e/plugin-hooks/test_user_prompt_submit_e2e.py b/tests/e2e/plugin-hooks/test_user_prompt_submit_e2e.py index 137ed678..115310ea 100644 --- a/tests/e2e/plugin-hooks/test_user_prompt_submit_e2e.py +++ b/tests/e2e/plugin-hooks/test_user_prompt_submit_e2e.py @@ -45,10 +45,10 @@ class TestEnglishModeKeywords: """Detect English mode keywords: PLAN, ACT, EVAL, AUTO.""" @pytest.mark.parametrize("keyword,mode", [ - ("PLAN design auth feature", "PLAN"), - ("ACT implement the changes", "ACT"), - ("EVAL review the code", "EVAL"), - ("AUTO implement user dashboard", "AUTO"), + ("PLAN design auth feature for login.ts", "PLAN"), + ("ACT implement the auth changes in login.ts", "ACT"), + ("EVAL review the auth.ts implementation", "EVAL"), + ("AUTO implement user dashboard in dashboard.tsx", "AUTO"), ]) def test_detects_english_keyword(self, mock_env, keyword, mode): result = run_hook( @@ -73,10 +73,10 @@ class TestKoreanModeKeywords: """Detect Korean mode keywords.""" @pytest.mark.parametrize("keyword,mode", [ - ("계획 인증 기능 설계", "PLAN"), - ("실행 변경 사항 구현", "ACT"), - ("평가 코드 리뷰", "EVAL"), - ("자동 대시보드 구현", "AUTO"), + ("계획 login.ts 인증 기능 설계", "PLAN"), + ("실행 auth.ts 인증 변경 사항 구현", "ACT"), + ("평가 login.ts 인증 코드 리뷰", "EVAL"), + ("자동 dashboard.tsx 대시보드 구현", "AUTO"), ]) def test_detects_korean_keyword(self, mock_env, keyword, mode): result = run_hook( @@ -92,10 +92,10 @@ class TestJapaneseModeKeywords: """Detect Japanese mode keywords.""" @pytest.mark.parametrize("keyword,mode", [ - ("計画 認証機能の設計", "PLAN"), - ("実行 変更の実装", "ACT"), - ("評価 コードレビュー", "EVAL"), - ("自動 ダッシュボード実装", "AUTO"), + ("計画 auth.ts 認証機能の設計", "PLAN"), + ("実行 auth.ts 変更の実装", "ACT"), + ("評価 login.ts コードレビュー", "EVAL"), + ("自動 dashboard.tsx ダッシュボード実装", "AUTO"), ]) def test_detects_japanese_keyword(self, mock_env, keyword, mode): result = run_hook( @@ -111,10 +111,10 @@ class TestChineseModeKeywords: """Detect Chinese mode keywords.""" @pytest.mark.parametrize("keyword,mode", [ - ("计划 设计认证功能", "PLAN"), - ("执行 实施变更", "ACT"), - ("评估 代码审查", "EVAL"), - ("自动 实现仪表板", "AUTO"), + ("计划 auth.ts 设计认证功能", "PLAN"), + ("执行 auth.ts 实施认证变更", "ACT"), + ("评估 login.ts 代码审查", "EVAL"), + ("自动 dashboard.tsx 实现仪表板", "AUTO"), ]) def test_detects_chinese_keyword(self, mock_env, keyword, mode): result = run_hook( @@ -130,10 +130,10 @@ class TestSpanishModeKeywords: """Detect Spanish mode keywords.""" @pytest.mark.parametrize("keyword,mode", [ - ("PLANIFICAR diseñar autenticación", "PLAN"), - ("ACTUAR implementar cambios", "ACT"), - ("EVALUAR revisar código", "EVAL"), - ("AUTOMÁTICO implementar dashboard", "AUTO"), + ("PLANIFICAR diseñar autenticación en auth.ts", "PLAN"), + ("ACTUAR implementar cambios en login.ts", "ACT"), + ("EVALUAR revisar código en auth.ts", "EVAL"), + ("AUTOMÁTICO implementar dashboard en dashboard.tsx", "AUTO"), ]) def test_detects_spanish_keyword(self, mock_env, keyword, mode): result = run_hook( @@ -160,7 +160,7 @@ def test_context_contains_parse_mode_hint(self, mock_env): def test_context_contains_agent_name(self, mock_env): result = run_hook( "user-prompt-submit.py", - input_data={"prompt": "AUTO implement feature"}, + input_data={"prompt": "AUTO implement feature in auth.ts"}, env=mock_env, ) assert "# Mode: AUTO" in result.stdout @@ -169,7 +169,7 @@ def test_case_insensitive_detection(self, mock_env): """Keywords should be detected case-insensitively.""" result = run_hook( "user-prompt-submit.py", - input_data={"prompt": "plan design something"}, + input_data={"prompt": "plan design something for auth.ts"}, env=mock_env, ) assert result.succeeded