fix(agents): distinguish undeclared and empty skill allowed-tools (#5669)

* fix(agents): distinguish undeclared and empty skill allowed-tools

* fix(ci): reduce guidance and isolate PostgreSQL test mocks

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

backend/packages/harness/deerflow/agents/AGENTS.md

@@ -1,18 +1,17 @@
 ### Agent System
 
 **Lead Agent** (`packages/harness/deerflow/agents/lead_agent/agent.py`):
-- Entry point: `make_lead_agent(config: RunnableConfig)` registered in `langgraph.json`.
-  Its signature and bare-graph return type are a published ABI: LangGraph Server calls it
-  directly, so neither may change.
-- `assemble_lead_agent(config, *, app_config=None) -> LeadAgentAssembly(graph, descriptor)`
-  is the richer entry point the Gateway uses; `make_lead_agent` is a thin wrapper returning
-  `.graph`. The descriptor is built by
-  `deerflow/agents/assembly_descriptor.py::build_assembly_descriptor()` and captures what
-  only the factory knows — the model resolved after runtime overrides, the rendered prompt
-  hash, the tool list left by authorization, and the composed middleware stack in order.
-  Consumers of a factory result must unwrap `.graph` defensively (see
-  `runtime/runs/worker.py::_agent_graph`), because a third-party factory still returns a
-  bare graph.
+- `make_lead_agent(config: RunnableConfig)` is the published `langgraph.json`
+  entry point; preserve its signature and bare-graph return type.
+- Gateway calls `assemble_lead_agent(config, *, app_config=None)` for a
+  `LeadAgentAssembly(graph, descriptor)`; `make_lead_agent` returns `.graph`.
+  `assembly_descriptor.py::build_assembly_descriptor()` records the model after
+  runtime overrides, rendered prompt hash, authorized tools and middleware order.
+  Consumers must unwrap via `runtime/runs/worker.py::_agent_graph` to also accept
+  third-party factories returning bare graphs.
+  The hashed skill catalog preserves `allowed_tools=None` (legacy allow-all),
+  `()` (explicit allow-none for business tools), and non-empty allowlists as
+  distinct policy states; allowlist ordering does not affect the fingerprint.
 - Dynamic model selection via `create_chat_model()` with thinking/vision support
 - Tools loaded via `get_available_tools()` - combines sandbox, built-in, MCP, community, and subagent tools
 - System prompt generated by `apply_prompt_template()` with skills, memory, and subagent instructions

backend/packages/harness/deerflow/agents/assembly_descriptor.py

@@ -467,7 +467,8 @@ def build_assembly_descriptor(
         {
             "name": str(getattr(skill, "name", "")),
             "description": str(getattr(skill, "description", "")),
-            "allowed_tools": sorted(str(item) for item in (getattr(skill, "allowed_tools", None) or ())),
+            # None preserves legacy allow-all; an empty declaration allows no business tools.
+            "allowed_tools": None if (allowed_tools := getattr(skill, "allowed_tools", None)) is None else sorted(str(item) for item in allowed_tools),
             "content_hash": _skill_content_hash(skill),
             "secrets_autonomous": bool(getattr(skill, "secrets_autonomous", True)),
             "required_secrets": sorted(f"{getattr(requirement, 'name', '')}:{bool(getattr(requirement, 'optional', False))}" for requirement in (getattr(skill, "required_secrets", None) or ())),

backend/tests/test_agent_assembly_descriptor.py

@@ -662,7 +662,7 @@ class TestSkillCatalogHashesContent:
     allowed-tools are untouched."""
 
     @staticmethod
-    def _skill(skill_dir: Path, *, required_secrets=(), secrets_autonomous=True):
+    def _skill(skill_dir: Path, *, allowed_tools=None, required_secrets=(), secrets_autonomous=True):
         from deerflow.skills.types import Skill, SkillCategory
 
         skill_file = skill_dir / "SKILL.md"
@@ -674,6 +674,7 @@ class TestSkillCatalogHashesContent:
             skill_file=skill_file,
             relative_path=Path(skill_dir.name),
             category=SkillCategory.CUSTOM,
+            allowed_tools=allowed_tools,
             required_secrets=required_secrets,
             secrets_autonomous=secrets_autonomous,
         )
@@ -719,6 +720,17 @@ class TestSkillCatalogHashesContent:
         with_secret = self._skill(Path("/nonexistent/skill-b"), required_secrets=(SecretRequirement(name="API_KEY"),))
         assert self._build([no_secrets]).fingerprint != self._build([with_secret]).fingerprint
 
+    def test_allowed_tools_declaration_states_have_distinct_fingerprints(self, tmp_path):
+        descriptors = [self._build([self._skill(tmp_path, allowed_tools=allowed_tools)]) for allowed_tools in (None, (), ("bash",))]
+
+        assert len({descriptor.fingerprint for descriptor in descriptors}) == 3
+
+    def test_allowed_tools_order_does_not_change_the_fingerprint(self, tmp_path):
+        before = self._build([self._skill(tmp_path, allowed_tools=("bash", "read_file"))])
+        reordered = self._build([self._skill(tmp_path, allowed_tools=("read_file", "bash"))])
+
+        assert before.fingerprint == reordered.fingerprint
+
     def test_a_missing_skill_file_is_undescribable_not_fatal(self, tmp_path):
         skill = self._skill(tmp_path / "missing-skill")
         descriptor = self._build([skill])

backend/tests/test_persistence_engine_postgres_config.py

@@ -55,7 +55,7 @@ def test_postgres_engine_kwargs_allow_command_timeout_opt_out() -> None:
 
 
 @pytest.mark.asyncio
-async def test_configured_command_timeout_ends_stalled_command() -> None:
+async def test_configured_command_timeout_ends_stalled_command(monkeypatch) -> None:
     config = DatabaseConfig(
         backend="postgres",
         postgres_url="postgresql://user:password@localhost/deerflow",
@@ -79,8 +79,9 @@ async def test_configured_command_timeout_ends_stalled_command() -> None:
 
     bootstrap_schema = AsyncMock()
 
+    # Patch one key: patch.dict restores all of sys.modules and races with background imports.
+    monkeypatch.setitem(sys.modules, "asyncpg", ModuleType("asyncpg"))
     with (
-        patch.dict(sys.modules, {"asyncpg": ModuleType("asyncpg")}),
         patch.object(engine_mod, "create_async_engine", side_effect=_create_engine),
         patch.object(engine_mod, "async_sessionmaker", return_value=MagicMock()),
         patch("deerflow.persistence.bootstrap.bootstrap_schema", new=bootstrap_schema),
@@ -101,7 +102,7 @@ async def test_configured_command_timeout_ends_stalled_command() -> None:
 
 
 @pytest.mark.asyncio
-async def test_init_engine_from_config_preserves_longer_command_timeout_override() -> None:
+async def test_init_engine_from_config_preserves_longer_command_timeout_override(monkeypatch) -> None:
     config = DatabaseConfig(
         backend="postgres",
         postgres_url="postgresql://user:password@localhost/deerflow",
@@ -112,8 +113,8 @@ async def test_init_engine_from_config_preserves_longer_command_timeout_override
     mock_engine.dispose = AsyncMock()
     bootstrap_schema = AsyncMock()
 
+    monkeypatch.setitem(sys.modules, "asyncpg", ModuleType("asyncpg"))
     with (
-        patch.dict(sys.modules, {"asyncpg": ModuleType("asyncpg")}),
         patch.object(engine_mod, "create_async_engine", return_value=mock_engine) as create_engine,
         patch.object(engine_mod, "async_sessionmaker", return_value=MagicMock()),
         patch("deerflow.persistence.bootstrap.bootstrap_schema", new=bootstrap_schema),
@@ -129,14 +130,14 @@ async def test_init_engine_from_config_preserves_longer_command_timeout_override
 
 
 @pytest.mark.asyncio
-async def test_init_engine_postgres_uses_hardened_kwargs() -> None:
+async def test_init_engine_postgres_uses_hardened_kwargs(monkeypatch) -> None:
     url = "postgresql+asyncpg://user:password@localhost/deerflow"
     mock_engine = MagicMock()
     mock_engine.dispose = AsyncMock()
     bootstrap_schema = AsyncMock()
 
+    monkeypatch.setitem(sys.modules, "asyncpg", ModuleType("asyncpg"))
     with (
-        patch.dict(sys.modules, {"asyncpg": ModuleType("asyncpg")}),
         patch.object(engine_mod, "create_async_engine", return_value=mock_engine) as create_engine,
         patch.object(engine_mod, "async_sessionmaker", return_value=MagicMock()),
         patch("deerflow.persistence.bootstrap.bootstrap_schema", new=bootstrap_schema),
@@ -151,7 +152,7 @@ async def test_init_engine_postgres_uses_hardened_kwargs() -> None:
 
 
 @pytest.mark.asyncio
-async def test_init_engine_postgres_retry_uses_hardened_kwargs() -> None:
+async def test_init_engine_postgres_retry_uses_hardened_kwargs(monkeypatch) -> None:
     url = "postgresql+asyncpg://user:password@localhost/deerflow"
     initial_engine = MagicMock()
     initial_engine.dispose = AsyncMock()
@@ -160,8 +161,8 @@ async def test_init_engine_postgres_retry_uses_hardened_kwargs() -> None:
     bootstrap_schema = AsyncMock(side_effect=[Exception("database does not exist"), None])
     auto_create = AsyncMock()
 
+    monkeypatch.setitem(sys.modules, "asyncpg", ModuleType("asyncpg"))
     with (
-        patch.dict(sys.modules, {"asyncpg": ModuleType("asyncpg")}),
         patch.object(engine_mod, "create_async_engine", side_effect=[initial_engine, retry_engine]) as create_engine,
         patch.object(engine_mod, "async_sessionmaker", return_value=MagicMock()),
         patch.object(engine_mod, "_auto_create_postgres_db", new=auto_create),