diff --git a/README.md b/README.md index ee0cfed8..b6710d14 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ tenant = "mycorp_demo.cli:tenant_option" `agent_env.cli_plugins` entries are click commands or groups added next to the built-ins, under the entry-point name whatever the command object is called: `my-tools` above is `agent-env my-tools`. `agent_env.cli_root_options` entries are optional `click.Option` instances with `expose_value=False`; their callback runs before the subcommand, so it can set `AGENT_ENV_CONFIG` and call `agent_env.config.reset_config()` to select the config for the whole process. Plugins load when `agent_env.cli` is imported, after the built-ins. Clash rules: a plugin command whose entry-point name core already uses, or a flag core owns, is skipped with a warning on stderr, and core wins. Two different root options on the same flag are both left off with a warning, and `agent-env plugin list` reports each as a `conflict`; the CLI still starts, so `agent-env plugin remove` can settle it. The same `click.Option` object exported by two entry points, from one package or two, is attached once. Nothing grafts onto existing groups. -Explorer routes: subclass `agent_env.explorer.plugin.ExplorerPlugin`, set the `type` ClassVar, and return a FastAPI `APIRouter` from the `router` property. List it under `[explorer.plugins] impls` or declare an `agent_env.explorer_plugins` entry point; `from_config()` takes no arguments, and a plugin reads its own settings with `agent_env.plugins.settings` (see [Plugin settings](#plugin-settings)). `agent-env up --no-bootstrap` mounts it before the core routers and needs no Docker. +Explorer routes: subclass `agent_env.explorer.plugin.ExplorerPlugin`, set the `type` ClassVar, and return a FastAPI `APIRouter` from the `router` property. List it under `[explorer.plugins] impls` or declare an `agent_env.explorer_plugins` entry point; `from_config()` takes no arguments, and a plugin reads its own settings with `agent_env.plugins.settings` (see [Plugin settings](#plugin-settings)). `agent-env up --no-bootstrap` mounts it before the core routers and needs no Docker. An entity id can contain `/` (a bundle's ids are `@local//`, and so are the instance ids of its runs), so in an explorer URL an id is one percent-encoded path segment, with `/` sent as `%2F`. Under the core prefixes `/api/v1/{artifacts,envs,tasks,agents,evals,task-instances}/` the explorer routes on the encoded path, so a plugin route there receives the segment still encoded and decodes it with `urllib.parse.unquote`. ### Bundles from installed packages diff --git a/src/agent_env/explorer/app.py b/src/agent_env/explorer/app.py index a2bdcd0b..d23c801c 100644 --- a/src/agent_env/explorer/app.py +++ b/src/agent_env/explorer/app.py @@ -18,6 +18,7 @@ from agent_env.config import get_config, get_runner, runtime from agent_env.config.errors import ConfigError +from agent_env.explorer.entity_ids import EncodedIdRouting from agent_env.explorer.openapi_docs import docs_metadata, enrich_openapi_schema from agent_env.explorer.plugin import load_plugins from agent_env.explorer.routers import conversations as conversations_router @@ -31,6 +32,9 @@ API = "/api/v1" +# The routes that take an entity or instance id in the path (see explorer/entity_ids.py). +_ENTITY_ID_PREFIXES = tuple(f"{API}/{c}/" for c in ("artifacts", "envs", "tasks", "agents", "evals", "task-instances")) + # Host headers we always accept; everything else is refused — a DNS-rebinding defense. # (The packaged explorer binds loopback only — see LOCAL_BIND_HOST — so this is # defense-in-depth, not the sole barrier.) @@ -120,6 +124,7 @@ def create_app(static_dir: Optional[str] = None) -> FastAPI: allow_methods=["GET", "POST", "OPTIONS"], allow_headers=["*"], ) + app.add_middleware(EncodedIdRouting, prefixes=_ENTITY_ID_PREFIXES) # A no-auth local explorer needs two guards CORS alone doesn't give (CORS withholds # the *response*, not the *request*). The bind is loopback-only, so the Host allow-list diff --git a/src/agent_env/explorer/entity_ids.py b/src/agent_env/explorer/entity_ids.py new file mode 100644 index 00000000..0a278688 --- /dev/null +++ b/src/agent_env/explorer/entity_ids.py @@ -0,0 +1,47 @@ +"""Entity ids in explorer URLs: an id is one percent-encoded path segment, so ``/`` in an id +travels as ``%2F``. ASGI servers decode ``%2F`` before routing, which would split a +namespaced id such as ``@local/pkg/name`` across segments; ``EncodedIdRouting`` routes the +collection paths on the raw path instead, and ``EntityId`` decodes the matched segment. +""" + +from __future__ import annotations + +import re +from typing import Annotated +from urllib.parse import unquote + +from fastapi import Path +from pydantic import AfterValidator + +_KEPT_ESCAPES = re.compile(r"(%2[fF]|%25)") + +EntityId = Annotated[ + str, + Path(description="The entity id, percent-encoded as one path segment: a '/' in the id is sent as %2F."), + AfterValidator(unquote), +] + + +def route_path(raw_path: str) -> str: + """Decode a raw URL path except ``%2F`` and ``%25``, so an encoded id stays one segment + and ``unquote`` on the matched segment recovers it exactly.""" + return "".join( + part if _KEPT_ESCAPES.fullmatch(part) else unquote(part) + for part in _KEPT_ESCAPES.split(raw_path) + ) + + +class EncodedIdRouting: + """ASGI middleware: for requests under ``prefixes``, route on ``route_path`` of the raw + path. Other paths, including explorer-plugin routes, are left as the server decoded them.""" + + def __init__(self, app, prefixes: tuple[str, ...]) -> None: + self.app = app + self.prefixes = prefixes + + async def __call__(self, scope, receive, send) -> None: + if scope["type"] == "http" and scope.get("raw_path"): + root = scope.get("root_path", "") + if scope["path"][len(root):].startswith(self.prefixes): + scope = dict(scope, path=route_path(scope["raw_path"].decode("latin-1"))) + await self.app(scope, receive, send) diff --git a/src/agent_env/explorer/routers/common.py b/src/agent_env/explorer/routers/common.py index 4993743d..7fbe71a6 100644 --- a/src/agent_env/explorer/routers/common.py +++ b/src/agent_env/explorer/routers/common.py @@ -12,6 +12,7 @@ from agent_env.artifact.registry import canonical_type, equivalent_types from agent_env.config import get_config +from agent_env.explorer.entity_ids import EntityId from agent_env.store import Filter, Sort @@ -150,7 +151,7 @@ def list_items( ) @router.get("/{entity_id}", summary=f"Get {noun.title()}") - def get_item(entity_id: str, version: Optional[int] = None) -> dict: + def get_item(entity_id: EntityId, version: Optional[int] = None) -> dict: store = docs() if version is not None: doc = store.find_one(collection, Filter.of(**{id_field: entity_id, "version": version})) @@ -163,7 +164,7 @@ def get_item(entity_id: str, version: Optional[int] = None) -> dict: @router.get("/{entity_id}/versions", summary=f"List {noun.title()} Versions") def list_versions( - entity_id: str, + entity_id: EntityId, limit: int = Query(100, ge=1, le=500), offset: int = Query(0, ge=0), ) -> list[dict]: diff --git a/src/agent_env/explorer/routers/conversations.py b/src/agent_env/explorer/routers/conversations.py index fa04ee33..f4ea31f3 100644 --- a/src/agent_env/explorer/routers/conversations.py +++ b/src/agent_env/explorer/routers/conversations.py @@ -4,6 +4,7 @@ from fastapi import APIRouter +from agent_env.explorer.entity_ids import EntityId from agent_env.explorer.routers.common import docs from agent_env.store import Filter, Sort @@ -15,7 +16,7 @@ @router.get("/task-instances/{task_instance_id}/conversations") -def list_conversations(task_instance_id: str) -> dict: +def list_conversations(task_instance_id: EntityId) -> dict: """A2A conversations recorded for a task instance, oldest first.""" conversations = docs().query( _CONVERSATIONS_COLLECTION, diff --git a/src/agent_env/explorer/routers/runs.py b/src/agent_env/explorer/routers/runs.py index 6d3de089..37338706 100644 --- a/src/agent_env/explorer/routers/runs.py +++ b/src/agent_env/explorer/routers/runs.py @@ -17,6 +17,7 @@ from pydantic import BaseModel, ConfigDict from agent_env.config import get_config, get_runner +from agent_env.explorer.entity_ids import EntityId from agent_env.explorer.routers.common import PaginatedResponse, docs from agent_env.runner.runner import RunStatus from agent_env.store import Filter, Sort @@ -143,7 +144,7 @@ def _resolve_task_version(task_id: str, version: Optional[int]) -> int: @router.post("/{task_id}/run", response_model=RunResponse) -async def start_run(task_id: str, body: RunRequest | None = None) -> RunResponse: +async def start_run(task_id: EntityId, body: RunRequest | None = None) -> RunResponse: body = body or RunRequest() version = _resolve_task_version(task_id, body.version) handle = await get_runner().submit( @@ -184,7 +185,7 @@ class StartRunsResponse(BaseModel): @router.post("/{task_id}/runs", response_model=StartRunsResponse) -async def start_runs(task_id: str, body: StartRunsRequest | None = None) -> StartRunsResponse: +async def start_runs(task_id: EntityId, body: StartRunsRequest | None = None) -> StartRunsResponse: """Start a group of runs, returning their handles immediately. ``seeds`` wins over ``count`` when both are given (one run per seed).""" body = body or StartRunsRequest() @@ -222,14 +223,14 @@ async def start_runs(task_id: str, body: StartRunsRequest | None = None) -> Star @router.post("/{task_id}/cancel-run") -async def cancel_run(task_id: str, workflow_id: str = Query(...)) -> dict: +async def cancel_run(task_id: EntityId, workflow_id: str = Query(...)) -> dict: canceled = await get_runner().cancel(workflow_id) return {"workflow_id": workflow_id, "canceled": canceled} @router.get("/{task_id}/runs", response_model=PaginatedResponse) def list_runs( - task_id: str, + task_id: EntityId, limit: int = Query(50, ge=1, le=500), offset: int = Query(0, ge=0), ) -> PaginatedResponse: @@ -245,7 +246,7 @@ def list_runs( @router.get("/{task_id}/instances", response_model=PaginatedResponse) def list_instances( - task_id: str, + task_id: EntityId, task_version: Optional[int] = None, limit: int = Query(50, ge=1, le=500), offset: int = Query(0, ge=0), @@ -264,7 +265,7 @@ def list_instances( @router.get("/{task_id}/instances/{instance_id}") -def get_instance(task_id: str, instance_id: str) -> dict: +def get_instance(task_id: EntityId, instance_id: EntityId) -> dict: doc = docs().find_one(TASK_INSTANCES_COLLECTION, Filter.of(instance_id=instance_id)) if doc is None: raise HTTPException(status_code=404, detail=f"instance {instance_id} not found") @@ -373,7 +374,7 @@ def _group_status(task_id: str, run_group_id: str) -> dict: @router.get("/{task_id}/run-groups", response_model=PaginatedResponse) def list_run_groups( - task_id: str, + task_id: EntityId, task_version: Optional[int] = None, limit: int = Query(20, ge=1, le=200), offset: int = Query(0, ge=0), @@ -422,12 +423,12 @@ def list_run_groups( @router.get("/{task_id}/run-groups/{run_group_id}") -def get_run_group(task_id: str, run_group_id: str) -> dict: +def get_run_group(task_id: EntityId, run_group_id: str) -> dict: return _group_status(task_id, run_group_id) @router.get("/{task_id}/run-groups/{run_group_id}/stream") -async def stream_run_group(task_id: str, run_group_id: str): +async def stream_run_group(task_id: EntityId, run_group_id: str): """Server-sent snapshots until every run in the group is terminal, ending with a ``complete`` event.""" from fastapi.responses import StreamingResponse @@ -449,7 +450,7 @@ async def events(): @router.get("/{task_id}/instances/{instance_id}/progress") -def instance_progress(task_id: str, instance_id: str) -> dict: +def instance_progress(task_id: EntityId, instance_id: EntityId) -> dict: """Step progress for one instance. Polled while a run is in flight.""" doc = docs().find_one(TASK_INSTANCES_COLLECTION, Filter.of(instance_id=instance_id)) if doc is None: diff --git a/src/agent_env/explorer/ui/src/components/env-detail-page.tsx b/src/agent_env/explorer/ui/src/components/env-detail-page.tsx index 1ac043a6..d272703c 100644 --- a/src/agent_env/explorer/ui/src/components/env-detail-page.tsx +++ b/src/agent_env/explorer/ui/src/components/env-detail-page.tsx @@ -80,7 +80,7 @@ export function EnvDetailPage({ useEffect(() => { let cancelled = false; setLoading(true); - apiFetch(`${BACKEND_URL}/api/v1/envs/${envId}`) + apiFetch(`${BACKEND_URL}/api/v1/envs/${encodeURIComponent(envId)}`) .then(res => { if (!res.ok) throw new Error(`Failed to fetch (${res.status})`); return res.json(); @@ -98,7 +98,7 @@ export function EnvDetailPage({ Promise.all( mcpIds.map(async id => { try { - const r = await apiFetch(`${BACKEND_URL}/api/v1/envs/${id}`); + const r = await apiFetch(`${BACKEND_URL}/api/v1/envs/${encodeURIComponent(id)}`); if (r.ok) return [id, await r.json()] as const; } catch { /* ignore */ diff --git a/src/agent_env/explorer/ui/src/components/run-step-progress.tsx b/src/agent_env/explorer/ui/src/components/run-step-progress.tsx index 5b0af0f5..b19f60e0 100644 --- a/src/agent_env/explorer/ui/src/components/run-step-progress.tsx +++ b/src/agent_env/explorer/ui/src/components/run-step-progress.tsx @@ -63,7 +63,7 @@ export function RunStepProgress({ }; const poll = () => { apiFetch( - `${BACKEND_URL}/api/v1/tasks/${taskId}/instances/${instanceId}/progress`, + `${BACKEND_URL}/api/v1/tasks/${encodeURIComponent(taskId)}/instances/${encodeURIComponent(instanceId)}/progress`, { signal: controller.signal }, ) .then(r => diff --git a/src/agent_env/explorer/ui/src/components/task-detail-page.tsx b/src/agent_env/explorer/ui/src/components/task-detail-page.tsx index c3becde0..0b5416cd 100644 --- a/src/agent_env/explorer/ui/src/components/task-detail-page.tsx +++ b/src/agent_env/explorer/ui/src/components/task-detail-page.tsx @@ -942,7 +942,7 @@ export function TaskDetailPage({ useEffect(() => { let cancelled = false; setLoading(true); - apiFetch(`${BACKEND_URL}/api/v1/tasks/${taskId}`) + apiFetch(`${BACKEND_URL}/api/v1/tasks/${encodeURIComponent(taskId)}`) .then(res => { if (!res.ok) throw new Error(`Failed to fetch (${res.status})`); return res.json(); diff --git a/src/agent_env/explorer/ui/src/components/universe-detail-page.tsx b/src/agent_env/explorer/ui/src/components/universe-detail-page.tsx index aad0dc57..be5bf75b 100644 --- a/src/agent_env/explorer/ui/src/components/universe-detail-page.tsx +++ b/src/agent_env/explorer/ui/src/components/universe-detail-page.tsx @@ -44,7 +44,7 @@ export function UniverseDetailPage({ const [reloadKey, setReloadKey] = useState(0); useEffect(() => { - apiFetch(`${BACKEND_URL}/api/v1/artifacts/${universeId}/versions`) + apiFetch(`${BACKEND_URL}/api/v1/artifacts/${encodeURIComponent(universeId)}/versions`) .then(res => (res.ok ? res.json() : [])) .then((list: VersionEntry[]) => { setVersions(Array.isArray(list) ? list : []); @@ -56,8 +56,8 @@ export function UniverseDetailPage({ setLoading(true); const url = selectedVersion != null - ? `${BACKEND_URL}/api/v1/artifacts/${universeId}?version=${selectedVersion}` - : `${BACKEND_URL}/api/v1/artifacts/${universeId}`; + ? `${BACKEND_URL}/api/v1/artifacts/${encodeURIComponent(universeId)}?version=${selectedVersion}` + : `${BACKEND_URL}/api/v1/artifacts/${encodeURIComponent(universeId)}`; apiFetch(url) .then(res => { if (!res.ok) throw new Error(`Failed to fetch (${res.status})`); diff --git a/tst/unit/explorer/entity_ids_test.py b/tst/unit/explorer/entity_ids_test.py new file mode 100644 index 00000000..2824924c --- /dev/null +++ b/tst/unit/explorer/entity_ids_test.py @@ -0,0 +1,28 @@ +from urllib.parse import quote, unquote + +import pytest + +from agent_env.explorer.entity_ids import route_path + +IDS = [ + "hello-flat", + "@local/agentenv-framework/hello/hello", + "@local/~/stuff/instances/foo", + "@local/~/Dropbox (Personal)/a&b+c,d@e/tickets", + "@local/~/Été/Straße/tâche", + "weird%2Fbare", + "100%25done", + "odd#id?x", +] + + +@pytest.mark.parametrize("entity_id", IDS) +def test_an_encoded_id_stays_one_segment_and_decodes_exactly(entity_id): + parts = route_path(f"/api/v1/tasks/{quote(entity_id, safe='')}/runs").split("/") + + assert parts[:4] == ["", "api", "v1", "tasks"] and parts[5:] == ["runs"] + assert unquote(parts[4]) == entity_id + + +def test_everything_but_slash_and_percent_is_decoded(): + assert route_path("/a%20b/%40local%2Fx/%25/%2f/%C3%A9") == "/a b/@local%2Fx/%25/%2f/é" diff --git a/tst/unit/explorer/explorer_app_test.py b/tst/unit/explorer/explorer_app_test.py index 5b6234d3..d77809ad 100644 --- a/tst/unit/explorer/explorer_app_test.py +++ b/tst/unit/explorer/explorer_app_test.py @@ -3,6 +3,8 @@ import dataclasses import gzip import tempfile +import time +from urllib.parse import quote import pytest @@ -16,6 +18,7 @@ from agent_env.runner.local_runner import LocalRunner from agent_env.store.object_store.local_object_store import LocalFilesystemObjectStore from agent_env.store.document_store.sqlite_document_store import LocalSqliteDocumentStore +from agent_env.task import Task from agent_env.explorer.routers import objects as objects_router from tst.unit.store.fakes import FakeObjectStore @@ -243,6 +246,82 @@ def test_run_for_unknown_task_is_404(client): assert client.get("/api/v1/tasks/ghost/runs").json()["total"] == 0 +NAMESPACED_IDS = [ + "@local/~/stuff/instances/foo", + "@local/~/triage/runs", + "@local/~/Dropbox (Personal)/a&b+c,d@e/tickets", + "@local/~/Été/Straße/tâche", + "team/triage/versions", + "weird%2Fbare", + "100%25done", +] +ID_ROUTE_COLLECTIONS = {"artifacts": "artifacts", "envs": "envs", "tasks": "tasks", "agents": "a2a_agents", "evals": "evals"} + + +@pytest.mark.parametrize("entity_id", NAMESPACED_IDS) +@pytest.mark.parametrize("path, collection", ID_ROUTE_COLLECTIONS.items()) +def test_an_id_is_one_encoded_path_segment(client, path, collection, entity_id): + """An id's own segments, even ones named like a route, never split it.""" + get_config().get_document_store().insert( + collection, {"id": entity_id, "version": 1, "created_at_utc": "2026-01-04T00:00:00Z"}) + encoded = quote(entity_id, safe="") + + got = client.get(f"/api/v1/{path}/{encoded}") + assert got.status_code == 200 and got.json()["id"] == entity_id + assert [d["id"] for d in client.get(f"/api/v1/{path}/{encoded}/versions").json()] == [entity_id] + assert client.get(f"/api/v1/{path}/{entity_id}").status_code == 404 + + +@pytest.mark.parametrize("task_id", ["@local/~/triage/runs", "team%2F100%25/runs"]) +def test_the_run_routes_address_a_task_whose_id_ends_in_a_route_name(client, monkeypatch, task_id): + class _Task: + version = 1 + + async def run(self, **kw): + return kw.get("context") + + monkeypatch.setattr(Task, "get", classmethod(lambda cls, tid, ver=None: _Task())) + get_config().get_document_store().insert("tasks", {"id": task_id, "version": 1, "created_at_utc": "2026-01-04T00:00:00Z"}) + base = f"/api/v1/tasks/{quote(task_id, safe='')}" + + single = client.post(f"{base}/run", json={"version": 1}).json() + group = client.post(f"{base}/runs", json={"version": 1, "count": 1}).json() + deadline = time.time() + 5 + while time.time() < deadline: + runs = client.get(f"{base}/runs").json()["items"] + if len(runs) == 2 and all(r["status"] == "COMPLETED" for r in runs): + break + time.sleep(0.05) + + assert {r["task_id"] for r in runs} == {task_id} and len(runs) == 2 + # The stub task records no instance, so seed the ones a real run would. + for run in runs: + get_config().get_document_store().insert("task_instances", { + "instance_id": run["instance_id"], "task_id": task_id, "task_version": 1, "status": "completed", + "total_steps": 1, "current_step": 1, "completed_steps": [{"step_id": "s", "status": "success"}], + }) + assert single["instance_id"] in {i["instance_id"] for i in client.get(f"{base}/instances").json()["items"]} + assert client.get(f"{base}/instances/{single['instance_id']}").json()["task_id"] == task_id + assert client.get(f"{base}/instances/{single['instance_id']}/progress").json()["instance_id"] == single["instance_id"] + assert group["run_group_id"] in {g["run_group_id"] for g in client.get(f"{base}/run-groups").json()["items"]} + assert client.get(f"{base}/run-groups/{group['run_group_id']}").json()["task_id"] == task_id + assert "event: complete" in client.get(f"{base}/run-groups/{group['run_group_id']}/stream").text + assert client.post(f"{base}/cancel-run", params={"workflow_id": single["workflow_id"]}).status_code == 200 + + +def test_a_bundle_runs_namespaced_instance_id_is_one_encoded_segment(client): + task_id, instance_id = "@local/~/triage/runs", "@local/~/triage/runs-7f3a9c2e" + store = get_config().get_document_store() + store.insert("task_instances", {"instance_id": instance_id, "task_id": task_id, "task_version": 1, + "status": "completed", "total_steps": 1, "current_step": 1, "completed_steps": []}) + store.insert("agent_env_a2a_conversations", {"task_instance_id": instance_id, "created_at_utc": "2026-01-04T00:00:00Z"}) + task, instance = quote(task_id, safe=""), quote(instance_id, safe="") + + assert client.get(f"/api/v1/tasks/{task}/instances/{instance}").json()["instance_id"] == instance_id + assert client.get(f"/api/v1/tasks/{task}/instances/{instance}/progress").json()["instance_id"] == instance_id + assert len(client.get(f"/api/v1/task-instances/{instance}/conversations").json()["conversations"]) == 1 + + def test_start_runs_caps_the_batch_size(client, monkeypatch): from agent_env.task import Task