Refresh models on 412 (etag mismatch) #8304

aibrahim-oai · 2025-12-19T03:05:55Z

store etag on client
make TurnContext
refactor refresh_available_models to always refresh without checking cache. Only used in try_refresh_available_models and refreshing mid session
send etag to responses api
remove etag from ModelResponses because we send it as a header

aibrahim-oai · 2025-12-19T03:40:43Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Lines 2412 to 2421 in e01610f

    
           let model_supports_parallel = turn_context 
        
               .client 
        
               .get_model_family() 
        
               .supports_parallel_tool_calls; 
        
           let prompt = Prompt { 
        
               input, 
        
               tools: router.specs(), 
        
               parallel_tool_calls: model_supports_parallel && sess.enabled(Feature::ParallelToolCalls), 
        
               base_instructions_override: turn_context.base_instructions.clone(),

Recompute prompt after model refresh

When an outdated models error occurs, run_turn refreshes the models list and calls turn_context.client.update_model_family(...), but the Prompt is built once before the retry loop with parallel_tool_calls derived from the old model family. Subsequent retries reuse the stale prompt, so if the refreshed model family changes supports_parallel_tool_calls (or other prompt-derived capabilities), the retried request can still send the wrong parallel_tool_calls value. This can cause the retry to fail again (e.g., if the model now disallows parallel tool calls) or miss newly supported behavior. Consider rebuilding the prompt (or at least recomputing parallel_tool_calls) after updating the model family.

ℹ️ 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".

aibrahim-oai · 2025-12-19T03:55:56Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

chatgpt-codex-connector · 2025-12-19T03:57:56Z

codex-rs/core/src/codex.rs

+                    turn_context.client.update_model_family(model_family);
+                    turn_context.client.update_models_etag(models_etag);


Rebuild tools config after model_family refresh

When a 412 triggers OutdatedModels, you update the ModelClient’s model_family/etag and then retry, but the turn’s ToolsConfig/ToolRouter were built once before the loop from the old model_family. If the refreshed models list changes capabilities (e.g., shell_type, apply_patch_tool_type, or experimental_supported_tools), the retry still advertises stale tool definitions, which can keep failing or omit newly required tools. Consider rebuilding the tools config/router (or storing them in a lock like model_family) when the model family is refreshed.

Useful? React with 👍 / 👎.

aibrahim-oai · 2025-12-19T05:26:12Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

chatgpt-codex-connector · 2025-12-19T05:30:30Z

codex-rs/core/src/codex.rs

+                        *turn_context = sess
+                            .new_default_turn_with_sub_id(turn_context.sub_id.clone())
+                            .await;


Preserve per-turn output schema on 412 retry

When a 412 triggers the OutdatedModels retry, the code rebuilds the turn context via new_default_turn_with_sub_id, which reuses the session configuration but drops per-turn overrides like final_output_json_schema (only applied in new_turn_from_configuration when Op::UserTurn supplies it). If a caller requested a structured JSON response, the retry will run without the schema and can return unstructured output, breaking clients that depend on the schema. This only occurs for turns that set final_output_json_schema and then hit the 412 path.

Useful? React with 👍 / 👎.

aibrahim-oai · 2025-12-19T05:59:05Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

chatgpt-codex-connector · 2025-12-19T06:01:39Z

codex-rs/core/src/codex.rs

+        if let Some(final_output_json_schema) = updates.final_output_json_schema.clone() {
+            next_configuration.final_output_json_schema = final_output_json_schema;


Avoid persisting per-turn output schema

Persisting final_output_json_schema in SessionConfiguration makes the schema stick across future turns instead of being scoped to a single UserTurn. Since Op::UserInput and subsequent UserTurns don’t clear it unless the caller explicitly sends a new value, later turns can be unintentionally constrained by an old schema, causing unexpected validation failures or malformed outputs. This is a regression from the previous per-turn override behavior where the schema was applied only for the specific turn.

Useful? React with 👍 / 👎.

aibrahim-oai · 2025-12-19T06:06:17Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

chatgpt-codex-connector · 2025-12-19T06:09:35Z

codex-rs/core/src/codex.rs

+    let session_configuration = sess.state.lock().await.session_configuration.clone();
+    *turn_context = sess
+        .new_turn_from_configuration(
+            turn_context.sub_id.clone(),
+            session_configuration,


Preserve per-turn settings when rebuilding context

When a 412 triggers refresh_models_and_reset_turn_context, the code rebuilds the turn context from session_configuration instead of the existing TurnContext. This drops per-turn overrides (e.g., review threads set base_instructions to REVIEW_PROMPT and disable WebSearch/ViewImage in spawn_review_thread), so the retried request can run with the wrong instructions/tools after an outdated-models error. That changes behavior mid-turn and can invalidate review outputs or re-enable tools that were intentionally disabled for the turn.

Useful? React with 👍 / 👎.

aibrahim-oai · 2025-12-19T06:20:49Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

chatgpt-codex-connector · 2025-12-19T06:24:32Z

codex-rs/core/src/codex.rs

+    let models_etag = sess.services.models_manager.get_models_etag().await;
+    turn_context.client.update_model_family(model_family).await;
+    turn_context.client.update_models_etag(models_etag).await;


Rebuild tool config after models refresh

After refreshing models on a 412, you only update the client’s model family and ETag, but TurnContext.tools_config is still the one built before the refresh. ToolsConfig::new depends on the model family (e.g., shell_type, apply_patch_tool_type, experimental_supported_tools), so if the catalog change altered tool capabilities, the retry will keep advertising the old tool set and can continue to fail or send unsupported tools. Consider rebuilding tools_config (or a new turn context) alongside the refreshed model family/etag so retries reflect the updated model metadata.

Useful? React with 👍 / 👎.

aibrahim-oai · 2025-12-19T06:39:05Z

@codex review this

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

codex-rs/core/src/client.rs

aibrahim-oai · 2025-12-19T06:57:15Z

@codex review this

chatgpt-codex-connector · 2025-12-19T07:05:27Z

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ 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".

pakrym-oai · 2025-12-19T17:19:01Z

codex-rs/core/src/api_bridge.rs

                        status,
                        request_id: extract_request_id(headers.as_ref()),
                    })
+                } else if status == http::StatusCode::PRECONDITION_FAILED


