From 35bb952defad13896e705c02300eff255e1cf089 Mon Sep 17 00:00:00 2001 From: carlotestor Date: Wed, 23 Sep 2026 18:00:35 +0000 Subject: [PATCH] fix: fingerprint model-provider plugin file contents, not just manifest version --- api/config.py | 23 +++++++++++-- .../architecture/models-cache-invalidation.md | 4 +-- ...profile_switch_models_cache_source_axes.py | 32 +++++++++++++++++++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/api/config.py b/api/config.py index 1650865d..bf55bcb0 100644 --- a/api/config.py +++ b/api/config.py @@ -7978,7 +7978,7 @@ def _plugin_manifest_fields(plugin_dir: Path) -> dict | None: def _models_cache_plugin_fingerprint(home: Path) -> list: - """Return ``[dir, name, version]`` for the profile's model-provider plugins. + """Return ``[dir, file stamps]`` for the profile's model-provider plugins. Mirrors providers' discovery: every dir under ``plugins/model-providers/`` plus flat ``plugins/`` entries whose manifest declares that kind. @@ -7998,11 +7998,28 @@ def _models_cache_plugin_fingerprint(home: Path) -> list: fields = _plugin_manifest_fields(child) if flat and (fields or {}).get("kind") != "model-provider": continue - fields = fields or {} - found.append([str(child.relative_to(plugins_root)), fields.get("name", ""), fields.get("version", "")]) + found.append([str(child.relative_to(plugins_root)), _plugin_tree_stamps(child)]) return found +def _plugin_tree_stamps(plugin_dir: Path) -> list: + # The loader execs __init__.py, which can import sibling modules or read data files, + # so stamp every file in the tree; bytecode caches are derived and skipped. + stamps = [] + for root, dirs, files in os.walk(plugin_dir): + dirs[:] = sorted(d for d in dirs if d != "__pycache__" and not d.startswith(".")) + for name in sorted(files): + if name.endswith((".pyc", ".pyo")): + continue + path = os.path.join(root, name) + try: + st = os.stat(path) + except OSError: + continue + stamps.append([os.path.relpath(path, plugin_dir), st.st_mtime_ns, st.st_size]) + return stamps + + def _models_cache_source_fingerprint() -> dict: """Return the current config/auth/catalog fingerprint for /api/models cache. diff --git a/docs/architecture/models-cache-invalidation.md b/docs/architecture/models-cache-invalidation.md index fab7a7d8..19eb48fc 100644 --- a/docs/architecture/models-cache-invalidation.md +++ b/docs/architecture/models-cache-invalidation.md @@ -32,7 +32,7 @@ the fingerprint captured at publish time. | `config_yaml` | stat identity: `mtime_ns` + size (`_models_cache_file_fingerprint`) | The file is rewritten only on deliberate user edits, and any edit can change the provider/model set, so the cheap conservative identity wins. | | `auth_json` | content hash with a volatile-key deny-list (`_auth_store_semantic_fingerprint`, `_AUTH_FINGERPRINT_VOLATILE_KEYS`) | The credential store is rewritten roughly every 14 minutes by credential-pool / OAuth refresh; none of those rotating fields feed `detected_providers` or the returned catalog, and stat identity made the 24h cache churn on every refresh (RCA `t_d127953d` / `t_16551f61`). | | `env` | sorted **names** of non-empty keys in the active profile's `.env` (`_models_cache_env_fingerprint`); values are never recorded | Env credentials decide `detected_providers`. Adding or removing a key changes the catalog; rotating a value does not. | -| `plugins` | `[dir, name, version]` for each model-provider plugin in the active profile home (`_models_cache_plugin_fingerprint`): every dir under `plugins/model-providers/`, plus flat `plugins/` entries whose `plugin.yaml` declares `kind: model-provider` | Plugin providers feed the catalog through `api/plugin_providers.py`. Installing, removing, or upgrading one must invalidate the catalog. The discovery rules match `providers._scan_home_layer`. | +| `plugins` | `[dir, [relpath, mtime_ns, size] per file]` for each model-provider plugin in the active profile home (`_models_cache_plugin_fingerprint`, `_plugin_tree_stamps`): every dir under `plugins/model-providers/`, plus flat `plugins/` entries whose `plugin.yaml` declares `kind: model-provider`; `__pycache__`/`.pyc` are skipped | Plugin providers feed the catalog through `api/plugin_providers.py`, and `fallback_models` comes from the plugin code the loader execs (`__init__.py` and anything it imports), so any file edit must invalidate the catalog even without a `version` bump. The discovery rules match `providers._scan_home_layer`. | | `catalog` | baked-in provider catalog sha256 (`_PROVIDER_MODELS` + `_PROVIDER_DISPLAY`) plus the Codex local catalog (`_codex_models_cache_fingerprint`, `_CODEX_CACHE_FINGERPRINT_VOLATILE_KEYS`) | A restart after a catalog change must not keep serving a persisted payload for up to 24h (#2443). Codex rewrites `~/.codex/models_cache.json` on its own timer, bumping `mtime_ns` and size while models, `etag`, and `client_version` stay identical, so the Codex axis hashes **content** with only the refresh timestamps (`fetched_at`, `updated_at`) removed (#7540, #7556). | ## Invariant: deny-lists are one-directional @@ -150,7 +150,7 @@ executed `/api/profile/switch` route passes `delete_disk=False`. of that mode. It saves a real snapshot, changes one source in the target profile, runs the switch, and asserts a fresh rebuild for each of these cases: adding or removing a `.env` key, installing a plugin (in either location), -removing a plugin, bumping a plugin's version, and deleting then recreating the +removing a plugin, bumping a plugin's version, editing plugin code without a version bump, and deleting then recreating the profile. Two tests pin the speed-up: with unchanged sources, and after rotating a `.env` value, the switch still reuses the snapshot, and no secret value is written to disk. diff --git a/tests/test_profile_switch_models_cache_source_axes.py b/tests/test_profile_switch_models_cache_source_axes.py index ffba7d5e..dd7c4b09 100644 --- a/tests/test_profile_switch_models_cache_source_axes.py +++ b/tests/test_profile_switch_models_cache_source_axes.py @@ -193,3 +193,35 @@ def test_recreate_same_name_profile_does_not_resurrect_old_catalog(two_profiles, (two_profiles.demo_home / "config.yaml").write_text("model:\n default: demo-model\n", encoding="utf-8") os.utime(two_profiles.demo_home / "config.yaml", (1_000_000, 1_000_000)) assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model" + + +@pytest.mark.parametrize("flat", [False, True], ids=["model-providers-dir", "flat-install-dir"]) +def test_switch_after_plugin_code_change_without_version_bump_rejects_snapshot(two_profiles, flat): + """fallback_models lives in the plugin's code, which the loader imports; the version may not move.""" + plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=flat) + init = plugin / "__init__.py" + init.write_text("FALLBACK_MODELS = ('acme-1',)\n", encoding="utf-8") + os.utime(init, (1_000_000, 1_000_000)) + _save_demo_snapshot(two_profiles) + init.write_text("FALLBACK_MODELS = ('acme-1', 'acme-2')\n", encoding="utf-8") + assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model" + + +def test_switch_after_plugin_submodule_change_rejects_snapshot(two_profiles): + plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=False) + (plugin / "__init__.py").write_text("from .models import FALLBACK_MODELS\n", encoding="utf-8") + sub = plugin / "models.py" + sub.write_text("FALLBACK_MODELS = ('acme-1',)\n", encoding="utf-8") + os.utime(sub, (1_000_000, 1_000_000)) + _save_demo_snapshot(two_profiles) + sub.write_text("FALLBACK_MODELS = ('acme-1', 'acme-2')\n", encoding="utf-8") + assert _switch_to_demo_and_fetch(two_profiles)["default_model"] == "fresh-model" + + +def test_plugin_bytecode_cache_does_not_churn_fingerprint(two_profiles): + plugin = _write_plugin(two_profiles.demo_home, "acme", "1.0.0", flat=False) + (plugin / "__init__.py").write_text("X = 1\n", encoding="utf-8") + before = two_profiles.cfg._models_cache_plugin_fingerprint(two_profiles.demo_home) + (plugin / "__pycache__").mkdir() + (plugin / "__pycache__" / "__init__.cpython-311.pyc").write_bytes(b"x") + assert two_profiles.cfg._models_cache_plugin_fingerprint(two_profiles.demo_home) == before