[eric] template: declare pydantic, httpx and uvicorn instead of riding fastapi's extras, with a general undeclared-import check (ENG-287)

2026-09-07 18:27:45 +02:00 · 2026-08-13 02:09:36 -07:00
parent fc09059595
commit b2eef18069
5 changed files with 131 additions and 24 deletions
@@ -22,6 +22,7 @@ from backend.apps.agents.manager.streaming import post_tool_hook as post_tool_ho
 from backend.apps.agents.manager.streaming import stop_hook as stop_hook_mod
 from backend.apps.agents.manager.streaming.HookContext import HookContext
 from backend.apps.agents.manager.permissions.build_effective_tool_lists import build_effective_tool_lists
+from backend.apps.agents.manager.run.empty_finish import apply_toolless_continuation
 from backend.apps.agents.manager.register_builtin_mcp_servers import register_builtin_mcp_servers
 from backend.apps.agents.manager.configure_provider_env import configure_provider_env
 from backend.apps.agents.manager.session.workspace_git import ensure_cwd_git_repo
@@ -153,20 +154,11 @@ class RunOptions(AgentManagerProtocol):
            browser_delegation_tools, invoke_agent_tools,
        )

-        # A toolless continuation is the whole point of the final nudge: with nothing allowed, the
-        # only move left is the text the user is missing. Cleared by the loop once the turn starts.
-        if getattr(session, "pending_continuation_toolless", False):
-            effective_allowed = []
-            mcp_servers = {}
-
+        effective_allowed, mcp_servers = apply_toolless_continuation(session, effective_allowed, mcp_servers)
        composed_prompt = append_web_tools_hint(composed_prompt, need_web_mcp, effective_allowed)

-        # Log effective tool lists
-        google_allowed = [t for t in effective_allowed if "google-workspace" in t]
-        reddit_allowed = [t for t in effective_allowed if "reddit" in t]
-        builtin_allowed = [t for t in effective_allowed if not t.startswith("mcp__")]
-        logger.info(f"[MCP-DEBUG] effective_allowed: {len(effective_allowed)} total "
-                    f"(builtins={len(builtin_allowed)}, google={len(google_allowed)}, reddit={len(reddit_allowed)})")
+        p_builtins = sum(1 for t in effective_allowed if not t.startswith("mcp__"))
+        logger.info(f"[MCP-DEBUG] effective_allowed: {len(effective_allowed)} total, builtins={p_builtins}")
        if effective_disallowed:
            logger.info(f"[MCP-DEBUG] effective_disallowed: {effective_disallowed}")

@@ -141,3 +141,15 @@ def turn_finished_empty(session: AgentSession) -> bool:
        if role in ("user", "system"):
            return False
    return False
+
+
+@typechecked
+def apply_toolless_continuation(session: AgentSession, allowed: List[str], mcp_servers: dict) -> tuple:
+    """Strip every tool for the final nudge's turn, so "do not call any more tools" is a fact.
+
+    Lives here rather than in RunOptions because this module already decides WHEN a turn is the
+    final nudge; splitting the decision from its consequence is how the wording-only version
+    survived a release."""
+    if not getattr(session, "pending_continuation_toolless", False):
+        return allowed, mcp_servers
+    return [], {}
@@ -5,6 +5,11 @@ description = "OpenSwarm web app backend — FastAPI + SubApp plugin pattern"
 requires-python = ">=3.10"
 dependencies = [
    "fastapi[standard]",
+    # Declared even though fastapi already requires it: an agent writing a SubApp reaches straight
+    # for BaseModel, and a dependency you rely on should be one you asked for.
+    "pydantic>=2.9.0",
+    "httpx",
+    "uvicorn",
    "typeguard==4.4.2",
    "swarm-debug",
 ]
@@ -57,20 +57,33 @@ def test_the_final_nudge_is_marked_toolless(monkeypatch: Any) -> None:
    )


-def test_the_options_builder_actually_empties_the_list() -> None:
-    """The flag is worthless unless the turn's tool list is really emptied. This asserts the wiring
-    exists at the one place that decides it, so the seal cannot be a field nobody reads."""
+def test_the_toolless_turn_really_gets_no_tools() -> None:
+    """Behaviour, not a source grep: the flag must actually empty what the turn is handed.
+
+    An earlier version of this asserted the string "effective_allowed = []" appeared in
+    RunOptions, which broke the moment the logic moved and proved nothing about the result.
+    """
+    allowed = ["Bash", "Read", "Write", "WebSearch"]
+    servers = {"openswarm-core": {"env": {}}}
+
+    s_off = p_session()
+    assert empty_finish.apply_toolless_continuation(s_off, allowed, servers) == (allowed, servers), (
+        "an ordinary turn must keep every tool it was given"
+    )
+
+    s_on = p_session()
+    s_on.pending_continuation_toolless = True
+    got_allowed, got_servers = empty_finish.apply_toolless_continuation(s_on, allowed, servers)
+    assert got_allowed == [], f"final turn still offered {len(got_allowed)} tool(s): {got_allowed}"
+    assert got_servers == {}, "final turn still had MCP servers attached, so tools remain reachable"
+
+
+def test_run_options_actually_calls_it() -> None:
+    """The helper is only a seal if the options path invokes it."""
    import inspect
    from backend.apps.agents.manager.run import RunOptions
