feat(chat-core): route ChatEmbed's send through the transport wire - #8599
feat(chat-core): route ChatEmbed's send through the transport wire#8599CrysisDeu wants to merge 1 commit into
Conversation
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Wire-as-seam keeps receipt classification single-sourced while the scoped [DESIGN-REVIEWED] f915040 |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of I have everything I need: blind read reconciled, diff and strings read, screenshots verified (including the shot-08 anomaly), capture harness understood, and no recordings committed. Final review: UX-Verdict: CONCERNS Recovery flow reads well cold, but the after/refused-light frame is missing its error row, and the red failure copy leaves the reader with nothing to act on. Watch
Evidence gaps
Suggestions
[UX-REVIEWED] 150f90a |
GPT 5.6 Review —
|
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All verification counts are done. Composing the review. First-Principles-Verdict: CONCERNS The fix lands at cause level and deletes two duplicate spellings; what rides along is ChatEmbed's new hard requirement on the host Redux store. What this change shipsIntent: stop an embed's failed chat send from looking sent while the user's text is silently lost — a FIX (routed through the shared transport rather than patched in place).
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] f915040 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
45b35ec to
d40a771
Compare
d40a771 to
4aead39
Compare
|
GPT 5.6 — disposition for
|
|
First Principles — disposition for
|
4aead39 to
33a10cb
Compare
|
UX — disposition for
|
|
GPT 5.6 — disposition for
|
|
First Principles — disposition for
|
|
/ai-review override gpt 33a10cb: the wire's reject→transport-error branch fires only when the scoped api threw before any request left (permission check) or fetch itself rejected — a received-but-dropped response resolves and is classified unknown/refused; this mirrors the dashboard wire contract merged in #5909, and the onSend half of the finding was accepted and fixed in this SHA. |
Human judgment recorded@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
33a10cb to
21ba33a
Compare
|
UX — disposition for
|
|
First Principles — disposition for
|
85d0b9f to
e0be365
Compare
b4deb25 to
b5c6442
Compare
b5c6442 to
5819abf
Compare
5819abf to
e3ac2af
Compare
#8689) ChatPage's composer send and its mid-turn steer each hand-rolled the POST to /api/chat: an AbortController and 10 s timer, the resolves-not-rejects trap, readSendReceipt, and a catch that had to tell an abort from a network failure. The steer path was worse -- a separate api.steerChat helper with no deadline whose failure was only a console line. Both now call sendTurn (chat-core transport) and branch on receipt.status; the receipt policy (error row, composer restore, optimistic confirm, card retirement, ask resolution) is unchanged and stays on the surface. sendTurn gains the two flags this endpoint takes that ChatPage sets: steer (inject into the running turn / skip the sub-agent hold) and colorTheme. api.steerChat has no send-path consumer left on this surface (useSceneInteraction still uses it; its own P2 slice). RFC chat-core extraction, P2 (transport), third slice: after ChatPane (#5909), ChatEmbed (#8599) and SideChat (#8655), every user-facing composer send is on sendTurn.
e3ac2af to
d744c97
Compare
|
GPT 5.6 — disposition for
|
|
Design — disposition for
|
|
UX — disposition for
|
|
First Principles — disposition for
|
|
@bolichen97 — taking option (b), now complete at head
Requesting re-review. |
|
/ai-review override gpt 150f90a: The send-tail is a role:'error' row inside the chat transcript rendered by the registry's ErrorCard, which errors-use-error-notice itself lists under 'Does NOT flag' (the agent is already in that conversation, so a hand-off would be circular); the code is byte-identical to head fe0c36b, which this lane passed with no findings — only PNGs were added. |
Human judgment recorded@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
ChatEmbed (the app-sdk's embeddable chat) posted to the bare /api/chat, read its SSE stream as JSON, and called the resulting SyntaxError success; a refused POST rejected the mutation nothing read. Either way the composer had already been cleared, so a failed send looked sent and the text was gone. sendTurn gains an injectable wire (the fetch seam: one POST that resolves on every HTTP status and rejects only when the request never left or the deadline fired); dashboardSendWire is the default, so ChatPane and ChatPage are unchanged. appSendWire re-expresses the scoped AppApi outcomes in that shape (2xx JSON -> ok; 2xx non-JSON -> unknown; non-2xx -> refused with the server's reason via a typed AppApiError; permission denial -> refused with a human sentence; rejected fetch -> transport-error; deadline -> response-late). ChatEmbed sends through sendTurn over that wire. refused / transport-error render an error row and hand the draft back as typed; response-late renders a "Delivery not confirmed" notice and hands the draft back, retired only when the poll shows THIS send's own user row past the send-start point -- by identity only (meta.sendId, or membership in a merged row's meta.sendIds), never by text. The server keeps the id on every path a send can take: a dispatched send persists it on its row, and a send queued behind a busy slot carries it through the queue entry onto the drained row (#8853), so there is no text-matching fallback to false-retire on. unknown does nothing; dispatched / queued let the poll render. mintSendId now lives once in chat-core/transport; ChatPane's and ChatPage's inline minters use it. useChatSession's seed send moves off the bare SSE endpoint onto the same wire (fire-and-forget; the slot is the deliverable). The composer swap (#8631): ChatEmbed's bare <input> + send button becomes the native ChatInput inside a SlotProvider for the embedded slot, narrowed by omission (no upload, voice, agent/model/project chrome, typed command menus, prompt optimizer or slot-approval chrome) with a fail-closed `embedded` preset, a `sending` prop, and sendOnEnter via the new useChatConfig hook. Enter/IME handling, autosize, the running-state send affordance and the composer's a11y contract are now the shared ones; recovered drafts keep their paragraph breaks. Two now-unreferenced app-sdk keys (send, send_message) are removed from every catalog.
|
UX — disposition for
|
Problem / Motivation
ChatEmbed(the app-sdk's embeddable chat) was the third of four hand-written send paths. It posted to the bare/api/chat, which answers with an SSE stream; the scopedAppApiJSON helper buffered that stream and threwSyntaxError, and the embed's.catchcalled that success. A refused POST (409 slot-agent mismatch, 403 from an app that never granted/api/chat) rejected the mutation, but nothing readsendMutation.isError. Either way the composer had already been cleared, so a failed send looked sent and the text was gone.Why it matters
The chat-core extraction RFC (P0 contract inventory) named this the worst of the four paths: it is the one that can lose a user's turn with no trace. It is also the one surface that cannot import the dashboard client, because an app's sends must stay a permission the host app grants (
allowedApiPaths).What changed (motivation → approach → change)
Transport (
website/src/chat-core/transport/sendTurn.ts).sendTurnwas hard-wired to the dashboard client'sapi.sendChat. It gains an injectable wire — the fetch seam: one POST that resolves on every HTTP status and rejects only when the request never left or the deadline signal fired.dashboardSendWireis the default, so ChatPane (#5909) is unchanged. Receipt classification (readSendReceipt, the abort deadline, the six statuses) stays in the one place.App-sdk wire (
website/src/app-sdk/appSendWire.ts, new). Re-expresses the scopedAppApi.postoutcomes in the seam's shape: 2xx JSON → resolvedok; 2xx non-JSON (SyntaxError) → resolvedokwhosejson()rejects, i.e.unknown— the exact shape the old path swallowed; non-2xx → resolved!okcarrying the body text so the server's reason survives; a scoped-api permission denial → resolved!okwhose body carries a human sentence ("This app isn't allowed to send chat messages.", new keyappSdk.chatEmbed.app_not_allowed_to_send, 12 locales + en-XA) as therefusedreason — "check your connection" would be advice that can never succeed, and the raw app/path/grant detail is developer vocabulary, so it goes to the console instead — via a typedAppApiPermissionErrorthrown by the permission check; offline / fetch rejected →transport-error; deadline →AbortError→response-late(the scoped helper takes no signal, so the deadline is honoured at the receipt). The POST targets/api/chat?ws=1; the scoped path check reads only the pathname, so no new permission is required from apps.To read status and body back without parsing the
API <status>: <text>message string,createScopedApinow throwsAppApiError(website/src/app-sdk/apiError.ts). It is deliberately not re-exported from the barrel — the vendor stub mirrors the barrel value-for-value, so that would be an SDK surface change. Apps keep seeing the sameErrorwith the same message.ChatEmbed (
website/src/app-sdk/ChatEmbed.tsx). Sends throughsendTurnwith the app-sdk wire (theagentthe embed was mounted with rides on the wire, not the transport contract). Receipt policy:refused/transport-error→ anerrorrow at the transcript tail (the existing registry renderer, no new component) — the server's reason framed throughpages.chatPage.send_failed_with_error("Send failed: …", the same core key App.tsx's feedback send uses, so a raw "slot agent mismatch" does not read as the agent erroring mid-work); a reason-less transport failure states its cause ("Couldn't send — check your connection and try again.", new core keyappSdk.chatEmbed.send_failed_connection) instead of a bare "Send failed" — and the text handed back viamergeRecoveredDraftas typed (the composer's draft, whitespace and all — the wire carries the trimmed text, the recovery gives back what the user wrote; a paragraph-break join, rendered as such because the composer is nowChatInput's textarea — see the second commit below);response-late→ the text handed back under anoticerow ("Delivery not confirmed — …", retired only when the poll shows this message's own user row — identified by identity only: the client-mintedmeta.sendIdthe embed stamps on every send (mintSendId, now owned once bychat-core/transport— ChatPage's and ChatPane's identical inline minters are replaced by the shared one in this PR), read back off the polled slot detail either as the row'smeta.sendIdor as a member of a merged row'smeta.sendIds. The server keeps that id on every path a send can take — a dispatched send persists it on its row directly, and a send queued behind a busy slot carries it on the queue entry so the drain writes it onto the row (#8853, which this PR depends on) — so there is no text-matching fallback: an id-less row with the same words is some other writer's (an older backend, an injection, a resend) and is never proof (isOwnUserRow) — appended past the transcript length captured when the send started, at which point the restored text is also taken back out of the composer — unless the user has edited it since. Unrelated growth (a prior turn still streaming, a cron/sub-agent injection, a same-text row with another id, an older backend that dropsmeta) is not proof and leaves both notice and text alone, so a late turn that did land never sits above a standing "not confirmed" with a resendable copy below it, and an undelivered one is never silently withdrawn; new i18n keysappSdk.chatEmbed.delivery_unconfirmed/delivery_unconfirmed_optionin all 12 locales + en-XA) — this embed keeps no optimistic bubble, so once the composer cleared the user's text had no visible copy left, which is the one indeterminate outcome the transport contract names as recoverable (ChatPane leaves it alone only because its bubble is still on screen);unknown→ nothing (a 2xx was received, restoring could duplicate a running turn);dispatched/queued→ the poll renders it. An option chip's direct send never restores into the composer (it did not consume the draft — the same gate ChatPane/ChatPage keep). A host-suppliedonSendrejection proves nothing about delivery (a host may have posted and lost the answer), so it is treated as unconfirmed — notice + text handed back — never as a failure that invites a duplicate-turn retry. On a follow-up-chip send the unconfirmed notice uses its own copy (delivery_unconfirmed_option: "…re-pick the option if it is missing") because the draft-clobber gate means nothing was restored. The send-tail row is folded into the top-anchored embed's new-message scroll hash, so a failure/unconfirmed row is announced rather than appended below the fold, and mirrored into a politerole="status"live region so a screen-reader user hears the outcome instead of only noticing the button re-enable. Polling, approvals, follow-up chips and the composer itself are untouched.useChatSessionseed (website/src/app-sdk/useChatSession.ts). The RFC's "four hand-written send paths" undercounted: the app-sdk session hook seeded a new slot through the same bare/api/chatPOST with the same swallowedSyntaxError. It now sends the seed throughsendTurnover the app-sdk wire, fire-and-forget (the slot is the deliverable; creation must not wait out the transport deadline for an answer it ignores). The receipt is deliberately not acted on — the slot exists either way, and a seed that did not go out is an empty session, not a lost user message — but the SSE-as-JSON root cause is gone from the app-sdk entirely.RFC placement. Chat-core extraction P2 (transport), second slice. After #5909 (ChatPane), #8689 (ChatPage's send and steer, merged 2026-09-05) and this PR, 3 of 4 user-facing send paths plus the app-sdk seed send are on
sendTurn. Corrected inventory — the RFC's "four paths" undercounted; the remaining hand-writtenPOST /api/chatsites offsendTurnare: SideChat (the declared P2 remainder, next PR), plus the app-local and background senders outsideapp-sdk/that are NOT converted here and need their own slot in the series:apps/mochi/panel/panelBridge.ts(echoes the user bubble before the POST and never reads the receipt — the same looks-sent class),apps/design-tweak/api.ts(resolves a non-JSON 2xx as{ok: true}),hooks/useSceneInteraction.tsx(re-implementsreadSendReceiptclassification without the deadline),apps/design-critique/api.ts(the identical.catch(SyntaxError)swallow on the SSE endpoint this PR removes from ChatEmbed),App.tsx(the feedback send), and the two non-interactiveapi.sendChatcallersapps/issue-radar/agentSession.tsandapps/auto-improvement/agentSession.ts(no composer to restore into; they need the receipt classification, not the recovery policy). SideChat is #8655 (stacked on this branch; a second receipt adapter for the/side/*reject-semantics endpoint). Rebased onto main after #8689:SendWirePayloadnow also carriessteer/colorThemeso the default dashboard wire forwards ChatPage's flags unchanged; the app-sdk wire ignores them (the server refuses app-authenticated steers). ChatPane's reason-less transport failure still reads bare "Send failed" (ChatPane.tsx:427); it takes the shared cause-stating key in #8655, where that key is promoted topages.chatPage.*.Second commit — the composer swap (#8631, merged into this branch 2026-09-05). Reviewed and converged as its own PR (10 rounds, all five lanes) before being merged here so the two land together. ChatEmbed's bare
<input>is replaced by the realChatInputin aSlotProviderwith a fail-closedembeddedpreset — every dashboard capability defaults off, and a source-shape test forbids any= truedefault exceptconnected— asendingprop (spinner, "Sending…",aria-busy; the button is notdisabledwhile sending so the host's draft-clear does not strand focus),sendOnEnterforwarded via the newhooks/useChatConfig.ts, andChatEmbed.noDashboardClient.test.tsx, a recording-Proxy invariant that the embedded composer makes no dashboard-client call. This is what lets recovered text keep its paragraph breaks (above) and deletes the bespoke input+button. RFC §4.1 is ratified host-only for ChatEmbed — it mounts under the dashboard store; the store-freeChatInputseam that a standalone npm publish would need is P3 model-layer work, tracked in #8651 and recorded in the RFC so the decision is made, not deferred to publish time. Full history in #8631. Since the merge: an Enter pressed while a send is in flight is acknowledged — the spinner button pulses and "Sending…" is announced once to a live region — instead of silently early-returning. #6825 (optimistic bubble across a slot refetch) is the explicit downstream rebase point: itsmarkSendFailedpolicy re-bases onto transport callers after this lands; its semantics are not imported here. #6823 / #6307 / #5819 / #4904 are independent.Tests
chatCoreTransport.contract.test.ts(+3): an injected wire receives{message, slot, meta}plus anAbortSignaland the dashboard fetch is not touched; its resolved non-2xx isrefusedand its rejectiontransport-error; the deadline fires through it asresponse-late. 14 pass.appSendWire.contract.test.ts(new, 9): agent + JSON-receipt path on the wire; queued ≠ dispatched;AppApiError→refusedwith server reason (and with a non-JSON HTML 500 body);SyntaxErroron 2xx →unknown(the old swallowed shape); permission denial →refusedwith the human sentence + console detail; rejected fetch →transport-error; deadline →response-latewithout a signal; a settlement after the deadline does not flip the receipt.ChatEmbed.sendReceipt.test.tsx(new, 24): refused → error row with server reason + text handed back; transport failure → generic copy + handed back; merge with text typed mid-flight; unreadable 2xx → no row, no restore; deadline →noticerow + text handed back; dispatched → nothing; next send clears the row; option-chip send does not clobber the draft;onSendrejection → notice + handed back, no error row; timed-out chip send says "re-pick the option", not "text is back"; a failure row in a top-anchored embed triggers the new-message scroll; proven delivery (own user row bymeta.sendId) retires the notice and takes back an untouched restore, leaves an edited draft alone, a send the poll showed before the deadline leaves nothing behind, unrelated transcript growth keeps both notice and text, a same-text row with another sendId is not proof, a same-text row with no sendId is not proof either (no text fallback), a merged queued row naming this send inmeta.sendIdsis proof, a permission denial shows the human sentence, a transport failure names the connection, a refused send hands back the draft as typed (leading/trailing whitespace kept) while the wire carried the trimmed text, an Enter mid-flight pulses the spinner and announces "Sending…" without a second request, and an error row stays whatever arrives.ChatEmbed.test.tsx: send-path expectations updated to/api/chat?ws=1; default receipt{ok: true}. 35 pass.useChatSession.test.ts: seed expectations updated to the JSON-receipt path; a refused seed still yields a ready session.npm run typecheck, changed-file ESLint, and the i18n added-lines gate (0 untranslated) are clean.Manual verification
Screenshot harnesses:
website/capture/chat-embed-send-receipt.*(first commit: realChatEmbedunder the realAppApiProvider, fetch stubbed to refuse the send; asserts the error row and the restored composer before capturing — and now waits for the row'sanimate-scale-inentrance to finish before the frame is taken, since a first-frame capture showed an empty gap where the row was) andwebsite/capture/chat-embed-composer.*(second commit: the same surface with the mountedChatInput, light and dark).Screenshots / video
Frames are of the branch head's surface — the mounted
ChatInput— so they show both commits, and the PNGs are committed in this head's tree undertemp-screenshots/chat-embed-composer/(before/after, light/dark). BEFORE ismain(bare<input>, and a refused send left it looking exactly like idle: no row, composer empty, text gone).Permission denial (transport-error receipt), and light theme
First-commit-only frames (bare input, before the composer swap) — kept for the receipt-row history
Related Issues
Chat-core extraction RFC, phase P2 (transport). Builds on #5909 and #8689. Depends on #8853 (gateway-side
sendIdthrough the queue drain; opened from this PR's GPT finding). Downstream rebase point: #6825. no linked issue: RFC phase work tracked in the RFC, not an issue.Checklist
SendWireContribution License Agreement
N/A — repository placeholder only; no OSPO CLA text has been supplied.