Skip to content

Commit 4d5de9c

Browse files
committed
codex: retry without --profile when the first attempt fails fast
ucode's codex launch always ran `codex --profile ucode <args>`, but codex only accepts the global `--profile` on runtime subcommands. Server-family subcommands (app-server, mcp-server, exec-server, remote-control) reject it up front: Error: --profile only applies to runtime commands and `codex mcp`: ... so e.g. `ucode codex app-server` was functionally broken. Run codex with `--profile` first; if that attempt exits nonzero *and* does so fast (< 3s), relaunch without `--profile`. No stderr capture, no temp file, no subcommand allow-list to maintain. The signal is timing, not the error text: codex's `--profile` rejection is a CLI parse-time error (~0.15s, before it touches auth/gateway/network), whereas a session that actually starts can only fail after a network round-trip (seconds) — there is no overlap, and the exit code alone can't tell them apart (both are 1). The fast-failure gate is what keeps this strictly better than the status quo: without it, a genuinely-failing `codex exec` would be silently re-run without `--profile` — i.e. on the user's own OpenAI login, since ucode writes a *named-profile* file and no `--profile` means no ucode routing. stdio is inherited (no capture), so Ctrl-C reaches codex directly and quitting an interactive session propagates a KeyboardInterrupt past the retry check rather than tripping it. Co-authored-by: Isaac
1 parent a0a0d1c commit 4d5de9c

2 files changed

Lines changed: 107 additions & 15 deletions

File tree

src/ucode/agents/codex.py

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
import os
66
import re
7+
import subprocess
8+
import sys
9+
import time
710
from pathlib import Path
811

912
from ucode.agent_updates import available_npm_package_update
@@ -391,12 +394,48 @@ def _gpt_version_key(entry: tuple[str, tuple[int, int | None, int | None, str]])
391394
return max(parsed, key=_gpt_version_key)[0]
392395

393396

397+
# codex rejects the global --profile on subcommands that don't accept it
398+
# (app-server, mcp-server, ...) with a CLI *parse-time* error — before it touches
399+
# auth, the gateway, or the network — so the rejection exits almost instantly.
400+
# We use that to decide when to retry without --profile (see launch()). This
401+
# window is well above codex's ~0.15s cold-start floor and far below the seconds
402+
# any real session needs to connect and then fail, so it never catches a genuine
403+
# failure. Its exit code (1) is indistinguishable from an ordinary failure, so
404+
# elapsed time is the signal we key on rather than stderr text.
405+
_PROFILE_REJECTED_MAX_SECONDS = 3.0
406+
407+
394408
def launch(state: dict, tool_args: list[str]) -> None:
395409
binary = SPEC["binary"]
396410
workspace = state.get("workspace")
397411
if workspace:
398412
os.environ["OAUTH_TOKEN"] = get_databricks_token(workspace, state.get("profile"))
399-
exec_or_spawn([binary, "--profile", CODEX_PROFILE_NAME, *tool_args])
413+
# Run codex with --profile first — the TUI and runtime subcommands
414+
# (exec/resume/mcp/...) keep ucode's Databricks routing, including any added
415+
# by future codex versions. codex rejects the global --profile on
416+
# server-family subcommands (app-server, mcp-server, ...), which are
417+
# caller-configured anyway (e.g. omnigent runs `codex app-server` with its
418+
# own CODEX_HOME); on that rejection we relaunch without --profile.
419+
#
420+
# The retry is gated on the attempt failing *fast*: the rejection is a
421+
# parse-time error (~0.15s), whereas a session that actually starts can only
422+
# fail after a network round-trip (seconds). Without that gate a genuinely
423+
# failing `codex exec` would be silently re-run without --profile — i.e. on
424+
# the user's own OpenAI login instead of the Databricks gateway (ucode writes
425+
# a *named-profile* file, so no --profile means no ucode routing). stdio is
426+
# inherited (no capture), so Ctrl-C reaches codex directly and the resulting
427+
# KeyboardInterrupt propagates past the retry check — quitting an interactive
428+
# session is never mistaken for a --profile rejection.
429+
started = time.monotonic()
430+
returncode = subprocess.run([binary, "--profile", CODEX_PROFILE_NAME, *tool_args]).returncode
431+
if returncode != 0 and time.monotonic() - started < _PROFILE_REJECTED_MAX_SECONDS:
432+
# Fast failure: most likely codex rejected --profile on this subcommand.
433+
# Relaunch without it, handing over the terminal. (A fast failure for
434+
# any other reason — e.g. a bad flag — just re-fails the same way here,
435+
# with no ucode routing to lose since the subcommand had none.)
436+
exec_or_spawn([binary, *tool_args])
437+
return # unreachable in production (exec replaces the process)
438+
sys.exit(returncode)
400439

401440

402441
def validate_cmd(binary: str) -> list[str]:

tests/test_agent_codex.py

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import os
66

7+
import pytest
8+
79
from ucode.agents import codex
810
from ucode.config_io import read_toml_safe
911

