[codex] attribute model catalog HTTP state#26080
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff2571354d
ℹ️ 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".
cooper-oai
force-pushed
the
cooper/codex-native-integrity/9
branch
from
ff25713 to
0f22670
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
cooper-oai
force-pushed
the
cooper/codex-native-integrity/8
branch
from
961fe04 to
c6869d5
Compare
cooper-oai
force-pushed
the
cooper/codex-native-integrity/9
branch
from
0f22670 to
d41149a
Compare
|
@codex review |
cooper-oai
force-pushed
the
cooper/codex-native-integrity/8
branch
from
c6869d5 to
37a29a6
Compare
cooper-oai
force-pushed
the
cooper/codex-native-integrity/9
branch
from
d41149a to
0301b58
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0301b5865b
ℹ️ 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".
| chrono = { workspace = true, features = ["serde"] } | ||
| codex-app-server-protocol = { workspace = true } | ||
| codex-collaboration-mode-templates = { workspace = true } | ||
| codex-http-state = { workspace = true } |
There was a problem hiding this comment.
This adds a new Rust dependency for codex-models-manager, but the commit updates only Cargo.toml/Cargo.lock and leaves MODULE.bazel.lock unchanged, so the Bazel dependency graph can drift and just bazel-lock-check/Bazel CI may fail for this crate. The repository rule requires refreshing and including MODULE.bazel.lock whenever Rust dependencies change (AGENTS.md lines 34-37).
Useful? React with 👍 / 👎.
| http_state_surface: Some(HttpStateSurface::from_app_server_client_name( | ||
| app_server_client_name.as_deref().unwrap_or_default(), | ||
| )), |
There was a problem hiding this comment.
This path now passes a client-derived HttpStateSurface into app-server thread startup, which changes user-facing agent startup/model-catalog behavior, but I only found unit-level plumbing coverage rather than an app-server/core integration test that starts a thread and verifies the surface is carried into the agent/model refresh path. The repo guidance says agent logic changes MUST add an integration test under core/suite (AGENTS.md lines 103-107).
Useful? React with 👍 / 👎.
|
|
||
| let config = Arc::new(config); | ||
| let http_state_surface = http_state_surface.unwrap_or_else(|| { | ||
| http_state_surface_for_session(&session_source, /*client*/ None) |
There was a problem hiding this comment.
This positional None is commented as /*client*/, but the callee parameter is named app_server_client_name, so it violates the repository's argument_comment_lint convention and can fail the local/CI lint. The applicable rule requires the comment before opaque positional literals to exactly match the callee signature (AGENTS.md lines 15-18).
Useful? React with 👍 / 👎.
| refresh_strategy: RefreshStrategy, | ||
| http_state_surface: HttpStateSurface, | ||
| ) -> CoreResult<()> { | ||
| self.refresh_available_models_inner(refresh_strategy, Some(http_state_surface)) |
There was a problem hiding this comment.
When a surface-specific refresh uses OnlineIfUncached, this still delegates to the existing cache path, so a fresh models_cache.json written by another app-server client/surface is returned before any /models request is made with the supplied HttpStateSurface. If Desktop and TUI/CLI share the same CODEX_HOME and the backend returns a surface-specific catalog or integrity-state behavior, the second client can silently use the first client's catalog instead of attributing a refresh to its own surface.
Useful? React with 👍 / 👎.
1 participant
Summary
Stack
PR 9 of 15. Depends on #26067. Followed by #26081.
Validation
CARGO_INCREMENTAL=0 cargo check -p codex-models-manager -p codex-model-provider -p codex-core -p codex-app-serverCARGO_INCREMENTAL=0 just test -p codex-models-managerCARGO_INCREMENTAL=0 just test -p codex-model-providerCARGO_INCREMENTAL=0 just fix -p codex-models-manager -p codex-model-provider -p codex-core -p codex-app-server -p codex-memories-writejust fmtjust bazel-lock-checkgit diff --check