Skip to content

Commit 63da551

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: run the launch with `--profile` first, stderr captured to a temp file, and read it 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 bare interactive TUI (no subcommand token — a subcommand never starts with "-") still execs directly with --profile, which every codex version accepts. Co-authored-by: Isaac
1 parent a0a0d1c commit 63da551

4 files changed

Lines changed: 185 additions & 2 deletions

File tree

src/ucode/agents/codex.py

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
build_tool_base_url,
2121
get_databricks_token,
2222
)
23-
from ucode.launcher import exec_or_spawn
23+
from ucode.launcher import exec_or_spawn, run_with_stderr_fallback
2424
from ucode.state import mark_tool_managed, save_state
2525
from ucode.telemetry import agent_version, ucode_version
2626

@@ -391,12 +391,38 @@ def _gpt_version_key(entry: tuple[str, tuple[int, int | None, int | None, str]])
391391
return max(parsed, key=_gpt_version_key)[0]
392392

393393

394+
# codex accepts the global --profile only on its runtime subcommands (plus the
395+
# bare interactive TUI); server-family subcommands (app-server, mcp-server,
396+
# exec-server, remote-control) and utilities reject it up front by printing
397+
# this to stderr and exiting nonzero. The accepted set drifts across codex
398+
# versions, so rather than maintain a version-dependent list we try with
399+
# --profile and fall back exactly on this rejection. Stable prefix verified on
400+
# codex 0.137 and 0.141; if a future codex rewords it, the launch fails loudly
401+
# with codex's own error instead of silently dropping ucode's routing.
402+
_PROFILE_REJECTED_MARKER = "--profile only applies to runtime commands"
403+
404+
394405
def launch(state: dict, tool_args: list[str]) -> None:
395406
binary = SPEC["binary"]
396407
workspace = state.get("workspace")
397408
if workspace:
398409
os.environ["OAUTH_TOKEN"] = get_databricks_token(workspace, state.get("profile"))
399-
exec_or_spawn([binary, "--profile", CODEX_PROFILE_NAME, *tool_args])
410+
with_profile = [binary, "--profile", CODEX_PROFILE_NAME, *tool_args]
411+
if all(arg.startswith("-") for arg in tool_args):
412+
# No subcommand token (a subcommand never starts with "-"), so this is
413+
# the interactive TUI, which always accepts --profile: hand over the
414+
# terminal directly. A flag value without a dash (e.g. `--model gpt-5`)
415+
# merely routes the TUI through the supervised fallback below, which
416+
# accepts --profile just the same — never the reverse.
417+
exec_or_spawn(with_profile)
418+
return
419+
# A subcommand may reject the global --profile. Try with it first — so
420+
# runtime subcommands (exec/resume/mcp/...) keep ucode's routing, including
421+
# ones added by future codex versions — and relaunch without it only on
422+
# codex's specific rejection error. Rejected subcommands are caller-
423+
# configured anyway (e.g. omnigent runs `codex app-server` with its own
424+
# CODEX_HOME), so omitting --profile there just stays out of the way.
425+
run_with_stderr_fallback(with_profile, [binary, *tool_args], _PROFILE_REJECTED_MARKER)
400426

401427

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

src/ucode/launcher.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@
66
import signal
77
import subprocess
88
import sys
9+
import tempfile
10+
11+
# How much of the child's captured stderr to scan for the marker in
12+
# run_with_stderr_fallback. A flag-rejection error is one short line printed
13+
# first, so this is generous headroom.
14+
_STDERR_HEAD_LIMIT = 64 * 1024
915

1016

