codex: retry without --profile when the first attempt fails fast#184
codex: retry without --profile when the first attempt fails fast#184Edwinhe03 wants to merge 1 commit into
Conversation
c577f42 to
bcff2ab
Compare
bcff2ab to
7d540fb
Compare
|
am i testing this right? it seems to error on start? i checked out your branch no app-server: with app-server |
|
|
follow up: |
7d540fb to
a9961a2
Compare
a9961a2 to
b37ab3d
Compare
| _CODEX_PROFILE_SUBCOMMANDS = frozenset( | ||
| {"exec", "review", "resume", "archive", "delete", "unarchive", "fork", "mcp", "sandbox"} | ||
| ) |
There was a problem hiding this comment.
will we also need to maintain this list ? it also will depend on the cli version of codex that the customer is on
There was a problem hiding this comment.
yes, I can't see how we can get around this. but problem statement is ucode unconditionally adds --profile to all codex commands when codex doesn't support --profile on all subcommands. do you have any ideas?
There was a problem hiding this comment.
can you just do a try catch on the specific error?
63da551 to
52830b0
Compare
| # codex accepts the global --profile only on its runtime subcommands (plus the | ||
| # bare interactive TUI); server-family subcommands (app-server, mcp-server, | ||
| # exec-server, remote-control) and utilities reject it up front by printing | ||
| # this to stderr and exiting nonzero. The accepted set drifts across codex | ||
| # versions, so rather than maintain a version-dependent list we try with | ||
| # --profile and fall back exactly on this rejection. Stable prefix verified on | ||
| # codex 0.137 and 0.141; if a future codex rewords it, the launch fails loudly | ||
| # with codex's own error instead of silently dropping ucode's routing. |
There was a problem hiding this comment.
can you make this less verbose? or just rm this comment
| stderr write, so nothing has to read alongside the running process. The | ||
| trade is that codex's stderr appears when it exits, not live. | ||
| """ | ||
| with tempfile.TemporaryFile() as stderr_file: |
There was a problem hiding this comment.
why do we need to wrap in this temp file? it seems diff behavior from before - exec_or_spawn doesnt have the temp file
There was a problem hiding this comment.
exec_or_spawn doesn't capture any output. we could do piping, but then need to consider error output streaming and pipe buffer
| _exec_or_spawn_with_fallback( | ||
| [binary, "--profile", CODEX_PROFILE_NAME, *tool_args], | ||
| [binary, *tool_args], | ||
| _PROFILE_REJECTED_MARKER, | ||
| ) |
There was a problem hiding this comment.
instead of a new method, is it possible to do something inline + more simple like:
try:
exec_or_spawn
except Exception as e:
if e.msg == _PROFILE_REJECTED_MARKER:
exec_or_spawn([binary, *tool_args])
52830b0 to
881ca21
Compare
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
881ca21 to
4d5de9c
Compare
What
ucode codex's launch always rancodex --profile ucode <args>. But codex only accepts the global--profileon runtime subcommands. Server-family subcommands (app-server,mcp-server,exec-server,remote-control) reject it immediately:So
ucode codex app-server(and the other server subcommands) was functionally broken.Change
Dead simple, inline in
launch(): run codex with--profilefirst; 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
--profilerejection is a CLI parse-time error — it happens before codex touches auth, the gateway, or the network — so it exits in ~0.15s (measured, stable across 0.137/0.141/0.144). A codex command that actually starts a session can only fail after a network round-trip, i.e. seconds. There is no overlap, and the exit code alone can't distinguish them (the rejection and an ordinary failure both exit1), so elapsed time is what we key on.Why the fast-failure gate matters — this is what makes it strictly better than the status quo rather than a lateral move:
codex exec "prompt"(transient gateway error, bad model) would be silently re-run without--profile. Since ucode writes a named-profile file (ucode.config.toml, not the defaultconfig.toml), no--profilemeans no ucode routing — codex falls back toprovider: openai, i.e. the user's own OpenAI login. The gate ensures only the fast parse-time rejection is retried; a real session failure (seconds) is propagated untouched.KeyboardInterruptthat propagates past the retry check, so a normal quit is never mistaken for a rejection.Self-adapting across codex versions: a future runtime subcommand keeps
--profile(accepted, no fast failure); a future server subcommand falls back (fast reject). No ucode change needed either way.Touches only
codex.py+test_agent_codex.py;launcher.pyis untouched.Verification
ruff+tyclean.launch()tests (fakesubprocess.run+ monkeypatchedtime.monotonic) cover: runs with--profile+ sets OAUTH_TOKEN; success → no retry; fast nonzero (app-server/mcp-server) → relaunch without--profile; slow nonzero → NO retry (exit propagated); fast zero-exit → no retry.--profilerejection exits in ~0.13s (bounded by node cold-start); a realexecagainst a bad gateway fails in ~25s. ~180× separation.codex.launch():app-server --listen:--profilerejected fast → relaunch → socket bound, server alive.execagainst a bogus gateway: fails slowly (25s), exit 1, and does NOT retry ontoprovider: openai— the misroute the gate prevents.This pull request and its description were written by Isaac.