fix(desktop): bound thread /query and surface load errors, not false-empty

[wpfleger96]

Long threads in the desktop app sometimes never load (stuck on a skeleton until you close and reopen the panel), and a failed load silently renders as "No replies in this branch yet" — presenting a broken fetch as an authoritative empty thread with no way to recover. This fixes both, the two IMPORTANT findings from the thread-load investigation.

Defect 1 — unbounded /query request

The shared reqwest::Client in relay.rs sets no timeout, and neither /query request builder set a per-request .timeout(...). A stalled or half-open connection (headers or body never arrive) leaves the request pending forever, so a thread-history load hangs on the skeleton indefinitely.

Fix: a 30s per-request deadline on both /query builders, funnelled through one send_query_request helper so the timeout can never be applied to one builder and dropped from the other. Scoped per-request rather than client-level because the same client also serves STT/TTS model downloads, builderlab auth, and the media proxy — a client-level timeout would cut those off. The deadline sits above the 25s WS HISTORY_TIMEOUT_MS so a slow-but-live relay isn't cut off before the WebSocket path would be. A timeout surfaces through classify_request_error as the stable "relay unreachable: request timed out" string.

send() resolves as soon as response headers arrive, so a relay that returns headers and then stalls the body trips the deadline during body consumption, not at send() — and that consumption happens on two paths: parse_json_response for 2xx, and relay_error_message for a non-success status (500/429/…). Both paths route their body-consumption error through one shared classify_body_timeout helper so they can't drift: a stalled body surfaces the stable "relay unreachable: request timed out" string on either path rather than the malformed-response bucket (2xx) or a bare "relay returned 500" status label (non-2xx). A genuinely non-stalled error still keeps its status classification.

Defect 2 — terminal error painted as empty

ChannelScreen consumed only isPending/data from the thread-replies query. Once React Query exhausted its one retry, isPending was false and the zero-length data fell through selectDeferredListRenderState to the "empty" state — indistinguishable from a genuinely empty branch, with no retry affordance.

Fix: plumb isError + refetch through ChannelScreen → ChannelPane → MessageThreadPanel. A pure selectThreadRepliesSurface helper decides the paint in strict precedence — the load-bearing invariant is that a terminal error never resolves to "empty", and cached replies stay visible non-destructively under a later error (the error card only surfaces when there is nothing to show). The panel renders an explicit "Couldn't load replies" + Retry card (testids message-thread-replies-error / message-thread-replies-retry).

ProjectConversationPanel is a second producer of the same shared panel and used to hard-code threadRepliesPending={false} with no error/retry, so a failed load in a Projects conversation still painted the false-empty. It now propagates the same isPending/isError/refetch from its useThreadReplies query.

The multi-root useThreadRepliesForRoots hook (the Huddle transcript and Projects-agent conversation surfaces) had the same gap in its useQueries combine: it returned only { events, isPending }, so a failed reply subtree contributed zero rows and vanished. The combine is now a pure, unit-testable combineThreadRepliesResults that exposes aggregate isError/error plus a refetch that re-runs only the failed subtrees. Both multi-root consumers render the shared "Couldn't load replies" + Retry card when a subtree fails: the Projects-agent conversation after its transcript, and the Huddle transcript as a non-destructive banner above the timeline. useHuddleChannelMessages used to read only .events and discard the aggregate state, so one summarized root failing left the flattened transcript presenting as complete; it now propagates threadRepliesError/onRetryThreadReplies through ChannelScreen into ChannelPane, where successful rows stay visible and onRetry re-runs only the failed subtrees.

The error card carries role="alert" so its asynchronous appearance is announced to assistive tech — without a live region a screen-reader user parked in the composer never learns the load failed or that Retry became available.

Tests

