From 8c7a9781fa44b64701b3932082f46f2d7bbc80d7 Mon Sep 17 00:00:00 2001 From: arkon Date: Wed, 10 Jun 2026 16:15:55 +0200 Subject: [PATCH] =?UTF-8?q?fix(codex):=20review=20fixes=20=E2=80=94=20enve?= =?UTF-8?q?lope=20handling,=20mode=20guards,=20UI=20parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocker: runCodex read raw response shapes, but the global preSerialization hook (server.ts) wraps every payload in the { success, data } envelope — status.available was always undefined, so the UI unconditionally printed "Codex CLI not found" and could never start a session; the created session was also never auto-selected (data.sessionId vs data.data.sessionId). Fixed both reads to match runOpenCode, and updated the test mocks to the real wire shape (plus a selectSession assertion) so envelope drift fails the test. Guard parity: export isExternalCliMode() from session.ts and use it in the ralph-config guard, all three respawn guards, and the six restore/ setup guards in server.ts that previously only excluded 'opencode' — codex sessions could otherwise get a Ralph tracker or respawn controller attached (idle detection is Claude-specific and output- silence respawn cycling would misfire on a quiet codex TUI). UI parity: cx tab badge, "Kill Tmux & Codex" dialog title, and the missing CSS (.run-mode-dot.codex, .tab-mode.codex, .mode-codex button colors — purple) so the Codex menu dot is no longer invisible. Removed the dead object-literal runMode getter that Object.assign flattens (superseded by the defineProperty accessor this PR adds). Verified end-to-end on an isolated instance with a stub codex binary: 10/10 Playwright checks (menu/dot/label/button styling, session created + auto-selected, cx badge, TUI output streamed, ralph+respawn guards reject codex) and --dangerously-bypass-approvals-and-sandbox + --model observed on the spawned command line. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/session.ts | 2 +- src/web/public/app.js | 6 ++++-- src/web/public/session-ui.js | 15 +++++++++------ src/web/public/styles.css | 22 ++++++++++++++++++++++ src/web/routes/ralph-routes.ts | 11 +++++++---- src/web/routes/respawn-routes.ts | 19 ++++++++++--------- src/web/server.ts | 26 +++++++++++++------------- test/run-mode-ui.test.ts | 12 +++++++++--- 8 files changed, 75 insertions(+), 38 deletions(-) diff --git a/src/session.ts b/src/session.ts index 25f456dc..d0657d4f 100644 --- a/src/session.ts +++ b/src/session.ts @@ -125,7 +125,7 @@ const CTRL_L_PATTERN = /\x0c/g; const NEWLINE_SPLIT_PATTERN = /\r?\n/; /** True for external-CLI run modes (non-Claude) that use their own TUI and output format. */ -function isExternalCliMode(mode: SessionMode): boolean { +export function isExternalCliMode(mode: SessionMode): boolean { return mode === 'opencode' || mode === 'codex'; } diff --git a/src/web/public/app.js b/src/web/public/app.js index 729ef861..f5babd80 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2572,7 +2572,7 @@ class CodemanApp { - ${mode === 'shell' ? '' : mode === 'opencode' ? '' : ''} + ${mode === 'shell' ? '' : mode === 'opencode' ? '' : mode === 'codex' ? '' : ''} ${(() => { const p = parseSessionPrefix(name); return p && p.suffix ? '' + escapeHtml(p.prefix) + ': ' + escapeHtml(p.suffix) + '' : escapeHtml(name); })()} @@ -3374,7 +3374,9 @@ class CodemanApp { if (killTitle) { killTitle.textContent = session.mode === 'opencode' ? 'Kill Tmux & OpenCode' - : 'Kill Tmux & Claude Code'; + : session.mode === 'codex' + ? 'Kill Tmux & Codex' + : 'Kill Tmux & Claude Code'; } document.getElementById('closeConfirmModal').classList.add('active'); diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 8b1041be..7a43a544 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -151,7 +151,7 @@ Object.assign(CodemanApp.prototype, { return this.run(); }, - /** Run using the selected mode (Claude Code or OpenCode) */ + /** Run using the selected mode (Claude Code, OpenCode, or Codex) */ async run() { const mode = this._runMode || 'claude'; if (mode === 'opencode') { @@ -163,8 +163,9 @@ Object.assign(CodemanApp.prototype, { return this.runClaude(); }, - /** Get/set the run mode, persisted in localStorage */ - get runMode() { return this._runMode || 'claude'; }, + // Note: `runMode` is an accessor defined via Object.defineProperty at the bottom of + // this file — an object-literal getter here would be flattened to a static value by + // Object.assign (it copies values, not accessor descriptors). setRunMode(mode) { this._runMode = mode; @@ -593,7 +594,7 @@ Object.assign(CodemanApp.prototype, { try { const statusRes = await fetch('/api/codex/status'); - const status = await statusRes.json(); + const status = (await statusRes.json()).data; if (!status.available) { this.terminal.writeln('\x1b[1;31m Codex CLI not found.\x1b[0m'); this.terminal.writeln('\x1b[90m Install with: npm install -g @openai/codex\x1b[0m'); @@ -618,8 +619,10 @@ Object.assign(CodemanApp.prototype, { const data = await res.json(); if (!data.success) throw new Error(data.error || 'Failed to start Codex'); - if (data.sessionId) { - await this.selectSession(data.sessionId); + // Switch to the new session (don't pre-set activeSessionId — selectSession + // early-returns when IDs match, skipping buffer load and sendResize) + if (data.data.sessionId) { + await this.selectSession(data.data.sessionId); } this.terminal.focus(); diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 1b9198da..5c72773d 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -1064,6 +1064,11 @@ body.solo-mode .btn-lifecycle-log { color: #10b981; } +.session-tab .tab-mode.codex { + background: rgba(168, 85, 247, 0.2); + color: #a855f7; +} + /* Timer Banner - Compact */ .timer-banner { display: flex; @@ -2745,6 +2750,22 @@ body.solo-mode .btn-lifecycle-log { color: #a7f3d0; } +/* Codex mode colors */ +.btn-toolbar.btn-run.mode-codex, +.btn-toolbar.btn-run-gear.mode-codex { + background: linear-gradient(135deg, #2a0a3e 0%, #350b4d 50%, #400d5e 100%); + border-color: rgba(168, 85, 247, 0.5); + color: #d8b4fe; + box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2), inset 0 1px 0 rgba(255, 255, 255, 0.06); +} +.btn-toolbar.btn-run.mode-codex:hover, +.btn-toolbar.btn-run-gear.mode-codex:hover { + background: linear-gradient(135deg, #400d5e 0%, #581c87 50%, #6b21a8 100%); + box-shadow: 0 0 12px rgba(168, 85, 247, 0.35), 0 2px 8px rgba(168, 85, 247, 0.2), inset 0 1px 0 rgba(255, 255, 255, 0.08); + border-color: rgba(192, 132, 252, 0.6); + color: #e9d5ff; +} + /* Dropdown menu */ .run-mode-menu { display: none; @@ -2797,6 +2818,7 @@ body.solo-mode .btn-lifecycle-log { } .run-mode-dot.claude { background: #3b82f6; } .run-mode-dot.opencode { background: #10b981; } +.run-mode-dot.codex { background: #a855f7; } .run-mode-sep { height: 1px; diff --git a/src/web/routes/ralph-routes.ts b/src/web/routes/ralph-routes.ts index 615858f4..ab971a51 100644 --- a/src/web/routes/ralph-routes.ts +++ b/src/web/routes/ralph-routes.ts @@ -9,7 +9,7 @@ import { join, dirname, resolve, relative, isAbsolute } from 'node:path'; import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; import fs from 'node:fs/promises'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js'; -import { Session } from '../../session.js'; +import { Session, isExternalCliMode } from '../../session.js'; import { RespawnController } from '../../respawn-controller.js'; import { RalphConfigSchema, FixPlanImportSchema, RalphPromptWriteSchema, RalphLoopStartSchema } from '../schemas.js'; import { SseEvent } from '../sse-events.js'; @@ -44,9 +44,12 @@ export function registerRalphRoutes( }; const session = findSessionOrFail(ctx, id); - // Ralph tracker is not supported for opencode sessions - if (session.mode === 'opencode') { - return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Ralph tracker is not supported for opencode sessions'); + // Ralph tracker is not supported for external-CLI sessions (opencode/codex) + if (isExternalCliMode(session.mode)) { + return createErrorResponse( + ApiErrorCode.INVALID_INPUT, + `Ralph tracker is not supported for ${session.mode} sessions` + ); } // Handle reset first (before other config) diff --git a/src/web/routes/respawn-routes.ts b/src/web/routes/respawn-routes.ts index 15749437..834ee3bf 100644 --- a/src/web/routes/respawn-routes.ts +++ b/src/web/routes/respawn-routes.ts @@ -11,6 +11,7 @@ import { SseEvent } from '../sse-events.js'; import { findSessionOrFail, autoConfigureRalph, parseBody } from '../route-helpers.js'; import type { SessionPort, EventPort, RespawnPort, ConfigPort, InfraPort } from '../ports/index.js'; import { getLifecycleLog } from '../../session-lifecycle-log.js'; +import { isExternalCliMode } from '../../session.js'; import { AI_CHECK_MODEL, AI_IDLE_CHECK_MAX_CONTEXT, @@ -88,9 +89,9 @@ export function registerRespawnRoutes( } const session = findSessionOrFail(ctx, id); - // Respawn is not supported for opencode sessions - if (session.mode === 'opencode') { - return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Respawn is not supported for opencode sessions'); + // Respawn is not supported for external-CLI sessions (opencode/codex) + if (isExternalCliMode(session.mode)) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, `Respawn is not supported for ${session.mode} sessions`); } // Create or get existing controller @@ -231,9 +232,9 @@ export function registerRespawnRoutes( return createErrorResponse(ApiErrorCode.SESSION_BUSY, 'Session is busy'); } - // Respawn is not supported for opencode sessions - if (session.mode === 'opencode') { - return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Respawn is not supported for opencode sessions'); + // Respawn is not supported for external-CLI sessions (opencode/codex) + if (isExternalCliMode(session.mode)) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, `Respawn is not supported for ${session.mode} sessions`); } try { @@ -296,9 +297,9 @@ export function registerRespawnRoutes( const body = reResult.data as { config?: Partial; durationMinutes?: number }; const session = findSessionOrFail(ctx, id); - // Respawn is not supported for opencode sessions - if (session.mode === 'opencode') { - return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Respawn is not supported for opencode sessions'); + // Respawn is not supported for external-CLI sessions (opencode/codex) + if (isExternalCliMode(session.mode)) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, `Respawn is not supported for ${session.mode} sessions`); } // Check if session is running (has a PID) diff --git a/src/web/server.ts b/src/web/server.ts index 67f776c2..3b91feb7 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -42,7 +42,7 @@ import { execSync } from 'node:child_process'; import { hostname as getHostname } from 'node:os'; import { dataPath } from '../config/instance.js'; import { EventEmitter } from 'node:events'; -import { Session, type BackgroundTask } from '../session.js'; +import { Session, isExternalCliMode, type BackgroundTask } from '../session.js'; import type { ClaudeMode, SessionState } from '../types.js'; import { RespawnController, RespawnConfig } from '../respawn-controller.js'; import type { TerminalMultiplexer } from '../mux-interface.js'; @@ -1189,8 +1189,8 @@ export class WebServer extends EventEmitter { this.runSummaryTrackers.set(session.id, summaryTracker); summaryTracker.recordSessionStarted(session.mode, session.workingDir); - // Set working directory for Ralph tracker to auto-load @fix_plan.md (not supported for opencode sessions) - if (session.mode !== 'opencode') { + // Set working directory for Ralph tracker to auto-load @fix_plan.md (not supported for external CLIs) + if (!isExternalCliMode(session.mode)) { session.ralphTracker.setWorkingDir(session.workingDir); } @@ -2016,8 +2016,8 @@ export class WebServer extends EventEmitter { ); } } - // Ralph / Todo tracker (not supported for opencode sessions) - if (session.mode !== 'opencode') { + // Ralph / Todo tracker (not supported for external-CLI sessions) + if (!isExternalCliMode(session.mode)) { if (savedState.ralphAutoEnableDisabled) { session.ralphTracker.disableAutoEnable(); console.log(`[Server] Restored Ralph auto-enable disabled for session ${session.id}`); @@ -2046,8 +2046,8 @@ export class WebServer extends EventEmitter { if (savedState.flickerFilterEnabled !== undefined) { session.flickerFilterEnabled = savedState.flickerFilterEnabled; } - // Respawn controller (not supported for opencode sessions) - if (session.mode !== 'opencode' && savedState.respawnEnabled && savedState.respawnConfig) { + // Respawn controller (not supported for external-CLI sessions) + if (!isExternalCliMode(session.mode) && savedState.respawnEnabled && savedState.respawnConfig) { try { this.restoreRespawnController(session, savedState.respawnConfig, 'state.json'); } catch (err) { @@ -2056,9 +2056,9 @@ export class WebServer extends EventEmitter { } } - // Fallback: restore respawn from mux-sessions.json if state.json didn't have it (not supported for opencode) + // Fallback: restore respawn from mux-sessions.json if state.json didn't have it (not supported for external CLIs) if ( - session.mode !== 'opencode' && + !isExternalCliMode(session.mode) && !this.respawnControllers.has(session.id) && muxSession.respawnConfig?.enabled ) { @@ -2073,9 +2073,9 @@ export class WebServer extends EventEmitter { } // Fallback: restore Ralph state from state-inner.json if not already set and not explicitly disabled - // Ralph tracker is not supported for opencode sessions + // Ralph tracker is not supported for external-CLI sessions if ( - session.mode !== 'opencode' && + !isExternalCliMode(session.mode) && !session.ralphTracker.enabled && !session.ralphTracker.autoEnableDisabled ) { @@ -2086,9 +2086,9 @@ export class WebServer extends EventEmitter { } } - // Fallback: auto-detect completion phrase from CLAUDE.md (not supported for opencode) + // Fallback: auto-detect completion phrase from CLAUDE.md (not supported for external CLIs) if ( - session.mode !== 'opencode' && + !isExternalCliMode(session.mode) && session.ralphTracker.enabled && !session.ralphTracker.loopState.completionPhrase ) { diff --git a/test/run-mode-ui.test.ts b/test/run-mode-ui.test.ts index 30ee35e1..3624b4f2 100644 --- a/test/run-mode-ui.test.ts +++ b/test/run-mode-ui.test.ts @@ -97,10 +97,12 @@ describe('Codex quick start settings', () => { document: { getElementById: (id: string) => elements[id] ?? null, }, + // Mock responses use the real wire shape: the global preSerialization hook in + // server.ts wraps route payloads into the { success, data } envelope. fetch: async (url: string, init?: { body?: string }) => { requests.push({ url, body: init?.body ? JSON.parse(init.body) : undefined }); - if (url === '/api/codex/status') return { json: async () => ({ available: true }) }; - if (url === '/api/quick-start') return { json: async () => ({ success: true, sessionId: 'sess-1' }) }; + if (url === '/api/codex/status') return { json: async () => ({ success: true, data: { available: true } }) }; + if (url === '/api/quick-start') return { json: async () => ({ success: true, data: { sessionId: 'sess-1' } }) }; throw new Error(`unexpected fetch: ${url}`); }, console, @@ -116,7 +118,10 @@ describe('Codex quick start settings', () => { }); app.getCaseSettings = () => ({}); app.buildEnvOverrides = () => ({}); - app.selectSession = async () => {}; + const selected: string[] = []; + app.selectSession = async (id: string) => { + selected.push(id); + }; await app.runCodex(); @@ -125,5 +130,6 @@ describe('Codex quick start settings', () => { mode: 'codex', codexConfig: { dangerouslyBypassApprovals: true, renderMode: 'hybrid' }, }); + expect(selected).toEqual(['sess-1']); }); });