fix(desktop): prevent pending plate flashes before Host admission

[Sun-GLiang]

Summary

A regular send() is shown optimistically in the transcript, but requests next_turn from the Host. Before Host admission, the local outbox repeated that requested placement and briefly moved the message into the pending plate. This affected both a new Session's first message and an ordinary send in an existing idle Session.

• A local delivery update without a Host Turn identity keeps the message's already displayed placement. Host admission and queue_update determine its eventual Turn or queue position.
• Ordinary sends persist localDisplayPlacement: current_turn separately from the Host placement request. On restart there is no optimistic row to merge, so the local outbox needs this presentation intent to restore the row in the transcript. The field travels inside the local-submit payload and is validated in Main; preload strips it from direct Host admission.
• Explicit follow-ups and steering begin in the pending plate and remain there while local delivery is pending. When a direct waitForHostAdmission request starts a Turn, its returned row is placed in the transcript immediately.

Before	After

Verification

• The renderer component test renders ChatView and Composer at optimistic send, local delivery, failed delivery, and Host admission. It verifies that ordinary messages remain in the transcript and explicit follow-ups remain in the pending plate.
• Action and projection tests cover an existing idle Session, a new Session's first send with a failed local delivery update, explicit steering and follow-up, and a directly admitted Host-started Turn. Local store and queue tests cover restart recovery and Host queue ownership.
• The local-submit IPC test covers the object-form localDisplayPlacement and rejects invalid values. The five focused suites passed locally (64/64).
• npm --workspace @maka/desktop run build:test, desktop typecheck, touched-file Biome lint, npm run format:check, and the strict-base renderer architecture check passed locally. The CI test job passed at the final head.
• Desktop E2E passed in CI at the final head. The before/after frames above were captured during the original verification.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Maka (OpenAI GPT-6) authored the initial fix and visual verification. Codex (OpenAI GPT-6) reviewed the change, revised the projection and restart recovery, covered existing and new Sessions, fixed direct Host admission placement, addressed both non-blocking review notes, and updated this description.

Checklist

• Tests cover the changed behavior
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[liugddx]

The diagnosis is right and nicely isolated: the flash is the local outbox republishing the requested placement (session-local-messages.tsx:59 → message.placement) over a row the renderer had already put in the transcript, and the before/after pair makes that unambiguous. The renderer regression that walks optimistic → outbox → admission through the real ChatView/Composer split (chat-view.tsx:380 / composer-message-queue.tsx:57) is the right altitude for this — it is the test that actually fails on main. Built build:test at head and ran the three touched suites: 28 pass / 0 fail.

My one real objection is to the predicate, not the layer.

P2 — The fix is keyed to "new Session" but the defect is "outbox placement with no Host evidence", so the identical flash survives on an idle existing Session. Every send() asks for next_turn regardless of busy state (app-shell-chat-actions.ts:361), the default bridge path is session-local:submit which persists that requested placement and returns locally_saved without publishing anything (session-local-service.ts:771,796; app-shell-chat-actions.ts:275), and busy sends don't come through send() at all — they go enqueueFollowUp → enqueueMessage with next_turn from the start (app-shell.tsx:1452). So for the existing-Session branch (app-shell-chat-actions.ts:425) the sequence is exactly the one this PR fixes: optimistic current_turn in the transcript → outbox Delivering…/next_turn, no turnId → row jumps into the plate → accepted+turnId → back to the transcript. Repro: open any idle Session with history, send "hi". The outbox's next_turn carries zero information here — it is the same value whether the Host turns out to be idle or busy — and a message the Host genuinely queues is already handled by the authority: accepted without turnId is retired (session-local-messages.tsx:46-50) and the Host queue_update renders it in the plate. Fix: drop the flag and make the merge rule "a local outbox update without hostTurnId does not change placement": ...(update.deliveryStatus !== undefined && update.hostTurnId === undefined ? { transientPlacement: current.transientPlacement } : {}).

Second authority. provisionalFirstSend is a narrower copy of "has the Host said anything about this row yet", which hostTurnId/Host-queue membership already answer; it is now the ninth place that decides plate membership (alongside chat-view.tsx:380, composer-message-queue.tsx:57, session-local-messages.tsx:46-50,59, app-shell-chat-actions.ts:286,393,426, use-quote-companion.ts:488). It is also memory-only: a first send that ends failed/unknown renders in the transcript today, but after a restart the outbox republishes it without the flag and the same row reappears in the plate. Keying on outbox state instead removes both the field (chat-view.tsx:149) and the inconsistency. If you'd rather keep the flag for this PR, the two spreads at transient-message-projection.ts:57-63 collapse to one — while the flag is set current.transientPlacement is necessarily current_turn, so both placement conditions are redundant.

