diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index f5ea5bb1eba..dc9b0c5d1cc 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -88,12 +88,18 @@ import { type LegendListRef } from "@legendapp/list/react"; import { getAnchoredTurnMetrics, type TimelineScrollMode } from "./chat/timelineScrollAnchoring"; import { buildPendingUserInputAnswers, + findFirstUnansweredPendingUserInputQuestionIndex, derivePendingUserInputProgress, setPendingUserInputCustomAnswer, + setPendingUserInputSelectedOption, togglePendingUserInputOptionSelection, type PendingUserInputDraftAnswer, } from "../pendingUserInput"; import { useUiStateStore } from "../uiStateStore"; +import { + usePendingUserInputDraftStore, + usePendingUserInputThreadDraft, +} from "../pendingUserInputDraftStore"; import { buildPlanImplementationThreadTitle, buildPlanImplementationPrompt, @@ -256,6 +262,9 @@ const EMPTY_ACTIVITIES: OrchestrationThreadActivity[] = []; const EMPTY_PROVIDERS: ServerProvider[] = []; const EMPTY_PROVIDER_SKILLS: ServerProvider["skills"] = []; const EMPTY_PENDING_USER_INPUT_ANSWERS: Record = {}; +// ponytail: window during which we suppress restoring a just-cleared custom +// draft back into the composer, so clicking a preset doesn't echo stale text. +const PENDING_CUSTOM_RESTORE_SUPPRESSION_MS = 500; const PreviewPanel = lazy(() => import("./preview/PreviewPanel").then((module) => ({ default: module.PreviewPanel })), ); @@ -1091,6 +1100,14 @@ function ChatViewContent(props: ChatViewProps) { ? store.getDraftSession(draftId) : null, ); + const pendingUserInputDraftThread = usePendingUserInputThreadDraft(threadId); + const setPendingUserInputDraftQuestionIndex = usePendingUserInputDraftStore( + (store) => store.setQuestionIndex, + ); + const setPendingUserInputDraftAnswer = usePendingUserInputDraftStore((store) => store.setAnswer); + const clearInactivePendingUserInputDraftRequests = usePendingUserInputDraftStore( + (store) => store.clearInactiveRequests, + ); const promptRef = useRef(""); const composerImagesRef = useRef([]); const composerTerminalContextsRef = useRef([]); @@ -1117,11 +1134,7 @@ function ChatViewContent(props: ChatViewProps) { const [respondingUserInputRequestIds, setRespondingUserInputRequestIds] = useState< ApprovalRequestId[] >([]); - const [pendingUserInputAnswersByRequestId, setPendingUserInputAnswersByRequestId] = useState< - Record> - >({}); - const [pendingUserInputQuestionIndexByRequestId, setPendingUserInputQuestionIndexByRequestId] = - useState>({}); + const respondingUserInputRequestIdSetRef = useRef(new Set()); const shouldUsePlanSidebarSheet = useMediaQuery(RIGHT_PANEL_INLINE_LAYOUT_MEDIA_QUERY); // Tracks whether the user explicitly dismissed the sidebar for the active turn. const planSidebarDismissedForTurnRef = useRef(null); @@ -1735,17 +1748,21 @@ function ChatViewContent(props: ChatViewProps) { () => derivePendingUserInputs(threadActivities), [threadActivities], ); + const activePendingUserInputRequestIds = useMemo( + () => pendingUserInputs.map((pendingUserInput) => pendingUserInput.requestId), + [pendingUserInputs], + ); const activePendingUserInput = pendingUserInputs[0] ?? null; const activePendingDraftAnswers = useMemo( () => activePendingUserInput - ? (pendingUserInputAnswersByRequestId[activePendingUserInput.requestId] ?? + ? (pendingUserInputDraftThread.answersByRequestId[activePendingUserInput.requestId] ?? EMPTY_PENDING_USER_INPUT_ANSWERS) : EMPTY_PENDING_USER_INPUT_ANSWERS, - [activePendingUserInput, pendingUserInputAnswersByRequestId], + [activePendingUserInput, pendingUserInputDraftThread.answersByRequestId], ); const activePendingQuestionIndex = activePendingUserInput - ? (pendingUserInputQuestionIndexByRequestId[activePendingUserInput.requestId] ?? 0) + ? (pendingUserInputDraftThread.questionIndexByRequestId[activePendingUserInput.requestId] ?? 0) : 0; const activePendingProgress = useMemo( () => @@ -1818,6 +1835,111 @@ function ChatViewContent(props: ChatViewProps) { activeThread?.session ?? null, localDispatchStartedAt, ); + useEffect(() => { + if (!activeThread) { + return; + } + if (threadActivities === EMPTY_ACTIVITIES && activePendingUserInputRequestIds.length === 0) { + return; + } + clearInactivePendingUserInputDraftRequests(threadId, activePendingUserInputRequestIds); + }, [ + activePendingUserInputRequestIds, + activeThread, + clearInactivePendingUserInputDraftRequests, + threadActivities, + threadId, + ]); + const lastSyncedPendingInputRef = useRef<{ + requestId: string | null; + questionId: string | null; + } | null>(null); + const pendingAnswerWriteRef = useRef<{ + requestId: string | null; + questionId: string | null; + source: "option" | "custom" | null; + }>({ + requestId: null, + questionId: null, + source: null, + }); + const suppressedPendingCustomRestoreRef = useRef<{ + requestId: string | null; + questionId: string | null; + value: string | null; + expiresAtMs: number; + }>({ + requestId: null, + questionId: null, + value: null, + expiresAtMs: 0, + }); + useEffect(() => { + const nextCustomAnswer = activePendingProgress?.customAnswer; + if (typeof nextCustomAnswer !== "string") { + lastSyncedPendingInputRef.current = null; + pendingAnswerWriteRef.current = { + requestId: null, + questionId: null, + source: null, + }; + suppressedPendingCustomRestoreRef.current = { + requestId: null, + questionId: null, + value: null, + expiresAtMs: 0, + }; + return; + } + const nextRequestId = activePendingUserInput?.requestId ?? null; + const nextQuestionId = activePendingProgress?.activeQuestion?.id ?? null; + const questionChanged = + lastSyncedPendingInputRef.current?.requestId !== nextRequestId || + lastSyncedPendingInputRef.current?.questionId !== nextQuestionId; + const textChangedExternally = promptRef.current !== nextCustomAnswer; + + lastSyncedPendingInputRef.current = { + requestId: nextRequestId, + questionId: nextQuestionId, + }; + if ( + pendingAnswerWriteRef.current.requestId !== nextRequestId || + pendingAnswerWriteRef.current.questionId !== nextQuestionId + ) { + pendingAnswerWriteRef.current = { + requestId: nextRequestId, + questionId: nextQuestionId, + source: activePendingProgress?.activeDraft?.answerSource ?? null, + }; + } + + if (!questionChanged && !textChangedExternally) { + return; + } + if ( + !questionChanged && + nextCustomAnswer.length === 0 && + promptRef.current.length > 0 && + promptRef.current.trim().length === 0 + ) { + return; + } + + promptRef.current = nextCustomAnswer; + const nextCursor = questionChanged + ? collapseExpandedComposerCursor(nextCustomAnswer, nextCustomAnswer.length) + : (composerRef.current?.readSnapshot().cursor ?? nextCustomAnswer.length); + composerRef.current?.resetCursorState({ + cursor: nextCursor, + prompt: nextCustomAnswer, + detectTrigger: true, + }); + }, [ + activePendingProgress?.activeDraft?.answerSource, + activePendingProgress?.customAnswer, + activePendingUserInput?.requestId, + activePendingProgress?.activeQuestion?.id, + ]); useEffect(() => { attachmentPreviewHandoffByMessageIdRef.current = attachmentPreviewHandoffByMessageId; }, [attachmentPreviewHandoffByMessageId]); @@ -4283,28 +4405,33 @@ function ChatViewContent(props: ChatViewProps) { const onRespondToUserInput = useCallback( async (requestId: ApprovalRequestId, answers: Record) => { - if (!activeThreadId) return; + if (!activeThreadId || respondingUserInputRequestIdSetRef.current.has(requestId)) return; + respondingUserInputRequestIdSetRef.current.add(requestId); setRespondingUserInputRequestIds((existing) => existing.includes(requestId) ? existing : [...existing, requestId], ); - const result = await respondToThreadUserInput({ - environmentId, - input: { - threadId: activeThreadId, - requestId, - answers, - }, - }); - if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { - const error = squashAtomCommandFailure(result); - setThreadError( - activeThreadId, - error instanceof Error ? error.message : "Failed to submit user input.", - ); + try { + const result = await respondToThreadUserInput({ + environmentId, + input: { + threadId: activeThreadId, + requestId, + answers, + }, + }); + if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { + const error = squashAtomCommandFailure(result); + setThreadError( + activeThreadId, + error instanceof Error ? error.message : "Failed to submit user input.", + ); + } + return result; + } finally { + respondingUserInputRequestIdSetRef.current.delete(requestId); + setRespondingUserInputRequestIds((existing) => existing.filter((id) => id !== requestId)); } - setRespondingUserInputRequestIds((existing) => existing.filter((id) => id !== requestId)); - return result; }, [activeThreadId, environmentId, respondToThreadUserInput, setThreadError], ); @@ -4314,79 +4441,162 @@ function ChatViewContent(props: ChatViewProps) { if (!activePendingUserInput) { return; } - setPendingUserInputQuestionIndexByRequestId((existing) => ({ - ...existing, - [activePendingUserInput.requestId]: nextQuestionIndex, - })); + setPendingUserInputDraftQuestionIndex( + threadId, + activePendingUserInput.requestId, + nextQuestionIndex, + ); }, - [activePendingUserInput], + [activePendingUserInput, setPendingUserInputDraftQuestionIndex, threadId], ); const onSelectActivePendingUserInputOption = useCallback( - (questionId: string, optionLabel: string) => { - if (!activePendingUserInput) { + ( + questionId: string, + optionLabel: string, + options?: { + advanceToNextQuestion?: boolean; + submitIfComplete?: boolean; + }, + ) => { + if (!activePendingUserInput || activePendingIsResponding) { return; } - setPendingUserInputAnswersByRequestId((existing) => { - const question = - (activePendingProgress?.activeQuestion?.id === questionId - ? activePendingProgress.activeQuestion - : undefined) ?? - activePendingUserInput.questions.find((entry) => entry.id === questionId); - if (!question) { - return existing; - } - - return { - ...existing, - [activePendingUserInput.requestId]: { - ...existing[activePendingUserInput.requestId], - [questionId]: togglePendingUserInputOptionSelection( - question, - existing[activePendingUserInput.requestId]?.[questionId], - optionLabel, - ), - }, - }; - }); + const question = + activePendingProgress?.activeQuestion?.id === questionId + ? activePendingProgress.activeQuestion + : activePendingUserInput.questions.find((entry) => entry.id === questionId); + if (!question) { + return; + } + const previousDraft = activePendingDraftAnswers[questionId]; + const isMultiSelect = question.multiSelect === true; + // ponytail: option source is authoritative so a stale custom draft cannot + // override a clicked preset at submit (#918/#528 race). + const nextDraftAnswer = isMultiSelect + ? togglePendingUserInputOptionSelection(question, previousDraft, optionLabel) + : setPendingUserInputSelectedOption(previousDraft, optionLabel); + pendingAnswerWriteRef.current = { + requestId: activePendingUserInput.requestId, + questionId, + source: "option", + }; + suppressedPendingCustomRestoreRef.current = { + requestId: activePendingUserInput.requestId, + questionId, + value: + typeof previousDraft?.customAnswer === "string" && previousDraft.customAnswer.length > 0 + ? previousDraft.customAnswer + : null, + expiresAtMs: performance.now() + PENDING_CUSTOM_RESTORE_SUPPRESSION_MS, + }; + setPendingUserInputDraftAnswer( + threadId, + activePendingUserInput.requestId, + questionId, + nextDraftAnswer, + ); promptRef.current = ""; - composerRef.current?.resetCursorState({ cursor: 0 }); + composerRef.current?.resetCursorState({ cursor: 0, prompt: "" }); + // Multi-select toggles in place; single-select advances or submits. + if (isMultiSelect) { + return; + } + if (options?.submitIfComplete) { + const nextResolvedAnswers = buildPendingUserInputAnswers(activePendingUserInput.questions, { + ...activePendingDraftAnswers, + [questionId]: nextDraftAnswer, + }); + if (nextResolvedAnswers) { + void onRespondToUserInput(activePendingUserInput.requestId, nextResolvedAnswers); + return; + } + setPendingUserInputDraftQuestionIndex( + threadId, + activePendingUserInput.requestId, + findFirstUnansweredPendingUserInputQuestionIndex(activePendingUserInput.questions, { + ...activePendingDraftAnswers, + [questionId]: nextDraftAnswer, + }), + ); + return; + } + if ( + options?.advanceToNextQuestion && + activePendingProgress && + !activePendingProgress.isLastQuestion + ) { + setPendingUserInputDraftQuestionIndex( + threadId, + activePendingUserInput.requestId, + activePendingProgress.questionIndex + 1, + ); + } }, - [activePendingProgress?.activeQuestion, activePendingUserInput, composerRef], + [ + activePendingDraftAnswers, + activePendingProgress, + activePendingUserInput, + activePendingIsResponding, + onRespondToUserInput, + setPendingUserInputDraftAnswer, + setPendingUserInputDraftQuestionIndex, + threadId, + ], ); const onChangeActivePendingUserInputCustomAnswer = useCallback( ( questionId: string, value: string, - nextCursor: number, - expandedCursor: number, + _nextCursor: number, + _expandedCursor: number, _cursorAdjacentToMention: boolean, ) => { if (!activePendingUserInput) { return; } - promptRef.current = value; - setPendingUserInputAnswersByRequestId((existing) => ({ - ...existing, - [activePendingUserInput.requestId]: { - ...existing[activePendingUserInput.requestId], - [questionId]: setPendingUserInputCustomAnswer( - existing[activePendingUserInput.requestId]?.[questionId], - value, - ), - }, - })); - const snapshot = composerRef.current?.readSnapshot(); + const suppressedRestore = suppressedPendingCustomRestoreRef.current; if ( - snapshot?.value !== value || - snapshot.cursor !== nextCursor || - snapshot.expandedCursor !== expandedCursor + suppressedRestore.requestId === activePendingUserInput.requestId && + suppressedRestore.questionId === questionId && + suppressedRestore.value !== null && + suppressedRestore.value === value && + suppressedRestore.expiresAtMs >= performance.now() ) { - composerRef.current?.focusAt(nextCursor); + return; } + suppressedPendingCustomRestoreRef.current = { + requestId: null, + questionId: null, + value: null, + expiresAtMs: 0, + }; + if ( + pendingAnswerWriteRef.current.requestId === activePendingUserInput.requestId && + pendingAnswerWriteRef.current.questionId === questionId && + pendingAnswerWriteRef.current.source === "option" && + value === activePendingDraftAnswers[questionId]?.customAnswer + ) { + return; + } + pendingAnswerWriteRef.current = { + requestId: activePendingUserInput.requestId, + questionId, + source: + value.trim().length > 0 + ? "custom" + : (activePendingDraftAnswers[questionId]?.answerSource ?? null), + }; + promptRef.current = value; + setPendingUserInputDraftAnswer( + threadId, + activePendingUserInput.requestId, + questionId, + setPendingUserInputCustomAnswer(activePendingDraftAnswers[questionId], value), + ); }, - [activePendingUserInput, composerRef], + [activePendingDraftAnswers, activePendingUserInput, setPendingUserInputDraftAnswer, threadId], ); const onAdvanceActivePendingUserInput = useCallback(() => { @@ -4394,13 +4604,25 @@ function ChatViewContent(props: ChatViewProps) { return; } if (activePendingProgress.isLastQuestion) { + if (activePendingIsResponding) { + return; + } if (activePendingResolvedAnswers) { void onRespondToUserInput(activePendingUserInput.requestId, activePendingResolvedAnswers); + return; } + setActivePendingUserInputQuestionIndex( + findFirstUnansweredPendingUserInputQuestionIndex( + activePendingUserInput.questions, + activePendingDraftAnswers, + ), + ); return; } setActivePendingUserInputQuestionIndex(activePendingProgress.questionIndex + 1); }, [ + activePendingDraftAnswers, + activePendingIsResponding, activePendingProgress, activePendingResolvedAnswers, activePendingUserInput, @@ -5194,7 +5416,6 @@ function ChatViewContent(props: ChatViewProps) { onImplementPlanInNewThread={onImplementPlanInNewThread} onRespondToApproval={onRespondToApproval} onSelectActivePendingUserInputOption={onSelectActivePendingUserInputOption} - onAdvanceActivePendingUserInput={onAdvanceActivePendingUserInput} onPreviousActivePendingUserInputQuestion={ onPreviousActivePendingUserInputQuestion } diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index 5f9aec837ca..8f621e3c926 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -512,8 +512,14 @@ export interface ChatComposerProps { requestId: ApprovalRequestId, decision: ProviderApprovalDecision, ) => Promise; - onSelectActivePendingUserInputOption: (questionId: string, optionLabel: string) => void; - onAdvanceActivePendingUserInput: () => void; + onSelectActivePendingUserInputOption: ( + questionId: string, + optionLabel: string, + options?: { + advanceToNextQuestion?: boolean; + submitIfComplete?: boolean; + }, + ) => void; onPreviousActivePendingUserInputQuestion: () => void; onChangeActivePendingUserInputCustomAnswer: ( questionId: string, @@ -594,7 +600,6 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) onImplementPlanInNewThread, onRespondToApproval, onSelectActivePendingUserInputOption, - onAdvanceActivePendingUserInput, onPreviousActivePendingUserInputQuestion, onChangeActivePendingUserInputCustomAnswer, onProviderModelSelect, @@ -2097,11 +2102,10 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps)
) : showPlanFollowUpPrompt && activeProposedPlan ? ( @@ -2137,11 +2141,10 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) >
; questionIndex: number; - onToggleOption: (questionId: string, optionLabel: string) => void; - onAdvance: () => void; + onSelectOption: ( + questionId: string, + optionLabel: string, + options?: { + advanceToNextQuestion?: boolean; + submitIfComplete?: boolean; + }, + ) => void; } export const ComposerPendingUserInputPanel = memo(function ComposerPendingUserInputPanel({ pendingUserInputs, - respondingRequestIds, + isResponding, answers, questionIndex, - onToggleOption, - onAdvance, + onSelectOption, }: PendingUserInputPanelProps) { if (pendingUserInputs.length === 0) return null; const activePrompt = pendingUserInputs[0]; @@ -33,11 +37,10 @@ export const ComposerPendingUserInputPanel = memo(function ComposerPendingUserIn ); }); @@ -47,29 +50,28 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( isResponding, answers, questionIndex, - onToggleOption, - onAdvance, + onSelectOption, }: { prompt: PendingUserInput; isResponding: boolean; answers: Record; questionIndex: number; - onToggleOption: (questionId: string, optionLabel: string) => void; - onAdvance: () => void; + onSelectOption: ( + questionId: string, + optionLabel: string, + options?: { + advanceToNextQuestion?: boolean; + submitIfComplete?: boolean; + }, + ) => void; }) { const progress = derivePendingUserInputProgress(prompt.questions, answers, questionIndex); const activeQuestion = progress.activeQuestion; - const autoAdvanceTimerRef = useRef(null); - const onAdvanceRef = useRef(onAdvance); const [optimisticSingleSelect, setOptimisticSingleSelect] = useState<{ questionId: string; optionLabel: string; } | null>(null); - useEffect(() => { - onAdvanceRef.current = onAdvance; - }, [onAdvance]); - useEffect(() => { if (!activeQuestion || activeQuestion.multiSelect || !optimisticSingleSelect) { return; @@ -79,7 +81,7 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( return; } if ( - progress.customAnswer.trim().length === 0 && + !progress.usingCustomAnswer && progress.selectedOptionLabels.includes(optimisticSingleSelect.optionLabel) ) { setOptimisticSingleSelect(null); @@ -87,33 +89,24 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( }, [ activeQuestion, optimisticSingleSelect, - progress.customAnswer, + progress.usingCustomAnswer, progress.selectedOptionLabels, ]); - // Clear auto-advance timer on unmount - useEffect(() => { - return () => { - if (autoAdvanceTimerRef.current !== null) { - window.clearTimeout(autoAdvanceTimerRef.current); - } - }; - }, []); - + // ponytail: deterministic auto-advance/submit replaces the prior timeout race. + // Selecting a preset immediately advances (or submits on the last question) + // instead of waiting on a timer that could fire after stale custom edits. const handleOptionSelection = useEffectEvent((questionId: string, optionLabel: string) => { if (activeQuestion?.multiSelect) { - onToggleOption(questionId, optionLabel); + onSelectOption(questionId, optionLabel); return; } setOptimisticSingleSelect({ questionId, optionLabel }); - onToggleOption(questionId, optionLabel); - if (autoAdvanceTimerRef.current !== null) { - window.clearTimeout(autoAdvanceTimerRef.current); - } - autoAdvanceTimerRef.current = window.setTimeout(() => { - autoAdvanceTimerRef.current = null; - onAdvanceRef.current(); - }, 200); + onSelectOption( + questionId, + optionLabel, + progress.isLastQuestion ? { submitIfComplete: true } : { advanceToNextQuestion: true }, + ); }); // Keyboard shortcut: number keys 1-9 select corresponding options when focus is @@ -150,7 +143,7 @@ const ComposerPendingUserInputCard = memo(function ComposerPendingUserInputCard( return null; } - const customAnswerActive = progress.customAnswer.trim().length > 0; + const customAnswerActive = progress.usingCustomAnswer; return (
diff --git a/apps/web/src/pendingUserInput.test.ts b/apps/web/src/pendingUserInput.test.ts index 3d1cb336e1f..5edd0f88106 100644 --- a/apps/web/src/pendingUserInput.test.ts +++ b/apps/web/src/pendingUserInput.test.ts @@ -7,6 +7,7 @@ import { findFirstUnansweredPendingUserInputQuestionIndex, resolvePendingUserInputAnswer, setPendingUserInputCustomAnswer, + setPendingUserInputSelectedOption, togglePendingUserInputOptionSelection, } from "./pendingUserInput"; @@ -41,7 +42,27 @@ const multiSelectQuestion = { } as const; describe("resolvePendingUserInputAnswer", () => { - it("prefers a custom answer over selected options", () => { + it("resolves a custom answer when the explicit source is custom", () => { + expect( + resolvePendingUserInputAnswer(singleSelectQuestion, { + answerSource: "custom", + selectedOptionLabels: ["Keep current envelope"], + customAnswer: "Keep the existing envelope for one release", + }), + ).toBe("Keep the existing envelope for one release"); + }); + + it("resolves a selected option when the explicit source is option", () => { + expect( + resolvePendingUserInputAnswer(singleSelectQuestion, { + answerSource: "option", + selectedOptionLabels: ["Scaffold only"], + customAnswer: "Old custom answer", + }), + ).toBe("Scaffold only"); + }); + + it("prefers a custom answer over a selected option for legacy drafts", () => { expect( resolvePendingUserInputAnswer(singleSelectQuestion, { selectedOptionLabels: ["Orchestration-first"], @@ -70,20 +91,55 @@ describe("resolvePendingUserInputAnswer", () => { expect( setPendingUserInputCustomAnswer( { - selectedOptionLabels: ["Server", "Web"], + answerSource: "option", + selectedOptionLabels: ["Preserve existing tags"], }, "doesn't matter", ), ).toEqual({ + answerSource: "custom", customAnswer: "doesn't matter", }); }); + + it("keeps the selected option active when custom text is cleared", () => { + expect( + setPendingUserInputCustomAnswer( + { + answerSource: "option", + selectedOptionLabels: ["Preserve existing tags"], + }, + "", + ), + ).toEqual({ + answerSource: "option", + selectedOptionLabels: ["Preserve existing tags"], + customAnswer: "", + }); + }); + + it("sets the selected option as the explicit answer source", () => { + expect( + setPendingUserInputSelectedOption( + { + answerSource: "custom", + customAnswer: "Keep the old custom answer", + }, + "Preserve existing tags", + ), + ).toEqual({ + answerSource: "option", + selectedOptionLabels: ["Preserve existing tags"], + customAnswer: "", + }); + }); }); describe("togglePendingUserInputOptionSelection", () => { it("toggles options for multi-select questions", () => { expect(togglePendingUserInputOptionSelection(multiSelectQuestion, undefined, "Server")).toEqual( { + answerSource: "option", customAnswer: "", selectedOptionLabels: ["Server"], }, @@ -98,6 +154,7 @@ describe("togglePendingUserInputOptionSelection", () => { "Server", ), ).toEqual({ + answerSource: "option", customAnswer: "", selectedOptionLabels: ["Web"], }); @@ -125,9 +182,11 @@ describe("buildPendingUserInputAnswers", () => { ], { scope: { + answerSource: "option", selectedOptionLabels: ["Orchestration-first"], }, compat: { + answerSource: "custom", customAnswer: "Keep the current envelope for one release window", }, }, @@ -176,6 +235,7 @@ describe("pending user input question progress", () => { expect( countAnsweredPendingUserInputQuestions(questions, { scope: { + answerSource: "option", selectedOptionLabels: ["Orchestration-first"], }, }), @@ -186,6 +246,7 @@ describe("pending user input question progress", () => { expect( findFirstUnansweredPendingUserInputQuestionIndex(questions, { scope: { + answerSource: "option", selectedOptionLabels: ["Orchestration-first"], }, }), @@ -196,9 +257,11 @@ describe("pending user input question progress", () => { expect( findFirstUnansweredPendingUserInputQuestionIndex(questions, { scope: { + answerSource: "option", selectedOptionLabels: ["Orchestration-first"], }, compat: { + answerSource: "custom", customAnswer: "Keep it for one release window", }, }), @@ -211,6 +274,7 @@ describe("pending user input question progress", () => { questions, { scope: { + answerSource: "option", selectedOptionLabels: ["Orchestration-first"], }, }, @@ -222,6 +286,7 @@ describe("pending user input question progress", () => { selectedOptionLabels: ["Orchestration-first"], customAnswer: "", resolvedAnswer: "Orchestration-first", + usingCustomAnswer: false, answeredQuestionCount: 1, isLastQuestion: false, isComplete: false, @@ -247,4 +312,25 @@ describe("pending user input question progress", () => { isComplete: true, }); }); + + it("marks preset selections as active even if a stale custom value exists", () => { + expect( + derivePendingUserInputProgress( + questions, + { + scope: { + answerSource: "option", + selectedOptionLabels: ["Orchestration-first"], + customAnswer: "stale custom answer", + }, + }, + 0, + ), + ).toMatchObject({ + selectedOptionLabels: ["Orchestration-first"], + customAnswer: "stale custom answer", + resolvedAnswer: "Orchestration-first", + usingCustomAnswer: false, + }); + }); }); diff --git a/apps/web/src/pendingUserInput.ts b/apps/web/src/pendingUserInput.ts index d3a7a129378..31af4b663dd 100644 --- a/apps/web/src/pendingUserInput.ts +++ b/apps/web/src/pendingUserInput.ts @@ -1,6 +1,13 @@ import type { UserInputQuestion } from "@t3tools/contracts"; +// ponytail: answerSource makes the active input source authoritative so a +// clicked preset cannot be overwritten by stale custom editor state at submit +// (the #918/#528 preset/custom race). Kept as a discriminator over upstream's +// existing selectedOptionLabels[] shape to preserve multiSelect. +export type PendingUserInputAnswerSource = "option" | "custom"; + export interface PendingUserInputDraftAnswer { + answerSource?: PendingUserInputAnswerSource; selectedOptionLabels?: string[]; customAnswer?: string; } @@ -50,11 +57,22 @@ export function resolvePendingUserInputAnswer( draft: PendingUserInputDraftAnswer | undefined, ): string | string[] | null { const customAnswer = normalizeDraftAnswer(draft?.customAnswer); - if (customAnswer) { + const selectedOptionLabels = normalizeSelectedOptionLabels(draft?.selectedOptionLabels); + + if (draft?.answerSource === "custom") { return customAnswer; } + if (draft?.answerSource === "option") { + if (question.multiSelect) { + return selectedOptionLabels.length > 0 ? selectedOptionLabels : null; + } + return selectedOptionLabels[0] ?? null; + } - const selectedOptionLabels = normalizeSelectedOptionLabels(draft?.selectedOptionLabels); + // No explicit source — legacy fallback (custom takes precedence). + if (customAnswer) { + return customAnswer; + } if (question.multiSelect) { return selectedOptionLabels.length > 0 ? selectedOptionLabels : null; } @@ -66,14 +84,23 @@ export function setPendingUserInputCustomAnswer( draft: PendingUserInputDraftAnswer | undefined, customAnswer: string, ): PendingUserInputDraftAnswer { - const selectedOptionLabels = - customAnswer.trim().length > 0 - ? undefined - : normalizeSelectedOptionLabels(draft?.selectedOptionLabels); + const normalizedCustomAnswer = normalizeDraftAnswer(customAnswer); + // ponytail: collapse whitespace-only drafts to "" so we never persist pure- + // whitespace custom text that resolves to no answer but clutters the store. + const storedCustomAnswer = normalizedCustomAnswer ? customAnswer : ""; + const selectedOptionLabels = normalizedCustomAnswer + ? [] + : normalizeSelectedOptionLabels(draft?.selectedOptionLabels); + const answerSource: PendingUserInputAnswerSource | undefined = normalizedCustomAnswer + ? "custom" + : selectedOptionLabels.length > 0 + ? "option" + : undefined; return { - customAnswer, - ...(selectedOptionLabels && selectedOptionLabels.length > 0 ? { selectedOptionLabels } : {}), + ...(answerSource ? { answerSource } : {}), + customAnswer: storedCustomAnswer, + ...(selectedOptionLabels.length > 0 ? { selectedOptionLabels } : {}), }; } @@ -89,19 +116,31 @@ export function togglePendingUserInputOptionSelection( : [...selectedOptionLabels, optionLabel]; return { - customAnswer: "", ...(nextSelectedOptionLabels.length > 0 - ? { selectedOptionLabels: nextSelectedOptionLabels } + ? { answerSource: "option" as const, selectedOptionLabels: nextSelectedOptionLabels } : {}), + customAnswer: "", }; } return { + answerSource: "option", customAnswer: "", selectedOptionLabels: [optionLabel], }; } +export function setPendingUserInputSelectedOption( + _draft: PendingUserInputDraftAnswer | undefined, + selectedOptionLabel: string, +): PendingUserInputDraftAnswer { + return { + answerSource: "option", + selectedOptionLabels: [selectedOptionLabel], + customAnswer: "", + }; +} + export function buildPendingUserInputAnswers( questions: ReadonlyArray, draftAnswers: Record, @@ -152,6 +191,13 @@ export function derivePendingUserInputProgress( ? resolvePendingUserInputAnswer(activeQuestion, activeDraft) : null; const customAnswer = activeDraft?.customAnswer ?? ""; + const normalizedCustomAnswer = normalizeDraftAnswer(customAnswer); + const usingCustomAnswer = + activeDraft?.answerSource === "custom" + ? normalizedCustomAnswer !== null + : activeDraft?.answerSource === "option" + ? false + : normalizedCustomAnswer !== null; const answeredQuestionCount = countAnsweredPendingUserInputQuestions(questions, draftAnswers); const isLastQuestion = questions.length === 0 ? true : normalizedQuestionIndex >= questions.length - 1; @@ -163,7 +209,7 @@ export function derivePendingUserInputProgress( selectedOptionLabels: normalizeSelectedOptionLabels(activeDraft?.selectedOptionLabels), customAnswer, resolvedAnswer, - usingCustomAnswer: customAnswer.trim().length > 0, + usingCustomAnswer, answeredQuestionCount, isLastQuestion, isComplete: buildPendingUserInputAnswers(questions, draftAnswers) !== null, diff --git a/apps/web/src/pendingUserInputDraftStore.ts b/apps/web/src/pendingUserInputDraftStore.ts new file mode 100644 index 00000000000..b4c332f054c --- /dev/null +++ b/apps/web/src/pendingUserInputDraftStore.ts @@ -0,0 +1,338 @@ +import type { ApprovalRequestId, ThreadId } from "@t3tools/contracts"; +import { create } from "zustand"; +import { createJSONStorage, persist } from "zustand/middleware"; + +import { createDebouncedStorage, type DebouncedStorage } from "./lib/storage"; +import type { PendingUserInputDraftAnswer } from "./pendingUserInput"; + +export const PENDING_USER_INPUT_DRAFT_STORAGE_KEY = "t3code:pending-user-input-drafts:v1"; + +const noopPendingUserInputStorage: DebouncedStorage = { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + flush: () => {}, +}; + +function createPendingUserInputStorage(): DebouncedStorage { + if (typeof window === "undefined") { + return noopPendingUserInputStorage; + } + try { + return createDebouncedStorage(window.localStorage); + } catch { + return noopPendingUserInputStorage; + } +} + +const pendingUserInputDebouncedStorage = createPendingUserInputStorage(); + +if (typeof window !== "undefined") { + window.addEventListener("beforeunload", () => { + pendingUserInputDebouncedStorage.flush(); + }); +} + +interface PendingUserInputThreadDraftState { + answersByRequestId: Record>; + questionIndexByRequestId: Record; +} + +interface PendingUserInputDraftStoreState { + draftsByThreadId: Record; + setQuestionIndex: ( + threadId: ThreadId, + requestId: ApprovalRequestId, + questionIndex: number, + ) => void; + setAnswer: ( + threadId: ThreadId, + requestId: ApprovalRequestId, + questionId: string, + answer: PendingUserInputDraftAnswer, + ) => void; + clearInactiveRequests: ( + threadId: ThreadId, + activeRequestIds: ReadonlyArray, + ) => void; +} + +const EMPTY_PENDING_USER_INPUT_THREAD_DRAFT = Object.freeze({ + answersByRequestId: {}, + questionIndexByRequestId: {}, +}) as PendingUserInputThreadDraftState; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function normalizePersistedDraftAnswer(value: unknown): PendingUserInputDraftAnswer | null { + if (!isRecord(value)) { + return null; + } + const answerSource = + value.answerSource === "option" || value.answerSource === "custom" + ? value.answerSource + : undefined; + const selectedOptionLabels = Array.isArray(value.selectedOptionLabels) + ? Array.from( + new Set( + value.selectedOptionLabels + .filter((label): label is string => typeof label === "string") + .map((label) => label.trim()) + .filter((label) => label.length > 0), + ), + ) + : []; + const customAnswer = typeof value.customAnswer === "string" ? value.customAnswer : undefined; + const normalized: PendingUserInputDraftAnswer = { + ...(answerSource ? { answerSource } : {}), + ...(answerSource !== "option" && customAnswer !== undefined ? { customAnswer } : {}), + ...(answerSource !== "custom" && selectedOptionLabels.length > 0 + ? { selectedOptionLabels } + : {}), + }; + return Object.keys(normalized).length > 0 ? normalized : null; +} + +function normalizeAnswersByRequestId( + value: unknown, +): PendingUserInputThreadDraftState["answersByRequestId"] { + if (!isRecord(value)) { + return {}; + } + const answersByRequestId: PendingUserInputThreadDraftState["answersByRequestId"] = {}; + for (const [requestId, answersByQuestionId] of Object.entries(value)) { + if (!isRecord(answersByQuestionId)) { + continue; + } + const normalizedAnswers: Record = {}; + for (const [questionId, answer] of Object.entries(answersByQuestionId)) { + const normalizedAnswer = normalizePersistedDraftAnswer(answer); + if (normalizedAnswer) { + normalizedAnswers[questionId] = normalizedAnswer; + } + } + if (Object.keys(normalizedAnswers).length > 0) { + answersByRequestId[requestId as ApprovalRequestId] = normalizedAnswers; + } + } + return answersByRequestId; +} + +function normalizeThreadDraft(value: unknown): PendingUserInputThreadDraftState { + if (!isRecord(value)) { + return EMPTY_PENDING_USER_INPUT_THREAD_DRAFT; + } + const answersByRequestId = normalizeAnswersByRequestId(value.answersByRequestId); + const questionIndexByRequestId = Object.fromEntries( + Object.entries(isRecord(value.questionIndexByRequestId) ? value.questionIndexByRequestId : {}) + .filter( + ([, questionIndex]) => typeof questionIndex === "number" && Number.isFinite(questionIndex), + ) + .map(([requestId, questionIndex]) => { + const safeQuestionIndex = questionIndex as number; + return [requestId, Math.max(0, Math.floor(safeQuestionIndex))]; + }), + ) as PendingUserInputThreadDraftState["questionIndexByRequestId"]; + return { answersByRequestId, questionIndexByRequestId }; +} + +function mergeThreadDrafts( + persistedDraft: PendingUserInputThreadDraftState | undefined, + currentDraft: PendingUserInputThreadDraftState | undefined, +): PendingUserInputThreadDraftState { + return { + answersByRequestId: { + ...persistedDraft?.answersByRequestId, + ...currentDraft?.answersByRequestId, + }, + questionIndexByRequestId: { + ...persistedDraft?.questionIndexByRequestId, + ...currentDraft?.questionIndexByRequestId, + }, + }; +} + +function mergePersistedPendingUserInputDrafts( + persistedState: unknown, + currentState: PendingUserInputDraftStoreState, +): PendingUserInputDraftStoreState { + if (!isRecord(persistedState) || !isRecord(persistedState.draftsByThreadId)) { + return currentState; + } + const persistedDraftsByThreadId = Object.fromEntries( + Object.entries(persistedState.draftsByThreadId) + .map(([threadId, draft]) => [threadId, normalizeThreadDraft(draft)] as const) + .filter(([, draft]) => !shouldRemoveThreadDraft(draft)), + ) as Record; + const threadIds = new Set([ + ...Object.keys(persistedDraftsByThreadId), + ...Object.keys(currentState.draftsByThreadId), + ]); + const draftsByThreadId = Object.fromEntries( + Array.from(threadIds) + .map( + (threadId) => + [ + threadId, + mergeThreadDrafts( + persistedDraftsByThreadId[threadId as ThreadId], + currentState.draftsByThreadId[threadId as ThreadId], + ), + ] as const, + ) + .filter(([, draft]) => !shouldRemoveThreadDraft(draft)), + ) as Record; + return { ...currentState, draftsByThreadId }; +} + +function shouldRemoveThreadDraft(draft: PendingUserInputThreadDraftState | undefined): boolean { + if (!draft) { + return true; + } + return ( + Object.keys(draft.answersByRequestId).length === 0 && + Object.keys(draft.questionIndexByRequestId).length === 0 + ); +} + +export const usePendingUserInputDraftStore = create()( + persist( + (set) => ({ + draftsByThreadId: {}, + setQuestionIndex: (threadId, requestId, questionIndex) => { + if (threadId.length === 0 || requestId.length === 0) { + return; + } + set((state) => { + const threadDraft = + state.draftsByThreadId[threadId] ?? EMPTY_PENDING_USER_INPUT_THREAD_DRAFT; + const nextQuestionIndex = Math.max(0, Math.floor(questionIndex)); + if (threadDraft.questionIndexByRequestId[requestId] === nextQuestionIndex) { + return state; + } + return { + draftsByThreadId: { + ...state.draftsByThreadId, + [threadId]: { + answersByRequestId: threadDraft.answersByRequestId, + questionIndexByRequestId: { + ...threadDraft.questionIndexByRequestId, + [requestId]: nextQuestionIndex, + }, + }, + }, + }; + }); + }, + setAnswer: (threadId, requestId, questionId, answer) => { + if (threadId.length === 0 || requestId.length === 0 || questionId.length === 0) { + return; + } + set((state) => { + const threadDraft = + state.draftsByThreadId[threadId] ?? EMPTY_PENDING_USER_INPUT_THREAD_DRAFT; + const requestAnswers = threadDraft.answersByRequestId[requestId] ?? {}; + const currentAnswer = requestAnswers[questionId]; + const prevLabels = currentAnswer?.selectedOptionLabels; + const nextLabels = answer.selectedOptionLabels; + const labelsEqual = + prevLabels === nextLabels || + (prevLabels != null && + nextLabels != null && + prevLabels.length === nextLabels.length && + prevLabels.every((l, i) => l === nextLabels[i])); + if ( + currentAnswer?.answerSource === answer.answerSource && + currentAnswer?.customAnswer === answer.customAnswer && + labelsEqual + ) { + return state; + } + return { + draftsByThreadId: { + ...state.draftsByThreadId, + [threadId]: { + answersByRequestId: { + ...threadDraft.answersByRequestId, + [requestId]: { + ...requestAnswers, + [questionId]: answer, + }, + }, + questionIndexByRequestId: threadDraft.questionIndexByRequestId, + }, + }, + }; + }); + }, + clearInactiveRequests: (threadId, activeRequestIds) => { + if (threadId.length === 0) { + return; + } + set((state) => { + const threadDraft = state.draftsByThreadId[threadId]; + if (!threadDraft) { + return state; + } + const activeRequestIdSet = new Set(activeRequestIds); + let answersChanged = false; + const nextAnswersByRequestId = Object.fromEntries( + Object.entries(threadDraft.answersByRequestId).filter(([requestId]) => { + const keep = activeRequestIdSet.has(requestId as ApprovalRequestId); + answersChanged ||= !keep; + return keep; + }), + ) as PendingUserInputThreadDraftState["answersByRequestId"]; + let indexChanged = false; + const nextQuestionIndexByRequestId = Object.fromEntries( + Object.entries(threadDraft.questionIndexByRequestId).filter(([requestId]) => { + const keep = activeRequestIdSet.has(requestId as ApprovalRequestId); + indexChanged ||= !keep; + return keep; + }), + ) as PendingUserInputThreadDraftState["questionIndexByRequestId"]; + if (!answersChanged && !indexChanged) { + return state; + } + const nextThreadDraft: PendingUserInputThreadDraftState = { + answersByRequestId: nextAnswersByRequestId, + questionIndexByRequestId: nextQuestionIndexByRequestId, + }; + if (shouldRemoveThreadDraft(nextThreadDraft)) { + const { [threadId]: _removed, ...restDraftsByThreadId } = state.draftsByThreadId; + return { draftsByThreadId: restDraftsByThreadId }; + } + return { + draftsByThreadId: { + ...state.draftsByThreadId, + [threadId]: nextThreadDraft, + }, + }; + }); + }, + }), + { + name: PENDING_USER_INPUT_DRAFT_STORAGE_KEY, + version: 1, + storage: createJSONStorage(() => pendingUserInputDebouncedStorage), + partialize: (state) => ({ + draftsByThreadId: Object.fromEntries( + Object.entries(state.draftsByThreadId).filter( + ([, draft]) => !shouldRemoveThreadDraft(draft), + ), + ) as Record, + }), + merge: mergePersistedPendingUserInputDrafts, + }, + ), +); + +export function usePendingUserInputThreadDraft( + threadId: ThreadId, +): PendingUserInputThreadDraftState { + return usePendingUserInputDraftStore( + (state) => state.draftsByThreadId[threadId] ?? EMPTY_PENDING_USER_INPUT_THREAD_DRAFT, + ); +}