Add per-thread multi-terminal sessions across server contracts and UI#18
Add per-thread multi-terminal sessions across server contracts and UI#18juliusmarminge merged 6 commits intomainfrom
Conversation
- Support multiple terminal IDs per thread across contracts, server manager, and WS handling - Persist terminal history per terminal ID and allow closing all terminals for a thread - Update web thread state and terminal drawer to create, split, and switch active terminals - Expand tests for multi-terminal behavior and terminal ID propagation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughSessions are refactored to be terminal-scoped (threadId + terminalId). Added terminalId everywhere: contracts/schemas, server session/history management, websocket handlers, web state, persistence v7, UI components and tests to support multiple terminals per thread, per-terminal history files, and lifecycle (open/write/close/clear) semantics. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
Add per-thread multi-terminal sessions and UI controls across web app and server to support split and grouped terminals with persistence v7Introduce 📍Where to StartStart with the terminal contracts ( Macroscope summarized 93d0ac4. |
Greptile OverviewGreptile SummaryThis PR adds multi-terminal support per thread, allowing users to create, split, and manage multiple isolated terminal sessions within a single conversation thread. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ChatView
participant Store
participant ThreadTerminalDrawer
participant TerminalViewport
participant WebSocket
participant TerminalManager
participant PTY
User->>ChatView: Click "New Terminal"
ChatView->>Store: dispatch(NEW_THREAD_TERMINAL)
Store->>Store: Generate unique terminalId
Store->>Store: Add to terminalIds array
Store->>Store: Set as activeTerminalId
Store->>Store: Switch layout to "tabs"
Store-->>ChatView: Updated thread state
ChatView->>ThreadTerminalDrawer: Render with new terminalIds
ThreadTerminalDrawer->>TerminalViewport: Mount new terminal viewport
TerminalViewport->>TerminalViewport: Create xterm.js Terminal
TerminalViewport->>WebSocket: terminal.open({threadId, terminalId, cwd, cols, rows})
WebSocket->>TerminalManager: open(input)
TerminalManager->>TerminalManager: Create sessionKey from threadId+terminalId
TerminalManager->>TerminalManager: readHistory(threadId, terminalId)
TerminalManager->>PTY: spawn shell process
PTY-->>TerminalManager: Process started
TerminalManager->>TerminalManager: Store session in Map with composite key
TerminalManager-->>WebSocket: TerminalSessionSnapshot
WebSocket-->>TerminalViewport: Snapshot with history
TerminalViewport->>TerminalViewport: Write history to terminal
TerminalViewport->>TerminalViewport: Focus terminal
User->>TerminalViewport: Type command
TerminalViewport->>WebSocket: terminal.write({threadId, terminalId, data})
WebSocket->>TerminalManager: write(input)
TerminalManager->>TerminalManager: requireSession(threadId, terminalId)
TerminalManager->>PTY: process.write(data)
PTY->>TerminalManager: onData(output)
TerminalManager->>TerminalManager: Append to session.history
TerminalManager->>TerminalManager: queuePersist(threadId, terminalId, history)
TerminalManager->>WebSocket: emit("event", {type: "output", threadId, terminalId, data})
WebSocket->>TerminalViewport: Push terminal event
TerminalViewport->>TerminalViewport: terminal.write(output)
User->>ChatView: Click "Split Terminal"
ChatView->>Store: dispatch(SPLIT_THREAD_TERMINAL)
Store->>Store: Generate new terminalId
Store->>Store: Set splitTerminalIds=[first, second]
Store->>Store: Switch layout to "split"
Store-->>ChatView: Updated thread state
ChatView->>ThreadTerminalDrawer: Render split layout
ThreadTerminalDrawer->>TerminalViewport: Mount two viewports side-by-side
Note over TerminalViewport,PTY: Each viewport opens its own session with unique terminalId
Last reviewed commit: d9ab1bf |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| } | ||
|
|
||
| private async deleteAllHistoryForThread(threadId: string): Promise<void> { | ||
| const threadPrefix = `${toSafeThreadId(threadId)}_`; |
There was a problem hiding this comment.
🟢 Low
src/terminalManager.ts:690 Using _ as a filename delimiter between base64url values (e.g., threadId, terminalId) can collide with encoded data and risk incorrect matching/deletion. Suggest a separator outside the base64url alphabet (e.g., . or ~) for threadPrefix/historyPath, and adjust the matching/deletion logic accordingly.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/server/src/terminalManager.ts around line 690:
Using `_` as a filename delimiter between base64url values (e.g., `threadId`, `terminalId`) can collide with encoded data and risk incorrect matching/deletion. Suggest a separator outside the base64url alphabet (e.g., `.` or `~`) for `threadPrefix`/`historyPath`, and adjust the matching/deletion logic accordingly.
- Add terminal close controls in tab and split terminal views - Send `exit` before closing and update active terminal/focus in chat state - Add reducer support and tests for closing tabs, split fallback, and final-terminal drawer hide
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/ThreadTerminalDrawer.tsx (1)
111-371:⚠️ Potential issue | 🟠 MajorRemove
autoFocusfrom the init effect dependencies to prevent unnecessary terminal recreation.Including
autoFocusin the dependency array causes the entire effect to re-run wheneverautoFocuschanges, triggering terminal disposal and recreation along with extraapi.terminal.open()andapi.terminal.resize()calls, resulting in visible flicker. The dedicatedautoFocuseffect (lines 372–382) already handles focus independently, so the init effect can safely omit this dependency.🛠️ Suggested fix
- }, [api, autoFocus, cwd, terminalId, threadId]); + }, [api, cwd, terminalId, threadId]);
🤖 Fix all issues with AI agents
In `@apps/web/src/store.ts`:
- Around line 152-191: The normalizeThreadTerminals function currently preserves
terminalLayout:"split" even when validSplitIds.length < 2; change it so when
thread.terminalLayout === "split" but there are fewer than 2 valid split ids you
downgrade the layout to "tabs" if terminalIds.length > 1 or to "single"
otherwise, clear splitTerminalIds to an empty array, and ensure activeTerminalId
is selected from terminalIds (falling back to DEFAULT_THREAD_TERMINAL_ID) —
update the branch that computes validSplitIds/splitTerminalIds to return an
object with terminalLayout set to the appropriate downgraded value and a
consistent activeTerminalId when validSplitIds.length < 2.
- Replace `terminalLayout`/`splitTerminalIds` with `terminalGroups` and `activeTerminalGroupId` - Update terminal drawer UI to render grouped splits with a terminal sidebar - Bump persisted web state to v7 with legacy v6 migration coverage in tests
Co-authored-by: macroscopeapp[bot] <170038800+macroscopeapp[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/components/ChatView.tsx`:
- Around line 303-315: The close handler currently only sends "exit\n" and then
dispatches CLOSE_THREAD_TERMINAL; update the handler (the lambda that captures
activeThreadId, api, dispatch and calls api.terminal.write) to first attempt
api.terminal.close({ threadId: activeThreadId, terminalId }) and await it (or
call it and catch errors), and if api.terminal.close is not available or fails,
fall back to api.terminal.write({ threadId: activeThreadId, terminalId, data:
"exit\n" }); preserve the existing dispatch({ type: "CLOSE_THREAD_TERMINAL",
threadId: activeThreadId, terminalId }) and setTerminalFocusRequestId update,
and ensure all API calls are wrapped in .catch to avoid blocking the UI.
- Call `api.terminal.close` when available when closing a terminal tab - Fall back to writing `exit\n` to preserve compatibility with older/uninitialized terminals
- Pass `deleteHistory: true` when closing a terminal - Fallback path now clears terminal before sending `exit` - Use `gitCwd` as terminal cwd when available
1 participant
Summary
TerminalManagerwith session keys scoped bythreadId + terminalIdterminalIdthrough terminal events/snapshots/contracts and update WebSocket tests/mocks accordinglyterminalIds,activeTerminalId,terminalLayout,splitTerminalIds)Testing
apps/server/src/terminalManager.test.ts: added checks for isolated multi-terminal sessions,clearedeventterminalId, and close-all behavior withoutterminalIdapps/server/src/wsServer.test.ts: updated terminal mock/session assertions to include default and explicitterminalIdpackages/contracts/src/terminal.test.ts: updated/added coverage for terminal contract schema changesapps/web/src/persistenceSchema.test.tsandapps/web/src/store.test.ts: updated coverage for new thread terminal persistence/store fieldsSummary by CodeRabbit
New Features
Bug Fixes