Skip to content

fix(desktop): isolate preview artifact chips per session view - #66578

Closed
stantheman0128 wants to merge 3 commits into
NousResearch:mainfrom
stantheman0128:fix/66411-multi-session-preview-isolation
Closed

stantheman0128 wants to merge 3 commits into
NousResearch:mainfrom
stantheman0128:fix/66411-multi-session-preview-isolation

Conversation

@stantheman0128

Copy link
Copy Markdown
Contributor

Summary

Closes #66411

Credit: Stan Shih (@stantheman0128). Developed with AI assistance (Cursor / Grok).

In multi-session Desktop split, tool fallback recorded HTML/localhost preview chips under the global \. Both panes stay mounted, so focusing session A while session B's tool rows were still mounted re-keyed B's artifacts into A's composer status stack.

The composer already reads \[sessionId] per pane. This PR makes the write path match: ToolFallback uses useSessionView() (\ / \) instead of the global active session stores.

Verification

`
cd apps/desktop
npx vitest run --project ui src/components/assistant-ui/tool/fallback-preview-isolation.test.tsx src/components/assistant-ui/tool/fallback.test.ts src/components/assistant-ui/tool/fallback-model.test.ts

Test Files 3 passed | Tests 31 passed

`

`
python scripts/check-windows-footguns.py --diff HEAD~1

No Windows footguns found

`

Regression covers: tile view session-b with global active session-a records only under session-b.

Made with Cursor

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 17, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Comment

Critical

  • None

Assessment

Adds contributor email record for TheUnathiCodex and a test regression file for the TUI gateway queue-on-busy behavior. Tests verify that busy submit correctly interrupts, queues, or steers based on the configured mode.

Looks Good

  • New contributor added to emails directory
  • Tests cover interrupt/queue/steer modes for busy state handling
  • _session(agent=agent, running=True) correctly simulates a running session

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused Desktop regression fix. The premise is confirmed on current main: ToolEntry writes preview artifacts using the global $activeSessionId and $currentCwd at apps/desktop/src/components/assistant-ui/tool/fallback.tsx:313-323, while tiles provide their own session view to the ChatView subtree at apps/desktop/src/app/chat/session-tile.tsx:135-163.

The proposed useSessionView() write path matches the existing per-session read path in apps/desktop/src/app/chat/composer/status-stack/index.tsx:79, and the added test exercises the primary/tile identity mismatch that causes the leak. The only recordPreviewArtifact writer is this fallback path, so no sibling writer remains unaddressed.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 18, 2026
@stantheman0128
stantheman0128 force-pushed the fix/66411-multi-session-preview-isolation branch from 8530d36 to 915050d Compare July 21, 2026 09:28
@stantheman0128

Copy link
Copy Markdown
Contributor Author

Thanks for the overlapping work on #68683.

That PR covers the same #66411 chip isolation case and also owns dismissal persistence across session switch / compress / restart via a session-owned publisher. Ours is the narrower write-path fix only.

Happy to close this in favor of #68683 once that one lands (or sooner if maintainers prefer). Thanks @nv-cho for the fuller treatment and for the credit note.

@stantheman0128

Copy link
Copy Markdown
Contributor Author

Closing as author to bring our open PRs on hermes-agent back within a healthy throttle (we had 10 open with only sweeper keep_open and no concrete maintainer change requests for days).

Keeping three Windows-focused PRs open for now:

Happy to reopen this one if a maintainer wants it prioritized. Thanks for the patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop multi-session — preview artifact chips from one session appear in another session's composer status stack

4 participants