feat(desktop): show sends while attachments prepare

[wesbillman]

Summary

Show a send-scoped pending timeline row after destination and audience preflight while deferred media or link-preview preparation finishes.

• stage one stable local row and let relay publication adopt its key instead of inserting a second optimistic message
• remove only that row on preparation, upload, cancellation, or relay failure
• render preparation status without borrowing composer-owned object URLs
• preserve the captured channel/thread destination across navigation
• close the cancel-before-upload-start race that could otherwise leave a phantom row

Stage 1 scope and known deferrals

This intentionally wires the staged row through ChannelScreen / ChannelPane, covering channel and thread composers. InboxDetailPane and NewMessageScreen retain the previous pill-only behavior for now.

Images remain on the existing blocking foreground-upload path. Deferred video/large files and link previews use the pending-row path. Plain-text sends also pass through the staged-key handoff, which is behaviorally equivalent to the existing optimistic insertion but keeps one mutation contract.

The following acceptance-matrix coverage is explicitly deferred rather than implied complete:

• relay/upload failure recovery while the user has already typed a new draft
• two simultaneous sends exercised through the full composer UI
• relay echo arriving before the mutation OK response

For media sends, staged content does not yet contain the uploaded imeta lines, so content matching cannot dedupe an early relay echo. Stable-key adoption is the primary dedupe until mutation success; an echo-before-OK could briefly render beside the staged row.

Related issue

No direct issue found. Builds on the deferred-media and background-preview work in #4522 and #5697.

Testing

Verified at pushed HEAD d6d1a9fa4e51221a47dfe8de54a0e3a55642c2ee:

• desktop unit suite: 4,907 passed, including targeted pending-row reconciliation and cancel-before-start coverage
• focused Playwright: 2 passed (sends immediately and keeps upload progress across channels; canceling a background upload prevents the message from publishing)
• pre-commit desktop checks and file-size ratchet passed
• pre-push gate passed: desktop checks, typecheck, unit tests, Rust tests, Tauri checks, and mobile tests

The focused Playwright cases verify the pending status in the timeline, stable row identity across channel navigation, exactly one resolved row, and row removal on cancellation.

[jedwards27]

Verdict: REQUEST CHANGES

Reviewed: 78cbffeb64c01220e705adf0aa9690fdbd0d7a37..496b353389ce2e13fb68c2f82c829b907d00f02b (exact head 496b353389ce2e13fb68c2f82c829b907d00f02b)

Risk: high — this changes optimistic/authoritative message reconciliation and therefore the event identity used by video review, replies, and reactions.

P1 — accepted video sends can leave a duplicate phantom row and never acquire a usable durable event identity

The exact-head runtime journey visibly produces a staged video row marked Sending…, then a second accepted-looking row appears while the first remains stuck for the full recording. Review never opens. This is not merely a test selector retaining a local key: product actions require a durable relay event ID.

Recording (exact-head Playwright artifact, 30.8s):
https://d24qwcpro867f5.cloudfront.net/repos/buzz/prs/5962/pr5962-exact-head-video-review-failure.mp4

• [00:00–00:02] staged video row appears with Sending….
• [00:02–00:30.8] a second video row is visible; the first stays wedged and review never opens.
• The semantic snapshot contains two separate message articles, one still carrying Sending… (desktop/test-results/.../error-context.md:338-415).

The reconciliation path preserves the pending row's local identity (desktop/src/features/messages/lib/messageMerge.ts:34-55; live overlay enters it at channelWindowStore.ts:215-221), while MessageRow and downstream actions consume message.id (MessageRow.tsx:662-680,886; videoReviewContext.ts:216-244). The resulting optimistic ID cannot safely serve as the review root, so the journey stalls before publishing a comment against a nonexistent event.

Reproduction: after a clean pnpm build:e2e, the three focused video-review tests failed at the durable-ID wait on every candidate attempt (9/9, video-attachment.spec.ts:245-250,309-314). Current CI independently has the same three persistent failures. On exact base 78cbffeb64c0, all three journeys progressed past this identity transition and ultimately passed; one first attempt flaked later at the unrelated animated speed-menu click, then passed on retry.

Preserve localKey/renderKey only for React row continuity, but ensure every settled row and action adopts the authoritative relay event id; remove the staged copy from every relevant store even when uploaded media changes message content. Add a causal E2E proving: exactly one settled row, no lingering Sending…, durable data-message-id, review opens, and a timecoded comment persists under that durable root. Please also cover cancel/upload failure/relay rejection with row removal, draft recovery, focus/status announcement, and a usable retry path.

Validation at matching clean candidate HEAD:

• pnpm build:e2e — passed.
• focused video-review E2E, 3 tests × 3 attempts — 9 failed, same durable-ID timeout.
• just desktop-ci — passed at 496b353389ce…; this does not exercise the failing user journey.
• GitHub Desktop Smoke E2E (4) and aggregate Desktop — failed at this head.
• Independent product/adversarial inspection confirmed the visible duplicate/phantom state and inaccessible recovery.

Residual risk: The mock-boundary recording proves the renderer/store contract and user-visible failure, not a native WKWebView/media pipeline. A fix should additionally provide a clean exact-head journey showing send → one durable row → review → persisted comment; head movement invalidates this review.

[themiguelamador]

Verdict: REQUEST CHANGES

Reviewed exact head 496b353389ce2e13fb68c2f82c829b907d00f02b.

P1 — ChannelPane drops the staged optimisticId before calling onSendMessage. The public callback type includes the argument, but the local wrapper ended at forceRest, so useSendMessageMutation creates a second optimistic row instead of adopting the staged upload row. For media sends whose final content differs from the staged content, the relay echo cannot content-match that row; it remains stuck as Sending beside the accepted row, and video review cannot obtain a durable event ID.

