fix(desktop): isolate review submit by composer surface

[unsupportedpastels]

Proposed priority: P1

This is a cross-session action-isolation failure. A single Review-pane action can submit the same commit/push/PR instruction to multiple open sessions and projects, potentially causing unintended commits and pull requests in unrelated worktrees.

Summary

• Prevent Review-pane agent-ship actions from fanning out across mounted session composers.
• Bind external submit requests to the exact visible composer surface present at click time.
• Preserve tile/project routing when Review is opened from a session tile or changed-files card.
• Fail closed when the originating composer surface is missing, hidden, or remounted.
• Preserve Review origin through narrow-pane toggles, pane restoration, and focus-pane opens.

Root cause

The composer submit bus is window-global because inactive session panes remain mounted. Every composer subscribed to the bus, but the handler did not sufficiently bind a request to the visible composer that initiated the action. A Review-pane request targeting main could therefore be claimed by multiple mounted composers.

The Review pane also tracked only its repository cwd, so a tile-scoped review could lose the session/project that opened it and route the agent-ship prompt back to the main composer.

Fix

• Capture a unique composer surface ID with useId() for each mounted ChatView.
• Dispatch external submit requests synchronously and validate the requested target/surface against visible panes.
• Use useLayoutEffect and pane visibility checks so hidden keep-alive panes cannot claim requests.
• Store the Review pane's originating cwd, composer target, and exact surface ID.
• Route Review actions from coding rails and changed-files cards to the originating tile.
• Disable agent-ship when the origin is unavailable after reload or remount.
• Re-home the origin on narrow overlay toggles and preserve it when restoring the Review pane.

Relationship to existing work

This PR is a proposal for maintainer evaluation alongside #85911. It carries forward the original review-submit isolation direction while adding exact originating-surface binding, project-aware Review routing, reload handling, and narrow-pane safeguards. The goal is to provide a concrete candidate for review, not to make a claim about the disposition of #85911.

Credit to @Youtiaowei for the original implementation and analysis in #85911.

Test plan

• npx vitest run --project ui — 539 files, 5,042 tests passed
• Focused composer/review tests — 77 tests passed
• npm run typecheck
• ESLint on changed files
• Prettier check on changed files
• git diff --check

[andrexibiza]

Reviewed exact head 56175a5993b3cc9bef2ac0ed11c32ac21855815c against current main b5455fdd16fe608214f91149233660e1836b067c (the PR is directly based on current main). There were no existing formal reviews, inline threads, or PR discussion on this head.

The core isolation shape is right: capture a mounted ChatView surface identity synchronously at click time, require {target, surfaceId, paneVisible, !inputDisabled} at the receiver, and fail closed if the concrete surface cannot be proven. The per-surface useId() rather than session ID is important because fresh chats have no durable ID and the same stored session can legitimately be mounted more than once. The broader addition over #85911—carrying Review's originating target/worktree so a tile-scoped review returns to that tile—is also the correct next boundary.

I found one merge blocker in that new boundary:

Blocker: the normal wide-layout toggleReview() path drops the exact surface identity

ChatBar now calls:

toggleReview(scope.target === 'main' ? null : (cwd ?? null), scope.target, composerSurfaceId)

and toggleReview() correctly accepts all three pieces of origin proof. The narrow/collapsed branch preserves them, and the new test explicitly covers that branch. But the ordinary non-collapsed branch ends with:

if (isPaneVisible(REVIEW_PANE_ID)) {
  closeReview()
} else {
  revealReview(scopeCwd, scopeTarget)
}

The third argument is omitted. revealReview() therefore receives scopeSurfaceId = null, persists $reviewScopeSurfaceId = null, and ReviewShipBar then disables the agent-ship action via disabled={!hasFiles || !scopeSurfaceId}.

So on the normal wide Desktop layout, opening Review from the very ChatBar/coding rail this PR is trying to bind precisely loses the new authority at the last façade boundary and makes “Let Hermes commit and open a PR” unavailable. The narrow overlay path works; the wide path does not. This is exactly the proof-discard class the PR is otherwise closing.

Required fix: forward scopeSurfaceId in the wide revealReview(...) call and add the symmetric non-collapsed regression. The current review.test.ts only exercises matchesQuery(...) === true, which is why this escaped.

Topology / attribution

• #86280 by @Youtiaowei is the source defect: a Review submit can cross a keep-alive session boundary and execute commit/PR work in the wrong context.
• #85911 by @Youtiaowei is the narrower existing implementation of the synchronous surface-capture / exact receiver gate. It already received positive review on that core mechanism. #90097 is best treated as a broader superseding candidate, not an unrelated duplicate: it carries the same isolation primitive forward and adds tile/project Review-origin routing, reload/remount fail-closed behavior, and pane-restoration handling. Please preserve that provenance if #90097 is the one that lands.
• #66661 is complementary rather than duplicate: it addresses rejected-submit text being restored into the wrong composer after dispatch, while this PR governs which composer may accept the Review submission in the first place.

The remaining architecture I checked is coherent: ComposerSurfaceProvider is above the ChatView/composer pair; useLayoutEffect closes the stale-listener window; explicit surfaceId requests revalidate visibility before dispatch; Review ship uses the stored exact {target, surfaceId} rather than re-inferring an owner at click time; and pane-focus restoration binds a currently visible main surface instead of broadcasting.

Validation state

Author-reported Desktop validation is strong (5,042 UI tests plus focused composer/review tests, typecheck, ESLint, Prettier, diff check). Exact-head Docker and Nix are green. The main CI workflow concluded failure but produced zero jobs, so there is no executed hosted CI job matrix to interpret as a code regression or as approval evidence.

Re-review gate: forward the exact surface through the wide toggleReview branch, add the missing wide-layout regression, and attach an executed exact-head Desktop/CI matrix if the hosted workflow becomes available.

[unsupportedpastels]

Thanks @andrexibiza — the blocker is confirmed and fixed.

Blocker (wide-layout toggleReview): the non-collapsed branch called revealReview(scopeCwd, scopeTarget) without the third argument, persisting $reviewScopeSurfaceId = null and disabling agent-ship on the normal wide layout. Fixed in d34c4f5 by forwarding scopeSurfaceId through that branch, and the symmetric wide-layout regression is added next to the narrow-overlay one (matchesQuery → false, tree-backed path, asserts all three origin pieces survive).

Also rebased onto current main (b5455fd).

Re-verified at the new head:

• Full desktop UI suite: 539 files / 5,043 tests passed
• Focused composer/review tests: 78 passed
• npm run typecheck, ESLint, Prettier, git diff --check — all clean

On topology/attribution: agreed on all three points — #86280 as the source defect, #85911 as the original implementation of the isolation primitive, and #66661 as complementary. Happy for the maintainers to treat this as the broader candidate carrying that provenance forward.

[OutThisLife]

Superseded by #90113.

That salvage keeps the surface-id isolation and tile origin target, rebased onto current main, and:

• captures the live composer surface at click time instead of persisting a useId()
• toasts when the originating chat isn't on screen, instead of a silent no-op
• restores the tab-switch and new-session fan-out tests from fix(desktop): isolate review submit to active composer #85911

You're credited via Co-authored-by. Thanks for carrying #85911 forward and adding project-aware Review routing.