From 0ab01a7ca646b3f85ba8b5f81a0f1ff5c64032d7 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 6 Apr 2026 15:52:46 -0700 Subject: [PATCH] Add multi-select support for pending user inputs - Preserve question multi-select metadata end to end - Toggle option selections in the composer and submit arrays - Cover keyboard and answer-building behavior with tests --- .../src/provider/Layers/CodexAdapter.test.ts | 2 + .../src/provider/Layers/CodexAdapter.ts | 2 + apps/web/src/components/ChatView.browser.tsx | 212 +++++++++++++++--- apps/web/src/components/ChatView.tsx | 18 +- .../chat/ComposerPendingUserInputPanel.tsx | 42 ++-- apps/web/src/pendingUserInput.test.ts | 168 +++++++++----- apps/web/src/pendingUserInput.ts | 78 +++++-- apps/web/src/session-logic.test.ts | 4 + apps/web/src/session-logic.ts | 1 + 9 files changed, 398 insertions(+), 129 deletions(-) diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts index b5eb873e85..db91e8da0d 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.test.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts @@ -723,6 +723,7 @@ lifecycleLayer("CodexAdapterLive lifecycle", (it) => { description: "Allow workspace writes only", }, ], + multiSelect: true, }, ], }, @@ -749,6 +750,7 @@ lifecycleLayer("CodexAdapterLive lifecycle", (it) => { if (events[0]?.type === "user-input.requested") { assert.equal(events[0].requestId, "req-user-input-1"); assert.equal(events[0].payload.questions[0]?.id, "sandbox_mode"); + assert.equal(events[0].payload.questions[0]?.multiSelect, true); } assert.equal(events[1]?.type, "user-input.resolved"); diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index 8b9f3b59e7..957ae1b2bc 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -382,6 +382,7 @@ function toUserInputQuestions(payload: Record | undefined) { header, question: prompt, options, + multiSelect: question.multiSelect === true, }; }) .filter( @@ -392,6 +393,7 @@ function toUserInputQuestions(payload: Record | undefined) { header: string; question: string; options: Array<{ label: string; description: string }>; + multiSelect: boolean; } => question !== undefined, ); diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 1d9108aab9..8e848205da 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -12,6 +12,7 @@ import { type ServerLifecycleWelcomePayload, type ThreadId, type TurnId, + type UserInputQuestion, WS_METHODS, OrchestrationSessionStatus, DEFAULT_SERVER_SETTINGS, @@ -541,12 +542,51 @@ function createSnapshotWithLongProposedPlan(): OrchestrationReadModel { }; } -function createSnapshotWithPendingUserInput(): OrchestrationReadModel { +function createSnapshotWithPendingUserInput(options?: { + questions?: ReadonlyArray; +}): OrchestrationReadModel { const snapshot = createSnapshotForTargetUser({ targetMessageId: "msg-user-pending-input-target" as MessageId, targetText: "question thread", }); + const questions = + options?.questions ?? + ([ + { + id: "scope", + header: "Scope", + question: "What should this change cover?", + options: [ + { + label: "Tight", + description: "Touch only the footer layout logic.", + }, + { + label: "Broad", + description: "Also adjust the related composer controls.", + }, + ], + multiSelect: false, + }, + { + id: "risk", + header: "Risk", + question: "How aggressive should the imaginary plan be?", + options: [ + { + label: "Conservative", + description: "Favor reliability and low-risk changes.", + }, + { + label: "Balanced", + description: "Mix quick wins with one structural improvement.", + }, + ], + multiSelect: false, + }, + ] satisfies ReadonlyArray); + return { ...snapshot, threads: snapshot.threads.map((thread) => @@ -561,38 +601,7 @@ function createSnapshotWithPendingUserInput(): OrchestrationReadModel { summary: "User input requested", payload: { requestId: "req-browser-user-input", - questions: [ - { - id: "scope", - header: "Scope", - question: "What should this change cover?", - options: [ - { - label: "Tight", - description: "Touch only the footer layout logic.", - }, - { - label: "Broad", - description: "Also adjust the related composer controls.", - }, - ], - }, - { - id: "risk", - header: "Risk", - question: "How aggressive should the imaginary plan be?", - options: [ - { - label: "Conservative", - description: "Favor reliability and low-risk changes.", - }, - { - label: "Balanced", - description: "Mix quick wins with one structural improvement.", - }, - ], - }, - ], + questions, }, turnId: null, sequence: 1, @@ -2902,6 +2911,143 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + it("does not trigger numeric option shortcuts while the composer is focused", async () => { + const mounted = await mountChatView({ + viewport: WIDE_FOOTER_VIEWPORT, + snapshot: createSnapshotWithPendingUserInput(), + }); + + try { + const composerEditor = await waitForComposerEditor(); + composerEditor.focus(); + + const event = new KeyboardEvent("keydown", { + key: "2", + bubbles: true, + cancelable: true, + }); + composerEditor.dispatchEvent(event); + await waitForLayout(); + + expect(event.defaultPrevented).toBe(false); + expect(document.body.textContent).toContain("What should this change cover?"); + expect(document.body.textContent).not.toContain( + "How aggressive should the imaginary plan be?", + ); + await waitForButtonByText("Next question"); + } finally { + await mounted.cleanup(); + } + }); + + it("submits multi-select questionnaire answers as arrays", async () => { + const mounted = await mountChatView({ + viewport: WIDE_FOOTER_VIEWPORT, + snapshot: createSnapshotWithPendingUserInput({ + questions: [ + { + id: "scope", + header: "Scope", + question: "Which areas should this change cover?", + options: [ + { + label: "Server", + description: "Touch server orchestration.", + }, + { + label: "Web", + description: "Touch the browser UI.", + }, + ], + multiSelect: true, + }, + { + id: "risk", + header: "Risk", + question: "How aggressive should the imaginary plan be?", + options: [ + { + label: "Balanced", + description: "Mix quick wins with one structural improvement.", + }, + ], + multiSelect: false, + }, + ], + }), + resolveRpc: (body) => { + if (body._tag === ORCHESTRATION_WS_METHODS.dispatchCommand) { + return { + sequence: fixture.snapshot.snapshotSequence + 1, + }; + } + return undefined; + }, + }); + + try { + const serverOption = await waitForButtonContainingText("Server"); + serverOption.click(); + await waitForLayout(); + + expect(document.body.textContent).toContain("Which areas should this change cover?"); + + const webOption = await waitForButtonContainingText("Web"); + webOption.click(); + await waitForLayout(); + + expect(document.body.textContent).toContain("Which areas should this change cover?"); + + const nextButton = await waitForButtonByText("Next question"); + expect(nextButton.disabled).toBe(false); + nextButton.click(); + + await vi.waitFor( + () => { + expect(document.body.textContent).toContain( + "How aggressive should the imaginary plan be?", + ); + }, + { timeout: 8_000, interval: 16 }, + ); + + const balancedOption = await waitForButtonContainingText("Balanced"); + balancedOption.click(); + + const submitButton = await waitForButtonByText("Submit answers"); + expect(submitButton.disabled).toBe(false); + submitButton.click(); + + await vi.waitFor( + () => { + const dispatchRequest = wsRequests.find( + (request) => + request._tag === ORCHESTRATION_WS_METHODS.dispatchCommand && + request.type === "thread.user-input.respond", + ) as + | { + _tag: string; + type?: string; + answers?: Record; + } + | undefined; + + expect(dispatchRequest).toMatchObject({ + _tag: ORCHESTRATION_WS_METHODS.dispatchCommand, + type: "thread.user-input.respond", + answers: { + scope: ["Server", "Web"], + risk: "Balanced", + }, + }); + }, + { timeout: 8_000, interval: 16 }, + ); + } finally { + await mounted.cleanup(); + } + }); + it("keeps plan follow-up footer actions fused and aligned after a real resize", async () => { const mounted = await mountChatView({ viewport: WIDE_FOOTER_VIEWPORT, diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 5d160f4bda..7c649b5003 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -61,6 +61,7 @@ import { buildPendingUserInputAnswers, derivePendingUserInputProgress, setPendingUserInputCustomAnswer, + togglePendingUserInputOptionSelection, type PendingUserInputDraftAnswer, } from "../pendingUserInput"; import { useStore } from "../store"; @@ -3207,19 +3208,24 @@ export default function ChatView({ threadId }: ChatViewProps) { [activePendingUserInput], ); - const onSelectActivePendingUserInputOption = useCallback( + const onToggleActivePendingUserInputOption = useCallback( (questionId: string, optionLabel: string) => { if (!activePendingUserInput) { return; } + const question = activePendingUserInput.questions.find((entry) => entry.id === questionId); + if (!question) { + return; + } setPendingUserInputAnswersByRequestId((existing) => ({ ...existing, [activePendingUserInput.requestId]: { ...existing[activePendingUserInput.requestId], - [questionId]: { - selectedOptionLabel: optionLabel, - customAnswer: "", - }, + [questionId]: togglePendingUserInputOptionSelection( + question, + existing[activePendingUserInput.requestId]?.[questionId], + optionLabel, + ), }, })); promptRef.current = ""; @@ -4063,7 +4069,7 @@ export default function ChatView({ threadId }: ChatViewProps) { respondingRequestIds={respondingRequestIds} answers={activePendingDraftAnswers} questionIndex={activePendingQuestionIndex} - onSelectOption={onSelectActivePendingUserInputOption} + onToggleOption={onToggleActivePendingUserInputOption} onAdvance={onAdvanceActivePendingUserInput} /> diff --git a/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx b/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx index c8cad7bf36..2a1efb4b20 100644 --- a/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx +++ b/apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx @@ -13,7 +13,7 @@ interface PendingUserInputPanelProps { respondingRequestIds: ApprovalRequestId[]; answers: Record; questionIndex: number; - onSelectOption: (questionId: string, optionLabel: string) => void; + onToggleOption: (questionId: string, optionLabel: string) => void; onAdvance: () => void; } @@ -22,7 +22,7 @@ export const ComposerPendingUserInputPanel = memo(function ComposerPendingUserIn respondingRequestIds, answers, questionIndex, - onSelectOption, + onToggleOption, onAdvance, }: PendingUserInputPanelProps) { if (pendingUserInputs.length === 0) return null; @@ -36,7 +36,7 @@ export const ComposerPendingUserInputPanel = memo(function ComposerPendingUserIn isResponding={respondingRequestIds.includes(activePrompt.requestId)} answers={answers} questionIndex={questionIndex} - onSelectOption={onSelectOption} + onToggleOption={onToggleOption} onAdvance={onAdvance} /> ); @@ -47,14 +47,14 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( isResponding, answers, questionIndex, - onSelectOption, + onToggleOption, onAdvance, }: { prompt: PendingUserInput; isResponding: boolean; answers: Record; questionIndex: number; - onSelectOption: (questionId: string, optionLabel: string) => void; + onToggleOption: (questionId: string, optionLabel: string) => void; onAdvance: () => void; }) { const progress = derivePendingUserInputProgress(prompt.questions, answers, questionIndex); @@ -70,9 +70,12 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( }; }, []); - const selectOptionAndAutoAdvance = useCallback( + const handleOptionSelection = useCallback( (questionId: string, optionLabel: string) => { - onSelectOption(questionId, optionLabel); + onToggleOption(questionId, optionLabel); + if (activeQuestion?.multiSelect) { + return; + } if (autoAdvanceTimerRef.current !== null) { window.clearTimeout(autoAdvanceTimerRef.current); } @@ -81,13 +84,12 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( onAdvance(); }, 200); }, - [onSelectOption, onAdvance], + [activeQuestion?.multiSelect, onAdvance, onToggleOption], ); - // Keyboard shortcut: number keys 1-9 select corresponding option and auto-advance. - // Works even when the Lexical composer (contenteditable) has focus — the composer - // doubles as a custom-answer field during user input, and when it's empty the digit - // keys should pick options instead of typing into the editor. + // Keyboard shortcut: number keys 1-9 select corresponding options when focus is + // outside editable fields. Multi-select prompts toggle options in place; single- + // select prompts keep the existing auto-advance behavior. useEffect(() => { if (!activeQuestion || isResponding) return; const handler = (event: globalThis.KeyboardEvent) => { @@ -96,11 +98,8 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( if (target instanceof HTMLInputElement || target instanceof HTMLTextAreaElement) { return; } - // If the user has started typing a custom answer in the contenteditable - // composer, let digit keys pass through so they can type numbers. if (target instanceof HTMLElement && target.isContentEditable) { - const hasCustomText = progress.customAnswer.length > 0; - if (hasCustomText) return; + return; } const digit = Number.parseInt(event.key, 10); if (Number.isNaN(digit) || digit < 1 || digit > 9) return; @@ -109,11 +108,11 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( const option = activeQuestion.options[optionIndex]; if (!option) return; event.preventDefault(); - selectOptionAndAutoAdvance(activeQuestion.id, option.label); + handleOptionSelection(activeQuestion.id, option.label); }; document.addEventListener("keydown", handler); return () => document.removeEventListener("keydown", handler); - }, [activeQuestion, isResponding, selectOptionAndAutoAdvance, progress.customAnswer.length]); + }, [activeQuestion, handleOptionSelection, isResponding]); if (!activeQuestion) { return null; @@ -134,16 +133,19 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard(

{activeQuestion.question}

+ {activeQuestion.multiSelect ? ( +

Select one or more options.

+ ) : null}
{activeQuestion.options.map((option, index) => { - const isSelected = progress.selectedOptionLabel === option.label; + const isSelected = progress.selectedOptionLabels.includes(option.label); const shortcutKey = index < 9 ? index + 1 : null; return (