Skip to content

Commit 881ca21

Browse files
committed
codex: try --profile, fall back on codex's rejection error
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) and utilities reject it up front: Error: --profile only applies to runtime commands and `codex mcp`: ... so e.g. `ucode codex app-server` was functionally broken. Rather than maintain a version-dependent allow-list of which subcommands accept `--profile` (the accepted set drifts across codex releases), catch codex's own rejection: launch() spawns codex with `--profile`, capturing stderr to a temp file, and reads it once codex exits. Only a nonzero exit whose stderr contains the rejection message relaunches without `--profile` (swallowing the expected noise); any other outcome replays the captured stderr and propagates the exit code unchanged, so an unrelated error is never silently retried without ucode's routing. A temp file, unlike a pipe, has no backpressure, so codex can never block on a stderr write. The rejection is instant and side-effect-free (verified on codex 0.137 and 0.141), so the fallback costs ~150ms once. Inlined directly in launch() (no separate helper); launcher.py is untouched. Co-authored-by: Isaac
1 parent a0a0d1c commit 881ca21

2 files changed

Lines changed: 118 additions & 13 deletions

File tree

src/ucode/agents/codex.py

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

55
import os
66
import re
7+
import signal
8+
import subprocess
9+
import sys
10+
import tempfile
711
from pathlib import Path
812

913
from ucode.agent_updates import available_npm_package_update
@@ -391,12 +395,49 @@ def _gpt_version_key(entry: tuple[str, tuple[int, int | None, int | None, str]])
391395
return max(parsed, key=_gpt_version_key)[0]
392396

393397

398+
# Stderr prefix codex prints (and exits nonzero) when --profile is passed to a
399+
# subcommand that doesn't accept it. Stable across codex 0.137 and 0.141.
400+
_PROFILE_REJECTED_MARKER = "--profile only applies to runtime commands"
401+
_STDERR_HEAD_LIMIT = 64 * 1024 # bytes of stderr to scan for the marker
402+
403+
394404
def launch(state: dict, tool_args: list[str]) -> None:
395405
binary = SPEC["binary"]
396406
workspace = state.get("workspace")
397407
if workspace:
398408
os.environ["OAUTH_TOKEN"] = get_databricks_token(workspace, state.get("profile"))
399-
exec_or_spawn([binary, "--profile", CODEX_PROFILE_NAME, *tool_args])
409+
# Spawn codex with --profile first — the TUI and runtime subcommands
410+
# (exec/resume/mcp/...) keep ucode's routing, including subcommands added by
411+
# future codex versions. codex rejects the global --profile on server-family
412+
# subcommands (app-server, mcp-server, ...), which are caller-configured
413+
# anyway (e.g. omnigent runs `codex app-server` with its own CODEX_HOME); on
414+
# exactly that rejection, relaunch without --profile. Any other failure
415+
# propagates unchanged, so an unrelated error is never silently retried
416+
# without ucode's routing. codex's stderr goes to a temp file — unlike a
417+
# pipe it has no backpressure, so codex can't block on it — read once codex
418+
# exits (the trade: its stderr surfaces at exit, not live).
419+
with tempfile.TemporaryFile() as stderr_file:
420+
proc = subprocess.Popen(
421+
[binary, "--profile", CODEX_PROFILE_NAME, *tool_args], stderr=stderr_file
422+
)
423+
try:
424+
returncode = proc.wait()
425+
except KeyboardInterrupt:
426+
# Forward Ctrl-C and let codex set its own exit code (mirrors
427+
# exec_or_spawn's Windows path) rather than racing it.
428+
proc.send_signal(signal.SIGINT)
429+
returncode = proc.wait()
430+
stderr_file.seek(0)
431+
head = stderr_file.read(_STDERR_HEAD_LIMIT)
432+
if returncode != 0 and _PROFILE_REJECTED_MARKER.encode() in head:
433+
exec_or_spawn([binary, *tool_args]) # swallow the expected rejection
434+
return # unreachable in production (exec/exit); keeps tests honest
435+
# Replay codex's stderr and propagate its exit code.
436+
stderr_file.seek(0)
437+
for chunk in iter(lambda: stderr_file.read(65536), b""):
438+
sys.stderr.write(chunk.decode(errors="replace"))
439+
sys.stderr.flush()
440+
sys.exit(returncode)
400441

401442

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

tests/test_agent_codex.py

