Skip to content

Commit 52830b0

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: every launch runs with `--profile` first, stderr captured to a temp file and read once the process 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 file, unlike a pipe, has no backpressure, so nothing has to read alongside the running process. The rejection is instant and side-effect-free (verified on codex 0.137 and 0.141), so the fallback costs ~150ms once. The helper is codex-local (_exec_or_spawn_with_fallback in codex.py) rather than a launcher.py API — it's not generalizable yet; launcher.py is untouched. Co-authored-by: Isaac
1 parent a0a0d1c commit 52830b0

2 files changed

Lines changed: 155 additions & 14 deletions

File tree

src/ucode/agents/codex.py

Lines changed: 68 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,75 @@ 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+
# codex accepts the global --profile only on its runtime subcommands (plus the
399+
# bare interactive TUI); server-family subcommands (app-server, mcp-server,
400+
# exec-server, remote-control) and utilities reject it up front by printing
401+
# this to stderr and exiting nonzero. The accepted set drifts across codex
402+
# versions, so rather than maintain a version-dependent list we try with
403+
# --profile and fall back exactly on this rejection. Stable prefix verified on
404+
# codex 0.137 and 0.141; if a future codex rewords it, the launch fails loudly
405+
# with codex's own error instead of silently dropping ucode's routing.
406+
_PROFILE_REJECTED_MARKER = "--profile only applies to runtime commands"
407+
408+
# How much of codex's captured stderr to scan for the marker. The rejection is
409+
# one short line printed first, so this is generous headroom.
410+
_STDERR_HEAD_LIMIT = 64 * 1024
411+
412+
413+
def _exec_or_spawn_with_fallback(
414+
argv: list[str], fallback_argv: list[str], stderr_marker: str
415+
) -> None:
416+
"""``exec_or_spawn``, but relaunch as ``fallback_argv`` iff codex rejects a flag.
417+
418+
Runs ``argv`` with stderr captured to a temp file and reads it once the
419+
process exits. Only a nonzero exit whose stderr contains ``stderr_marker``
420+
— codex's own rejection message — execs ``fallback_argv`` in its place,
421+
swallowing the expected rejection noise. Every other outcome (success, or
422+
a failure for any other reason) replays the captured stderr and propagates
423+
the exit code unchanged, so an unrelated error is never silently retried
424+
with different flags.
425+
426+
A file, unlike a pipe, has no backpressure: codex can never block on a
427+
stderr write, so nothing has to read alongside the running process. The
428+
trade is that codex's stderr appears when it exits, not live.
429+
"""
430+
with tempfile.TemporaryFile() as stderr_file:
431+
proc = subprocess.Popen(argv, stderr=stderr_file)
432+
try:
433+
returncode = proc.wait()
434+
except KeyboardInterrupt:
435+
# Forward Ctrl-C and let codex decide its own exit code rather
436+
# than racing it (mirrors exec_or_spawn's Windows path).
437+
proc.send_signal(signal.SIGINT)
438+
returncode = proc.wait()
439+
stderr_file.seek(0)
440+
if returncode != 0 and stderr_marker.encode() in stderr_file.read(_STDERR_HEAD_LIMIT):
441+
exec_or_spawn(fallback_argv)
442+
return # unreachable in production (exec/exit); keeps tests honest
443+
# Replay everything codex wrote to stderr.
444+
stderr_file.seek(0)
445+
for chunk in iter(lambda: stderr_file.read(65536), b""):
446+
sys.stderr.write(chunk.decode(errors="replace"))
447+
sys.stderr.flush()
448+
sys.exit(returncode)
449+
450+
394451
def launch(state: dict, tool_args: list[str]) -> None:
395452
binary = SPEC["binary"]
396453
workspace = state.get("workspace")
397454
if workspace:
398455
os.environ["OAUTH_TOKEN"] = get_databricks_token(workspace, state.get("profile"))
399-
exec_or_spawn([binary, "--profile", CODEX_PROFILE_NAME, *tool_args])
456+
# Try with --profile first — the TUI and runtime subcommands
457+
# (exec/resume/mcp/...) keep ucode's routing, including subcommands added
458+
# by future codex versions — and relaunch without it only on codex's
459+
# specific rejection error. Rejected subcommands are caller-configured
460+
# anyway (e.g. omnigent runs `codex app-server` with its own CODEX_HOME),
461+
# so omitting --profile there just stays out of the way.
462+
_exec_or_spawn_with_fallback(
463+
[binary, "--profile", CODEX_PROFILE_NAME, *tool_args],
464+
[binary, *tool_args],
465+
_PROFILE_REJECTED_MARKER,
466+
)
400467

401468

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

tests/test_agent_codex.py

Lines changed: 87 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
from __future__ import annotations
44

55
import os
6+
import sys
7+
8+
import pytest
69

710
from ucode.agents import codex
811
from ucode.config_io import read_toml_safe
@@ -416,23 +419,94 @@ def test_skips_git_repo_check(self):
416419

417420

418421
class TestCodexLaunch:
419-
def test_sets_oauth_token_and_ucode_profile_before_exec(self, monkeypatch):
420-
exec_calls: list[tuple[str, list[str]]] = []
422+
"""Every launch goes through the try-with-profile / fall-back-on-rejection
423+
path instead of a version-dependent allow-list."""
421424

