From d2cb4c444ca76f35ca43c447e300e2e5d4297d28 Mon Sep 17 00:00:00 2001 From: luckeyfaraday Date: Wed, 17 Jun 2026 14:57:59 +0200 Subject: [PATCH] Make provider setup append models instead of overwriting `codex-shim setup ` previously wrote a single-model payload, discarding any models already configured in the provider settings file. Re-running setup to add a model silently dropped the previous one, so only the most recently configured model appeared in the picker (and in-flight sessions on the dropped slug started 404ing once the shim reloaded the file). Setup now merges the entered model into the existing rows, keyed by model id: a new id is appended so every configured model keeps appearing, and an existing id updates that row in place (still idempotent). Existing rows are read raw so unknown/camelCase fields (extra_headers, no_image_support, etc.) survive the rewrite untouched. Co-Authored-By: Claude Opus 4.8 --- README.md | 10 +++-- codex_shim/cli.py | 62 ++++++++++++++++++++++++------- tests/test_provider_workflows.py | 64 ++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 965e9eb4..58e83fb3 100644 --- a/README.md +++ b/README.md @@ -291,9 +291,13 @@ MiniMax Token Plan uses `~/.codex-shim/minimax-models.json` and port `8767`. Real API keys belong only in those local files; committed-safe examples live in `examples/openrouter-models.example.json` and `examples/minimax-models.example.json`. -To change the stored model or key later, rerun `codex-shim setup `. -The prompt shows current values and lets Enter keep them; API keys are not -echoed, and Enter keeps the existing key when one is already present. +Rerun `codex-shim setup ` any time to add or update a model. Entering +a **new** model id appends it to the provider file, so every model you set up +keeps appearing in the picker instead of replacing the previous one. Entering a +model id that is already stored updates that entry in place (idempotent). The +prompt shows current values and lets Enter keep them; API keys are not echoed, +and Enter keeps the existing key when one is already present. To remove a model, +edit the provider JSON file directly. Useful overrides: diff --git a/codex_shim/cli.py b/codex_shim/cli.py index 9a4d6fb0..5890b4b2 100644 --- a/codex_shim/cli.py +++ b/codex_shim/cli.py @@ -351,25 +351,61 @@ def _write_provider_settings( spec.settings_path.parent.chmod(0o700) except OSError: pass + entry = { + "model": model, + "provider": spec.default_provider, + "base_url": base_url.rstrip("/"), + "api_key": api_key, + "display_name": display_name, + "max_context_limit": context, + } + models = _merge_model_row(_existing_model_rows(spec.settings_path), entry) old_umask = os.umask(0o077) try: - payload = { - "models": [ - { - "model": model, - "provider": spec.default_provider, - "base_url": base_url.rstrip("/"), - "api_key": api_key, - "display_name": display_name, - "max_context_limit": context, - } - ] - } - spec.settings_path.write_text(json.dumps(payload, indent=2) + "\n") + spec.settings_path.write_text(json.dumps({"models": models}, indent=2) + "\n") finally: os.umask(old_umask) +def _existing_model_rows(settings_path: Path) -> list[dict]: + """Return the raw model rows already stored in a provider settings file. + + Reads the JSON directly (rather than through ModelSettings) so unknown or + camelCase fields on existing entries survive a rewrite untouched. + """ + try: + data = json.loads(settings_path.read_text()) + except (OSError, json.JSONDecodeError): + return [] + if isinstance(data, dict): + rows = data.get("models") or data.get("customModels") or [] + elif isinstance(data, list): + rows = data + else: + rows = [] + return [row for row in rows if isinstance(row, dict)] + + +def _merge_model_row(existing: list[dict], entry: dict) -> list[dict]: + """Merge a setup entry into existing rows, keyed by model id. + + Re-running setup for a model id already present updates that row in place + (idempotent); a new model id is appended so previously configured models + keep appearing in the picker instead of being overwritten. + """ + merged: list[dict] = [] + replaced = False + for row in existing: + if str(row.get("model") or "").strip() == entry["model"]: + merged.append(entry) + replaced = True + else: + merged.append(row) + if not replaced: + merged.append(entry) + return merged + + def _first_model_slug(settings_path: Path) -> str | None: models = ModelSettings(settings_path).load() return models[0].slug if models else None diff --git a/tests/test_provider_workflows.py b/tests/test_provider_workflows.py index ab49ab93..0cee24a2 100644 --- a/tests/test_provider_workflows.py +++ b/tests/test_provider_workflows.py @@ -44,6 +44,70 @@ def test_provider_setup_writes_private_settings_file(tmp_path, monkeypatch): assert row["max_context_limit"] == 131000 +def _run_setup(monkeypatch, model, display, key="sk-test"): + monkeypatch.setattr(cli.sys.stdin, "isatty", lambda: True) + monkeypatch.setattr(cli.getpass, "getpass", lambda prompt: key) + answers = iter([model, display, "https://openrouter.ai/api/v1", "131000"]) + monkeypatch.setattr("builtins.input", lambda prompt: next(answers)) + assert cli.setup_provider("test-openrouter") == 0 + + +def test_provider_setup_appends_new_model(tmp_path, monkeypatch): + spec = _spec(tmp_path) + monkeypatch.setitem(cli.PROVIDER_SPECS, "test-openrouter", spec) + + _run_setup(monkeypatch, "z-ai/glm-5.2", "GLM-5.2") + _run_setup(monkeypatch, "xiaomi/mimo-v2.5", "MiMo-v2.5") + + rows = json.loads(spec.settings_path.read_text())["models"] + models = [row["model"] for row in rows] + assert models == ["z-ai/glm-5.2", "xiaomi/mimo-v2.5"] + + +def test_provider_setup_updates_existing_model_in_place(tmp_path, monkeypatch): + spec = _spec(tmp_path) + monkeypatch.setitem(cli.PROVIDER_SPECS, "test-openrouter", spec) + + _run_setup(monkeypatch, "z-ai/glm-5.2", "GLM-5.2", key="sk-old") + _run_setup(monkeypatch, "z-ai/glm-5.2", "GLM 5.2 Renamed", key="sk-new") + + rows = json.loads(spec.settings_path.read_text())["models"] + assert len(rows) == 1 + assert rows[0]["display_name"] == "GLM 5.2 Renamed" + assert rows[0]["api_key"] == "sk-new" + + +def test_provider_setup_preserves_unknown_fields_on_other_models(tmp_path, monkeypatch): + spec = _spec(tmp_path) + spec.settings_path.write_text( + json.dumps( + { + "models": [ + { + "model": "z-ai/glm-5.2", + "provider": "generic-chat-completion-api", + "base_url": "https://openrouter.ai/api/v1", + "api_key": "sk-keep", + "display_name": "GLM-5.2", + "extra_headers": {"X-Title": "codex"}, + "no_image_support": True, + } + ] + }, + indent=2, + ) + ) + monkeypatch.setitem(cli.PROVIDER_SPECS, "test-openrouter", spec) + + _run_setup(monkeypatch, "xiaomi/mimo-v2.5", "MiMo-v2.5") + + rows = json.loads(spec.settings_path.read_text())["models"] + glm = next(row for row in rows if row["model"] == "z-ai/glm-5.2") + assert glm["extra_headers"] == {"X-Title": "codex"} + assert glm["no_image_support"] is True + assert glm["api_key"] == "sk-keep" + + def test_provider_setup_non_interactive_missing_settings_fails(tmp_path, monkeypatch): spec = _spec(tmp_path)