Lines changed: 76 additions & 12 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,85 @@ 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+
"""launch() spawns codex with --profile first, and relaunches without it only
422+
on codex's --profile-rejection error. A fake Popen stands in for codex: its
423+
wait() writes the given stderr into the temp file launch() reads."""
424+
425+
@staticmethod
426+
def _patch(monkeypatch, *, returncode: int, stderr: bytes = b""):
427+
spawned: list[list[str]] = []
428+
fallbacks: list[list[str]] = []
429+
430+
class _Proc:
431+
def __init__(self, stderr_file):
432+
self._stderr_file = stderr_file
433+
434+
def wait(self):
435+
self._stderr_file.write(stderr)
436+
self._stderr_file.flush()
437+
return returncode
438+
439+
def send_signal(self, sig): # pragma: no cover - interrupt not exercised
440+
pass
421441

422-
def fake_execvp(binary: str, args: list[str]) -> None:
423-
exec_calls.append((binary, args))
424-
raise RuntimeError("stop")
442+
def fake_popen(argv, stderr=None, **kwargs):
443+
spawned.append(argv)
444+
return _Proc(stderr)
425445

446+
monkeypatch.setattr(codex.subprocess, "Popen", fake_popen)
447+
monkeypatch.setattr(codex, "exec_or_spawn", lambda argv: fallbacks.append(argv))
448+
monkeypatch.setattr(codex, "get_databricks_token", lambda workspace, profile=None: "tok")
449+
return spawned, fallbacks
450+
451+
def test_sets_oauth_token_and_spawns_with_profile(self, monkeypatch):
426452
monkeypatch.delenv("OAUTH_TOKEN", raising=False)
453+
spawned, fallbacks = self._patch(monkeypatch, returncode=0)
427454
monkeypatch.setattr(
428455
codex, "get_databricks_token", lambda workspace, profile=None: "fresh-token"
429456
)
430-
monkeypatch.setattr(os, "execvp", fake_execvp)
431-
432-
try:
457+
with pytest.raises(SystemExit) as exc:
433458
codex.launch({"workspace": WS}, ["--search"])
434-
except RuntimeError as exc:
435-
assert str(exc) == "stop"
436-
459+
assert exc.value.code == 0
437460
assert os.environ["OAUTH_TOKEN"] == "fresh-token"
438-
assert exec_calls == [("codex", ["codex", "--profile", "ucode", "--search"])]
461+
assert spawned == [["codex", "--profile", "ucode", "--search"]]
462+
assert fallbacks == []
463+
464+
def test_success_propagates_exit_and_replays_stderr(self, monkeypatch, capsys):
465+
spawned, fallbacks = self._patch(monkeypatch, returncode=0, stderr=b"warn: heads up\n")
466+
with pytest.raises(SystemExit) as exc:
467+
codex.launch({"workspace": WS}, ["exec", "hi"])
468+
assert exc.value.code == 0
469+
assert spawned == [["codex", "--profile", "ucode", "exec", "hi"]]
470+
assert fallbacks == []
471+
assert "warn: heads up" in capsys.readouterr().err
472+
473+
def test_profile_rejection_relaunches_without_profile_and_swallows_it(
474+
self, monkeypatch, capsys
475+
):
476+
marker = f"Error: {codex._PROFILE_REJECTED_MARKER} and `codex mcp`\n".encode()
477+
for args in (["app-server", "--listen", "u"], ["mcp-server"]):
478+
spawned, fallbacks = self._patch(monkeypatch, returncode=1, stderr=marker)
479+
codex.launch({"workspace": WS}, args)
480+
assert spawned == [["codex", "--profile", "ucode", *args]]
481+
assert fallbacks == [["codex", *args]]
482+
# The expected rejection is noise once we relaunch — swallowed.
483+
assert capsys.readouterr().err == ""
484+
485+
def test_unrelated_failure_does_not_fall_back(self, monkeypatch, capsys):
486+
# An unmarked failure must NOT retry without --profile — that would
487+
# silently drop ucode's routing.
488+
spawned, fallbacks = self._patch(monkeypatch, returncode=3, stderr=b"boom\n")
489+
with pytest.raises(SystemExit) as exc:
490+
codex.launch({"workspace": WS}, ["exec", "hi"])
491+
assert exc.value.code == 3
492+
assert fallbacks == []
493+
assert "boom" in capsys.readouterr().err
494+
495+
def test_marker_on_zero_exit_does_not_fall_back(self, monkeypatch, capsys):
496+
# The marker only triggers the fallback paired with a nonzero exit.
497+
marker = f"{codex._PROFILE_REJECTED_MARKER}\n".encode()
498+
spawned, fallbacks = self._patch(monkeypatch, returncode=0, stderr=marker)
499+
with pytest.raises(SystemExit) as exc:
500+
codex.launch({"workspace": WS}, [])
501+
assert exc.value.code == 0
502+
assert fallbacks == []

0 commit comments

Comments
 (0)