Skip to content

Commit 845fe9b

Browse files
committed
feat(mcp-server): add Clarification Gate to PLAN and parse_mode
Closes #1371 Make clarification a first-class runtime contract in the parse_mode PLAN response. Instead of relying on prompt-style guidance, the handler now detects materially ambiguous requests and forces the AI client to ask before solutioning. New fields on PLAN/AUTO parse_mode response: - clarificationNeeded: boolean — true when the request is ambiguous - planReady: boolean — mutually exclusive with clarificationNeeded - questionBudget: number — remaining clarification rounds (defaults 3) - nextQuestion: string — single highest-value question (when asking) - clarificationTopics: string[] — ordered ambiguity topics (when asking) - assumptionNote: string — emitted when budget is exhausted Ambiguity heuristics (conservative, tunable) extracted into clarification-gate.ts as a pure testable function: - Request length < 20 chars without tech reference - Vague intent verbs (improve/enhance/refactor/개선/향상/...) without scope - No concrete technical reference (file path, function call, CamelCase, snake_case, backtick code span) - Explicit override phrases ("just do it", "use your judgment", "알아서 해") skip the gate entirely - Specific file paths or identifiers anchor the request as planReady Budget mechanism: caller passes the previous round's questionBudget via the new question_budget input parameter; the gate decrements on each ambiguous round and falls back to planning with an explicit assumption note when the budget reaches 0. This prevents infinite clarification loops. Backward-compatible: fields are omitted entirely for ACT/EVAL modes and additive for PLAN/AUTO. Existing consumers of parse_mode are unaffected. Tests: - 74 unit tests for clarification-gate heuristics + behavior contract - 10 integration tests in mode.handler.spec.ts covering ambiguous, clear, override, budget-exhausted, AUTO, and ACT/EVAL skip paths - Full suite: 6030 passing (2 skipped), lint/format/typecheck/circular all green
1 parent 81537f1 commit 845fe9b

4 files changed

Lines changed: 780 additions & 0 deletions

File tree

