From f34907393cdd6682c2ed9913b312009176226bda Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 7 Jun 2026 22:07:25 -0700 Subject: [PATCH] Reject unknown terminal modes instead of spawning dead terminals new-tab/split-pane via the MCP (and the REST API generally) accepted any `mode` string and passed it straight to the spawn path. For a mode that is neither the built-in 'shell' nor a registered coding CLI, buildSpawnSpec fell back to exec-ing the mode name itself (`cli?.command || mode`), so e.g. `mode: "terminal"` spawned a process literally named "terminal" that died instantly with "execvp(3) failed: No such file or directory" -- leaving a tab whose terminal was already gone. Validate the mode at the authoritative spawn point: buildSpawnSpec now throws UnknownTerminalModeError for any non-shell mode that isn't a registered coding CLI, listing the valid modes. The REST router maps that error to a 400, so the MCP surfaces a clear message instead of a zombie terminal. This mirrors the validation the WebSocket terminal.create path already performs. Adds getKnownTerminalModes/isKnownTerminalMode helpers and regression tests at both the buildSpawnSpec and REST-route layers. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/agent-api/router.ts | 8 +++-- server/terminal-registry.ts | 34 ++++++++++++++++++++++ test/server/agent-tabs-write.test.ts | 30 +++++++++++++++++++ test/unit/server/terminal-registry.test.ts | 18 +++++++++++- 4 files changed, 87 insertions(+), 3 deletions(-) diff --git a/server/agent-api/router.ts b/server/agent-api/router.ts index 5f3226260..cf39e27af 100644 --- a/server/agent-api/router.ts +++ b/server/agent-api/router.ts @@ -14,7 +14,7 @@ import { } from '../coding-cli/codex-launch-config.js' import { INVALID_RAW_CODEX_RESUME_MESSAGE } from '../coding-cli/codex-app-server/restore-decision.js' import { makeSessionKey } from '../coding-cli/types.js' -import { terminalIdFromCreateError, type ProviderSettings, type TerminalInputResult } from '../terminal-registry.js' +import { terminalIdFromCreateError, UnknownTerminalModeError, type ProviderSettings, type TerminalInputResult } from '../terminal-registry.js' import { MAX_TERMINAL_TITLE_OVERRIDE_LENGTH } from '../terminals-router.js' import { logger } from '../logger.js' import { ok, approx, fail } from './response.js' @@ -36,7 +36,11 @@ class AgentRouteInputError extends Error { } function agentRouteErrorStatus(error: unknown): number { - return error instanceof CodexLaunchConfigError || error instanceof AgentRouteInputError ? 400 : 500 + return error instanceof CodexLaunchConfigError + || error instanceof AgentRouteInputError + || error instanceof UnknownTerminalModeError + ? 400 + : 500 } function errorMessage(error: unknown): string { diff --git a/server/terminal-registry.ts b/server/terminal-registry.ts index 193dc59a5..1ef821d68 100644 --- a/server/terminal-registry.ts +++ b/server/terminal-registry.ts @@ -126,6 +126,33 @@ export function registerCodingCliCommands(specs: Map, terminalId?: string, ) { + // Reject modes that aren't the built-in shell or a registered coding CLI. + // Otherwise the fallbacks below (`cli?.command || mode`, and the WSL bash + // fallback) try to exec the mode name itself, spawning a terminal that dies + // instantly with "execvp(3) failed: No such file or directory". + if (mode !== 'shell' && !codingCliCommands.has(mode)) { + throw new UnknownTerminalModeError(mode) + } // Strip inherited env vars that interfere with child terminal behaviour: // - CLAUDECODE: causes child Claude processes to refuse to start ("nested session" error) // - CI/NO_COLOR/FORCE_COLOR/COLOR: disables interactive color in user PTYs diff --git a/test/server/agent-tabs-write.test.ts b/test/server/agent-tabs-write.test.ts index d34a44685..8a3e3966a 100644 --- a/test/server/agent-tabs-write.test.ts +++ b/test/server/agent-tabs-write.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi } from 'vitest' import express from 'express' import request from 'supertest' import { createAgentApiRouter } from '../../server/agent-api/router' +import { UnknownTerminalModeError } from '../../server/terminal-registry' import { FakeCodexLaunchPlanner } from '../helpers/coding-cli/fake-codex-launch-planner.js' import { INVALID_RAW_CODEX_RESUME_MESSAGE } from '../../server/coding-cli/codex-app-server/restore-decision.js' @@ -684,4 +685,33 @@ describe('tab endpoints', () => { expect(renameTab).toHaveBeenCalledWith('missing', 'Ghost') expect(broadcastUiCommand).not.toHaveBeenCalled() }) + + it('rejects an unknown terminal mode with a 400 and rolls back the tab', async () => { + const app = express() + app.use(express.json()) + const closeTab = vi.fn() + // A spawn for an unmodelled mode (e.g. mode: 'terminal') throws from the + // registry; the route must surface a 400, not spawn a dead terminal. + const registry = { + create: vi.fn(() => { throw new UnknownTerminalModeError('terminal') }), + } + const layoutStore = { + createTab: () => ({ tabId: 'tab_1', paneId: 'pane_1' }), + attachPaneContent: vi.fn(), + closeTab, + } + app.use('/api', createAgentApiRouter({ + layoutStore, + registry: registry as any, + wsHandler: { broadcastUiCommand: vi.fn() }, + })) + + const res = await request(app).post('/api/tabs').send({ name: 'x', mode: 'terminal' }) + + expect(res.status).toBe(400) + expect(res.body.status).toBe('error') + expect(res.body.message).toMatch(/Invalid terminal mode/) + // The half-created tab is cleaned up rather than left dangling. + expect(closeTab).toHaveBeenCalledWith('tab_1') + }) }) diff --git a/test/unit/server/terminal-registry.test.ts b/test/unit/server/terminal-registry.test.ts index 6bc5f74a3..47c40e1ce 100644 --- a/test/unit/server/terminal-registry.test.ts +++ b/test/unit/server/terminal-registry.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { isLinuxPath, getSystemShell, escapeCmdExe, buildSpawnSpec, TerminalRegistry, isWsl, isWindowsLike, modeSupportsResume, buildTerminalSessionRef } from '../../../server/terminal-registry' +import { isLinuxPath, getSystemShell, escapeCmdExe, buildSpawnSpec, TerminalRegistry, isWsl, isWindowsLike, modeSupportsResume, buildTerminalSessionRef, UnknownTerminalModeError } from '../../../server/terminal-registry' import { isValidClaudeSessionId } from '../../../server/claude-session-id' import { CODEX_DURABILITY_SCHEMA_VERSION } from '../../../shared/codex-durability' import * as fs from 'fs' @@ -733,6 +733,22 @@ describe('buildSpawnSpec Unix paths', () => { process.env = originalEnv }) + describe('invalid mode', () => { + beforeEach(() => { + mockPlatform('linux') + }) + + it('throws instead of returning a spec that would exec the mode name', () => { + // Regression: an unknown mode used to fall through to `file: mode`, so + // new-tab({ mode: 'terminal' }) spawned a process literally named + // "terminal" that died with "execvp(3) failed: No such file or directory". + expect(() => buildSpawnSpec('terminal', '/home/user', 'system')) + .toThrow(UnknownTerminalModeError) + expect(() => buildSpawnSpec('terminal', '/home/user', 'system')) + .toThrow(/Invalid terminal mode: 'terminal'/) + }) + }) + describe('macOS shell mode', () => { beforeEach(() => { mockPlatform('darwin')