fix(codex): accept codex login as authentication (#1010)

[frankbria]

Closes #1010.

The bug

cf work start --engine codex --execute and cf work batch run --engine codex called require_openai_api_key(), which exits when OPENAI_API_KEY is unset. codex does not need that variable — codex login stores ChatGPT-plan credentials in auth.json, and #914's end-to-end demo drove a real codex app-server to completion with none set.

So the advertised --engine codex was unusable for the common case, while the adapter and the binary both worked fine.

The env var is not just incomplete proof — the file says so

A real logged-in auth.json on this machine:

auth_mode      = 'chatgpt'
OPENAI_API_KEY = None          # literally null
tokens         = {access_token, account_id, id_token, refresh_token}

codex login records OPENAI_API_KEY: null in the very file that proves you are authenticated.

Fix

CodexAdapter.is_authenticated() accepts either route — the env var, an API key stored in auth.json (codex login --api-key), or ChatGPT tokens. Presence of the file is not enough: codex logout can leave it behind with empty tokens. $CODEX_HOME is honoured, since the CLI documents it.

require_codex_auth() replaces require_openai_api_key() at both call sites, and its message names both ways in.

A second surface, found by running the command

AC3 asks that cf engines check report codex ready when logged in. It did not, for a different reason: the command treats any unsatisfied entry as an unmet requirement and exits 1, so requirements() listing OPENAI_API_KEY reported a working logged-in codex as broken.

requirements() now returns {} and check_ready() reports codex_binary + authenticated — the question actually being asked. (credential_env_vars() is overridden explicitly on this adapter, so it is unaffected.)

Verification against the real logged-in codex

With no OPENAI_API_KEY in the environment:

codex_home:       /home/frankbria/.codex
is_authenticated: True
check_ready:      {'codex_binary': True, 'authenticated': True}

And all three AC4 cases:

logged in,  no key  → {'codex_binary': True, 'authenticated': True}
logged out, no key  → {'codex_binary': True, 'authenticated': False}
logged out, w/ key  → {'codex_binary': True, 'authenticated': True}

Testing

tests/core/adapters/test_codex_auth_1010.py — 14 tests covering logged-in-no-env-key, env-key-no-login, key-stored-in-auth.json, neither, logged-out-with-empty-tokens, corrupt file, $CODEX_HOME relocation, and both engines check verdicts. ruff clean.

One test pins the unauthenticated case against load_env_files(), since this machine (and CI) may carry a real OPENAI_API_KEY in a .env.

Note

This composes with #996: ~/.codex is symlinked into the delegated agent's sandbox home, so the login keeps working for the spawned codex app-server too.

[github-actions]

Claude finished @frankbria's task in 4m 17s —— View job

---

GLM Precision Review — complete ✅

• Gather PR diff + existing comments
• Read changed files in full context (codex adapter, validators, cli/app, engine_registry, subprocess_adapter, engines_commands)
• Confirm defects against real code
• Post inline findings (1)
• Post summary table

Result: 1 minor finding — is_authenticated() doesn't catch UnicodeDecodeError from read_text() on a non-UTF-8 auth.json. Full details in the inline comment + summary comment below. No critical/major defects; the core auth logic and the requirements() → {} change are correct.

[github-actions]

[minor] Path.read_text() can raise UnicodeDecodeError (a ValueError), which except (OSError, json.JSONDecodeError) does not catch — a genuinely corrupt auth.json crashes is_authenticated() instead of returning False.

Failure scenario: auth.json is truncated or written with non-UTF-8 bytes (e.g. a partial write interrupted mid-codex login, or an editor that re-saved in a legacy encoding). read_text() defaults to strict decoding and raises UnicodeDecodeError, which is neither an OSError nor a JSONDecodeError, so it propagates uncaught through require_codex_auth() — the user gets a Python traceback at cf work start --engine codex --execute instead of the intended "codex is not authenticated" message. The existing corrupt-file test only exercises invalid-JSON-over-valid-UTF-8 ("{not json"), which is why this path slipped through. JSONDecodeError is itself a ValueError, so broadening to ValueError covers both.

Suggested change

	except (OSError, json.JSONDecodeError):
	except (OSError, ValueError):

[github-actions]

GLM precision review

severity	file:line	finding
minor	codeframe/core/adapters/codex.py:186	is_authenticated() doesn't catch UnicodeDecodeError from read_text() on a non-UTF-8 auth.json → uncaught traceback instead of "not authenticated"

1 finding (1 minor). No critical/major defects.

Verified correct: is_authenticated() ordering (env OPENAI_API_KEY → stored key in auth.json → ChatGPT tokens.access_token), the logged-out/empty-tokens and non-dict guards, codex_home() ($CODEX_HOME + empty-string fallback), and check_ready(). The requirements() → {} change is safe — CodexAdapter does not subclass SubprocessAdapter (so subprocess_adapter.py:120 tuple(cls.requirements()) doesn't apply) and credential_env_vars() is explicitly overridden, leaving only check_requirements/engines check as the consumer.

[github-actions]

Claude finished @frankbria's task in 4m 2s —— View job

---

GLM Precision Review — re-review on synchronize

New commit c1fc7df is test-only (tests/core/adapters/test_codex_auth_1010.py, +7/-2); production code (codex.py, validators.py, app.py) is unchanged from the prior review. Scanned for new defects only.

• Gather PR diff + existing comments (skip already-posted findings)
• Read changed files in full context
• Confirm any new defects against real code
• Post inline findings — none
• Post summary

✅ GLM review: no new defects in this synchronize

Verified the test change is correct and non-vacuous:

• check_requirements("codex") (engine_registry.py:156) now returns {"codex_binary": …, "authenticated": …} — requirements() yields {} (no env-var keys), then check_ready() is merged in. So reqs["authenticated"] and "OPENAI_API_KEY" not in reqs assert the real credential claim.
• The old all(reqs.values()) / not all(reqs.values()) were genuinely fragile: engines_check (engines_commands.py:84-96) exits 1 on any False entry, and codex_binary is legitimately False on every CI runner — so the positive test could fail and the negative test could pass trivially, masking a regression. The new targeted assertions fix exactly that.
• Confirmed CodexAdapter does not subclass SubprocessAdapter, so requirements() → {} has no consumer besides check_requirements (the subprocess_adapter.py:120 tuple(cls.requirements()) path does not apply).

Note: the one minor finding from the prior review — is_authenticated() at codeframe/core/adapters/codex.py:186 doesn't catch UnicodeDecodeError from read_text() (broaden except (OSError, json.JSONDecodeError) → except (OSError, ValueError)) — is still open in this commit (the line is unchanged). Not re-posted as an inline comment per the no-repeat rule; the original comment on that line remains active.

[github-actions]

Claude finished @frankbria's task in 2m 12s —— View job

---

GLM Precision Review — re-review on synchronize ✅

New commit cedae3d ("fix(codex): fail closed on a corrupt auth.json") closes the one open finding from the prior review — the UnicodeDecodeError gap at codex.py:186. Scanned for new defects only; per the no-repeat rule the now-resolved finding is not re-posted.

• Gather PR diff + existing comments (skip already-posted/resolved findings)
• Read changed files in full context
• Confirm any new defects against real code
• Post inline findings — none
• Post summary

✅ GLM review: no new defects in this synchronize

The fix is correct and non-vacuously tested:

• is_authenticated() (codeframe/core/adapters/codex.py:184-186) widens except (OSError, json.JSONDecodeError) → except (OSError, json.JSONDecodeError, ValueError). UnicodeDecodeError subclasses ValueError, so Path.read_text() on a non-UTF-8 auth.json now reads as "not authenticated" instead of propagating uncaught through require_codex_auth(). The try-block performs only read_text() + json.loads(str), so broadening to ValueError cannot mask an unrelated logic error — JSONDecodeError is itself a ValueError, making its explicit listing redundant-but-harmless (style, out of scope).
• test_a_non_utf8_auth_json_is_not_authenticated genuinely exercises the new path: it writes b"\xff\xfe\x00\x00not-utf8" (\xff is an impossible UTF-8 lead byte → UnicodeDecodeError) and would have errored out before the fix. The codex_home fixture (tests/core/adapters/test_codex_auth_1010.py:29-35) clears OPENAI_API_KEY and points $CODEX_HOME at the temp dir, so the assert … is False is not vacuous on a machine (or CI) carrying a real key.
• test_a_directory_where_auth_json_should_be_is_not_authenticated covers the already-correct OSError half (IsADirectoryError) — extra coverage of pre-existing behavior, not new logic.

The prior UnicodeDecodeError finding is resolved; no inline comments this round. Production code outside this hunk is unchanged since the prior review (which found no critical/major defects).