Problem
ClarificationForm has two send paths with different guarantees.
The internal path (sendMessage from app-context-chat) owns a delivery identity: it keeps one clientMessageId for an unresolved submission, mints a new one only on success or when the server sets retry_with_new_id, and reads disposition/userFacing off the rejection to decide whether the visitor may submit again.
The injected path is just onSend(message, files, metadata) (frontend/src/components/chat/clarification-form.tsx). It receives no delivery id and no attempt context, and its only current provider (frontend/src/components/build/agent-builder-chat.tsx) throws a plain Error. Meanwhile the form's catch applies retryWithNewId and outcome_unknown handling to both branches, so the builder path is governed by a protocol it never participates in: it can retry or commit without the duplicate-turn guarantees the internal path has.
Nothing is broken today — the builder chat has no upload step and no durable turn identity — but the asymmetry is invisible at the call site and will bite whoever wires the next onSend provider.
Proposed fix
- Give the callback a declared delivery contract: pass the attempt context (client message id, cancellation) in, and type its failures with the same discriminated shape the internal path uses (
disposition, retryWithNewId, userFacing), rather than duck-typing those properties off whatever was thrown.
- Or have the form own one delivery identity for both branches and require providers to honour it.
A related maintainability thread: AppContext.sendMessage takes config?: any, and uploadFiles returns a bare array with no shared lifecycle/error shape. One small discriminated contract would cover all three.
Context
Raised as a minor finding in the #1472 review, deliberately not fixed there — it changes a callback signature and its provider, which is unrelated to that PR's retry/surfacing scope. Related: #1468, #1471.
Problem
ClarificationFormhas two send paths with different guarantees.The internal path (
sendMessagefromapp-context-chat) owns a delivery identity: it keeps oneclientMessageIdfor an unresolved submission, mints a new one only on success or when the server setsretry_with_new_id, and readsdisposition/userFacingoff the rejection to decide whether the visitor may submit again.The injected path is just
onSend(message, files, metadata)(frontend/src/components/chat/clarification-form.tsx). It receives no delivery id and no attempt context, and its only current provider (frontend/src/components/build/agent-builder-chat.tsx) throws a plainError. Meanwhile the form's catch appliesretryWithNewIdandoutcome_unknownhandling to both branches, so the builder path is governed by a protocol it never participates in: it can retry or commit without the duplicate-turn guarantees the internal path has.Nothing is broken today — the builder chat has no upload step and no durable turn identity — but the asymmetry is invisible at the call site and will bite whoever wires the next
onSendprovider.Proposed fix
disposition,retryWithNewId,userFacing), rather than duck-typing those properties off whatever was thrown.A related maintainability thread:
AppContext.sendMessagetakesconfig?: any, anduploadFilesreturns a bare array with no shared lifecycle/error shape. One small discriminated contract would cover all three.Context
Raised as a minor finding in the #1472 review, deliberately not fixed there — it changes a callback signature and its provider, which is unrelated to that PR's retry/surfacing scope. Related: #1468, #1471.