Skip to content

fix(app): preserve thread model choices #2061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Yuimi-chaya wants to merge 1 commit into
base: main
Choose a base branch
from
+102 −22
Open

fix(app): preserve thread model choices #2061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions assets/inject/renderer-inject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3331,9 +3331,10 @@
}

function codexRemoteSessionProviderRequestMethod(method) {
// app-server restores persisted model/provider/reasoning for thread/resume only
// when the caller supplies none of those overrides.
return [
"thread/start",
"thread/resume",
"start-conversation",
"start-thread-for-host",
"thread-prewarm-start",
Expand All @@ -3349,8 +3350,7 @@
if (!params || typeof params !== "object" || Array.isArray(params)) return params;
const profile = codexRemoteSessionActiveProfile();
const pureApi = String(profile?.relayMode || "") === "pureApi";
const isExtendedPureApiRequest = requestMethod === "thread/resume" || requestMethod === "turn/start";
if (isExtendedPureApiRequest && !pureApi) return params;
if (requestMethod === "turn/start" && !pureApi) return params;
const hasModelProvider = Object.prototype.hasOwnProperty.call(params, "modelProvider")
|| Object.prototype.hasOwnProperty.call(params, "model_provider");
if (requestMethod === "turn/start" && !hasModelProvider) return params;
Expand Down Expand Up @@ -6566,7 +6566,7 @@
displayName: metadata?.displayName || modelName,
description: metadata?.description || codexModelCatalog.provider_name || codexModelCatalog.model_provider || "Custom model",
hidden: false,
isDefault: (codexModelCatalog.default_model || codexModelCatalog.model) === modelName,
isDefault: false,
defaultReasoningEffort: metadata?.defaultReasoningEffort || "medium",
supportedReasoningEfforts: modelReasoningEfforts(modelName),
};
Expand Down Expand Up @@ -6675,13 +6675,6 @@
value.hidden_models = value.hidden_models.filter((name) => !names.includes(name));
if (value.hidden_models.length !== before) changed = true;
}
if (value.defaultModel == null && names.length > 0) {
value.defaultModel = codexPlusModelDescriptor(names[0]);
changed = true;
} else if (typeof value.defaultModel === "string" && names.includes(value.defaultModel) && value.model == null) {
value.model = value.defaultModel;
changed = true;
}
return changed;
}

