diff --git a/docs/workspace-runtime-package.md b/docs/workspace-runtime-package.md index 1b2927e2..aec351bc 100644 --- a/docs/workspace-runtime-package.md +++ b/docs/workspace-runtime-package.md @@ -1,5 +1,14 @@ # Workspace Runtime PyPI Package +## Requires Python >= 3.11 + +The wheel pins `requires_python>=3.11`. On Python 3.10 or older, `pip install +molecule-ai-workspace-runtime` fails with `Could not find a version that +satisfies the requirement (from versions: none)` — the pin filters the only +available artifact before pip even attempts install. Upgrade the interpreter +(`brew install python@3.12` / `apt install python3.12` / etc.) or use a +3.11+ venv. + ## Overview The shared workspace runtime infrastructure has **one editable source** and diff --git a/scripts/build_runtime_package.py b/scripts/build_runtime_package.py index 1d9f3a9d..d922da1d 100755 --- a/scripts/build_runtime_package.py +++ b/scripts/build_runtime_package.py @@ -56,6 +56,7 @@ TOP_LEVEL_MODULES = { "a2a_mcp_server", "a2a_tools", "a2a_tools_delegation", + "a2a_tools_memory", "a2a_tools_rbac", "adapter_base", "agent", diff --git a/workspace-server/internal/handlers/external_connection.go b/workspace-server/internal/handlers/external_connection.go index b12d08fa..c10bf059 100644 --- a/workspace-server/internal/handlers/external_connection.go +++ b/workspace-server/internal/handlers/external_connection.go @@ -198,6 +198,13 @@ const externalUniversalMcpTemplate = `# Universal MCP — standalone register + # Pair with the Claude Code or Python SDK tab if your runtime needs # inbound A2A delivery (canvas messages → agent conversation turns). +# Requires Python >= 3.11. On 3.10 or older pip says +# "Could not find a version that satisfies the requirement +# (from versions: none)" — the wheel's requires_python pin filters +# the only available artifact before pip even attempts install. +# Upgrade the interpreter (brew install python@3.12 / apt install +# python3.12 / etc.) or use a 3.11+ venv. + # 1. Install the workspace runtime wheel: pip install molecule-ai-workspace-runtime diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index 639c8ba9..3dfe2fbd 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -7,6 +7,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "log" "os" @@ -79,7 +80,16 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX } } - ctxLookup := context.Background() + // 5s timeout bounds the lookup independently of any HTTP request + // context. createWorkspaceTree runs in goroutines spawned from the + // /org/import handler, so plumbing the request context here would + // cascade-cancel into provisionWorkspaceAuto and abort in-flight + // EC2 provisioning if the client disconnected mid-import — that's + // the wrong behaviour. A short bounded timeout protects the + // per-row SELECT against a wedged DB without taking the + // drop-everything-on-disconnect tradeoff. + ctxLookup, cancelLookup := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelLookup() // Idempotency: if a workspace with the same (parent_id, name) already // exists, skip the INSERT + canvas_layouts + broadcast + provisioning. // This is what makes /org/import safe to call multiple times — the @@ -91,6 +101,15 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX // (parent exists, some children missing) backfill the missing children // instead of either no-op'ing the whole subtree or duplicating the // existing children. + // + // /org/import is ADDITIVE-ONLY, never destructive. Children present + // in the existing tree but absent from the new template are + // preserved (no DELETE on diff). Skip-path also does NOT propagate + // updates to existing nodes — a re-import that adds an + // initial_memory or schedule to an existing workspace is silently + // dropped (the function bypasses seedInitialMemories, schedule SQL, + // channel config for skipped rows). To force-update an existing + // tree, delete and re-import or use a future /org/sync route. existingID, existing, lookupErr := h.lookupExistingChild(ctxLookup, ws.Name, parentID) if lookupErr != nil { return fmt.Errorf("idempotency check for %s: %w", ws.Name, lookupErr) @@ -605,6 +624,12 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX // // On sql.ErrNoRows: returns ("", false, nil) — caller should INSERT. // On a real DB error: returns ("", false, err) — caller propagates. +// +// errors.Is is wrap-safe — a future caller wrapping the error +// (database/sql can wrap driver errors with %w in some setups) would +// silently break a `err == sql.ErrNoRows` equality check, causing the +// no-rows path to fall through to the "real DB error" branch and +// abort the import. errors.Is unwraps. func (h *OrgHandler) lookupExistingChild(ctx context.Context, name string, parentID *string) (string, bool, error) { var existingID string err := db.DB.QueryRowContext(ctx, ` @@ -614,7 +639,7 @@ func (h *OrgHandler) lookupExistingChild(ctx context.Context, name string, paren AND status != 'removed' LIMIT 1 `, name, parentID).Scan(&existingID) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return "", false, nil } if err != nil { diff --git a/workspace-server/internal/handlers/org_import_idempotency_test.go b/workspace-server/internal/handlers/org_import_idempotency_test.go index cefc6e74..1f2955cb 100644 --- a/workspace-server/internal/handlers/org_import_idempotency_test.go +++ b/workspace-server/internal/handlers/org_import_idempotency_test.go @@ -2,7 +2,9 @@ package handlers import ( "context" + "database/sql" "errors" + "fmt" "go/ast" "go/parser" "go/token" @@ -123,6 +125,36 @@ func TestLookupExistingChild_DBError_Propagates(t *testing.T) { } } +// TestLookupExistingChild_WrappedNoRows_TreatedAsNotFound — pins the +// wrap-safety of the errors.Is(err, sql.ErrNoRows) check. The previous +// `err == sql.ErrNoRows` equality would fall through to the +// "real DB error" branch on a wrapped no-rows error, aborting the +// import for what is in fact the no-rows happy path. driver/sql +// wrapping is currently a non-issue but a future driver change or a +// caller that wraps the result via fmt.Errorf("…: %w", err) would +// silently break the equality check. errors.Is unwraps. +func TestLookupExistingChild_WrappedNoRows_TreatedAsNotFound(t *testing.T) { + mock := setupTestDB(t) + parent := "parent-1" + wrapped := fmt.Errorf("driver-wrapped: %w", sql.ErrNoRows) + mock.ExpectQuery(`SELECT id FROM workspaces`). + WithArgs("Alpha", &parent). + WillReturnError(wrapped) + + h := &OrgHandler{} + id, found, err := h.lookupExistingChild(context.Background(), "Alpha", &parent) + + if err != nil { + t.Fatalf("expected wrapped no-rows to be treated as not-found (err=nil), got: %v", err) + } + if found { + t.Errorf("expected found=false on wrapped no-rows, got found=true") + } + if id != "" { + t.Errorf("expected empty id on wrapped no-rows, got %q", id) + } +} + // workspacesInsertRE matches a SQL literal that begins (after optional // leading whitespace) with `INSERT INTO workspaces` followed by `(` — // requiring the open-paren rules out lookalikes like diff --git a/workspace/a2a_tools.py b/workspace/a2a_tools.py index b482a3be..42dcb1d4 100644 --- a/workspace/a2a_tools.py +++ b/workspace/a2a_tools.py @@ -325,115 +325,14 @@ async def tool_get_workspace_info(source_workspace_id: str | None = None) -> str return json.dumps(info, indent=2) -async def tool_commit_memory( - content: str, - scope: str = "LOCAL", - source_workspace_id: str | None = None, -) -> str: - """Save important information to persistent memory. - - GLOBAL scope is writable only by root workspaces (tier == 0). - RBAC memory.write permission is required for all scope levels. - The source workspace_id is embedded in every record so the platform - can enforce cross-workspace isolation and audit trail. - - ``source_workspace_id`` selects which registered workspace this - memory belongs to when the agent is registered into multiple - workspaces (PR-1 / multi-workspace mode). When unset, falls back - to the module-level WORKSPACE_ID — single-workspace operators see - no behaviour change. - """ - if not content: - return "Error: content is required" - content = _redact_secrets(content) - scope = scope.upper() - if scope not in ("LOCAL", "TEAM", "GLOBAL"): - scope = "LOCAL" - - # RBAC: require memory.write permission (mirrors builtin_tools/memory.py) - if not _check_memory_write_permission(): - return ( - "Error: RBAC — this workspace does not have the 'memory.write' " - "permission for this operation." - ) - - # Scope enforcement: only root workspaces (tier 0) can write GLOBAL memory. - # This prevents tenant workspaces from poisoning org-wide memory (GH#1610). - if scope == "GLOBAL" and not _is_root_workspace(): - return ( - "Error: RBAC — only root workspaces (tier 0) can write to GLOBAL scope. " - "Non-root workspaces may use LOCAL or TEAM scope." - ) - - src = source_workspace_id or WORKSPACE_ID - try: - async with httpx.AsyncClient(timeout=10.0) as client: - resp = await client.post( - f"{PLATFORM_URL}/workspaces/{src}/memories", - json={ - "content": content, - "scope": scope, - # Embed source workspace so the platform can namespace-isolate - # and audit cross-workspace writes (GH#1610 fix). - "workspace_id": src, - }, - headers=_auth_headers_for_heartbeat(src), - ) - data = resp.json() - if resp.status_code in (200, 201): - return json.dumps({"success": True, "id": data.get("id"), "scope": scope}) - return f"Error: {data.get('error', resp.text)}" - except Exception as e: - return f"Error saving memory: {e}" - - -async def tool_recall_memory( - query: str = "", - scope: str = "", - source_workspace_id: str | None = None, -) -> str: - """Search persistent memory for previously saved information. - - RBAC memory.read permission is required (mirrors builtin_tools/memory.py). - The workspace_id is sent as a query parameter so the platform can - cross-validate it against the auth token and defend against any future - path traversal / cross-tenant read bugs in the platform itself. - - ``source_workspace_id`` selects which registered workspace's memories - to search when the agent is registered into multiple workspaces. - Unset → defaults to the module-level WORKSPACE_ID. - """ - # RBAC: require memory.read permission (mirrors builtin_tools/memory.py) - if not _check_memory_read_permission(): - return ( - "Error: RBAC — this workspace does not have the 'memory.read' " - "permission for this operation." - ) - - src = source_workspace_id or WORKSPACE_ID - params: dict[str, str] = {"workspace_id": src} - if query: - params["q"] = query - if scope: - params["scope"] = scope.upper() - try: - async with httpx.AsyncClient(timeout=10.0) as client: - resp = await client.get( - f"{PLATFORM_URL}/workspaces/{src}/memories", - params=params, - headers=_auth_headers_for_heartbeat(src), - ) - data = resp.json() - if isinstance(data, list): - if not data: - return "No memories found." - lines = [] - for m in data: - lines.append(f"[{m.get('scope', '?')}] {m.get('content', '')}") - return "\n".join(lines) - return json.dumps(data) - except Exception as e: - return f"Error recalling memory: {e}" +# Memory tool handlers — extracted to a2a_tools_memory (RFC #2873 iter 4c). +# Re-imported here so call sites + tests that reference +# ``a2a_tools.tool_commit_memory`` / ``tool_recall_memory`` keep +# resolving identically. +from a2a_tools_memory import ( # noqa: E402 (import after the top-of-module imports) + tool_commit_memory, + tool_recall_memory, +) # --------------------------------------------------------------------------- @@ -550,6 +449,52 @@ async def tool_chat_history( return json.dumps(rows) +def _enrich_inbound_for_agent(d: dict) -> dict: + """Add peer_name / peer_role / agent_card_url to a poll-path message. + + The PUSH path (a2a_mcp_server._build_channel_notification) already + enriches the meta dict with these fields, so a Claude Code host + with channel-push sees them. The POLL path goes through + InboxMessage.to_dict, which is intentionally identity-free (the + storage layer doesn't know about the registry cache). Without this + helper, every non-Claude-Code MCP client that uses inbox_peek / + wait_for_message gets a plain message and the receiving agent + can't tell who's writing — breaking the contract documented in + a2a_mcp_server.py:303-345 ("In both paths the same fields apply"). + + Cache-first non-blocking enrichment (same shape as push): on cache + miss the helper returns the bare message; the next call within the + 5-min TTL hits the warm cache. Failure to enrich is non-fatal — + the agent still gets text + peer_id + kind + activity_id, just + without the friendly identity. + """ + peer_id = d.get("peer_id") or "" + if not peer_id: + # canvas_user — no peer to enrich; helper returns the plain + # message unchanged so the canvas reply path still works. + return d + try: + from a2a_client import ( # local import — avoid module-load cycle + _agent_card_url_for, + enrich_peer_metadata_nonblocking, + ) + except Exception: # noqa: BLE001 + # If a2a_client is unavailable (test harness, partial install), + # degrade gracefully — agent still gets the bare envelope. + return d + record = enrich_peer_metadata_nonblocking(peer_id) + if record is not None: + if name := record.get("name"): + d["peer_name"] = name + if role := record.get("role"): + d["peer_role"] = role + # agent_card_url is constructable from peer_id alone — surface it + # even when registry enrichment misses, so the receiving agent has + # a single endpoint to hit for the peer's full capability list. + d["agent_card_url"] = _agent_card_url_for(peer_id) + return d + + async def tool_inbox_peek(limit: int = 10) -> str: """Return up to ``limit`` pending inbound messages without removing them.""" import inbox # local import — avoids a circular dep at module load @@ -558,7 +503,7 @@ async def tool_inbox_peek(limit: int = 10) -> str: if state is None: return _INBOX_NOT_ENABLED_MSG messages = state.peek(limit=limit if isinstance(limit, int) else 10) - return json.dumps([m.to_dict() for m in messages]) + return json.dumps([_enrich_inbound_for_agent(m.to_dict()) for m in messages]) async def tool_inbox_pop(activity_id: str) -> str: @@ -606,4 +551,4 @@ async def tool_wait_for_message(timeout_secs: float = 60.0) -> str: message = await loop.run_in_executor(None, state.wait, timeout) if message is None: return json.dumps({"timeout": True, "timeout_secs": timeout}) - return json.dumps(message.to_dict()) + return json.dumps(_enrich_inbound_for_agent(message.to_dict())) diff --git a/workspace/a2a_tools_memory.py b/workspace/a2a_tools_memory.py new file mode 100644 index 00000000..3e2cff4b --- /dev/null +++ b/workspace/a2a_tools_memory.py @@ -0,0 +1,141 @@ +"""Memory tool handlers — single-concern slice of the a2a_tools surface. + +Extracted from ``a2a_tools.py`` (RFC #2873 iter 4c). Owns the two +agent-memory MCP tools: + + * ``tool_commit_memory`` — write to the workspace's persistent memory. + * ``tool_recall_memory`` — search the workspace's persistent memory. + +Both go through the platform's ``/workspaces/:id/memories`` endpoint; +the platform is the source of truth for namespace isolation + audit +trail. Local responsibility here is RBAC enforcement BEFORE hitting +the network so a denied operation surfaces a clear in-band error +instead of an opaque platform 403. + +Imports the RBAC primitives from ``a2a_tools_rbac`` (iter 4a). +""" +from __future__ import annotations + +import json + +import httpx + +from a2a_client import PLATFORM_URL, WORKSPACE_ID +from a2a_tools_rbac import ( + auth_headers_for_heartbeat as _auth_headers_for_heartbeat, + check_memory_read_permission as _check_memory_read_permission, + check_memory_write_permission as _check_memory_write_permission, + is_root_workspace as _is_root_workspace, +) +from builtin_tools.security import _redact_secrets + + +async def tool_commit_memory( + content: str, + scope: str = "LOCAL", + source_workspace_id: str | None = None, +) -> str: + """Save important information to persistent memory. + + GLOBAL scope is writable only by root workspaces (tier == 0). + RBAC memory.write permission is required for all scope levels. + The source workspace_id is embedded in every record so the platform + can enforce cross-workspace isolation and audit trail. + + ``source_workspace_id`` selects which registered workspace this + memory belongs to when the agent is registered into multiple + workspaces (PR-1 / multi-workspace mode). When unset, falls back + to the module-level WORKSPACE_ID — single-workspace operators see + no behaviour change. + """ + if not content: + return "Error: content is required" + content = _redact_secrets(content) + scope = scope.upper() + if scope not in ("LOCAL", "TEAM", "GLOBAL"): + scope = "LOCAL" + + # RBAC: require memory.write permission (mirrors builtin_tools/memory.py) + if not _check_memory_write_permission(): + return ( + "Error: RBAC — this workspace does not have the 'memory.write' " + "permission for this operation." + ) + + # Scope enforcement: only root workspaces (tier 0) can write GLOBAL memory. + # This prevents tenant workspaces from poisoning org-wide memory (GH#1610). + if scope == "GLOBAL" and not _is_root_workspace(): + return ( + "Error: RBAC — only root workspaces (tier 0) can write to GLOBAL scope. " + "Non-root workspaces may use LOCAL or TEAM scope." + ) + + src = source_workspace_id or WORKSPACE_ID + try: + async with httpx.AsyncClient(timeout=10.0) as client: + resp = await client.post( + f"{PLATFORM_URL}/workspaces/{src}/memories", + json={ + "content": content, + "scope": scope, + # Embed source workspace so the platform can namespace-isolate + # and audit cross-workspace writes (GH#1610 fix). + "workspace_id": src, + }, + headers=_auth_headers_for_heartbeat(src), + ) + data = resp.json() + if resp.status_code in (200, 201): + return json.dumps({"success": True, "id": data.get("id"), "scope": scope}) + return f"Error: {data.get('error', resp.text)}" + except Exception as e: + return f"Error saving memory: {e}" + + +async def tool_recall_memory( + query: str = "", + scope: str = "", + source_workspace_id: str | None = None, +) -> str: + """Search persistent memory for previously saved information. + + RBAC memory.read permission is required (mirrors builtin_tools/memory.py). + The workspace_id is sent as a query parameter so the platform can + cross-validate it against the auth token and defend against any future + path traversal / cross-tenant read bugs in the platform itself. + + ``source_workspace_id`` selects which registered workspace's memories + to search when the agent is registered into multiple workspaces. + Unset → defaults to the module-level WORKSPACE_ID. + """ + # RBAC: require memory.read permission (mirrors builtin_tools/memory.py) + if not _check_memory_read_permission(): + return ( + "Error: RBAC — this workspace does not have the 'memory.read' " + "permission for this operation." + ) + + src = source_workspace_id or WORKSPACE_ID + params: dict[str, str] = {"workspace_id": src} + if query: + params["q"] = query + if scope: + params["scope"] = scope.upper() + try: + async with httpx.AsyncClient(timeout=10.0) as client: + resp = await client.get( + f"{PLATFORM_URL}/workspaces/{src}/memories", + params=params, + headers=_auth_headers_for_heartbeat(src), + ) + data = resp.json() + if isinstance(data, list): + if not data: + return "No memories found." + lines = [] + for m in data: + lines.append(f"[{m.get('scope', '?')}] {m.get('content', '')}") + return "\n".join(lines) + return json.dumps(data) + except Exception as e: + return f"Error recalling memory: {e}" diff --git a/workspace/tests/test_a2a_tools_impl.py b/workspace/tests/test_a2a_tools_impl.py index 43f149cb..b53bb8f3 100644 --- a/workspace/tests/test_a2a_tools_impl.py +++ b/workspace/tests/test_a2a_tools_impl.py @@ -702,9 +702,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(201, {"id": "mem-1"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("Remember this", scope="local") data = json.loads(result) @@ -716,9 +716,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(200, {"id": "mem-2"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("Remember this", scope="INVALID") data = json.loads(result) @@ -728,9 +728,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(200, {"id": "mem-3"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("Team info", scope="TEAM") data = json.loads(result) @@ -741,9 +741,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(201, {"id": "mem-4"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=True): result = await a2a_tools.tool_commit_memory("Global info", scope="GLOBAL") data = json.loads(result) @@ -753,9 +753,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(200, {"id": "mem-5"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("info") data = json.loads(result) @@ -766,9 +766,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(201, {"id": "mem-6"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("info") data = json.loads(result) @@ -779,9 +779,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(400, {"error": "bad request payload"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("info") assert "Error" in result @@ -791,9 +791,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_exc=RuntimeError("storage failure")) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("info") assert "Error saving memory" in result @@ -808,9 +808,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(201, {"id": "mem-poison"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("poisoned GLOBAL memory", scope="GLOBAL") # Must NOT have called the platform — early rejection @@ -824,9 +824,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(201, {"id": "mem-7"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=False), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=False), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): result = await a2a_tools.tool_commit_memory("should be denied", scope="LOCAL") mc.post.assert_not_called() @@ -838,9 +838,9 @@ class TestToolCommitMemory: import a2a_tools mc = _make_http_mock(post_resp=_resp(201, {"id": "mem-8"})) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_write_permission", return_value=True), \ - patch("a2a_tools._is_root_workspace", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_write_permission", return_value=True), \ + patch("a2a_tools_memory._is_root_workspace", return_value=False): await a2a_tools.tool_commit_memory("test content", scope="LOCAL") call_kwargs = mc.post.call_args.kwargs @@ -865,8 +865,8 @@ class TestToolRecallMemory: {"scope": "TEAM", "content": "We use Python 3.11"}, ] mc = _make_http_mock(get_resp=_resp(200, memories)) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=True): result = await a2a_tools.tool_recall_memory(query="capital") assert "[LOCAL]" in result @@ -878,8 +878,8 @@ class TestToolRecallMemory: import a2a_tools mc = _make_http_mock(get_resp=_resp(200, [])) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=True): result = await a2a_tools.tool_recall_memory(query="anything") assert result == "No memories found." @@ -890,8 +890,8 @@ class TestToolRecallMemory: payload = {"error": "search unavailable"} mc = _make_http_mock(get_resp=_resp(200, payload)) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=True): result = await a2a_tools.tool_recall_memory() parsed = json.loads(result) @@ -901,8 +901,8 @@ class TestToolRecallMemory: import a2a_tools mc = _make_http_mock(get_exc=RuntimeError("search service down")) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=True): result = await a2a_tools.tool_recall_memory(query="test") assert "Error recalling memory" in result @@ -913,8 +913,8 @@ class TestToolRecallMemory: import a2a_tools mc = _make_http_mock(get_resp=_resp(200, [])) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=True): await a2a_tools.tool_recall_memory(query="paris", scope="local") call_kwargs = mc.get.call_args.kwargs @@ -928,8 +928,8 @@ class TestToolRecallMemory: import a2a_tools mc = _make_http_mock(get_resp=_resp(200, [])) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=True): await a2a_tools.tool_recall_memory() call_kwargs = mc.get.call_args.kwargs @@ -942,8 +942,8 @@ class TestToolRecallMemory: import a2a_tools mc = _make_http_mock(get_resp=_resp(200, [])) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=True): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=True): await a2a_tools.tool_recall_memory(scope="team") call_kwargs = mc.get.call_args.kwargs @@ -960,8 +960,8 @@ class TestToolRecallMemory: import a2a_tools mc = _make_http_mock(get_resp=_resp(200, [{"scope": "GLOBAL", "content": "secret"}])) - with patch("a2a_tools.httpx.AsyncClient", return_value=mc), \ - patch("a2a_tools._check_memory_read_permission", return_value=False): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=mc), \ + patch("a2a_tools_memory._check_memory_read_permission", return_value=False): result = await a2a_tools.tool_recall_memory(query="secret") mc.get.assert_not_called() diff --git a/workspace/tests/test_a2a_tools_inbox_enrichment.py b/workspace/tests/test_a2a_tools_inbox_enrichment.py new file mode 100644 index 00000000..9a4d2b45 --- /dev/null +++ b/workspace/tests/test_a2a_tools_inbox_enrichment.py @@ -0,0 +1,150 @@ +"""Tests for `_enrich_inbound_for_agent` — the poll-path companion to +the push-path enrichment in `a2a_mcp_server._build_channel_notification`. + +The MCP poll path (inbox_peek / wait_for_message) returns +`InboxMessage.to_dict()`, which has `activity_id, text, peer_id, kind, +method, created_at` but NOT the registry-resolved `peer_name`, +`peer_role`, or `agent_card_url`. The receiving agent then sees a +plain message and can't tell who's writing — breaking the universal +contract documented in `a2a_mcp_server.py:303-345` ("In both paths +the same fields apply"). + +The enrichment helper closes that gap. These tests pin: + - canvas_user (peer_id="") passes through unchanged + - peer_agent with cache hit gets peer_name + peer_role + agent_card_url + - peer_agent with cache miss still gets agent_card_url (constructable + from peer_id alone) + - a2a_client unavailable (test harness without registry) degrades + gracefully — agent still gets the bare envelope +""" + +from __future__ import annotations + +import os + +# a2a_client.py reads WORKSPACE_ID at import time and raises if it's +# unset. Stamp a stub before any test pulls in a2a_tools (which transitively +# imports a2a_client). conftest.py mocks the SDK but not this env var. +os.environ.setdefault("WORKSPACE_ID", "00000000-0000-0000-0000-000000000001") + +import sys +import types +from unittest.mock import patch + + +PEER_UUID = "11111111-2222-3333-4444-555555555555" + + +def test_canvas_user_passes_through_unchanged(): + from a2a_tools import _enrich_inbound_for_agent + + base = { + "activity_id": "act-1", + "text": "hello from canvas", + "peer_id": "", + "kind": "canvas_user", + "method": "message/send", + "created_at": "2026-05-05T11:00:00Z", + } + + out = _enrich_inbound_for_agent(dict(base)) + + # Plain pass-through — no enrichment fields added for canvas_user. + assert out == base + assert "peer_name" not in out + assert "peer_role" not in out + assert "agent_card_url" not in out + + +def test_peer_agent_cache_hit_adds_name_role_and_card_url(): + from a2a_tools import _enrich_inbound_for_agent + + record = {"name": "ops-agent", "role": "sre"} + card_url = f"https://platform.example/registry/{PEER_UUID}/agent-card" + + with patch( + "a2a_client.enrich_peer_metadata_nonblocking", + return_value=record, + ), patch( + "a2a_client._agent_card_url_for", + return_value=card_url, + ): + out = _enrich_inbound_for_agent({ + "activity_id": "act-2", + "text": "ping", + "peer_id": PEER_UUID, + "kind": "peer_agent", + "method": "message/send", + "created_at": "2026-05-05T11:01:00Z", + }) + + assert out["peer_name"] == "ops-agent" + assert out["peer_role"] == "sre" + assert out["agent_card_url"] == card_url + + +def test_peer_agent_cache_miss_still_gets_agent_card_url(): + """agent_card_url is constructable from peer_id alone — surface it + even when registry enrichment misses, so the receiving agent has a + single endpoint to hit for the peer's full capability list.""" + from a2a_tools import _enrich_inbound_for_agent + + card_url = f"https://platform.example/registry/{PEER_UUID}/agent-card" + + with patch( + "a2a_client.enrich_peer_metadata_nonblocking", + return_value=None, # cache miss + ), patch( + "a2a_client._agent_card_url_for", + return_value=card_url, + ): + out = _enrich_inbound_for_agent({ + "activity_id": "act-3", + "text": "ping", + "peer_id": PEER_UUID, + "kind": "peer_agent", + "method": "message/send", + "created_at": "2026-05-05T11:02:00Z", + }) + + assert "peer_name" not in out + assert "peer_role" not in out + assert out["agent_card_url"] == card_url + + +def test_peer_agent_a2a_client_unavailable_degrades_gracefully(monkeypatch): + """If a2a_client can't be imported (test harness, partial install), + return the bare envelope — agent still gets text + peer_id + kind + + activity_id, just without the friendly identity.""" + from a2a_tools import _enrich_inbound_for_agent + + # Stub a2a_client import to fail. + real_module = sys.modules.pop("a2a_client", None) + fake = types.ModuleType("a2a_client") + # Deliberately omit enrich_peer_metadata_nonblocking and + # _agent_card_url_for so the helper's fallback path fires. + sys.modules["a2a_client"] = fake + + try: + out = _enrich_inbound_for_agent({ + "activity_id": "act-4", + "text": "ping", + "peer_id": PEER_UUID, + "kind": "peer_agent", + "method": "message/send", + "created_at": "2026-05-05T11:03:00Z", + }) + finally: + if real_module is not None: + sys.modules["a2a_client"] = real_module + else: + sys.modules.pop("a2a_client", None) + + # Bare envelope passes through — receiving agent still has enough + # to act, even if the friendly identity is missing. + assert out["peer_id"] == PEER_UUID + assert out["text"] == "ping" + assert out["kind"] == "peer_agent" + assert "peer_name" not in out + assert "peer_role" not in out + assert "agent_card_url" not in out diff --git a/workspace/tests/test_a2a_tools_memory.py b/workspace/tests/test_a2a_tools_memory.py new file mode 100644 index 00000000..fb2ff027 --- /dev/null +++ b/workspace/tests/test_a2a_tools_memory.py @@ -0,0 +1,69 @@ +"""Drift gate + smoke tests for ``a2a_tools_memory`` (RFC #2873 iter 4c). + +The full behavior matrix (RBAC denies, scope enforcement, platform +HTTP error paths) lives in ``test_a2a_tools_impl.py`` (TestToolCommitMemory ++ TestToolRecallMemory) which patches `a2a_tools_memory.foo` after the +iter 4c retarget. + +This file pins: + + 1. **Drift gate** — every previously-public symbol on ``a2a_tools`` + (``tool_commit_memory``, ``tool_recall_memory``) is the EXACT same + callable as ``a2a_tools_memory.foo``. Refactor wrapping silently + loses the existing test coverage; this gate makes that drift fail + fast. + 2. **Import contract** — ``a2a_tools_memory`` does NOT pull in + ``a2a_tools`` at module-load time. The handlers depend on + ``a2a_tools_rbac`` (the layered architecture) and ``a2a_client``, + not on the kitchen-sink module that re-exports them. +""" +from __future__ import annotations + +import sys + +import pytest + + +@pytest.fixture(autouse=True) +def _require_workspace_id(monkeypatch): + monkeypatch.setenv("WORKSPACE_ID", "00000000-0000-0000-0000-000000000000") + monkeypatch.setenv("PLATFORM_URL", "http://test.invalid") + yield + + +# ============== Drift gate ============== + +class TestBackCompatAliases: + def test_tool_commit_memory_alias(self): + import a2a_tools + import a2a_tools_memory + assert a2a_tools.tool_commit_memory is a2a_tools_memory.tool_commit_memory + + def test_tool_recall_memory_alias(self): + import a2a_tools + import a2a_tools_memory + assert a2a_tools.tool_recall_memory is a2a_tools_memory.tool_recall_memory + + +# ============== Import contract ============== + +class TestImportContract: + def test_memory_module_does_not_load_a2a_tools(self, monkeypatch): + """`a2a_tools_memory` must depend on `a2a_tools_rbac` (the layered + architecture) and `a2a_client`, NEVER on the kitchen-sink + `a2a_tools`. Top-level `from a2a_tools import …` would defeat + the modularization goal and risk a circular-import.""" + # Drop both modules to control import order + for m in ("a2a_tools", "a2a_tools_memory"): + sys.modules.pop(m, None) + + # Import memory module. Should succeed without a2a_tools loaded. + import a2a_tools_memory # noqa: F401 + assert "a2a_tools_memory" in sys.modules + + def test_a2a_tools_re_exports_memory_handlers(self): + """The opposite direction: a2a_tools must surface every memory + symbol so existing call sites + tests work unchanged.""" + import a2a_tools + assert hasattr(a2a_tools, "tool_commit_memory") + assert hasattr(a2a_tools, "tool_recall_memory") diff --git a/workspace/tests/test_mcp_memory.py b/workspace/tests/test_mcp_memory.py index 117e5417..d2a7ac35 100644 --- a/workspace/tests/test_mcp_memory.py +++ b/workspace/tests/test_mcp_memory.py @@ -63,7 +63,7 @@ async def test_commit_memory_success(monkeypatch): mcp = _load_mcp() client = FakeClient() - monkeypatch.setattr("a2a_tools.httpx.AsyncClient", lambda **kw: client) + monkeypatch.setattr("a2a_tools_memory.httpx.AsyncClient", lambda **kw: client) result = await mcp.handle_tool_call("commit_memory", { "content": "Architecture decision: use Go for backend", @@ -92,7 +92,7 @@ async def test_commit_memory_default_scope(monkeypatch): mcp = _load_mcp() client = FakeClient() - monkeypatch.setattr("a2a_tools.httpx.AsyncClient", lambda **kw: client) + monkeypatch.setattr("a2a_tools_memory.httpx.AsyncClient", lambda **kw: client) result = await mcp.handle_tool_call("commit_memory", { "content": "Some note", @@ -108,7 +108,7 @@ async def test_recall_memory_success(monkeypatch): mcp = _load_mcp() client = FakeClient() - monkeypatch.setattr("a2a_tools.httpx.AsyncClient", lambda **kw: client) + monkeypatch.setattr("a2a_tools_memory.httpx.AsyncClient", lambda **kw: client) result = await mcp.handle_tool_call("recall_memory", {"query": "architecture"}) @@ -127,7 +127,7 @@ async def test_recall_memory_empty(monkeypatch): async def get(self, url, params=None, headers=None, **kwargs): return FakeResponse(200, []) - monkeypatch.setattr("a2a_tools.httpx.AsyncClient", lambda **kw: EmptyClient()) + monkeypatch.setattr("a2a_tools_memory.httpx.AsyncClient", lambda **kw: EmptyClient()) result = await mcp.handle_tool_call("recall_memory", {}) assert "No memories found" in result @@ -139,7 +139,7 @@ async def test_recall_memory_with_scope_filter(monkeypatch): mcp = _load_mcp() client = FakeClient() - monkeypatch.setattr("a2a_tools.httpx.AsyncClient", lambda **kw: client) + monkeypatch.setattr("a2a_tools_memory.httpx.AsyncClient", lambda **kw: client) await mcp.handle_tool_call("recall_memory", {"scope": "TEAM"}) diff --git a/workspace/tests/test_secret_redact.py b/workspace/tests/test_secret_redact.py index d0975969..ecc268e8 100644 --- a/workspace/tests/test_secret_redact.py +++ b/workspace/tests/test_secret_redact.py @@ -357,7 +357,7 @@ class TestA2AToolCommitMemoryRedactsSecrets: fake_client.post = _capture - with patch("a2a_tools.httpx.AsyncClient", return_value=fake_client): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=fake_client): await a2a_tools.tool_commit_memory(content_with_secret) stored = captured.get("content", "") @@ -385,7 +385,7 @@ class TestA2AToolCommitMemoryRedactsSecrets: fake_client.post = _capture - with patch("a2a_tools.httpx.AsyncClient", return_value=fake_client): + with patch("a2a_tools_memory.httpx.AsyncClient", return_value=fake_client): await a2a_tools.tool_commit_memory(f"key={key}") stored = captured.get("content", "")