Add Codex CLI direct-mode support to thv llm setup - #5789
Conversation
Codex is a supported client but has no llm gateway integration yet. Add direct mode: a custom model_provider in ~/.codex/config.toml pointed at the gateway, authenticated via a command-backed bearer token (thv llm token) rather than an API key file, mirroring Claude Code's apiKeyHelper but shaped for Codex's TOML/argv config. Codex's Responses-API client appends "/responses" straight onto base_url, so the gateway's "/v1" prefix is baked into base_url rather than left to Codex, unlike the optional Anthropic path prefix.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5789 +/- ##
==========================================
- Coverage 70.78% 70.78% -0.01%
==========================================
Files 684 685 +1
Lines 69353 69435 +82
==========================================
+ Hits 49091 49147 +56
- Misses 16654 16691 +37
+ Partials 3608 3597 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds first-class support for configuring the OpenAI Codex CLI as a direct-mode client of ToolHive’s LLM gateway via thv llm setup/teardown. It introduces a Codex-specific “codex-auth” mode that patches ~/.codex/config.toml using TOML semantics (including Codex’s command/argv-based bearer token auth) rather than the existing JSON-pointer patch mechanism used by other clients.
Changes:
- Add new LLM-gateway mode
codex-authand plumb argv-form token helper fields (TokenHelperPath/TokenHelperArgs) through setup/config application. - Implement a dedicated Codex TOML writer to create/remove
model_providerandmodel_providers.toolhive-gatewayentries (including/v1base URL handling and token-helper command/args). - Add unit + e2e coverage for Codex setup/teardown behavior and the new token-helper argv builder.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/cli_llm_all_clients_test.go | Adds dedicated Codex TOML assertions for setup/teardown outside the JSON-based matrix. |
| pkg/llmgateway/config.go | Introduces ModeCodexAuth and extends ApplyConfig with argv-style token helper fields. |
| pkg/llm/setup.go | Wires argv-form token helper into tool configuration and adds a TLS-skip note for Codex mode. |
| pkg/llm/setup_test.go | Adds unit tests for buildTokenHelperArgv and Codex-specific TLS-skip messaging. |
| pkg/client/llm_gateway.go | Dispatches configure/revert to Codex’s TOML writer for codex-auth mode. |
| pkg/client/llm_gateway_codex.go | New TOML-based configure/revert implementation for Codex gateway auth configuration. |
| pkg/client/llm_gateway_codex_test.go | New unit tests for Codex TOML writer (write, idempotency, preservation, revert semantics). |
| pkg/client/config.go | Registers Codex as an LLM gateway client using codex-auth mode and ~/.codex/config.toml. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes three issues from PR review: setup could fail unnecessarily for Codex-only runs when thv's own executable path had shell-unsafe characters, even though Codex's argv-based auth never uses the shell-string helper; configureCodexAuth silently wrote an invalid provider entry when the token helper was unset; and a gateway URL already ending in "/v1" was doubled to "/v1/v1" in Codex's base_url.
JAORMX
left a comment
There was a problem hiding this comment.
Panel review (spec / standards / domain axes). Strong PR overall — clean, well-documented, good test coverage, and it reuses the in-codebase TOML/lock/atomic-write helpers correctly rather than reinventing them. Security posture is sound: the token is never persisted, the config is written 0600, and the argv path correctly drops the shell-metachar validation because there's no shell downstream.
Inline comments cover the findings. Summary:
Worth addressing before merge
- Codex is missing from the
Longhelp text anddocs/cli/(seeconfig.gocomment) — the one real blocker. - Non-table
model_providersis silently clobbered on setup (llm_gateway_codex.go). refresh_interval_msfrom the issue's auth spec isn't written.--tls-skip-verifywarning severity reads backwards for Codex.
Judgement calls (non-blocking)
- Per-mode behavior is now spread across ~5 sites (two dispatch chains +
warnTLSSkipVerify+usesAnthropicBaseURL+tokenHelperCommandNeeded); a mode-capability descriptor would collapse them, but deferring is defensible at 3 modes. - Direct-mode-only: the issue flagged confirming gateway Responses-API support before direct mode and offered proxy mode as the dependency-free fallback. The manual round-trip test in the description is good evidence it works — worth noting that confirmation explicitly.
- Revert removes
model_providerrather than restoring the prior value — document the semantics.
No duplication or library-reuse findings — the parallel Codex writer is structurally similar to the credential-helper one but behaviorally independent, so a shared abstraction would be the wrong call.
Happy to help push fixes for the concrete items.
Follow-up on the review of #5789, fixing the concrete findings while leaving the design judgement calls (mode-capability descriptor, proxy fallback) for separate discussion. - Write refresh_interval_ms into Codex's auth table via a new CodexHelperTTL constant (= ClaudeCodeHelperTTL), so Codex's token-helper cadence stays inside the token source's preemptive refresh window like the other clients. Guard it in the invariant test. - Refuse to overwrite a pre-existing non-table model_providers instead of silently clobbering the user's config; mirrors the revert path's type check. - Surface Codex in "thv llm setup" help text and regenerate CLI docs so the new client is discoverable. - Promote the Codex --tls-skip-verify message from Note to Warning and state plainly that the flag was not applied. - Document revertCodexAuth's remove-not-restore semantics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Pushed a follow-up commit (17682ef) addressing the concrete findings from the review above, to save you a round-trip:
Left alone as design calls for you/the team rather than mechanical fixes: the mode-capability-descriptor refactor (per-mode behaviour is now spread across ~5 sites) and the direct-mode-vs-proxy-fallback question. |
Follow-up to the review nitpick on warnTLSSkipVerify: its switch mixed raw "direct"/"proxy" string literals with the llmgateway.ModeCodexAuth constant. The llmgateway package already declares these as the single source of truth, so switch on the constants for all cases. Also update the now-stale ToolConfig.Mode doc comment, which still listed only "direct"/"proxy" though "codex-auth" is now a valid value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Opened #5790 as the follow-up cleanup: it sweeps the remaining |
Summary
Codex CLI isn't a supported target for
thv llm setupyet — it can't bepointed at the LLM gateway the way Claude Code, Cursor, and other clients
already are. This adds direct-mode support for Codex.
model_providerentry written into its~/.codex/config.toml(the same file its MCP-server registration alreadyuses), authenticated via a command-backed bearer token
(
[model_providers.<id>.auth]invokingthv llm token --skip-browser)rather than a static API key — mirroring Claude Code's
apiKeyHelper,but shaped for Codex's TOML/argv config instead of JSON/shell-string.
/responsesdirectly ontobase_url(the same convention OpenAI's ownbase_url=".../v1"follows),so the gateway's
/v1prefix is baked intobase_urlat write timerather than left to Codex to add.
model_providers.*entries or
mcp_serversconfig already in the file; revert only clearsToolHive's own provider and only clears
model_providerif it stillpoints at ours.
pkg/client/llm_gateway_codex.go) sinceCodex's config format and auth shape don't fit the existing JSON-Pointer
LLMGatewayKeysmechanism the other direct-mode clients share.Closes #5783
Type of change
Test plan
task test)task test-e2e)task lint-fix)Manually ran
thv llm setup --client codexandthv llm teardown --client codexagainst a live gateway, confirming
~/.codex/config.tomlgets the expectedmodel_provider/model_providers.toolhive-gatewaytable on setup and a cleanremoval (with foreign entries untouched) on teardown.
Implementation plan
Approved implementation plan
Add Codex CLI direct-mode support to
thv llm setup(#5783)Why:
thv llm setupdoesn't support Codex CLI yet. We're adding direct mode: Codex's~/.codex/config.tomlgets a custommodel_providerpointed at the gateway, using Codex's command-backed auth ([model_providers.<id>.auth]) to invokethv llm token, same idea as Claude Code'sapiKeyHelperbut TOML/argv-shaped instead of JSON/shell-string.What:
pkg/llmgateway/config.go: newModeCodexAuthconstant; addTokenHelperPath/TokenHelperArgstoApplyConfig(argv form, vs. Claude Code's shell-stringTokenHelperCommand).pkg/client/config.go: give the existingCodexclient entryLLMGatewayMode: ModeCodexAuth,LLMBinaryName: "codex",LLMSettingsFile/RelPathpointing at~/.codex/config.toml(same file its MCP config already uses).pkg/client/llm_gateway_codex.go(new):configureCodexAuth/revertCodexAuth, reusingreadTOMLConfig/writeTOMLConfigfromconfig_editor.go. Writesmodel_provider = "toolhive-gateway"+ amodel_providers.toolhive-gatewaytable (name,base_url,wire_api = "responses",auth.command/auth.args), preserving any othermodel_providers.*entries. Revert deletes just that sub-table and clearsmodel_provideronly if still ours.pkg/client/llm_gateway.go: add a dispatch branch forModeCodexAuthnext to the existingModeCredentialHelperone.pkg/llm/setup.go:buildTokenHelperArgv()(mirrorsbuildTokenHelperCommandbut returns argv(path, []string{"llm","token","--skip-browser"}); always--skip-browsersince Codex has no interactive-context signal like Claude Desktop's shim); wire intoconfigureDetectedTools; add awarnTLSSkipVerifycase for Codex ("not supported", like the Gemini CLI case).usesAnthropicBaseURLstays unchanged — Codex is deliberately excluded so it gets plainGatewayURL, not the Anthropic-prefixed URL.pkg/client; a dedicated e2eDescribeblock incli_llm_all_clients_test.go(parallel to the existing claude-desktop block, since the generic matrix asserts via JSON and can't parse TOML).task docsafter, per CLAUDE.md.Verification:
task test,task lint-fix,task test-e2e, then manually runthv llm setup --client codex+codexagainst staging to confirm an actual request round-trips through the gateway, thenthv llm teardown --client codexto confirm clean removal.Does this introduce a user-facing change?
Yes —
thv llm setup/thv llm teardownnow support Codex CLI as a direct-modeclient, alongside the existing supported clients.
Special notes for reviewers
The gateway's
/v1path segment is baked directly into the writtenbase_url,unlike the optional Anthropic path prefix other clients use — this is required
because Codex's Responses-API client always appends
/responsesstraight ontowhatever
base_urlis configured, with no separate path-prefix concept ofits own.
Generated with Claude Code