P3 — Two unrelated edits at the first-send site (non-blocking). void publishTransientUserMessage(...) at :392 is a no-op (the function returns void), and dropping inlineReferences: [] at :396 is behaviour-neutral (chat-turn.tsx:273 defaults ?? []) but leaves the two send() branches spelled differently. Revert both.

Tests. The projection test at transient-message-projection.test.ts:202 and the first-loop assertions of the renderer test fail on main — keep them. :223 ("ordinary mid-turn send moves into the pending plate") passes on main and models a path send() doesn't take (a busy send starts life as next_turn); worse, it and the renderer test's ordinaryFollowUp block pin the idle-Session flash above as intended behaviour, so they'll need inverting under the fix. :233 (steering) passes on main via the pre-existing pendingSteering inheritance and exercises none of the new branch — delete or fold into a combined case. The busy-race assertions at app-shell-busy-race-settlement.test.ts:380,437 assert a field name rather than behaviour, and :380 enshrines the remaining bug; better to publish a next_turn/Sending outbox update inside the mocked submitMessage and assert the row is still current_turn. Missing: an idle-existing-Session send, and a first send whose outbox reports failed.

Body correction. "Ordinary sends into an existing Session can be real follow-ups" — on this path they can't be distinguished from the outbox alone, and busy sends route through enqueueMessage; worth rewording once the predicate changes. "The component regression failed on the pre-fix projection" holds.

Not yours: submitAndProject writes the requested placement even when disposition === 'turn_started' (app-shell-chat-actions.ts:286), so a waitForHostAdmission send (revision, Workbar, WorkHub) that starts a Turn sits in the plate until durable data arrives; turn_started ? 'current_turn' : placement would fix it, separately.

Dead-ends, so nobody re-runs them: a genuine busy-race first send does not get stuck in the transcript (the accepted/no-turnId retire drops the row, flag with it); the flag cannot persist forever (any hostTurnId, admission reply without deliveryStatus, retire or durable reconcile clears it); it doesn't leak through projectQueuedTransientMessages, companion side-chat (shares the merge but never sets the flag) or WorkHub; the optimistic publish cannot lose a race to the first outbox refresh (synchronous, before activate/submit).

Net: the smallest correct version is ~3 production lines — no new field, one merge spread that ignores placement from outbox updates carrying no hostTurnId, leaving the Host queue as the sole authority for the plate — and it fixes new and idle-existing Sessions alike.

[liugddx]

P2 — predicate, not layer. The defect is "outbox placement with no Host evidence", not "first send of a new Session": session-local-messages.tsx:59 republishes the requested next_turn for every send(), idle or busy, so it carries no information until a turnId arrives. Suggest dropping the flag and keying on the update itself:

...(update.deliveryStatus !== undefined && update.hostTurnId === undefined
  ? { transientPlacement: current.transientPlacement } : {}),

Genuine follow-ups still reach the plate via the Host queue (accepted without turnId is retired at session-local-messages.tsx:46-50). If you keep the flag, these two spreads collapse to one — while it is set, current.transientPlacement is necessarily current_turn.

[liugddx]

The existing-Session branch at :425 publishes the same current_turn row without this flag, then submitIntoSession asks for next_turn (:361) and gets locally_saved. Repro: idle Session with history, send "hi" — transcript → plate → transcript, the same flash. Busy sends don't come through here (enqueueFollowUp → enqueueMessage, app-shell.tsx:1452).

[liugddx]

This passes on main and models a path send() doesn't take (a busy send starts as next_turn). Together with the renderer test's ordinaryFollowUp block it pins the idle-Session flash as intended behaviour; under the suggested predicate it would need inverting.

[jackwener]

No findings. I reviewed exact head dddcf7ee9b829d3715b5f2cf525b944906797c83.

The earlier existing-Session gap is closed on this revision. A local outbox update that has delivery state but no Host Turn identity can no longer move an already-visible ordinary send into the pending plate. Ordinary sends also persist their local display placement separately from the Host placement request, so the same presentation survives restart, while accepted messages without a Turn identity are still retired in favor of the Host-owned queue. Direct admission that starts a Turn now places the row in the transcript immediately.

