Skip to content

🤖 fix: a pop-out terminal's exit closes only its own window - #5749

Merged
ThomasK33 merged 3 commits into
mainfrom
fix/popout-exit-closes-own-window
Oct 6, 2026
Merged

ThomasK33 merged 3 commits into
mainfrom
fix/popout-exit-closes-own-window

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

Summary

In the desktop app, when the shell in one pop-out terminal window exits, only that window closes now. Other pop-out terminal windows of the same workspace stay open.

Fixes #5739

Background

terminal-window.tsx called terminal.closeWindow({ workspaceId }) when its shell exited, and TerminalWindowManager.closeTerminalWindow(workspaceId) closed every pop-out of that workspace. Since #5732, those windows keep their sessions (app-closed windows are not user closes), but they were still hidden, and their shells came back as sidebar tabs on the next load.

Implementation

  1. terminal.closeWindow takes an optional sessionId (.nullish()). The pop-out window sends its own session when its shell exits.
  2. A new Electron-free TerminalWindowRegistry (src/desktop/terminalWindowRegistry.ts) records which session each pop-out window shows, and selects the windows to close: only the one showing sessionId, or all of them when the field is absent (unchanged for other callers).
  3. TerminalWindowManager uses the registry instead of its own Map<workspaceId, Set<BrowserWindow>>. I kept the registry free of Electron imports because terminalWindowManager.ts cannot load under bun (electron has no BrowserWindow export there), so the close rule needed a testable home.
  4. It also merges a duplicate doc comment on TerminalService.getWorkspaceSessionIds left by 🤖 fix: fade sideways-scrolling rows, full skill descriptions, keep pop-out terminals out of the sidebar #5732 (flagged by that PR's final check).

Validation

src/desktop/terminalWindowRegistry.test.ts "when one of two pop-outs exits, only its window is closed and the other stays open": two pop-outs are open in one workspace and one exits. The test asserts that only that window is selected, and that the other stays registered. With the old "close every window of the workspace" rule restored inside the registry, the test fails:

+   "window-b",
- Expected  - 0
+ Received  + 1
(fail) TerminalWindowRegistry (#5739) > when one of two pop-outs exits, only its window is closed and the other stays open

No screenshots: the behavior exists only in the Electron app (in browser mode closeWindow is a no-op on the server, and each browser pop-out is its own tab). I did not run Electron for this change. Its window wiring is the 1:1 replacement of the old Map/Set shown in the diff.

Risks

Low. Callers that omit sessionId keep the old close-all behavior. A pop-out opened without a session (none exist today, because openTerminalPopout always passes one) is closed only by a close-all.


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $16.86

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T13:46:26.170519Z 209bfbc New commits
🔒 Security Review ✅ Completed 2026-10-06T13:46:04.919150Z 209bfbc New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33
ThomasK33 force-pushed the fix/popout-exit-closes-own-window branch from fe281ae to 209bfbc Compare October 6, 2026 13:44
@ThomasK33
ThomasK33 added this pull request to the merge queue Oct 6, 2026
Merged via the queue into main with commit 31a1242 Oct 6, 2026
33 checks passed
@ThomasK33
ThomasK33 deleted the fix/popout-exit-closes-own-window branch October 6, 2026 14:08
@ThomasK33 ThomasK33 added backlog and removed backlog labels Oct 6, 2026
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.

🤖 A pop-out terminal's exit closes every pop-out terminal of the workspace

1 participant