diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index ab28f14..4aa2d01 100644 --- a/internal/tui/config_test.go +++ b/internal/tui/config_test.go @@ -410,6 +410,208 @@ func TestCodexConfigDoesNotTouchAnExistingChoice(t *testing.T) { } } +// The file this repairs is the one a member actually turns up with: the +// provider section is ours and already fine, and Codex still will not open - +// not the CLI, not the desktop app, which shows "failed to read +// configuration layers: duplicate key" in a dialog and quits. Two runs of an +// older setup put two model_context_window lines at the end of the file, +// which is both a duplicate key and, after a [projects.*] header, not a root +// key at all. Repairing only the wire_api left that member exactly as stuck. +func TestCodexConfigOpensAFileItLeftUnloadable(t *testing.T) { + codexModel, _ := catalog.Get(catalog.Coding) + path := tempConfig(t, "config.toml") + existing := fmt.Sprintf(`model = "gpt-5" + +[projects."/home/member/repo"] +trust_level = "trusted" + +model_context_window = %d + +model_context_window = %d + +[model_providers.nan] +name = "NaN" +base_url = "https://api.nan.builders/v1" +wire_api = "chat" +`, codexModel.Context, codexModel.Context) + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + data, _ := os.ReadFile(path) + content := string(data) + + if n := strings.Count(content, "model_context_window"); n != 1 { + t.Errorf("%d model_context_window keys, so Codex still refuses the file:\n%s", n, content) + } + window, header := strings.Index(content, "model_context_window"), strings.Index(content, "[projects.") + if window > header { + t.Errorf("the surviving window is inside a table, where Codex does not read it:\n%s", content) + } + if !strings.Contains(content, `wire_api = "responses"`) { + t.Error("the wire_api repair stopped happening once the pruning was added") + } + if !strings.Contains(content, `trust_level = "trusted"`) { + t.Error("the member's own project entry was lost") + } +} + +// A window the member set themselves is a window they meant, whatever it +// says. Ours is recognisable by its value, and only inside a table - in the +// root it is indistinguishable from theirs, so there the first one stands. +func TestCodexConfigKeepsAWindowTheMemberDeclared(t *testing.T) { + path := tempConfig(t, "config.toml") + existing := `model = "gpt-5" +model_context_window = 200000 + +[model_providers.nan] +base_url = "https://api.nan.builders/v1" +wire_api = "responses" +` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + data, _ := os.ReadFile(path) + if string(data) != existing { + t.Errorf("a file with nothing wrong with it was rewritten:\n%s", data) + } +} + +// Disconnecting takes the section out. The window went in beside it and has +// no meaning without it, and leaving two of them behind would hand back a +// file Codex does not open with nothing in it left to blame. +func TestCodexRemovalTakesTheWindowItWrote(t *testing.T) { + codexModel, _ := catalog.Get(catalog.Coding) + path := tempConfig(t, "config.toml") + existing := fmt.Sprintf(`model = "gpt-5" + +[projects."/home/member/repo"] +trust_level = "trusted" + +model_context_window = %d + +[model_providers.nan] +base_url = "https://api.nan.builders/v1" +wire_api = "responses" +`, codexModel.Context) + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + if err := removeCodexConfig(path); err != nil { + t.Fatal(err) + } + data, _ := os.ReadFile(path) + content := string(data) + + if strings.Contains(content, "model_context_window") { + t.Errorf("the window we wrote outlived the section it belonged to:\n%s", content) + } + if strings.Contains(content, "api.nan.builders") { + t.Error("the provider section survived a disconnect") + } + if !strings.Contains(content, `trust_level = "trusted"`) { + t.Error("the member's own project entry was lost") + } +} + +// `codex --model ` moves the model and leaves model_context_window +// behind, so a 262,144-token model runs with whatever window config.toml +// declares. A file per model is the only place Codex 0.155 lets that window +// travel with the model it belongs to. +func TestCodexProfilesGiveEveryModelItsOwnWindow(t *testing.T) { + path := tempConfig(t, "config.toml") + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + home := filepath.Dir(path) + + for _, m := range catalog.ChatModels() { + profile := filepath.Join(home, codexProfileName(m.ID)+".config.toml") + data, err := os.ReadFile(profile) + if err != nil { + t.Errorf("%s has no profile, so it is only reachable with the wrong window: %v", m.ID, err) + continue + } + body := string(data) + if !strings.Contains(body, fmt.Sprintf("model = %q", m.ID)) { + t.Errorf("%s: the profile does not select the model it is named after:\n%s", m.ID, body) + } + if !strings.Contains(body, fmt.Sprintf("model_context_window = %d", m.Context)) { + t.Errorf("%s: window is not the one the cluster serves:\n%s", m.ID, body) + } + if !strings.Contains(body, `model_provider = "nan"`) { + t.Errorf("%s: the profile would run against whatever provider is default:\n%s", m.ID, body) + } + } +} + +// Codex refuses a --profile with a dot in it ("pass a plain name such as +// `work`"), which is every id on the cluster that carries a version number. +func TestCodexProfileNamesAreNamesCodexAccepts(t *testing.T) { + for _, m := range catalog.ChatModels() { + name := codexProfileName(m.ID) + if strings.Contains(name, ".") { + t.Errorf("%s -> %s: Codex rejects this outright", m.ID, name) + } + if !strings.HasPrefix(name, "nan-") { + t.Errorf("%s -> %s: without the prefix it is not ours to remove again", m.ID, name) + } + } + if got := codexProfileName("glm5.3-flash"); got != "nan-glm53-flash" { + t.Errorf("codexProfileName(glm5.3-flash) = %s", got) + } +} + +// The provider section carries the key. A copy of it in eight more files is +// eight more files to rotate, and eight more to leak. +func TestCodexProfilesCarryNoKey(t *testing.T) { + path := tempConfig(t, "config.toml") + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + home := filepath.Dir(path) + for _, m := range catalog.ChatModels() { + data, err := os.ReadFile(filepath.Join(home, codexProfileName(m.ID)+".config.toml")) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), testKey) { + t.Errorf("%s: the profile has the API key in it", m.ID) + } + } +} + +func TestCodexRemovalTakesTheProfilesAndNothingElse(t *testing.T) { + path := tempConfig(t, "config.toml") + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + home := filepath.Dir(path) + // A profile of the member's own, sitting in the same directory. + theirs := filepath.Join(home, "work.config.toml") + if err := os.WriteFile(theirs, []byte("model = \"gpt-5\"\n"), 0o600); err != nil { + t.Fatal(err) + } + + if err := removeCodexConfig(path); err != nil { + t.Fatal(err) + } + for _, m := range catalog.ChatModels() { + profile := filepath.Join(home, codexProfileName(m.ID)+".config.toml") + if _, err := os.Stat(profile); !os.IsNotExist(err) { + t.Errorf("%s: the profile outlived the disconnect", m.ID) + } + } + if _, err := os.Stat(theirs); err != nil { + t.Errorf("a profile the member wrote was deleted: %v", err) + } +} + func TestFactoryConfigMarksWhatCannotSeeImages(t *testing.T) { path := tempConfig(t, "settings.json") if err := writeFactoryConfig(path, testKey); err != nil { diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 462a5c5..2a9b41b 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -9,6 +9,7 @@ import ( "path/filepath" "runtime" "sort" + "strconv" "strings" "time" @@ -2424,6 +2425,8 @@ func removePiDefaults(settingsPath string) error { func writeCodexConfig(cfgPath, apiKey string) error { data, _ := os.ReadFile(cfgPath) + codexModel, _ := catalog.Get(catalog.Coding) + if strings.Contains(string(data), "api.nan.builders") { // Already pointing at the cluster, so there is nothing to add - but // the section may be one we wrote back when it said wire_api = @@ -2431,14 +2434,27 @@ func writeCodexConfig(cfgPath, apiKey string) error { // all: it prints the error and exits before the TUI is up. A member // whose Codex does not open is the last member who can be asked to // disconnect and reconnect it, so the repair happens here. - if repaired, changed := codexWireAPIRepaired(data); changed { - return writeConfigFile(cfgPath, repaired) + // + // The same member is also the one most likely to be carrying a + // model_context_window we appended once per run, back when it went to + // the end of the file. Two of them is a duplicate key, and Codex + // refuses the whole file for it - the desktop app too, which opens on + // an error dialog and nothing else. Repairing the wire_api alone left + // that member exactly as stuck as before, so the pruning runs first + // and puts the key back where it is read from. + repaired, pruned := codexContextWindowsPruned(data, codexModel.Context) + if pruned { + repaired = []byte(withCodexContextWindow(string(repaired), codexModel.Context)) + } + rewired, changed := codexWireAPIRepaired(repaired) + if pruned || changed { + if err := writeConfigFile(cfgPath, rewired); err != nil { + return err + } } - return nil + return writeCodexProfiles(filepath.Dir(cfgPath)) } - codexModel, _ := catalog.Get(catalog.Coding) - // If no existing config, write a complete starter config. if len(data) == 0 { content := fmt.Sprintf(`model = %q @@ -2451,7 +2467,10 @@ base_url = "https://api.nan.builders/v1" experimental_bearer_token = %q wire_api = "responses" `, codexModel.ID, codexModel.Context, apiKey) - return writeConfigFile(cfgPath, []byte(content)) + if err := writeConfigFile(cfgPath, []byte(content)); err != nil { + return err + } + return writeCodexProfiles(filepath.Dir(cfgPath)) } // Existing config: only append the provider section; preserve user's model/provider choices. @@ -2463,7 +2482,64 @@ experimental_bearer_token = %q wire_api = "responses" `, apiKey) content := strings.TrimRight(string(data), "\n") + "\n" + section - return writeConfigFile(cfgPath, []byte(withCodexContextWindow(content, codexModel.Context))) + if err := writeConfigFile(cfgPath, []byte(withCodexContextWindow(content, codexModel.Context))); err != nil { + return err + } + return writeCodexProfiles(filepath.Dir(cfgPath)) +} + +// One file per chat model, so every model on the cluster is a flag away with +// the window it is actually served at. +// +// `codex --model ` switches the model and nothing else, and the +// model_context_window in config.toml stays where it was: point it at a +// 262,144-token model with a million-token window declared and Codex compacts +// against a window that is not there. Codex 0.155 layers +// $CODEX_HOME/.config.toml over the base config for `--profile `, +// which is the one place a per-model window can live without a second copy of +// the member's provider, MCP servers and permissions. +// +// No API key goes in them: the provider section in config.toml is what +// carries it, and a key in eight files is a key to rotate in eight files. +func writeCodexProfiles(codexHome string) error { + for _, m := range catalog.ChatModels() { + body := fmt.Sprintf(`# Written by the NaN CLI. Disconnecting Codex removes it. +model = %q +model_provider = "nan" +model_context_window = %d +`, m.ID, m.Context) + path := filepath.Join(codexHome, codexProfileName(m.ID)+".config.toml") + if err := writeConfigFile(path, []byte(body)); err != nil { + return err + } + } + return nil +} + +// Codex takes "a plain name" for --profile and rejects a dot in it outright, +// so the ids that carry a version number cannot be used as they are spelled: +// glm5.3-flash is refused, glm53-flash is not. The nan- prefix is what makes +// the file ours to delete later, and what keeps it clear of a profile the +// member wrote themselves. +func codexProfileName(id string) string { + return "nan-" + strings.ReplaceAll(id, ".", "") +} + +// Takes back only the files this CLI wrote, by the prefix it wrote them under. +func removeCodexProfiles(codexHome string) error { + entries, err := os.ReadDir(codexHome) + if err != nil { + return nil + } + for _, e := range entries { + name := e.Name() + if !e.IsDir() && strings.HasPrefix(name, "nan-") && strings.HasSuffix(name, ".config.toml") { + if err := os.Remove(filepath.Join(codexHome, name)); err != nil && !os.IsNotExist(err) { + return err + } + } + } + return nil } // Codex has no metadata for a model on this cluster, so without @@ -2498,6 +2574,70 @@ func withCodexContextWindow(content string, window int) string { return strings.TrimRight(content, "\n") + "\n" + declared + "\n" } +// Takes out every model_context_window this CLI is responsible for putting +// somewhere it does not belong, and leaves at most one standing. +// +// Two of them anywhere in the file is a duplicate key, and Codex does not +// load a config with one: `codex` exits on the error and the desktop app +// opens an error dialog instead of a window. That is how a member ends up +// unable to use Codex at all, for a key that only ever existed to stop it +// compacting against a guessed window. +// +// Inside a table the key is not a root key at all - it reads as a key of +// whatever table came last, usually the last [projects.*] the member +// trusted - so one carrying our own value there is ours, appended by a +// version that wrote it at the end of the file, and it goes. A value that is +// not ours is left where it is even there: it is not ours to judge. In the +// root table the first one stands and any repeat after it is the duplicate. +// +// Done line by line for the same reason the wire_api repair is: a round trip +// through a TOML library reflows a document the member also edits by hand. +func codexContextWindowsPruned(data []byte, window int) ([]byte, bool) { + ours := strconv.Itoa(window) + lines := strings.Split(string(data), "\n") + out := make([]string, 0, len(lines)) + inTable, keptRoot, pruned := false, false, false + for i := 0; i < len(lines); i++ { + line := lines[i] + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "[") { + inTable = true + out = append(out, line) + continue + } + key, value, ok := strings.Cut(trimmed, "=") + if !ok || strings.TrimSpace(key) != "model_context_window" { + out = append(out, line) + continue + } + drop := false + switch { + case inTable: + drop = strings.TrimSpace(value) == ours + case keptRoot: + drop = true + default: + keptRoot = true + } + if !drop { + out = append(out, line) + continue + } + pruned = true + // The line was written with a blank line on either side of it. Left + // alone both survive it and the file gains a gap where a key used to + // be, which is a diff the member did not ask for on a file they read. + if len(out) > 0 && strings.TrimSpace(out[len(out)-1]) == "" && + i+1 < len(lines) && strings.TrimSpace(lines[i+1]) == "" { + i++ + } + } + if !pruned { + return data, false + } + return []byte(strings.Join(out, "\n")), true +} + // Brings our own [model_providers.nan] section up to the only wire protocol // Codex still speaks, and touches nothing else in the file - not another // provider's wire_api, not the member's own keys, not a byte of the layout. @@ -2811,7 +2951,19 @@ func removeCodexConfig(cfgPath string) error { out = append(out, line) } result := strings.TrimRight(strings.Join(out, "\n"), "\n") + "\n" - if result == "\n" { + // The section is gone, and so is the reason for the model_context_window + // that went in beside it. Left behind it is a key the member never wrote, + // naming a window no model of theirs has - and if there are two of them + // it is a file their Codex does not open, now with nothing left in it to + // say who did that or which tool to disconnect to undo it. + codexModel, _ := catalog.Get(catalog.Coding) + if pruned, changed := codexContextWindowsPruned([]byte(result), codexModel.Context); changed { + result = string(pruned) + } + if err := removeCodexProfiles(filepath.Dir(cfgPath)); err != nil { + return err + } + if strings.TrimSpace(result) == "" { return os.Remove(cfgPath) } return writeConfigFile(cfgPath, []byte(result))