Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion plugins/codex/scripts/lib/app-server.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
});

Expand Down
17 changes: 17 additions & 0 deletions plugins/codex/scripts/lib/codex.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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/)
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 10 additions & 0 deletions plugins/codex/scripts/lib/git.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand Down
9 changes: 8 additions & 1 deletion plugins/codex/scripts/lib/process.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,21 @@ 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,
encoding: "utf8",
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
});

Expand Down