From d073add71cf28254a8c6fbda4d907fc5ff5695e7 Mon Sep 17 00:00:00 2001 From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:28:48 -0700 Subject: [PATCH] fix(config): bind extensions cache to parsed revision (#6420) --- README.md | 2 + .../harness/deerflow/config/AGENTS.md | 7 +- .../deerflow/config/extensions_config.py | 35 ++++---- .../test_extensions_config_signed_read.py | 79 +++++++++++++++++++ 4 files changed, 103 insertions(+), 20 deletions(-) create mode 100644 backend/tests/test_extensions_config_signed_read.py diff --git a/README.md b/README.md index a92f975468..25447778e7 100644 --- a/README.md +++ b/README.md @@ -758,6 +758,8 @@ Capability Center > Plugins adds, replaces, and deletes one MCP server at a time Targeted updates accept both DeerFlow's `type` field and the MCP-spec `transport` field for SSE/HTTP servers. Runtime MCP and skill updates replace `extensions_config.json` atomically, so an interrupted write cannot leave the shared configuration truncated or partially written. Every Gateway worker or instance that reads the same file picks up a new revision on its next read (the cache checks the file's content signature), so MCP and skill changes made through one replica apply to the others without a restart; a missing, truncated, or invalid revision keeps the previous configuration until a complete one lands, including when the file disappears during a reload. The admin MCP cache reset advances a durable generation marker in the writable config directory. Every Gateway worker mounting that same directory retires its own cached tools and pooled sessions before the next lookup; replicas with independent filesystems are not implicitly covered. If no config path is available, the API reports a process-local reset instead. +The parsed extensions configuration and its recorded content digest come from the same read, so a racing edit followed by a timestamp-preserving backup restore cannot leave a different revision cached indefinitely. + `extensions_config.json` accepts UTF-8 with or without a leading byte-order mark (BOM), including files saved as UTF-8 with BOM by an editor. MCP routing hints can also prefer a specific MCP tool for matching requests without forbidding other tools. When `tool_search` defers MCP schemas, matching routing metadata can auto-promote up to `tool_search.auto_promote_top_k` deferred schemas before the model call. diff --git a/backend/packages/harness/deerflow/config/AGENTS.md b/backend/packages/harness/deerflow/config/AGENTS.md index 79d8241095..247a1bccc3 100644 --- a/backend/packages/harness/deerflow/config/AGENTS.md +++ b/backend/packages/harness/deerflow/config/AGENTS.md @@ -108,9 +108,10 @@ Extensions are optional only in the fallback *search* mode (priority 3-4 above): **Extensions Config Caching**: `get_extensions_config()` caches the parsed file but revalidates it on every call against the resolved path and the `(mtime, size, sha256)` signature from `file_signature.get_config_signature`, so an edit made by another Gateway worker, or by another instance sharing the file (the Helm home volume, the compose bind mount), is visible to this process on its next read without a restart or an explicit reload. Writers (MCP router, skill toggle, `DeerFlowClient`) still call `reload_extensions_config()` after their write, which records the written revision so the next read does not reload it again. Once a configuration has been loaded, a revision that cannot be loaded — the file vanished, or it is truncated or invalid, for example midway through the non-atomic `EBUSY` overwrite fallback — keeps the last-known-good configuration and is logged once at warning level with the exception type only (validation messages can embed resolved `$VAR` secrets); the first load still raises, so a broken file at startup stays loud. `reload_extensions_config(config_path=...)` makes the cache follow that file (later edits included) until a reset or an argument-less reload. `set_extensions_config()` pins an injected instance until `reload_extensions_config()` or `reset_extensions_config()`; tests that inject one must reset it afterwards, because the `extensions` snapshot of `get_app_config()` follows the pinned instance too. The local-bash absolute path allowlist (`sandbox/tools.py::_get_mcp_allowed_paths`) is derived from this singleton, which is why the revalidation is a security property rather than an optimization. Pinned by `tests/test_extensions_config_freshness.py` and the extensions cases in `tests/test_app_config_reload.py`. -Freshness reloads parse the probed path explicitly. A search-mode file vanishing -between signature collection and parsing must retain the last-known-good cache, -including AppConfig's middleware snapshot, until a readable revision returns. +Extensions loads parse the bytes from `read_config_with_signature`, recording +that read's digest rather than an earlier probe's. Freshness reloads read the +probed path explicitly; a missing or invalid revision keeps the last-known-good +cache, including AppConfig's middleware snapshot, until a readable revision returns. ### Config Schema diff --git a/backend/packages/harness/deerflow/config/extensions_config.py b/backend/packages/harness/deerflow/config/extensions_config.py index 2f5584be8c..ddf3f9a001 100644 --- a/backend/packages/harness/deerflow/config/extensions_config.py +++ b/backend/packages/harness/deerflow/config/extensions_config.py @@ -17,7 +17,7 @@ from typing import Any, Literal from pydantic import BaseModel, ConfigDict, Field, ValidationInfo, field_validator, model_validator from deerflow.config._boolean_guards import reject_boolean -from deerflow.config.file_signature import ConfigSignature, get_config_signature +from deerflow.config.file_signature import ConfigSignature, get_config_signature, read_config_with_signature from deerflow.config.runtime_paths import existing_project_file from deerflow.constants import ( DEFAULT_MCP_SESSION_INIT_TIMEOUT, @@ -535,11 +535,16 @@ class ExtensionsConfig(BaseModel): # Return empty config if extensions config file is not found return cls(mcp_servers={}, skills={}) + return cls._from_path_with_signature(resolved_path)[0] + + @classmethod + def _from_path_with_signature(cls, resolved_path: Path) -> tuple["ExtensionsConfig", ConfigSignature]: + """Parse the exact bytes whose signature the singleton will record.""" try: - with open(resolved_path, encoding="utf-8-sig") as f: - config_data = json.load(f) + data, signature = read_config_with_signature(resolved_path) + config_data = json.loads(data.decode("utf-8-sig")) config_data = cls.resolve_env_variables(config_data) - return cls.model_validate(config_data) + return cls.model_validate(config_data), signature except json.JSONDecodeError as e: raise ValueError(f"Extensions config file at {resolved_path} is not valid JSON: {e}") from e except Exception as e: @@ -818,22 +823,18 @@ def _probe_extensions_config_state() -> tuple[Path | None, ConfigSignature | Non def _load_and_cache_extensions_config(config_path: str | None = None) -> ExtensionsConfig: """Load the config from disk and record the revision it came from. - The caller holds ``_extensions_config_lock``. The signature is probed - *before* parsing, so it describes the parsed revision or an older one, - never a newer one: a write that lands between the probe and the parse - then shows up as one extra reload on the next call. Probing afterwards - could record the newer revision's signature against the older content, a - stale state the comparison could never detect. + The caller holds ``_extensions_config_lock``. Parsing and the recorded + digest use one read, so a competing edit or backup restore cannot leave + one revision cached under another revision's signature. """ global _extensions_config, _extensions_config_path, _extensions_config_signature global _extensions_config_is_custom, _extensions_config_source, _extensions_config_rejected resolved_path = ExtensionsConfig.resolve_config_path(config_path) - signature = get_config_signature(resolved_path) if resolved_path is not None else None - # ``from_file`` keeps resolving the path itself (and is called without an - # argument when none was given) so callers that substitute it see the - # same call shape as before. - loaded = ExtensionsConfig.from_file(config_path) if config_path else ExtensionsConfig.from_file() + if resolved_path is None: + loaded, signature = ExtensionsConfig(), None + else: + loaded, signature = ExtensionsConfig._from_path_with_signature(resolved_path) _extensions_config = loaded _extensions_config_path = resolved_path _extensions_config_signature = signature @@ -906,7 +907,7 @@ def get_extensions_config() -> ExtensionsConfig: try: # Parse the probed file: a second search could publish an empty # config if that file disappeared before parsing. - loaded = ExtensionsConfig.from_file(str(current_path)) + loaded, loaded_signature = ExtensionsConfig._from_path_with_signature(current_path) except Exception as exc: _keep_last_known_good(current_path, current_signature, "Extensions config at %s changed but could not be loaded (%s)", current_path, _describe_load_failure(exc)) return _extensions_config @@ -914,7 +915,7 @@ def get_extensions_config() -> ExtensionsConfig: logger.info("Extensions config at %s changed on disk; reloaded", current_path) _extensions_config = loaded _extensions_config_path = current_path - _extensions_config_signature = current_signature + _extensions_config_signature = loaded_signature _extensions_config_rejected = None return loaded diff --git a/backend/tests/test_extensions_config_signed_read.py b/backend/tests/test_extensions_config_signed_read.py new file mode 100644 index 0000000000..c5dddce29d --- /dev/null +++ b/backend/tests/test_extensions_config_signed_read.py @@ -0,0 +1,79 @@ +"""Cache parsed JSON and its own signature, including timestamp-preserving restores.""" + +import json +import shutil +from pathlib import Path + +import pytest + +import deerflow.config.extensions_config as extensions +from deerflow.config.file_signature import read_config_with_signature + + +def payload(argument): + return {"mcpServers": {"example": {"type": "stdio", "command": "echo", "args": [argument]}}} + + +@pytest.mark.parametrize("load_mode", ["first-load", "explicit-reload", "automatic-reload"]) +@pytest.mark.parametrize("race_phase", ["before-parse", "before-read"]) +def test_cache_signature_identifies_the_parsed_revision(tmp_path: Path, monkeypatch, load_mode, race_phase): + path = tmp_path / "extensions.json" + backup = tmp_path / "backup.json" + monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(path)) + extensions.reset_extensions_config() + try: + if load_mode == "automatic-reload": + extensions.atomic_write_extensions_config(path, payload("initial")) + extensions.get_extensions_config() + extensions.atomic_write_extensions_config(path, payload("alpha")) + shutil.copy2(path, backup) + original_load, original_loads = json.load, json.loads + original_signature = extensions.get_config_signature + edited = False + + def peer_edit(): + nonlocal edited + if not edited: + edited = True + # Real I/O at the parser boundary: legacy json.load reads the + # replacement; a parser handed already-read bytes retains alpha. + path.write_text(json.dumps(payload("beta")), encoding="utf-8") + + def load_after_peer_edit(*args, **kwargs): + peer_edit() + return original_load(*args, **kwargs) + + def loads_after_peer_edit(*args, **kwargs): + peer_edit() + return original_loads(*args, **kwargs) + + def signature_before_peer_edit(selected_path): + signature = original_signature(selected_path) + peer_edit() + return signature + + def signed_read_after_peer_edit(selected_path): + peer_edit() + return read_config_with_signature(selected_path) + + with monkeypatch.context() as race: + if race_phase == "before-parse": + race.setattr(json, "load", load_after_peer_edit) + race.setattr(json, "loads", loads_after_peer_edit) + else: + race.setattr(extensions, "get_config_signature", signature_before_peer_edit) + race.setattr(extensions, "read_config_with_signature", signed_read_after_peer_edit, raising=False) + if load_mode == "explicit-reload": + extensions.reload_extensions_config() + else: + extensions.get_extensions_config() + assert edited and json.loads(path.read_text(encoding="utf-8"))["mcpServers"]["example"]["args"] == ["beta"] + + # Restore both bytes and metadata, as cp -p/backup restoration can do. + shutil.copy2(backup, path) + assert path.read_bytes() == backup.read_bytes() + assert path.stat().st_mtime_ns == backup.stat().st_mtime_ns + for _ in range(3): + assert extensions.get_extensions_config().mcp_servers["example"].args == ["alpha"] + finally: + extensions.reset_extensions_config()