Skip to content

Commit 35b1df9

Browse files
author
SqlRush
committed
fix: codexgo binary honors custom/real model providers from config (closes drop-in gap)
buildAssembly previously always used CreateOpenAIProvider(nil) + OPENAI_API_KEY, ignoring config's model_provider selection, custom [model_providers.*] base_url, and env_key — so 'codex exec' against a custom provider silently fell back to the mock. - internal/cli/config_load.go: project selected provider id, [model_providers] map, default model, openai_base_url from the merged config (-c/profile already folded). - internal/cli/provider_select.go (new): resolve the active provider (merge configured onto built-ins, validate), env_key bearer auth resolver; mock only when no credential resolves. - internal/cli/assembly.go: build api.Provider for the SELECTED provider (custom base_url/wire_api/headers honored), pick auth resolver per requires_openai_auth, honor model selection, thread provider id+model into appserver.Defaults. - cmd_exec/review/default/app_server consume resolved Defaults. - internal/paritytest TestParityTurnExec upgraded to BINARY-vs-BINARY: the codexgo binary now actually contacts the fake endpoint (not the mock) — byte-identical normalized JSONL turn vs real codex. Behavioral drop-in proven at binary level. Full tree green: 89 packages pass; parity suite (subcommand/models/apply_patch/turn) passes binary-vs-binary; cross-compiles.
1 parent 5ebebd4 commit 35b1df9

10 files changed

Lines changed: 571 additions & 237 deletions

File tree

docs/PARITY.md

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ Living record of differential validation of `codexgo` against the reference
3131

3232
| Surface | Command | Result |
3333
|---|---|---|
34-
| `exec --json` turn lifecycle | `codex exec --json "hello"` vs codexgo |**Pass** — see `TestParityTurnExec`. Both binaries are pointed at the **same fake `/v1/responses` SSE endpoint** via the **same drop-in `config.toml`** (`[model_providers.parity]`, `env_key`), and produce a **byte-identical normalized JSONL stream**: same event-type sequence, same final agent message, same usage. No real OpenAI credentials required. |
34+
| `exec --json` turn lifecycle | `codex exec --json "hello"` vs codexgo |**Pass** — see `TestParityTurnExec`. **Both binaries** are pointed at the **same fake `/v1/responses` SSE endpoint** via the **same drop-in `config.toml`** (`[model_providers.parity]`, `env_key`), and produce a **byte-identical normalized JSONL stream**: same event-type sequence, same final agent message, same usage. The codexgo binary now honors the custom `model_provider` selection, its `base_url`, and its `env_key` directly — no in-process harness. No real OpenAI credentials required. |
3535

3636
### `TestParityTurnExec` — the turn-level differential
3737

