diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 5704355c9fc..99aad32b1ab 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -576,6 +576,10 @@ behavior for review cards. Its dismissal save and decisions are simulated; server persistence and permission boundaries have separate regression tests. The demo uses real thread/form components with fixture response callbacks. The Agent Chat stories share the same component fixture. +The **Send instead of confirming** and **Reopen skipped confirmation** stories +exercise sending a separate message while a completion review is open, then +opening that unchanged request from the feed. These use the production thread +with simulated message saves and decision callbacks. Use **Composer → New task** to review task creation through the production `TaskChatComposer`. The editor, file menu, work modes, assignee/model picker, diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index e607a41a137..da675e50752 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -1975,6 +1975,12 @@ Dismissal never creates an answer, decision, execution, or continuation wake. Raw runtime requests retain browser-local dismissal; reopening restores the original request and draft. A failed save leaves the request open with an error. +Sending a separate message closes the current composer request immediately, +before the send finishes. After a successful send, earlier requests stay in the +feed and can be reopened manually; a newer request can still open normally. +If sending fails, restore the previous form unless the person has opened a +different request. This presentation change does not answer or approve a request. + A saved ordinary question becomes historical after a newer human task message or explicit human dismissal. It remains answerable after completion without reopening work or waking the agent. Dismissed ordinary requests, including native diff --git a/ui/src/components/TaskChatThread.test.tsx b/ui/src/components/TaskChatThread.test.tsx index 2888e573599..468cd24e9a6 100644 --- a/ui/src/components/TaskChatThread.test.tsx +++ b/ui/src/components/TaskChatThread.test.tsx @@ -3950,6 +3950,82 @@ describe.each([true, false])("Unanswered question history (conversationMode=%s)" expect(container.querySelector('[data-testid="task-chat-unanswered-question"]')).not.toBeNull(); }); + it.each(["request_confirmation", "request_checkbox_confirmation"] as const)("immediately closes %s while a new message is sending and keeps it reopenable", async kind => { + const fixtures = await import("@/fixtures/issueThreadInteractionFixtures"); + const request = kind === "request_confirmation" + ? fixtures.genericPendingRequestConfirmationInteraction + : fixtures.pendingRequestCheckboxConfirmationInteraction; + let finishSend!: () => void; + const onAdd = vi.fn(() => new Promise(resolve => { finishSend = resolve; })); + const onAcceptInteraction = vi.fn(); + const onRejectInteraction = vi.fn(); + const show = () => render(); + show(); + expect(takeover()).not.toBeNull(); + await act(async () => editorCallbacks.change("Let's discuss something else first.")); + await act(async () => container.querySelector('[data-testid="task-chat-composer-send"]')!.click()); + expect(onAdd).toHaveBeenCalledTimes(1); + expect(takeover()).toBeNull(); + await act(async () => finishSend()); + expect(pendingIndicator()).toBeNull(); + flushSync(() => root!.unmount()); + root = createRoot(container); + show(); + expect(takeover()).toBeNull(); + await act(async () => container.querySelector(`[aria-label="Open request: ${request.title}"]`)!.click()); + expect(takeover()).not.toBeNull(); + expect(request.status).toBe("pending"); + expect(onAcceptInteraction).not.toHaveBeenCalled(); + expect(onRejectInteraction).not.toHaveBeenCalled(); + await act(async () => editorCallbacks.change("Keep the review for later.")); + await act(async () => container.querySelector('[data-testid="task-chat-composer-send"]')!.click()); + expect(takeover()).toBeNull(); + await act(async () => finishSend()); + expect(takeover()).toBeNull(); + }); + + it("leaves an earlier confirmation in history after a newer user message, including on reload", async () => { + const request = { ...genericPendingRequestConfirmationInteraction, createdAt: old.createdAt }; + const show = (comments = movedOn.slice(0, 0)) => render(); + show(); + expect(takeover()).not.toBeNull(); + await act(async () => show(movedOn)); + expect(takeover()).toBeNull(); + expect(pendingIndicator()).toBeNull(); + flushSync(() => root!.unmount()); + root = createRoot(container); + show(movedOn); + expect(takeover()).toBeNull(); + await act(async () => container.querySelector(`[aria-label="Open request: ${request.title}"]`)!.click()); + expect(takeover()).not.toBeNull(); + }); + + it("keeps a new confirmation open when an earlier message finishes sending", async () => { + const request = genericPendingRequestConfirmationInteraction; + const next = { ...request, id: "new-review", title: "New review", createdAt: new Date("2026-11-06T12:00:00Z") }; + let finishSend!: () => void; + const onAdd = vi.fn(() => new Promise(resolve => { finishSend = resolve; })); + const show = (interactions = [request]) => render(); + show(); + await act(async () => editorCallbacks.change("Let's discuss something else first.")); + await act(async () => container.querySelector('[data-testid="task-chat-composer-send"]')!.click()); + expect(takeover()).toBeNull(); + await act(async () => show([request, next])); + expect(takeover()?.textContent).toContain("New review"); + await act(async () => finishSend()); + expect(takeover()?.textContent).toContain("New review"); + }); + + it("restores the confirmation when sending a separate message fails", async () => { + render(); + await act(async () => editorCallbacks.change("Let's discuss something else first.")); + await act(async () => container.querySelector('[data-testid="task-chat-composer-send"]')!.click()); + expect(takeover()).not.toBeNull(); + expect(container.querySelector('[data-testid="mock-editor"]')?.textContent).toContain("Let's discuss something else first."); + }); + it.each(["Cancel", "dismiss"])("leaves only the history card after %s on a fresh question", async action => { render(); await click("Yes"); diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index 317fc7f4b9f..3ead170180a 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -2773,21 +2773,19 @@ export function TaskChatThread(props: TaskChatThreadProps) { input.kind === "durable" ? Boolean(input.interaction.dismissedAt) || dismissedInputIds.has(input.interaction.id) || dismissedInputIds.has(input.key) : dismissedInputIds.has(input.key), [dismissedInputIds]); - const pendingReminderInputs = useMemo(() => pendingComposerInputs.filter(input => - !isInputDismissed(input) && !(input.kind === "durable" && input.interaction.kind === "ask_user_questions")), - [pendingComposerInputs, isInputDismissed]); const [dismissError, setDismissError] = useState(null); const latestUserComment = useMemo(() => comments .filter(comment => !comment.deletedAt && comment.authorUserId && !comment.authorAgentId && !comment.createdByRunId) .reduce<(typeof comments)[number] | null>((latest, comment) => !latest || toMs(comment.createdAt) >= toMs(latest.createdAt) ? comment : latest, null), [comments]); const latestUserCommentId = latestUserComment?.id ?? null; - // A later message leaves a question answerable in history, without reopening - // its form on every render or reload. Explicit selection can still reopen it. const currentPendingInputs = useMemo(() => pendingComposerInputs.filter(input => - !isInputDismissed(input) && !(input.kind === "durable" && input.interaction.kind === "ask_user_questions" + !isInputDismissed(input) && !(input.kind === "durable" && latestUserComment && toMs(input.interaction.createdAt) < toMs(latestUserComment.createdAt))), [pendingComposerInputs, isInputDismissed, latestUserComment]); + const pendingReminderInputs = useMemo(() => currentPendingInputs.filter(input => + !(input.kind === "durable" && input.interaction.kind === "ask_user_questions")), + [currentPendingInputs]); const currentPendingKeys = useMemo(() => new Set(currentPendingInputs.map(input => input.key)), [currentPendingInputs]); const [takeoverMode, setTakeoverMode] = useState<"open" | "normal">("open"); const [selectedPendingKey, setSelectedPendingKey] = useState( @@ -2807,17 +2805,18 @@ export function TaskChatThread(props: TaskChatThreadProps) { lastInputUserCommentRef.current = latestUserCommentId; knownPendingKeysRef.current = currentKeys; setSelectedPendingKey((current) => { + if (hasNewInput) return currentPendingInputs[0]?.key ?? null; const currentInput = pendingComposerInputs.find(input => input.key === current); - const questionMovedOn = movedOn && currentInput?.kind === "durable" - && currentInput.interaction.kind === "ask_user_questions" && !currentPendingKeys.has(currentInput.key); + const requestMovedOn = movedOn && currentInput?.kind === "durable" + && !isInputDismissed(currentInput) && !currentPendingKeys.has(currentInput.key); return current && currentKeys.has(current) && (currentPendingKeys.has(current) || explicitlyOpenedInputRef.current === current) - && !questionMovedOn + && !requestMovedOn ? current : (currentPendingInputs[0]?.key ?? null); }); if (hasNewInput) setTakeoverMode("open"); - }, [pendingComposerInputs, latestUserCommentId, currentPendingInputs, currentPendingKeys]); + }, [pendingComposerInputs, latestUserCommentId, currentPendingInputs, currentPendingKeys, isInputDismissed]); const selectedPendingInput = pendingComposerInputs.find((input) => input.key === selectedPendingKey) ?? currentPendingInputs[0] ?? @@ -2867,19 +2866,22 @@ export function TaskChatThread(props: TaskChatThreadProps) { agentId: args[2].assigneeAgentId, agent: agentMap?.get(args[2].assigneeAgentId ?? ""), } : undefined); + const previouslyOpenedInput = explicitlyOpenedInputRef.current; + explicitlyOpenedInputRef.current = null; + setTakeoverMode("normal"); try { await onAdd(...args); - if (selectedPendingInput?.kind === "durable" && selectedPendingInput.interaction.kind === "ask_user_questions") { - dismissInput(selectedPendingInput.interaction.id); - setSelectedPendingKey(null); - setTakeoverMode("normal"); - } + dismissInput(pendingComposerInputs.map(input => input.kind === "durable" ? input.interaction.id : input.key)); } catch (error) { + if (selectedPendingInputKeyRef.current === selectedPendingInput?.key) { + explicitlyOpenedInputRef.current = previouslyOpenedInput; + setTakeoverMode(takeoverMode); + } rejectSubmission(attempt, error); throw error; } }, - [beginSubmission, rejectSubmission, onAdd, selectedPendingInput, dismissInput, agentMap], + [beginSubmission, rejectSubmission, onAdd, pendingComposerInputs, selectedPendingInput, dismissInput, agentMap, takeoverMode], ); const optimisticRunnerStartup = hasAgentAssignee && diff --git a/ui/src/hooks/useDismissedTaskQuestions.ts b/ui/src/hooks/useDismissedTaskQuestions.ts index b390d138d13..ad69f706b5b 100644 --- a/ui/src/hooks/useDismissedTaskQuestions.ts +++ b/ui/src/hooks/useDismissedTaskQuestions.ts @@ -16,10 +16,10 @@ export function useDismissedTaskQuestions(issueId?: string | null, userId?: stri const [state, setState] = useState(() => ({ storageKey, ids: loadDismissedQuestions(storageKey) })); const dismissedQuestionIds = useMemo(() => state.storageKey === storageKey ? state.ids : loadDismissedQuestions(storageKey), [state, storageKey]); - const dismissQuestion = useCallback((interactionId: string) => { + const dismissQuestion = useCallback((interactionId: string | readonly string[]) => { // Another tab may have saved more dismissals since this thread rendered. const ids = new Set([...loadDismissedQuestions(storageKey), ...dismissedQuestionIds]); - ids.add(interactionId); + for (const id of typeof interactionId === "string" ? [interactionId] : interactionId) ids.add(id); try { if (storageKey) localStorage.setItem(storageKey, JSON.stringify([...ids])); } catch { diff --git a/ui/storybook/fixtures/unansweredQuestionChat.tsx b/ui/storybook/fixtures/unansweredQuestionChat.tsx index a15985d9df3..5ff7e919bcb 100644 --- a/ui/storybook/fixtures/unansweredQuestionChat.tsx +++ b/ui/storybook/fixtures/unansweredQuestionChat.tsx @@ -33,7 +33,7 @@ export function QuestionChat({ movedOn = false, multiple = false, answered = fal return client; }); const [interactions, setInteractions] = useState([ - ...(review ? [{ ...genericPendingRequestConfirmationInteraction, id: "completion-review", title: "Review requested", summary: "Provide app configuration to finish integration.", payload: { version: 1 as const, prompt: "Provide app configuration to finish integration.", acceptLabel: "Approve completion", rejectLabel: "Continue work", target: { type: "custom" as const, key: "native_completion_review", label: "Completion decision" } } }] : []), + ...(review ? [{ ...genericPendingRequestConfirmationInteraction, id: "completion-review", createdAt: question.createdAt, title: "Review requested", summary: "Provide app configuration to finish integration.", payload: { version: 1 as const, prompt: "Provide app configuration to finish integration.", acceptLabel: "Approve completion", rejectLabel: "Continue work", target: { type: "custom" as const, key: "native_completion_review", label: "Completion decision" } } }] : []), ...(review ? [] : [answered ? { ...question, status: "answered" as const, resolvedAt: new Date("2026-04-01T12:06:00Z"), result: { version: 1 as const, answers: [{ questionId: "color", optionIds: ["blue"] }] } } : question]), ...(multiple ? [otherQuestion] : []), ]); diff --git a/ui/storybook/stories/questions-and-approvals/task-chat-unanswered-questions.stories.tsx b/ui/storybook/stories/questions-and-approvals/task-chat-unanswered-questions.stories.tsx index 3baaee007fb..132a9bfb312 100644 --- a/ui/storybook/stories/questions-and-approvals/task-chat-unanswered-questions.stories.tsx +++ b/ui/storybook/stories/questions-and-approvals/task-chat-unanswered-questions.stories.tsx @@ -22,9 +22,42 @@ const meta = { export default meta; type Story = StoryObj; +function resetDismissals() { + localStorage.removeItem("paperclip:task-question-dismissals:user-board:task-unanswered-questions-demo"); +} + +async function sendInsteadOfConfirming(canvasElement: HTMLElement) { + const canvas = within(canvasElement); + await waitFor(() => expect(canvas.getByTestId("task-chat-composer-takeover")).toBeVisible()); + await userEvent.type(canvas.getByRole("textbox", { name: "editable markdown" }), "Leave that review for later. Show me the integration status."); + await userEvent.click(canvas.getByRole("button", { name: /^Send$/ })); + await expect(canvas.queryByTestId("task-chat-composer-takeover")).not.toBeInTheDocument(); + await expect(canvas.queryByTestId("task-chat-pending-input-indicator")).not.toBeInTheDocument(); + await expect(canvas.getByRole("button", { name: "Open request: Review requested" })).toBeVisible(); +} + // Keep the test-drive story's dismissal preference across actual page reloads. export const TestDrive: Story = {}; export const CompletionReview: Story = { args: { review: true } }; +export const SendInsteadOfConfirming: Story = { + args: { review: true }, + beforeEach: resetDismissals, + play: async ({ canvasElement }) => { + await sendInsteadOfConfirming(canvasElement); + }, +}; +export const ReopenSkippedConfirmation: Story = { + args: { review: true }, + beforeEach: resetDismissals, + play: async ({ canvasElement }) => { + await sendInsteadOfConfirming(canvasElement); + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Open request: Review requested" })); + await expect(canvas.getByTestId("task-chat-composer-takeover")).toBeVisible(); + await expect(canvas.getByRole("button", { name: "Approve completion" })).toBeVisible(); + await expect(canvas.getByRole("button", { name: "Continue work" })).toBeVisible(); + }, +}; export const MovedOn: Story = { args: { movedOn: true } }; export const MultipleUnanswered: Story = { args: { multiple: true } }; export const AnsweredHistory: Story = { args: { answered: true } }; @@ -44,6 +77,6 @@ export const DismissAndAnswerLater: Story = { await userEvent.click(canvas.getByTestId("task-chat-unanswered-question")); await expect(canvas.getByRole("radio", { name: "Green" })).toBeChecked(); await userEvent.click(canvas.getByRole("button", { name: /^(Send|Submit) answers$/ })); - await waitFor(() => expect(canvas.getByTestId("task-chat-answered-questions-receipt")).toBeVisible()); + await waitFor(() => expect(canvas.getByText("Answers recorded", { exact: true })).toBeVisible()); }, };