fix(desktop): recover root boundary from assistant-ui lookup races

[frizikk]

What does this PR do?

Restores a narrow, bounded recovery path for the Desktop root error boundary when the known transient assistant-ui lookup race escapes the existing message-local boundary.

The root boundary recognizes the same current-and-legacy error family as MessageRenderBoundary:

/(useClientLookup|tapClient(Lookup|Resource)).*out of bounds/

Eligible root failures retry on the next timer turn so React can reconcile against the current store snapshot. Recovery remains intentionally limited to three automatic attempts in a five-second window; persistent failures remain visible and manually retryable.

This refresh also fixes a React development StrictMode lifecycle bug found while exercising the real root composition: the synthetic unmount cleared the timer scheduled by componentDidCatch, but the replayed mount did not restore it. The boundary now retains recovery ownership separately from the timer handle and reschedules only that owned attempt on remount. Real unmount still cancels the timer.

Rebased exactly onto origin/main 05f5df6bdb7cfcec57aadc9ac93d7a08c684cd5d. Local immutable candidate: 6800a1399434e06e733f18068177df974e52a3b7.

Related Issue

Fixes #64308. Related to the broader assistant-ui race tracked by #45403.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• apps/desktop/src/components/error-boundary.tsx
  • matches only the current/legacy assistant-ui lookup out-of-bounds family;
  • preserves an owned recovery across a StrictMode synthetic unmount without weakening real-unmount cleanup;
  • resets the bounded recovery window at the exact five-second boundary;
  • keeps manual reset responsible for cancelling an active timer and starting a fresh budget;
  • exposes the narrowly scoped RootErrorBoundary composition so only the real root owner can auto-recover.
• apps/desktop/src/main.tsx
  • uses RootErrorBoundary in the real Desktop root under StrictMode.
• apps/desktop/src/components/error-boundary.test.tsx
  • covers transient recovery, persistent budget exhaustion, five-second window reset, manual reset with an active timer followed by renewed exhaustion, pending-timer unmount cleanup, scoped-boundary exclusion, classifier near misses, real root composition under test/development StrictMode, and production-style composition without StrictMode.
• apps/desktop/src/components/assistant-ui/message-render-boundary.test.tsx
  • proves classifier parity for useClientLookup, tapClientLookup, and tapClientResource without introducing a shared speculative classifier framework.

How to Test

1. Install committed workspace dependencies in development mode.
2. Run the focused root and message-local boundary tests.
3. Run Desktop typecheck and ESLint on the touched TypeScript files.
4. Run the production Desktop build and final diff/status checks.

Commands run locally:

NODE_ENV=development npm ci --include=dev

NODE_ENV=test NODE_OPTIONS='--max-old-space-size=8192 --localstorage-file=/tmp/hermes-pr-64310-focused-final.json' \
  npm --workspace apps/desktop exec -- vitest run --project ui \
  src/components/error-boundary.test.tsx \
  src/components/assistant-ui/message-render-boundary.test.tsx

NODE_ENV=test npm --workspace apps/desktop run typecheck

NODE_ENV=test npm --workspace apps/desktop exec -- eslint \
  src/components/error-boundary.tsx \
  src/components/error-boundary.test.tsx \
  src/components/assistant-ui/message-render-boundary.test.tsx \
  src/main.tsx

npm --workspace apps/desktop exec -- prettier --check \
  src/components/error-boundary.tsx \
  src/components/error-boundary.test.tsx \
  src/components/assistant-ui/message-render-boundary.test.tsx \
  src/main.tsx

NODE_ENV=production npm --workspace apps/desktop run build

git diff --check 05f5df6bdb7cfcec57aadc9ac93d7a08c684cd5d..HEAD
git status --porcelain=v1

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate — not repeated during this local-only refresh of the existing PR
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — N/A for this Desktop-only TypeScript change; focused Vitest, typecheck, ESLint, Prettier, and production build passed
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: CachyOS Linux 7.1.5, Node.js 25.4.0, npm 11.7.0

