Two more P1 follow-ups from the round 7 reviewer pass:
1. Device-login log filter no longer leaks sensitive lines.
The old `_safe_to_forward` used unanchored substring matches like
`"logged in"`, so a line such as
Not logged in: refresh_token=rt_LEAK auth.json=/home/u/.codex/auth.json
slipped through the safe-vocabulary filter and was streamed to the
browser. A malicious codex shim earlier on PATH can print that
line trivially, defeating the "opaque output stays in backend
logs" safety guarantee the route claimed.
Round 7b fix: anchored regex set (must start with one of the
known upstream phrases) plus an explicit blocklist for
refresh_token / access_token / api_key / secret / auth.json / the
codex config dir / "not logged in" / "not authenticated". A line
that matches the blocklist is dropped regardless of which safe
pattern would otherwise have accepted it. New tests reconstruct
the regex set inline and assert both the leak cases drop and the
clean upstream phrases pass.
2. `_run_cli` timeout cleanup no longer 500s on a kill race.
`_run_cli` would call `proc.kill()` after `os.killpg(pid, SIGTERM)`
reaped the process group. If the SIGTERM landed first, the
subsequent `proc.kill()` raised `ProcessLookupError` and bubbled
out of `_run_cli`, turning `/api/codex/status` into a 500 during
a timeout race. The device-login cleanup at codex_provider.py
already wraps the same destructive call in a try / except. Mirror
that exception guard here so the two timeout paths behave the
same.
Round 6 reviewer noted that the warn-and-proceed path in
`_start_thread_with_system` is "failing open" on a server-side
chat surface: an SDK rev that does not expose ApprovalMode or
SandboxMode would log a warning then call `thread_start(model=...)`
with NO safety kwargs, letting the model run under the SDK's
`auto_review` default. For a route that takes a user-controlled
prompt and can spawn shell commands or file writes, that is the
wrong tradeoff.
Now fails closed: when `_safe_thread_safety_kwargs()` returns the
empty dict the helper raises `CodexUnavailableError`, which the
route layer translates to a 503 with a clear error message telling
the operator to upgrade `openai_codex` (or set the explicit
override env var). The error message names the override so users
who hit this on a pre-release alpha can opt in with eyes open
rather than discovering the unsafe default after the fact.
`UNSLOTH_CODEX_ALLOW_UNSAFE_DEFAULTS=1` is the deliberately-verbose
escape hatch. Variable name long and explicit so it does not creep
into production environments by accident, kept on the codex
subprocess safe-list so the round 6 SDK env-scrub wrapper does not
delete it before the gate sees it.
Tests: 50 cases total (was 49). The previous old-SDK test was
renamed and replaced by two new ones:
- `test_thread_start_fails_closed_when_safety_unavailable` asserts
the raise fires and `thread_start` is never called.
- `test_thread_start_allows_unsafe_defaults_with_explicit_opt_in`
asserts the override env var lets the request through and
`thread_start` runs without the safety kwargs (with a logged
warning).
The `_install_fake_codex_sdk` helper now injects fake ApprovalMode
and SandboxMode by default so the general translation tests do
not need to opt into the override; the two round-6b tests above
pass `with_safety_enums=False` to exercise the fail-closed branch.
Reviewer round 6 surfaced five real follow-ups on top of rounds 5
through 5g. Each one is a fix for an asymmetric guard or a wrong-
shape lookup in the new Codex provider code:
1. Re-gate `installed=True` on having BOTH the SDK AND a `codex`
binary on PATH. Round 5 widened the gate to SDK-only, but the
login route still shells out to the binary, so an SDK-only host
would surface a Codex row whose Sign-in button immediately
failed with "codex CLI not found on PATH". The canonical
`openai-codex` package depends on `openai-codex-cli-bin` which
places the shim on PATH for free, so the common install still
lights up; the gate just refuses to advertise a provider Studio
cannot actually drive end-to-end.
2. `_safe_thread_safety_kwargs` now also probes `.api` and
`.generated.v2_all` for `SandboxMode`. Upstream `openai_codex`
exports `ApprovalMode` at the top level but `SandboxMode` lives
under `openai_codex.generated.v2_all`. The previous lookup
returned `{}` on the canonical SDK install, so every thread_start
ran with the unsafe `auto_review` default. Submodule probe
resolves the canonical layout and keeps backwards-compat with
builds that DID re-export at the top level.
3. `_coerce_text` now applies the answer-event-type filter on the
object path too. The upstream SDK emits typed payload classes
like `CommandExecutionOutputDelta`, `FileChangeDelta`,
`ToolCallDelta`, `PatchApplyDelta`, etc., all of which carry a
`.delta` string of local stdout / file paths / tool args. The
dict path already filtered these out; the object path used to
return `.delta` unconditionally, so a real SDK install could
leak tool output into the visible chat reply.
4. `_ScrubbedEnvAsyncCodex` is now process-wide concurrency-safe
AND fails-closed if the SDK constructor raises:
- Refcount each scrubbed key under an `asyncio.Lock` so a fan-out
wrapper that exits early cannot restore a secret while another
wrapper is still inside SDK startup (round 6 reproduced this:
wrapper A exited, wrapper B's SDK saw the restored HF_TOKEN).
- Move `_async_codex_cls()` and its `__aenter__` INSIDE a
try/except in `__aenter__`; on failure, run the release path
so the scrubbed env vars are restored even though `__aexit__`
never fires for the failed construction.
5. `_run_cli` now detaches into its own process group via
`start_new_session=True` (Unix) / `CREATE_NEW_PROCESS_GROUP`
(Windows) and kills the whole group on timeout, matching the
protected path in `stream_codex_device_login`. A shimmed
`codex login status` that forks a helper and blocks no longer
leaves the child running after we killed the parent.
Frontend follow-up: chat-adapter now routes every rendered yield
through a `renderFullContent()` helper so the Codex per-tab text
accumulated in earlier `_toolEvent` frames is preserved when the
synthesis content delta arrives. Previously the next regular
content yield rebuilt `parts` from `cumulativeText` alone and the
tab section vanished from the final assistant message.
Tests: 49 cases total (was 47). New regressions:
- `installed_requires_both_cli_and_sdk` (round 6 revert).
- `safety_kwargs_finds_sandbox_mode_in_submodule` (canonical SDK
layout where `SandboxMode` is in `.generated.v2_all`).
- `scrubbed_env_construction_failure_restores_env` (no permanent
env leak when the SDK constructor raises).
- Expanded `coerce_text_drops_non_answer_event_types` to also
exercise the object-shape code path with `CommandExecutionOutputDelta`,
`FileChangeDelta`, `ToolCallDelta`, `PatchApplyDelta`,
`PlanUpdateDelta`, `AgentReasoningDelta`, plus the
positive `AgentMessageDelta` allow-through.
Five tightening fixes driven by the reviewer pass on top of round 4:
1. Drop OPENAI_API_KEY from the codex subprocess safe-list.
The OpenAI provider key belongs to the OpenAI provider; a shimmed
`codex` binary on PATH must not receive it. Users who want to wire
the same key into Codex now set CODEX_OPENAI_API_KEY, which is the
one OpenAI-shaped key we still forward.
2. Fail-closed env scrub on the SDK path.
When AppServerConfig is missing from the installed openai_codex
build, the bare AsyncCodex() constructor used to inherit the full
os.environ via the SDK's internal os.environ.copy(). Replaced the
fallback with a _ScrubbedEnvAsyncCodex wrapper that swaps
os.environ for the lifetime of the session so HF_TOKEN, GH_TOKEN,
WANDB_API_KEY etc never reach the spawned app-server.
3. Prefer the upstream-canonical base_instructions kwarg.
The real openai_codex SDK takes the system prompt as
`base_instructions`; our previous helper only knew `system`. Now
tries base_instructions first, falls back to system, then inlines
the system text in the user prompt as a last resort.
4. Filter visible text to answer-bearing event types only.
_coerce_text used to render any payload that exposed a `delta` /
`text` / `content` field, which let command output, file paths and
tool call arguments leak into the Chat Completions reply. Gated on
a _ANSWER_EVENT_TYPES allow-list (message.delta, completed,
text_delta, etc.); untyped legacy dicts still pass through.
5. Treat the SDK as the install gate.
openai-codex-cli-bin ships the codex runtime that backs
AsyncCodex(...), so SDK alone is sufficient to drive the provider.
`installed` no longer also requires a standalone codex binary on
PATH; cli_path remains reported separately so the UI can still
show whether a CLI is also installed.
Also added the matching positive-match regex line ("Authenticated:
Yes") for one more login-status wording the CLI ships in some
locales.
Tests: grown to 39 cases. New regressions cover the SDK-only install
gate, base_instructions kwarg priority + system fallback, the
fail-closed env scrub wrapper, the answer-only delta filter, the
Authenticated: Yes wording, and the CODEX_OPENAI_API_KEY-vs-
OPENAI_API_KEY split.
Fourth reviewer.py pass surfaced one more security finding and a
handful of correctness gaps. Each is small but the env-scrub for the
SDK path closes the asymmetric-fix loop opened in the previous round.
* Codex SDK construction now passes an `AppServerConfig(env=...)`
that overrides every non-safe-listed env key to an empty string.
Upstream openai/codex/sdk/python/client.py builds the spawn env as
`os.environ.copy()` then `env.update(self.config.env)`, so this
scrubs HF_TOKEN / GH_TOKEN / WANDB_API_KEY / ANTHROPIC_API_KEY etc.
out of the codex app-server subprocess env on the chat / parallel
/ synthesis paths, matching the CLI/login paths from the previous
round. The helper falls back to bare `AsyncCodex()` when the SDK
version does not expose AppServerConfig, with logged warning.
* Install hint now names the actual upstream PyPI project,
`openai-codex` (canonical), with `codex_app_server` documented as
the legacy alias. The probe still accepts both import names so
forward compat is preserved.
* Device-auth URL regex broadened to accept upstream's current
`chatgpt.com/activate` shape and any `/device|/activate|/verify`
variant, not just `/codex/device`. The frontend can now open the
verification page on CLI builds that print the documented
ChatGPT-style URL.
* `_run_codex_synthesis` now takes a `system` arg and forwards it
to `thread_start(system=...)`, falling back to a prompt-prefix on
older SDK revs that reject the kwarg. Previously a fan-out with
"Always answer in Spanish" produced Spanish per-tab attempts but
an English synthesis.
* `_detect_logged_in` negative regex now also matches "Not signed
in", "Please sign in" (alternative localisations / future CLI
releases). Same word-boundary anchoring as before.
* Frontend `CodexLoginEvent` union gains `device_code` and a `code`
field. `CodexLoginButton` now renders the one-time code under the
verification URL so users on a headless / remote install can copy
the code without scraping the log pane. Also fixes a closure-stale
bug where setError(message) was followed by a stale `error` read,
losing specific backend errors; the new path keeps `lastStreamError`
inside the closure.
* Replaced four hardcoded `/mnt/disks/...` paths in the new
regression tests with `_backend_file()` resolved from `__file__`,
so the suite runs in any checkout (CI, local dev, the review
worker tree). Found by the round-4 reviewer.
Four new pytest cases pin the behaviour:
`test_not_signed_in_wording_also_handled`,
`test_device_url_accepts_generic_verification_url`,
`test_synthesis_call_forwards_system_prompt`, and
`test_sdk_env_scrubbed_via_appserverconfig`. 32/32 codex_provider
tests pass; `tsc --noEmit` clean.
Third reviewer.py pass found three remaining sharp edges. Each fix
is small and paired with a regression test where applicable.
* Codex subprocess env is now scrubbed to a safe-list before spawn.
Both `_run_cli` in codex_availability and the device-auth spawn
in stream_codex_device_login switch from `env=os.environ.copy()`
to `env=_codex_subprocess_env()`, which forwards only PATH /
HOME / USER / Windows-equivalents / CODEX_HOME / OPENAI_API_KEY /
OPENAI_BASE_URL. Other-provider secrets like HF_TOKEN, GH_TOKEN,
WANDB_API_KEY, ANTHROPIC_API_KEY no longer reach the local codex
binary, so a shimmed `codex` earlier on PATH cannot harvest them.
* `_stream_thread_run` now tracks `emitted_any` and refuses to fall
through to the buffered `await thread.run(prompt)` after either
streaming helper has already yielded text. Previously a network
glitch mid-stream re-executed the same Codex turn, which can
duplicate file writes, shell commands, and other Codex side
effects. The buffered path is now reserved for the zero-output
case (no streaming helper resolved, or streaming returned empty).
* `CodexLoginButton` now aborts the SSE reader on unmount via a
useEffect cleanup that calls `abortRef.current?.abort()`. The
underlying `codex login --device-auth` subprocess no longer
keeps streaming (and holding a device-auth session) after the
dialog closes.
Two new pytest cases pin the behaviour: `test_codex_subprocess_env_scrubbed`
sets HF/GH/WANDB/ANTHROPIC keys and asserts none reach the codex
env while OPENAI_API_KEY / CODEX_HOME survive; and
`test_partial_stream_failure_does_not_replay_turn` injects a fake
`turn().stream()` that yields "partial output " then raises, and
asserts `thread.run()` is never called. 28/28 codex_provider tests
pass; `tsc --noEmit` clean.
Followups on the post-merge review pass for the Codex SDK chat
provider. Verified against codex-cli 0.133.0 + the upstream
`openai/codex` Rust + Python sources, then pinned each fix with
a regression test in `test_codex_provider.py` (24/24 passing).
* Probe both `openai_codex` (canonical upstream Python package at
`openai/codex/sdk/python`) and the legacy `codex_app_server`
alias. Without this the availability probe always reported
`sdk_importable: false` even when the SDK was installed, so the
provider was permanently hidden.
* Switch the device-auth and login-status invocations from
`codex auth login --device-auth` / `codex auth status` to the
real upstream subcommands `codex login --device-auth` and
`codex login status`. The former path returns
`unrecognized subcommand 'auth'` on a real CLI.
* Strip ANSI control sequences before extracting the device URL
(upstream wraps the URL in `\x1b[34m...\x1b[0m`) and tighten the
pattern to the canonical `.../codex/device` shape. Also surface
the one-time code as a `device_code` SSE event so the UI can
show it alongside the URL.
* Fix `_detect_logged_in` substring footgun: `"logged in" in
combined` matched inside `"not logged in"`, flipping logged-out
users to logged-in. Anchor on word boundaries with negative
prefixes winning regardless of return code.
* Cancel in-flight fan-out workers on SSE disconnect. Previously
every parallel Codex turn ran to completion against a
disconnected client and burned quota; now `_stream_codex_parallel`
cancels its worker + drain tasks in a try/finally on
`CancelledError`/`GeneratorExit`.
* Tear down the device-login subprocess on disconnect via
`start_new_session=True` + `os.killpg(SIGTERM)` (Unix) or
`CREATE_NEW_PROCESS_GROUP` + `CTRL_BREAK_EVENT` (Windows), with
a bounded `proc.wait()` and `proc.kill()` fallback. Previously
`finally: await proc.wait()` blocked the SSE close path because
`codex login --device-auth` only exits on user action.
* Render the full conversation transcript in `_last_user_prompt`
instead of returning only the most recent user message. The PR
opens a fresh thread per request so prior assistant turns were
dropped, degrading multi-turn chats to single-shot prompts.
Single-turn input is unchanged.
* Make `ChatCompletionRequest.parallel_calls` default to 1 (`int`
with `ge=1, le=20`) instead of `Optional[int] = None`. The
runtime already coerced `None` -> 1, but the schema now matches
the documented `[1, 20]` range.
* Replace the registry's hardcoded `default_models` (which
contained `o3`, not in the upstream catalog) with the current
`gpt-5.5 / 5.4 / 5.4-mini / 5.3-codex / 5.2` set from
`codex-rs/models-manager/models.json`.
* Stop echoing `str(exc)` in SSE error frames in both
`routes/inference.py` and `routes/codex.py`. The Codex SDK can
raise with local paths, env-var content, or traceback fragments
(CodeQL `py/information-exposure-through-exception`). Surface a
generic message + `exception_type` discriminator; log the full
reason server-side via `logger.error(..., exc_type=..., error=...)`.
Doc / comment updates throughout to refer to `codex login` /
`openai_codex` rather than the older incorrect strings.
Tested: pytest 24 cases in `test_codex_provider.py` (the original
14 + 10 new `TestCodexHardenedRegressions`) plus the rest of the
Studio-backend test suite the PR touches (209 passing). Also
verified live against Studio launched from this branch on a
Blackwell B200 via `UNSLOTH_STUDIO_HOME=$WORKSPACE/temp/...
./install.sh --local` then a Playwright probe.
Wires the OpenAI Codex CLI / Python SDK (codex_app_server) into Studio
as a new chat provider type. Hosts that don't have the CLI or the SDK
installed never see the entry; on logged-out hosts the provider config
dialog renders a device-auth Sign-in button that surfaces the
verification URL and streams CLI progress back over SSE.
Backend
- new core/inference/codex_availability.py probes the CLI + SDK and
reports {installed, logged_in, version, supported_models}; it never
imports codex_app_server at module top level so the rest of the
backend keeps starting cleanly on hosts that don't have the SDK.
- new core/inference/codex_provider.py wraps AsyncCodex and translates
Codex events into OpenAI chat-completion chunks. Supports the
thread.run_streaming path with a non-streaming fallback for older
SDK revs.
- parallel_calls > 1 fans the turn out across N tasks (capped at 20)
via asyncio.gather and emits codex_tab_open / codex_tab_chunk /
codex_tab_close tool-events per attempt plus a final codex_gather
synthesis event. A separate standalone Codex call produces the
unified answer.
- new routes/codex.py exposes GET /api/codex/status and POST
/api/codex/login. The login route shells out to
codex auth login --device-auth and streams events; the first event
carries the verification URL so the frontend can window.open it.
- ChatCompletionRequest gains a parallel_calls field bounded [1, 20]
by pydantic. The codex registry entry stays hidden by default; the
/api/codex/status probe is the authoritative gate.
- routes/inference.py dispatches provider_type=codex through the
local CLI/SDK pipeline instead of the standard HTTP client, with
graceful error surfacing for CodexUnavailableError.
Frontend
- new api/codex-api.ts exposes fetchCodexStatus() and an async
generator streamCodexDeviceLogin() that drives the SSE stream and
yields parsed events.
- new components/codex-parallel-tabs.tsx renders the tabbed parallel-
calls UI with a Synthesis tab highlighted once the codex_gather
event arrives. Pure reducer keeps the state transitions unit-
testable.
- new components/codex-login-button.tsx posts to /api/codex/login,
opens the verification URL in a new tab via window.open, and shows
the streamed CLI log as it lands.
- external-providers.ts exports CODEX_PROVIDER_TYPE,
CODEX_MAX_PARALLEL_CALLS, isCodexProviderType, and
clampCodexParallelCalls. Codex is marked text-only so the composer
hides image-attach affordances when selected.
Tests
- tests/test_codex_provider.py (14 cases) covers the availability
probe across the four install / login states, the streaming +
parallel-calls translation against a fake codex_app_server module
injected into sys.modules, the [1, 20] pydantic clamp, the
CodexUnavailableError surfacing path, and the parallel_calls=1
single-call shape (no tab tool-events).
Daniel Han