-
-    src = inspect.getsource(RunOptions)
-    assert "pending_continuation_toolless" in src, (
-        "nothing in RunOptions reads the flag, so the final turn still ships a full tool list"
-    )
-    idx = src.index("pending_continuation_toolless")
-    window = src[idx: idx + 320]
-    assert "effective_allowed = []" in window, (
-        "the flag is read but the allowed-tool list is not emptied"
+    assert "apply_toolless_continuation(" in inspect.getsource(RunOptions), (
+        "RunOptions never calls the helper, so the final turn still ships a full tool list"
    )


@@ -0,0 +1,85 @@
+"""Every third-party module a generated app imports must be declared by the template (ENG-287).
+
+The filed premise was wrong and the measurement said so: nothing in the template imports
+pydantic, `app_builder_skill.md` never mentions it, and fastapi declares `pydantic>=2.9.0`
+as a core dependency, so there was no live breakage. What was true is narrower: the
+template relied on a package it never asked for.
+
+This test is the general form, not the pydantic special case. It reads what the template
+actually imports and asserts the manifest covers it, so the next helper an agent leans on
+cannot become an undeclared dependency quietly.
+
+Run:
+    backend/.venv/bin/python -m pytest backend/tests/test_template_declares_what_it_imports.py -v
+"""
+
+import ast
+import os
+import re
+from typing import List, Set
+
+TEMPLATE = os.path.join("backend", "apps", "outputs", "webapp_template", "backend")
+# Shipped with the app, not from PyPI, so they are not manifest entries.
+P_LOCAL_PACKAGES = {"backend", "config", "apps"}
+P_STDLIB_HINT = {
+    "os", "sys", "json", "typing", "pathlib", "asyncio", "logging", "datetime", "time",
+    "re", "subprocess", "shutil", "uuid", "contextlib", "dataclasses", "enum", "math",
+    "collections", "functools", "itertools", "tempfile", "io", "base64", "hashlib",
+    "sqlite3", "csv", "random", "traceback", "urllib", "http", "socket", "threading",
+}
+
+
+def p_declared() -> Set[str]:
+    """Only the dependencies array. Scraping the whole file also picked up the project name and
+    version, which pad the declared set and could hide a genuinely missing package."""
+    with open(os.path.join(TEMPLATE, "pyproject.toml")) as fh:
+        body = fh.read()
+    # The closing bracket must be the one at line start: a non-greedy .*? stops at the "]" inside
+    # "fastapi[standard]" and silently returns an EMPTY declared set, which marks everything missing.
+    block = re.search(r"^dependencies\s*=\s*\[(.*?)^\]", body, re.S | re.M)
+    assert block, "no dependencies array in the template pyproject.toml"
+    names = re.findall(r'"([A-Za-z0-9_.\-]+)(?:\[[^\]]*\])?(?:[<>=!~][^"]*)?"', block.group(1))
+    return {n.split("[")[0].lower().replace("-", "_") for n in names}
+
+
+def p_imported() -> Set[str]:
+    found: Set[str] = set()
+    for base, dirs, files in os.walk(TEMPLATE):
+        for fn in files:
+            if not fn.endswith(".py"):
+                continue
+            with open(os.path.join(base, fn)) as fh:
+                try:
+                    tree = ast.parse(fh.read())
+                except SyntaxError:
+                    continue
+            for node in ast.walk(tree):
+                if isinstance(node, ast.Import):
+                    for a in node.names:
+                        found.add(a.name.split(".")[0])
+                elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module:
+                    found.add(node.module.split(".")[0])
+    return {m.lower() for m in found}
+
+
+def test_the_template_declares_every_third_party_import() -> None:
+    imported = p_imported()
+    assert imported, "walked the template and found no imports at all; the scan is broken"
+    third_party = {m for m in imported if m not in P_STDLIB_HINT and m not in P_LOCAL_PACKAGES}
+    declared = p_declared()
+    missing: List[str] = sorted(m for m in third_party if m.replace("-", "_") not in declared)
+    assert not missing, (
+        f"the template imports {missing} without declaring them; a generated app then relies on "
+        f"whatever a transitive dependency happens to provide. declared={sorted(declared)}"
+    )
+
+
+def test_pydantic_is_declared_even_though_fastapi_provides_it() -> None:
+    """The specific case that started this. Agents write BaseModel constantly; asking for the
+    package is how that stops being someone else's transitive gift."""
+    assert "pydantic" in p_declared()
+
+
+def test_the_scan_actually_reads_files() -> None:
+    """A walk that silently matches nothing would make the check above vacuously true."""
+    assert len(p_imported()) >= 3, f"only found {p_imported()}, the template scan is not working"