forked from molecule-ai/molecule-core
test(runtime): pin PR #2756's card-vs-setup decoupling with build_routes helper
PR #2756's contract — card route always mounted regardless of adapter.setup() outcome — lived inline in main.py's `# pragma: no cover` boot sequence. A future refactor that re-coupled the two would have silently bypassed PR #2756 and shipped the original "stuck booting forever" UX again, with no pytest catching it. This change extracts route assembly into workspace/boot_routes.py's build_routes(card, executor, adapter_error) and pins the contract with 6 integration tests using Starlette's TestClient: - test_card_route_serves_200_when_adapter_ready: happy path - test_card_route_serves_200_when_adapter_failed: misconfigured boot, card still 200, skill stubs survive - test_jsonrpc_returns_503_when_no_executor: full -32603 envelope with the adapter_error in error.data - test_jsonrpc_returns_503_with_generic_when_no_error_string: fallback reason for the rare case main.py reaches this branch without one - test_card_route_does_not_depend_on_executor: direct PR #2756 regression guard — both branches MUST mount the card route - test_executor_present_does_not_mount_not_configured_handler: sanity that a healthy workspace doesn't return -32603 to every request Conftest stubs extended with a2a.server.routes / request_handlers classes so the tests work under the existing a2a-mock infra (pattern matches the AgentCard/AgentSkill stubs added for PR #2765). main.py now calls build_routes; the inline if/else is gone. Same production behaviour, cleaner shape, regression-proof. Heavy a2a-sdk imports inside build_routes() are lazy (deferred to the executor-only branch) so tests that only exercise the not-configured path don't pull DefaultRequestHandler / InMemoryTaskStore. card_helpers + boot_routes registered in TOP_LEVEL_MODULES (build drift gate would have caught the missing entry on the wheel-publish smoke). All 18 related tests pass (test_boot_routes.py: 6, test_card_helpers.py: 6, test_not_configured_handler.py: 6). Closes #2761 Pairs with: PR #2756 (decouple agent-card from setup), PR #2765 (defensive isolation of enrichment + transcript) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1282c1c8ff
commit
4f4b6c4f90
@ -58,6 +58,7 @@ TOP_LEVEL_MODULES = {
|
||||
"adapter_base",
|
||||
"agent",
|
||||
"agents_md",
|
||||
"boot_routes",
|
||||
"card_helpers",
|
||||
"config",
|
||||
"configs_dir",
|
||||
|
||||
84
workspace/boot_routes.py
Normal file
84
workspace/boot_routes.py
Normal file
@ -0,0 +1,84 @@
|
||||
"""Build the Starlette routes for a workspace from its (card, adapter
|
||||
state) pair.
|
||||
|
||||
Pairs with PR #2756, which decoupled ``/.well-known/agent-card.json`` from
|
||||
``adapter.setup()`` failure. main.py was the only consumer and was
|
||||
``# pragma: no cover`` — so the wiring (card-route mounted unconditionally,
|
||||
JSON-RPC route swapped between DefaultRequestHandler and the
|
||||
not-configured handler based on ``adapter_ready``) had no pytest coverage.
|
||||
|
||||
A future refactor that re-couples the two would silently bypass PR #2756
|
||||
and shipped the original "stuck booting forever" UX again. That gap is
|
||||
what closes here: extract the route-assembly into a pure function whose
|
||||
behaviour is unit-testable with Starlette's TestClient, and have main.py
|
||||
call it. Issue molecule-core#2761.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
from starlette.routing import Route
|
||||
|
||||
from not_configured_handler import make_not_configured_handler
|
||||
|
||||
# Heavy a2a-sdk imports are lazy: deferred to inside build_routes so
|
||||
# tests that exercise only the not-configured branch (no executor) don't
|
||||
# need a2a.server.request_handlers / routes stubbed in their conftest.
|
||||
# Production boot pays the import cost once, on workspace startup.
|
||||
|
||||
|
||||
def build_routes(
|
||||
agent_card: Any,
|
||||
executor: Any | None,
|
||||
adapter_error: str | None,
|
||||
) -> list:
|
||||
"""Return the list of Starlette routes for this workspace.
|
||||
|
||||
Always mounts ``/.well-known/agent-card.json`` from ``agent_card``.
|
||||
|
||||
JSON-RPC route at ``/`` swaps based on adapter state:
|
||||
|
||||
* ``executor`` is non-None → ``DefaultRequestHandler`` with the
|
||||
executor (production happy-path).
|
||||
* ``executor`` is None → ``not_configured_handler`` returning JSON-RPC
|
||||
``-32603`` with ``adapter_error`` in ``error.data``. The
|
||||
workspace stays REACHABLE (operator can introspect, deprovision,
|
||||
redeploy with corrected env) instead of crash-looping invisibly.
|
||||
|
||||
The two branches are mutually exclusive — caller passes one or the
|
||||
other, never both. Test coverage at ``tests/test_boot_routes.py``
|
||||
pins the contract.
|
||||
"""
|
||||
from a2a.server.routes import create_agent_card_routes
|
||||
|
||||
routes: list = []
|
||||
routes.extend(create_agent_card_routes(agent_card))
|
||||
|
||||
if executor is not None:
|
||||
from a2a.server.request_handlers import DefaultRequestHandler
|
||||
from a2a.server.routes import create_jsonrpc_routes
|
||||
from a2a.server.tasks import InMemoryTaskStore
|
||||
|
||||
handler = DefaultRequestHandler(
|
||||
agent_executor=executor,
|
||||
task_store=InMemoryTaskStore(),
|
||||
agent_card=agent_card,
|
||||
)
|
||||
# enable_v0_3_compat=True is the JSON-RPC wire-compat path: clients
|
||||
# using v0.3-shaped payloads (`"role": "user"` lowercase + camelCase
|
||||
# Pydantic field names) can talk to us without re-deploying.
|
||||
# Outbound payloads must also use v0.3 shape — see main.py's
|
||||
# original comment block for the full a2a-sdk 1.x migration note.
|
||||
routes.extend(
|
||||
create_jsonrpc_routes(
|
||||
request_handler=handler,
|
||||
rpc_url="/",
|
||||
enable_v0_3_compat=True,
|
||||
)
|
||||
)
|
||||
else:
|
||||
routes.append(
|
||||
Route("/", make_not_configured_handler(adapter_error), methods=["POST"])
|
||||
)
|
||||
|
||||
return routes
|
||||
@ -277,54 +277,16 @@ async def main(): # pragma: no cover
|
||||
|
||||
# 7. Wrap in A2A.
|
||||
#
|
||||
# Regression fix (#204): PR #198 tried to wire push_config_store +
|
||||
# push_sender to satisfy #175 (push notification capability), but
|
||||
# PushNotificationSender is an abstract base class in the a2a-sdk and
|
||||
# can't be instantiated directly. Passing it crashed main.py on startup
|
||||
# with `TypeError: Can't instantiate abstract class`. Dropped back to
|
||||
# DefaultRequestHandler's own defaults — pushNotifications capability
|
||||
# in the AgentCard below is still advertised via AgentCapabilities so
|
||||
# clients know we COULD do pushes; actually implementing them requires
|
||||
# a concrete sender subclass, tracked as a Phase-H follow-up to #175.
|
||||
routes = []
|
||||
routes.extend(create_agent_card_routes(agent_card))
|
||||
|
||||
if adapter_ready:
|
||||
handler = DefaultRequestHandler(
|
||||
agent_executor=executor,
|
||||
task_store=InMemoryTaskStore(),
|
||||
# a2a-sdk 1.x added agent_card as a required positional/keyword
|
||||
# argument — it's used internally for capability dispatch (e.g.
|
||||
# routing tasks/get historyLength based on the card's protocol
|
||||
# version). Pass the same agent_card we registered with the
|
||||
# platform so the handler's capability surface matches what the
|
||||
# AgentCard advertises.
|
||||
agent_card=agent_card,
|
||||
)
|
||||
# v1: replace A2AStarletteApplication with Starlette route factory.
|
||||
# rpc_url is required in a2a-sdk 1.x (was implicit at root in 0.x).
|
||||
# Use '/' to match a2a.utils.constants.DEFAULT_RPC_URL — that's also
|
||||
# what the platform's a2a_proxy.go POSTs to (it forwards to the
|
||||
# workspace's URL without appending a path). Card endpoint stays at
|
||||
# the well-known path /.well-known/agent-card.json (handled by
|
||||
# create_agent_card_routes default).
|
||||
# enable_v0_3_compat=True is the JSON-RPC wire-compat path: clients
|
||||
# using v0.3-shaped payloads (`"role": "user"` lowercase + camelCase
|
||||
# Pydantic field names) can talk to us without re-deploying.
|
||||
routes.extend(create_jsonrpc_routes(request_handler=handler, rpc_url="/", enable_v0_3_compat=True))
|
||||
else:
|
||||
# Misconfigured: serve the card but reject JSON-RPC with -32603 so
|
||||
# canvas surfaces a useful "agent not configured: <reason>" instead
|
||||
# of letting requests time out. Handler factory is in its own module
|
||||
# so the behavior is unit-testable (workspace/tests/test_not_configured_handler.py).
|
||||
from starlette.routing import Route
|
||||
from not_configured_handler import make_not_configured_handler
|
||||
|
||||
routes.append(
|
||||
Route("/", make_not_configured_handler(adapter_error), methods=["POST"])
|
||||
)
|
||||
|
||||
app = Starlette(routes=routes)
|
||||
# Route assembly is in workspace/boot_routes.py so the contract —
|
||||
# card always mounted, JSON-RPC route swaps based on adapter state
|
||||
# (DefaultRequestHandler when executor is non-None, not_configured
|
||||
# handler returning -32603 otherwise) — is unit-testable with
|
||||
# Starlette's TestClient. main.py is `# pragma: no cover` so without
|
||||
# this extraction a future refactor that re-coupled card + setup()
|
||||
# would silently bypass PR #2756. tests/test_boot_routes.py pins
|
||||
# the four-branch contract.
|
||||
from boot_routes import build_routes
|
||||
app = Starlette(routes=build_routes(agent_card, executor, adapter_error))
|
||||
|
||||
# 8. Register with platform
|
||||
# When adapter.setup() failed, advertise via configuration_status so
|
||||
|
||||
@ -181,6 +181,77 @@ def _make_a2a_mocks():
|
||||
types_mod.AgentInterface = AgentInterface
|
||||
types_mod.AgentCard = AgentCard
|
||||
|
||||
# a2a.server.routes — used by boot_routes.build_routes (PR #2756 chain
|
||||
# / #2761) to mount /.well-known/agent-card.json. The real SDK builds
|
||||
# a Starlette route that serializes the card on each request; the stub
|
||||
# mirrors that behaviour with json.dumps over the card's __dict__ so
|
||||
# TestClient.get("/.well-known/agent-card.json") returns the same
|
||||
# shape canvas would see in production.
|
||||
routes_mod = ModuleType("a2a.server.routes")
|
||||
|
||||
def _create_agent_card_routes(card):
|
||||
from starlette.responses import JSONResponse
|
||||
from starlette.routing import Route
|
||||
|
||||
async def _card_handler(_request):
|
||||
# Convert the stub AgentCard into a JSON-serialisable dict.
|
||||
# Real a2a.types.AgentCard is a Pydantic model with proper
|
||||
# serialisation; the stub stores attrs raw, so we walk
|
||||
# __dict__ and serialise nested AgentSkill objects too.
|
||||
def _to_dict(obj):
|
||||
if hasattr(obj, "__dict__"):
|
||||
return {k: _to_dict(v) for k, v in vars(obj).items()}
|
||||
if isinstance(obj, list):
|
||||
return [_to_dict(x) for x in obj]
|
||||
if isinstance(obj, dict):
|
||||
return {k: _to_dict(v) for k, v in obj.items()}
|
||||
return obj
|
||||
|
||||
return JSONResponse(_to_dict(card))
|
||||
|
||||
return [Route("/.well-known/agent-card.json", _card_handler, methods=["GET"])]
|
||||
|
||||
def _create_jsonrpc_routes(request_handler=None, rpc_url="/", **_kwargs):
|
||||
from starlette.responses import JSONResponse
|
||||
from starlette.routing import Route
|
||||
|
||||
async def _jsonrpc_handler(_request):
|
||||
# Stub: real DefaultRequestHandler dispatches to the executor;
|
||||
# tests that need real behaviour will use a test-side mock.
|
||||
# This stub just returns a JSON-RPC envelope so the not-configured
|
||||
# branch's discriminator (`error.data` containing "setup() failed")
|
||||
# has something to differ from.
|
||||
return JSONResponse({"jsonrpc": "2.0", "result": "stub-jsonrpc-handler"})
|
||||
|
||||
return [Route(rpc_url, _jsonrpc_handler, methods=["POST"])]
|
||||
|
||||
routes_mod.create_agent_card_routes = _create_agent_card_routes
|
||||
routes_mod.create_jsonrpc_routes = _create_jsonrpc_routes
|
||||
sys.modules["a2a.server.routes"] = routes_mod
|
||||
|
||||
# a2a.server.request_handlers — used by boot_routes' executor branch.
|
||||
# DefaultRequestHandler stub takes the same kwargs as the real one;
|
||||
# tests that exercise the executor path don't poke at the handler's
|
||||
# internals, only that it gets mounted at "/".
|
||||
rh_mod = ModuleType("a2a.server.request_handlers")
|
||||
|
||||
class DefaultRequestHandler:
|
||||
def __init__(self, agent_executor=None, task_store=None, agent_card=None, **_kwargs):
|
||||
self.agent_executor = agent_executor
|
||||
self.task_store = task_store
|
||||
self.agent_card = agent_card
|
||||
|
||||
rh_mod.DefaultRequestHandler = DefaultRequestHandler
|
||||
sys.modules["a2a.server.request_handlers"] = rh_mod
|
||||
|
||||
# InMemoryTaskStore is exposed via a2a.server.tasks (already stubbed
|
||||
# above with TaskUpdater). Add it as a no-op class.
|
||||
class _InMemoryTaskStore:
|
||||
def __init__(self):
|
||||
pass
|
||||
|
||||
tasks_mod.InMemoryTaskStore = _InMemoryTaskStore
|
||||
|
||||
# a2a.helpers (v1: moved from a2a.utils, renamed new_agent_text_message
|
||||
# → new_text_message). Mock both names — production code only calls
|
||||
# new_text_message, but if any test still references the old name it
|
||||
|
||||
213
workspace/tests/test_boot_routes.py
Normal file
213
workspace/tests/test_boot_routes.py
Normal file
@ -0,0 +1,213 @@
|
||||
"""Integration tests for boot_routes.build_routes — pin the contract that
|
||||
PR #2756's card-vs-setup decoupling depends on.
|
||||
|
||||
Why these matter (issue #2761): main.py is ``# pragma: no cover``. The
|
||||
inline if/else that mounted ``DefaultRequestHandler`` vs the
|
||||
not-configured handler had no pytest coverage; a future refactor that
|
||||
re-coupled card and setup() would have shipped the original "stuck
|
||||
booting forever" UX again. Extracting to ``boot_routes.build_routes``
|
||||
+ these tests make the contract regression-proof.
|
||||
|
||||
Each test exercises a real Starlette TestClient against the routes —
|
||||
no uvicorn, no socket, but every assertion is the same one canvas's
|
||||
TranscriptHandler / a2a_proxy would make in production.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
# Make workspace/ importable in test isolation — same pattern as the
|
||||
# adjacent tests (test_not_configured_handler.py, test_card_helpers.py).
|
||||
WORKSPACE_DIR = Path(__file__).resolve().parents[1]
|
||||
if str(WORKSPACE_DIR) not in sys.path:
|
||||
sys.path.insert(0, str(WORKSPACE_DIR))
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def agent_card():
|
||||
"""Build a minimal AgentCard the way main.py does at boot."""
|
||||
from a2a.types import (
|
||||
AgentCard,
|
||||
AgentCapabilities,
|
||||
AgentInterface,
|
||||
AgentSkill,
|
||||
)
|
||||
|
||||
return AgentCard(
|
||||
name="test-agent",
|
||||
description="test-agent",
|
||||
version="0.0.0",
|
||||
supported_interfaces=[
|
||||
AgentInterface(protocol_binding="https://a2a.g/v1", url="http://test:8000")
|
||||
],
|
||||
capabilities=AgentCapabilities(streaming=True, push_notifications=False),
|
||||
skills=[
|
||||
AgentSkill(id="echo", name="echo", description="echo", tags=[], examples=[])
|
||||
],
|
||||
default_input_modes=["text/plain"],
|
||||
default_output_modes=["text/plain"],
|
||||
)
|
||||
|
||||
|
||||
# ---- card route always mounted, regardless of adapter state -------------
|
||||
|
||||
|
||||
def test_card_route_serves_200_when_adapter_ready(agent_card):
|
||||
"""Adapter setup OK → card serves 200, the canonical happy path."""
|
||||
from starlette.applications import Starlette
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from boot_routes import build_routes
|
||||
|
||||
fake_executor = MagicMock()
|
||||
app = Starlette(routes=build_routes(agent_card, fake_executor, None))
|
||||
client = TestClient(app)
|
||||
resp = client.get("/.well-known/agent-card.json")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
assert body["name"] == "test-agent"
|
||||
|
||||
|
||||
def test_card_route_serves_200_when_adapter_failed(agent_card):
|
||||
"""Adapter setup raised → card route is STILL mounted with the same
|
||||
static skills. This is the entire point of PR #2756: a misconfigured
|
||||
workspace stays REACHABLE so canvas can show the user a clear error
|
||||
instead of silently looking dead."""
|
||||
from starlette.applications import Starlette
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from boot_routes import build_routes
|
||||
|
||||
app = Starlette(
|
||||
routes=build_routes(
|
||||
agent_card, executor=None, adapter_error="MISSING_API_KEY"
|
||||
)
|
||||
)
|
||||
client = TestClient(app)
|
||||
resp = client.get("/.well-known/agent-card.json")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
assert body["name"] == "test-agent"
|
||||
# Skill stubs survive even though setup() didn't run.
|
||||
assert any(s.get("id") == "echo" for s in body.get("skills", []))
|
||||
|
||||
|
||||
# ---- JSON-RPC route swaps based on executor presence -------------------
|
||||
|
||||
|
||||
def test_jsonrpc_returns_503_when_no_executor(agent_card):
|
||||
"""The not-configured branch: POST / returns 503 with JSON-RPC -32603
|
||||
and the adapter_error in error.data. This is what canvas sees when a
|
||||
user tries to message a workspace whose setup() failed — turns a
|
||||
"stuck silent" workspace into "agent not configured: <reason>"."""
|
||||
from starlette.applications import Starlette
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from boot_routes import build_routes
|
||||
|
||||
app = Starlette(
|
||||
routes=build_routes(
|
||||
agent_card,
|
||||
executor=None,
|
||||
adapter_error="RuntimeError: Neither OPENAI_API_KEY nor MINIMAX_API_KEY is set",
|
||||
)
|
||||
)
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/",
|
||||
json={"jsonrpc": "2.0", "id": 42, "method": "message/send"},
|
||||
)
|
||||
assert resp.status_code == 503
|
||||
body = resp.json()
|
||||
assert body["jsonrpc"] == "2.0"
|
||||
assert body["id"] == 42 # echoed
|
||||
assert body["error"]["code"] == -32603
|
||||
assert "MINIMAX_API_KEY" in body["error"]["data"]
|
||||
|
||||
|
||||
def test_jsonrpc_returns_503_with_generic_when_no_error_string(agent_card):
|
||||
"""Defensive: if main.py reached this branch without a captured
|
||||
error string (shouldn't happen in practice but the helper is
|
||||
defensive), the handler still returns -32603 with a generic
|
||||
fallback so the operator gets a useful response shape."""
|
||||
from starlette.applications import Starlette
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from boot_routes import build_routes
|
||||
|
||||
app = Starlette(
|
||||
routes=build_routes(agent_card, executor=None, adapter_error=None)
|
||||
)
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/", json={"jsonrpc": "2.0", "id": 1, "method": "message/send"}
|
||||
)
|
||||
assert resp.status_code == 503
|
||||
assert resp.json()["error"]["code"] == -32603
|
||||
# Falls back to generic "adapter.setup() failed".
|
||||
assert "setup() failed" in resp.json()["error"]["data"]
|
||||
|
||||
|
||||
# ---- Specific regression: re-coupling card to setup would break this ---
|
||||
|
||||
|
||||
def test_card_route_does_not_depend_on_executor(agent_card):
|
||||
"""Direct regression test for PR #2756. If a future refactor moved
|
||||
create_agent_card_routes into the executor-only branch, this test
|
||||
would catch it: the card MUST be served from a code path that runs
|
||||
even when executor is None."""
|
||||
from boot_routes import build_routes
|
||||
|
||||
routes_with_executor = build_routes(agent_card, MagicMock(), None)
|
||||
routes_without_executor = build_routes(agent_card, None, "err")
|
||||
|
||||
# Both branches mount /.well-known/agent-card.json. Find by path.
|
||||
def has_card_route(routes):
|
||||
for r in routes:
|
||||
for attr in ("path", "path_format"):
|
||||
p = getattr(r, attr, None)
|
||||
if p and "agent-card.json" in p:
|
||||
return True
|
||||
return False
|
||||
|
||||
assert has_card_route(routes_with_executor), (
|
||||
"card route MUST be mounted on the executor-present path"
|
||||
)
|
||||
assert has_card_route(routes_without_executor), (
|
||||
"card route MUST be mounted on the executor-missing path "
|
||||
"(this is the PR #2756 contract — re-coupling here breaks tenant readiness)"
|
||||
)
|
||||
|
||||
|
||||
def test_executor_present_does_not_mount_not_configured_handler(agent_card):
|
||||
"""Sanity: when executor is present, the not-configured handler
|
||||
must NOT be mounted at /. Otherwise a healthy workspace would
|
||||
return -32603 to every JSON-RPC call.
|
||||
|
||||
We call POST / with a malformed JSON-RPC body and assert the
|
||||
response is NOT the -32603 not-configured envelope. (The real
|
||||
DefaultRequestHandler may return its own error for the malformed
|
||||
payload, but it won't have ``data: "adapter.setup() failed"``.)"""
|
||||
from starlette.applications import Starlette
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from boot_routes import build_routes
|
||||
|
||||
fake_executor = MagicMock()
|
||||
app = Starlette(routes=build_routes(agent_card, fake_executor, None))
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/", json={"jsonrpc": "2.0", "id": 1, "method": "message/send"}
|
||||
)
|
||||
body = resp.json() if resp.headers.get("content-type", "").startswith("application/json") else {}
|
||||
# Whatever DefaultRequestHandler does, it isn't the not-configured
|
||||
# envelope. The cheap discriminator: error.data won't say "setup() failed".
|
||||
err = body.get("error") or {}
|
||||
data = err.get("data") if isinstance(err, dict) else ""
|
||||
assert "setup() failed" not in (data or ""), (
|
||||
"executor-present branch must not mount the not-configured handler"
|
||||
)
|
||||
Loading…
Reference in New Issue
Block a user