launch: avoid legacy Codex App profiles (#16364)

* launch: avoid legacy Codex App profiles --------- Co-authored-by: ParthSareen <parth.sareen@ollama.com>
2026-07-23 09:10:53 -05:00 · 2026-06-02 02:49:56 +08:00
parent 630882621b
commit 5e9636fa05
3 changed files with 204 additions and 22 deletions
--- a/cmd/launch/codex.go
+++ b/cmd/launch/codex.go
@@ -356,6 +356,24 @@ func (c codexParsedConfig) String(path ...string) (string, bool) {
 	return value, true
 }

+func (c codexParsedConfig) Exists(path ...string) bool {
+	if len(path) == 0 {
+		return false
+	}
+	var current any = c.values
+	for _, part := range path {
+		table, ok := current.(map[string]any)
+		if !ok {
+			return false
+		}
+		current, ok = table[part]
+		if !ok {
+			return false
+		}
+	}
+	return true
+}
+
 func (c codexParsedConfig) RootString(key string) string {
 	value, _ := c.RootStringOK(key)
 	return value
--- a/cmd/launch/codex_app.go
+++ b/cmd/launch/codex_app.go
@@ -85,14 +85,7 @@ func (c *CodexApp) ConfigureWithModels(primary string, models []LaunchModel) err
 	if err := writeCodexAppModelCatalog(catalogPath, primary, codexAppCatalogModels(primary, models)); err != nil {
 		return err
 	}
-	return writeCodexLaunchProfile(configPath, codexLaunchProfileOptions{
-		activate:           true,
-		profileName:        codexAppProfileName,
-		setRootModelConfig: true,
-		model:              primary,
-		modelCatalogPath:   catalogPath,
-		backupIntegration:  codexAppIntegrationName,
-	})
+	return writeCodexAppConfig(configPath, primary, catalogPath)
 }

 func (c *CodexApp) CurrentModel() string {
@@ -160,7 +153,7 @@ func codexAppCatalogHealthy(config codexParsedConfig, profileName string) bool {
 	if config.RootString(codexRootModelCatalogJSONKey) != catalogPath {
 		return false
 	}
-	if config.ProfileString(profileName, codexRootModelCatalogJSONKey) != catalogPath {
+	if config.Exists("profiles", profileName) && config.ProfileString(profileName, codexRootModelCatalogJSONKey) != catalogPath {
 		return false
 	}
 	data, err := os.ReadFile(catalogPath)
@@ -176,6 +169,69 @@ func codexAppCatalogHealthy(config codexParsedConfig, profileName string) bool {
 	return len(catalog.Models) > 0
 }

+func writeCodexAppConfig(configPath, model, modelCatalogPath string) error {
+	baseURL := codexBaseURL()
+
+	content, readErr := os.ReadFile(configPath)
+	text := ""
+	if readErr == nil {
+		text = string(content)
+	} else if !os.IsNotExist(readErr) {
+		return readErr
+	}
+	if _, err := codexParseConfig(text); err != nil {
+		return err
+	}
+
+	text = codexRemoveRootValue(text, codexRootProfileKey)
+	text = codexRemoveSection(text, codexProfileHeaderFor(codexAppProfileName))
+	text = codexSetRootStringValue(text, codexRootModelKey, model)
+	text = codexSetRootStringValue(text, codexRootModelProviderKey, codexAppProfileName)
+	text = codexSetRootStringValue(text, codexRootModelCatalogJSONKey, modelCatalogPath)
+	text = codexUpsertSection(text, codexProviderHeaderFor(codexAppProfileName), []string{
+		fmt.Sprintf("name = %q", codexProviderName),
+		fmt.Sprintf("base_url = %q", baseURL),
+		`wire_api = "responses"`,
+	})
+
+	parsed, err := codexParseConfig(text)
+	if err != nil {
+		return err
+	}
+	if err := codexValidateAppConfigText(parsed, model, modelCatalogPath, baseURL); err != nil {
+		return err
+	}
+	if err := os.MkdirAll(filepath.Dir(configPath), 0o755); err != nil {
+		return err
+	}
+	return fileutil.WriteWithBackup(configPath, []byte(text), codexAppIntegrationName)
+}
+
+func codexValidateAppConfigText(config codexParsedConfig, model, modelCatalogPath, baseURL string) error {
+	if got, ok := config.RootStringOK(codexRootProfileKey); ok {
+		return fmt.Errorf("generated Codex App config still contains legacy profile = %q", got)
+	}
+	if config.Exists("profiles", codexAppProfileName) {
+		return fmt.Errorf("generated Codex App config still contains legacy profiles.%s table", codexAppProfileName)
+	}
+	for _, check := range []struct {
+		path []string
+		want string
+	}{
+		{[]string{codexRootModelKey}, model},
+		{[]string{codexRootModelProviderKey}, codexAppProfileName},
+		{[]string{codexRootModelCatalogJSONKey}, modelCatalogPath},
+		{[]string{"model_providers", codexAppProfileName, "name"}, codexProviderName},
+		{[]string{"model_providers", codexAppProfileName, "base_url"}, baseURL},
+		{[]string{"model_providers", codexAppProfileName, "wire_api"}, "responses"},
+	} {
+		if got, ok := config.String(check.path...); !ok || got != check.want {
+			return fmt.Errorf("generated Codex App config missing %s = %q", strings.Join(check.path, "."), check.want)
+		}
+	}
+	return nil
+}
+
 func (c *CodexApp) Onboard() error {
 	return config.MarkIntegrationOnboarded(codexAppIntegrationName)
 }
@@ -953,6 +1009,12 @@ func saveCodexAppRestoreState(configPath string) error {
 			return err
 		}
 		upgraded := codexAppRestoreStateFromText(configText)
+		if codexAppRootStillManaged(configText) {
+			// Legacy restore state did not record root model settings. If the
+			// current config is still ours, do not save our generated root
+			// values as the user's restore target.
+			upgraded = codexAppRestoreState{}
+		}
 		upgraded.HadProfile = existing.HadProfile
 		upgraded.Profile = existing.Profile
 		return writeCodexAppRestoreState(upgraded)
--- a/cmd/launch/codex_app_test.go
+++ b/cmd/launch/codex_app_test.go
@@ -157,7 +157,7 @@ func TestCodexAppInstalledUsesMacBundleIDFallback(t *testing.T) {
 	}
 }

-func TestCodexAppConfigureActivatesOllamaProfile(t *testing.T) {
+func TestCodexAppConfigureActivatesOllamaProviderWithoutLegacyProfile(t *testing.T) {
 	tmpDir := t.TempDir()
 	setTestHome(t, tmpDir)
 	t.Setenv("OLLAMA_HOST", "http://127.0.0.1:9999")
@@ -191,15 +191,9 @@ func TestCodexAppConfigureActivatesOllamaProfile(t *testing.T) {
 	}

 	for _, want := range []string{
-		fmt.Sprintf(`profile = %q`, codexAppProfileName),
 		`model = "llama3.2"`,
 		fmt.Sprintf(`model_provider = %q`, codexAppProfileName),
 		fmt.Sprintf(`model_catalog_json = %q`, catalogPath),
-		codexProfileHeaderFor(codexAppProfileName),
-		`model = "llama3.2"`,
-		`openai_base_url = "http://127.0.0.1:9999/v1/"`,
-		fmt.Sprintf(`model_provider = %q`, codexAppProfileName),
-		`model_catalog_json = "`,
 		codexProviderHeaderFor(codexAppProfileName),
 		`name = "Ollama"`,
 		`base_url = "http://127.0.0.1:9999/v1/"`,
@@ -210,6 +204,12 @@ func TestCodexAppConfigureActivatesOllamaProfile(t *testing.T) {
 			t.Fatalf("expected config to contain %q, got:\n%s", want, content)
 		}
 	}
+	if got, ok := codexRootStringValueOK(content, "profile"); ok {
+		t.Fatalf("legacy root profile should be removed, got %q in:\n%s", got, content)
+	}
+	if strings.Contains(content, codexProfileHeaderFor(codexAppProfileName)) {
+		t.Fatalf("legacy app profile section should not be generated, got:\n%s", content)
+	}
 	if got := c.CurrentModel(); got != "llama3.2" {
 		t.Fatalf("CurrentModel = %q, want llama3.2", got)
 	}
@@ -270,8 +270,8 @@ func TestCodexAppConfigureUsesAppSpecificProfileWithoutTouchingCLIProfile(t *tes
 		t.Fatal(err)
 	}
 	content := string(data)
-	if got := codexRootStringValue(content, "profile"); got != codexAppProfileName {
-		t.Fatalf("root profile = %q, want %q", got, codexAppProfileName)
+	if got, ok := codexRootStringValueOK(content, "profile"); ok {
+		t.Fatalf("legacy root profile should be removed, got %q in:\n%s", got, content)
 	}
 	if got := codexSectionStringValue(content, codexProfileHeader(), "openai_base_url"); got != "http://cli.invalid/v1/" {
 		t.Fatalf("CLI profile base URL = %q, want preserved CLI URL in:\n%s", got, content)
@@ -279,8 +279,11 @@ func TestCodexAppConfigureUsesAppSpecificProfileWithoutTouchingCLIProfile(t *tes
 	if got := codexSectionStringValue(content, codexProviderHeader(), "name"); got != "CLI Ollama" {
 		t.Fatalf("CLI provider name = %q, want preserved CLI provider in:\n%s", got, content)
 	}
-	if got := codexSectionStringValue(content, codexProfileHeaderFor(codexAppProfileName), "model"); got != "llama3.2" {
-		t.Fatalf("app profile model = %q, want llama3.2", got)
+	if strings.Contains(content, codexProfileHeaderFor(codexAppProfileName)) {
+		t.Fatalf("legacy app profile section should not be generated, got:\n%s", content)
+	}
+	if got := codexRootStringValue(content, "model"); got != "llama3.2" {
+		t.Fatalf("root model = %q, want llama3.2", got)
 	}
 	if got := codexSectionStringValue(content, codexProviderHeaderFor(codexAppProfileName), "base_url"); got != "http://127.0.0.1:9999/v1/" {
 		t.Fatalf("app provider base URL = %q", got)
@@ -306,8 +309,8 @@ func TestCodexAppConfigureUsesConnectableHostForUnspecifiedBindAddress(t *testin
 	if strings.Contains(content, "0.0.0.0") {
 		t.Fatalf("config should not write bind-only host, got:\n%s", content)
 	}
-	if got := codexSectionStringValue(content, codexProfileHeaderFor(codexAppProfileName), "openai_base_url"); got != "http://127.0.0.1:11434/v1/" {
-		t.Fatalf("app profile openai_base_url = %q, want connectable loopback URL", got)
+	if strings.Contains(content, codexProfileHeaderFor(codexAppProfileName)) {
+		t.Fatalf("legacy app profile section should not be generated, got:\n%s", content)
 	}
 	if got := codexSectionStringValue(content, codexProviderHeaderFor(codexAppProfileName), "base_url"); got != "http://127.0.0.1:11434/v1/" {
 		t.Fatalf("app provider base_url = %q, want connectable loopback URL", got)
@@ -635,6 +638,105 @@ func TestCodexAppConfigureUpgradesLegacyRestoreState(t *testing.T) {
 	}
 }

+func TestCodexAppConfigureMigratesLegacyManagedConfigWithoutPollutingRestoreState(t *testing.T) {
+	tmpDir := t.TempDir()
+	setTestHome(t, tmpDir)
+	t.Setenv("OLLAMA_HOST", "http://127.0.0.1:9999")
+	withCodexAppPlatform(t, "darwin")
+
+	var openCalls int
+	withCodexAppProcessHooks(t,
+		func() bool { return false },
+		func() error { return nil },
+		func() error {
+			openCalls++
+			return nil
+		},
+	)
+
+	configPath := filepath.Join(tmpDir, ".codex", "config.toml")
+	if err := os.MkdirAll(filepath.Dir(configPath), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	catalogPath := mustCodexAppModelCatalogPath(t)
+	existing := "" +
+		fmt.Sprintf(`profile = %q`, codexAppProfileName) + "\n" +
+		`model = "llama3.2"` + "\n" +
+		fmt.Sprintf(`model_provider = %q`, codexAppProfileName) + "\n" +
+		fmt.Sprintf(`model_catalog_json = %q`, catalogPath) + "\n\n" +
+		codexProfileHeaderFor(codexAppProfileName) + "\n" +
+		`model = "llama3.2"` + "\n" +
+		fmt.Sprintf(`model_provider = %q`, codexAppProfileName) + "\n" +
+		fmt.Sprintf(`model_catalog_json = %q`, catalogPath) + "\n\n" +
+		codexProviderHeaderFor(codexAppProfileName) + "\n" +
+		`name = "Ollama"` + "\n" +
+		`base_url = "http://127.0.0.1:9999/v1/"` + "\n" +
+		`wire_api = "responses"` + "\n\n" +
+		"[profiles.default]\n" +
+		`model = "gpt-5.5"` + "\n"
+	if err := os.WriteFile(configPath, []byte(existing), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(filepath.Dir(codexAppRestoreStatePath()), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(codexAppRestoreStatePath(), []byte(`{"had_profile":true,"profile":"default"}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	c := &CodexApp{}
+	if err := c.ConfigureWithModels("qwen3:8b", testLaunchModels("qwen3:8b")); err != nil {
+		t.Fatalf("ConfigureWithModels returned error: %v", err)
+	}
+
+	state, err := loadCodexAppRestoreState()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !state.HadProfile || state.Profile != "default" {
+		t.Fatalf("profile restore state = (%v, %q), want default", state.HadProfile, state.Profile)
+	}
+	if state.HadModel || state.HadModelProvider || state.HadModelCatalogJSON {
+		t.Fatalf("legacy restore state should not capture managed root values: %+v", state)
+	}
+
+	data, err := os.ReadFile(configPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	migrated := string(data)
+	if got, ok := codexRootStringValueOK(migrated, "profile"); ok {
+		t.Fatalf("legacy root profile should be removed during migration, got %q in:\n%s", got, migrated)
+	}
+	if strings.Contains(migrated, codexProfileHeaderFor(codexAppProfileName)) {
+		t.Fatalf("legacy app profile section should be removed during migration, got:\n%s", migrated)
+	}
+
+	if err := c.Restore(); err != nil {
+		t.Fatalf("Restore returned error: %v", err)
+	}
+
+	data, err = os.ReadFile(configPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	restored := string(data)
+	if got := codexRootStringValue(restored, "profile"); got != "default" {
+		t.Fatalf("root profile = %q, want default in:\n%s", got, restored)
+	}
+	for _, key := range []string{"model", "model_provider", "model_catalog_json"} {
+		if got, ok := codexRootStringValueOK(restored, key); ok {
+			t.Fatalf("root %s should be removed on restore, got %q in:\n%s", key, got, restored)
+		}
+	}
+	if strings.Contains(restored, codexProfileHeaderFor(codexAppProfileName)) || strings.Contains(restored, codexProviderHeaderFor(codexAppProfileName)) {
+		t.Fatalf("owned app config should be removed on restore, got:\n%s", restored)
+	}
+	if openCalls != 1 {
+		t.Fatalf("open calls = %d, want 1", openCalls)
+	}
+}
+
 func TestCodexAppRestoreRestoresPreviousProfile(t *testing.T) {
 	tmpDir := t.TempDir()
 	setTestHome(t, tmpDir)