I verified the production flow from renderer submission through the preload boundary, durable local outbox, restored local-message projection, Host admission, and queue ownership. Removing the new merge rule made five focused regressions fail, including an idle existing Session, a failed first send, and the real ChatView/Composer split.

The exact-head affected suites passed 64/64. A clean synthetic merge onto current main (c299cc17a7f006f69dc7b27f89c00c9962afcb77, tree 21eba556e245f1162307a77fa6406eadc39dec85) passed the corresponding 65/65 tests, Desktop build, Biome, renderer architecture 112/112, E2E budget, ASF headers, and diff checks. The hosted exact-head test check is successful. The PR is mergeable and the generative-tool declaration and commit trailers are present.

This is a technical GO at the reviewed head. I am leaving a COMMENT review; I did not approve or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[liugddx]

Re-reviewed at dddcf7e. This is the version I'd want: the merge rule is now the single predicate "a local delivery update without hostTurnId keeps the shown placement" (transient-message-projection.ts:54-57), provisionalFirstSend is gone, and new-Session first sends, idle existing-Session sends and failed local delivery all go through the same path. Folding in the turn_started → current_turn fix at app-shell-chat-actions.ts:286 with its own test was a good call too.

Built build:test at head; the five touched suites (transient-message-projection, chat-view-optimistic-render, app-shell-busy-race-settlement, session-local, message-queue-ui-state) are 64 pass / 0 fail. Then two seeded reverts. Restoring the merge-base mergeTransientMessageProjection fails 5 tests across projection, renderer and busy-race. Reverting session-local-messages.tsx:59 to plain message.placement fails the restart-recovery test in message-queue-ui-state. So the new tests actually guard both halves of the change.

Correcting my first pass. I wrote that keying on outbox state would also fix the restart inconsistency. It wouldn't. After a restart there is no optimistic row to merge against, so the outbox's first publish is the only placement signal, and it can't tell an ordinary send() apart from an explicit follow-up: both request next_turn. A persisted presentation bit is the minimal answer to the problem I raised, and localDisplayPlacement is that bit. It survives the two intent rewrites in the dispatcher (session-local-service.ts:437-457, both spread record.intent). It never reaches the Host: preload strips it (preload.ts:2352) and normalizeSessionSendCommand would drop it anyway. Records written before this change fall back to placement, which is today's behaviour.

P3 — the field has two wire shapes (non-blocking). In the bridge contract it is a command property (bridge-contract.d.ts:1258), but preload pulls it out and sends it as a sixth positional IPC argument (preload.ts:2377) that session-local:submit validates on its own (session-local-service.ts:715-717). Keeping it on value and validating it next to normalizeSessionSendCommand would leave one shape end to end. Fine as-is if you prefer to keep the Host command type free of presentation fields.

P3 — unrelated churn (non-blocking). The result.turnId ? {…} : {} → result.turnId && {…} rewrites at :288 and :296 don't change behaviour. They're harmless, but they aren't part of this fix.

Tests: my earlier concerns are resolved. The ordinary-vs-explicit split is now explicit, steering asserts the retained placement, and idle existing-Session, failed-delivery, restart and direct turn_started cases are all pinned. The PR body now matches the code.

Net: this is now the smallest correct fix for the defect plus the restart case it implies. No blocking findings from me — approving; the two P3s are take-or-leave.

[Sun-GLiang]

Regarding P3 — the field has two wire shapes in this review: addressed in a9b18da90. localDisplayPlacement now travels in the value object for session-local:submit, and Main validates it beside normalizeSessionSendCommand. Preload still strips it from the direct Host-admission path. The local-submit test now covers persistence from the object payload and rejection of an invalid value. The five focused suites pass 64/64, and desktop typecheck passes.

[Sun-GLiang]

Regarding P3 — unrelated churn in this review: addressed in a9b18da90. Both result.turnId ? { ... } : {} expressions are restored. Follow-up 9652d67f1 inlines a single-use quotes fallback in the same function to satisfy the renderer architecture token budget. The strict-base architecture check, five focused suites (64/64), and desktop typecheck pass locally.

[Sun-GLiang]

Regarding the merge rule, restart correction, and test coverage in this review: agreed and retained. Outbox-only updates without a Host Turn identity keep the visible placement; the persisted localDisplayPlacement restores ordinary sends after restart, while explicit follow-ups and steering keep their own placement. The direct turn_started projection and its test remain. The final head passes the five focused suites (64/64) locally, and the CI test job, including Desktop E2E, is green. Thanks for the re-review and approval.