• stalled_query_request_times_out_with_classified_error — a loopback server that never responds; asserts the stable classified timeout string.
• stalled_response_body_times_out_with_classified_error — a loopback server that writes valid 2xx JSON headers then stalls the body past the deadline; asserts the classified timeout string, not the malformed bucket.
• stalled_error_response_body_times_out_with_classified_error — a loopback that writes 500 headers promising a body it never sends; asserts the classified timeout string rather than the 500 status label.
• non_stalled_error_response_yields_status_message — a promptly-served 500 still surfaces "relay returned 500 Internal Server Error", pinning that timeout preservation is scoped to actual timeouts.
• selectThreadRepliesSurface — pending→skeleton, terminal error→error (never empty), page-2 failure never empty, cached rows stay visible under error, successful-empty→empty, retry-success→list, streaming→pending, and huddle-transcript collapse.
• MessageThreadReplyState mounted test — terminal error renders the error card (asserting role="alert"), never the empty card.
• combineThreadRepliesResults — multi-root aggregation/order, a failed subtree surfaces the aggregate error and never drops rows, aggregate pending, refetch re-runs only failed queries, all-success yields no error.
• thread-load-failure.spec.ts (smoke E2E) — binds the real channel-thread panel wiring: forces a terminal get_thread_replies failure at the IPC boundary, asserts the error card renders (never the false-empty) and Retry recovers.
• project-conversation-load-failure.spec.ts (smoke E2E) — the same guard for the Projects conversation producer, driven through the Projects Channels-tab row.
• huddle-thread-load-failure.spec.ts (smoke E2E) — the consumer-level guard the combine unit test can't provide: drives the real Huddle wiring (useHuddleChannelMessages → ChannelScreen → ChannelPane) with two summarized roots, fails one subtree's fetch at the IPC boundary, asserts the surviving root's reply stays visible while the retry alert surfaces, then Retry recovers the failed subtree and clears the alert.

Structure

To stay under the desktop file-size ratchet, relay.rs's inline test module moved to relay/tests.rs, and two pure pieces were extracted from the panel: the empty/error reply cards (MessageThreadReplyState) and the per-row branch-highlight derivation (selectThreadRowHighlight).

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Verdict: REQUEST CHANGES

Reviewed: exact head 04b76b7615caaa8938b5f86e3c587e00cfead5c9 against exact base / merge-base 2edacde4d4c01490834725774aa878dbc373c41d.

Blocking findings

[P2] Projects conversations still turn a failed thread load into an authoritative empty thread

ProjectConversationPanel is a second producer of the shared MessageThreadPanel, but this PR only wires the new query failure state through ChannelScreen.

The Projects path calls the same useThreadReplies query (desktop/src/features/projects/ui/ProjectConversationPanel.tsx:83), then treats any fetched result, including a terminal error, as sufficient to commit the empty reply expansion (:134-142). When it renders the panel, it hard-codes threadRepliesPending={false} and passes neither threadRepliesError nor onRetryThreadReplies (:249-284). With no cached replies, a /query failure therefore reaches ThreadReplyRegion as not pending, not errored, and empty, which renders “No replies in this branch yet” with no recovery. That is the same user-facing defect this PR fixes in ordinary channel threads.

Please propagate threadRepliesQuery.isPending, .isError, and .refetch() through this producer too, and add a regression that drives the Projects conversation surface through terminal failure and retry.

[P2] A response-body timeout is mislabeled as a malformed relay response

The request deadline correctly covers connect through full response-body consumption, but the timeout classification only wraps RequestBuilder::send() (desktop/src-tauri/src/relay.rs:394-406). Once a relay sends successful headers, send() resolves; if the body then stalls until the request deadline, the error is raised by response.json() in parse_json_response. That function maps every body read/decode failure to MALFORMED_RESPONSE_MESSAGE (relay.rs:225-249), discarding reqwest::Error::is_timeout().

So the new bounded request no longer hangs, but a half-open body produces “relay returned malformed response” rather than the promised stable “relay unreachable: request timed out” classification. The added test only stalls before headers (desktop/src-tauri/src/relay/tests.rs:269-314), so it cannot catch this branch.

Please preserve timeout classification while consuming the body and add a loopback regression that sends valid 2xx JSON headers, then stalls the body beyond the deadline.

[P2 accessibility] The asynchronous load failure is silent to assistive technology

ThreadRepliesErrorCard appears only after the query and retry lifecycle reaches a terminal failure, but its newly inserted container has no role="alert", aria-live, status semantics, or focus management (desktop/src/features/messages/ui/MessageThreadReplyState.tsx:24-49). A screen-reader user can remain in the composer without learning that history failed or that Retry became available. Buzz’s product contract requires WCAG 2.1 AA.

Please give the asynchronous error announcement appropriate alert/live-region semantics and assert that contract in the mounted component test.

Validation and notes

• Exact-head GitHub CI is green; git diff --check is clean.
• Current fetched main (fc2ce6728b3b4805040c0a2f2cc5c15f1c1806ce) merge-tree is conflict-free.
• The scoped /query timeout, shared builder helper, ordinary channel error/retry wiring, and branch-highlight extraction otherwise trace correctly.
• I am not treating cached rows remaining visible under a failed background refetch as a blocker: that is an explicit non-destructive behavior in this PR. A secondary stale-data warning/retry could improve it, but it is not the original false-empty failure.
• Local Tauri test execution was blocked before compilation because the clean review worktree lacks desktop/src-tauri/binaries/buzz-acp-aarch64-apple-darwin; CI provides the broad exact-head evidence here.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Verdict: REQUEST CHANGES

