fix(frontend): retry transient clarification uploads and surface the real failure reason

frontend/src/components/chat/clarification-form.test.tsx

@@ -226,3 +226,398 @@ describe("ClarificationForm Session file capability", () => {
     expect(container.querySelector('input[type="file"]')).not.toBeNull()
   })
 })
+
+describe("ClarificationForm delivery failures", () => {
+  beforeEach(() => {
+    appContextMock.dispatch.mockReset()
+    appContextMock.filesDisabled = false
+    appContextMock.providerAvailable = true
+    appContextMock.sendMessage.mockReset()
+    toastErrorMock.mockReset()
+  })
+
+  afterEach(() => {
+    cleanup()
+  })
+
+  const deliveryError = (
+    message: string,
+    disposition: string,
+    userFacing = false,
+  ) => Object.assign(new Error(message), { disposition, userFacing })
+
+  const submitAnswer = async (onSend: ReturnType<typeof vi.fn>) => {
+    render(
+      <ClarificationForm
+        interactions={[{ type: "text_input" as const, field: "city", label: "City" }]}
+        onSend={onSend}
+      />,
+    )
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+    fireEvent.click(
+      screen.getByRole("button", { name: "chatPage.clarification.submit" }),
+    )
+  }
+
+  it("surfaces the backend rejection reason instead of the generic toast", async () => {
+    const onSend = vi.fn().mockRejectedValue(deliveryError(
+      "A previous guidance message is still being applied. Please wait for it to finish.",
+      "rejected",
+      true,
+    ))
+
+    await submitAnswer(onSend)
+
+    await waitFor(() => {
+      expect(toastErrorMock).toHaveBeenCalledWith(
+        "A previous guidance message is still being applied. Please wait for it to finish.",
+        { description: "chatPage.clarification.sendNotSent" },
+      )
+    })
+    expect(await screen.findByRole("alert")).toHaveTextContent(
+      "A previous guidance message is still being applied.",
+    )
+  })
+
+  it("keeps the form submittable after a failure that never reached the agent", async () => {
+    const onSend = vi.fn().mockRejectedValue(
+      deliveryError("Durable storage is temporarily unavailable", "not_sent", true),
+    )
+
+    await submitAnswer(onSend)
+
+    await waitFor(() => expect(toastErrorMock).toHaveBeenCalledWith(
+      "Durable storage is temporarily unavailable",
+      { description: "chatPage.clarification.sendNotSent" },
+    ))
+    const submit = screen.getByRole("button", {
+      name: "chatPage.clarification.submit",
+    })
+    expect(submit).toBeEnabled()
+    expect(screen.getByRole("textbox")).toHaveValue("Beijing")
+  })
+
+  it("tells the sender a delivery it could not confirm is safe to repeat", async () => {
+    const onSend = vi.fn().mockRejectedValue(deliveryError(
+      "The task is busy applying an earlier answer.",
+      "outcome_unknown",
+      true,
+    ))
+
+    await submitAnswer(onSend)
+
+    await waitFor(() => {
+      expect(toastErrorMock).toHaveBeenCalledWith(
+        "The task is busy applying an earlier answer.",
+        { description: "chatPage.clarification.sendDeliveryUnconfirmed" },
+      )
+    })
+  })
+
+  it("asks for a reload only when an attachment may need reconciling", async () => {
+    const onSend = vi.fn().mockRejectedValue(Object.assign(
+      new Error("The upload could not be completed or rolled back."),
+      { disposition: "outcome_unknown", userFacing: true, requiresReconciliation: true },
+    ))
+
+    await submitAnswer(onSend)
+
+    await waitFor(() => {
+      expect(toastErrorMock).toHaveBeenCalledWith(
+        "The upload could not be completed or rolled back.",
+        { description: "chatPage.clarification.sendOutcomeUnknown" },
+      )
+    })
+  })
+
+  it("keeps connection plumbing diagnostics away from the visitor", async () => {
+    const onSend = vi.fn().mockRejectedValue(deliveryError(
+      "Message not sent: the connection changed before delivery.",
+      "not_sent",
+    ))
+
+    await submitAnswer(onSend)
+
+    await waitFor(() => {
+      expect(toastErrorMock).toHaveBeenCalledWith(
+        "chatPage.clarification.sendError",
+        { description: "chatPage.clarification.sendNotSent" },
+      )
+    })
+    expect(await screen.findByRole("alert")).not.toHaveTextContent(
+      "the connection changed before delivery",
+    )
+  })
+
+  it("falls back to the generic string when the failure carries no reason", async () => {
+    const onSend = vi.fn().mockRejectedValue(new Error("   "))
+
+    await submitAnswer(onSend)
+
+    await waitFor(() => {
+      expect(toastErrorMock).toHaveBeenCalledWith(
+        "chatPage.clarification.sendError",
+        undefined,
+      )
+    })
+  })
+
+  it("clears the failure once the visitor edits an answer", async () => {
+    const onSend = vi.fn().mockRejectedValue(
+      deliveryError("Durable storage is temporarily unavailable", "not_sent", true),
+    )
+
+    await submitAnswer(onSend)
+
+    await screen.findByRole("alert")
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Shanghai" } })
+    await waitFor(() => expect(screen.queryByRole("alert")).toBeNull())
+  })
+})
+
+describe("ClarificationForm resubmission safety", () => {
+  beforeEach(() => {
+    appContextMock.dispatch.mockReset()
+    appContextMock.filesDisabled = false
+    appContextMock.providerAvailable = true
+    appContextMock.sendMessage.mockReset()
+    toastErrorMock.mockReset()
+  })
+
+  afterEach(() => {
+    cleanup()
+  })
+
+  const deliveryError = (
+    message: string,
+    disposition: string,
+    extra: Record<string, unknown> = {},
+  ) => Object.assign(new Error(message), { disposition, userFacing: true, ...extra })
+
+  const renderForm = () => render(
+    <ClarificationForm
+      interactions={[{ type: "text_input" as const, field: "city", label: "City" }]}
+    />,
+  )
+
+
+  const submit = () => fireEvent.click(
+    screen.getByRole("button", { name: "chatPage.clarification.submit" }),
+  )
+
+  const sentIds = () => appContextMock.sendMessage.mock.calls.map(
+    ([, config]) => (config as { clientMessageId?: string })?.clientMessageId,
+  )
+
+  it("stops a second submission when an attachment may already have landed", async () => {
+    // Uploaded bytes have no server-side dedup, so this is the one case a
+    // human has to reconcile before sending the draft again.
+    appContextMock.sendMessage.mockRejectedValue(deliveryError(
+      "The upload could not be completed or rolled back.",
+      "outcome_unknown",
+      { requiresReconciliation: true },
+    ))
+    renderForm()
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+
+    submit()
+
+    await waitFor(() => expect(screen.getByRole("button", {
+      name: "chatPage.clarification.submit",
+    })).toBeDisabled())
+    // Editing an answer must not talk the visitor back into resubmitting.
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Shanghai" } })
+    expect(screen.getByRole("button", {
+      name: "chatPage.clarification.submit",
+    })).toBeDisabled()
+    expect(screen.getByRole("alert")).toHaveTextContent(
+      "chatPage.clarification.sendOutcomeUnknown",
+    )
+    expect(appContextMock.sendMessage).toHaveBeenCalledTimes(1)
+  })
+
+  it("lets an unknown delivery outcome be retried under the same id", async () => {
+    // A reconnect during ack-wait is an ordinary event. The turn keeps its
+    // client message id, so the server adjudicates a duplicate instead of the
+    // form locking the visitor out until they reload the page.
+    appContextMock.sendMessage.mockRejectedValue(deliveryError(
+      "Message delivery was not acknowledged. Your draft was kept.",
+      "outcome_unknown",
+    ))
+    renderForm()
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(1))
+    expect(screen.getByRole("button", {
+      name: "chatPage.clarification.submit",
+    })).toBeEnabled()
+    // The copy must match the state: submitting again is safe here, so it
+    // must not tell the visitor to reload first.
+    expect(screen.getByRole("alert")).toHaveTextContent(
+      "chatPage.clarification.sendDeliveryUnconfirmed",
+    )
+    expect(screen.getByRole("alert")).not.toHaveTextContent(
+      "chatPage.clarification.sendOutcomeUnknown",
+    )
+
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(2))
+    const [first, second] = sentIds()
+    expect(second).toBe(first)
+  })
+
+  it("clears a previous round's block when the form is asked again", async () => {
+    // The live turn render path keeps one instance across clarification
+    // rounds, so a stale block would silently disable round two.
+    appContextMock.sendMessage.mockRejectedValue(deliveryError(
+      "The upload could not be completed or rolled back.",
+      "outcome_unknown",
+      { requiresReconciliation: true },
+    ))
+    const interactions = [{ type: "text_input" as const, field: "city", label: "City" }]
+    const { rerender } = render(
+      <ClarificationForm interactions={interactions} active />,
+    )
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+    submit()
+    await waitFor(() => expect(screen.getByRole("button", {
+      name: "chatPage.clarification.submit",
+    })).toBeDisabled())
+
+    rerender(<ClarificationForm interactions={interactions} active={false} />)
+    rerender(<ClarificationForm interactions={interactions} active />)
+
+    expect(screen.getByRole("button", {
+      name: "chatPage.clarification.submit",
+    })).toBeEnabled()
+    expect(screen.queryByRole("alert")).toBeNull()
+
+    appContextMock.sendMessage.mockResolvedValue(undefined)
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Shanghai" } })
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(2))
+    const [first, second] = sentIds()
+    expect(second).not.toBe(first)
+  })
+
+  it("retries an unresolved submission under its original client message id", async () => {
+    appContextMock.sendMessage.mockRejectedValue(
+      deliveryError("Durable storage is temporarily unavailable", "not_sent"),
+    )
+    renderForm()
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(1))
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(2))
+
+    const [first, second] = sentIds()
+    expect(first).toBeTruthy()
+    expect(second).toBe(first)
+  })
+
+  it("mints a fresh client message id when the server asks for one", async () => {
+    appContextMock.sendMessage.mockRejectedValue(deliveryError(
+      "Message id was already used for different content or files.",
+      "rejected",
+      { retryWithNewId: true },
+    ))
+    renderForm()
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(1))
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(2))
+
+    const [first, second] = sentIds()
+    expect(first).toBeTruthy()
+    expect(second).not.toBe(first)
+  })
+})
+
+describe("ClarificationForm undecided deliveries", () => {
+  beforeEach(() => {
+    appContextMock.dispatch.mockReset()
+    appContextMock.filesDisabled = false
+    appContextMock.providerAvailable = true
+    appContextMock.sendMessage.mockReset()
+    toastErrorMock.mockReset()
+  })
+
+  afterEach(() => {
+    cleanup()
+  })
+
+  const submit = () => fireEvent.click(
+    screen.getByRole("button", { name: "chatPage.clarification.submit" }),
+  )
+
+  const sentIds = () => appContextMock.sendMessage.mock.calls.map(
+    ([, config]) => (config as { clientMessageId?: string })?.clientMessageId,
+  )
+
+  it("never mints a new id after an undecided attempt reached the server", async () => {
+    // The server only asks for a new id when a claim already exists under the
+    // old one - which means the undecided answer did land. Minting another
+    // would answer the same question twice.
+    appContextMock.sendMessage
+      .mockRejectedValueOnce(Object.assign(
+        new Error("The message is still being applied. Please retry shortly."),
+        { disposition: "outcome_unknown", userFacing: true },
+      ))
+      .mockRejectedValueOnce(Object.assign(
+        new Error("Message id was already used for different content or files."),
+        { disposition: "rejected", userFacing: true, retryWithNewId: true },
+      ))
+    render(
+      <ClarificationForm
+        interactions={[{ type: "text_input" as const, field: "city", label: "City" }]}
+      />,
+    )
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(1))
+    // The form invites this: an unconfirmed delivery leaves Submit enabled.
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Shanghai" } })
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(2))
+
+    const [first, second] = sentIds()
+    expect(second).toBe(first)
+    // Nothing further may be sent: the answer is in an unresolved state only a
+    // reload can settle.
+    expect(screen.getByRole("button", {
+      name: "chatPage.clarification.submit",
+    })).toBeDisabled()
+    expect(screen.getByRole("alert")).toHaveTextContent(
+      "chatPage.clarification.sendOutcomeUnknown",
+    )
+  })
+
+  it("still mints a new id when the server refuses one that never landed", async () => {
+    appContextMock.sendMessage
+      .mockRejectedValueOnce(Object.assign(
+        new Error("Message id was already used for different content or files."),
+        { disposition: "rejected", userFacing: true, retryWithNewId: true },
+      ))
+      .mockResolvedValueOnce(undefined)
+    render(
+      <ClarificationForm
+        interactions={[{ type: "text_input" as const, field: "city", label: "City" }]}
+      />,
+    )
+    fireEvent.change(screen.getByRole("textbox"), { target: { value: "Beijing" } })
+
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(1))
+    submit()
+    await waitFor(() => expect(appContextMock.sendMessage).toHaveBeenCalledTimes(2))
+
+    const [first, second] = sentIds()
+    expect(second).not.toBe(first)
+  })
+})