@@ -89,36 +89,41 @@ codexgo: {"type":"turn.completed","usage":{"input_tokens":11,"cached_input_token
8989
(`thread-00000000000000000001`). Both are opaque per-run identifiers. The test
9090
strips `thread_id` (and any item `id`) before comparing.
9191

92-
**Drop-in gap found (genuine, documented): the codexgo binary does not honor a
93-
custom provider's `base_url`/`env_key`.**
92+
**Drop-in gap CLOSED: the codexgo binary now honors a custom provider's
93+
`base_url`/`env_key`.**
9494

95-
The real codex binary is driven *exactly* as a user would: the actual binary,
96-
`exec --json`, configured purely through `config.toml`. It picks up
95+
Both binaries are now driven *exactly* as a user would: the actual binary,
96+
`exec --json`, configured purely through `config.toml`. Each picks up
9797
`[model_providers.parity]`, sends `Authorization: Bearer dummy`, POSTs to the
9898
fake `/v1/responses`, and emits a normal turn.
9999

100-
**codexgo is driven in-process** through `internal/appserver.Assemble` plus a real
101-
`appserver.NewModelClientFactory` whose provider points at the same server. This
102-
is necessary because codexgo's `cmd/codex exec` assembly
103-
(`internal/cli/assembly.go``buildAssembly`) currently:
104-
105-
- always builds the model provider with `modelproviderinfo.CreateOpenAIProvider(nil)`,
106-
ignoring any custom `[model_providers.<id>]` `base_url`; and
107-
- resolves credentials only from `OPENAI_API_KEY` / `CODEX_API_KEY` / `auth.json`
108-
(`internal/cli/model_client.go`), ignoring a provider's custom `env_key`.
109-
110-
As a result, the **codexgo binary** run against this config.toml silently falls
111-
back to the scripted mock and never contacts the server (verified: no request
112-
reaches the test server, and it emits the mock reply). The in-process harness
113-
exercises the *same* real code path the binary would use against
114-
`api.openai.com``core.ResponsesModelClient` → the `internal/api` SSE parser →
115-
the exec turn loop → the exec JSONL sink — just with the provider base_url
116-
retargeted, so the turn-level behavioral comparison is faithful. Closing the
117-
binary-level gap (threading the parsed config's `model_provider` /
118-
`model_providers` and the provider `env_key` into `buildAssembly`) is the
119-
remaining work to make a *custom-provider* `config.toml` fully drop-in for the
120-
codexgo binary; the OpenAI-provider path (`OPENAI_API_KEY` + default base_url) is
121-
already wired in the binary.
100+
codexgo's `cmd/codex exec` assembly (`internal/cli/assembly.go`
101+
`buildAssemblyWithDefaults`, with provider selection in
102+
`internal/cli/provider_select.go`) now:
103+
104+
- reads the resolved `model_provider` selection and the `[model_providers]` map
105+
from the loaded config (projected through `internal/cli/config_load.go`), merges
106+
the configured providers onto the built-in catalog
107+
(`modelproviderinfo.MergeConfiguredModelProviders` over
108+
`BuiltInModelProviders`, honoring `openai_base_url`), and builds the
109+
`api.Provider` for the **selected** provider — so a custom
110+
`[model_providers.<id>]` `base_url` (and `wire_api`, `http_headers`, retry, …)
111+
is honored;
112+
- resolves credentials honoring the provider's `env_key` first (a static
113+
`Authorization: Bearer <env_key value>`), and only falls back to the
114+
`OPENAI_API_KEY` / `CODEX_API_KEY` / `auth.json` login path for
115+
`requires_openai_auth` providers; and
116+
- honors the configured `model` (over `CODEX_MODEL`, over the mock slug) and
117+
threads the resolved provider id + model into the exec/review/TUI session
118+
defaults.
119+
120+
The scripted mock remains the fallback **only** when no usable credential /
121+
provider resolves (preserving the offline/dev behavior and
122+
`CODEX_EXEC_MOCK_REPLY`). As a result, the **codexgo binary** run against this
123+
config.toml now contacts the server and produces a real turn whose normalized
124+
JSONL stream is byte-identical to the real codex binary's — proving the binary
125+
itself is a behavioral drop-in for a custom provider. The OpenAI-provider path
126+
(`OPENAI_API_KEY` + default base_url) is wired through the same code.
122127

123128
## Pending (need a one-time authenticated recording — maintainer)
124129

@@ -156,12 +161,12 @@ further offline differentials:
156161

157162
Format/CLI-surface parity is validated and faithful (model catalog identical;
158163
subcommand set complete; `apply_patch` byte-identical). The **turn-level
159-
behavioral drop-in proof is now done**: `TestParityTurnExec` runs one real
160-
`exec --json` model turn through both binaries against a fake `/v1/responses`
161-
endpoint with **no OpenAI credentials**, and the normalized JSONL streams are
162-
byte-identical (same event sequence, message text, and usage). The remaining
163-
honest caveats are (1) the codexgo *binary* does not yet honor a custom
164-
provider's `base_url`/`env_key` from `config.toml` (so codexgo is driven
165-
in-process for this test; the OpenAI-provider path is wired in the binary), and
166-
(2) tool-call / compaction / app-server wire-stream differentials are still
167-
pending.
164+
behavioral drop-in proof is now done, binary-vs-binary**: `TestParityTurnExec`
165+
runs one real `exec --json` model turn through **both built binaries** against a
166+
fake `/v1/responses` endpoint, configured purely through the same drop-in
167+
`config.toml` + `PARITY_FAKE_KEY`, with **no OpenAI credentials**, and the
168+
normalized JSONL streams are byte-identical (same event sequence, message text,
169+
and usage). The codexgo binary now honors a custom provider's `model_provider`
170+
selection, `base_url`, and `env_key` (previous in-process workaround removed). The
171+
remaining honest caveat is that tool-call / compaction / app-server wire-stream
172+
differentials are still pending.

internal/cli/assembly.go

Lines changed: 111 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/sqlrush/codexgo/internal/appserver"
1010
"github.com/sqlrush/codexgo/internal/config"
1111
"github.com/sqlrush/codexgo/internal/core"
12-
"github.com/sqlrush/codexgo/internal/modelproviderinfo"
1312
"github.com/sqlrush/codexgo/internal/protocol"
1413
)
1514