Reviewed: exact head 22783090fde967ca6dd252334c797f6b6b8ddbfa against exact base / merge-base 2edacde4d4c01490834725774aa878dbc373c41d.

The three blockers from my prior review are fixed on this head: ordinary and Projects conversation panels now propagate terminal query state and Retry, successful-header body stalls retain timeout classification, and the asynchronous error card is an alert live region. One deadline path remains incomplete.

Blocking finding

[P2] Non-2xx response-body stalls discard the timeout classification

The new per-request deadline covers the full /query request, but send_query_request sends every non-success response to relay_error_message (desktop/src-tauri/src/relay.rs:411-419). That helper consumes the response with response.text().await.unwrap_or_default() (relay.rs:272-290). If a relay sends 500 or 429 headers and then stalls its declared body past the deadline, the timed-out reqwest::Error is discarded; the caller receives relay returned 500 or a generic rate-limit message rather than the promised stable relay unreachable: request timed out classification.

The new body-stall regression covers only a successful 2xx response parsed through parse_json_response (desktop/src-tauri/src/relay/tests.rs:325-375), so this branch remains untested. Please preserve is_timeout() when consuming error bodies and add a non-2xx stalled-body regression through send_query_request.

Non-blocking follow-ups

• useThreadRepliesForRoots still drops aggregate terminal errors, so Huddle and Projects-agent transcripts can silently omit a failed reply subtree. This behavior predates the PR and is distinct from the dedicated panel's false-empty card, so I am not blocking this diff on it.
• A deep-linked thread whose head is absent from the channel window can remain on the head-resolution skeleton independently of the reply-query error. That separate head-load path is also outside this PR's stated fix.

Validation

• Exact head remained unchanged and git diff --check is clean.
• Applicable GitHub CI is green, including Desktop Core and all four Desktop Smoke E2E shards.
• I did not duplicate CI-equivalent broad suites locally.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Verdict: REQUEST CHANGES

Reviewed: exact head be94db695bd6c21e5c1d12827ce1db4d474a97fd against exact base / merge-base 2edacde4d4c01490834725774aa878dbc373c41d.

The prior /query timeout/classification blockers and the Projects conversation failure/retry wiring are fixed on this head. One multi-root consumer still violates the new aggregate error contract.

Blocking finding

[P2] Huddle transcripts still silently omit a failed reply subtree

combineThreadRepliesResults now preserves successful events while setting aggregate isError and exposing a failed-only refetch when any root query fails (desktop/src/features/messages/useThreadReplies.ts:142-161). Its stated contract is that a multi-root consumer must not silently present a partial transcript as complete.

The Huddle path consumes the same aggregate at desktop/src/features/channels/ui/useHuddleChannelMessages.ts:54-69, but reads only .events and returns only { resolvedMessages, threadSummaries }. ChannelScreen therefore receives no Huddle reply error or retry state (desktop/src/features/channels/ui/ChannelScreen.tsx:245-251). If one summarized root times out while another succeeds, that root contributes zero rows; the remaining channel messages and successful reply subtrees render with no warning and no recovery action. The Projects-agent consumer correctly renders ThreadRepliesErrorCard beside partial rows, but the other direct consumer remains unwired.

Please propagate the aggregate error/refetch state into the mounted Huddle transcript, show a non-destructive retry alert alongside any successful rows, and add a consumer-level regression. The new aggregate unit test proves the hook reports failure; it cannot catch a consumer discarding that report.

Validation and non-blocking notes

• Rust /query construction, request deadline, header/body timeout classification for 2xx and non-2xx responses, and cancellation trace correctly. Exact-head Desktop Core, Rust, unit, mobile, integration, and three smoke shards passed.
• Desktop Smoke E2E shard 3 failed only in the unrelated existing message-feedback-snapshots.spec.ts profile-hover CSS assertion after all retries; the new Projects failure E2E passed. The aggregate Desktop job merely reflects that shard failure.
• Cached single-root rows remain visible after a failed background refetch without an inline stale-data warning. Preserving the rows is an explicit behavior already covered by tests; a warning/Retry would be safer but is not a blocker for this fix.
• Current fetched origin/main is d97780b4777f2fe3430b4e30a7d47fc6837ee059; git merge-tree --write-tree HEAD origin/main succeeds, and git diff --check is clean at the reviewed head.