From c69e9ef0bc8b7f3d6e6ff6de9b20f59f7ae96887 Mon Sep 17 00:00:00 2001 From: baolong lin <851617213@qq.com> Date: Sun, 8 Nov 2026 10:25:19 +0800 Subject: [PATCH] fix(memory): make doctor validates the selected backend config (#7965) A stale memory.backend_config (DeerMem-only keys left in place after switching manager_class to openviking) surfaced only when MemoryMiddleware constructed the manager mid-conversation, after model tokens were already spent; make doctor reported Ready because MemoryConfig accepts backend_config as untyped data. doctor now runs the backend's own side-effect-free validator (OpenVikingConfig.from_backend_config) and reports the offending keys before an agent turn is accepted. Backends without a static validator are skipped, as is disabled memory. Fixes #7962 Signed-off-by: Batianxieshen1 <280706568+Batianxieshen1@users.noreply.github.com> Co-authored-by: Batianxieshen1 <280706568+Batianxieshen1@users.noreply.github.com> Co-authored-by: Totoro Co-authored-by: Willem Jiang --- backend/tests/test_doctor_script.py | 106 ++++++++++++++++++++++++++++ scripts/doctor.py | 85 ++++++++++++++++++++++ 2 files changed, 191 insertions(+) diff --git a/backend/tests/test_doctor_script.py b/backend/tests/test_doctor_script.py index ba0c6b38c3..6fa58b3fea 100644 --- a/backend/tests/test_doctor_script.py +++ b/backend/tests/test_doctor_script.py @@ -94,3 +94,109 @@ def test_no_arguments_ignores_inherited_runtime_configuration(doctor_checkout: P assert external_config.read_bytes() == config_bytes assert external_env.read_bytes() == env_bytes assert not external_home.exists() + + +class TestMemoryBackendConfigCheck: + """check_memory_backend_config validates the selected backend statically.""" + + @pytest.fixture + def doctor(self): + import importlib.util + + spec = importlib.util.spec_from_file_location("doctor_module", REPO_ROOT / "scripts" / "doctor.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + @staticmethod + def _config(manager_class: str, backend_config: dict | None = None, enabled: bool = True): + from types import SimpleNamespace + + return SimpleNamespace( + memory=SimpleNamespace( + enabled=enabled, + manager_class=manager_class, + backend_config=backend_config or {}, + ) + ) + + def test_stale_deermem_field_fails_and_names_the_key(self, doctor): + result = doctor._check_memory_backend_config(self._config("openviking", {"debounce_seconds": 30})) + + assert result.status == "fail" + assert "debounce_seconds" in result.detail + assert result.fix + + def test_valid_config_passes(self, doctor, monkeypatch): + monkeypatch.setenv("OPENVIKING_API_KEY", "test-key") + config = self._config("openviking", {"base_url": "http://127.0.0.1:1933", "owner_user_id": "user-1"}) + + assert doctor._check_memory_backend_config(config).status == "ok" + + def test_other_backends_are_skipped(self, doctor): + result = doctor._check_memory_backend_config(self._config("deermem", {"debounce_seconds": 30})) + + assert result.status == "skip" + + def test_disabled_memory_is_skipped(self, doctor): + result = doctor._check_memory_backend_config(self._config("openviking", {"debounce_seconds": 30}, enabled=False)) + + assert result.status == "skip" + + def test_custom_openviking_subclass_path_is_skipped(self, doctor): + # A user-defined subclass owns its backend_config shape; the built-in + # validator must not reject it. + result = doctor._check_memory_backend_config(self._config("myapp.memory.CustomOpenVikingManager", {"anything": True})) + + assert result.status == "skip" + + def test_dotted_manager_path_is_validated(self, doctor): + result = doctor._check_memory_backend_config( + self._config( + "deerflow.agents.memory.backends.openviking.openviking_manager.OpenVikingMemoryManager", + {"debounce_seconds": 30}, + ) + ) + + assert result.status == "fail" + + def test_missing_api_key_advises_dotenv(self, doctor, monkeypatch): + monkeypatch.delenv("OPENVIKING_API_KEY", raising=False) + config = self._config("openviking", {"base_url": "http://127.0.0.1:1933", "owner_user_id": "user-1"}) + + result = doctor._check_memory_backend_config(config) + + assert result.status == "fail" + assert "USER API key is missing" in result.detail + assert result.fix == "Add OPENVIKING_API_KEY= to your .env file" + + +def test_memory_backend_config_failure_is_reported_end_to_end(doctor_checkout: Path): + (doctor_checkout / "config.yaml").write_text( + "config_version: 5\nmodels: []\nsandbox:\n use: deerflow.sandbox.local:LocalSandboxProvider\nmemory:\n enabled: true\n manager_class: openviking\n backend_config:\n debounce_seconds: 30\n", + encoding="utf-8", + ) + + # This check needs the harness importable in the subprocess, so unlike the + # disposable-checkout runs above, point PYTHONPATH at the repo's backend + # packages (the harness is path-installed, not pip-installed). + repo_packages = REPO_ROOT / "backend" / "packages" + separator = ";" if os.name == "nt" else ":" + env = {name: value for name, value in os.environ.items() if not name.startswith("DEER_FLOW_") and name not in {"PYTHONHOME"}} + env["PYTHONPATH"] = separator.join([str(repo_packages / "harness"), str(repo_packages / "extension-api"), str(REPO_ROOT / "backend")]) + result = subprocess.run( + [sys.executable, str(doctor_checkout / "scripts" / "doctor.py")], + cwd=doctor_checkout, + env=env, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=False, + timeout=60, + ) + + assert result.returncode == 1, result.stderr + assert "memory backend config (openviking)" in result.stdout + assert "Unknown OpenViking backend_config fields: debounce_seconds" in result.stdout + assert "Remove the fields named in the error" in result.stdout diff --git a/scripts/doctor.py b/scripts/doctor.py index 95e2001ad1..a74039ce73 100644 --- a/scripts/doctor.py +++ b/scripts/doctor.py @@ -413,6 +413,90 @@ def check_config_loadable(config_path: Path) -> CheckResult: ) +def check_memory_backend_config(config_path: Path) -> CheckResult: + """Statically validate the selected memory backend's backend_config. + + A stale backend_config (DeerMem-only keys left in place after switching + manager_class to openviking) used to surface only when MemoryMiddleware + constructed the manager mid-conversation, after model tokens had already + been spent. from_backend_config is side-effect-free, so the same errors + can be reported here before an agent turn is accepted. + """ + if not config_path.exists(): + return CheckResult("memory backend config", "skip") + + try: + config = _load_app_config(config_path) + except Exception: + # check_config_loadable already reports load failures. + return CheckResult("memory backend config", "skip") + + return _check_memory_backend_config(config) + + +def _check_memory_backend_config(config: object) -> CheckResult: + memory = getattr(config, "memory", None) + manager_class = str(getattr(memory, "manager_class", "") or "").strip() + label = f"memory backend config ({manager_class or 'unspecified'})" + if memory is None or not getattr(memory, "enabled", True): + return CheckResult(label, "skip", "memory disabled") + + backend_config = getattr(memory, "backend_config", None) or {} + if not _resolves_to_builtin_openviking(manager_class): + return CheckResult(label, "skip", "no static validator for this backend") + + try: + from deerflow.agents.memory.backends.openviking.config import OpenVikingConfig + except Exception as exc: + # Importing the backend package executes openviking/__init__.py, which + # pulls pydantic and deerflow internals; a broken backend environment + # degrades to a skip instead of crashing the report (same policy as + # resolve_config_path). + return CheckResult(label, "skip", str(exc)) + + try: + OpenVikingConfig.from_backend_config(dict(backend_config)) + except Exception as exc: + detail = str(exc).strip().splitlines()[0] if str(exc).strip() else repr(exc) + return CheckResult(label, "fail", detail, fix=_memory_backend_config_fix(detail)) + return CheckResult(label, "ok") + + +def _resolves_to_builtin_openviking(manager_class: str) -> bool: + """True when manager_class resolves to the built-in OpenViking manager. + + Only the built-in backend's config contract is known here: a custom + ``OpenVikingMemoryManager`` subclass pointed at by a dotted path owns its + backend_config shape, so doctor must not reject it with the built-in + rules. Resolution failures are the factory's to report (it fails loudly + in get_memory_manager), not this check's. + """ + try: + from deerflow.agents.memory.backends.openviking.openviking_manager import OpenVikingMemoryManager + from deerflow.agents.memory.manager import _resolve_manager_class + except Exception: + return False + try: + return _resolve_manager_class(manager_class) is OpenVikingMemoryManager + except Exception: + return False + + +def _memory_backend_config_fix(detail: str) -> str | None: + """Remediation matched to the validator's failure modes; None lets the + verbatim error speak for itself.""" + if "USER API key is missing; set" in detail: + var = detail.rsplit("set ", 1)[-1].strip().rstrip(".") + return f"Add {var}= to your .env file" + if "Unknown OpenViking backend_config fields" in detail: + return "Remove the fields named in the error from memory.backend_config, or switch manager_class back" + if "trusted mode is no longer supported" in detail: + return "Use a USER API key (api_key_env) and configure owner_user_id instead of auth_mode/account" + if "custom HTTP client fields are no longer supported" in detail: + return "Remove the custom HTTP client fields from memory.backend_config" + return None + + def check_llm_api_key(config_path: Path) -> list[CheckResult]: """Check that each model's env var is set in the environment.""" if not config_path.exists(): @@ -923,6 +1007,7 @@ def main() -> int: config_failure or check_config_exists(config_path), check_config_version(config_path, project_root), check_config_loadable(config_path), + check_memory_backend_config(config_path), check_models_configured(config_path), ] sections.append(("Configuration", cfg_checks))