feat: harden ask tool validation and fallback

2026-04-19 18:15:19 +02:00
parent 9c559c9084
commit 050e8800cd
8 changed files with 464 additions and 19 deletions
@@ -25,24 +25,26 @@ export function registerAskTool(pi: ExtensionAPI) {
 		parameters: AskParamsSchema,

 		async execute(_toolCallId, params: AskParams, _signal, _onUpdate, ctx) {
+			const validation = validateParams(params);
+			if (!validation.ok) {
+				return {
+					content: [{ type: "text", text: `Error: ${validation.error}` }],
+					details: errorResultDetails(params, true),
+				};
+			}
+
 			if (!ctx.hasUI) {
-				const details = errorResultDetails(params, true);
 				return {
 					content: [
 						{
 							type: "text",
-							text: "Error: UI not available (running in non-interactive mode)",
+							text: formatNonInteractiveMessage(validation.state),
 						},
 					],
-					details,
-				};
-			}
-
-			if (!params.questions?.length) {
-				const details = errorResultDetails(params, true);
-				return {
-					content: [{ type: "text", text: "Error: No questions provided" }],
-					details,
+					details: {
+						...toAskResult(validation.state),
+						cancelled: true,
+					},
 				};
 			}

@@ -106,3 +108,46 @@ function errorResultDetails(params: AskParams, cancelled: boolean): AskResult {
 		};
 	}
 }
