From b2eef18069bd5655af0e0fd280e017682ec27be6 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Thu, 13 Aug 2026 02:09:36 -0700 Subject: [PATCH] [eric] template: declare pydantic, httpx and uvicorn instead of riding fastapi's extras, with a general undeclared-import check (ENG-287) --- backend/apps/agents/manager/run/RunOptions.py | 16 +--- .../apps/agents/manager/run/empty_finish.py | 12 +++ .../webapp_template/backend/pyproject.toml | 5 ++ backend/tests/test_final_nudge_is_toolless.py | 37 +++++--- .../test_template_declares_what_it_imports.py | 85 +++++++++++++++++++ 5 files changed, 131 insertions(+), 24 deletions(-) create mode 100644 backend/tests/test_template_declares_what_it_imports.py diff --git a/backend/apps/agents/manager/run/RunOptions.py b/backend/apps/agents/manager/run/RunOptions.py index 005c1735..3d17e79c 100644 --- a/backend/apps/agents/manager/run/RunOptions.py +++ b/backend/apps/agents/manager/run/RunOptions.py @@ -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}") diff --git a/backend/apps/agents/manager/run/empty_finish.py b/backend/apps/agents/manager/run/empty_finish.py index 893089d8..95aa16c8 100644 --- a/backend/apps/agents/manager/run/empty_finish.py +++ b/backend/apps/agents/manager/run/empty_finish.py @@ -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 [], {} diff --git a/backend/apps/outputs/webapp_template/backend/pyproject.toml b/backend/apps/outputs/webapp_template/backend/pyproject.toml index 8f0d7f51..12875047 100644 --- a/backend/apps/outputs/webapp_template/backend/pyproject.toml +++ b/backend/apps/outputs/webapp_template/backend/pyproject.toml @@ -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", ] diff --git a/backend/tests/test_final_nudge_is_toolless.py b/backend/tests/test_final_nudge_is_toolless.py index b8788861..bd0547ca 100644 --- a/backend/tests/test_final_nudge_is_toolless.py +++ b/backend/tests/test_final_nudge_is_toolless.py @@ -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" ) diff --git a/backend/tests/test_template_declares_what_it_imports.py b/backend/tests/test_template_declares_what_it_imports.py new file mode 100644 index 00000000..688d2463 --- /dev/null +++ b/backend/tests/test_template_declares_what_it_imports.py @@ -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"