Expand Down Expand Up @@ -6748,12 +6741,8 @@
changed = true;
}
});
const nextValue = {
...value,
available_models: availableModels,
default_model: names[0] || value.default_model,
};
if (!changed && nextValue.default_model === value.default_model) return config;
if (!changed) return config;
const nextValue = { ...value, available_models: availableModels };
try {
config.value = nextValue;
} catch {
Expand Down
101 changes: 96 additions & 5 deletions crates/codex-plus-core/tests/cdp_bridge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1673,10 +1673,92 @@ fn injection_script_unlocks_custom_model_catalog() {
assert!(!script.contains("function patchObjectGraphForModels"));
assert!(!script.contains("window.dispatchEvent = function patchedCodexPlusDispatchEvent"));
assert!(script.contains("String(name) === \"107580212\""));
assert!(script.contains("isDefault: false"));
assert!(!script.contains("value.defaultModel = codexPlusModelDescriptor(names[0])"));
assert!(!script.contains("value.model = value.defaultModel"));
assert!(!script.contains("default_model: value.default_model || names[0]"));
assert!(!script.contains("default_model: names[0] || value.default_model"));
assert!(script.contains("window.addEventListener(\"codex-message-from-view\""));
assert!(!script.contains("querySelectorAll(\"button, [role='menu']"));
}

#[test]
fn model_whitelist_never_claims_the_host_default_model() {
let script = assets::injection_script(57321);
let start = script
.find("function codexPlusModelDescriptor(modelName)")
.expect("model descriptor patch should exist");
let end = script[start..]
.find("\n function statsigClients()")
.map(|offset| start + offset)
.expect("model whitelist patch should have a stable end marker");
let function_source = &script[start..end];
let temp = tempfile::tempdir().expect("temp dir should be created");
let harness_path = temp.path().join("model-default-harness.cjs");
std::fs::write(
&harness_path,
format!(
r#"
const codexModelCatalog = {{
default_model: "supplier-default",
model: "supplier-default",
provider_name: "Supplier",
model_provider: "custom",
}};
const codexPlusModelMetadata = () => null;
const modelReasoningEfforts = () => [];
const applyCodexPlusModelMetadata = () => false;
const codexPlusModelNames = () => ["supplier-default", "extra-model"];
{function_source}

const missingDefault = {{ value: {{ available_models: ["native-model"] }} }};
const patchedMissingDefault = patchStatsigModelDynamicConfig(missingDefault);
if (Object.prototype.hasOwnProperty.call(patchedMissingDefault.value, "default_model")) process.exit(2);
if (!patchedMissingDefault.value.available_models.includes("supplier-default")) process.exit(3);
if (!patchedMissingDefault.value.available_models.includes("extra-model")) process.exit(4);

const existingDefault = {{ value: {{ available_models: ["native-model"], default_model: "thread-model" }} }};
const patchedExistingDefault = patchStatsigModelDynamicConfig(existingDefault);
if (patchedExistingDefault.value.default_model !== "thread-model") process.exit(5);

const missingContainerDefault = {{
models: [{{ model: "native-model", isDefault: true }}],
availableModels: ["native-model"],
}};
patchModelContainer(missingContainerDefault);
if (Object.prototype.hasOwnProperty.call(missingContainerDefault, "defaultModel")) process.exit(6);
if (Object.prototype.hasOwnProperty.call(missingContainerDefault, "model")) process.exit(7);
const injectedDefault = missingContainerDefault.models.find((item) => item.model === "supplier-default");
if (!injectedDefault || injectedDefault.isDefault !== false) process.exit(8);

const hostDefault = {{ model: "native-model" }};
const hostModel = {{ model: "thread-model" }};
const existingContainerDefault = {{
models: [{{ model: "native-model", isDefault: true }}],
availableModels: ["native-model"],
defaultModel: hostDefault,
model: hostModel,
}};
patchModelContainer(existingContainerDefault);
if (existingContainerDefault.defaultModel !== hostDefault) process.exit(9);
if (existingContainerDefault.model !== hostModel) process.exit(10);
"#
),
)
.expect("model default harness should be written");

let output = Command::new("node")
.arg(&harness_path)
.output()
.expect("node should run model default harness");
assert!(
output.status.success(),
"node harness failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
}

#[test]
fn injection_script_exposes_fast_service_tier_control() {
let script = assets::injection_script(57321);
Expand Down Expand Up @@ -1970,12 +2052,14 @@ fn injection_script_applies_fast_service_tier_contract() {
assert_eq!(cases["openAiNormalizationDisabled"], true);
assert_eq!(cases["refreshedCustomProviderOverride"], "refreshed_vendor");
assert_eq!(cases["refreshedOpenAiProvider"], "openai");
assert_eq!(cases["refreshedPureApiResumeProvider"], "custom");
assert_eq!(cases["refreshedPureApiResumeProvider"], "openai");
assert_eq!(cases["refreshedPureApiResumeModel"], "gpt-5.6-terra");
assert_eq!(cases["failedRefreshProviderUnchanged"], "openai");
assert_eq!(cases["missingActiveProviderUnchanged"], true);
assert_eq!(cases["missingActiveRecoveryUnscheduled"], true);
assert_eq!(cases["pureApiThreadStartProvider"], "custom");
assert_eq!(cases["pureApiThreadResumeProvider"], "custom");
assert_eq!(cases["pureApiThreadResumeUnchanged"], true);
assert_eq!(cases["pureApiNativeResumeUnchanged"], true);
assert_eq!(cases["pureApiTurnStartProvider"], "custom");
assert_eq!(cases["pureApiTurnWithoutProviderUnchanged"], true);
assert_eq!(cases["pureApiOtherProviderUnchanged"], true);
Expand Down Expand Up @@ -2495,8 +2579,10 @@ window.__codexSessionDeleteBridge = async (path) => {{
await appServerClient.sendRequest("thread/resume", {{
threadId: "thread-mobile-pure-api-refresh",
modelProvider: "openai",
model: "gpt-5.6-terra",
}}, {{ signal: "pure-api-switch" }});
const refreshedPureApiResumeProvider = appServerCalls.at(-1)?.params?.modelProvider || "";
const refreshedPureApiResumeModel = appServerCalls.at(-1)?.params?.model || "";
delete window.__codexSessionDeleteBridge;
api.setBackendSettings({{
relayProfilesEnabled: true,
Expand Down Expand Up @@ -2530,10 +2616,13 @@ api.setBackendSettings({{
}});
const pureApiParams = {{ cwd: "C:/mobile", modelProvider: "openai" }};
const pureApiThreadStartProvider = api.applyProviderOverride("thread/start", pureApiParams)?.modelProvider;
const pureApiThreadResumeProvider = api.applyProviderOverride("thread/resume", {{
const pureApiThreadResumeParams = {{
threadId: "thread-mobile-pure-api",
model_provider: "openai",
}})?.modelProvider;
}};
const pureApiThreadResumeUnchanged = api.applyProviderOverride("thread/resume", pureApiThreadResumeParams) === pureApiThreadResumeParams;
const pureApiNativeResumeParams = {{ threadId: "thread-mobile-native-resume" }};
const pureApiNativeResumeUnchanged = api.applyProviderOverride("thread/resume", pureApiNativeResumeParams) === pureApiNativeResumeParams;
const pureApiTurnStartProvider = api.applyProviderOverride("turn/start", {{
threadId: "thread-mobile-pure-api",
modelProvider: "openai",
Expand Down Expand Up @@ -2609,11 +2698,13 @@ process.stdout.write(JSON.stringify({{
refreshedCustomProviderOverride,
refreshedOpenAiProvider,
refreshedPureApiResumeProvider,
refreshedPureApiResumeModel,
failedRefreshProviderUnchanged,
missingActiveProviderUnchanged,
missingActiveRecoveryUnscheduled,
pureApiThreadStartProvider,
pureApiThreadResumeProvider,
pureApiThreadResumeUnchanged,
pureApiNativeResumeUnchanged,
pureApiTurnStartProvider,
pureApiTurnWithoutProviderUnchanged,
pureApiOtherProviderUnchanged,
Expand Down
Loading