+
+function validateParams(
+	params: AskParams
+):
+	| { ok: true; state: ReturnType<typeof createInitialState> }
+	| { ok: false; error: string } {
+	try {
+		return {
+			ok: true,
+			state: createInitialState(params),
+		};
+	} catch (error) {
+		return {
+			ok: false,
+			error:
+				error instanceof Error ? error.message : "Invalid ask_user payload",
+		};
+	}
+}
+
+function formatNonInteractiveMessage(
+	state: ReturnType<typeof createInitialState>
+): string {
+	const lines = [
+		"Needs user input: ask_user requires interactive UI.",
+		"Run same tool call in interactive session, or ask user these questions manually:",
+	];
+
+	for (const [index, question] of state.questions.entries()) {
+		lines.push(`${index + 1}. ${question.label}: ${question.prompt}`);
+		for (const option of question.options) {
+			lines.push(`   - ${option.label} [${option.value}]`);
+		}
+		if (question.type !== "preview") {
+			lines.push("   - Type your own [custom]");
+		}
+	}
+
+	lines.push(
+		"details.questions contains normalized pending questions. details.answers stays empty until user responds."
+	);
+	return lines.join("\n");
+}
@@ -2,19 +2,23 @@ import { Type } from "@sinclair/typebox";

 export const AskOptionSchema = Type.Object({
 	value: Type.String({
+		minLength: 1,
 		description:
 			"Machine-readable value returned for this option in the result",
 	}),
 	label: Type.String({
+		minLength: 1,
 		description: "Short visible option label shown in the list",
 	}),
 	description: Type.Optional(
 		Type.String({
+			minLength: 1,
 			description: "Optional one-line explanation to help the user choose",
 		})
 	),
 	preview: Type.Optional(
 		Type.String({
+			minLength: 1,
 			description:
 				"Optional preview content shown in the dedicated preview pane for preview questions",
 		})
@@ -23,15 +27,18 @@ export const AskOptionSchema = Type.Object({

 export const AskQuestionSchema = Type.Object({
 	id: Type.String({
+		minLength: 1,
 		description:
 			"Stable question identifier used as the key in returned answers",
 	}),
 	label: Type.Optional(
 		Type.String({
+			minLength: 1,
 			description: "Short tab label, e.g. Goal, Audience, Tone, Scope",
 		})
 	),
 	prompt: Type.String({
+		minLength: 1,
 		description:
 			"Direct question shown to the user; ask about one decision at a time",
 	}),
@@ -51,6 +58,7 @@ export const AskQuestionSchema = Type.Object({
 		})
 	),
 	options: Type.Array(AskOptionSchema, {
+		minItems: 1,
 		description: "Answer options; prefer 2-4 clear choices when possible",
 	}),
 });
@@ -58,11 +66,13 @@ export const AskQuestionSchema = Type.Object({
 export const AskParamsSchema = Type.Object({
 	title: Type.Optional(
 		Type.String({
+			minLength: 1,
 			description:
 				"Optional short title shown above the clarification flow, e.g. README direction",
 		})
 	),
 	questions: Type.Array(AskQuestionSchema, {
+		minItems: 1,
 		description: "Questions to ask in the interactive clarification flow",
 	}),
 });
@@ -1,6 +1,12 @@
-import type { AskParams, AskQuestion, AskQuestionInput } from "../types.ts";
+import type {
+	AskOption,
+	AskParams,
+	AskQuestion,
+	AskQuestionInput,
+} from "../types.ts";

 export function normalizeQuestions(params: AskParams): AskQuestion[] {
+	validateQuestions(params.questions);
 	return params.questions.map((question, index) =>
 		normalizeQuestion(question, index)
 	);
@@ -11,9 +17,137 @@ function normalizeQuestion(
 	index: number
 ): AskQuestion {
 	return {
-		...question,
+		id: question.id.trim(),
 		label: question.label?.trim() || `Q${index + 1}`,
+		prompt: question.prompt.trim(),
 		type: question.type ?? "single",
-		required: question.required !== false,
+		required: question.required ?? false,
+		options: question.options.map(normalizeOption),
 	};
 }
+
+function normalizeOption(option: AskOption): AskOption {
+	return {
+		value: option.value.trim(),
+		label: option.label.trim(),
+		description: option.description?.trim(),
+		preview: option.preview?.trim(),
+	};
+}
+
+function validateQuestions(questions: AskParams["questions"]): void {
+	if (questions.length === 0) {
+		throw new Error("At least one question is required");
+	}
+
+	const questionIds = new Set<string>();
+	for (const [questionIndex, question] of questions.entries()) {
+		validateQuestion(question, questionIndex, questionIds);
+	}
+}
+
+function validateQuestion(
+	question: AskQuestionInput,
+	questionIndex: number,
+	questionIds: Set<string>
+): void {
+	const questionNumber = questionIndex + 1;
+	const questionId = question.id.trim();
+	const questionType = question.type ?? "single";
+
+	assertNonEmpty(questionId, `Question ${questionNumber}: id is required`);
+	assertUnique(
+		questionIds,
+		questionId,
+		`Question ${questionNumber}: duplicate question id "${questionId}"`
+	);
+	assertOptionalNonEmpty(
+		question.label,
+		`Question ${questionNumber}: label must not be empty`
+	);
+	assertNonEmpty(
+		question.prompt.trim(),
+		`Question ${questionNumber}: prompt is required`
+	);
+	assertHasItems(
+		question.options,
+		`Question ${questionNumber}: at least one option is required`
+	);
+
+	const optionValues = new Set<string>();
+	for (const [optionIndex, option] of question.options.entries()) {
+		validateOption(
+			option,
+			optionIndex,
+			optionValues,
+			questionNumber,
+			questionType
+		);
+	}
+}
+
+function validateOption(
+	option: AskOption,
+	optionIndex: number,
+	optionValues: Set<string>,
+	questionNumber: number,
+	questionType: AskQuestion["type"]
+): void {
+	const optionNumber = optionIndex + 1;
+	const prefix = `Question ${questionNumber}, option ${optionNumber}`;
+	const optionValue = option.value.trim();
+
+	assertNonEmpty(optionValue, `${prefix}: value is required`);
+	assertUnique(
+		optionValues,
+		optionValue,
+		`${prefix}: duplicate option value "${optionValue}"`
+	);
+	assertNonEmpty(option.label.trim(), `${prefix}: label is required`);
+	assertOptionalNonEmpty(
+		option.description,
+		`${prefix}: description must not be empty`
+	);
+	assertOptionalNonEmpty(
+		option.preview,
+		`${prefix}: preview must not be empty`
+	);
+	if (questionType === "preview") {
+		assertNonEmpty(
+			option.preview?.trim(),
+			`${prefix}: preview questions require preview text for every option`
+		);
+	}
+}
+
+function assertHasItems(items: unknown[], errorMessage: string): void {
+	if (items.length === 0) {
+		throw new Error(errorMessage);
+	}
+}
+
+function assertNonEmpty(value: string | undefined, errorMessage: string): void {
+	if (!value) {
+		throw new Error(errorMessage);
+	}
+}
+
+function assertOptionalNonEmpty(
+	value: string | undefined,
+	errorMessage: string
+): void {
+	if (value !== undefined && !value.trim()) {
+		throw new Error(errorMessage);
+	}
+}
+
+function assertUnique(
+	seen: Set<string>,
+	value: string,
+	errorMessage: string
+): void {
+	if (seen.has(value)) {
+		throw new Error(errorMessage);
+	}
+	seen.add(value);
+}