mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 20:49:41 +02:00
fix(codex): review fixes — envelope handling, mode guards, UI parity
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) <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -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';
|
||||
}
|
||||
|
||||
|
||||
@@ -2572,7 +2572,7 @@ class CodemanApp {
|
||||
<span class="tab-status ${status}" aria-hidden="true"></span>
|
||||
<span class="tab-info">
|
||||
<span class="tab-name-row">
|
||||
${mode === 'shell' ? '<span class="tab-mode shell" aria-hidden="true">sh</span>' : mode === 'opencode' ? '<span class="tab-mode opencode" aria-hidden="true">oc</span>' : ''}
|
||||
${mode === 'shell' ? '<span class="tab-mode shell" aria-hidden="true">sh</span>' : mode === 'opencode' ? '<span class="tab-mode opencode" aria-hidden="true">oc</span>' : mode === 'codex' ? '<span class="tab-mode codex" aria-hidden="true">cx</span>' : ''}
|
||||
<span class="tab-name" data-session-id="${id}">${(() => { const p = parseSessionPrefix(name); return p && p.suffix ? '<span class="tab-prefix">' + escapeHtml(p.prefix) + '</span><span class="tab-suffix">: ' + escapeHtml(p.suffix) + '</span>' : escapeHtml(name); })()}</span>
|
||||
<span class="tab-detached-badge" aria-hidden="true">detached</span>
|
||||
</span>
|
||||
@@ -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');
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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<RespawnConfig>; 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)
|
||||
|
||||
+13
-13
@@ -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
|
||||
) {
|
||||
|
||||
Reference in New Issue
Block a user