422-
def fake_execvp(binary: str, args: list[str]) -> None:
423-
exec_calls.append((binary, args))
424-
raise RuntimeError("stop")
425+
@staticmethod
426+
def _capture(monkeypatch):
427+
calls: list[tuple[list[str], list[str], str]] = []
428+
monkeypatch.setattr(
429+
codex,
430+
"_exec_or_spawn_with_fallback",
431+
lambda argv, fallback_argv, marker: calls.append((argv, fallback_argv, marker)),
432+
)
433+
monkeypatch.setattr(codex, "get_databricks_token", lambda workspace, profile=None: "tok")
434+
return calls
425435

436+
def test_sets_oauth_token_before_launch(self, monkeypatch):
426437
monkeypatch.delenv("OAUTH_TOKEN", raising=False)
438+
calls = self._capture(monkeypatch)
427439
monkeypatch.setattr(
428440
codex, "get_databricks_token", lambda workspace, profile=None: "fresh-token"
429441
)
430-
monkeypatch.setattr(os, "execvp", fake_execvp)
431-
432-
try:
433-
codex.launch({"workspace": WS}, ["--search"])
434-
except RuntimeError as exc:
435-
assert str(exc) == "stop"
436-
442+
codex.launch({"workspace": WS}, ["--search"])
437443
assert os.environ["OAUTH_TOKEN"] == "fresh-token"
438-
assert exec_calls == [("codex", ["codex", "--profile", "ucode", "--search"])]
444+
assert calls[0][0] == ["codex", "--profile", "ucode", "--search"]
445+
446+
def test_tries_profile_first_with_no_profile_fallback(self, monkeypatch):
447+
for args in ([], ["--search"], ["exec", "say hi"], ["app-server", "--listen", "u"]):
448+
calls = self._capture(monkeypatch)
449+
codex.launch({"workspace": WS}, args)
450+
assert calls == [
451+
(
452+
["codex", "--profile", "ucode", *args],
453+
["codex", *args],
454+
codex._PROFILE_REJECTED_MARKER,
455+
)
456+
]
457+
458+
459+
class TestExecOrSpawnWithFallback:
460+
"""Real-subprocess tests: stderr is captured to a file and read at exit."""
461+
462+
@staticmethod
463+
def _script(code: str) -> list[str]:
464+
return [sys.executable, "-c", code]
465+
466+
def test_marked_failure_execs_fallback_and_swallows_rejection(self, monkeypatch, capsys):
467+
calls: list[list[str]] = []
468+
monkeypatch.setattr(codex, "exec_or_spawn", lambda argv: calls.append(argv))
469+
codex._exec_or_spawn_with_fallback(
470+
self._script("import sys; sys.stderr.write('Error: MARKER hit\\n'); sys.exit(1)"),
471+
["fallback", "argv"],
472+
"MARKER",
473+
)
474+
assert calls == [["fallback", "argv"]]
475+
# The expected rejection is noise once we relaunch — swallowed.
476+
assert capsys.readouterr().err == ""
477+
478+
def test_success_propagates_and_replays_stderr(self, monkeypatch, capsys):
479+
monkeypatch.setattr(codex, "exec_or_spawn", lambda argv: pytest.fail("fallback fired"))
480+
with pytest.raises(SystemExit) as exc:
481+
codex._exec_or_spawn_with_fallback(
482+
self._script("import sys; sys.stderr.write('warn: heads up\\n')"),
483+
["fallback"],
484+
"MARKER",
485+
)
486+
assert exc.value.code == 0
487+
# Whatever the child said on stderr reaches the caller at exit.
488+
assert "warn: heads up" in capsys.readouterr().err
489+
490+
def test_unrelated_failure_propagates_without_fallback(self, monkeypatch, capsys):
491+
# An unmarked failure must NOT retry with different flags — retrying
492+
# would silently drop --profile and ucode's routing with it.
493+
monkeypatch.setattr(codex, "exec_or_spawn", lambda argv: pytest.fail("fallback fired"))
494+
with pytest.raises(SystemExit) as exc:
495+
codex._exec_or_spawn_with_fallback(
496+
self._script("import sys; sys.stderr.write('boom\\n'); sys.exit(3)"),
497+
["fallback"],
498+
"MARKER",
499+
)
500+
assert exc.value.code == 3
501+
assert "boom" in capsys.readouterr().err
502+
503+
def test_marker_on_success_does_not_fall_back(self, monkeypatch, capsys):
504+
monkeypatch.setattr(codex, "exec_or_spawn", lambda argv: pytest.fail("fallback fired"))
505+
with pytest.raises(SystemExit) as exc:
506+
codex._exec_or_spawn_with_fallback(
507+
self._script("import sys; sys.stderr.write('mentions MARKER\\n'); sys.exit(0)"),
508+
["fallback"],
509+
"MARKER",
510+
)
511+
assert exc.value.code == 0
512+
assert "mentions MARKER" in capsys.readouterr().err

0 commit comments

Comments
 (0)