Repository navigation
fix(cli): support custom inference providers in patch commands - #1131
Conversation
soyeon-oai
left a comment
There was a problem hiding this comment.
Code review LGTM: traced custom-provider parsing/serialization, authentication and environment forwarding into standalone patch and patch-risk flows, including dotted provider IDs. No actionable regression found. This remains a draft; hosted Windows tests-2 has two Python-executable failures in import-scan tests, so CI is not fully green. I did not independently run tests or reproduce those failures on the base.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f8d085c63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| command: verify ? "verify-fix" : patch || assess ? "patch" : "validate", | ||
| auth: options.auth ?? "auto", | ||
| directory, | ||
| modelProvider: options.provider, | ||
| providerConfiguration: options.providerConfiguration, | ||
| modelProvider: provider, | ||
| providerConfiguration, |
There was a problem hiding this comment.
When validate, patch, or verify-fix is run with --auth api-key and a custom provider whose credential is declared through model_providers.<name>.env_key (for example, the documented GATEWAY_API_KEY), forwarding the provider here causes runCodexSkillCommand to call scanAuthentication, but that function recognizes provider-specific keys only for the built-in external providers and otherwise requires OPENAI_API_KEY or CODEX_API_KEY. The command therefore throws before spawning Codex unless the user supplies an unrelated OpenAI key; derive API-key authentication from providerConfiguration.env_key for configured custom providers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c792ab86ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const provider = | ||
| options.provider ?? (overrides["model_provider"] as string | undefined); |
There was a problem hiding this comment.
When a standalone validate, patch, or verify-fix selects model_provider="amazon-bedrock", mergedCodexConfig deliberately computes model_reasoning_summary="none" because some Bedrock models reject reasoning.summary (config.ts:225-227), but this path discards that effective property and later forwards only the model and effort. The allowlist also rejects an explicit model_reasoning_summary override, so affected models have no workaround and can reject the request before the skill runs; forward the effective Bedrock summary setting along with the newly accepted provider.
Useful? React with 👍 / 👎.
Summary
Standalone patch commands reject custom inference-provider settings that scans accept. Allow the same provider selection and configuration to reach Codex.
Changes
model_providerandmodel_providersin the sharedpatch,validate, andverify-fixoverride path.--codexsyntax, and add regression coverage. No new flags or default changes.Testing
Passed:
pnpm run build:cipnpm run typespnpm run formatgit diff --checkThe new provider regression failed before the fix and passes after it. Tests use synthetic providers and mock model calls; no live inference gateway was contacted.
The four-file skill/patch/authentication suite is not green in this local runtime: filesystem ownership checks and Git checkout checks fail. The unchanged base has the same 52 distinct failure names. The two added authentication cases also encounter this environment limitation. The full seeded suite was stopped after reproducing these environment failures; hosted CI is still required.
Risk and rollout
This uses the existing provider serialization and authentication path. The shared skill runner also makes these provider overrides available to validation and fix verification. Existing model defaults, sandbox policies, and approval policies do not change. Provider settings are scoped to each command. Merge and a CLI release are required before users receive the fix.
Public disclosure review