The fix is to accept and forward optimisticId through that wrapper. The file-attachment E2E also needs to stop asserting that the optimistic DOM ID survives settlement; it must assert that the pending ID disappears, exactly one settled row remains, preparation status is gone, and data-message-id is the durable relay event ID.

P3 — PendingMessagePreparation keys rows by kind and filename, so two queued files with the same name produce duplicate React keys. Include an occurrence discriminator; these rows are stateless.

Repository policy — all four commits retain prohibited automated-assistant co-author trailers. The branch history must be replaced, not merely followed by another commit.

A complete clean replacement is available as Complear/buzz@aa6f71788dfb3305ad8091679423800f0849f047 on review/pr-5962-fix. It is based on the actual PR merge base and contains the full PR tree plus both fixes, with a signed attribution-compliant commit.

Validation on that exact tree:

• desktop static checks, file-size guard, rem-text guard, and pubkey-truncation guard passed
• desktop unit suite: 4,908 passed
• E2E build passed
• all three video-review journeys cited in the earlier review passed
• cross-channel upload settlement and cancellation journeys passed
• the settled attachment assertion now proves the optimistic ID is removed and replaced by a durable event ID

Please replace the PR history with the clean replacement commit/tree.

[jedwards27]

Verdict: REQUEST CHANGES

Reviewed exact head 496b353389ce2e13fb68c2f82c829b907d00f02b against PR base 78cbffeb64c01220e705adf0aa9690fdbd0d7a37.

P1 — the main channel composer drops the staged row identity, leaving an accepted video send duplicated and permanently pending

MessageComposer passes the staged optimisticId as the seventh onSend argument (useMentionSendFlow.ts:547-555), and the mutation handler is prepared to adopt it (useChannelPaneHandlers.ts:281-301). However, the intervening main-channel wrapper ends at forceRest and calls onSendMessage with only six arguments (ChannelPane.tsx:302-327). Thread sends do not have this omission.

Consequently useSendMessageMutation.onMutate does not take its adoption branch (hooks.ts:664-670); it creates another optimistic row. The authoritative video has different content/tags from the pre-upload staged row, so content-based live reconciliation cannot replace the staged row (messageMerge.ts:13-31). The result is two rendered videos, one stuck at Sending…, and downstream actions cannot reliably acquire the durable relay event ID.

I reproduced this from a clean E2E build at the exact head:

pnpm build:e2e
pnpm exec playwright test tests/e2e/video-attachment.spec.ts \
  --project=smoke \
  --grep 'video upload previews use poster frames and inline videos open review mode|video review comments survive closing and reopening|video review comment composer posts timecoded thread replies' \
  --workers=1

The first selected journey failed at video-attachment.spec.ts:303: expected zero Sending labels after settlement, received one. The failure screenshot visibly contains two video rows, with the first still marked Sending…. GitHub's Desktop Smoke E2E (4) and aggregate Desktop checks are independently red at this SHA.

Forward optimisticId through ChannelPane.handleSendMessage. Then make the E2E assert the actual contract: the pending ID disappears, exactly one settled row remains, preparation/sending status is gone, the row exposes the authoritative relay event ID, and review plus a timecoded reply work against that durable root. The current file-attachment assertion at file-attachment.spec.ts:286-292 instead requires the pending DOM ID to survive settlement, which codifies the wrong identity contract.

P3 — same-name queued attachments produce duplicate React keys

PendingMessagePreparation.tsx:24-28 keys media rows as media-${filename}. Two queued files may legally share a filename, producing duplicate sibling keys and unstable reconciliation. Include an occurrence discriminator such as the existing map index for every row, not only when the filename is absent.

Repository policy — replace the branch history to remove automated-assistant co-author trailers

All four commits at this head contain Co-authored-by: Carl <…@buzz.block.builderlab.xyz>. These are automated-assistant attribution trailers and require history replacement rather than a follow-up commit.

Exact-head validation

• pnpm check — passed (existing warnings only).
• pnpm test — 4,908 passed, 0 failed.
• pnpm build:e2e — passed.
• Focused video-review journey — failed with the duplicate/stuck row described above.
• Live head rechecked immediately before review submission: 496b353389ce2e13fb68c2f82c829b907d00f02b.

Static and unit checks are green, but they do not exercise the broken optimistic-to-authoritative identity transition. This remains merge-blocking.

[jedwards27]

Additional merge-blocking reconciliation path at the same exact head

Even after forwarding optimisticId through the main composer wrapper, the window refresh path can independently strand a staged row.

Sequence:

1. the staged row exists in liveOverlay;
2. a page-zero refresh returns the authoritative accepted event before its live echo;
3. replaceNewestChannelWindow removes overlay entries by authoritative event id only (channelWindowStore.ts:104-122), so the optimistic-id row remains in the store;
4. render-cache projection can transfer its localKey to the page event (channelWindowReconciliation.ts:26-60), but does not mutate the window store;
5. a later live echo reconciles against liveOverlay, where it cannot see the page event and cannot content-match media whose staged and final payloads differ, leaving both rows alive.

A temporary exact-head unit probe reproduced [initial, pending, accepted] where [initial, accepted] was required. The source mechanism is consistent with the exact-head E2E duplicate/stuck-row failure reported above.

Please reconcile pending overlays into the authoritative window during page-zero replacement, not only in the flattened render projection, and add a causal regression for:

staged row -> head refresh contains authoritative send -> live echo -> one authoritative row

This belongs to the same P1 identity/deduplication family, but forwarding the dropped callback argument alone does not cover it.