From 61e487fff3fa86b52a07ed3a4da6f5f3810daf10 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 30 Jul 2025 03:50:32 +0200 Subject: [PATCH] fix: handle tmux session detachment gracefully instead of kill errors - Add detection for tmux attachment sessions (commands containing 'tmux attach' or names starting with 'tmux:') - Implement smart detachment using Ctrl-B,d sequence instead of SIGTERM - Add fallback to :detach-client command if initial detach fails - Update API responses to distinguish between 'killed' and 'detached' sessions - Prevents 500 errors when trying to kill the last tmux session This allows users to cleanly exit VibeTunnel tmux attachments without destroying the underlying tmux session, which can then be reattached later. --- web/src/server/pty/pty-manager.ts | 70 +++++++++++++++++++++++++++++++ web/src/server/pty/types.ts | 2 + web/src/server/routes/sessions.ts | 14 ++++++- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/web/src/server/pty/pty-manager.ts b/web/src/server/pty/pty-manager.ts index 656b35f9..8f646ab1 100644 --- a/web/src/server/pty/pty-manager.ts +++ b/web/src/server/pty/pty-manager.ts @@ -517,6 +517,14 @@ export class PtyManager extends EventEmitter { } } + // Detect if this is a tmux attachment session + const isTmuxAttachment = + (resolvedCommand.includes('tmux') && + (resolvedCommand.includes('attach-session') || + resolvedCommand.includes('attach') || + resolvedCommand.includes('a'))) || + sessionName.startsWith('tmux:'); + const session: PtySession = { id: sessionId, sessionInfo, @@ -531,6 +539,7 @@ export class PtyManager extends EventEmitter { isExternalTerminal: !!options.forwardToStdout, currentWorkingDir: workingDir, titleFilter: new TitleSequenceFilter(), + isTmuxAttachment, }; this.sessions.set(sessionId, session); @@ -1509,6 +1518,54 @@ export class PtyManager extends EventEmitter { } } + /** + * Detach from a tmux session gracefully + * @param sessionId The session ID of the tmux attachment + * @returns Promise that resolves when detached + */ + private async detachFromTmux(sessionId: string): Promise { + const session = this.sessions.get(sessionId); + if (!session || !session.isTmuxAttachment || !session.ptyProcess) { + return false; + } + + try { + logger.log(chalk.cyan(`Detaching from tmux session (${sessionId})`)); + + // Try the standard detach sequence first (Ctrl-B, d) + await this.sendInput(sessionId, { text: '\x02d' }); // \x02 is Ctrl-B + + // Wait for detachment + await new Promise((resolve) => setTimeout(resolve, 300)); + + // Check if the process is still running + if (!ProcessUtils.isProcessRunning(session.ptyProcess.pid)) { + logger.log(chalk.green(`Successfully detached from tmux (${sessionId})`)); + return true; + } + + // If still running, try sending the detach-client command + logger.debug('First detach attempt failed, trying detach-client command'); + await this.sendInput(sessionId, { text: ':detach-client\n' }); + + // Wait a bit longer + await new Promise((resolve) => setTimeout(resolve, 500)); + + // Final check + if (!ProcessUtils.isProcessRunning(session.ptyProcess.pid)) { + logger.log( + chalk.green(`Successfully detached from tmux using detach-client (${sessionId})`) + ); + return true; + } + + return false; + } catch (error) { + logger.error(`Error detaching from tmux: ${error}`); + return false; + } + } + /** * Kill a session with proper SIGTERM -> SIGKILL escalation * Returns a promise that resolves when the process is actually terminated @@ -1517,6 +1574,19 @@ export class PtyManager extends EventEmitter { const memorySession = this.sessions.get(sessionId); try { + // Special handling for tmux attachment sessions + if (memorySession?.isTmuxAttachment) { + const detached = await this.detachFromTmux(sessionId); + if (detached) { + // The PTY process should exit cleanly after detaching + // Let the normal exit handler clean up the session + return; + } + + logger.warn(`Failed to detach from tmux, falling back to normal kill`); + // Fall through to normal kill logic + } + // If we have an in-memory session with active PTY, kill it directly if (memorySession?.ptyProcess) { // If signal is already SIGKILL, send it immediately and wait briefly diff --git a/web/src/server/pty/types.ts b/web/src/server/pty/types.ts index dec4726a..ee83c85e 100644 --- a/web/src/server/pty/types.ts +++ b/web/src/server/pty/types.ts @@ -116,6 +116,8 @@ export interface PtySession { currentCommand?: string; // Command line of current foreground process commandStartTime?: number; // When current command started (timestamp) processPollingInterval?: NodeJS.Timeout; // Interval for checking process state + // Tmux attachment tracking + isTmuxAttachment?: boolean; // True if this session is attached to tmux } export class PtyError extends Error { diff --git a/web/src/server/routes/sessions.ts b/web/src/server/routes/sessions.ts index 7d3e807f..4f8cbf56 100644 --- a/web/src/server/routes/sessions.ts +++ b/web/src/server/routes/sessions.ts @@ -611,9 +611,19 @@ export function createSessionRoutes(config: SessionRoutesConfig): Router { logger.log(chalk.yellow(`local session ${sessionId} cleaned up`)); res.json({ success: true, message: 'Session cleaned up' }); } else { + // Check if this is a tmux attachment before killing + const isTmuxAttachment = + session.name?.startsWith('tmux:') || session.command?.includes('tmux attach'); + await ptyManager.killSession(sessionId, 'SIGTERM'); - logger.log(chalk.yellow(`local session ${sessionId} killed`)); - res.json({ success: true, message: 'Session killed' }); + + if (isTmuxAttachment) { + logger.log(chalk.yellow(`local session ${sessionId} detached from tmux`)); + res.json({ success: true, message: 'Detached from tmux session' }); + } else { + logger.log(chalk.yellow(`local session ${sessionId} killed`)); + res.json({ success: true, message: 'Session killed' }); + } } } catch (error) { logger.error('error killing session:', error);