1117
def exec_or_spawn(argv: list[str]) -> None:
@@ -34,3 +40,41 @@ def exec_or_spawn(argv: list[str]) -> None:
3440
proc.send_signal(signal.SIGINT)
3541
returncode = proc.wait()
3642
sys.exit(returncode)
43+
44+
45+
def run_with_stderr_fallback(argv: list[str], fallback_argv: list[str], stderr_marker: str) -> None:
46+
"""Run ``argv``; hand the terminal to ``fallback_argv`` iff it rejects a flag.
47+
48+
For launches where the binary may reject one of our flags depending on its
49+
version or subcommand (e.g. codex's global ``--profile``): run the preferred
50+
``argv`` with stderr captured to a temp file, and read it once the process
51+
exits. Only when it exits nonzero *and* that stderr contains
52+
``stderr_marker`` — the binary's own rejection message — is ``fallback_argv``
53+
exec'd in its place, swallowing the expected rejection noise. Every other
54+
outcome (success, or a failure for any other reason) replays the captured
55+
stderr and propagates the child's exit code unchanged, so an unrelated
56+
error is never silently retried with different flags.
57+
58+
A file, unlike a pipe, has no backpressure: the child can never block on a
59+
stderr write, so nothing has to read alongside the running process. The
60+
trade is that the child's stderr appears when it exits, not live.
61+
"""
62+
with tempfile.TemporaryFile() as stderr_file:
63+
proc = subprocess.Popen(argv, stderr=stderr_file)
64+
try:
65+
returncode = proc.wait()
66+
except KeyboardInterrupt:
67+
# Forward Ctrl-C and let the child decide its own exit code rather
68+
# than racing it (mirrors exec_or_spawn's Windows path).
69+
proc.send_signal(signal.SIGINT)
70+
returncode = proc.wait()
71+
stderr_file.seek(0)
72+
if returncode != 0 and stderr_marker.encode() in stderr_file.read(_STDERR_HEAD_LIMIT):
73+
exec_or_spawn(fallback_argv)
74+
return # unreachable in production (exec/exit); keeps tests honest
75+
# Replay everything the child wrote to stderr.
76+
stderr_file.seek(0)
77+
for chunk in iter(lambda: stderr_file.read(65536), b""):
78+
sys.stderr.write(chunk.decode(errors="replace"))
79+
sys.stderr.flush()
80+
sys.exit(returncode)

tests/test_agent_codex.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,3 +436,59 @@ def fake_execvp(binary: str, args: list[str]) -> None:
436436

437437
assert os.environ["OAUTH_TOKEN"] == "fresh-token"
438438
assert exec_calls == [("codex", ["codex", "--profile", "ucode", "--search"])]
439+
440+
441+
class TestCodexLaunchProfileRouting:
442+
"""launch() hands the interactive TUI straight to exec with --profile; any
443+
invocation with a subcommand token goes through the try-with-profile /
444+
fall-back-on-rejection path instead of a version-dependent allow-list."""
445+
446+
@staticmethod
447+
def _capture(monkeypatch):
448+
exec_calls: list[list[str]] = []
449+
fallback_calls: list[tuple[list[str], list[str], str]] = []
450+
monkeypatch.setattr(codex, "exec_or_spawn", lambda argv: exec_calls.append(argv))
451+
monkeypatch.setattr(
452+
codex,
453+
"run_with_stderr_fallback",
454+
lambda argv, fallback_argv, marker: fallback_calls.append(
455+
(argv, fallback_argv, marker)
456+
),
457+
)
458+
monkeypatch.setattr(codex, "get_databricks_token", lambda workspace, profile=None: "tok")
459+
return exec_calls, fallback_calls
460+
461+
def test_bare_tui_execs_with_profile(self, monkeypatch):
462+
exec_calls, fallback_calls = self._capture(monkeypatch)
463+
codex.launch({"workspace": WS}, [])
464+
assert exec_calls == [["codex", "--profile", "ucode"]]
465+
assert fallback_calls == []
466+
467+
def test_flags_only_tui_execs_with_profile(self, monkeypatch):
468+
exec_calls, fallback_calls = self._capture(monkeypatch)
469+
codex.launch({"workspace": WS}, ["--search"])
470+
assert exec_calls == [["codex", "--profile", "ucode", "--search"]]
471+
assert fallback_calls == []
472+
473+
def test_subcommand_tries_profile_with_rejection_fallback(self, monkeypatch):
474+
for sub in ("exec", "resume", "mcp", "app-server", "mcp-server", "remote-control"):
475+
exec_calls, fallback_calls = self._capture(monkeypatch)
476+
codex.launch({"workspace": WS}, [sub, "--listen", "u"])
477+
assert exec_calls == []
478+
assert fallback_calls == [
479+
(
480+
["codex", "--profile", "ucode", sub, "--listen", "u"],
481+
["codex", sub, "--listen", "u"],
482+
codex._PROFILE_REJECTED_MARKER,
483+
)
484+
]
485+
486+
def test_dashless_flag_value_routes_through_fallback(self, monkeypatch):
487+
# `--model gpt-5` is still the TUI; the conservative classifier sends it
488+
# through the fallback path, whose first attempt carries --profile — so
489+
# behavior is identical, just supervised.
490+
exec_calls, fallback_calls = self._capture(monkeypatch)
491+
codex.launch({"workspace": WS}, ["--model", "gpt-5"])
492+
assert exec_calls == []
493+
assert fallback_calls[0][0] == ["codex", "--profile", "ucode", "--model", "gpt-5"]
494+
assert fallback_calls[0][1] == ["codex", "--model", "gpt-5"]