Documentation & Housekeeping

• Relevant documentation N/A — no user-facing workflow or configuration changed
• cli-config.yaml.example N/A — no configuration changed
• CONTRIBUTING.md / AGENTS.md N/A — no architecture or contributor workflow changed
• Cross-platform impact considered — browser timer and React boundary behavior are renderer-platform independent
• Tool descriptions/schemas N/A — no tool behavior changed

For New Skills

N/A — this PR does not add a skill.

Screenshots / Logs

• StrictMode red proof before the lifecycle fix: focused root test file reported 1 failed, 7 passed; the fallback remained visible because the synthetic unmount had cleared the recovery timer.
• Final focused boundary tests: 2 files passed, 18 tests passed.
• Desktop typecheck: passed.
• ESLint on all four touched TypeScript/TSX files: passed with no warnings.
• Prettier check on all four touched TypeScript/TSX files: passed.
• Production build for exact candidate 6800a1399434e06e733f18068177df974e52a3b7: passed; assert-dist-built confirmed renderer assets and the build stamp recorded 6800a1399434.
• Final diff check: passed; Git porcelain is empty.
• Non-blocking inherited build warnings: deprecated advancedChunks, one generated-CSS optimizer warning, the existing Tabler large-barrel warning, and an ineffective dynamic-import warning for open-session.ts.
• npm ci reported 28 audit findings in the resolved dependency graph; no dependency or lockfile changes are included in this PR.
• Full Desktop UI/Electron suites were not rerun for this local refresh; the required focused lifecycle matrix and prescribed typecheck/lint/build gates above are the evidence for this candidate.

[teknium1]

Thanks for restoring this narrowly. Current main still reproduces the regression: apps/desktop/src/components/error-boundary.tsx:30-34 only logs a caught error, and render() then retains the root fallback at :47-51; the Desktop root mounts this boundary at apps/desktop/src/main.tsx:37.

The prior bounded recovery was added in 2e3efce66 / merged as #52704, then deleted by 344415892. The PR restores only the exact reported tapClientLookup path, keeps the retry root-only and bounded, and covers transient, persistent, scoped, and unrelated-error behavior. Its base boundary blob equals current main (87b6b7743c511e6a572180cd3f75147efaddddd1), so salvage should be clean.

Automated hermes-sweeper review.

[patrykkopycinski]

The recovery regex tapClientLookup doesn't match the current @assistant-ui error name. The function was renamed to useClientLookup — in @assistant-ui/store@0.2.19 (the version in this repo's package.json) there are zero occurrences of tapClientLookup in the dist bundle; the error is now useClientLookup: Index N out of bounds (length: M).

The parametrized test does not auto-recover %s at root actually asserts that useClientLookup: Index 6 out of bounds (length: 2) falls through to the fallback — the opposite of what should happen, since that's the live production error name.

The inner MessageRenderBoundary already handles this correctly by matching both names:

/(useClientLookup|tapClient(Lookup|Resource)).*out of bounds/

The root boundary should use the same pattern. A production crash from a session that hit context compression (496-message session, active list shrank to 12, stale subscriber read index 89) confirms the useClientLookup variant escapes the inner boundary and reaches the root, where this PR's regex wouldn't match it.

[frizikk]

Thanks — you're right. I verified the installed @assistant-ui/store@0.2.19 bundle directly: its out-of-bounds throw is now useClientLookup: Index ... out of bounds, while this PR's root matcher and one negative test still assumed the legacy name.

I've updated the PR to:

• match the same current-and-legacy error family as MessageRenderBoundary (useClientLookup, tapClientLookup, and tapClientResource);
• make useClientLookup the primary production regression case;
• retain coverage for both legacy names;
• keep non-bounds lookup errors and unrelated errors on the normal fallback.

