Skip to content

TerminalApp: foreground target window in FocusProtocolPane#353

Merged
DDKinger merged 1 commit into
mainfrom
dev/yuazha/session-mgmt-crosswindow-focus
Jun 24, 2026
Merged

TerminalApp: foreground target window in FocusProtocolPane#353
DDKinger merged 1 commit into
mainfrom
dev/yuazha/session-mgmt-crosswindow-focus

Conversation

@DDKinger

@DDKinger DDKinger commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Cross-window focus_pane (Enter on a Live session in the session-management view) could fail to bring the target window to the foreground when that window already had the target pane focused.

Repro

  1. Two windows: window1 and window2.
  2. window1 hosts session1 (in one of its panes); window2 hosts session2.
  3. From window2's session-management view, press Enter on session1.
  • ❌ When window1 was already focused on session1's own pane (e.g. the top shell pane), window1 did not come forward.
  • ✅ When window1 was focused on a different pane (e.g. the agent pane), it worked.

Root cause

TerminalPage::FocusProtocolPane only moved XAML focus within the target window via _SetFocusedTab / FocusPane. Those no-op when the pane is already focused, and the OS window previously only came forward as an accidental side effect of the focus actually transitioning. When the target pane was already focused, no transition happened → no activation → the window stayed in the background.

Fix

Raise SummonWindowRequested once the target pane is found, mirroring the desktop-notification activation path in TabManagement.cpp. The window is then reliably brought to the foreground regardless of its prior focus state. SummonWindow(toggleVisibility=false)_globalActivateWindow performs the activation; it is intra-process (the COM server runs inside WindowsTerminal.exe), so no AllowSetForegroundWindow is required.

Test plan

  • Full solution build (Debug/x64): 0 Error(s); WindowsTerminal.exe + CascadiaPackage link and pack cleanly.
  • Runtime: two windows; Enter on a session whose pane is already focused in its window → window surfaces and the pane is focused.

focus_pane (Enter on a Live session in the session management view) could fail to surface the target window when that window already had the target pane focused. FocusProtocolPane only moved XAML focus *within* the window via _SetFocusedTab/FocusPane — which no-op when the pane is already focused — and relied on the focus transition as an accidental side effect to bring the OS window forward.

Symptom: focusing session1 (in window1) from window2 worked only when window1's focus was on a different pane (e.g. the agent pane), but not when window1 was already focused on session1's own pane.

Fix: raise SummonWindowRequested once the target pane is found, mirroring the desktop-notification activation path in TabManagement.cpp, so the window is reliably brought to the foreground regardless of its prior focus state.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 24, 2026 03:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes cross-window focus_pane activation so that focusing a protocol pane in another window reliably brings the target OS window to the foreground even when that window already had the target pane focused (i.e., when intra-window XAML focus operations would otherwise no-op).

Changes:

  • Raise SummonWindowRequested inside TerminalPage::FocusProtocolPane once the target pane is located, ensuring OS-level window activation.
  • Preserve the existing tab/pane focusing logic (including the stashed agent-pane restore path) after summoning.

@DDKinger DDKinger merged commit 1f205dd into main Jun 24, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants