[codex] Add externally provided Codex auth#31274
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67d318abc9
ℹ️ 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".
67d318a to
075c379
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 075c379528
ℹ️ 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".
| pub requires_openai_auth: bool, | ||
| /// Whether the active credentials can authenticate Codex backend requests. | ||
| #[serde(default)] | ||
| pub has_codex_backend_auth: bool, |
There was a problem hiding this comment.
Document the new account API fields
This changes the v2 account/read response shape, but app-server/README.md still shows response examples without hasCodexBackendAuth and still lists the old account/updated auth-mode values, so clients following the documented app-server contract will implement stale payload handling. Update the app-server README alongside the schema change.
AGENTS.md reference: AGENTS.md:L298-L300
Useful? React with 👍 / 👎.
362d3e7 to
d10e3d9
Compare
| self.thread_manager | ||
| .plugins_manager() | ||
| .set_auth_mode(self.auth_manager.get_api_auth_mode()); | ||
| .set_auth_mode(self.auth_manager.get_plugin_routing_auth_mode()); |
There was a problem hiding this comment.
can this become set_uses_codex_backend?
There was a problem hiding this comment.
get_plugin_routing_auth_mode feels very hacky
| /// In-memory auth supplied by a trusted embedding runtime. | ||
| /// | ||
| /// This is never loaded from config files or user overrides. | ||
| pub external_auth_snapshot: Option<ExternalAuthSnapshot>, |
There was a problem hiding this comment.
Why do we need this on the config?
pakrym-oai
left a comment
There was a problem hiding this comment.
We should unify on ExternalAuth returning a ExternalAuthSnapshot instead having double code path of resolve + snapshot and separate auth_mode.
ExternalAuthSnapshot should be deleted from config and provided like we normally provide ExternalAuth (via AuthManager passed into ThreadManager)
d10e3d9 to
2e6428a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e6428a59c
ℹ️ 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".
cbffe23 to
de64bf2
Compare
Summary
Add an in-memory externally provided Codex auth snapshot with explicit runtime capabilities, installed through the existing
ExternalAuthprovider path.Testing
just fmt-checkcargo test -p codex-login --lib externally_provided_authcargo test -p codex-core --lib external_auth_snapshot_is_installed_from_runtime_configcargo test -p codex-model-provider --lib external_authcargo test -p codex-mcp-extension --test hosted_apps_mcp hosted_apps_mcp_accepts_external_provided_codex_authcargo check -p codex-app-server -p codex-core-api -p codex-thread-manager-sample