diff --git a/src/client/TerminalView.tsx b/src/client/TerminalView.tsx index eed678f..ac14191 100644 --- a/src/client/TerminalView.tsx +++ b/src/client/TerminalView.tsx @@ -99,6 +99,12 @@ export function TerminalView(props: { sessionId: string; tabId: string }) { window.clearTimeout(retry) observer.disconnect() inputSub.dispose() + // Tell the host the owning tab is gone so it releases the terminal + // quota immediately (best effort; a dropped socket gets the grace + // period on the host side instead). + if (socket !== null && socket.readyState === WebSocket.OPEN) { + socket.send(JSON.stringify({ type: 'close' })) + } socket?.close() term.dispose() connectRef.current = null diff --git a/src/client/index.tsx b/src/client/index.tsx index 9bcb286..8259880 100644 --- a/src/client/index.tsx +++ b/src/client/index.tsx @@ -1,17 +1,56 @@ /** - * Client half of dsh-better-sidebar: mounts the right sidebar portal and - * registers the turn-tail interception. Requires the runtime's slots and - * sessions services; the bundle itself is a module-table consumer only - * (react + ui-primitives + xterm, all provided or inlined). + * Client half of dsh-better-sidebar: mounts the right sidebar portal (inside + * an error boundary so a rendering failure shows an error strip instead of a + * blank panel) and registers the turn-tail interception. Requires the + * runtime's slots and sessions services; the bundle itself is a module-table + * consumer only (react + ui-primitives + xterm, all provided or inlined). */ +import { Component, createElement, type ErrorInfo, type ReactNode } from 'react' import { createRoot } from 'react-dom/client' import type { Context } from '../context-types.ts' import { Sidebar } from './Sidebar.tsx' import { registerTurnTailInterception } from './intercept.tsx' +import { t } from './locales.ts' +import css from './sidebar.module.css' /** Services required before mounting (provided by the client runtime). */ export const inject = ['slots', 'sessions'] +/** + * Error boundary over the sidebar tree: a render error must never blank the + * whole panel silently — it shows a dismissible error strip and logs the + * stack for diagnosis. + */ +class SidebarBoundary extends Component<{ children: ReactNode }, { error: string | null }> { + state = { error: null as string | null } + + static getDerivedStateFromError(error: unknown): { error: string } { + return { error: error instanceof Error ? error.message : String(error) } + } + + componentDidCatch(error: Error, info: ErrorInfo): void { + console.error('[dsh-better-sidebar] render error:', error, info.componentStack) + } + + render(): ReactNode { + if (this.state.error !== null) { + return ( +
+ dsh-better-sidebar: {this.state.error} + +
+ ) + } + return this.props.children + } +} + /** * Client plugin body. * @param ctx - the client cordis context (slots, sessions). @@ -22,7 +61,7 @@ export function apply(ctx: Context): void { host.setAttribute('data-dsh-better-sidebar', '') document.body.appendChild(host) const root = createRoot(host) - root.render() + root.render(createElement(SidebarBoundary, null, createElement(Sidebar, { ctx }))) return () => { root.unmount() host.remove() diff --git a/src/client/sidebar.module.css b/src/client/sidebar.module.css index 3b54ac8..c66275a 100644 --- a/src/client/sidebar.module.css +++ b/src/client/sidebar.module.css @@ -609,6 +609,24 @@ background: var(--dsw-alias-state-warn-tertiary); } +.boundaryError { + position: fixed; + right: 0; + top: 0; + bottom: 0; + z-index: 50; + display: flex; + flex-direction: column; + gap: 8px; + align-items: flex-start; + padding: 16px; + background: var(--dsw-alias-bg-layer-1); + border-left: 1px solid var(--dsw-alias-border-l2); + font: var(--dsw-font-xxs-12); + color: var(--dsw-alias-state-error-primary); + overflow: auto; +} + .terminalRetry { flex: none; padding: 1px 8px; diff --git a/src/index.ts b/src/index.ts index 6b70139..8c0ec3b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -36,6 +36,8 @@ const MEDIA_LIMIT = 20 * 1024 * 1024 const LIST_LIMIT = 1000 /** Terminals per session. */ const TERMINALS_PER_SESSION = 3 +/** How long a disconnected terminal process survives awaiting a reconnect. */ +const TERMINAL_RECONNECT_GRACE_MS = 30_000 /** Content types for the media route, by extension. */ const MEDIA_TYPES: Record = { @@ -325,7 +327,6 @@ async function attachTerminal( const exitSub = handle.pty.onExit(onExit) ws.on('message', (data) => { const text = data.toString('utf8') - if (handle.exited) return // Control frames are JSON with a known shape; anything else (including // JSON that is not a recognized control) is terminal input, verbatim. let control: { type?: unknown; cols?: unknown; rows?: unknown } | null = null @@ -337,8 +338,14 @@ async function attachTerminal( } catch { // Not JSON: terminal input. } + if (control !== null && control.type === 'close') { + // The owning tab was closed: release the quota immediately. + ptyManager.scheduleClose(handle.key, 0) + return + } + if (handle.exited) return if ( - control !== null && typeof control === 'object' + control !== null && control.type === 'resize' && typeof control.cols === 'number' && typeof control.rows === 'number' ) { @@ -350,6 +357,10 @@ async function attachTerminal( ws.on('close', () => { dataSub.dispose() exitSub.dispose() + // A bare socket drop (refresh, tab switch) leaves the process alive + // for a grace period so a quick reconnect keeps it; the reconnect's + // open() cancels the pending close. + ptyManager.scheduleClose(handle.key, TERMINAL_RECONNECT_GRACE_MS) }) } catch (error) { ws.close(1011, error instanceof Error ? error.message : String(error)) diff --git a/src/pty-manager.ts b/src/pty-manager.ts index c5092cf..0663265 100644 --- a/src/pty-manager.ts +++ b/src/pty-manager.ts @@ -60,6 +60,7 @@ export interface SidebarPty { */ export class PtyManager { private readonly sessions = new Map() + private readonly pendingCloses = new Map>() constructor( private readonly shell: string, @@ -76,7 +77,11 @@ export class PtyManager { } /** - * Open (or reuse) the terminal for a session/tab key. + * Open (or reuse) the terminal for a session/tab key. A handle whose + * process already exited is replaced with a fresh spawn (reconnecting a + * dead terminal must yield a live shell, not an input sink). Reopening + * also cancels any pending scheduled close (a reconnect within the grace + * window keeps the process alive). * @param sessionId - conversation id. * @param tabId - client tab id. * @param cwd - initial working directory (the session's cwd). @@ -87,8 +92,10 @@ export class PtyManager { */ open(sessionId: string, tabId: string, cwd: string, cols: number, rows: number): SidebarPty { const key = `${sessionId}:${tabId}` + this.cancelClose(key) const existing = this.sessions.get(key) - if (existing !== undefined) return existing + if (existing !== undefined && !existing.exited) return existing + if (existing !== undefined) this.close(key) if (this.keysOf(sessionId).length >= this.maxPerSession) { throw new SidebarError('pty-error', `terminal limit reached (${this.maxPerSession}) for this session`, 400) } @@ -120,6 +127,29 @@ export class PtyManager { return handle } + /** + * Schedule the terminal's destruction after `delayMs`. A tab close sends + * delay 0 (release the quota immediately); a bare socket drop (refresh, + * crash) uses the grace period so a quick reconnect keeps the process. + * `open()` cancels any pending close. + */ + scheduleClose(key: string, delayMs: number): void { + const handle = this.sessions.get(key) + if (handle === undefined) return + this.cancelClose(key) + const timer = setTimeout(() => { this.close(key) }, delayMs) + this.pendingCloses.set(key, timer) + } + + /** Cancel a pending scheduled close (the terminal is being reopened). */ + cancelClose(key: string): void { + const timer = this.pendingCloses.get(key) + if (timer !== undefined) { + clearTimeout(timer) + this.pendingCloses.delete(key) + } + } + /** Resolve a live handle by key, or undefined. */ get(key: string): SidebarPty | undefined { return this.sessions.get(key) @@ -127,6 +157,7 @@ export class PtyManager { /** Close a terminal and drop its state (the owning tab was closed). */ close(key: string): void { + this.cancelClose(key) const handle = this.sessions.get(key) if (handle === undefined) return this.sessions.delete(key) @@ -139,6 +170,8 @@ export class PtyManager { /** Close every terminal (plugin teardown). */ disposeAll(): void { + for (const timer of this.pendingCloses.values()) clearTimeout(timer) + this.pendingCloses.clear() for (const key of [...this.sessions.keys()]) this.close(key) } } diff --git a/tests/smoke.spec.ts b/tests/smoke.spec.ts index 8d41c12..9afd927 100644 --- a/tests/smoke.spec.ts +++ b/tests/smoke.spec.ts @@ -7,6 +7,7 @@ import { describe, expect, it } from 'vitest' import { apply } from '../src/index.ts' import * as git from '../src/git.ts' import { listDirectory } from '../src/fs-tree.ts' +import { defaultShell, PtyManager } from '../src/pty-manager.ts' import type { SidebarWebRoute, SidebarWebUpgradeRoute } from '../src/context-types.ts' interface FakeContext { @@ -58,6 +59,47 @@ describe('host plugin smoke', () => { expect(branches.names).toContain(branches.current) }) + it('pty manager releases the quota on close and respawns after exit', async () => { + const manager = new PtyManager(defaultShell(), 3) + try { + const first = manager.open('s1', 't1', process.cwd(), 80, 24) + expect(manager.keysOf('s1')).toHaveLength(1) + // Tab-close semantics (close frame): quota released immediately. + manager.scheduleClose(first.key, 0) + await new Promise(resolve => setTimeout(resolve, 50)) + expect(manager.keysOf('s1')).toHaveLength(0) + // Reopen spawns a fresh process. + const second = manager.open('s1', 't1', process.cwd(), 80, 24) + expect(second).not.toBe(first) + expect(manager.keysOf('s1')).toHaveLength(1) + // After the shell exits, a reconnect respawns instead of reusing the dead handle. + second.pty.write('exit\r') + const deadline = Date.now() + 5000 + while (!second.exited && Date.now() < deadline) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + expect(second.exited).toBe(true) + const third = manager.open('s1', 't1', process.cwd(), 80, 24) + expect(third.exited).toBe(false) + expect(third).not.toBe(second) + } finally { + manager.disposeAll() + } + }) + + it('pty manager: a reconnect within the grace period cancels the pending close', async () => { + const manager = new PtyManager(defaultShell(), 3) + try { + const handle = manager.open('s2', 't1', process.cwd(), 80, 24) + manager.scheduleClose(handle.key, 200) + manager.open('s2', 't1', process.cwd(), 80, 24) + await new Promise(resolve => setTimeout(resolve, 400)) + expect(manager.get(handle.key)).toBeDefined() + } finally { + manager.disposeAll() + } + }) + it('lists the repository root level', async () => { const listing = await listDirectory(process.cwd(), 1000) expect(listing.entries.some(entry => entry.name === 'src' && entry.isDir)).toBe(true)