🤖 fix: show error when terminal button fails #1861
Conversation
The 'New terminal' button in WorkspaceHeader silently failed when:
1. addTerminalRef.current was null (RightSidebar not mounted)
2. api was null (not connected to server)
Now returns { success, error } from all terminal-opening paths and
displays the error inline in the header (same pattern as editor errors),
auto-dismissing after 3 seconds.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf9d563465
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/browser/components/ChatPane.tsx
Outdated
| chatInputAPI, | ||
| jumpToBottom, | ||
| handleOpenTerminal: onOpenTerminal, | ||
| handleOpenTerminal: () => void onOpenTerminal(), |
There was a problem hiding this comment.
This inline wrapper creates a new handleOpenTerminal function on every render. useAIViewKeybinds installs global window keydown listeners in an effect that depends on handleOpenTerminal, so this change forces teardown/re‑add of those listeners on every ChatPane re-render (which is frequent during streaming). That can introduce unnecessary work and makes keybind handling more fragile under heavy updates. Since onOpenTerminal already accepts optional args, it can be passed directly (or otherwise stabilized) to keep the keybind effect stable.
Useful? React with 👍 / 👎.
Terminal functionality is now exclusively provided through integrated terminals in the RightSidebar. Removed code paths: Frontend: - useOpenTerminal hook (no longer needed) - terminal-window.tsx popup entry point - terminal.html popup window HTML - openTerminalPopout utility function - Command palette "New Terminal Window" commands - Pop-out button from terminal tabs - Mobile-only popout fallback logic in WorkspaceShell/Header Backend: - terminal.openWindow and terminal.closeWindow API endpoints - terminal.openNative API endpoint (spawned native terminals) - terminalWindowManager.ts (Electron BrowserWindow manager) - setTerminalWindowManager wiring in ServiceContainer and main.ts - All native terminal spawning code (macOS, Windows, Linux) Tests: - Removed openNative test suite (~300 lines) - Removed openWindow test case - Updated mocks in storybook and integration tests vite.config.ts: - Removed terminal.html from build entries This simplifies the codebase significantly: one terminal path instead of three (integrated, popout, native).
1 participant
Summary
Simplifies terminal UX to a single integrated RightSidebar path, adds inline error handling for failed terminal opens, and stabilizes the terminal keybind handler to avoid re-registering global listeners during streaming updates.
Background
The "New terminal" button could fail silently when the sidebar wasn’t mounted or the API was unavailable. At the same time, we wanted to remove the popout/native terminal surfaces to reduce complexity and keep terminal behavior consistent.
Implementation
onOpenTerminaldirectly into keybinds to keep listener registration stable.Validation
make typecheckmake static-checkRisks
Generated with
mux• Model:openai:gpt-5.2-codex• Thinking:xhigh• Cost:$11.21