use a status code, don't match on strings

pakrym-oai · 2025-12-19T17:20:02Z

codex-rs/core/src/client.rs

+    model_family: RwLock<ModelFamily>,
+    models_etag: RwLock<Option<String>>,


These two values are connected and should be only used/updated together. Let's not spread them across two fields.

pakrym-oai · 2025-12-19T17:20:54Z

codex-rs/core/src/client.rs

+        self.model_family.read().await.clone()
+    }
+
+    pub async fn get_models_etag(&self) -> Option<String> {


We shouldn't make the client event larger model storage God-object.

pakrym-oai · 2025-12-19T17:24:09Z

codex-rs/core/src/client_common.rs

 }

 impl Prompt {
+    pub(crate) async fn new(


why do we need this extracted?
This is one way to create the prompt but not the only one.

aibrahim-oai added 5 commits December 18, 2025 17:59

etag remove

beb8322

progress

677532f

progress

f8ba48d

rwlock

09693d2

unit test

e01610f

aibrahim-oai changed the title ~~tag~~ Refresh models on 412 (etag mismatch) Dec 19, 2025

chatgpt-codex-connector bot reviewed Dec 19, 2025

View reviewed changes

aibrahim-oai added 2 commits December 18, 2025 19:44

comments

985333f

comments

ecff4d4

chatgpt-codex-connector bot reviewed Dec 19, 2025

View reviewed changes

aibrahim-oai added 2 commits December 18, 2025 20:03

comments

359142f

test

1a5289a

chatgpt-codex-connector bot reviewed Dec 19, 2025

View reviewed changes

aibrahim-oai added 2 commits December 18, 2025 21:31

Merge branch 'main' into tag

42273d9

error

27cec53

chatgpt-codex-connector bot reviewed Dec 19, 2025

View reviewed changes

aibrahim-oai force-pushed the tag branch from 0978f99 to 27cec53 Compare December 19, 2025 06:03

final_output_json_schema

6912ba9

chatgpt-codex-connector bot reviewed Dec 19, 2025

View reviewed changes

rwlock

348d379

chatgpt-codex-connector bot reviewed Dec 19, 2025

View reviewed changes

rwlock

0a13237

chatgpt-codex-connector bot reviewed Dec 19, 2025

View reviewed changes

codex-rs/core/src/client.rs Show resolved Hide resolved

compact

124c7fc

pakrym-oai reviewed Dec 19, 2025

View reviewed changes

	let model_supports_parallel = turn_context
	.client
	.get_model_family()
	.supports_parallel_tool_calls;

	let prompt = Prompt {
	input,
	tools: router.specs(),
	parallel_tool_calls: model_supports_parallel && sess.enabled(Feature::ParallelToolCalls),
	base_instructions_override: turn_context.base_instructions.clone(),

		turn_context.client.update_model_family(model_family);
		turn_context.client.update_models_etag(models_etag);

		if let Some(final_output_json_schema) = updates.final_output_json_schema.clone() {
		next_configuration.final_output_json_schema = final_output_json_schema;

		model_family: RwLock<ModelFamily>,
		models_etag: RwLock<Option<String>>,

Refresh models on 412 (etag mismatch) #8304

Are you sure you want to change the base?

Refresh models on 412 (etag mismatch) #8304

Conversation

aibrahim-oai commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

aibrahim-oai commented Dec 19, 2025

Uh oh!

chatgpt-codex-connector bot commented Dec 19, 2025

Uh oh!

pakrym-oai Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

pakrym-oai Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

pakrym-oai Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

pakrym-oai Dec 19, 2025

Choose a reason for hiding this comment

aibrahim-oai commented Dec 19, 2025 •

edited

Loading