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.
8ee60019a4