@@ -18,53 +17,110 @@ import (
1817
// complete turn out of the box without credentials.
1918
const defaultMockReply = "Hello from codex (mock model). Set a real model client to run live."
2019

21-
// buildAssembly constructs the Codex engine, selecting a real provider-backed
22-
// model client when credentials are present and a scripted mock otherwise.
20+
// buildAssemblyWithDefaults constructs the Codex engine, selecting a real
21+
// provider-backed model client when a usable provider + credentials resolve and a
22+
// scripted mock otherwise, and returns the resolved per-session defaults
23+
// (provider id + default model) alongside it.
2324
//
24-
// It resolves the Codex home + store mode from the loaded configuration, builds a
25-
// login-backed auth resolver, and wires appserver.NewModelClientFactory with the
26-
// mock as the offline/dev fallback. This mirrors the Rust ModelClient::new path
27-
// plus the exec provider-selection fallback. Configuration errors degrade to the
28-
// pure-mock assembly so the exec / mcp-server / app-server paths still run.
29-
func buildAssembly() (*appserver.Assembly, error) {
25+
// It loads the merged configuration, resolves the active `model_provider`
26+
// selection against the merged built-in + configured provider catalog, builds the
27+
// matching credential resolver (the login-backed OpenAI auth path for
28+
// requires_openai_auth providers; a provider env_key bearer otherwise), and wires
29+
// appserver.NewModelClientFactory with the scripted mock as the offline/dev
30+
// fallback. This mirrors the Rust ModelClient::new + create_model_provider path
31+
// plus the exec provider-selection fallback. Any configuration / provider error
32+
// degrades to the pure-mock assembly so the exec / mcp-server / app-server paths
33+
// still run out of the box.
34+
//
35+
// Threading the resolved provider id and model through the returned defaults is
36+
// what lets the binary honor a config.toml `model` / `model_provider` selection
37+
// end to end, rather than always defaulting to the mock slug and the "openai"
38+
// provider id.
39+
func buildAssemblyWithDefaults() (*appserver.Assembly, appserver.Defaults, error) {
3040
fallback := mockClientFactory()
3141

32-
resolverCfg, ok := loadAuthResolverConfig()
42+
cfg, ok := loadProviderConfig()
3343
if !ok {
3444
// Configuration could not be loaded; run with the mock so the engine still
3545
// works offline (e.g. in tests or a fresh checkout without a config file).
36-
return assembleWithFactory(fallback, resolverCfg.CodexHome)
46+
return assembleResult(fallback, "", "", defaultModelProviderID)
47+
}
48+
49+
model := configDefaultModel(cfg)
50+
51+
selected, err := resolveSelectedProvider(cfg)
52+
if err != nil {
53+
// A bad provider selection must not break the offline paths; fall back to
54+
// the mock so the engine still runs.
55+
return assembleResult(fallback, cfg.CodexHome, model, defaultModelProviderID)
56+
}
57+
58+
resolver, ok := buildProviderAuthResolver(cfg, selected)
59+
if !ok {
60+
// No credential source applies to the selected provider; use the mock.
61+
return assembleResult(fallback, cfg.CodexHome, model, selected.ID)
3762
}
3863

39-
resolver := newLoginAuthResolver(resolverCfg)
40-
// The OpenAI provider is built with a nil base URL so ToAPIProvider selects the
41-
// correct default per resolved auth mode (the ChatGPT codex endpoint for
42-
// ChatGPT-style auth, the public OpenAI API for API-key auth). The configured
43-
// ChatGPT base URL only influences agent-identity JWKS verification, handled by
44-
// the resolver, not the Responses endpoint default.
4564
factory, err := appserver.NewModelClientFactory(appserver.RealModelClientFactoryConfig{
4665
AuthResolver: resolver,
47-
Provider: modelproviderinfo.CreateOpenAIProvider(nil),
66+
Provider: selected.Info,
4867
InstallationID: resolveInstallationID(),
4968
Fallback: fallback,
5069
})
5170
if err != nil {
52-
return nil, fmt.Errorf("cli: build model client factory: %w", err)
71+
return nil, appserver.Defaults{}, fmt.Errorf("cli: build model client factory: %w", err)
72+
}
73+
return assembleResult(factory, cfg.CodexHome, model, selected.ID)
74+
}
75+
76+
// buildProviderAuthResolver selects the credential resolver for the active
77+
// provider. requires_openai_auth providers (e.g. the built-in OpenAI provider)
78+
// use the login-backed resolver, which honors OPENAI_API_KEY / CODEX_API_KEY /
79+
// auth.json / ChatGPT login. Other providers authenticate from their declared
80+
// env_key. It returns ok == false when no credential source applies, so the
81+
// caller selects the mock fallback.
82+
func buildProviderAuthResolver(cfg loadedConfig, selected selectedProvider) (appserver.AuthResolver, bool) {
83+
if isOpenAIAuthProvider(selected.Info) {
84+
return newLoginAuthResolver(authResolverConfig{
85+
CodexHome: cfg.CodexHome,
86+
StoreMode: cfg.StoreMode,
87+
ChatgptBaseURL: cfg.ChatgptBaseURL,
88+
EnableCodexAPIKeyEnv: true,
89+
}), true
90+
}
91+
if envKeyDefined(selected.Info) {
92+
return &envKeyAuthResolver{info: selected.Info}, true
5393
}
54-
return assembleWithFactory(factory, resolverCfg.CodexHome)
94+
return nil, false
5595
}
5696

57-
// assembleWithFactory builds the engine with the given model-client factory and
58-
// codex home, applying the shared defaults.
59-
func assembleWithFactory(factory appserver.ModelClientFactory, codexHome string) (*appserver.Assembly, error) {
97+
// assembleResult builds the engine with the given model-client factory, codex
98+
// home, default model slug, and provider id, returning the assembly alongside the
99+
// resolved per-session Defaults. An empty codexHome is resolved from the
100+
// environment; an empty defaultModel falls back to CODEX_MODEL and then the mock
101+
// slug. The same resolved model + provider id flow into both the assembly's
102+
// models manager and the returned Defaults so the binary's exec/review/TUI paths
103+
// honor the configured selection.
104+
func assembleResult(factory appserver.ModelClientFactory, codexHome, defaultModel, providerID string) (*appserver.Assembly, appserver.Defaults, error) {
60105
if codexHome == "" {
61106
codexHome = resolveCodexHome()
62107
}
63-
return appserver.Assemble(appserver.AssemblyConfig{
108+
model := resolveDefaultModel(defaultModel)
109+
asm, err := appserver.Assemble(appserver.AssemblyConfig{
64110
ModelClientFactory: factory,
65111
CodexHome: codexHome,
66-
DefaultModel: defaultModelSlug(),
112+
DefaultModel: model,
67113
})
114+
if err != nil {
115+
return nil, appserver.Defaults{}, err
116+
}
117+
defaults := appserver.Defaults{
118+
Model: model,
119+
ProviderID: providerID,
120+
Cwd: resolveCwd(),
121+
UserAgent: "codex-cli-go",
122+
}
123+
return asm, defaults, nil
68124
}
69125

70126
// mockClientFactory builds the scripted-mock ModelClientFactory used as the
@@ -83,29 +139,40 @@ func mockClientFactory() appserver.ModelClientFactory {
83139
}
84140
}
85141

86-
// loadAuthResolverConfig projects the loaded CLI configuration into the auth
87-
// resolver config. It returns ok == false when configuration cannot be loaded,
88-
// in which case the caller falls back to the pure-mock assembly. The default root
89-
// options are used because buildAssembly has no parsed -c overrides in scope; the
90-
// richer wiring through ParsedCommandLine can replace this when the exec/app-server
91-
// dispatch threads the root options here.
92-
func loadAuthResolverConfig() (authResolverConfig, bool) {
142+
// loadProviderConfig loads and projects the merged CLI configuration used by the
143+
// assembly: the codex home, auth store mode, the active provider selection, the
144+
// configured providers map, and the default model. It returns ok == false when
145+
// configuration cannot be loaded, in which case the caller falls back to the
146+
// pure-mock assembly. The default root options are used because the assembly
147+
// builder has no parsed -c overrides in scope; the richer wiring through
148+
// ParsedCommandLine can replace this when the exec/app-server dispatch threads the
149+
// root options here.
150+
func loadProviderConfig() (loadedConfig, bool) {
93151
cfg, err := loadConfig(RootOptions{})
94152
if err != nil {
95-
return authResolverConfig{}, false
96-
}
97-
return authResolverConfig{
98-
CodexHome: cfg.CodexHome,
99-
StoreMode: cfg.StoreMode,
100-
ChatgptBaseURL: cfg.ChatgptBaseURL,
101-
EnableCodexAPIKeyEnv: true,
102-
}, true
153+
return loadedConfig{}, false
154+
}
155+
return cfg, true
103156
}
104157

105-
// defaultModelSlug returns the configured default model slug for the assembly. It
106-
// honors CODEX_MODEL when set and otherwise falls back to the mock slug so the
107-
// offline path keeps working out of the box.
108-
func defaultModelSlug() string {
158+
// configDefaultModel returns the configured `model` slug, or "" when unset. The
159+
// assembly's resolveDefaultModel then layers CODEX_MODEL and the mock slug below
160+
// it, so a config model wins over the env var, matching the Rust precedence
161+
// (config model resolved before the env-derived defaults).
162+
func configDefaultModel(cfg loadedConfig) string {
163+
if cfg.DefaultModel != nil && *cfg.DefaultModel != "" {
164+
return *cfg.DefaultModel
165+
}
166+
return ""
167+
}
168+
169+
// resolveDefaultModel returns the effective default model slug for the assembly.
170+
// The configured model wins; otherwise CODEX_MODEL is honored, and finally the
171+
// mock slug keeps the offline path working out of the box.
172+
func resolveDefaultModel(configModel string) string {
173+
if configModel != "" {
174+
return configModel
175+
}
109176
if slug := os.Getenv("CODEX_MODEL"); slug != "" {
110177
return slug
111178
}

internal/cli/cmd_app_server.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,12 @@ func runAppServerSubcommand(ctx context.Context, parsed ParsedCommandLine, strea
2828
}
2929
}
3030

31-
asm, err := buildAssembly()
31+
asm, defaults, err := buildAssemblyWithDefaults()
3232
if err != nil {
3333
fmt.Fprintf(streams.Stderr, "codex app-server: %v\n", err)
3434
return 1
3535
}
3636

37-
defaults := appserver.Defaults{
38-
Model: "gpt-mock",
39-
ProviderID: "openai",
40-
Cwd: resolveCwd(),
41-
UserAgent: "codex-cli-go",
42-
}
43-
4437
if err := appserver.ServeStdioWithProcessor(ctx, asm, defaults, streams.Stdin, streams.Stdout); err != nil {
4538
if errors.Is(err, io.EOF) || errors.Is(err, context.Canceled) {
4639
return 0

internal/cli/cmd_default.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66

7-
"github.com/sqlrush/codexgo/internal/appserver"
87
"github.com/sqlrush/codexgo/internal/appserverclient"
98
"github.com/sqlrush/codexgo/internal/tui"
109
)
@@ -21,19 +20,17 @@ func runDefaultNoSubcommand(ctx context.Context, _ ParsedCommandLine, streams St
2120
return 1
2221
}
2322

24-
asm, err := buildAssembly()
23+
asm, defaults, err := buildAssemblyWithDefaults()
2524
if err != nil {
2625
fmt.Fprintln(streams.Stderr, "codex:", err)
2726
return 1
2827
}
28+
// Keep the versioned interactive user-agent; the model + provider id come from
29+
// the resolved configuration so the TUI honors a custom `model_provider`.
30+
defaults.UserAgent = "codex-cli-go/" + Version
2931
client := appserverclient.StartInProcess(ctx, appserverclient.InProcessStartArgs{
3032
Assembly: asm,
31-
Defaults: appserver.Defaults{
32-
Model: "gpt-mock",
33-
ProviderID: "openai",
34-
Cwd: resolveCwd(),
35-
UserAgent: "codex-cli-go/" + Version,
36-
},
33+
Defaults: defaults,
3734
})
3835
defer client.Shutdown(context.WithoutCancel(ctx))
3936

internal/cli/cmd_exec.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66

7-
"github.com/sqlrush/codexgo/internal/appserver"
87
"github.com/sqlrush/codexgo/internal/exec"
98
)
109

@@ -18,7 +17,7 @@ func runExecSubcommand(ctx context.Context, parsed ParsedCommandLine, streams St
1817
return 2
1918
}
2019

21-
asm, err := buildAssembly()
20+
asm, defaults, err := buildAssemblyWithDefaults()
2221
if err != nil {
2322
fmt.Fprintln(streams.Stderr, "codex exec:", err)
2423
return 1
@@ -30,12 +29,7 @@ func runExecSubcommand(ctx context.Context, parsed ParsedCommandLine, streams St
3029
Stderr: streams.Stderr,
3130
StdinIsTerminal: streams.StdinIsTerminal,
3231
Assembly: asm,
33-
Defaults: appserver.Defaults{
34-
Model: "gpt-mock",
35-
ProviderID: "openai",
36-
Cwd: resolveCwd(),
37-
UserAgent: "codex-cli-go",
38-
},
32+
Defaults: defaults,
3933
}
4034

4135
return exec.Run(ctx, cli, env)

0 commit comments

Comments
 (0)