fix(desktop): recover both boundaries from assistant-ui lookup races, reactive edit context - #81232
Merged
Merged
Conversation
…poser context Two correctness holes left by the session-switch perf work (#72504 / #72524): 1. MessageRenderBoundary only cleared a swallowed transient useClientLookup error when the structural resetKey changed. Mid-turn, ids/roles/count are stable, so a lookup race during a stream left the boundary rendering null for the rest of the turn. The boundary now self-retries on a 0ms timer (rAF never fires in a parked renderer), bounded to 5 consecutive transient catches with the budget reset on recovery; the structural resetKey path is unchanged, and non-transient errors still re-throw. 2. cwd / gateway / sessionId were removed from the messageComponents memo deps and read through a render-time ref so session switches stop reminting the component types. But a mounted UserEditComposer only reads that ref when it renders, and a same-session change (cwd remap, gateway reconnect) leaves every ThreadMessageList prop referentially equal, so the memo'd list bails out and the open composer keeps stale values: @-completions, slash completions, and OS-drop uploads target the old cwd / gateway / session. Thread now provides the three values through a memoized ThreadEditContext; context propagates through the bail-out, the component type identity is untouched, and the transcript never remounts.
Contributor
૮ >ﻌ< ა ci reviewran on 24b7ca7
|
This was referenced Aug 7, 2026
alt-glitch
added
type/bug
OutThisLife
enabled auto-merge
August 7, 2026 17:43
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…undary-recovery-triage fix(desktop): recover both boundaries from assistant-ui lookup races, reactive edit context
33hodl
pushed a commit
to 33hodl/hermes-agent
that referenced
this pull request
Aug 12, 2026
…undary-recovery-triage fix(desktop): recover both boundaries from assistant-ui lookup races, reactive edit context
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Consolidates the two open PRs fixing the same assistant-ui lookup race into one
change, because they fix that race at two complementary layers and collide on a
shared test file.
@assistant-ui/store's index-keyed child-scope lookup throws — rather thanreturning
undefined— when a subscriber reads an index the message/parts listno longer has. It races during high-frequency store replacement (session switch
mid-stream, gateway reconnect replay, post-compaction list shrink). On current
mainthat race has two ways to blank the UI, and neither self-heals:1. The message-local boundary strands a turn mid-stream.
MessageRenderBoundaryswallows the transient throw and rendersnulluntilresetKeychanges. Since #72504 that key is deliberately structural-only(
list.tsxstructuralSignature) — the right call, it stopped per-tokenreconciles of every turn — but that removed the implicit recovery: mid-turn the
structure is stable, so a race during a stream leaves the turn blank until an
unrelated message add/remove. The boundary now self-retries on a 0ms timer
(rAF never fires in a parked renderer), bounded to 5 consecutive transient
catches with the budget reset on successful recovery. A persistent failure falls
back to the old wait-for-structure behavior, and non-transient errors still
re-throw to the root.
2. What escapes the local boundary strands the whole app. The root
ErrorBoundaryonly logs incomponentDidCatch, so the root fallback sticksuntil the user restarts the app. Root recovery for exactly this error family
existed (added in
2e3efce66, merged as #52704) and was removed by344415892.This restores it, narrowly: root-label-gated, matching only the lookup
out-of-bounds family, bounded to 3 attempts per 5s window, with persistent
failures still visible and manually retryable. Also fixes a
StrictModelifecycle bug found while exercising the real root composition — the synthetic
unmount cleared the timer scheduled by
componentDidCatchand the replayedmount never restored it, so recovery ownership is now tracked separately from
the timer handle.
Both layers share one classifier,
/(useClientLookup|tapClient(Lookup|Resource)).*out of bounds/.The current production error name is
useClientLookup(renamed upstream in@assistant-ui/store@0.2.19); the legacytapClient*names stay covered.3. An open edit composer keeps a stale cwd / gateway / sessionId. #72524
moved those three values out of the
messageComponentsmemo deps into arender-time ref so session switches stop reminting the component types — load
bearing on the hot path. But a composer that is already open never re-reads
the ref: a same-session change (the agent relocating the session's cwd via
session.info, a gateway reconnect) leaves everyThreadMessageListpropreferentially equal, so the memo'd list bails out and
@-completions, slashcompletions, and OS-drop uploads act on the old context.
Threadnow providesthe three values through a memoized
ThreadEditContext. Context propagatesthrough the bail-out, component type identity is untouched, and the transcript
never remounts.
Related Issue
Fixes #72866
Fixes #64308
Supersedes #72867
Supersedes #64310
Related to the broader assistant-ui race tracked by #45403.
Type of Change
Changes Made
apps/desktop/src/components/assistant-ui/message-render-boundary.tsx:bounded timer self-retry for transient lookup errors, budget reset on
recovery, timer cleared on unmount and on structural reset.
apps/desktop/src/components/error-boundary.tsx: root-only bounded autorecovery for the same error family, recovery ownership preserved across a
StrictModesynthetic unmount without weakening real-unmount cleanup,window reset at the 5s boundary, manual reset cancels an active timer and
starts a fresh budget,
RootErrorBoundarycomposition so only the real rootowner auto-recovers.
apps/desktop/src/main.tsx: usesRootErrorBoundaryat the Desktop root.apps/desktop/src/components/assistant-ui/thread/index.tsx:editContextRefreplaced with a memoized
ThreadEditContextprovided around the list; theUserEditComposerwrapper reads it viauseContext. Comments explain whyneither memo deps nor a ref can carry these values.
resetKeychange, retry cap, budget resetacross streaks, no retry for non-transient errors, classifier parity across
useClientLookup/tapClientLookup/tapClientResource, root transientrecovery, persistent budget exhaustion, 5s window reset, manual reset with an
active timer, pending-timer unmount cleanup, scoped-boundary exclusion, real
root composition under
StrictModeand production-style without it,same-session cwd change reaching a mounted composer, session-switch control,
and a perf invariant proving a cwd rerender remounts no transcript DOM node.
How to Test
Proof of the bugs on
main: with the three source files reverted, thetimer-recovery, budget-reset, root-recovery, and same-session-cwd tests fail.
Credit
Consolidated from two independent contributions, authorship preserved via
cherry-pick:
ThreadEditContext(originally fix(desktop): self-retry transient boundary errors, reactive edit composer context #72867).
StrictModetimerownership fix (originally fix(desktop): recover root boundary from assistant-ui lookup races #64310).
tapClientLookupname while the live production error isuseClientLookup,with a real production trace (496-message session, active list shrank to 12,
stale subscriber read index 89). That review is why both layers now share one
classifier.
The only edit on top of their commits was resolving an additive conflict in
message-render-boundary.test.tsx, where both PRs added distinct test helpers —both are kept.
Checklist
Code
is the consolidation of the two that existed
Documentation & Housekeeping
cli-config.yaml.exampleN/A — no config keys changedCONTRIBUTING.md/AGENTS.mdN/A — no architecture changebrowser timers only; no platform API