Skip to content

fix(desktop): isolate review submit to one composer (supersedes #90097, #85911) - #90113

Merged
OutThisLife merged 2 commits into
mainfrom
bb/review-submit-isolation
Aug 19, 2026
Merged

fix(desktop): isolate review submit to one composer (supersedes #90097, #85911)#90113
OutThisLife merged 2 commits into
mainfrom
bb/review-submit-isolation

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

Summary

Supersedes #90097 and consolidates #85911.

Clicking Ask Hermes to open PR in the Review pane dispatched a window-level composer submit with target: 'main'. Every mounted composer accepted that event, so one click could commit and open a PR in every open session and project with dirty files.

#85911 had the right isolation (surface id, sync dispatch, visible pane) but is ~1600 commits behind and still hardcoded ship to main. #90097 rebased that and added tile origin tracking, but persisted a useId() on the Review store and required that exact surface to still be visible, so a remount or tab switch left the button enabled and the click did nothing.

This keeps the isolation, remembers which composer target opened Review, captures the live surface at click time, and toasts if that chat isn't on screen.

What this PR does

  • Pin submit to one visible composer surface
  • Route Review agent-ship to the originating composer target (tile vs main)
  • Fail closed with a toast when no reachable composer

What was dropped from #90097

  • Persisted $reviewScopeSurfaceId / useId() on the Review store

Test plan

  • npx vitest run --project ui src/app/chat/composer/hooks/use-composer-submit.test.tsx src/store/review.test.ts — 66 passed
  • Manual: two sessions + a project tile with dirty files; Ask Hermes to open PR ships only the origin chat

Closes #86280

Credit: @unsupportedpastels, @Youtiaowei

OutThisLife and others added 2 commits August 19, 2026 11:01
Review "Ask Hermes to open PR" was a window-level event that every mounted
composer claimed with `target === 'main'`, so one click shipped every open
session and project with dirty files. Bind the request to the visible
surface captured at click time.

Co-authored-by: unsupportedpastels <theoldwizard123@pm.me>
Co-authored-by: youtiaowei <youtiaowei@users.noreply.github.com>
The ship button always targeted `main`, so a tile Review still prompted
the workspace session. Remember the originating composer target with the
pane's cwd, capture the live surface at click, and toast if that chat
isn't on screen instead of dropping the click.

Co-authored-by: unsupportedpastels <theoldwizard123@pm.me>
Co-authored-by: youtiaowei <youtiaowei@users.noreply.github.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 19, 2026
@OutThisLife
OutThisLife merged commit 28803e6 into main Aug 19, 2026
9 checks passed
@OutThisLife
OutThisLife deleted the bb/review-submit-isolation branch August 19, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists 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.

[desktop] Review 'commit and open PR' can route to wrong keep-alive session

2 participants