diff --git a/frontend/src/components/chat/clarification-form.test.tsx b/frontend/src/components/chat/clarification-form.test.tsx index 8a1c4ec1cf..aa0c1dbff9 100644 --- a/frontend/src/components/chat/clarification-form.test.tsx +++ b/frontend/src/components/chat/clarification-form.test.tsx @@ -17,6 +17,10 @@ const appContextMock = vi.hoisted(() => ({ filesDisabled: false, providerAvailable: true, sendMessage: vi.fn(), + state: { + commandOutcomes: {} as Record, + clarificationSubmissions: {} as Record, + }, })) const toastErrorMock = vi.hoisted(() => vi.fn()) const mcpAppsMock = vi.hoisted(() => ({ @@ -899,3 +903,445 @@ describe("ClarificationForm blank option filtering", () => { expect(blankOptionSpans(container)).toHaveLength(0) }) }) + +describe("ClarificationForm terminal command outcomes", () => { + // Issue #1500: after a reply is durably accepted, its command can still + // reach a terminal disposition before a turn is established. Whether the + // form may invite a resend is decided by the structured outcomes the + // backend broadcasts for the round's tracked commands, never by task + // state alone. The accountable submissions live in context keyed by + // request id - an ordered list, so a resubmit after an ack timeout does + // not orphan the earlier command's outcome - and the gate survives the + // submitting component instance being replaced. + const ROUND = "inputreq_r1" + + beforeEach(() => { + appContextMock.dispatch.mockReset() + appContextMock.filesDisabled = false + appContextMock.providerAvailable = true + appContextMock.sendMessage.mockReset() + appContextMock.sendMessage.mockResolvedValue(undefined) + appContextMock.state = { commandOutcomes: {}, clarificationSubmissions: {} } + toastErrorMock.mockReset() + }) + + afterEach(() => { + cleanup() + }) + + const form = (active: boolean, requestId = ROUND) => ( + + ) + + const submitButton = () => + screen.queryByRole("button", { name: "chatPage.clarification.submit" }) + + // Mirrors the reducer: RECORD appends to the round's list. + const recordedSubmissions = () => + appContextMock.dispatch.mock.calls + .map(([action]) => action) + .filter((action) => action?.type === "RECORD_CLARIFICATION_SUBMISSION") + .map((action) => action.payload as { + requestId: string + commandId: string + accepted: boolean + }) + + const mirrorSubmissions = () => { + appContextMock.state = { + ...appContextMock.state, + clarificationSubmissions: { + [ROUND]: recordedSubmissions().map(({ commandId, accepted }) => ({ + commandId, + accepted, + })), + }, + } + } + + // Drives one accepted submission and mirrors the RECORD dispatch into the + // mock context state, the way the real reducer would. + const submitAccepted = async () => { + fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } }) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + await waitFor(() => + expect(appContextMock.sendMessage).toHaveBeenCalledTimes( + recordedSubmissions().length, + ), + ) + const record = recordedSubmissions().at(-1) + expect(record).toEqual({ + requestId: ROUND, + commandId: expect.any(String), + accepted: true, + }) + // The recorded command id is the client message id the delivery used. + const config = appContextMock.sendMessage.mock.lastCall?.[1] as { + clientMessageId?: string + } + expect(record!.commandId).toBe(config.clientMessageId) + mirrorSubmissions() + return record!.commandId + } + + const withOutcomes = (outcomes: Record) => { + appContextMock.state = { + ...appContextMock.state, + commandOutcomes: Object.fromEntries( + Object.entries(outcomes).map(([commandId, resendSafe]) => [ + commandId, + { resendSafe }, + ]), + ), + } + } + + it("reactivates the form and preserves the draft when the outcome proves retry safe", async () => { + const { rerender } = render(form(true)) + const commandId = await submitAccepted() + await waitFor(() => expect(screen.queryByRole("textbox")).toBeNull()) + + rerender(form(false)) + withOutcomes({ [commandId]: true }) + rerender(form(true)) + + await waitFor(() => expect(submitButton()).toBeEnabled()) + expect(screen.getByRole("textbox")).toHaveValue("Beijing") + expect( + screen.getByText("chatPage.clarification.replyNotApplied"), + ).toBeInTheDocument() + // The record is consumed so the resend is armed exactly once. + expect(appContextMock.dispatch).toHaveBeenCalledWith({ + type: "CLEAR_CLARIFICATION_SUBMISSION", + payload: { requestId: ROUND }, + }) + }) + + it("keeps the form locked and surfaces the ambiguity when the outcome is not proven safe", async () => { + const { rerender } = render(form(true)) + const commandId = await submitAccepted() + await waitFor(() => expect(screen.queryByRole("textbox")).toBeNull()) + + rerender(form(false)) + withOutcomes({ [commandId]: false }) + rerender(form(true)) + + // The notice is visible, the draft is intact, and nothing invites a + // duplicate submission of the accepted reply. + const alert = await screen.findByRole("alert") + expect(alert).toHaveTextContent("chatPage.clarification.replyOutcomeUnknown") + expect(screen.getByRole("textbox")).toHaveValue("Beijing") + expect(submitButton()).toBeDisabled() + }) + + it("locks and explains a reasserted round whose accepted reply is still in flight", async () => { + const { rerender } = render(form(true)) + await submitAccepted() + await waitFor(() => expect(screen.queryByRole("textbox")).toBeNull()) + + rerender(form(false)) + rerender(form(true)) + + // No outcome means the command may still be in flight: the form opens + // to explain the lock instead of showing a bare greyed-out button, and + // nothing invites a duplicate. + expect( + await screen.findByText("chatPage.clarification.replyPending"), + ).toBeInTheDocument() + expect(submitButton()).toBeDisabled() + }) + + it("is not reopened by a late terminal event after a turn is established", async () => { + const { rerender } = render(form(true)) + const commandId = await submitAccepted() + await waitFor(() => expect(screen.queryByRole("textbox")).toBeNull()) + + rerender(form(false)) + // The turn was established, the task is running, and only then does a + // stale resend-safe terminal frame arrive: with the task not waiting, + // nothing may reopen the submitted form. + withOutcomes({ [commandId]: true }) + rerender(form(false)) + + expect(screen.queryByRole("textbox")).toBeNull() + expect(submitButton()).toBeNull() + + // Only an authoritative return to waiting_for_user consumes the + // outcome and reactivates the form - this is the resend-safe branch + // actually running, so the test fails if the gating logic is removed. + rerender(form(true)) + await waitFor(() => expect(submitButton()).toBeEnabled()) + expect( + screen.getByText("chatPage.clarification.replyNotApplied"), + ).toBeInTheDocument() + }) + + it("gates a fresh component instance for a round another instance submitted", async () => { + // The virtual waiting message and the persisted timeline message render + // the same round in different component instances; replacing the + // submitting instance must not drop the gate. + render(form(true)) + const commandId = await submitAccepted() + cleanup() + + withOutcomes({ [commandId]: false }) + render(form(true)) + + const alert = await screen.findByRole("alert") + expect(alert).toHaveTextContent("chatPage.clarification.replyOutcomeUnknown") + expect(submitButton()).toBeDisabled() + }) + + it("locks a fresh component instance while the accepted reply is still in flight", async () => { + // A freshly mounted instance starts with isSubmitted=false, so without + // the in-flight lock it would offer Submit for a round whose durably + // accepted reply has not reached a terminal outcome yet (surfaced by + // review on #2126). + render(form(true)) + await submitAccepted() + cleanup() + + render(form(true)) + + await waitFor(() => expect(submitButton()).toBeDisabled()) + expect( + screen.getByText("chatPage.clarification.replyPending"), + ).toBeInTheDocument() + }) + + it("does not lock a fresh instance for an unconfirmed ack-timeout delivery", async () => { + appContextMock.state = { + commandOutcomes: {}, + clarificationSubmissions: { + [ROUND]: [{ commandId: "maybe-sent", accepted: false }], + }, + } + render(form(true)) + + // The reply may never have been accepted at all: the composer keeps its + // advisory retry until a terminal outcome for the command arrives. + await waitFor(() => expect(submitButton()).toBeEnabled()) + }) + + it("still surfaces an earlier command's unsafe outcome after a resubmit", async () => { + // The orphaned-outcome regression: an ack-timeout records command 1, + // the advisory retry sends command 2, and only then does command 1's + // terminal outcome arrive saying it may have been committed. The round + // must still lock and surface the ambiguity - a single-slot record + // would have overwritten command 1 and shown nothing. + appContextMock.sendMessage.mockRejectedValueOnce(Object.assign( + new Error("ack timed out"), + { disposition: "outcome_unknown", userFacing: true }, + )) + const { rerender } = render(form(true)) + + fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } }) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + await waitFor(() => expect(recordedSubmissions()).toHaveLength(1)) + expect(recordedSubmissions()[0].accepted).toBe(false) + mirrorSubmissions() + + // The advisory retry resubmits; the second record appends. + await waitFor(() => expect(submitButton()).toBeEnabled()) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + await waitFor(() => expect(recordedSubmissions()).toHaveLength(2)) + mirrorSubmissions() + const [first, second] = recordedSubmissions() + expect(first.commandId).not.toBe(second.commandId) + + withOutcomes({ [first.commandId]: false }) + rerender(form(true)) + + const alert = await screen.findByRole("alert") + expect(alert).toHaveTextContent("chatPage.clarification.replyOutcomeUnknown") + expect(submitButton()).toBeDisabled() + }) + + it("withholds the proven-safe unlock notice while an unconfirmed reply is unresolved", async () => { + // Command 1's fate is unknown (ack timeout, no outcome); command 2 is + // proven not applied. The round returns to the advisory stance it + // already accepted for command 1 - but without the "not applied, safe + // to resend" promise, which cannot be made for command 1, and without + // consuming the record, so command 1's late outcome still gates. + appContextMock.state = { + commandOutcomes: { "cmd-2": { resendSafe: true } }, + clarificationSubmissions: { + [ROUND]: [ + { commandId: "cmd-1", accepted: false }, + { commandId: "cmd-2", accepted: true }, + ], + }, + } + render(form(true)) + + await waitFor(() => expect(submitButton()).toBeEnabled()) + expect( + screen.queryByText("chatPage.clarification.replyNotApplied"), + ).toBeNull() + expect(appContextMock.dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: "CLEAR_CLARIFICATION_SUBMISSION" }), + ) + }) + + it("returns a pending-locked instance to advisory once its accepted reply is proven not applied", async () => { + // The submitting instance's sequence: ack timeout (cmd 1) -> advisory + // resubmit accepted (cmd 2) -> pending lock -> cmd 2's outcome proves + // it was not applied while cmd 1 stays unresolved. The pending notice + // is now false and the lock has no exit, so the round returns to the + // advisory stance instead of diverging from a freshly mounted instance. + appContextMock.sendMessage.mockRejectedValueOnce(Object.assign( + new Error("ack timed out"), + { disposition: "outcome_unknown", userFacing: true }, + )) + const { rerender } = render(form(true)) + + fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } }) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + await waitFor(() => expect(recordedSubmissions()).toHaveLength(1)) + mirrorSubmissions() + + // Advisory resubmit succeeds and the accepted reply goes pending. + await waitFor(() => expect(submitButton()).toBeEnabled()) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + await waitFor(() => expect(recordedSubmissions()).toHaveLength(2)) + mirrorSubmissions() + rerender(form(true)) + expect( + await screen.findByText("chatPage.clarification.replyPending"), + ).toBeInTheDocument() + expect(submitButton()).toBeDisabled() + + withOutcomes({ [recordedSubmissions()[1].commandId]: true }) + rerender(form(true)) + + await waitFor(() => expect(submitButton()).toBeEnabled()) + expect( + screen.queryByText("chatPage.clarification.replyPending"), + ).toBeNull() + expect( + screen.queryByText("chatPage.clarification.replyNotApplied"), + ).toBeNull() + }) + + it("replaces a lingering send-failure alert when the ambiguity notice takes over", async () => { + // An outcome_unknown send failure leaves its alert on screen; when the + // command's terminal outcome later proves unsafe, the two notices must + // not compete as two simultaneous role="alert" regions. + appContextMock.sendMessage.mockRejectedValueOnce(Object.assign( + new Error("ack timed out"), + { disposition: "outcome_unknown", userFacing: true }, + )) + const { rerender } = render(form(true)) + + fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } }) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + await waitFor(() => expect(recordedSubmissions()).toHaveLength(1)) + await screen.findByRole("alert") + mirrorSubmissions() + + withOutcomes({ [recordedSubmissions()[0].commandId]: false }) + rerender(form(true)) + + await waitFor(() => { + const alerts = screen.getAllByRole("alert") + expect(alerts).toHaveLength(1) + expect(alerts[0]).toHaveTextContent( + "chatPage.clarification.replyOutcomeUnknown", + ) + }) + }) + + it("records the submission when the delivery outcome is unknown", async () => { + appContextMock.sendMessage.mockRejectedValue(Object.assign( + new Error("ack timed out"), + { disposition: "outcome_unknown", userFacing: true }, + )) + render(form(true)) + + fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } }) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + + // The reply may still have been durably accepted, so its eventual + // terminal outcome must gate this round like an acknowledged one - + // recorded as unconfirmed, which must not lock the form while no + // terminal outcome exists. + await waitFor(() => expect(recordedSubmissions()).toHaveLength(1)) + expect(recordedSubmissions()[0]).toEqual({ + requestId: ROUND, + commandId: expect.any(String), + accepted: false, + }) + mirrorSubmissions() + expect(submitButton()).toBeEnabled() + }) + + it("still reactivates a form that never submitted, ignoring unrelated outcomes", async () => { + appContextMock.state = { + clarificationSubmissions: {}, + commandOutcomes: { + "someone-elses-command": { resendSafe: false }, + }, + } + const { rerender } = render(form(false)) + rerender(form(true)) + + await waitFor(() => expect(submitButton()).toBeEnabled()) + expect(screen.queryByRole("alert")).toBeNull() + }) + + it("resets the gate for a new clarification round", async () => { + const { rerender } = render(form(true)) + const commandId = await submitAccepted() + await waitFor(() => expect(screen.queryByRole("textbox")).toBeNull()) + + rerender(form(false)) + withOutcomes({ [commandId]: false }) + rerender(form(true)) + await screen.findByRole("alert") + + rerender(form(true, "inputreq_r2")) + + await waitFor(() => expect(submitButton()).toBeEnabled()) + expect(screen.getByRole("textbox")).toHaveValue("") + expect(screen.queryByRole("alert")).toBeNull() + }) + + it("does not record a submission for a round without a request id", async () => { + render( + , + ) + + fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } }) + fireEvent.click( + screen.getByRole("button", { name: "chatPage.clarification.submit" }), + ) + + await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(1)) + // With no round identity to bind to, gating is skipped entirely rather + // than risking a recorded reply gating a different question. + expect(appContextMock.dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: "RECORD_CLARIFICATION_SUBMISSION" }), + ) + }) +}) diff --git a/frontend/src/components/chat/clarification-form.tsx b/frontend/src/components/chat/clarification-form.tsx index 6e2def03fc..e7e8d65c95 100644 --- a/frontend/src/components/chat/clarification-form.tsx +++ b/frontend/src/components/chat/clarification-form.tsx @@ -1,5 +1,10 @@ import React, { useEffect, useLayoutEffect, useMemo, useRef, useState } from "react" -import { Interaction } from "@/contexts/app-context-chat" +import { + ClarificationSubmission, + Interaction, + TerminalCommandOutcome, +} from "@/contexts/app-context-chat" +import { generateClientMessageId } from "@/lib/utils" import { Input } from "@/components/ui/input" import { Button } from "@/components/ui/button" import { Textarea } from "@/components/ui/textarea" @@ -133,11 +138,17 @@ export function ClarificationForm({ }: ClarificationFormProps) { // If onSend is provided, use it (e.g., from builder chat), otherwise use useApp let sendMessage: any, dispatch: any, contextFilesDisabled: boolean | undefined; + let commandOutcomes: Record | undefined; + let clarificationSubmissions: + | Record + | undefined; try { const appCtx = useApp(); sendMessage = appCtx.sendMessage; dispatch = appCtx.dispatch; contextFilesDisabled = appCtx.filesDisabled; + commandOutcomes = appCtx.state?.commandOutcomes; + clarificationSubmissions = appCtx.state?.clarificationSubmissions; } catch { // We might not be in the app context (e.g., agent builder chat) } @@ -184,6 +195,11 @@ export function ClarificationForm({ errorCode: ClientErrorCode | null } | null >(null) + // Which outcome notice the form owes the user. Raw category only; the + // sentence is resolved at render so a locale switch reaches it. + const [outcomeNotice, setOutcomeNotice] = useState< + "notApplied" | "unconfirmed" | "pending" | null + >(null) useLayoutEffect(() => { latestRequestIdRef.current = requestId @@ -194,18 +210,123 @@ export function ClarificationForm({ setIsSubmitted(!active && !isConnectAppsOnly) setIsOpen(active || isConnectAppsOnly) setSendFailure(null) + // A new round is a new question: the previous round's outcome notice no + // longer belongs on top of it. + setOutcomeNotice(null) }, [active, isConnectAppsOnly, requestId]) + // The reply commands this round is still accountable for, read from + // context so the gate survives this component instance being replaced + // (virtual waiting message vs. persisted timeline message render the same + // round). One round can hold several: an ack-timeout entry stays + // resubmittable, so a resubmit appends rather than replaces, and every + // tracked command's outcome still gets matched. + const outstandingSubmissions = useMemo( + () => (requestId ? clarificationSubmissions?.[requestId] : undefined) ?? [], + [requestId, clarificationSubmissions], + ) + // Reduced to booleans before entering the effect's dependency list: an + // unrelated command's outcome landing must not re-run the effect - that + // rerun used to wipe a visible failure alert. + const hasOutstanding = outstandingSubmissions.length > 0 + // Any tracked reply whose terminal outcome cannot prove non-application + // may have been committed: the round must surface that and stay locked. + const anyUnsafeOutcome = outstandingSubmissions.some((submission) => { + const outcome = commandOutcomes?.[submission.commandId] + return outcome !== undefined && outcome.resendSafe !== true + }) + // Every tracked reply is proven not applied: resending is safe. + const allProvenNotApplied = + hasOutstanding + && outstandingSubmissions.every( + (submission) => commandOutcomes?.[submission.commandId]?.resendSafe === true, + ) + // A durably acknowledged reply is still awaiting its terminal outcome. + // (An unconfirmed ack-timeout entry without an outcome deliberately does + // not count: it may never have been accepted, and the composer keeps its + // advisory retry for it - which is also why it blocks the proven-safe + // unlock above without forcing a lock here.) + const confirmedPending = outstandingSubmissions.some( + (submission) => + submission.accepted && commandOutcomes?.[submission.commandId] === undefined, + ) + // Distinguishes the round that has heard nothing yet from the one whose + // resolved replies are all proven safe while unconfirmed ones remain. + const anyOutcome = outstandingSubmissions.some( + (submission) => commandOutcomes?.[submission.commandId] !== undefined, + ) + useEffect(() => { - if (active) { - // A new clarification round reuses this component instance on the live - // turn render path, so a stale round-1 failure alert would sit on top - // of round 2's question. - setIsSubmitted(false) - setIsOpen(true) - setSendFailure(null) + if (!active) return + // Task state alone answers whether the task accepts input; it does not + // prove that repeating this round's accepted reply is safe (#1500). + // While a submission is outstanding, reactivation requires terminal + // outcomes that prove non-application for every tracked reply. + if (hasOutstanding) { + if (anyUnsafeOutcome) { + // Committed or unknown: surface the ambiguity without inviting a + // duplicate. The chat input remains available for a deliberate + // fresh message. The send-failure alert would compete with this + // one (two role="alert" regions), and this notice supersedes it. + setOutcomeNotice("unconfirmed") + setIsSubmitted(true) + setIsOpen(true) + setSendFailure(null) + return + } + if (confirmedPending) { + // Accepted and still being applied: lock even a freshly mounted + // instance (whose initial isSubmitted is false), and say why the + // form is locked instead of leaving a bare greyed-out button. + setOutcomeNotice("pending") + setIsSubmitted(true) + setIsOpen(true) + return + } + if (!allProvenNotApplied) { + if (anyOutcome) { + // Every resolved reply is proven not applied; only unconfirmed + // ack-timeout replies remain, and their duplicate risk is the + // same risk the advisory retry already accepted for them. Match + // the fresh-instance stance: return the round to advisory + // instead of leaving the submitting instance locked behind a + // now-stale pending notice. No proven-safe notice - that promise + // cannot be made while an unconfirmed reply is unresolved. + setOutcomeNotice((notice) => (notice === "pending" ? null : notice)) + setIsSubmitted(false) + setIsOpen(true) + } + // Nothing resolved yet: keep the composer's advisory retry - and + // its ack-timeout warning alert - exactly as they stand. + return + } + // Every tracked reply is proven not applied: consume the record so + // the resend is armed once, then reactivate below with the draft + // intact. + dispatch?.({ + type: "CLEAR_CLARIFICATION_SUBMISSION", + payload: { requestId }, + }) + setOutcomeNotice("notApplied") } - }, [active]) + // A new clarification round reuses this component instance on the live + // turn render path, so a stale round-1 failure alert would sit on top + // of round 2's question. The outcome notice is deliberately not cleared + // here: it explains why the form re-opened, and it falls away on the + // next submit or the next round. + setIsSubmitted(false) + setIsOpen(true) + setSendFailure(null) + }, [ + active, + hasOutstanding, + anyUnsafeOutcome, + allProvenNotApplied, + confirmedPending, + anyOutcome, + requestId, + dispatch, + ]) const normalizedInteractions = useMemo(() => { const seenFields = new Set() @@ -377,9 +498,33 @@ export function ClarificationForm({ } }) + // Minted here, not in sendMessage, so this round knows the id the + // durable command will carry and can correlate its terminal outcome + // (#1500). The builder onSend path has no durable command behind it and + // takes no part in outcome gating; neither does a round without a + // request id - the gate would have no round identity to bind to, and a + // recorded reply could end up gating a different question. + const clientMessageId = generateClientMessageId() + // ``accepted`` separates a durably acknowledged reply from one whose + // ack timed out: only a confirmed acceptance may lock a freshly mounted + // form while its terminal outcome is still pending. + const recordSubmission = (accepted: boolean) => { + if (onSend || !dispatch) return + if (typeof submittedRequestId !== "string" || !submittedRequestId) return + dispatch({ + type: "RECORD_CLARIFICATION_SUBMISSION", + payload: { + requestId: submittedRequestId, + commandId: clientMessageId, + accepted, + }, + }) + } + try { setIsSubmitting(true) setSendFailure(null) + setOutcomeNotice(null) // If textMessage is empty but we have files, send a generic message? const outboundFiles = filesDisabled ? [] : files const finalMessage = textMessage || (outboundFiles.length > 0 ? t("chatPage.clarification.uploadedFiles") : t("chatPage.clarification.confirmed")) @@ -387,7 +532,12 @@ export function ClarificationForm({ if (onSend) { await onSend(finalMessage, outboundFiles, metadata); } else if (sendMessage) { - await sendMessage(finalMessage, { force: true, metadata }, outboundFiles) + await sendMessage( + finalMessage, + { force: true, metadata, clientMessageId }, + outboundFiles, + ) + recordSubmission(true) } if (latestRequestIdRef.current !== submittedRequestId) return @@ -397,6 +547,13 @@ export function ClarificationForm({ dispatch({ type: "UPDATE_TASK_STATUS", payload: { status: "running" } }) } } catch (error) { + if (readSendDisposition(error) === "outcome_unknown") { + // An ack timeout: the reply may still have been durably accepted, + // so its eventual terminal outcome must gate this round exactly as + // an acknowledged submission's would - but as an unconfirmed + // delivery it must not lock the form while no outcome exists. + recordSubmission(false) + } if (latestRequestIdRef.current !== submittedRequestId) return console.error("Failed to send clarification response", error) // The rejection reason ("a previous guidance message is still being @@ -713,6 +870,21 @@ export function ClarificationForm({ ))} + {outcomeNotice === "unconfirmed" && ( +
+ {t("chatPage.clarification.replyOutcomeUnknown")} +
+ )} + {outcomeNotice === "pending" && ( +
+ {t("chatPage.clarification.replyPending")} +
+ )} + {outcomeNotice === "notApplied" && ( +
+ {t("chatPage.clarification.replyNotApplied")} +
+ )} {sendFailure && (
{sendFailureMessage}
diff --git a/frontend/src/contexts/app-context-chat.test.tsx b/frontend/src/contexts/app-context-chat.test.tsx index 69cdbbe940..c702912fc7 100644 --- a/frontend/src/contexts/app-context-chat.test.tsx +++ b/frontend/src/contexts/app-context-chat.test.tsx @@ -214,6 +214,7 @@ function StateProbe() {
{String(state.isHistoryLoading)}
{String(state.filePreview.isOpen)}
{String(state.isProcessing)}
+
{JSON.stringify(state.commandOutcomes)}
) } @@ -6646,3 +6647,374 @@ describe("error frame display projection", () => { ).toEqual(expected) }) }) + +function ClarificationSubmissionProbe() { + const { state, dispatch } = useApp() + return ( + <> +
+ {JSON.stringify(state.clarificationSubmissions)} +
+