Lines changed: 282 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,282 @@
1+
import { describe, it, expect } from 'vitest';
2+
import {
3+
evaluateClarification,
4+
hasOverridePhrase,
5+
hasVagueIntent,
6+
hasTechnicalReference,
7+
DEFAULT_QUESTION_BUDGET,
8+
CLARIFICATION_TOPICS,
9+
MIN_PROMPT_LENGTH,
10+
} from './clarification-gate';
11+
12+
describe('clarification-gate', () => {
13+
describe('hasOverridePhrase', () => {
14+
it.each([
15+
['just do it'],
16+
['Just Do It and move on'],
17+
['use your judgment here'],
18+
['use your best guess'],
19+
['use your discretion'],
20+
['go ahead with it'],
21+
['make assumptions where needed'],
22+
['assume defaults'],
23+
['assume reasonable behavior'],
24+
['알아서 해'],
25+
['알아서 진행해줘'],
26+
['알아서 처리'],
27+
['그냥 해'],
28+
['임의로 진행'],
29+
])('detects override phrase in %p', input => {
30+
expect(hasOverridePhrase(input)).toBe(true);
31+
});
32+
33+
it.each([
34+
['implement OAuth2 login flow'],
35+
['just another feature request'],
36+
['refactor the login module'],
37+
[''],
38+
])('returns false for non-override %p', input => {
39+
expect(hasOverridePhrase(input)).toBe(false);
40+
});
41+
});
42+
43+
describe('hasVagueIntent', () => {
44+
it.each([
45+
['improve the UI'],
46+
['make it better'],
47+
['enhance performance'],
48+
['optimize the flow'],
49+
['optimise the flow'],
50+
['refactor this'],
51+
['clean up the code'],
52+
['clean up'],
53+
['tweak the settings'],
54+
['fix stuff'],
55+
['fix things'],
56+
['fix issues'],
57+
['로그인 개선'],
58+
['성능 향상'],
59+
['최적화 필요'],
60+
['코드 정리'],
61+
])('detects vague intent in %p', input => {
62+
expect(hasVagueIntent(input)).toBe(true);
63+
});
64+
65+
it.each([
66+
['implement login'],
67+
['add a new endpoint'],
68+
['write unit tests'],
69+
['create a button component'],
70+
])('returns false for concrete intent %p', input => {
71+
expect(hasVagueIntent(input)).toBe(false);
72+
});
73+
});
74+
75+
describe('hasTechnicalReference', () => {
76+
it.each([
77+
['add tests to src/auth.ts'],
78+
['update apps/mcp-server/src/main.ts'],
79+
['fix bug in login.tsx'],
80+
['modify config.yaml'],
81+
['update the package.json'],
82+
['call parseMode() from the handler'],
83+
['use the ModeHandler class'],
84+
['rename parse_mode function'],
85+
['check the user_profile field'],
86+
['fix `handleRequest` in the router'],
87+
['invoke fetchUser() helper'],
88+
])('detects technical reference in %p', input => {
89+
expect(hasTechnicalReference(input)).toBe(true);
90+
});
91+
92+
it.each([['improve the UI'], ['make things faster'], ['개선해줘'], ['fix stuff'], ['']])(
93+
'returns false for non-technical %p',
94+
input => {
95+
expect(hasTechnicalReference(input)).toBe(false);
96+
},
97+
);
98+
});
99+
100+
describe('evaluateClarification', () => {
101+
describe('clear PLAN requests (planReady path)', () => {
102+
it('returns planReady=true for a prompt with an explicit file path', () => {
103+
const result = evaluateClarification(
104+
'add unit tests to apps/mcp-server/src/auth/auth.service.ts',
105+
);
106+
107+
expect(result.planReady).toBe(true);
108+
expect(result.clarificationNeeded).toBe(false);
109+
expect(result.nextQuestion).toBeUndefined();
110+
expect(result.clarificationTopics).toBeUndefined();
111+
expect(result.questionBudget).toBe(DEFAULT_QUESTION_BUDGET);
112+
});
113+
114+
it('returns planReady=true for a prompt with a function identifier', () => {
115+
const result = evaluateClarification('refactor parseMode() to handle localized keywords');
116+
117+
expect(result.planReady).toBe(true);
118+
expect(result.clarificationNeeded).toBe(false);
119+
});
120+
121+
it('returns planReady=true for a prompt with a PascalCase class reference', () => {
122+
const result = evaluateClarification('add logging to ModeHandler for debugging');
123+
124+
expect(result.planReady).toBe(true);
125+
expect(result.clarificationNeeded).toBe(false);
126+
});
127+
128+
it('returns planReady=true for a well-specified implementation request', () => {
129+
const result = evaluateClarification(
130+
'implement password reset endpoint that sends an email with a reset link',
131+
);
132+
133+
expect(result.planReady).toBe(true);
134+
expect(result.clarificationNeeded).toBe(false);
135+
});
136+
137+
it('preserves the caller-provided budget on a clear path', () => {
138+
const result = evaluateClarification('add tests to src/auth.ts', {
139+
questionBudget: 2,
140+
});
141+
142+
expect(result.questionBudget).toBe(2);
143+
});
144+
});
145+
146+
describe('ambiguous PLAN requests (clarification path)', () => {
147+
it('returns clarificationNeeded=true for a short vague prompt', () => {
148+
const result = evaluateClarification('개선해줘');
149+
150+
expect(result.clarificationNeeded).toBe(true);
151+
expect(result.planReady).toBe(false);
152+
expect(result.nextQuestion).toBeTruthy();
153+
expect(result.clarificationTopics?.length).toBeGreaterThan(0);
154+
});
155+
156+
it('returns clarificationNeeded=true for vague intent verbs without scope', () => {
157+
const result = evaluateClarification('improve the thing and make it better');
158+
159+
expect(result.clarificationNeeded).toBe(true);
160+
expect(result.planReady).toBe(false);
161+
expect(result.clarificationTopics).toContain(CLARIFICATION_TOPICS.VAGUE_INTENT);
162+
});
163+
164+
it('returns clarificationNeeded=true for a too-short prompt without tech reference', () => {
165+
expect('fix it'.length).toBeLessThan(MIN_PROMPT_LENGTH);
166+
const result = evaluateClarification('fix it');
167+
168+
expect(result.clarificationNeeded).toBe(true);
169+
expect(result.planReady).toBe(false);
170+
});
171+
172+
it('emits a single highest-value next question, not a list', () => {
173+
const result = evaluateClarification('개선해줘');
174+
175+
expect(result.nextQuestion).toBeTruthy();
176+
expect(typeof result.nextQuestion).toBe('string');
177+
// Single question, no bullet lists
178+
expect(result.nextQuestion).not.toMatch(/\n\s*[-*]/);
179+
});
180+
181+
it('decrements the budget on each ambiguous round', () => {
182+
const r1 = evaluateClarification('improve it', { questionBudget: 3 });
183+
expect(r1.clarificationNeeded).toBe(true);
184+
expect(r1.questionBudget).toBe(2);
185+
186+
const r2 = evaluateClarification('improve it', { questionBudget: 2 });
187+
expect(r2.clarificationNeeded).toBe(true);
188+
expect(r2.questionBudget).toBe(1);
189+
190+
const r3 = evaluateClarification('improve it', { questionBudget: 1 });
191+
expect(r3.clarificationNeeded).toBe(true);
192+
expect(r3.questionBudget).toBe(0);
193+
});
194+
195+
it('orders clarificationTopics by priority (vague-intent first when applicable)', () => {
196+
const result = evaluateClarification('개선');
197+
198+
expect(result.clarificationTopics?.[0]).toBe(CLARIFICATION_TOPICS.VAGUE_INTENT);
199+
});
200+
});
201+
202+
describe('override phrases', () => {
203+
it('returns planReady=true even when prompt is otherwise ambiguous', () => {
204+
const result = evaluateClarification('improve it, just do it');
205+
206+
expect(result.planReady).toBe(true);
207+
expect(result.clarificationNeeded).toBe(false);
208+
expect(result.questionBudget).toBe(DEFAULT_QUESTION_BUDGET);
209+
});
210+
211+
it('honors Korean override phrase 알아서', () => {
212+
const result = evaluateClarification('개선해줘 알아서 해');
213+
214+
expect(result.planReady).toBe(true);
215+
expect(result.clarificationNeeded).toBe(false);
216+
});
217+
218+
it('does not decrement budget on override path', () => {
219+
const result = evaluateClarification('improve it, use your judgment', {
220+
questionBudget: 2,
221+
});
222+
223+
expect(result.questionBudget).toBe(2);
224+
});
225+
});
226+
227+
describe('budget exhausted', () => {
228+
it('returns planReady=true with assumptionNote when budget=0', () => {
229+
const result = evaluateClarification('improve it', { questionBudget: 0 });
230+
231+
expect(result.planReady).toBe(true);
232+
expect(result.clarificationNeeded).toBe(false);
233+
expect(result.questionBudget).toBe(0);
234+
expect(result.assumptionNote).toBeTruthy();
235+
expect(result.assumptionNote).toMatch(/assum/i);
236+
});
237+
238+
it('returns planReady=true when budget is negative (defensive)', () => {
239+
const result = evaluateClarification('개선', { questionBudget: -1 });
240+
241+
expect(result.planReady).toBe(true);
242+
expect(result.clarificationNeeded).toBe(false);
243+
expect(result.questionBudget).toBe(0);
244+
});
245+
246+
it('does not trigger budget-exhausted path when request is already clear', () => {
247+
// Even with budget=0 the response is planReady; the distinguishing
248+
// factor is that a clear request does not need an assumptionNote.
249+
const clearResult = evaluateClarification('add tests to src/auth.ts', {
250+
questionBudget: 0,
251+
});
252+
253+
expect(clearResult.planReady).toBe(true);
254+
// Budget-exhausted path always sets assumptionNote
255+
expect(clearResult.assumptionNote).toBeTruthy();
256+
});
257+
});
258+
259+
describe('edge cases', () => {
260+
it('handles an empty prompt as clear (no fields to ask about)', () => {
261+
const result = evaluateClarification('');
262+
263+
// An empty string isn't "too short" (length 0), and no vague verbs or
264+
// tech references trigger — fall through to planReady.
265+
expect(result.planReady).toBe(true);
266+
});
267+
268+
it('uses DEFAULT_QUESTION_BUDGET when options are omitted', () => {
269+
const result = evaluateClarification('add tests to src/auth.ts');
270+
271+
expect(result.questionBudget).toBe(DEFAULT_QUESTION_BUDGET);
272+
});
273+
274+
it('does not include nextQuestion on planReady path', () => {
275+
const result = evaluateClarification('add tests to src/auth.ts');
276+
277+
expect(result.nextQuestion).toBeUndefined();
278+
expect(result.clarificationTopics).toBeUndefined();
279+
});
280+
});
281+
});
282+
});

0 commit comments

Comments
 (0)