@@ -416,23 +418,74 @@ def test_skips_git_repo_check(self):
416418

417419

418420
class TestCodexLaunch:
419-
def test_sets_oauth_token_and_ucode_profile_before_exec(self, monkeypatch):
420-
exec_calls: list[tuple[str, list[str]]] = []
421-
422-
def fake_execvp(binary: str, args: list[str]) -> None:
423-
exec_calls.append((binary, args))
424-
raise RuntimeError("stop")
425-
421+
"""launch() runs codex with --profile first and relaunches without it only
422+
when that attempt fails *fast* — codex's --profile rejection is a parse-time
423+
error, so a fast nonzero exit means the subcommand didn't accept --profile.
424+
A slow failure is a real session error and is propagated unchanged."""
425+
426+
@staticmethod
427+
def _patch(monkeypatch, *, returncode: int, elapsed: float):
428+
"""Stub subprocess.run to return `returncode` and make launch() perceive
429+
`elapsed` seconds between its two time.monotonic() reads."""
430+
runs: list[list[str]] = []
431+
fallbacks: list[list[str]] = []
432+
433+
def fake_run(argv, **kwargs):
434+
runs.append(argv)
435+
return codex.subprocess.CompletedProcess(argv, returncode)
436+
437+
# launch() reads time.monotonic() once before run and once after.
438+
clock = iter([100.0, 100.0 + elapsed])
439+
monkeypatch.setattr(codex.subprocess, "run", fake_run)
440+
monkeypatch.setattr(codex.time, "monotonic", lambda: next(clock))
441+
monkeypatch.setattr(codex, "exec_or_spawn", lambda argv: fallbacks.append(argv))
442+
monkeypatch.setattr(codex, "get_databricks_token", lambda workspace, profile=None: "tok")
443+
return runs, fallbacks
444+
445+
def test_sets_oauth_token_and_runs_with_profile(self, monkeypatch):
426446
monkeypatch.delenv("OAUTH_TOKEN", raising=False)
447+
runs, fallbacks = self._patch(monkeypatch, returncode=0, elapsed=0.5)
427448
monkeypatch.setattr(
428449
codex, "get_databricks_token", lambda workspace, profile=None: "fresh-token"
429450
)
430-
monkeypatch.setattr(os, "execvp", fake_execvp)
431-
432-
try:
451+
with pytest.raises(SystemExit) as exc:
433452
codex.launch({"workspace": WS}, ["--search"])
434-
except RuntimeError as exc:
435-
assert str(exc) == "stop"
436-
453+
assert exc.value.code == 0
437454
assert os.environ["OAUTH_TOKEN"] == "fresh-token"
438-
assert exec_calls == [("codex", ["codex", "--profile", "ucode", "--search"])]
455+
assert runs == [["codex", "--profile", "ucode", "--search"]]
456+
assert fallbacks == []
457+
458+
def test_success_propagates_exit_without_retry(self, monkeypatch):
459+
runs, fallbacks = self._patch(monkeypatch, returncode=0, elapsed=0.2)
460+
with pytest.raises(SystemExit) as exc:
461+
codex.launch({"workspace": WS}, ["exec", "hi"])
462+
assert exc.value.code == 0
463+
assert runs == [["codex", "--profile", "ucode", "exec", "hi"]]
464+
assert fallbacks == []
465+
466+
def test_fast_failure_relaunches_without_profile(self, monkeypatch):
467+
# codex rejects --profile on server-family subcommands at parse time —
468+
# a fast nonzero exit → relaunch without --profile.
469+
for args in (["app-server", "--listen", "u"], ["mcp-server"]):
470+
runs, fallbacks = self._patch(monkeypatch, returncode=1, elapsed=0.15)
471+
codex.launch({"workspace": WS}, args)
472+
assert runs == [["codex", "--profile", "ucode", *args]]
473+
assert fallbacks == [["codex", *args]]
474+
475+
def test_slow_failure_does_not_retry(self, monkeypatch):
476+
# A session that started and then failed (seconds) must NOT be re-run
477+
# without --profile — that would silently drop ucode's Databricks routing
478+
# (relaunching the user's prompt on their own OpenAI login).
479+
runs, fallbacks = self._patch(monkeypatch, returncode=1, elapsed=8.0)
480+
with pytest.raises(SystemExit) as exc:
481+
codex.launch({"workspace": WS}, ["exec", "hi"])
482+
assert exc.value.code == 1
483+
assert fallbacks == []
484+
485+
def test_fast_success_does_not_retry(self, monkeypatch):
486+
# The retry is gated on a *nonzero* exit; a fast clean exit just returns.
487+
runs, fallbacks = self._patch(monkeypatch, returncode=0, elapsed=0.15)
488+
with pytest.raises(SystemExit) as exc:
489+
codex.launch({"workspace": WS}, [])
490+
assert exc.value.code == 0
491+
assert fallbacks == []

0 commit comments

Comments
 (0)