mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-12 22:20:33 +00:00
* fix(skills): fail closed when the extensions config cannot be read * test(skills): cover a disabled skill when the extensions config cannot be read * style(tests): format the fail-closed regression for ruff * style(tests): fit the docstring and signature inside the 240-column limit * fix(skills): fail closed in the user-scoped listing when the config cannot be read * test(skills): cover the user-scoped listing with an unreadable extensions config * fix(gateway): keep skill read surfaces loud about an unreadable extensions config Folding the user-scoped degraded mode in (#7704) also stopped the skill read endpoints from failing on a config that cannot be read: the storage now answers with an all-disabled listing, which regressed the #7155 contract (a generic 500 with the host path only in the server log) into a 200 that presents a broken deployment as a legitimately empty one. Keep both properties: the storage still fails closed for the runtime gate, and the read surfaces re-validate the config explicitly so the raised error reaches each handler's generic 500 branch unchanged. First-run behavior is untouched: no config file resolves to an empty config without raising, so default-enabled still applies. * fix(skills): redact config-load failures and keep the read preflight off the event loop Review follow-ups: - Log the failure shape instead of the exception text in both degraded modes: from_file resolves $VAR placeholders before validation, so its message can embed resolved credentials. Reuse _describe_load_failure, the loader's own redaction helper. - Run the gateway read preflight through asyncio.to_thread in the capability adapter, matching the scan it precedes. - Cover the user-scoped degraded branch with a CUSTOM skill that has per-user state on disk, so the second read's guard is directly regression-tested (verified red with the guard removed). --------- Co-authored-by: Jay0130-a <309071492+Jay0130-a@users.noreply.github.com>
92 lines
4.1 KiB
Python
92 lines
4.1 KiB
Python
"""A broken extensions config must not take down a user-scoped skill listing.
|
|
|
|
``UserScopedSkillStorage._load_skills_unlocked`` re-reads the extensions config to AND the per-user state
|
|
with the global default. That second read is unguarded, so a config that cannot be read raised out of
|
|
``load_skills()`` for user-scoped deployments even after the local/global path learned to degrade.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from deerflow.config.extensions_config import ExtensionsConfig, reset_extensions_config
|
|
from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
|
|
|
|
_SKILL_MD = """---
|
|
name: {name}
|
|
description: {name} description
|
|
---
|
|
body
|
|
"""
|
|
|
|
|
|
def _raise_unreadable_config(*_args: object, **_kwargs: object) -> None:
|
|
# What one schema-invalid ``mcpServers`` entry raises from
|
|
# ``McpServerConfig._validate_header_names``; a malformed JSON file raises
|
|
# ``ValueError`` from the same call site.
|
|
raise ValueError("headers maps the same HTTP header under two spellings ('X-Api-Key' and 'x-api-key')")
|
|
|
|
|
|
@pytest.fixture()
|
|
def user_skills_root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
|
"""A skills root with one public skill and a readable extensions config."""
|
|
root = tmp_path / "skills"
|
|
public = root / "public" / "demo"
|
|
public.mkdir(parents=True)
|
|
(public / "SKILL.md").write_text(_SKILL_MD.format(name="demo"), encoding="utf-8")
|
|
config_path = tmp_path / "extensions_config.json"
|
|
config_path.write_text(json.dumps({"mcpServers": {}, "skills": {}}), encoding="utf-8")
|
|
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(config_path))
|
|
reset_extensions_config()
|
|
return root
|
|
|
|
|
|
def test_user_scoped_listing_degrades_when_the_config_cannot_be_read(user_skills_root: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
|
storage = UserScopedSkillStorage("owner", host_path=str(user_skills_root))
|
|
|
|
assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", True)]
|
|
|
|
monkeypatch.setattr(ExtensionsConfig, "from_file", classmethod(_raise_unreadable_config))
|
|
|
|
# The listing survives: no exception, every skill present and disabled.
|
|
assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", False)]
|
|
assert storage.load_skills(enabled_only=True) == []
|
|
|
|
|
|
def test_user_scoped_custom_skill_with_per_user_state_degrades_when_the_config_cannot_be_read(
|
|
user_skills_root: Path,
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""CUSTOM/LEGACY skills are the second read's own responsibility.
|
|
|
|
Their per-user state exists only to be ANDed with the global default, so
|
|
the degraded branch has to cover them too: with a per-user state present,
|
|
an unreadable config still lists the custom skill — disabled — instead of
|
|
raising out of ``load_skills()``. Dropping the guard turns this red: the
|
|
second read raises out of the listing.
|
|
"""
|
|
from deerflow.config.paths import Paths
|
|
|
|
# Route the user custom root (and the per-user states file) into tmp.
|
|
monkeypatch.setattr("deerflow.config.paths._paths", Paths(base_dir=tmp_path / "home"))
|
|
custom_dir = tmp_path / "home" / "users" / "owner" / "skills" / "custom" / "my-tool"
|
|
custom_dir.mkdir(parents=True)
|
|
(custom_dir / "SKILL.md").write_text(_SKILL_MD.format(name="my-tool"), encoding="utf-8")
|
|
states_file = tmp_path / "home" / "users" / "owner" / "skills" / "_skill_states.json"
|
|
states_file.write_text(json.dumps({"my-tool": {"enabled": True}}), encoding="utf-8")
|
|
|
|
storage = UserScopedSkillStorage("owner", host_path=str(user_skills_root))
|
|
|
|
assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", True), ("my-tool", True)]
|
|
|
|
monkeypatch.setattr(ExtensionsConfig, "from_file", classmethod(_raise_unreadable_config))
|
|
|
|
# The listing survives: every skill present (per-user state on disk for
|
|
# the custom one), none enabled, no exception.
|
|
assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", False), ("my-tool", False)]
|
|
assert storage.load_skills(enabled_only=True) == []
|