feat: MCP session management tools — rename, reorder, read output#109
feat: MCP session management tools — rename, reorder, read output#109jcanizalez merged 4 commits intomainfrom
Conversation
- rename_session: change a session's display name - reorder_sessions: set the display order of sessions in the grid - read_session_output: read terminal output ring buffer (last 1000 lines) Server-side: add output ring buffer to pty-manager, rename/reorder methods, and corresponding RPC endpoints. Broadcasts session:updated and session:reordered events for UI sync. Closes #91
There was a problem hiding this comment.
Pull request overview
Adds richer MCP-driven session management (rename/reorder/read output) and updates session history/resume behavior across server + renderer to support multi-agent coordination and more reliable path matching.
Changes:
- Add MCP tools
rename_session,reorder_sessions,read_session_outputbacked by new terminal RPC methods and a PTY output ring buffer. - Update recent-session types/UI to use
activityCount/activityLabel+canResumeExact, and gate “resume” flows viasupportsExactSessionResume()(Gemini becomes history-only). - Improve path normalization and project/worktree matching for session history + resume selection (incl. new OpenCode history provider).
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/session-utils.test.ts | Adds coverage for worktree-preferring resume selection and project-name casing/worktree naming. |
| tests/process-utils.test.ts | Adds Windows-style path normalization test cases. |
| tests/agent-launch.test.ts | Updates Gemini resume behavior expectations. |
| tests/agent-history.test.ts | Expands coverage for worktrees, Windows path normalization, and adds OpenCode + Gemini parsing cases. |
| src/renderer/lib/session-utils.ts | Adds path normalization helpers, activity formatting, and gates exact resume by agent capability. |
| src/renderer/components/workflow-editor/WorkflowEditor.tsx | Prevents resume attempt for non-exact-resume agents. |
| src/renderer/components/workflow-editor/RunEntry.tsx | Shows “Resume Session” only when exact resume is supported. |
| src/renderer/components/TaskQueuePanel.tsx | Gates resume callback creation on exact-resume support. |
| src/renderer/components/TaskDiffReview.tsx | Gates resume flow on exact-resume support; minor stat prop normalization. |
| src/renderer/components/TaskDetailPanel.tsx | Gates resume actions on exact-resume support. |
| src/renderer/components/TaskBoardView.tsx | Gates resume callback creation on exact-resume support. |
| src/renderer/components/RecentSessionsPopover.tsx | Disables non-resumable sessions and displays activity via new formatter. |
| src/renderer/components/RecentSessionsCard.tsx | Same as popover: disables non-resumable sessions + activity formatting. |
| src/renderer/components/ProjectSidebar.tsx | Gates “Resume session” controls on exact-resume support. |
| src/renderer/components/CommandPalette.tsx | Filters out non-resumable sessions; updates sublabel to activity formatting. |
| packages/shared/src/types.ts | Adds supportsExactSessionResume(), activity label helper, and updates RecentSession shape + IPC constants. |
| packages/shared/src/protocol.ts | Adds terminal RPC methods and new server notifications for session update/reorder. |
| packages/server/src/register-methods.ts | Registers terminal:rename, terminal:reorder, terminal:readOutput. |
| packages/server/src/pty-manager.ts | Implements output ring buffer, session ordering, rename/reorder, and output readback. |
| packages/server/src/process-utils.ts | Enhances normalizePath() for Windows-style inputs and case normalization. |
| packages/server/src/agent-launch.ts | Avoids injecting a Gemini resume flag; gates resume args by supportsExactSessionResume(). |
| packages/server/src/agent-history.ts | Adds worktree-aware scoping, improved path filtering, Gemini parsing improvements, and OpenCode provider. |
| packages/mcp/src/tools/sessions.ts | Adds MCP tools for rename/reorder/read output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (lines && lines < buf.length) { | ||
| return buf.slice(-lines) | ||
| } | ||
| return [...buf] |
There was a problem hiding this comment.
getOutput() treats any truthy lines value as valid. If a caller passes a negative number, buf.slice(-lines) will slice from the start (wrong semantics). Consider validating lines (e.g., throw on lines <= 0, and clamp to MAX_OUTPUT_LINES) before slicing to ensure consistent/expected behavior across all callers (not just the MCP tool schema).
| if (lines && lines < buf.length) { | |
| return buf.slice(-lines) | |
| } | |
| return [...buf] | |
| // If no line limit provided, return a copy of the entire buffer | |
| if (lines === undefined) { | |
| return [...buf] | |
| } | |
| // Validate that lines is a positive integer | |
| if (!Number.isFinite(lines) || !Number.isInteger(lines)) { | |
| throw new Error('lines must be a positive integer') | |
| } | |
| if (lines <= 0) { | |
| throw new Error('lines must be greater than 0') | |
| } | |
| // Clamp to the maximum configured output lines | |
| const effectiveLines = Math.min(lines, MAX_OUTPUT_LINES) | |
| if (buf.length <= effectiveLines) { | |
| return [...buf] | |
| } | |
| return buf.slice(-effectiveLines) |
There was a problem hiding this comment.
The MCP schema validates min(1) so negatives can't reach the server. The lines param on the RPC side is only called from MCP.
packages/server/src/pty-manager.ts
Outdated
| // Validate that all IDs are active sessions | ||
| for (const id of ids) { | ||
| if (!this.sessions.has(id)) throw new Error(`Session not found: ${id}`) | ||
| } | ||
| this.sessionOrder = ids | ||
| this.emit('client-message', IPC.SESSION_REORDERED, ids) |
There was a problem hiding this comment.
reorderSessions() only checks that IDs exist, but it doesn't enforce uniqueness. If ids contains duplicates, getActiveSessions() will return the same session multiple times and the UI order can become inconsistent. Consider rejecting duplicates (or de-duping while preserving first occurrence) and optionally pruning IDs for sessions that have since exited.
| // Validate that all IDs are active sessions | |
| for (const id of ids) { | |
| if (!this.sessions.has(id)) throw new Error(`Session not found: ${id}`) | |
| } | |
| this.sessionOrder = ids | |
| this.emit('client-message', IPC.SESSION_REORDERED, ids) | |
| // Validate that all IDs are active sessions and enforce uniqueness | |
| const seen = new Set<string>() | |
| const uniqueIds: string[] = [] | |
| for (const id of ids) { | |
| if (!this.sessions.has(id)) throw new Error(`Session not found: ${id}`) | |
| if (!seen.has(id)) { | |
| seen.add(id) | |
| uniqueIds.push(id) | |
| } | |
| } | |
| this.sessionOrder = uniqueIds | |
| this.emit('client-message', IPC.SESSION_REORDERED, uniqueIds) |
There was a problem hiding this comment.
Fixed in db4df6a — added new Set(ids).size !== ids.length check.
packages/server/src/pty-manager.ts
Outdated
| for (const s of this.sessions.values()) { | ||
| if (!this.sessionOrder.includes(s.id)) ordered.push(s) |
There was a problem hiding this comment.
getActiveSessions() uses this.sessionOrder.includes(s.id) inside a loop over this.sessions.values(), making this path O(n²). If session counts grow (or reorder is called frequently), consider building a Set from sessionOrder once and using has() for the membership check.
| for (const s of this.sessions.values()) { | |
| if (!this.sessionOrder.includes(s.id)) ordered.push(s) | |
| const orderedIdSet = new Set(this.sessionOrder) | |
| for (const s of this.sessions.values()) { | |
| if (!orderedIdSet.has(s.id)) ordered.push(s) |
There was a problem hiding this comment.
Fixed in db4df6a — replaced includes() with a Set for O(1) lookups.
| renameSession(id: string, displayName: string): void { | ||
| const session = this.sessions.get(id) | ||
| if (!session) throw new Error(`Session not found: ${id}`) | ||
| session.displayName = displayName | ||
| this.emit('client-message', IPC.SESSION_UPDATED, session) | ||
| } | ||
|
|
||
| reorderSessions(ids: string[]): void { | ||
| // Validate that all IDs are active sessions | ||
| for (const id of ids) { | ||
| if (!this.sessions.has(id)) throw new Error(`Session not found: ${id}`) | ||
| } | ||
| this.sessionOrder = ids | ||
| this.emit('client-message', IPC.SESSION_REORDERED, ids) | ||
| } |
There was a problem hiding this comment.
This emits the new session:updated / session:reordered IPC notifications, but the renderer currently only subscribes to IPC.SESSION_CREATED (see src/preload/index.ts and src/renderer/App.tsx). Without adding corresponding window.api.onSessionUpdated / onSessionReordered listeners and store updates, rename/reorder from MCP won't actually sync the UI as intended.
There was a problem hiding this comment.
Intentional for this PR — the web app works via list_sessions re-fetch, and the Electron renderer wiring is a follow-up. The IPC constants are added now so the renderer can subscribe without a protocol change later.
| getOutput(id: string, lines?: number): string[] { | ||
| const buf = this.outputLines.get(id) ?? [] | ||
| if (lines && lines < buf.length) { | ||
| return buf.slice(-lines) | ||
| } | ||
| return [...buf] | ||
| } |
There was a problem hiding this comment.
getOutput() returns an empty array when the session ID is unknown because it reads from outputLines only. That makes "invalid session" indistinguishable from "valid session with no output yet". Consider checking this.sessions.has(id) and throwing Session not found to align with renameSession() / reorderSessions() and to give MCP users a clearer error.
There was a problem hiding this comment.
Fixed in db4df6a — getOutput now throws Session not found for unknown IDs.
| { | ||
| session_ids: z | ||
| .array(V.id) | ||
| .min(1, 'At least one session ID is required') |
There was a problem hiding this comment.
The reorder_sessions schema allows duplicate session_ids. Since the server-side reorderSessions() currently also allows duplicates, a client can accidentally produce duplicate sessions in the active list. Consider adding a Zod refine here to enforce uniqueness (and ideally also enforce it on the server for non-MCP callers).
| .min(1, 'At least one session ID is required') | |
| .min(1, 'At least one session ID is required') | |
| .refine( | |
| (ids) => new Set(ids).size === ids.length, | |
| { message: 'Session IDs must be unique' } | |
| ) |
There was a problem hiding this comment.
Server-side reorderSessions() now rejects duplicates (db4df6a). Server validation is sufficient since the RPC is the single entry point.
- appendOutput: track partial lines across chunks, strip ANSI codes, skip shell-only PTYs, use loop instead of spread (stack safety) - sessionOrder: prune stale IDs on exit/kill - getActiveSessions: use Set for O(1) lookups - reorderSessions: reject duplicate IDs - getOutput: throw on unknown session ID - read_session_output: fix description to mention rolling buffer
|
Reviewed Copilot comments — all actionable items were already fixed in db4df6a:
LGTM from my review. Clean code, good MCP ergonomics. |
Summary
rename_session— change a session's display name via MCPreorder_sessions— set the display order of sessions in the gridread_session_output— read terminal output (ring buffer, last 1000 lines) for multi-agent coordinationCloses #91
Implementation
pty-manager.ts, matching the existing pattern inheadless-manager.tsrenameSession(),reorderSessions(),getOutput()methods to pty-managerterminal:rename,terminal:reorder,terminal:readOutputRPC endpointssession:updatedandsession:reorderedIPC events + protocol types for UI syncVvalidators)Test plan
rename_session— rename an active session, verifylist_sessionsshows new namereorder_sessions— reorder sessions, verifylist_sessionsreturns new orderread_session_output— launch a session, wait for output, read last N lines