A mutation check that restores the old legacy-only matcher now fails the two current-name assertions, and the corrected focused suites pass 11/11. I also rebased the PR onto current main and reran Desktop typecheck, changed-file lint/formatting, Electron tests, and the production build.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address the stale assistant-ui lookup race or its root-boundary fallout: #41787 introduced next-tick root recovery, #52704 bounded that recovery, and #64310 restores and updates it after its removal from main. The contributor evidence identifies #44562 as another occurrence of the lookup-index race, rather than a malformed tool-result failure.

Related pull requests

• fix(desktop): recover root boundary from transient render races #41787 [closed] duplicate — (+148/-0) — n/a: Adds root-only next-tick recovery for tapClientLookup and two related transient renderer errors, plus a stale-index regression test; it remains relevant as the original mitigation but was closed after contributor review documented its supersession by bounded salvage fix(desktop): recover root error boundary from transient render races (salvage #41787) #52704.
• fix(desktop): recover root error boundary from transient render races (salvage #41787) #52704 [merged] best fix — (+183/-0) — n/a: Extends fix(desktop): recover root boundary from transient render races #41787 with at most three root recovery attempts per five-second window and tests transient, persistent, scoped, and unrelated failures; this merged reference implementation is still relevant because its recovery was later removed from main by 3444158.
• fix(desktop): recover root boundary from assistant-ui lookup races #64310 best fix — (+302/-5) — n/a: Restores bounded root-only recovery on the affected main lineage, matches current useClientLookup plus legacy tapClientLookup/tapClientResource errors, and adds StrictMode timer-ownership and production-root regression coverage. This supports the recorded best-fix finding for [Bug]: Desktop root tapClientLookup recovery was removed from main #64308 and the maintainer-bot keep_open review.

Duplicates

#41787 and #52704 implement substantially the same root-boundary mitigation, with #52704 superseding #41787 by bounding retries; #64310 restores and updates the #52704 approach after its removal from main.

Suggested consolidation

Keep #64310 open with a salvage path, consistent with the maintainer-bot keep_open verdict: retain its bounded root-only recovery, current-and-legacy lookup matcher, StrictMode handling, and focused regression tests. Keep #41787 closed as superseded by #52704, retain merged #52704 as the reference implementation, and consolidate #44562 under canonical race issue #45403 while #64308 tracks the removal regression.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I41693(["issue #41693 (closed)"])
    I44562(["issue #44562 (open)"])
    I64308(["issue #64308 (open)"])
    subgraph Dup41787 ["PRs duplicating each other"]
        P41787["PR #41787 (closed)"]
        P52704["PR #52704 (merged)"]
        P64310["PR #64310 (open)"]
    end
    P64310 -->|fixes| I41693
    P64310 -->|fixes| I44562
    P64310 -->|best fix| I64308
    class I41693 closed
    class I44562 open
    class I64308 open
    class P41787 closed
    class P52704 merged
    class P64310 open
    class P52704 best
    class P52704 best
    class P64310 best
    class P64310 target
    click I41693 "https://github.com/NousResearch/hermes-agent/issues/41693"
    click I44562 "https://github.com/NousResearch/hermes-agent/issues/44562"
    click I64308 "https://github.com/NousResearch/hermes-agent/issues/64308"
    click P41787 "https://github.com/NousResearch/hermes-agent/pull/41787"
    click P52704 "https://github.com/NousResearch/hermes-agent/pull/52704"
    click P64310 "https://github.com/NousResearch/hermes-agent/pull/64310"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 28 kB of PR diffs, 26 kB of issue/PR text, 16 kB of discussion (14 comments), 14 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[OutThisLife]

Superseded by #81232.

Consolidated with #72867 into one PR: both fix the same useClientLookup ... out of bounds store race at complementary layers, and the two branches conflict on message-render-boundary.test.tsx. All three of your commits were cherry-picked, so authorship is preserved in git history, and you're credited in the new PR body.

Your root-boundary recovery and the StrictMode timer-ownership fix carried over unchanged, as did the shared classifier after @patrykkopycinski's catch on the tapClientLookup vs useClientLookup rename. Thanks for turning that review around so cleanly.