tests/test_launcher.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import sys
56
from unittest.mock import MagicMock, patch
67

78
import pytest
@@ -61,3 +62,59 @@ def test_windows_keyboard_interrupt_forwards_sigint(self):
6162
launcher.exec_or_spawn(["claude.exe"])
6263
proc.send_signal.assert_called_once_with(launcher.signal.SIGINT)
6364
assert exc.value.code == 130
65+
66+
67+
class TestRunWithStderrFallback:
68+
"""Real-subprocess tests: stderr is captured to a file and read at exit."""
69+
70+
@staticmethod
71+
def _script(code: str) -> list[str]:
72+
return [sys.executable, "-c", code]
73+
74+
def test_marked_failure_execs_fallback_and_swallows_rejection(self, monkeypatch, capsys):
75+
calls: list[list[str]] = []
76+
monkeypatch.setattr(launcher, "exec_or_spawn", lambda argv: calls.append(argv))
77+
launcher.run_with_stderr_fallback(
78+
self._script("import sys; sys.stderr.write('Error: MARKER hit\\n'); sys.exit(1)"),
79+
["fallback", "argv"],
80+
"MARKER",
81+
)
82+
assert calls == [["fallback", "argv"]]
83+
# The expected rejection is noise once we relaunch — swallowed.
84+
assert capsys.readouterr().err == ""
85+
86+
def test_success_propagates_and_replays_stderr(self, monkeypatch, capsys):
87+
monkeypatch.setattr(launcher, "exec_or_spawn", lambda argv: pytest.fail("fallback fired"))
88+
with pytest.raises(SystemExit) as exc:
89+
launcher.run_with_stderr_fallback(
90+
self._script("import sys; sys.stderr.write('warn: heads up\\n')"),
91+
["fallback"],
92+
"MARKER",
93+
)
94+
assert exc.value.code == 0
95+
# Whatever the child said on stderr reaches the caller at exit.
96+
assert "warn: heads up" in capsys.readouterr().err
97+
98+
def test_unrelated_failure_propagates_without_fallback(self, monkeypatch, capsys):
99+
# An unmarked failure must NOT retry with different flags — retrying
100+
# would silently drop the preferred argv's routing.
101+
monkeypatch.setattr(launcher, "exec_or_spawn", lambda argv: pytest.fail("fallback fired"))
102+
with pytest.raises(SystemExit) as exc:
103+
launcher.run_with_stderr_fallback(
104+
self._script("import sys; sys.stderr.write('boom\\n'); sys.exit(3)"),
105+
["fallback"],
106+
"MARKER",
107+
)
108+
assert exc.value.code == 3
109+
assert "boom" in capsys.readouterr().err
110+
111+
def test_marker_on_success_does_not_fall_back(self, monkeypatch, capsys):
112+
monkeypatch.setattr(launcher, "exec_or_spawn", lambda argv: pytest.fail("fallback fired"))
113+
with pytest.raises(SystemExit) as exc:
114+
launcher.run_with_stderr_fallback(
115+
self._script("import sys; sys.stderr.write('mentions MARKER\\n'); sys.exit(0)"),
116+
["fallback"],
117+
"MARKER",
118+
)
119+
assert exc.value.code == 0
120+
assert "mentions MARKER" in capsys.readouterr().err

0 commit comments

Comments
 (0)