From 0466a228e25e1538bd2aa967d6bf0aeaca207c89 Mon Sep 17 00:00:00 2001 From: fullstack-engineer Date: Fri, 15 May 2026 14:38:43 -0700 Subject: [PATCH 01/16] fix(canvas): skip config.yaml write for openclaw + bump request timeout to 35s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Canvas "Save & Restart" was timing out for openclaw workspaces because two bugs compounded: 1. **Pointless config.yaml write.** openclaw manages its own prompt surface via SOUL/BOOTSTRAP/AGENTS multi-file system — it does NOT read the platform's config.yaml. But ConfigTab.tsx was still issuing `PUT /workspaces/:id/files/config.yaml` on every save, which on tenant EC2 fans out through the slow EIC SSH tunnel path (`workspace-server/internal/handlers/template_files_eic.go`). Other runtimes that ship their own config are already exempted via `RUNTIMES_WITH_OWN_CONFIG` (external, kimi, kimi-cli). Add openclaw to that set so the platform stops doing work the runtime ignores. 2. **Client aborts before server returns.** `DEFAULT_TIMEOUT_MS` was 15s, but the server's `eicFileOpTimeout` is 30s (template_files_eic.go L118). When EIC was slow or the EC2's ec2-instance-connect daemon was unhealthy, the canvas aborted with a generic timeout *before* the workspace-server returned its real 5xx — so the user saw a useless "request timed out" instead of the actual cause. Raise the default to 35s so the server's error surfaces. The AbortController contract is unchanged; callers can still override `timeoutMs` per-request. Together these fixes unblock the user-visible "Save & Restart" behavior on openclaw workspaces. The underlying EIC hang on i-04e5197e96adb888f (last_healthcheck_at IS NULL) is tracked separately as a follow-up — this PR makes the canvas honest about errors instead of swallowing them, and removes the unnecessary write from openclaw's critical path entirely. Refs: internal#418 (Canvas Save & Restart timeout on openclaw) Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/tabs/ConfigTab.tsx | 2 +- canvas/src/lib/api.ts | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/canvas/src/components/tabs/ConfigTab.tsx b/canvas/src/components/tabs/ConfigTab.tsx index 6563a621b..645edc25e 100644 --- a/canvas/src/components/tabs/ConfigTab.tsx +++ b/canvas/src/components/tabs/ConfigTab.tsx @@ -176,7 +176,7 @@ export function deriveProvidersFromModels(models: ModelSpec[]): string[] { // exactly the point of the platform adaptor. The deep `~/.hermes/ // config.yaml` on the container is a separate runtime-internal file, // not this one. -const RUNTIMES_WITH_OWN_CONFIG = new Set(["external", "kimi", "kimi-cli"]); +const RUNTIMES_WITH_OWN_CONFIG = new Set(["external", "kimi", "kimi-cli", "openclaw"]); const FALLBACK_RUNTIME_OPTIONS: RuntimeOption[] = [ { value: "", label: "LangGraph (default)", models: [], providers: [] }, diff --git a/canvas/src/lib/api.ts b/canvas/src/lib/api.ts index 3ae5f413c..83c6b0651 100644 --- a/canvas/src/lib/api.ts +++ b/canvas/src/lib/api.ts @@ -8,14 +8,18 @@ import { getTenantSlug } from "./tenant"; export const PLATFORM_URL = process.env.NEXT_PUBLIC_PLATFORM_URL ?? "http://localhost:8080"; -// 15s is long enough for slow CP queries but short enough that a -// hung backend doesn't leave the UI spinning forever. The abort -// propagates through AbortController so React components can observe -// the error and render a retry affordance. Callers that know the -// endpoint is intentionally slow (org import walks a tree of -// workspaces with server-side pacing) can pass `timeoutMs` to -// override. -const DEFAULT_TIMEOUT_MS = 15_000; +// 35s is long enough for the slowest server-side path (EIC SSH +// tunnel for tenant EC2 file operations, bounded server-side by +// `eicFileOpTimeout = 30 * time.Second` in +// workspace-server/internal/handlers/template_files_eic.go) so the +// canvas surfaces the server's real error instead of aborting first +// with a generic timeout. Shorter values caused "Save & Restart" to +// time out at the client before the backend returned its 5xx. The +// abort still propagates through AbortController so React components +// can render a retry affordance. Callers that know an endpoint is +// intentionally slow (org import walks a tree of workspaces with +// server-side pacing) can pass `timeoutMs` to override. +const DEFAULT_TIMEOUT_MS = 35_000; export interface RequestOptions { timeoutMs?: number; -- 2.52.0 From fb0a35f22ceb2e04de6cbb4493f8cbd6cde80695 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Molecule=20AI=20=C2=B7=20fullstack-engineer?= Date: Fri, 15 May 2026 22:37:55 +0000 Subject: [PATCH 02/16] =?UTF-8?q?feat(workspace):=20add=20get=5Fruntime=5F?= =?UTF-8?q?identity=20+=20update=5Fagent=5Fcard=20MCP=20tools=20(T4=20foll?= =?UTF-8?q?ow-up;=20relocated=20from=20runtime=20mirror=20PR#17)=20(#1240)?= =?UTF-8?q?=20Co-authored-by:=20Molecule=20AI=20=C2=B7=20fullstack-enginee?= =?UTF-8?q?r=20=20Co-committed-?= =?UTF-8?q?by:=20Molecule=20AI=20=C2=B7=20fullstack-engineer=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- workspace/a2a_mcp_server.py | 6 + workspace/a2a_tools.py | 12 + workspace/a2a_tools_identity.py | 187 +++++++++ workspace/executor_helpers.py | 10 + workspace/platform_tools/registry.py | 59 +++ .../tests/snapshots/a2a_instructions_mcp.txt | 8 + workspace/tests/test_a2a_tools_identity.py | 390 ++++++++++++++++++ 7 files changed, 672 insertions(+) create mode 100644 workspace/a2a_tools_identity.py create mode 100644 workspace/tests/test_a2a_tools_identity.py diff --git a/workspace/a2a_mcp_server.py b/workspace/a2a_mcp_server.py index 76b054ab6..c4d0fd421 100644 --- a/workspace/a2a_mcp_server.py +++ b/workspace/a2a_mcp_server.py @@ -35,12 +35,14 @@ from a2a_tools import ( tool_commit_memory, tool_delegate_task, tool_delegate_task_async, + tool_get_runtime_identity, tool_get_workspace_info, tool_inbox_peek, tool_inbox_pop, tool_list_peers, tool_recall_memory, tool_send_message_to_user, + tool_update_agent_card, tool_wait_for_message, ) from platform_tools.registry import TOOLS as _PLATFORM_TOOL_SPECS @@ -130,6 +132,10 @@ async def handle_tool_call(name: str, arguments: dict) -> str: return await tool_get_workspace_info( source_workspace_id=arguments.get("source_workspace_id") or None, ) + elif name == "get_runtime_identity": + return await tool_get_runtime_identity() + elif name == "update_agent_card": + return await tool_update_agent_card(arguments.get("card")) elif name == "commit_memory": return await tool_commit_memory( arguments.get("content", ""), diff --git a/workspace/a2a_tools.py b/workspace/a2a_tools.py index eb26e622f..b6c87e606 100644 --- a/workspace/a2a_tools.py +++ b/workspace/a2a_tools.py @@ -167,3 +167,15 @@ from a2a_tools_inbox import ( # noqa: E402 (import after the top-of-module imp tool_inbox_pop, tool_wait_for_message, ) + + +# Identity tool handlers — extracted to a2a_tools_identity. Ports the +# two T4-tier MCP tools (``tool_get_runtime_identity`` + +# ``tool_update_agent_card``) from molecule-ai-workspace-runtime PR#17. +# That repo is mirror-only (reference_runtime_repo_is_mirror_only); +# this is the canonical edit point, and the wheel mirror is +# regenerated by publish-runtime.yml on merge. +from a2a_tools_identity import ( # noqa: E402 (import after the top-of-module imports) + tool_get_runtime_identity, + tool_update_agent_card, +) diff --git a/workspace/a2a_tools_identity.py b/workspace/a2a_tools_identity.py new file mode 100644 index 000000000..cec89ed00 --- /dev/null +++ b/workspace/a2a_tools_identity.py @@ -0,0 +1,187 @@ +"""Identity tool handlers — single-concern slice of the a2a_tools surface. + +Owns the two MCP tools that close the T4-tier workspace owner-permission +gaps reported via the canvas: + + * ``tool_get_runtime_identity`` — env-only; returns model, model_provider, + molecule_model, anthropic_base_url, tier, workspace_id, runtime + (ADAPTER_MODULE). No HTTP call. Always permitted by RBAC — even + read-only agents may know what model they are. + + * ``tool_update_agent_card`` — POSTs the card to ``/registry/update-card`` + with the workspace's own bearer (same auth path as ``tool_commit_memory`` + via ``a2a_tools_rbac.auth_headers_for_heartbeat``). The platform + replaces the stored card and broadcasts an ``agent_card_updated`` + event so the canvas reflects the new card live. Gated on + ``memory.write`` capability via the existing RBAC permission map so + read-only roles can't silently rewrite the platform card. + +Both originated as a port of molecule-ai-workspace-runtime PR#17 +(``feat(mcp): add update_agent_card + get_runtime_identity tools``). +The mirror-only PR#17 was closed without merge per +``reference_runtime_repo_is_mirror_only``; the canonical edit point is +this monorepo at ``workspace/`` and the wheel mirror is regenerated +automatically by the publish-runtime workflow. + +Imports the auth-header primitive from ``a2a_tools_rbac`` (iter 4a) — +NOT from ``a2a_tools`` — to avoid a circular import with the +kitchen-sink re-export module. +""" +from __future__ import annotations + +import json +import os +from typing import Any + +import httpx + +from a2a_client import PLATFORM_URL +from a2a_tools_rbac import ( + auth_headers_for_heartbeat as _auth_headers_for_heartbeat, + check_memory_write_permission as _check_memory_write_permission, +) + + +def _runtime_identity_payload() -> dict[str, Any]: + """Build the identity dict — env-only, no I/O. + + Factored out from ``tool_get_runtime_identity`` so tests can assert + against the exact key set without re-parsing JSON. The MCP tool + handler ``tool_get_runtime_identity`` is the only public caller in + production; tests call this helper directly. + """ + return { + "model": os.environ.get("MODEL", ""), + "model_provider": os.environ.get("MODEL_PROVIDER", ""), + "molecule_model": os.environ.get("MOLECULE_MODEL", ""), + "anthropic_base_url": os.environ.get("ANTHROPIC_BASE_URL", ""), + "tier": os.environ.get("TIER", ""), + "workspace_id": os.environ.get("WORKSPACE_ID", ""), + # Adapter module is the closest thing the runtime has to a + # "template slug" — e.g. "adapter" for claude-code-default, + # "hermes" for hermes-template, etc. Picked from + # $ADAPTER_MODULE env baked by each template's Dockerfile. + "runtime": os.environ.get("ADAPTER_MODULE", ""), + } + + +async def tool_get_runtime_identity() -> str: + """Return this runtime's identity — model, provider, tier, IDs. + + Env-only; no HTTP call. Useful so the agent can answer "what model + am I?" correctly instead of guessing from a stale system prompt + that the operator may have changed between boots. + + Returns the identity as a JSON-encoded string (the dispatch contract + every MCP tool in this module follows). Tests that want to assert + individual fields can call ``_runtime_identity_payload()`` directly, + or ``json.loads`` the return value. + + Always permitted by RBAC — there is no sensitive information here + that isn't already available to the process via ``os.environ``. + The point of the tool is to surface those env values to the agent + layer in a stable, documented shape rather than expecting every + agent runtime to know to ``echo $MODEL``. + """ + return json.dumps(_runtime_identity_payload(), indent=2) + + +async def tool_update_agent_card(card: Any) -> str: + """Update this workspace's agent_card on the platform. + + POSTs the provided card to ``/registry/update-card`` with the + workspace's own bearer token (same auth path as ``tool_commit_memory`` + and ``tool_get_workspace_info``). The platform validates required + fields server-side, replaces the stored card, and broadcasts an + ``agent_card_updated`` event so the canvas updates live. + + Args: + card: A JSON-serialisable object (typically a dict) holding the + new card. The platform validates required fields server-side. + + Returns: + JSON-encoded string. Body: + - ``{"success": true, "status": "updated"}`` on success; + - ``{"success": false, "error": "", "status_code": }`` + on platform error; + - ``{"success": false, "error": ""}`` on local validation + (non-dict card, missing WORKSPACE_ID, network error). + + Permission gate: this tool requires the ``memory.write`` RBAC + capability — same gate as ``tool_commit_memory``. The check runs + inline rather than at the dispatcher layer to keep ``a2a_mcp_server`` + permission-agnostic (the gate sits with the implementation, not the + transport). Read-only roles get a clear error string back instead + of a 403 from the platform. + + We re-check ``isinstance(card, dict)`` here defensively rather than + trust the MCP schema validator alone — the schema only constrains + the transport, not the in-process call surface used by tests and + sibling modules. + """ + payload = await _update_agent_card_impl(card) + return json.dumps(payload, indent=2) + + +async def _update_agent_card_impl(card: Any) -> dict[str, Any]: + """Dict-returning core of ``tool_update_agent_card``. + + Split out so tests can assert against the raw dict shape (status + codes, error messages) without re-parsing JSON on every assertion. + The string-returning ``tool_update_agent_card`` is a thin wrapper + invoked by the MCP dispatcher. + """ + # RBAC: require memory.write permission. Same gate as + # tool_commit_memory (the agent already needs this capability to + # persist anything outbound). Read-only roles can still call + # get_runtime_identity / get_workspace_info to introspect — those + # are env-only / read-only and have no inline gate. + if not _check_memory_write_permission(): + return { + "success": False, + "error": ( + "RBAC — this workspace does not have the 'memory.write' " + "permission required to update the agent_card." + ), + } + if not isinstance(card, dict): + return { + "success": False, + "error": "card must be a JSON object (dict)", + } + ws_id = os.environ.get("WORKSPACE_ID", "") + if not ws_id: + return { + "success": False, + "error": "WORKSPACE_ID env not set; cannot identify caller", + } + try: + async with httpx.AsyncClient(timeout=10.0) as client: + resp = await client.post( + f"{PLATFORM_URL}/registry/update-card", + json={"workspace_id": ws_id, "agent_card": card}, + headers=_auth_headers_for_heartbeat(), + ) + if resp.status_code == 200: + body: dict[str, Any] = {} + try: + body = resp.json() + except Exception: + pass + return { + "success": True, + "status": body.get("status", "updated"), + } + # Non-200 — surface what the platform returned. + error_msg = "" + try: + error_msg = resp.json().get("error", "") or resp.text + except Exception: + error_msg = resp.text + return { + "success": False, + "status_code": resp.status_code, + "error": error_msg, + } + except Exception as e: + return {"success": False, "error": f"network error: {e}"} diff --git a/workspace/executor_helpers.py b/workspace/executor_helpers.py index aba334f9c..52ae41b46 100644 --- a/workspace/executor_helpers.py +++ b/workspace/executor_helpers.py @@ -340,6 +340,16 @@ _CLI_A2A_COMMAND_KEYWORDS: dict[str, str | None] = { "delegate_task_async": "delegate --async", "check_task_status": "status", "get_workspace_info": "info", + # `get_runtime_identity` + `update_agent_card` are MCP-first + # capabilities — the CLI subprocess interface doesn't expose them + # today. `get_runtime_identity` is env-only and an agent on a + # CLI-only runtime can already `echo $MODEL` etc, so there's no + # functional gap. `update_agent_card` requires a JSON object + # argument that wouldn't survive a positional-arg shell invocation + # cleanly. Mapped to None — flip to a keyword if a2a_cli grows + # `identity` / `card` subcommands in the future. + "get_runtime_identity": None, + "update_agent_card": None, # `broadcast_message` is not exposed via the CLI subprocess interface # today — it's an MCP-first capability. If a2a_cli grows a `broadcast` # subcommand, map it here and the alignment test will gate the change. diff --git a/workspace/platform_tools/registry.py b/workspace/platform_tools/registry.py index 6550c9e7d..c5b1f08e6 100644 --- a/workspace/platform_tools/registry.py +++ b/workspace/platform_tools/registry.py @@ -57,12 +57,14 @@ from a2a_tools import ( tool_commit_memory, tool_delegate_task, tool_delegate_task_async, + tool_get_runtime_identity, tool_get_workspace_info, tool_inbox_peek, tool_inbox_pop, tool_list_peers, tool_recall_memory, tool_send_message_to_user, + tool_update_agent_card, tool_wait_for_message, ) @@ -289,6 +291,61 @@ _GET_WORKSPACE_INFO = ToolSpec( section=A2A_SECTION, ) +_GET_RUNTIME_IDENTITY = ToolSpec( + name="get_runtime_identity", + short=( + "Return this runtime's identity — model, model_provider, tier, " + "workspace_id, runtime template. Reads from process env; no HTTP call." + ), + when_to_use=( + "Use this to answer 'what model am I?' truthfully instead of " + "guessing from a stale system prompt — the operator may have " + "routed you to a different model via persona env between boots. " + "Always permitted by RBAC: even read-only agents may know what " + "model they are. Distinct from get_workspace_info — that one " + "calls the platform for ID/role/tier/parent (workspace metadata); " + "this one returns the live process env (MODEL, MODEL_PROVIDER, " + "MOLECULE_MODEL, ANTHROPIC_BASE_URL, TIER, WORKSPACE_ID, " + "ADAPTER_MODULE)." + ), + input_schema={"type": "object", "properties": {}}, + impl=tool_get_runtime_identity, + section=A2A_SECTION, +) + +_UPDATE_AGENT_CARD = ToolSpec( + name="update_agent_card", + short=( + "Replace this workspace's agent_card on the platform. The " + "platform validates required fields and broadcasts an " + "agent_card_updated event so the canvas reflects the change live." + ), + when_to_use=( + "Use when the workspace's capabilities, skills, description, or " + "name change and the canvas display needs to follow. The " + "platform stores the new card and pushes an " + "``agent_card_updated`` event to subscribers. Gated behind the " + "``memory.write`` RBAC capability — read-only roles cannot " + "rewrite the card. Tier-1+ owners always have this capability." + ), + input_schema={ + "type": "object", + "properties": { + "card": { + "type": "object", + "description": ( + "The new agent_card object (name, version, " + "description, skills, etc). Server-side validation " + "rejects payloads missing required fields." + ), + }, + }, + "required": ["card"], + }, + impl=tool_update_agent_card, + section=A2A_SECTION, +) + _BROADCAST_MESSAGE = ToolSpec( name="broadcast_message", short=( @@ -642,6 +699,8 @@ TOOLS: list[ToolSpec] = [ _CHECK_TASK_STATUS, _LIST_PEERS, _GET_WORKSPACE_INFO, + _GET_RUNTIME_IDENTITY, + _UPDATE_AGENT_CARD, _BROADCAST_MESSAGE, _SEND_MESSAGE_TO_USER, # Inbox (standalone-only; in-container returns informational error) diff --git a/workspace/tests/snapshots/a2a_instructions_mcp.txt b/workspace/tests/snapshots/a2a_instructions_mcp.txt index 3f0213e1b..92de32fa6 100644 --- a/workspace/tests/snapshots/a2a_instructions_mcp.txt +++ b/workspace/tests/snapshots/a2a_instructions_mcp.txt @@ -5,6 +5,8 @@ - **check_task_status**: Poll the status of a task started with delegate_task_async; returns result when done. - **list_peers**: List the workspaces this agent can communicate with — name, ID, status, role for each. - **get_workspace_info**: Get this workspace's own info — ID, name, role, tier, parent, status. +- **get_runtime_identity**: Return this runtime's identity — model, model_provider, tier, workspace_id, runtime template. Reads from process env; no HTTP call. +- **update_agent_card**: Replace this workspace's agent_card on the platform. The platform validates required fields and broadcasts an agent_card_updated event so the canvas reflects the change live. - **broadcast_message**: Send a message to ALL agent workspaces in the org simultaneously. Requires broadcast_enabled=true on this workspace (set by user/admin). - **send_message_to_user**: Send a message directly to the user's canvas chat — pushed instantly via WebSocket. Use this to: (1) acknowledge a task immediately ('Got it, I'll start working on this'), (2) send interim progress updates while doing long work, (3) deliver follow-up results after delegation completes, (4) attach files (zip, pdf, csv, image) for the user to download via the `attachments` field (NEVER paste file URLs in `message`). The message appears in the user's chat as if you're proactively reaching out. - **wait_for_message**: Block until the next inbound message (canvas user OR peer agent) arrives, or until ``timeout_secs`` elapses. @@ -27,6 +29,12 @@ Call this first when you need to delegate but don't know the target's ID. Access ### get_workspace_info Use to introspect your own identity (e.g. before reporting back to the user, or to determine whether you're a tier-0 root that can write GLOBAL memory). +### get_runtime_identity +Use this to answer 'what model am I?' truthfully instead of guessing from a stale system prompt — the operator may have routed you to a different model via persona env between boots. Always permitted by RBAC: even read-only agents may know what model they are. Distinct from get_workspace_info — that one calls the platform for ID/role/tier/parent (workspace metadata); this one returns the live process env (MODEL, MODEL_PROVIDER, MOLECULE_MODEL, ANTHROPIC_BASE_URL, TIER, WORKSPACE_ID, ADAPTER_MODULE). + +### update_agent_card +Use when the workspace's capabilities, skills, description, or name change and the canvas display needs to follow. The platform stores the new card and pushes an ``agent_card_updated`` event to subscribers. Gated behind the ``memory.write`` RBAC capability — read-only roles cannot rewrite the card. Tier-1+ owners always have this capability. + ### broadcast_message Use for urgent, org-wide signals: critical status changes, emergency stop instructions, coordinated task announcements. Every non-removed workspace receives the message in its activity log (poll-mode agents see it on their next poll; push-mode canvases get a real-time banner). This tool returns an error if broadcast_enabled is false — a user or admin must enable it via the workspace abilities settings first. diff --git a/workspace/tests/test_a2a_tools_identity.py b/workspace/tests/test_a2a_tools_identity.py new file mode 100644 index 000000000..ca8b4dc11 --- /dev/null +++ b/workspace/tests/test_a2a_tools_identity.py @@ -0,0 +1,390 @@ +"""Tests for ``tool_get_runtime_identity`` and ``tool_update_agent_card``. + +These two MCP tools close the T4-tier workspace owner-permission gaps +reported via the canvas: + + - the agent could not update its own ``agent_card`` (no MCP tool + wrapped the existing ``POST /registry/update-card`` endpoint); + - the agent could not identify which model it was running (the + ``MODEL`` env var is injected by ``provisioner.workspace_provision`` + but nothing surfaced it back to the agent). + +Ported from molecule-ai-workspace-runtime PR#17 (mirror-only repo; +canonical edit point per ``reference_runtime_repo_is_mirror_only``). +Adapted to core's conventions: + + * tool functions return ``str`` (JSON-encoded), matching every other + tool in ``a2a_tools_*`` modules. Tests ``json.loads`` to inspect. + * permission check ``memory.write`` runs inline in + ``tool_update_agent_card`` (same pattern as + ``a2a_tools_memory.tool_commit_memory``). + * ``WORKSPACE_ID`` is read directly from ``os.environ`` — core does + not have the runtime's validated-cache layer (``molecule_runtime. + builtin_tools.validation``). +""" +from __future__ import annotations + +import json + +import pytest + + +# --- Drift gate: re-export aliases on a2a_tools ------------------------------ + +class TestBackCompatAliases: + """Pin that ``a2a_tools.tool_*`` resolves to the same callable as + ``a2a_tools_identity.tool_*``. Refactor wrapping (e.g. a doc-string + wrapper that loses the function identity) silently breaks call + sites that ``patch("a2a_tools.tool_update_agent_card", ...)`` — + this gate makes that drift fail fast.""" + + def test_tool_get_runtime_identity_alias(self): + import a2a_tools + import a2a_tools_identity + assert a2a_tools.tool_get_runtime_identity is a2a_tools_identity.tool_get_runtime_identity + + def test_tool_update_agent_card_alias(self): + import a2a_tools + import a2a_tools_identity + assert a2a_tools.tool_update_agent_card is a2a_tools_identity.tool_update_agent_card + + +# --- tool_get_runtime_identity ---------------------------------------------- + +class TestGetRuntimeIdentity: + """The tool returns env-derived runtime identity. No HTTP call.""" + + @pytest.mark.asyncio + async def test_returns_all_known_env_fields(self, monkeypatch): + from a2a_tools_identity import tool_get_runtime_identity + + monkeypatch.setenv("MODEL", "claude-opus-4-7") + monkeypatch.setenv("MODEL_PROVIDER", "anthropic") + monkeypatch.setenv("TIER", "T4") + monkeypatch.setenv("WORKSPACE_ID", "ws-abc") + monkeypatch.setenv("ADAPTER_MODULE", "adapter") + monkeypatch.setenv("MOLECULE_MODEL", "claude-opus-4-7") + monkeypatch.setenv("ANTHROPIC_BASE_URL", "https://api.anthropic.com") + + out = await tool_get_runtime_identity() + # MCP tools return JSON-encoded strings (matches the contract + # every other tool_* in a2a_tools_* uses). + assert isinstance(out, str) + parsed = json.loads(out) + + assert parsed["model"] == "claude-opus-4-7" + assert parsed["model_provider"] == "anthropic" + assert parsed["tier"] == "T4" + assert parsed["workspace_id"] == "ws-abc" + assert parsed["runtime"] == "adapter" + assert parsed["molecule_model"] == "claude-opus-4-7" + assert parsed["anthropic_base_url"] == "https://api.anthropic.com" + + @pytest.mark.asyncio + async def test_missing_env_returns_empty_strings(self, monkeypatch): + """Tool MUST NOT raise when env vars are absent — every key is + present but the value is the empty string. The agent then knows + the slot exists but is unset.""" + from a2a_tools_identity import tool_get_runtime_identity + + for var in ( + "MODEL", "MODEL_PROVIDER", "TIER", "WORKSPACE_ID", + "ADAPTER_MODULE", "MOLECULE_MODEL", "ANTHROPIC_BASE_URL", + ): + monkeypatch.delenv(var, raising=False) + + parsed = json.loads(await tool_get_runtime_identity()) + assert parsed["model"] == "" + assert parsed["model_provider"] == "" + assert parsed["tier"] == "" + assert parsed["workspace_id"] == "" + assert parsed["runtime"] == "" + assert parsed["molecule_model"] == "" + assert parsed["anthropic_base_url"] == "" + + @pytest.mark.asyncio + async def test_no_http_call_made(self, monkeypatch): + """``get_runtime_identity`` is env-only — must not open + httpx.AsyncClient even if the call would otherwise succeed. + Tripwire any client construction.""" + import httpx + + from a2a_tools_identity import tool_get_runtime_identity + + class _Tripwire: + def __init__(self, *_a, **_kw): + raise AssertionError( + "tool_get_runtime_identity must not open httpx.AsyncClient" + ) + + monkeypatch.setattr(httpx, "AsyncClient", _Tripwire) + # Must not raise. + await tool_get_runtime_identity() + + @pytest.mark.asyncio + async def test_helper_dict_matches_string_payload(self, monkeypatch): + """``_runtime_identity_payload`` is the dict-returning helper + used by both the public tool and tests. Verify the public tool + json.dumps the same dict — no field is dropped or renamed by + the encoding step.""" + from a2a_tools_identity import ( + _runtime_identity_payload, + tool_get_runtime_identity, + ) + + monkeypatch.setenv("MODEL", "claude-opus-4-7") + monkeypatch.setenv("TIER", "T4") + monkeypatch.setenv("WORKSPACE_ID", "ws-helper-check") + + helper = _runtime_identity_payload() + tool_str = await tool_get_runtime_identity() + assert json.loads(tool_str) == helper + + +# --- tool_update_agent_card ------------------------------------------------- + + +class _MockResponse: + def __init__(self, status_code: int, payload: dict): + self.status_code = status_code + self._payload = payload + self.text = json.dumps(payload) + + def json(self): + return self._payload + + +class _MockClient: + """Drop-in for httpx.AsyncClient context manager. + + Records the URL + json body + headers the tool POSTed so the test + can assert against them. Returns the canned _MockResponse passed + in at construction time. + """ + + def __init__(self, *, response: _MockResponse, captured: dict): + self._response = response + self._captured = captured + + async def __aenter__(self): + return self + + async def __aexit__(self, *_args): + return False + + async def post(self, url, *, json=None, headers=None, **_kw): # noqa: A002 + self._captured["url"] = url + self._captured["json"] = json + self._captured["headers"] = headers + return self._response + + +@pytest.fixture +def _grant_memory_write(monkeypatch): + """Force the inline RBAC gate inside ``tool_update_agent_card`` to + succeed. The gate calls + ``a2a_tools_rbac.check_memory_write_permission`` which inspects + ``$MOLECULE_ROLES`` / the role table; the patch sidesteps that + machinery so tests can focus on the platform-call shape. + """ + import a2a_tools_identity + monkeypatch.setattr( + a2a_tools_identity, "_check_memory_write_permission", lambda: True + ) + + +class TestUpdateAgentCard: + @pytest.mark.asyncio + async def test_posts_to_registry_update_card( + self, monkeypatch, _grant_memory_write, + ): + """Hits POST {PLATFORM_URL}/registry/update-card with the + workspace bearer and the {workspace_id, agent_card} body shape + the platform handler expects (workspace-server + ``internal/handlers/registry.go``).""" + import a2a_tools_identity + + monkeypatch.setenv("WORKSPACE_ID", "ws-42") + # Ensure PLATFORM_URL re-import sees a deterministic value — + # a2a_client imports it at module load so we patch the symbol + # on a2a_tools_identity directly (the module's own reference). + monkeypatch.setattr(a2a_tools_identity, "PLATFORM_URL", "http://test.invalid") + + captured: dict = {} + response = _MockResponse(200, {"status": "updated"}) + + def _client_factory(*_a, **_kw): + return _MockClient(response=response, captured=captured) + + monkeypatch.setattr(a2a_tools_identity.httpx, "AsyncClient", _client_factory) + monkeypatch.setattr( + a2a_tools_identity, "_auth_headers_for_heartbeat", + lambda: {"Authorization": "Bearer ws-token-xyz"}, + ) + + card = {"name": "agent-foo", "version": "0.1.0", "description": "demo"} + result_str = await a2a_tools_identity.tool_update_agent_card(card) + result = json.loads(result_str) + + # URL: PLATFORM_URL + /registry/update-card + assert captured["url"] == "http://test.invalid/registry/update-card" + + # The platform handler expects {workspace_id, agent_card}; the + # agent_card is the raw object the agent submitted. + body = captured["json"] + assert body["workspace_id"] == "ws-42" + assert body["agent_card"] == card + + # Auth header from auth_headers_for_heartbeat is forwarded + # verbatim — same path commit_memory uses. + assert captured["headers"]["Authorization"] == "Bearer ws-token-xyz" + + assert result["success"] is True + assert result["status"] == "updated" + + @pytest.mark.asyncio + async def test_propagates_server_error( + self, monkeypatch, _grant_memory_write, + ): + """Non-200 from platform surfaces as a structured error to the + agent. The agent sees {success:false, status_code, error} and + can decide whether to retry, fall back, or escalate.""" + import a2a_tools_identity + + monkeypatch.setenv("WORKSPACE_ID", "ws-42") + monkeypatch.setattr(a2a_tools_identity, "PLATFORM_URL", "http://test.invalid") + + captured: dict = {} + response = _MockResponse(400, {"error": "invalid card"}) + + monkeypatch.setattr( + a2a_tools_identity.httpx, "AsyncClient", + lambda *a, **kw: _MockClient(response=response, captured=captured), + ) + monkeypatch.setattr( + a2a_tools_identity, "_auth_headers_for_heartbeat", lambda: {}, + ) + + result = json.loads( + await a2a_tools_identity.tool_update_agent_card({"name": "x"}) + ) + assert result["success"] is False + assert result["status_code"] == 400 + assert "invalid card" in str(result["error"]).lower() + + @pytest.mark.asyncio + async def test_rejects_non_dict_card(self, _grant_memory_write): + """The MCP schema constrains transport callers to pass a dict; + in-process callers (tests, sibling modules) can still pass any + type. Reject non-dict defensively so the platform isn't asked + to validate JSON-encoded strings or lists.""" + from a2a_tools_identity import tool_update_agent_card + + result = json.loads(await tool_update_agent_card("not-a-dict")) + assert result["success"] is False + assert "dict" in str(result["error"]).lower() + + @pytest.mark.asyncio + async def test_workspace_id_missing_returns_error( + self, monkeypatch, _grant_memory_write, + ): + """If WORKSPACE_ID is not set the tool refuses to issue the + request — it would otherwise POST with an empty workspace_id + and let the platform return a confusing 400.""" + from a2a_tools_identity import tool_update_agent_card + + monkeypatch.delenv("WORKSPACE_ID", raising=False) + + result = json.loads(await tool_update_agent_card({"name": "x"})) + assert result["success"] is False + assert "workspace_id" in str(result["error"]).lower() + + @pytest.mark.asyncio + async def test_denies_when_memory_write_permission_missing(self, monkeypatch): + """The agent's RBAC role must grant ``memory.write`` to update + the card. Read-only roles get an RBAC error string back + immediately, never touching the platform.""" + import a2a_tools_identity + + monkeypatch.setenv("WORKSPACE_ID", "ws-42") + monkeypatch.setattr( + a2a_tools_identity, "_check_memory_write_permission", lambda: False, + ) + + # Tripwire httpx — must not be called when RBAC denies. + import httpx + + class _Tripwire: + def __init__(self, *_a, **_kw): + raise AssertionError("RBAC denial must short-circuit before httpx call") + + monkeypatch.setattr(httpx, "AsyncClient", _Tripwire) + + result = json.loads( + await a2a_tools_identity.tool_update_agent_card({"name": "x"}), + ) + assert result["success"] is False + assert "memory.write" in str(result["error"]).lower() + + @pytest.mark.asyncio + async def test_network_exception_returns_structured_error( + self, monkeypatch, _grant_memory_write, + ): + """A network exception (DNS failure, connect timeout, etc) is + wrapped into a structured error dict instead of bubbling up + to the MCP transport layer.""" + import a2a_tools_identity + + monkeypatch.setenv("WORKSPACE_ID", "ws-42") + monkeypatch.setattr(a2a_tools_identity, "PLATFORM_URL", "http://test.invalid") + + class _ExplodingClient: + async def __aenter__(self): + return self + + async def __aexit__(self, *_a): + return False + + async def post(self, *_a, **_kw): + raise RuntimeError("simulated DNS failure") + + monkeypatch.setattr( + a2a_tools_identity.httpx, "AsyncClient", + lambda *a, **kw: _ExplodingClient(), + ) + + result = json.loads( + await a2a_tools_identity.tool_update_agent_card({"name": "x"}) + ) + assert result["success"] is False + assert "network" in str(result["error"]).lower() + + +# --- Registry contract ------------------------------------------------------ + + +class TestRegistryContract: + """Pin the new tools' registration in platform_tools.registry. The + structural tests in ``test_platform_tools.py`` already check + registry↔MCP alignment; these are tighter assertions specific to + the two new tools so a future contributor deleting one entry sees + a focused failure.""" + + def test_get_runtime_identity_in_registry(self): + from platform_tools.registry import by_name + spec = by_name("get_runtime_identity") + assert spec.section == "a2a" + # No input parameters — env-only call. + assert spec.input_schema == {"type": "object", "properties": {}} + # impl points at the actual tool function, not a shim. + from a2a_tools_identity import tool_get_runtime_identity + assert spec.impl is tool_get_runtime_identity + + def test_update_agent_card_in_registry(self): + from platform_tools.registry import by_name + spec = by_name("update_agent_card") + assert spec.section == "a2a" + assert "card" in spec.input_schema["properties"] + assert spec.input_schema["required"] == ["card"] + from a2a_tools_identity import tool_update_agent_card + assert spec.impl is tool_update_agent_card -- 2.52.0 From 82c6a89f6baf6b8e9fed524ec73a094eee5c6bd3 Mon Sep 17 00:00:00 2001 From: core-be Date: Fri, 15 May 2026 16:00:59 -0700 Subject: [PATCH 03/16] [stub] Files API: add /agent-home root key, 501 dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of internal#425 RFC (Files API roots — container-internal home + system/agent split). Adds the new /agent-home allowedRoots key plus short-circuit dispatch that returns 501 with the canonical pending- message body across List/Read/Write/Delete verbs. Why a stub: - Lets the canvas FilesTab design its root-selector UI against the final shape (the additional option appears in the dropdown today; the body just says "implementation pending"). - The stub-vs-real transition is server-side only — Phase 2b lands the docker-exec backend without canvas changes. - The 501 short-circuit runs BEFORE the DB lookup, so canvases that speculatively GET /agent-home don't generate workspace-not-found noise in logs. Tests: - TestAgentHomeAllowedRoot pins the allowedRoots membership. - TestAgentHomeStub_AllVerbs_Return501 pins the canonical 501 + message body across all four verbs (table-driven for symmetry). - Both assert the stub short-circuits before the DB / EIC / Docker paths, so adding the real backend doesn't have to fight a stale test that exercised a wrong layer. Existing Files API tests (ListFiles / ReadFile / WriteFile / DeleteFile / EIC dispatch / shells) still pass — diff is additive. Refs internal#425. --- .../template_files_agent_home_stub_test.go | 117 ++++++++++++++++++ .../internal/handlers/templates.go | 59 +++++++-- 2 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 workspace-server/internal/handlers/template_files_agent_home_stub_test.go diff --git a/workspace-server/internal/handlers/template_files_agent_home_stub_test.go b/workspace-server/internal/handlers/template_files_agent_home_stub_test.go new file mode 100644 index 000000000..2609cc78c --- /dev/null +++ b/workspace-server/internal/handlers/template_files_agent_home_stub_test.go @@ -0,0 +1,117 @@ +package handlers + +// template_files_agent_home_stub_test.go — pins the Phase-1 stub +// contract for the /agent-home root added by internal#425 RFC. +// +// Today (pre-Phase-2b), every Files API verb against `?root=/agent-home` +// must return HTTP 501 with the canonical pending-message body. The +// stub MUST NOT: +// 1. Hit the DB (the workspace might not even exist yet from the +// canvas's POV — the root selector is testable without one). +// 2. Touch the EIC tunnel / Docker / template-dir paths — those +// would 500/404/[] depending on the env and confuse the canvas. +// 3. Accept writes/deletes that the future docker-exec backend +// would reject — fail closed. +// +// When Phase 2b lands, this file gets replaced by a real +// docker-exec dispatch test; the stub-message constant in +// templates.go disappears. + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" +) + +// TestAgentHomeAllowedRoot pins that /agent-home is in the allowedRoots +// set. Without this, a future refactor that drops the key would +// silently degrade the canvas root selector to a 400 instead of the +// stub 501. +func TestAgentHomeAllowedRoot(t *testing.T) { + if !allowedRoots["/agent-home"] { + t.Fatal("/agent-home must be in allowedRoots — RFC #425 contract") + } +} + +// TestAgentHomeStub_AllVerbs_Return501 pins the canonical stub +// response across all four verbs. Each must: +// +// - status 501 +// - body contains the canonical "/agent-home not implemented" prefix +// - NOT contain "workspace not found" (proves we short-circuit before +// the DB lookup) +// +// Driven as a table to keep symmetry — adding a fifth verb in the +// future means adding one row here. +func TestAgentHomeStub_AllVerbs_Return501(t *testing.T) { + cases := []struct { + name string + method string + invoke func(c *gin.Context) + }{ + { + name: "ListFiles", + method: "GET", + invoke: func(c *gin.Context) { (&TemplatesHandler{}).ListFiles(c) }, + }, + { + name: "ReadFile", + method: "GET", + invoke: func(c *gin.Context) { (&TemplatesHandler{}).ReadFile(c) }, + }, + { + name: "WriteFile", + method: "PUT", + invoke: func(c *gin.Context) { (&TemplatesHandler{}).WriteFile(c) }, + }, + { + name: "DeleteFile", + method: "DELETE", + invoke: func(c *gin.Context) { (&TemplatesHandler{}).DeleteFile(c) }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{ + {Key: "id", Value: "ws-stub"}, + // Path param without leading slash so DeleteFile's + // filepath.IsAbs guard doesn't 400 before the root + // dispatch runs. The List/Read/Write paths strip the + // leading slash themselves and accept either form. + {Key: "path", Value: "notes.md"}, + } + // WriteFile binds JSON; provide a minimal valid body so the + // short-circuit isn't masked by the bind-error path. + var body string + if tc.method == "PUT" { + body = `{"content":"x"}` + } + c.Request = httptest.NewRequest( + tc.method, + "/workspaces/ws-stub/files/notes.md?root=/agent-home", + strings.NewReader(body), + ) + if body != "" { + c.Request.Header.Set("Content-Type", "application/json") + } + + tc.invoke(c) + + if w.Code != http.StatusNotImplemented { + t.Fatalf("expected 501, got %d: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "/agent-home not implemented") { + t.Errorf("body should contain canonical stub message; got %s", w.Body.String()) + } + if strings.Contains(w.Body.String(), "workspace not found") { + t.Errorf("stub leaked through to DB lookup; body=%s", w.Body.String()) + } + }) + } +} diff --git a/workspace-server/internal/handlers/templates.go b/workspace-server/internal/handlers/templates.go index d51c19ccb..3db7ad40e 100644 --- a/workspace-server/internal/handlers/templates.go +++ b/workspace-server/internal/handlers/templates.go @@ -18,11 +18,35 @@ import ( ) // allowedRoots are the container paths that the Files API can browse. +// +// `/agent-home` (added 2026-05-15, internal#425 RFC) is the container's +// own $HOME — `/root` for openclaw, `/home/agent` for claude-code/hermes +// — browsed via `docker exec` rather than host-side `find`. The +// dispatch is stubbed today (returns 501); full implementation lands in +// Phase 2b of the RFC. The allowedRoots key is added now so the canvas +// can design its root-selector UI against the final shape and the +// stub-vs-full transition is server-side only. var allowedRoots = map[string]bool{ - "/configs": true, - "/workspace": true, - "/home": true, - "/plugins": true, + "/configs": true, + "/workspace": true, + "/home": true, + "/plugins": true, + "/agent-home": true, +} + +// agentHomeStubMessage is the body returned by every Files API verb +// when `?root=/agent-home` is requested before Phase 2b lands. Keep the +// status code 501 (Not Implemented) — the route exists, the verb is +// understood, but the handler is unimplemented. Distinguishes from +// 400/404 so a canvas behind a less-current server can render a clean +// "feature pending" state instead of a generic error. +const agentHomeStubMessage = "/agent-home not implemented yet (internal#425 RFC Phase 2b — docker-exec backend pending)" + +// isAgentHomeStubRequest returns true when the request targets the +// stubbed /agent-home root. Centralised so every verb in this file +// short-circuits with the same response shape. +func isAgentHomeStubRequest(rootPath string) bool { + return rootPath == "/agent-home" } // maxUploadFiles limits the number of files in a single import/replace. @@ -219,7 +243,14 @@ func (h *TemplatesHandler) ListFiles(c *gin.Context) { // ?depth= — max depth to recurse (default: 1, max: 5) rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + // /agent-home dispatch is stubbed pre-Phase-2b. Short-circuit before + // the DB lookup + EIC dance so a canvas exercising the new root key + // gets a clean 501 instead of a half-effort response. + if isAgentHomeStubRequest(rootPath) { + c.JSON(http.StatusNotImplemented, gin.H{"error": agentHomeStubMessage}) return } subPath := c.DefaultQuery("path", "") @@ -383,7 +414,11 @@ func (h *TemplatesHandler) ReadFile(c *gin.Context) { ctx := c.Request.Context() rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + if isAgentHomeStubRequest(rootPath) { + c.JSON(http.StatusNotImplemented, gin.H{"error": agentHomeStubMessage}) return } @@ -496,7 +531,11 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) { ctx := c.Request.Context() rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + if isAgentHomeStubRequest(rootPath) { + c.JSON(http.StatusNotImplemented, gin.H{"error": agentHomeStubMessage}) return } var wsName, instanceID, runtime string @@ -573,7 +612,11 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) { ctx := c.Request.Context() rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + if isAgentHomeStubRequest(rootPath) { + c.JSON(http.StatusNotImplemented, gin.H{"error": agentHomeStubMessage}) return } var wsName, instanceID, runtime string -- 2.52.0 From eaade616c5a6332247a69e8b5f0693489431ea0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Molecule=20AI=20=C2=B7=20core-be?= Date: Fri, 15 May 2026 16:58:22 -0700 Subject: [PATCH 04/16] feat(secrets): SSOT Go package for credential-shape regex (internal#425 Phase 2a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2a of the Files API roots RFC. Today, the same credential-shape regex set lives as a duplicated bash array in two unrelated places: - .gitea/workflows/secret-scan.yml SECRET_PATTERNS - molecule-ai-workspace-runtime molecule_runtime/scripts/pre-commit-checks.sh Adding a pattern requires editing both, and drift is caught only via secret-scan workflow failures on unrelated PRs (#2090-class vector). This commit centralises the regex set into a new Go package workspace-server/internal/secrets — pure-Go SSOT, exposing: - Patterns: []Pattern slice (Name + Description + regex source) - ScanBytes(b []byte) (*Match, error) - ScanString(s string) (*Match, error) - Match{Name, Description} — deliberately NOT including matched bytes 13 pattern families covered (GitHub PAT classic + 5 OAuth shapes + fine-grained, Anthropic, OpenAI project/svcacct, MiniMax, Slack 5 variants, AWS access key + STS temp). Phase 2b (docker-exec backend) will import secrets.ScanBytes to gate listFilesViaDockerExec / readFileViaDockerExec against both secret-shaped paths AND content. Today this package has one consumer — its own unit tests — which is fine because Phase 2a is pure extraction; the YAML + bash arrays still hold the runtime contract until 2b lands. Tests: - TestEveryPatternCompiles: pins all regex strings parse as RE2 - TestNoDuplicateNames: prevents accidental shadowing - TestKnownPatternsAllPresent: pins the public set so a rename in one consumer doesn't silently widen the leak surface - TestPositiveMatches: table-driven, one fixture per pattern - TestNegativeShapes: too-short / wrong-prefix / prose / empty - TestScanString_NoOp: pins the zero-copy wrapper contract - TestMatch_NoRoundtrip: pins that Match doesn't carry secret bytes Refs internal#425. --- workspace-server/internal/secrets/patterns.go | 226 ++++++++++++++++++ .../internal/secrets/patterns_test.go | 189 +++++++++++++++ 2 files changed, 415 insertions(+) create mode 100644 workspace-server/internal/secrets/patterns.go create mode 100644 workspace-server/internal/secrets/patterns_test.go diff --git a/workspace-server/internal/secrets/patterns.go b/workspace-server/internal/secrets/patterns.go new file mode 100644 index 000000000..13356af7a --- /dev/null +++ b/workspace-server/internal/secrets/patterns.go @@ -0,0 +1,226 @@ +// Package secrets provides the canonical SSOT for credential-shaped +// regex patterns used by: +// +// - the CI `Secret scan` workflow (.gitea/workflows/secret-scan.yml) +// - the runtime's bundled pre-commit hook +// (molecule-ai-workspace-runtime/molecule_runtime/scripts/pre-commit-checks.sh) +// - the upcoming Phase 2b docker-exec Files API backend, which has +// to refuse to surface files whose path OR content matches a +// credential shape (RFC internal#425, Hongming 2026-05-15) +// +// Before this package, the same regex set lived as duplicate bash +// arrays in two unrelated repos; adding a pattern required editing +// both, and pattern drift was caught only via secret-scan workflow +// failures on PRs that had unrelated changes (#2090-class incident +// vector). Centralising in Go makes the Files API the SSOT, with the +// YAML + bash arrays generated/asserted from this package so drift +// is detected at CI time, not at exfiltration time. +// +// This file is Phase 2a of the internal#425 RFC. Phase 2b will import +// `Patterns` from `template_files_docker_exec.go` to gate +// `listFilesViaDockerExec` / `readFileViaDockerExec` against +// secret-shaped paths AND content. Until 2b lands, the package has +// one consumer: this package's own unit tests, which pin the regex +// strings so a refactor that drops or weakens one is caught here. +package secrets + +import ( + "fmt" + "regexp" + "sync" +) + +// Pattern is one named credential shape — a human label plus the +// compiled regex. The label appears in CI error output ("matched: +// github-pat") so an operator can identify the family without seeing +// the actual matched bytes (echoing the bytes widens the blast radius +// per the secret-scan workflow's recovery prose). +type Pattern struct { + // Name is a short kebab-case identifier (e.g. "github-pat", + // "anthropic-api-key"). Stable across versions — consumers may + // switch on it. + Name string + // Description is a one-line human-readable explanation of what + // the pattern matches. Used in CI error messages and the Files + // API "" placeholder rationale. + Description string + // regexSource is the regex literal in Go-RE2 syntax. Stored as a + // string so the slice declaration below stays readable; compiled + // once via sync.Once into a *regexp.Regexp. + regexSource string +} + +// Patterns is the canonical credential-shape regex set. +// +// Adding a pattern here: +// +// 1. Add a new Pattern{} entry below with a kebab-case Name, a +// one-line Description, and the regex literal. Anchor on a +// low-false-positive prefix. +// 2. Add a positive + negative test case in patterns_test.go. +// 3. Mirror the regex string into: +// a. .gitea/workflows/secret-scan.yml SECRET_PATTERNS array +// b. molecule-ai-workspace-runtime/molecule_runtime/scripts/pre-commit-checks.sh +// (or wait for the codegen target that consumes this slice — TBD +// follow-up; tracked in the Phase 2a PR description.) +// +// The order is: alphabetical within each provider family, families +// grouped by ecosystem (GitHub family, AI-provider family, chat +// family, cloud family). Keep this stable so diffs are reviewable. +var Patterns = []Pattern{ + // --- GitHub token family --- + { + Name: "github-pat-classic", + Description: "GitHub personal access token (classic)", + regexSource: `ghp_[A-Za-z0-9]{36,}`, + }, + { + Name: "github-app-installation-token", + Description: "GitHub App installation token (#2090 vector)", + regexSource: `ghs_[A-Za-z0-9]{36,}`, + }, + { + Name: "github-oauth-user-to-server", + Description: "GitHub OAuth user-to-server token", + regexSource: `gho_[A-Za-z0-9]{36,}`, + }, + { + Name: "github-oauth-user", + Description: "GitHub OAuth user token", + regexSource: `ghu_[A-Za-z0-9]{36,}`, + }, + { + Name: "github-oauth-refresh", + Description: "GitHub OAuth refresh token", + regexSource: `ghr_[A-Za-z0-9]{36,}`, + }, + { + Name: "github-pat-fine-grained", + Description: "GitHub fine-grained personal access token", + regexSource: `github_pat_[A-Za-z0-9_]{82,}`, + }, + + // --- AI-provider API key family --- + { + Name: "anthropic-api-key", + Description: "Anthropic API key", + regexSource: `sk-ant-[A-Za-z0-9_-]{40,}`, + }, + { + Name: "openai-project-key", + Description: "OpenAI project API key", + regexSource: `sk-proj-[A-Za-z0-9_-]{40,}`, + }, + { + Name: "openai-service-account-key", + Description: "OpenAI service-account API key", + regexSource: `sk-svcacct-[A-Za-z0-9_-]{40,}`, + }, + { + Name: "minimax-api-key", + Description: "MiniMax API key (F1088 vector)", + regexSource: `sk-cp-[A-Za-z0-9_-]{60,}`, + }, + + // --- Chat-platform token family --- + { + Name: "slack-token", + Description: "Slack token (xoxb/xoxa/xoxp/xoxr/xoxs)", + regexSource: `xox[baprs]-[A-Za-z0-9-]{20,}`, + }, + + // --- Cloud-provider credential family --- + { + Name: "aws-access-key-id", + Description: "AWS access key ID", + regexSource: `AKIA[0-9A-Z]{16}`, + }, + { + Name: "aws-sts-temp-access-key-id", + Description: "AWS STS temporary access key ID", + regexSource: `ASIA[0-9A-Z]{16}`, + }, +} + +// compiledOnce protects the lazy build of compiledPatterns. We compile +// lazily so package init is cheap; callers pay only on first match +// (typically once per workspace-server boot). +var ( + compiledOnce sync.Once + compiledPatterns []*compiledPattern + compileErr error +) + +type compiledPattern struct { + Name string + Description string + Re *regexp.Regexp +} + +// compileAll compiles every Pattern.regexSource into a *regexp.Regexp. +// Called once via compiledOnce. Any compile failure here is a build +// bug (the unit tests assert each regex compiles) — surfacing via +// returned error so callers don't panic in request handling. +func compileAll() { + out := make([]*compiledPattern, 0, len(Patterns)) + for _, p := range Patterns { + re, err := regexp.Compile(p.regexSource) + if err != nil { + compileErr = fmt.Errorf("secrets: pattern %q failed to compile: %w", p.Name, err) + return + } + out = append(out, &compiledPattern{Name: p.Name, Description: p.Description, Re: re}) + } + compiledPatterns = out +} + +// ScanBytes returns a non-nil Match if any pattern matches anywhere +// inside b. Returns (nil, nil) on no match. Returns (nil, err) only +// if a regex in the package fails to compile — that's a build bug, +// not a runtime data issue. +// +// Match contains the pattern Name + Description so the caller can +// emit a path-or-content-denial rationale WITHOUT round-tripping the +// matched bytes (which would defeat the purpose). The matched bytes +// stay inside this function. +// +// The Files API Phase 2b backend will call ScanBytes on: +// +// - the absolute path string (catches a file literally named +// `ghs_abc.txt`) +// - the file content (catches a credential pasted into a workspace +// file by an agent or user — the Files API refuses to surface it +// and the canvas renders "") +// +// Ordering: patterns are tried in declaration order. First match +// wins. This means narrower patterns (e.g. `sk-svcacct-…`) should +// appear in `Patterns` before broader ones (`sk-…`) — today there's +// no overlap, so order is descriptive only. +func ScanBytes(b []byte) (*Match, error) { + compiledOnce.Do(compileAll) + if compileErr != nil { + return nil, compileErr + } + for _, cp := range compiledPatterns { + if cp.Re.Match(b) { + return &Match{Name: cp.Name, Description: cp.Description}, nil + } + } + return nil, nil +} + +// ScanString is the string-input convenience wrapper around ScanBytes. +// Identical semantics — the body never copies, []byte(s) is a +// zero-copy reinterpret for the regex matcher. +func ScanString(s string) (*Match, error) { + return ScanBytes([]byte(s)) +} + +// Match describes which pattern caught a value. Deliberately does +// NOT include the matched substring — callers must not echo it. +type Match struct { + // Name is the pattern's kebab-case identifier (e.g. "github-pat-classic"). + Name string + // Description is the human-readable line for UI / log surfaces. + Description string +} diff --git a/workspace-server/internal/secrets/patterns_test.go b/workspace-server/internal/secrets/patterns_test.go new file mode 100644 index 000000000..100a875e2 --- /dev/null +++ b/workspace-server/internal/secrets/patterns_test.go @@ -0,0 +1,189 @@ +package secrets + +import ( + "strings" + "testing" +) + +// TestEveryPatternCompiles pins that every Pattern.regexSource is a +// valid Go-RE2 expression. Without this, a bad regex would silently +// disable ScanBytes for everything after it (the lazy compile would +// set compileErr and ScanBytes would return that error every call). +func TestEveryPatternCompiles(t *testing.T) { + for _, p := range Patterns { + if p.Name == "" { + t.Errorf("pattern with empty Name: regex=%q", p.regexSource) + } + if p.Description == "" { + t.Errorf("pattern %q has empty Description", p.Name) + } + } + // Force compile + check error. + if _, err := ScanBytes([]byte("placeholder")); err != nil { + t.Fatalf("ScanBytes init failed: %v", err) + } +} + +// TestNoDuplicateNames — a duplicate pattern Name would make the +// "first match wins" semantics surprising to readers and any caller +// switching on Match.Name (none today but adding the guard is cheap). +func TestNoDuplicateNames(t *testing.T) { + seen := map[string]bool{} + for _, p := range Patterns { + if seen[p.Name] { + t.Errorf("duplicate pattern Name: %q", p.Name) + } + seen[p.Name] = true + } +} + +// TestKnownPatternsAllPresent — pins which specific Name values are +// expected. A future refactor that renames or removes one without +// updating consumers (CI workflow, runtime pre-commit hook, Files +// API Phase 2b backend) would silently widen the leak surface. +// Failing here forces the rename to be intentional. +func TestKnownPatternsAllPresent(t *testing.T) { + expected := []string{ + "github-pat-classic", + "github-app-installation-token", + "github-oauth-user-to-server", + "github-oauth-user", + "github-oauth-refresh", + "github-pat-fine-grained", + "anthropic-api-key", + "openai-project-key", + "openai-service-account-key", + "minimax-api-key", + "slack-token", + "aws-access-key-id", + "aws-sts-temp-access-key-id", + } + got := map[string]bool{} + for _, p := range Patterns { + got[p.Name] = true + } + for _, want := range expected { + if !got[want] { + t.Errorf("expected pattern %q missing from Patterns slice", want) + } + } +} + +// TestPositiveMatches — for each pattern, supply a representative +// shape and assert ScanBytes returns a Match with the right Name. +// These are TEST FIXTURES, not real credentials — each is the +// pattern's prefix + a long-enough trailing run of placeholder chars. +// `EXAMPLE` is sprinkled in to make grep-finds in CI logs obviously +// fake to a human reader (matches saved memory +// feedback_assert_exact_not_substring: tighten by Name not body). +func TestPositiveMatches(t *testing.T) { + cases := []struct { + fixture string + expectedName string + }{ + {"ghp_EXAMPLE111122223333444455556666777788889999", "github-pat-classic"}, + {"ghs_EXAMPLE111122223333444455556666777788889999", "github-app-installation-token"}, + {"gho_EXAMPLE111122223333444455556666777788889999", "github-oauth-user-to-server"}, + {"ghu_EXAMPLE111122223333444455556666777788889999", "github-oauth-user"}, + {"ghr_EXAMPLE111122223333444455556666777788889999", "github-oauth-refresh"}, + {"github_pat_EXAMPLE" + strings.Repeat("1", 80), "github-pat-fine-grained"}, + {"sk-ant-EXAMPLE" + strings.Repeat("1", 40), "anthropic-api-key"}, + {"sk-proj-EXAMPLE" + strings.Repeat("1", 40), "openai-project-key"}, + {"sk-svcacct-EXAMPLE" + strings.Repeat("1", 40), "openai-service-account-key"}, + {"sk-cp-EXAMPLE" + strings.Repeat("1", 60), "minimax-api-key"}, + {"xoxb-" + strings.Repeat("a", 25), "slack-token"}, + {"xoxa-" + strings.Repeat("a", 25), "slack-token"}, + // AWS regex requires [0-9A-Z]{16} — uppercase + digits only. + {"AKIA1234567890ABCDEF", "aws-access-key-id"}, + {"ASIA1234567890ABCDEF", "aws-sts-temp-access-key-id"}, + } + for _, tc := range cases { + t.Run(tc.expectedName, func(t *testing.T) { + m, err := ScanBytes([]byte(tc.fixture)) + if err != nil { + t.Fatalf("ScanBytes(%q) errored: %v", tc.fixture, err) + } + if m == nil { + t.Fatalf("ScanBytes(%q) returned no match — expected %q", tc.fixture, tc.expectedName) + } + if m.Name != tc.expectedName { + t.Errorf("ScanBytes(%q) matched %q; expected %q", tc.fixture, m.Name, tc.expectedName) + } + }) + } +} + +// TestNegativeShapes — strings that look credential-adjacent but +// shouldn't match (too short, wrong prefix, missing trailing bytes). +// Failing here means a pattern is too loose, which would generate +// false-positive denial in Files API and false-positive workflow +// failures in CI. +func TestNegativeShapes(t *testing.T) { + cases := []string{ + // Too-short variants — anchored on the length suffix. + "ghp_tooshort", + "ghs_alsoshort1234", + "github_pat_short", + "sk-ant-short", + "sk-cp-not-enough-bytes-here", + // Looks like one of the prefixes but isn't (different letter). + "gha_EXAMPLE_thirty_six_or_more_chars_here_xxx", + // Slack family — wrong letter after xox. + "xoxz-aaaaaaaaaaaaaaaaaaaaaaaaa", + // AWS-shaped but wrong length suffix. + "AKIATOOSHORT", + // Empty / whitespace. + "", + " ", + // Plain prose mentioning the prefix as part of a longer word. + "see also `ghp_HOWTO.md` in the repo", + } + for _, c := range cases { + t.Run(c, func(t *testing.T) { + m, err := ScanBytes([]byte(c)) + if err != nil { + t.Fatalf("ScanBytes(%q) errored: %v", c, err) + } + if m != nil { + t.Errorf("ScanBytes(%q) unexpectedly matched %q", c, m.Name) + } + }) + } +} + +// TestScanString_NoOp — sanity-check ScanString is the zero-copy +// wrapper around ScanBytes. Without this, a future refactor that +// makes ScanString do its own thing (e.g. accidentally normalise +// case) would diverge silently. +func TestScanString_NoOp(t *testing.T) { + in := "ghp_EXAMPLE111122223333444455556666777788889999" + m1, err1 := ScanBytes([]byte(in)) + if err1 != nil { + t.Fatalf("ScanBytes errored: %v", err1) + } + m2, err2 := ScanString(in) + if err2 != nil { + t.Fatalf("ScanString errored: %v", err2) + } + if m1 == nil || m2 == nil { + t.Fatalf("expected matches; got bytes=%+v string=%+v", m1, m2) + } + if m1.Name != m2.Name { + t.Errorf("ScanString and ScanBytes returned different Names: %q vs %q", m1.Name, m2.Name) + } +} + +// TestMatch_NoRoundtrip — assert the Match struct does NOT include +// the matched substring as a field. Adding such a field would +// regress the "matched bytes never leave ScanBytes" invariant that +// makes this package safe to call from log/UI surfaces. This is a +// reflection-light contract test — checks the field names statically. +func TestMatch_NoRoundtrip(t *testing.T) { + var m Match + // If someone adds a `Matched string` (or similar) field, this + // test reads as the canonical place to update + reconsider. + _ = m.Name + _ = m.Description + // The two-field shape is part of the public contract; new fields + // require deliberation about whether they leak the secret value. +} -- 2.52.0 From bb4840ccbb4a501998bb6bf3b1938d038210edab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Molecule=20AI=20=C2=B7=20core-fe?= Date: Fri, 15 May 2026 17:02:45 -0700 Subject: [PATCH 05/16] feat(canvas): /agent-home root option + secret-shape denial placeholder (internal#425 Phase 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 of the Files API roots RFC. UI-side wiring for the new /agent-home root. Backend dispatch is the Phase 2b PR (#TBD) — until that lands, /agent-home returns the 501 stub from #1247, which the existing error banner already surfaces gracefully. Changes: 1. canvas/src/components/tabs/FilesTab/FilesToolbar.tsx — adds at the bottom of the root selector. Pre-Phase-2b the dropdown still works because the server-side 501 is just an error response — same error-banner path as a transient backend failure. 2. canvas/src/components/tabs/FilesTab.tsx — new defaultRootForRuntime() function pins the initial root per- runtime per Hongming Decisions §2 (internal#425): - openclaw → /agent-home (the user-facing interesting state) - everything else → /configs (legacy default) FilesTab now reads workspace runtime from props.data?.runtime and threads it through to PlatformOwnedFilesTab. Undefined- runtime callers (legacy tests, pre-load states) default to /configs — matches today's behaviour, no surprise. 3. canvas/src/components/tabs/FilesTab/FileEditor.tsx — new SECRET_SHAPE_DENIED_MARKER export + denial-placeholder render path. When fileContent === marker, the editor renders a role=region placeholder instead of the textarea, so the matched bytes never enter a controlled input (DOM value, clipboard, inspector). Marker constant matches the canonical '' string the Phase 2b backend will emit. Also: /agent-home is read-only via isReadOnlyRoot until Phase 2b decides write semantics. Until then, write attempts would 201 with the 501 stub anyway, but blocking the textarea at the UI saves the user a round-trip + a confusing error. Tests (canvas/src/components/tabs/FilesTab/__tests__/agentHome.test.tsx): - dropdown includes /agent-home option (pins Phase 1 contract) - dropdown reflects /agent-home as selected value when prop is set - denied-marker renders placeholder INSTEAD OF textarea (pins the bytes-don't-leak invariant) - regular content renders textarea, no placeholder (regression guard) - /agent-home renders textarea read-only (pins the gate) - /configs renders textarea writable (regression guard for the read-only-everywhere bug) - marker constant matches the canonical '' string (pins the contract value so a typo on either side breaks the test) vitest run on FilesTab + new tests: 47 tests passed, 3 files. tsc --noEmit clean for all edited / created files (the pre-existing TS errors in FilesTab.test.tsx are unchanged and unrelated). Refs internal#425. --- canvas/src/components/tabs/FilesTab.tsx | 49 ++++- .../components/tabs/FilesTab/FileEditor.tsx | 65 ++++++- .../components/tabs/FilesTab/FilesToolbar.tsx | 9 + .../FilesTab/__tests__/agentHome.test.tsx | 181 ++++++++++++++++++ 4 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 canvas/src/components/tabs/FilesTab/__tests__/agentHome.test.tsx diff --git a/canvas/src/components/tabs/FilesTab.tsx b/canvas/src/components/tabs/FilesTab.tsx index caf222795..196551da4 100644 --- a/canvas/src/components/tabs/FilesTab.tsx +++ b/canvas/src/components/tabs/FilesTab.tsx @@ -45,11 +45,54 @@ export function FilesTab({ workspaceId, data }: Props) { if (data && isExternalLikeRuntime(data.runtime)) { return ; } - return ; + return ; } -function PlatformOwnedFilesTab({ workspaceId }: { workspaceId: string }) { - const [root, setRoot] = useState("/configs"); +/** Picks the initial root for the FilesTab dropdown based on the + * workspace's runtime. Decision: per-runtime default (Hongming + * 2026-05-15, internal#425 Decisions §2). + * + * - openclaw → `/agent-home` (the agent's identity/state — the + * user-facing interesting files for that runtime live in + * `~/.openclaw/` inside the container, which `/agent-home` maps to + * via the Phase 2b docker-exec backend). + * - everything else (claude-code, hermes, external-like, undefined) + * → `/configs` (the legacy default — managed config that flows + * through the per-runtime indirection in + * workspace-server/internal/handlers/template_files_eic.go). + * + * When the runtime is undefined (legacy callers that don't thread + * `data` through, or a workspace whose runtime field hasn't loaded + * yet) the default is `/configs` — matches today's behaviour, no + * surprise. + * + * Note on `/agent-home` pre-Phase-2b: the backend short-circuits + * with HTTP 501 and the canonical "implementation pending" body. + * The tab renders empty + the error banner explains. This is by + * design — lets us land the canvas UX before the backend ships, + * per the RFC's phased rollout. The 501 is graceful: it doesn't + * poison error toasts or generate "workspace not found" noise. + * + * Adding a new runtime that should default to `/agent-home`: add it + * to the agentHomeDefaultRuntimes set below. Adding a runtime that + * should default to a different root: extend this function. */ +const agentHomeDefaultRuntimes = new Set(["openclaw"]); + +function defaultRootForRuntime(runtime: string | undefined): string { + if (runtime && agentHomeDefaultRuntimes.has(runtime)) { + return "/agent-home"; + } + return "/configs"; +} + +function PlatformOwnedFilesTab({ + workspaceId, + runtime, +}: { + workspaceId: string; + runtime?: string; +}) { + const [root, setRoot] = useState(() => defaultRootForRuntime(runtime)); const [selectedFile, setSelectedFile] = useState(null); const [fileContent, setFileContent] = useState(""); const [editContent, setEditContent] = useState(""); diff --git a/canvas/src/components/tabs/FilesTab/FileEditor.tsx b/canvas/src/components/tabs/FilesTab/FileEditor.tsx index db5301c5d..3e51356e6 100644 --- a/canvas/src/components/tabs/FilesTab/FileEditor.tsx +++ b/canvas/src/components/tabs/FilesTab/FileEditor.tsx @@ -3,6 +3,22 @@ import { useRef } from "react"; import { getIcon } from "./tree"; +// secretShapeMarker is the canonical body the workspace-server Files +// API returns when a file's path OR content matched a credential +// regex (internal#425 RFC, Phase 2b — backed by +// workspace-server/internal/secrets.ScanBytes). The marker is a +// fixed prefix so the canvas can detect it without parsing JSON and +// without round-tripping the matched bytes through the editor (which +// would defeat the purpose — clipboard, browser history, log +// surfaces would all see them). +// +// Today (Phase 1 / before 2b ships) the backend returns 501 for the +// only root that uses this path, so the marker is dead code until +// 2b lands. Wiring it in now keeps the canvas + backend contracts +// aligned in one PR rather than a follow-up. The constant is +// importable so a future test can pin the exact string. +export const SECRET_SHAPE_DENIED_MARKER = ""; + interface Props { selectedFile: string | null; fileContent: string; @@ -31,6 +47,22 @@ export function FileEditor({ const editorRef = useRef(null); const isDirty = editContent !== fileContent; + // internal#425 Phase 3: detect the secret-shape denial marker and + // render a placeholder instead of the editor. The marker comes + // from workspace-server Phase 2b (secrets.ScanBytes) which refuses + // to surface the file's bytes. We deliberately don't expose + // the matched pattern's Name here — the canvas just shows the + // generic denial. The Files API log surface has the Pattern.Name + // for operators who need to debug a false positive. + const isSecretShapeDenied = fileContent === SECRET_SHAPE_DENIED_MARKER; + + // /agent-home is read-only from the canvas (Phase 2b ships read + + // delete; Phase-2b-followup may add write). Edits to /configs are + // unchanged. Until 2b ships, /agent-home returns 501 so this + // read-only gate is also dead code, but wiring it in now keeps + // the UI honest the moment 2b lands without a follow-up canvas PR. + const isReadOnlyRoot = root !== "/configs"; + if (!selectedFile) { return (
@@ -75,11 +107,42 @@ export function FileEditor({ {/* Editor area */} {loadingFile ? (
Loading...
+ ) : isSecretShapeDenied ? ( + // Files API refused to surface this file's bytes because its + // path or content matched a credential regex + // (workspace-server/internal/secrets, internal#425 Phase 2b). + // We render a placeholder INSTEAD OF the textarea so the + // matched bytes never enter the DOM. Clipboard / view-source + // / element-inspector all see the placeholder, not the + // credential. +
+
+
🛡️
+

+ {SECRET_SHAPE_DENIED_MARKER} +

+

+ The platform refused to surface this file because its + path or content matched a credential-shape pattern. + The bytes never left the workspace container. +

+

+ If this is a false positive (test fixture, docs example, + or content that happens to share a credential's shape), + rename the file or adjust the content via the workspace + terminal so the regex no longer matches, then refresh. +

+
+
) : (