diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index 127c8376..263791e0 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -186,11 +186,16 @@ class SpawnedCodexAppServerClient extends AppServerClientBase { } async initialize() { + // Pin to cmd.exe on Windows (NOT $SHELL) so the .cmd shim for codex is + // resolved via PATHEXT but no MSYS path translation kicks in on Git Bash. + // The original code used `process.env.SHELL || true`, which under Git Bash + // mangled Windows-style command flags. On POSIX, codex is a JS shebang + // file so direct spawn (shell:false) works. this.proc = spawn("codex", ["app-server"], { cwd: this.cwd, env: this.options.env ?? process.env, stdio: ["pipe", "pipe", "pipe"], - shell: process.platform === "win32" ? (process.env.SHELL || true) : false, + shell: process.platform === "win32" ? "cmd.exe" : false, windowsHide: true }); diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index f2fe88bd..baf4bd76 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -44,6 +44,17 @@ const TASK_THREAD_PREFIX = "Codex Companion Task"; const DEFAULT_CONTINUE_PROMPT = "Continue from the current thread state. Pick the next highest-value step and follow through until the task is resolved."; +// Permanent auth-fail patterns from the OpenAI API. The underlying Codex CLI's +// app-server retries indefinitely on these, but they never recover without +// re-authenticating, so we abort the captureTurn early. +const PERMANENT_AUTH_ERROR_RE = /\b(?:401\s+Unauthorized|403\s+Forbidden|Missing\s+bearer|invalid\s+api\s+key|authentication[_-]?failed)\b/i; + +function isPermanentAuthError(error) { + if (!error) return false; + const message = typeof error === "string" ? error : (error.message ?? ""); + return PERMANENT_AUTH_ERROR_RE.test(message); +} + function cleanCodexStderr(stderr) { return stderr .split(/\r?\n/) @@ -531,6 +542,12 @@ function applyTurnNotification(state, message) { case "error": state.error = message.params.error; emitProgress(state.onProgress, `Codex error: ${message.params.error.message}`, "failed"); + // Short-circuit on permanent auth failures so we don't waste API quota + // in the underlying CLI's reconnect loop (it retries 5x by default). + // 401/403 from OpenAI never recovers without re-authenticating. + if (isPermanentAuthError(message.params.error)) { + completeTurn(state, { id: state.turnId ?? "auth-failure", status: "failed" }); + } break; case "turn/completed": if ((message.params.threadId ?? null) !== state.threadId) { diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 1749cfc8..99328a70 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -140,6 +140,16 @@ export function resolveReviewTarget(cwd, options = {}) { const supportedScopes = new Set(["auto", "working-tree", "branch"]); if (baseRef) { + // Verify the ref exists locally before we kick off a Codex thread; without + // this check the user gets a confusing "Reconnecting..." API error several + // seconds later instead of the actual root cause (typo or unfetched ref). + const verify = git(cwd, ["rev-parse", "--verify", "--quiet", `${baseRef}^{commit}`]); + if (verify.status !== 0) { + throw new Error( + `Base ref "${baseRef}" does not resolve to a commit in this repository. ` + + `Did you mean a different branch, or do you need to fetch it first (e.g. \`git fetch origin ${baseRef}\`)?` + ); + } return { mode: "branch", label: `branch diff against ${baseRef}`, diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs index af28d1cf..eca6042d 100644 --- a/plugins/codex/scripts/lib/process.mjs +++ b/plugins/codex/scripts/lib/process.mjs @@ -2,6 +2,13 @@ import { spawnSync } from "node:child_process"; import process from "node:process"; export function runCommand(command, args = [], options = {}) { + // On Windows we MUST go through a shell to resolve npm-installed CLIs, which + // are .cmd shims (PATHEXT lookup is shell-only). The previous code used + // `process.env.SHELL || true`, which under Git Bash resolved to /usr/bin/bash + // and rewrote args like `/PID 123 /T /F` as Unix paths + // (`/c/Program Files/Git/PID 123 /T /F`), breaking taskkill. Pin to cmd.exe + // so we keep .cmd resolution but skip MSYS path translation. On POSIX + // shell:false is fine. const result = spawnSync(command, args, { cwd: options.cwd, env: options.env, @@ -9,7 +16,7 @@ export function runCommand(command, args = [], options = {}) { input: options.input, maxBuffer: options.maxBuffer, stdio: options.stdio ?? "pipe", - shell: process.platform === "win32" ? (process.env.SHELL || true) : false, + shell: process.platform === "win32" ? "cmd.exe" : false, windowsHide: true });