From 949bea2074311da5e310ac1c6d9e8e606e7c3393 Mon Sep 17 00:00:00 2001 From: Elvis Nieves <20828017+gabynevada@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:35:25 +0100 Subject: [PATCH] feat(chat): show an MCP form's choice and its optional note as one step An MCP form elicitation whose single-select field is directly followed by an optional plain text field now renders as one step: the options above, the composer answering the text field. Both values are submitted under their own keys, so a server can read "option", "option with a note" or "text only". The choice question carries the text field as companionText; any other form shape keeps one question per field. Co-Authored-By: Claude Opus 5.5 --- .../features/threads/PendingUserInputCard.tsx | 10 ++- .../features/threads/QuestionAttachments.tsx | 21 ++++- apps/server/src/mcp/toolkits/thread/tools.ts | 9 ++ .../Adapters/AcpAdapterV2.test.ts | 7 +- .../src/orchestration-v2/Orchestrator.ts | 41 +++++---- apps/web/src/components/chat/ChatComposer.tsx | 26 +++--- .../chat/ComposerPendingUserInputPanel.tsx | 16 +++- .../chat/usePendingThreadRequests.ts | 13 ++- apps/web/src/pendingUserInput.test.ts | 29 +++++++ apps/web/src/pendingUserInput.ts | 7 +- .../src/state/pendingUserInput.test.ts | 35 ++++++++ .../src/state/pendingUserInput.ts | 56 ++++++++++-- packages/contracts/src/orchestrationV2.ts | 17 ++++ packages/contracts/src/providerRuntime.ts | 9 ++ .../src/server/elicitationForm.test.ts | 13 ++- .../src/server/elicitationForm.ts | 3 + .../src/server/mcpElicitationForm.test.ts | 85 +++++++++++++++++++ .../src/server/mcpElicitationForm.ts | 46 +++++++++- .../shared/src/userInputConstraints.test.ts | 25 +++++- packages/shared/src/userInputConstraints.ts | 33 +++++++ 20 files changed, 450 insertions(+), 51 deletions(-) diff --git a/apps/mobile/src/features/threads/PendingUserInputCard.tsx b/apps/mobile/src/features/threads/PendingUserInputCard.tsx index 3a2127ea2931c..48cfd0d0b5de5 100644 --- a/apps/mobile/src/features/threads/PendingUserInputCard.tsx +++ b/apps/mobile/src/features/threads/PendingUserInputCard.tsx @@ -288,7 +288,15 @@ export function PendingUserInputCard(props: PendingUserInputCardProps) { ); })} - {question.allowCustomAnswer !== false ? ( + {question.companionText?.description ? ( + + {question.companionText.description} + + ) : null} + {question.allowCustomAnswer !== false || question.companionText ? ( (null); const [previewVideo, setPreviewVideo] = useState(null); const configs = useServerConfigs(); @@ -97,7 +99,13 @@ export function QuestionAttachments(props: { }; const paste = useNativePaste((uris) => { const scope = pickerScope.current; - if (!selectedThread || props.disabled || !configs.get(selectedThread.environmentId)) return; + if ( + !selectedThread || + props.disabled || + companion !== undefined || + !configs.get(selectedThread.environmentId) + ) + return; const key = questionAttachmentDraftKey( selectedThread.environmentId, selectedThread.id, @@ -126,10 +134,11 @@ export function QuestionAttachments(props: { ) .finally(() => changeQuestionAttachmentPreparation(key, -1)); }); - if (!selectedThread || props.question.allowCustomAnswer === false) return null; + if (!selectedThread || (props.question.allowCustomAnswer === false && companion === undefined)) + return null; const { environmentId, id: threadId } = selectedThread; const capabilities = configs.get(environmentId)?.environment.capabilities; - const canAttach = capabilities !== undefined; + const canAttach = capabilities !== undefined && companion === undefined; const key = questionAttachmentDraftKey( environmentId, threadId, @@ -225,7 +234,11 @@ export function QuestionAttachments(props: { onInputFocusChange?.(false); }} placeholder={ - props.question.options.length === 0 ? "Type your answer" : "Or type a custom answer" + companion + ? companion.header + : props.question.options.length === 0 + ? "Type your answer" + : "Or type a custom answer" } className="min-h-[54px] rounded-2xl border border-input-border bg-input px-3.5 py-3 font-sans text-base text-foreground" /> diff --git a/apps/server/src/mcp/toolkits/thread/tools.ts b/apps/server/src/mcp/toolkits/thread/tools.ts index c6670c4e953d3..c3d07defb0e7a 100644 --- a/apps/server/src/mcp/toolkits/thread/tools.ts +++ b/apps/server/src/mcp/toolkits/thread/tools.ts @@ -123,6 +123,15 @@ const question = Schema.Struct({ allowCustomAnswer: Schema.optional(Schema.Boolean), initialAnswer: Schema.optional(Schema.String), required: Schema.optional(Schema.Boolean), + // Answered under its own id, alongside the question's own answer. + companionText: Schema.optional( + Schema.Struct({ + id: Schema.String, + header: Schema.String, + description: Schema.optional(Schema.String), + required: Schema.optional(Schema.Boolean), + }), + ), }); const PendingRequestTool = Tool.make("t3_pending_request", { ...commandTool, diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 466c42ec1a31d..f9f7f7e9639f8 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -1892,11 +1892,12 @@ describe("AcpAdapterV2", () => { options: [{ label: "Pro", description: "Pro", value: "pro" }], allowCustomAnswer: false, required: true, + // The optional note after the choice is asked in the same step. + companionText: { id: "note", header: "Note (optional)", required: false }, }, - { id: "note", header: "Note (optional)", question: "Note", options: [], required: false }, ], - answers: { count: "3", plan: "pro" }, - result: { action: "accept", content: { count: 3, plan: "pro" } }, + answers: { count: "3", plan: "pro", note: "Yearly" }, + result: { action: "accept", content: { count: 3, plan: "pro", note: "Yearly" } }, }, ])("Droid elicitation $scenario", ({ schema, questions, answers, result, withAttachments }) => Effect.gen(function* () { diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index b32a4f2cbe089..5625becf73f4d 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -82,7 +82,7 @@ import { import { modelSelectionsEqual } from "@t3tools/shared/model"; import { modelSwitchSince } from "./modelSwitch.ts"; import { recordThreadOpen } from "@t3tools/shared/paletteSearch"; -import { userInputAnswerError } from "@t3tools/shared/userInputConstraints"; +import { userInputAnswersError } from "@t3tools/shared/userInputConstraints"; import { backgroundWorkHoldsCompletion, derivePendingBackgroundWork, @@ -9746,15 +9746,13 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ]), ); const questions = context.item?.type === "user_input_request" ? context.item.questions : []; - for (const question of questions) { - const error = userInputAnswerError(question, normalized[question.id]); - if (error !== null) { - return yield* new OrchestratorDispatchError({ - commandId: command.commandId, - commandType: command.type, - cause: error, - }); - } + const error = userInputAnswersError(questions, normalized); + if (error !== null) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: error, + }); } answers = normalized; } @@ -9793,7 +9791,20 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio cause: "The provider for this project is disabled.", }); } - const questionIds = new Set(approvalTurnItem.questions.map((question) => question.id)); + // A companion text is answered under its own id, right after its question. + const prompts = approvalTurnItem.questions.flatMap((question) => [ + { id: question.id, prompt: question.question, optional: question.required === false }, + ...(question.companionText === undefined + ? [] + : [ + { + id: question.companionText.id, + prompt: question.companionText.header, + optional: question.companionText.required !== true, + }, + ]), + ]); + const questionIds = new Set(prompts.map((prompt) => prompt.id)); if ( !Object.entries(answers ?? {}).every( ([questionId, answer]) => @@ -9806,11 +9817,11 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio cause: "Answer each question before sending.", }); } - for (const question of approvalTurnItem.questions) { - const answer = answers?.[question.id]; + for (const { id, prompt, optional } of prompts) { + const answer = answers?.[id]; const answered = isMessageQuestionAnswer(answer); // Clients send "" for a skipped optional question, such as an attachment-only answer. - if (!answered && question.required === false) continue; + if (!answered && optional) continue; if (!answered) { return yield* new OrchestratorDispatchError({ commandId: command.commandId, @@ -9819,7 +9830,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }); } const text = typeof answer === "string" ? answer : answer.join(", "); - replies.push(`${question.question}\n${text.trim()}`); + replies.push(`${prompt}\n${text.trim()}`); } if (replies.length === 0) { return yield* new OrchestratorDispatchError({ diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index 769b29b30f56b..54604d607b14e 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -410,6 +410,7 @@ export interface ChatComposerProps { initialAnswer?: string | undefined; multiSelect?: boolean | undefined; allowCustomAnswer?: boolean | undefined; + companionText?: { readonly header: string } | undefined; options: ReadonlyArray; } | null; } | null; @@ -1204,8 +1205,10 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) const composerSuggestionsVisible = composerMenuOpen; const composerSuggestionListVisible = composerSuggestionsVisible && composerMenuItems.length > 0; const activePendingUserInput = pendingUserInputs[0] ?? null; + // A choice with companion text still takes the composer's text, as that companion's answer. const isChoiceOnlyPendingQuestion = - activePendingProgress?.activeQuestion?.allowCustomAnswer === false; + activePendingProgress?.activeQuestion?.allowCustomAnswer === false && + activePendingProgress.activeQuestion.companionText === undefined; const showComposerTopDrawer = pendingUserInputs.length > 0 || (!isComposerCollapsedMobile && showPlanFollowUpPrompt && activeProposedPlan !== null); @@ -1611,7 +1614,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) ) => { refreshWorkspaceSnapshotOnTyping(); if (activePendingProgress?.activeQuestion && pendingUserInputs.length > 0) { - if (activePendingProgress.activeQuestion.allowCustomAnswer === false) return; + if (isChoiceOnlyPendingQuestion) return; setComposerCursor(nextCursor); setComposerTrigger( cursorAdjacentToMention ? null : detectComposerTrigger(nextPrompt, expandedCursor), @@ -1757,10 +1760,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) }; }, ): boolean => { - if ( - activePendingUserInput && - activePendingProgress?.activeQuestion?.allowCustomAnswer === false - ) { + if (activePendingUserInput && isChoiceOnlyPendingQuestion) { return false; } const currentText = promptRef.current; @@ -1849,7 +1849,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) projectSelectionRequired || isComposerApprovalState || activePendingIsResponding || - activePendingProgress?.activeQuestion?.allowCustomAnswer === false; + isChoiceOnlyPendingQuestion; const speechInput = useEnvironmentSpeechInput({ environmentId, projectId: activeProjectId, @@ -2807,9 +2807,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) if (records.length === 0) return; if (questionAttachmentTarget !== null) { const canAnswer = - canOperateThread && - !activePendingIsResponding && - activePendingProgress?.activeQuestion?.allowCustomAnswer !== false; + canOperateThread && !activePendingIsResponding && !isChoiceOnlyPendingQuestion; const text = `${records .map((record) => formatInlineContextReference(threadContextReference(record))) .join(" ")} `; @@ -3530,9 +3528,11 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) : activePendingProgress ? isChoiceOnlyPendingQuestion ? "Choose an option above" - : activePendingProgress.activeQuestion?.options.length === 0 - ? "Type your answer" - : "Type your own answer, or leave this blank to use the selected option" + : activePendingProgress.activeQuestion?.companionText + ? activePendingProgress.activeQuestion.companionText.header + : activePendingProgress.activeQuestion?.options.length === 0 + ? "Type your answer" + : "Type your own answer, or leave this blank to use the selected option" : showPlanFollowUpPrompt && activeProposedPlan ? "Add feedback to refine the plan, or leave this blank to implement it" : projectSelectionRequired diff --git a/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx b/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx index a19e7f205cef9..d4397586a87cb 100644 --- a/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx +++ b/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx @@ -96,7 +96,9 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( // Keep question identity stable for its shortcut listener. const progress = derivePendingUserInputProgress(prompt.questions, answers, questionIndex); const activeQuestion = progress.activeQuestion; - const customAnswerActive = progress.customAnswer.trim().length > 0; + // Companion text sits beside the chosen option; a custom answer replaces it. + const companionText = activeQuestion?.companionText; + const customAnswerActive = companionText === undefined && progress.customAnswer.trim().length > 0; // Read before later hooks to preserve the question's compiler cache. const draftSelectedOptions = activeQuestion?.options.map( (option) => @@ -129,7 +131,9 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( const handleOptionSelection = (questionId: string, optionValue: string) => { if (disabled || isResponding) return; - if (activeQuestion?.multiSelect) { + // The companion text is typed after picking, so that step waits for an explicit submit, and + // picking the chosen option again clears it. + if (activeQuestion?.multiSelect || companionText !== undefined) { onToggleOption(questionId, optionValue); return; } @@ -326,6 +330,14 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( ); })} + {companionText?.description ? ( +

+ {companionText.description} +

+ ) : null} diff --git a/apps/web/src/components/chat/usePendingThreadRequests.ts b/apps/web/src/components/chat/usePendingThreadRequests.ts index d31d6e56643d7..cf0e746e3b9a3 100644 --- a/apps/web/src/components/chat/usePendingThreadRequests.ts +++ b/apps/web/src/components/chat/usePendingThreadRequests.ts @@ -27,6 +27,7 @@ import { useStableCallback } from "../../hooks/useStableCallback"; import { getUploadedAttachments, useAttachmentUploadStore } from "../../lib/attachmentUploadQueue"; import { buildPendingUserInputAnswers, + pendingUserInputTakesText, setPendingUserInputCustomAnswer, togglePendingUserInputOptionSelection, type PendingUserInputDraftAnswer, @@ -93,7 +94,7 @@ function makePendingCustomAnswerSetter(input: { return; } const question = activePendingUserInput.questions.find((entry) => entry.id === questionId); - if (!question || question.allowCustomAnswer === false) { + if (!question || !pendingUserInputTakesText(question)) { return; } input.writePrompt(value); @@ -397,9 +398,16 @@ export function usePendingThreadRequests(input: { if (!activePendingUserInput) { return; } + const selectedQuestion = activePendingUserInput.questions.find( + (entry) => entry.id === questionId, + ); + // A companion text stays beside the option, so the composer keeps it. + const keepsText = selectedQuestion?.companionText !== undefined; // The option replaces the custom answer. Anything typed there is the // user's text, so it goes back to the thread draft instead of vanishing. - const displacedAnswer = activePendingAnswerDrafts[questionId]?.customAnswer; + const displacedAnswer = keepsText + ? undefined + : activePendingAnswerDrafts[questionId]?.customAnswer; const currentPrompt = useComposerDraftStore.getState().getComposerDraft(composerDraftTarget)?.prompt ?? ""; const nextPrompt = carryDisplacedCustomAnswerIntoPrompt(currentPrompt, displacedAnswer); @@ -450,6 +458,7 @@ export function usePendingThreadRequests(input: { }, }; }); + if (keepsText) return; promptRef.current = ""; composerRef.current?.resetCursorState({ cursor: 0 }); }; diff --git a/apps/web/src/pendingUserInput.test.ts b/apps/web/src/pendingUserInput.test.ts index 49b8cbc1906ef..2c5821138d788 100644 --- a/apps/web/src/pendingUserInput.test.ts +++ b/apps/web/src/pendingUserInput.test.ts @@ -119,6 +119,35 @@ describe("pending user input question progress", () => { }); }); +it("asks a choice and its companion text as one step", () => { + const paired = { + ...singleSelectQuestion, + allowCustomAnswer: false, + required: false, + companionText: { id: "note", header: "Your own answer (optional)", required: false }, + }; + const drafts = { scope: { selectedOptionValues: ["Orchestration-first"], customAnswer: "Soon" } }; + expect(derivePendingUserInputProgress([paired], drafts, 0)).toMatchObject({ + questionIndex: 0, + selectedOptionValues: ["Orchestration-first"], + customAnswer: "Soon", + resolvedAnswer: "Orchestration-first", + usingCustomAnswer: false, + answeredQuestionCount: 1, + isLastQuestion: true, + isComplete: true, + canAdvance: true, + }); + expect(buildPendingUserInputAnswers([paired], drafts)).toEqual({ + scope: "Orchestration-first", + note: "Soon", + }); + // Text alone is a complete answer too. + expect( + derivePendingUserInputProgress([paired], { scope: { customAnswer: "Mine" } }, 0), + ).toMatchObject({ customAnswer: "Mine", isLastQuestion: true, isComplete: true }); +}); + it("lets optional questions stay blank without skipping required ones", () => { const questions = [ { ...singleSelectQuestion, required: false }, diff --git a/apps/web/src/pendingUserInput.ts b/apps/web/src/pendingUserInput.ts index bbd0495692335..9c34239a0d432 100644 --- a/apps/web/src/pendingUserInput.ts +++ b/apps/web/src/pendingUserInput.ts @@ -4,6 +4,7 @@ import { formatThreadLink } from "@t3tools/shared/threadLinks"; import { buildPendingUserInputAnswers, normalizeSelectedOptionValues, + pendingUserInputTakesText, resolvePendingUserInputAnswer, type PendingUserInputAnswer, type PendingUserInputDraftAnswer, @@ -107,8 +108,11 @@ export function derivePendingUserInputProgress( const resolvedAnswer = activeQuestion ? resolvePendingUserInputAnswer(activeQuestion, activeDraft) : null; + // A choice's companion text is typed in the composer like a custom answer. const customAnswer = - activeQuestion?.allowCustomAnswer === false ? "" : (activeDraft?.customAnswer ?? ""); + activeQuestion && pendingUserInputTakesText(activeQuestion) + ? (activeDraft?.customAnswer ?? "") + : ""; const answeredQuestionCount = countAnsweredPendingUserInputQuestions(questions, draftAnswers); const isLastQuestion = questions.length === 0 ? true : normalizedQuestionIndex >= questions.length - 1; @@ -123,6 +127,7 @@ export function derivePendingUserInputProgress( customAnswer, resolvedAnswer, usingCustomAnswer: + activeQuestion?.companionText === undefined && (activeQuestion?.initialAnswer !== undefined ? customAnswer : customAnswer.trim()).length > 0, answeredQuestionCount, isLastQuestion, diff --git a/packages/client-runtime/src/state/pendingUserInput.test.ts b/packages/client-runtime/src/state/pendingUserInput.test.ts index 74ef0bbfcdbee..8b0d36a0751c2 100644 --- a/packages/client-runtime/src/state/pendingUserInput.test.ts +++ b/packages/client-runtime/src/state/pendingUserInput.test.ts @@ -183,6 +183,41 @@ describe("buildPendingUserInputAnswers", () => { compat: "Keep the current envelope for one release window", }); }); + + it("sends a paired choice and its companion text under their own ids", () => { + const paired = { + ...singleSelectQuestion, + allowCustomAnswer: false, + required: false, + companionText: { id: "note", required: false }, + }; + const picked = togglePendingUserInputOptionSelection(paired, undefined, "Node.js"); + const noted = setPendingUserInputCustomAnswer(paired, picked, " Behind a flag "); + // The note keeps the picked option, and picking another keeps the note. + expect(noted).toEqual({ customAnswer: " Behind a flag ", selectedOptionValues: ["Node.js"] }); + expect(togglePendingUserInputOptionSelection(paired, noted, "Orchestration-first")).toEqual({ + customAnswer: " Behind a flag ", + selectedOptionValues: ["Orchestration-first"], + }); + expect(buildPendingUserInputAnswers([paired], { scope: picked })).toEqual({ + scope: "Node.js", + }); + expect(buildPendingUserInputAnswers([paired], { scope: noted })).toEqual({ + scope: "Node.js", + note: "Behind a flag", + }); + // Picking the chosen option again clears it, leaving the text as the whole answer. + const textOnly = togglePendingUserInputOptionSelection(paired, noted, "Node.js"); + expect(buildPendingUserInputAnswers([paired], { scope: textOnly })).toEqual({ + note: "Behind a flag", + }); + expect(buildPendingUserInputAnswers([paired], { scope: { customAnswer: " " } })).toEqual({}); + expect( + buildPendingUserInputAnswers([{ ...paired, companionText: { id: "note", required: true } }], { + scope: picked, + }), + ).toBeNull(); + }); }); it.each([ diff --git a/packages/client-runtime/src/state/pendingUserInput.ts b/packages/client-runtime/src/state/pendingUserInput.ts index 8c6fbedad3a13..f32a374cb6849 100644 --- a/packages/client-runtime/src/state/pendingUserInput.ts +++ b/packages/client-runtime/src/state/pendingUserInput.ts @@ -6,6 +6,10 @@ export interface PendingUserInputQuestion { readonly multiSelect?: boolean | undefined; readonly allowCustomAnswer?: boolean | undefined; readonly initialAnswer?: string | undefined; + /** Text typed in the same step, kept in the draft's customAnswer and sent under its own id. */ + readonly companionText?: + | { readonly id: string; readonly required?: boolean | undefined } + | undefined; } export interface PendingUserInputDraftAnswer { @@ -57,6 +61,19 @@ export function normalizeSelectedOptionValues( ); } +/** Whether the user types in this question's step: its custom answer or its companion text. */ +export function pendingUserInputTakesText(question: PendingUserInputQuestion): boolean { + return question.allowCustomAnswer !== false || question.companionText !== undefined; +} + +/** The companion text to submit, or null when the question has none or it is blank. */ +export function resolvePendingUserInputCompanionAnswer( + question: PendingUserInputQuestion, + draft: PendingUserInputDraftAnswer | undefined, +): string | null { + return question.companionText === undefined ? null : normalizeDraftAnswer(draft?.customAnswer); +} + /** The answer to submit for one question, or null while it is unanswered. */ export function resolvePendingUserInputAnswer( question: PendingUserInputQuestion, @@ -92,6 +109,14 @@ export function setPendingUserInputCustomAnswer( draft: PendingUserInputDraftAnswer | undefined, customAnswer: string, ): PendingUserInputDraftAnswer { + if (question.companionText !== undefined) { + // Companion text sits beside the chosen option instead of replacing it. + const selectedOptionValues = normalizeSelectedOptionValues( + question, + draft?.selectedOptionValues, + ); + return { customAnswer, ...(selectedOptionValues.length > 0 ? { selectedOptionValues } : {}) }; + } if (question.allowCustomAnswer === false) { return draft ?? {}; } @@ -134,6 +159,8 @@ export function togglePendingUserInputOptionSelection( if (resolvedOptionValue === null) { return draft ?? {}; } + // An option replaces a custom answer, but companion text is kept beside it. + const customAnswer = question.companionText === undefined ? "" : (draft?.customAnswer ?? ""); if (question.multiSelect) { const selectedOptionValues = normalizeSelectedOptionValues( @@ -145,20 +172,32 @@ export function togglePendingUserInputOptionSelection( : [...selectedOptionValues, resolvedOptionValue]; return { - customAnswer: "", + customAnswer, ...(nextSelectedOptionValues.length > 0 ? { selectedOptionValues: nextSelectedOptionValues } : {}), }; } + // With companion text the choice can be cleared, leaving the text as the whole answer. + if ( + question.companionText !== undefined && + normalizeSelectedOptionValues(question, draft?.selectedOptionValues).includes( + resolvedOptionValue, + ) + ) { + return { customAnswer }; + } return { - customAnswer: "", + customAnswer, selectedOptionValues: [resolvedOptionValue], }; } -/** Every question's answer keyed by question id, or null while any is unanswered. */ +/** + * Every question's answer keyed by question id, and each companion text by its own id, or null + * while any required one is unanswered. + */ export function buildPendingUserInputAnswers( questions: ReadonlyArray, draftAnswers: Record, @@ -166,10 +205,17 @@ export function buildPendingUserInputAnswers( const entries: Array<[string, PendingUserInputAnswer]> = []; for (const question of questions) { - const answer = resolvePendingUserInputAnswer(question, draftAnswers[question.id]); + const draft = draftAnswers[question.id]; + const answer = resolvePendingUserInputAnswer(question, draft); if (answer !== null) { entries.push([question.id, answer]); - } else if (question.required !== false || draftAnswers[question.id]?.attachmentsBlocked) { + } else if (question.required !== false || draft?.attachmentsBlocked) { + return null; + } + const companionAnswer = resolvePendingUserInputCompanionAnswer(question, draft); + if (companionAnswer !== null) { + entries.push([question.companionText!.id, companionAnswer]); + } else if (question.companionText?.required === true) { return null; } } diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index 510e6b18d3bea..c0274acae4d77 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -1356,6 +1356,22 @@ export const OrchestrationV2UserInputAnswerConstraints = Schema.Struct({ export type OrchestrationV2UserInputAnswerConstraints = typeof OrchestrationV2UserInputAnswerConstraints.Type; +/** + * A free-text field asked in the same step as a single-select question, such as a note on the + * picked option or the user's own answer. Its answer is keyed by its own id; absent means the + * question stands alone. + */ +export const OrchestrationV2UserInputCompanionText = Schema.Struct({ + id: TrimmedNonEmptyString, + header: TrimmedNonEmptyString, + description: Schema.optional(TrimmedNonEmptyString), + /** Absent means the text may be left blank. */ + required: Schema.optional(Schema.Boolean), + constraints: Schema.optional(OrchestrationV2UserInputAnswerConstraints), +}); +export type OrchestrationV2UserInputCompanionText = + typeof OrchestrationV2UserInputCompanionText.Type; + export const OrchestrationV2UserInputQuestion = Schema.Struct({ id: TrimmedNonEmptyString, header: TrimmedNonEmptyString, @@ -1373,6 +1389,7 @@ export const OrchestrationV2UserInputQuestion = Schema.Struct({ initialAnswer: Schema.optional(Schema.String), required: Schema.optional(Schema.Boolean), constraints: Schema.optional(OrchestrationV2UserInputAnswerConstraints), + companionText: Schema.optional(OrchestrationV2UserInputCompanionText), }); export type OrchestrationV2UserInputQuestion = typeof OrchestrationV2UserInputQuestion.Type; diff --git a/packages/contracts/src/providerRuntime.ts b/packages/contracts/src/providerRuntime.ts index 4769bfd7955c7..a5c4160f16912 100644 --- a/packages/contracts/src/providerRuntime.ts +++ b/packages/contracts/src/providerRuntime.ts @@ -131,6 +131,14 @@ const UserInputQuestionOption = Schema.Struct({ value: Schema.optional(Schema.String), }); +/** A free-text field answered in the same step as a single-select question, keyed by its own id. */ +const UserInputCompanionText = Schema.Struct({ + id: TrimmedNonEmptyStringSchema, + header: TrimmedNonEmptyStringSchema, + description: Schema.optional(TrimmedNonEmptyStringSchema), + required: Schema.optional(Schema.Boolean), +}); + export const UserInputQuestion = Schema.Struct({ id: TrimmedNonEmptyStringSchema, header: TrimmedNonEmptyStringSchema, @@ -138,6 +146,7 @@ export const UserInputQuestion = Schema.Struct({ options: Schema.Array(UserInputQuestionOption), allowCustomAnswer: Schema.optional(Schema.Boolean), initialAnswer: Schema.optional(Schema.String), + companionText: Schema.optional(UserInputCompanionText), multiSelect: Schema.optional(Schema.Boolean).pipe( Schema.withConstructorDefault(Effect.succeed(false)), ), diff --git a/packages/provider-acp/src/server/elicitationForm.test.ts b/packages/provider-acp/src/server/elicitationForm.test.ts index 1e83ccaf0481d..038676a281bdc 100644 --- a/packages/provider-acp/src/server/elicitationForm.test.ts +++ b/packages/provider-acp/src/server/elicitationForm.test.ts @@ -71,13 +71,24 @@ it("preserves optional fields, defaults, constraints, and empty required lists", constraints: { type: "integer", minimum: 1 }, }); assert.strictEqual(form.questions[1]?.options[0]?.value, "pro"); + // The optional note after the choice is asked in its step and answered under its own id. + assert.strictEqual(form.questions[1]?.companionText?.id, "note"); + assert.deepStrictEqual( + form.questions.map((question) => question.id), + ["count", "plan", "tags"], + ); assert.strictEqual(form.questions[2]?.required, false); - assert.strictEqual(form.questions[3]?.required, false); assert.deepStrictEqual(form.content({ count: "3", plan: "pro" }), { count: 3, plan: "pro", tags: [], }); + assert.deepStrictEqual(form.content({ count: "3", plan: "pro", note: "Soon" }), { + count: 3, + plan: "pro", + note: "Soon", + tags: [], + }); assert.deepStrictEqual(form.content({ count: "bad", plan: "pro" }), undefined); const confirmation = parseElicitationForm({ type: "object", properties: {} }, "Continue?"); assert.strictEqual(confirmation?.questions[0]?.id, "confirm"); diff --git a/packages/provider-acp/src/server/elicitationForm.ts b/packages/provider-acp/src/server/elicitationForm.ts index 4aaa51a87c821..caed351dbcf78 100644 --- a/packages/provider-acp/src/server/elicitationForm.ts +++ b/packages/provider-acp/src/server/elicitationForm.ts @@ -190,6 +190,8 @@ export function parseElicitationForm( ); const mcpForm = mcpElicitationForm({ requestedSchema, message }); const questionMetadata = new Map(mcpForm?.questions.map((question) => [question.id, question])); + // A companion text field is asked within its choice, and answered under its own id. + const companionIds = new Set(mcpForm?.questions.flatMap((q) => q.companionText?.id ?? [])); const fields = new Map(); const questions: OrchestrationV2UserInputQuestion[] = []; for (const [id, value] of Object.entries(unknownRecord(schema?.properties) ?? {})) { @@ -198,6 +200,7 @@ export function parseElicitationForm( // Question ids must survive the contract's trim to round-trip answers. if (property === undefined || parsed === undefined || text(id) !== id) return undefined; fields.set(id, parsed.field); + if (companionIds.has(id)) continue; questions.push({ id, header: text(property.title) ?? `Question ${questions.length + 1}`, diff --git a/packages/provider-core/src/server/mcpElicitationForm.test.ts b/packages/provider-core/src/server/mcpElicitationForm.test.ts index 5aa8e15e9e25c..035e8494f4d85 100644 --- a/packages/provider-core/src/server/mcpElicitationForm.test.ts +++ b/packages/provider-core/src/server/mcpElicitationForm.test.ts @@ -237,6 +237,91 @@ describe("mcpElicitationForm", () => { expect(form?.content({ confirm: "Continue" })).toEqual({}); }); + const choiceWithNote = { + type: "object", + properties: { + option: { + type: "string", + title: "Your answer", + oneOf: [ + { const: "opt-1", title: "Follow-up" }, + { const: "opt-2", title: "Isolate now" }, + ], + }, + other: { + type: "string", + title: "Your own answer", + description: "Add a note, or answer in your own words.", + maxLength: 50, + }, + }, + required: [], + }; + + it("asks a choice and the optional text field after it as one step", () => { + const form = mcpElicitationForm({ + requester: "Skald", + message: "Should the sandbox isolate files?", + requestedSchema: choiceWithNote, + }); + expect(form?.questions).toEqual([ + { + id: "option", + header: "Your answer (optional)", + question: "Skald asks: Should the sandbox isolate files?", + options: [ + { label: "Follow-up", description: "Follow-up" }, + { label: "Isolate now", description: "Isolate now" }, + ], + allowCustomAnswer: false, + required: false, + companionText: { + id: "other", + header: "Your own answer (optional)", + description: + "Add a note, or answer in your own words.\n\nEnter text of at most 50 characters.", + required: false, + constraints: { maxLength: 50 }, + }, + }, + ]); + }); + + it("sends a paired choice and its text under their own keys", () => { + const form = mcpElicitationForm({ message: "Isolate?", requestedSchema: choiceWithNote }); + expect(form?.content({ option: "Isolate now" })).toEqual({ option: "opt-2" }); + expect(form?.content({ option: "Isolate now", other: " In a second PR " })).toEqual({ + option: "opt-2", + other: " In a second PR ", + }); + expect(form?.content({ other: "Do both" })).toEqual({ other: "Do both" }); + expect(form?.content({ other: " " })).toEqual({}); + expect(form?.content({})).toEqual({}); + }); + + it("keeps separate questions when the text is required, not adjacent, or follows a list", () => { + const choice = { type: "string", oneOf: [{ const: "a", title: "A" }] }; + const list = { type: "array", items: { enum: ["a", "b"] } }; + for (const requestedSchema of [ + { + type: "object", + properties: { option: choice, note: { type: "string" } }, + required: ["note"], + }, + { + type: "object", + properties: { option: choice, count: { type: "integer" }, note: { type: "string" } }, + }, + { type: "object", properties: { option: list, note: { type: "string" } } }, + ]) { + const form = mcpElicitationForm({ message: "Hi", requestedSchema }); + expect(form?.questions.map((question) => question.id)).toEqual( + Object.keys(requestedSchema.properties), + ); + expect(form?.questions.some((question) => question.companionText)).toBe(false); + } + }); + it("rejects schemas outside the flat MCP form subset", () => { for (const schema of [ undefined, diff --git a/packages/provider-core/src/server/mcpElicitationForm.ts b/packages/provider-core/src/server/mcpElicitationForm.ts index 07bc65a355c3d..f8577c4410366 100644 --- a/packages/provider-core/src/server/mcpElicitationForm.ts +++ b/packages/provider-core/src/server/mcpElicitationForm.ts @@ -165,9 +165,23 @@ function contentValue(field: Field, answer: unknown): McpElicitationContent[stri } } +/** An optional plain text field: no choices, format or suggested value. */ +function isOptionalPlainText(key: string, field: Field, requiredKeys: ReadonlySet) { + return ( + field.type === "string" && + !requiredKeys.has(key) && + field.enum == null && + field.oneOf == null && + field.format == null && + field.default == null + ); +} + /** * Maps an MCP form elicitation onto user-input questions, or returns null when the - * schema is not the flat form the MCP spec allows. + * schema is not the flat form the MCP spec allows. A single-select field directly followed by + * an optional plain text field is asked as one step, the text as its companion: MCP forms have + * no "pick one, add a note, or answer in your own words" field, so servers send the two. */ export function mcpElicitationForm(input: { readonly requester?: string; @@ -188,6 +202,10 @@ export function mcpElicitationForm(input: { : message ? `${input.requester} asks: ${message}` : `${input.requester} asks:`; + const describe = (field: Field, constraints = fieldConstraints(field)) => + [text(field.description), constraints && describeUserInputConstraints(constraints)] + .filter(Boolean) + .join("\n\n"); const questions: Array = []; for (const [index, [key, field]] of fields.entries()) { const options = fieldOptions(field); @@ -200,13 +218,24 @@ export function mcpElicitationForm(input: { ) { return null; } + // A companion field is asked within the choice before it. + if (questions.at(-1)?.companionText?.id === key) continue; const required = requiredKeys.has(key) && !acceptsEmptyList(key, field); const header = text(field.title) ?? key; const constraints = fieldConstraints(field); - const hint = constraints && describeUserInputConstraints(constraints); - const prompt = [index === 0 ? lead : undefined, text(field.description), hint] + const prompt = [index === 0 ? lead : undefined, describe(field, constraints)] .filter(Boolean) .join("\n\n"); + const next = fields[index + 1]; + const companion = + field.type === "string" && + options !== undefined && + next !== undefined && + isOptionalPlainText(next[0], next[1], requiredKeys) + ? next + : undefined; + const companionConstraints = companion && fieldConstraints(companion[1]); + const companionDescription = companion && describe(companion[1], companionConstraints); const initialAnswer = options === undefined && (typeof field.default === "string" || typeof field.default === "number") @@ -222,6 +251,17 @@ export function mcpElicitationForm(input: { ...(initialAnswer === undefined ? {} : { initialAnswer }), required, ...(constraints === undefined ? {} : { constraints }), + ...(companion === undefined + ? {} + : { + companionText: { + id: companion[0], + header: `${text(companion[1].title) ?? companion[0]} (optional)`, + ...(companionDescription ? { description: companionDescription } : {}), + required: false, + ...(companionConstraints === undefined ? {} : { constraints: companionConstraints }), + }, + }), }); } if (fields.length === 0) { diff --git a/packages/shared/src/userInputConstraints.test.ts b/packages/shared/src/userInputConstraints.test.ts index 362e1ee960a32..a07919005e51f 100644 --- a/packages/shared/src/userInputConstraints.test.ts +++ b/packages/shared/src/userInputConstraints.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; -import { parseUserInputNumber, userInputAnswerError } from "./userInputConstraints.ts"; +import { + parseUserInputNumber, + userInputAnswerError, + userInputAnswersError, +} from "./userInputConstraints.ts"; const question = (constraints: Parameters[0]["constraints"]) => ({ header: "Field", @@ -75,4 +79,23 @@ describe("userInputAnswerError", () => { expect(userInputAnswerError(choice, ["pro", "Pro"])).toBe("Plan: Choose one option."); expect(userInputAnswerError(question({ type: "integer" }), ["3"])).not.toBeNull(); }); + + it("checks a choice's companion text under its own id", () => { + const paired = { + id: "option", + header: "Your answer", + options: [{ label: "Follow-up", description: "Follow-up" }], + allowCustomAnswer: false, + required: false, + companionText: { id: "other", header: "Note", constraints: { maxLength: 5 } }, + }; + expect(userInputAnswersError([paired], { option: "Follow-up", other: "Soon" })).toBeNull(); + expect(userInputAnswersError([paired], { other: "Later" })).toBeNull(); + expect(userInputAnswersError([paired], { option: "Follow-up", other: "Much later" })).toBe( + "Note: Enter text of at most 5 characters.", + ); + expect(userInputAnswersError([paired], { option: "Something else" })).toBe( + "Your answer: Choose one of the listed options.", + ); + }); }); diff --git a/packages/shared/src/userInputConstraints.ts b/packages/shared/src/userInputConstraints.ts index 54bab5778eb7a..f174f014b7f7a 100644 --- a/packages/shared/src/userInputConstraints.ts +++ b/packages/shared/src/userInputConstraints.ts @@ -146,6 +146,39 @@ export function userInputAnswerError( : null; } +/** + * Returns why any answer breaks its question's rules, a companion text included, or null when + * every answer is acceptable. Answers are keyed by question id and companion text id. + */ +export function userInputAnswersError( + questions: ReadonlyArray< + Pick< + OrchestrationV2UserInputQuestion, + | "id" + | "header" + | "options" + | "allowCustomAnswer" + | "multiSelect" + | "required" + | "constraints" + | "companionText" + > + >, + answers: Readonly | undefined>>, +): string | null { + const answer = (id: string) => (Object.hasOwn(answers, id) ? answers[id] : undefined); + for (const question of questions) { + const companion = question.companionText; + const error = + userInputAnswerError(question, answer(question.id)) ?? + (companion === undefined + ? null + : userInputAnswerError({ ...companion, options: [] }, answer(companion.id))); + if (error !== null) return error; + } + return null; +} + /** Checks a string exactly as given against format and length rules, blank values included. */ export function breaksTextConstraints( constraints: NonNullable,