From 6b5707454d95ec0e64f76c61a87f0b3c651ffbff Mon Sep 17 00:00:00 2001 From: Kyle Seaman Date: Fri, 7 Aug 2026 23:56:30 +0000 Subject: [PATCH] feat(agents): author agent templates from the dashboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agent template is the kiro spec at `~/.kiro/agents/.json` — the system prompt, tool surface, auto-approve list, MCP servers and skill mapping an agent boots from. The Agent Templates tab could list, retarget the model, edit skills and delete one, but there was no route that CREATED one: `/api/agents/detail` served only GET/PATCH/DELETE, and PATCH accepted just `model` and `skills`. A template could therefore only arrive by installing a package, by an app shipping one, or by hand-writing JSON. Crews have full CRUD, but a crew only BINDS to a template — its picker offers whatever is already installed — so "make a new agent" meant adopting someone else's definition under a new label. Two capabilities close that: * `POST /api/agents/detail` writes `.json`, either from a conservative blank baseline or as a copy of an existing template (`from`). * `PATCH` accepts `prompt` and `description`, so a created template can be corrected without editing JSON by hand. The privilege surface is deliberately NOT writable through either verb. `tools`, `allowedTools` and `toolsSettings` are the auto-approve list and the bash deny patterns, so a request body cannot mint a spec that auto-approves everything: a copy inherits them from a spec already on disk, and a blank template gets a tool surface whose auto-approved subset is read-only — editing a file or running a command has to be approved the first time. Kiro Crew's own privileged MCP servers are absent from the blank baseline; getting that surface means duplicating `kirocrew`, which is an explicit act. Guards, each pinned by a revert-verified test: * The name must be safe as a bare filename stem, so traversal and separators are rejected before any path is built. * Managed spec names and the built-in `default` are reserved, read from `agent_files.OWNED_KIRO_AGENT_FILES` rather than duplicated. * A name any EXISTING spec already answers to conflicts, not just a filename collision — kiro-cli resolves by the `name` field, so a second spec declaring a package agent's name makes which one wins a coin flip. * The file is created with `O_EXCL`. The collision scan and the write are two steps and only the kernel can make "create only if absent" atomic against a concurrent POST; the test reproduces that window rather than trusting the scan. * `prompt`/`description` edits are refused on Kiro Crew's own specs, which are rewritten on every install — accepting them would show a save that silently reverts. The `managed` flag is served by the backend so the editor disables exactly what PATCH refuses instead of keeping a second copy of the owned-file list in TypeScript. * Validation runs before any mutation, so a rejected combined PATCH cannot leave a half-applied model change behind. Every rejection carries a machine-readable `code`; the prose is advisory, since the dashboard renders it into a localized UI. UI: a New template button on the Installed Agents card and a Duplicate action on the selected template, both opening one dialog (name, start-from, description, system prompt). The detail panel's read-only prompt block becomes an editor with explicit Save/Revert — a prompt is prose typed over many keystrokes, so save-per-keystroke would rewrite a spec kiro-cli reads for every half-finished sentence. A managed template shows the same content with the reason it is locked. 33 new strings across all 10 locales plus the pseudolocale. --- src/kiro_crew/dashboard/handlers/__init__.py | 1 + src/kiro_crew/dashboard/handlers/agents.py | 375 +++++++++++- src/kiro_crew/dashboard/server.py | 1 + test/test_agent_template_create.py | 533 ++++++++++++++++++ website/src/api/client.ts | 1 + .../components/AgentTemplateCreateDialog.tsx | 142 +++++ .../src/components/AgentTemplateEditor.tsx | 135 +++++ website/src/i18n/locales/bn.json | 40 +- website/src/i18n/locales/de.json | 40 +- website/src/i18n/locales/en-XA.json | 38 +- website/src/i18n/locales/en.json | 40 +- website/src/i18n/locales/es.json | 40 +- website/src/i18n/locales/fr.json | 40 +- website/src/i18n/locales/hi.json | 40 +- website/src/i18n/locales/it.json | 40 +- website/src/i18n/locales/ja.json | 40 +- website/src/i18n/locales/pt.json | 40 +- website/src/i18n/locales/ru.json | 40 +- website/src/i18n/locales/zh-CN.json | 40 +- website/src/pages/AgentsPage.tsx | 71 ++- .../src/test/AgentTemplateAuthoring.test.tsx | 244 ++++++++ 21 files changed, 1950 insertions(+), 31 deletions(-) create mode 100644 test/test_agent_template_create.py create mode 100644 website/src/components/AgentTemplateCreateDialog.tsx create mode 100644 website/src/components/AgentTemplateEditor.tsx create mode 100644 website/src/test/AgentTemplateAuthoring.test.tsx diff --git a/src/kiro_crew/dashboard/handlers/__init__.py b/src/kiro_crew/dashboard/handlers/__init__.py index 0612b9dc3e1..4dd8b8e7b96 100644 --- a/src/kiro_crew/dashboard/handlers/__init__.py +++ b/src/kiro_crew/dashboard/handlers/__init__.py @@ -60,6 +60,7 @@ def sel(): _get_config_lock, _installed_agent_config, api_agent_config, + api_agent_create, api_agent_detail, api_agents_installed, api_capability_agents_install, diff --git a/src/kiro_crew/dashboard/handlers/agents.py b/src/kiro_crew/dashboard/handlers/agents.py index 32cd9a10551..23d89f14814 100644 --- a/src/kiro_crew/dashboard/handlers/agents.py +++ b/src/kiro_crew/dashboard/handlers/agents.py @@ -1040,6 +1040,323 @@ async def api_slash_commands(request: web.Request) -> web.Response: ) +#: A template name has to be safe as a bare filename stem (``.json`` inside +#: the agents dir) AND usable as a kiro-cli ``--agent`` argument. The character +#: class excludes the separators and ``..`` that a traversal would need, so no +#: separate traversal check is required to keep the write inside the dir — the +#: post-join containment assert below is belt and braces, not the primary guard. +_VALID_AGENT_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") + +_MAX_AGENT_NAME_LEN = 64 +_MAX_AGENT_DESCRIPTION_LEN = 500 +#: Generous by design: a system prompt is prose and some are genuinely long. +#: The cap exists so one request cannot write an unbounded file, not to express +#: an opinion about prompt length. +_MAX_AGENT_PROMPT_LEN = 100_000 + + +def _managed_agent_filenames() -> tuple[str, ...]: + """The agent spec filenames Kiro Crew itself writes and rewrites. + + Read through ``agent_files`` rather than duplicated as literals so adding a + managed spec there automatically protects it here — both from being shadowed + by a new template of the same name and from prompt/description edits that + the next install would silently revert. + """ + from kiro_crew.agent_files import OWNED_KIRO_AGENT_FILES + + return OWNED_KIRO_AGENT_FILES + + +def _reserved_agent_names() -> set[str]: + """Names a new template may not take. + + The managed specs' own stems (a template called ``kirocrew`` would either + collide with the file or shadow it by ``name``), plus ``default`` — the + built-in agent that has no config file and is special-cased by + :func:`api_agent_detail`. + """ + return {Path(f).stem for f in _managed_agent_filenames()} | {"default"} + + +#: The spec a blank new template starts from. +#: +#: ``tools`` is the surface the agent CAN reach; ``allowedTools`` is the subset +#: auto-approved without asking. The split is the whole security story of a +#: template, so a created one starts with a useful tool surface but auto-approves +#: only side-effect-free reads — an agent that can edit files or run a command +#: has to ask the first time. Kiro Crew's own privileged MCP servers +#: (``@kirocrew-core`` and friends: spawn, cron, computer use) are deliberately +#: absent; a user who wants that surface duplicates ``kirocrew`` instead of +#: starting blank, which is an explicit act rather than a default. +_BLANK_TEMPLATE_TOOLS = ( + "execute_bash", + "fs_read", + "fs_write", + "code", + "grep", + "glob", + "web_fetch", + "web_search", +) +_BLANK_TEMPLATE_ALLOWED_TOOLS = ("fs_read", "code", "grep", "glob") + + +def _blank_template_spec() -> dict[str, Any]: + """A minimal, schema-clean kiro agent spec for a from-scratch template. + + Only fields kiro-cli understands: it rejects a spec with unknown fields and + then resolves no agent at all, so the safest new spec is a small one. + """ + return { + "tools": list(_BLANK_TEMPLATE_TOOLS), + "allowedTools": list(_BLANK_TEMPLATE_ALLOWED_TOOLS), + "resources": ["file://.kiro/steering/**/*.md"], + } + + +def _validate_new_agent_name(raw: Any) -> tuple[str, str, str]: + """Return ``(name, code, message)`` for a requested template name. + + ``code`` is empty when the name is acceptable. The machine-readable code is + what the dashboard branches on; the prose is advisory (RFC 9457 3.1.3). + """ + if not isinstance(raw, str): + return "", "name_required", "name must be a string" + name = raw.strip() + if not name: + return "", "name_required", "name is required" + if len(name) > _MAX_AGENT_NAME_LEN: + return ( + "", + "name_too_long", + f"name must be at most {_MAX_AGENT_NAME_LEN} characters", + ) + if not _VALID_AGENT_NAME_RE.match(name): + return ( + "", + "invalid_agent_name", + "name may contain only letters, digits, dot, dash and underscore, " + "and must start with a letter or digit", + ) + if name in _reserved_agent_names(): + return "", "agent_name_reserved", f"'{name}' is reserved" + return name, "", "" + + +def _validate_spec_text(raw: Any, field: str, limit: int) -> tuple[str, str, str]: + """Return ``(value, code, message)`` for a free-text spec field.""" + if not isinstance(raw, str): + return "", f"{field}_invalid", f"{field} must be a string" + if len(raw) > limit: + return "", f"{field}_too_long", f"{field} must be at most {limit} characters" + return raw, "", "" + + +def _read_agent_specs(agents_dir: Path) -> list[tuple[Path, dict[str, Any]]]: + """Every parseable spec in *agents_dir*, as ``(path, data)`` pairs. + + Unreadable and non-object files are skipped rather than raising: the dir is + shared with other tools, and one bad file must not block creating a template. + """ + out: list[tuple[Path, dict[str, Any]]] = [] + try: + candidates = sorted(agents_dir.glob("*.json")) + except OSError: + return out + for path in candidates: + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + continue + if isinstance(data, dict): + out.append((path, data)) + return out + + +def _clone_source_spec( + specs: list[tuple[Path, dict[str, Any]]], source: str +) -> dict[str, Any] | None: + """The spec to copy for a ``from`` request, matched the way kiro-cli resolves. + + Both the declared ``name`` and the file stem are accepted because a + package-installed agent's file is ``-.json`` while its + identity is the ``name`` field. + """ + for path, data in specs: + if spec_str(data, "name") == source or path.stem == source: + return data + return None + + +async def api_agent_create(request: web.Request) -> web.Response: + """POST /api/agents/detail — create a new agent template. + + Writes ``.json`` into the agents dir, either from a conservative blank + baseline or as a copy of an existing template (``from``). + + ``tools``, ``allowedTools`` and ``toolsSettings`` are deliberately NOT + accepted from the request body. They are the privilege surface — the + auto-approve list and the bash deny patterns — so a create call cannot mint + a spec that auto-approves everything. A copy inherits them from a spec that + already exists on disk; a blank template gets the read-only baseline. Editing + them is a separate, deliberate capability this endpoint does not grant. + """ + from kiro_crew.agent import kiro_agents_dir_path # noqa: F811 + + try: + body = await request.json() + except (json.JSONDecodeError, ValueError): + return web.json_response({"error": "invalid JSON", "code": "invalid_json"}, status=400) + if not isinstance(body, dict): + return web.json_response( + {"error": "body must be a JSON object", "code": "invalid_body"}, status=400 + ) + + def _denied(code: str, message: str) -> web.Response: + """A 400 that is also recorded — a rejected create is worth an audit line.""" + _sel().log_api_access( + caller=request.get("user", "dashboard"), + operation="agent_template.create", + outcome="denied", + source="dashboard", + resources=str(body.get("name", ""))[:_MAX_AGENT_NAME_LEN], + error=code, + ) + return web.json_response({"error": message, "code": code}, status=400) + + name, code, message = _validate_new_agent_name(body.get("name")) + if code: + return _denied(code, message) + description, code, message = _validate_spec_text( + body.get("description", ""), "description", _MAX_AGENT_DESCRIPTION_LEN + ) + if code: + return _denied(code, message) + prompt, code, message = _validate_spec_text( + body.get("prompt", ""), "prompt", _MAX_AGENT_PROMPT_LEN + ) + if code: + return _denied(code, message) + raw_from = body.get("from", "") + if not isinstance(raw_from, str): + return _denied("invalid_from", "from must be a string") + clone_from = raw_from.strip() + + state: DashboardState = request.app["state"] + agents_dir = kiro_agents_dir_path() + target = agents_dir / f"{name}.json" + # The name regex already excludes every separator, so this cannot fail; it is + # here so a future loosening of the regex trips this instead of writing + # outside the agents dir. + if target.parent.resolve() != agents_dir.resolve(): + return _denied("invalid_agent_name", "invalid name") + + async with _get_config_lock(): + specs = await asyncio.to_thread(_read_agent_specs, agents_dir) + # Reject a name any EXISTING spec already answers to, not just a file-name + # collision: kiro-cli resolves by the ``name`` field, so a second spec + # declaring a package agent's name makes which one wins a coin flip. + for path, data in specs: + if spec_str(data, "name") == name or path.stem == name: + _sel().log_api_access( + caller=request.get("user", "dashboard"), + operation="agent_template.create", + outcome="denied", + source="dashboard", + resources=name, + error="agent_template_exists", + ) + return web.json_response( + {"error": f"'{name}' already exists", "code": "agent_template_exists"}, + status=409, + ) + + if clone_from: + source = _clone_source_spec(specs, clone_from) + if source is None: + return web.json_response( + { + "error": f"template '{clone_from}' not found", + "code": "source_template_not_found", + }, + status=404, + ) + spec: dict[str, Any] = json.loads(json.dumps(source)) + else: + spec = _blank_template_spec() + spec["name"] = name + if description: + spec["description"] = description + else: + spec.pop("description", None) + if prompt: + spec["prompt"] = prompt + elif not clone_from: + spec.pop("prompt", None) + # Kiro Crew bookkeeping that older specs may carry; kiro-cli rejects + # unknown fields and drops the whole agent, so a copy must not inherit it. + spec.pop("model_managed", None) + spec.pop("cc_model", None) + + payload = json.dumps(spec, indent=2) + "\n" + + def _write() -> str: + """Create the file exclusively; returns an error code or ''.""" + agents_dir.mkdir(parents=True, exist_ok=True) + try: + # O_EXCL, not a write after the collision scan above: the scan and + # the write are two steps, and the kernel is the only party that + # can make "create only if absent" atomic against a concurrent + # POST or another tool writing the same path. + fd = os.open(target, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + except FileExistsError: + return "agent_template_exists" + except OSError as exc: + logger.warning("Failed to create agent template %s: %s", target, exc) + return "agent_template_write_failed" + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fh.write(payload) + except OSError as exc: + logger.warning("Failed to write agent template %s: %s", target, exc) + target.unlink(missing_ok=True) + return "agent_template_write_failed" + return "" + + write_code = await asyncio.to_thread(_write) + + if write_code: + _sel().log_api_access( + caller=request.get("user", "dashboard"), + operation="agent_template.create", + outcome="failure", + source="dashboard", + resources=name, + error=write_code, + ) + if write_code == "agent_template_exists": + return web.json_response( + {"error": f"'{name}' already exists", "code": write_code}, status=409 + ) + return web.json_response( + {"error": "could not write agent template", "code": write_code}, status=500 + ) + + # The list_agents() cache keys on a (count, newest-mtime-ns) signature, which + # a write inside the current mtime granularity would not move. + clear_list_agents_cache() + state.push_refresh("agents") + _sel().log_api_access( + caller=request.get("user", "dashboard"), + operation="agent_template.create", + outcome="success", + source="dashboard", + resources=f"{name} (from={clone_from})" if clone_from else name, + ) + return web.json_response({"ok": True, "name": name}) + + async def api_agent_detail(request: web.Request) -> web.Response: """GET/DELETE/PATCH /api/agents/detail/{name} — view, delete, or update agent config.""" name = request.match_info["name"] @@ -1089,6 +1406,38 @@ async def api_agent_detail(request: web.Request) -> web.Response: {"error": f"at most {MAX_AGENT_SKILLS} skills per agent"}, status=400, ) + # ``prompt`` and ``description`` are validated here, with + # ``skills``, so a bad value is rejected before any state + # mutation rather than half-applying a combined PATCH. + text_edits: dict[str, str] = {} + for field, limit in ( + ("prompt", _MAX_AGENT_PROMPT_LEN), + ("description", _MAX_AGENT_DESCRIPTION_LEN), + ): + if field not in patch_body: + continue + if f.name in _managed_agent_filenames(): + # Both fields are rewritten from the shipped defaults + # on every install and at boot self-heal, so accepting + # the edit would show a save that silently reverts. + return web.json_response( + { + "error": ( + f"{f.stem} is managed by Kiro Crew; its " + f"{field} cannot be edited" + ), + "code": "agent_template_managed", + }, + status=400, + ) + value, code, message = _validate_spec_text( + patch_body[field], field, limit + ) + if code: + return web.json_response( + {"error": message, "code": code}, status=400 + ) + text_edits[field] = value mapped: list[str] = [] loop = asyncio.get_running_loop() async with _get_config_lock(): @@ -1141,6 +1490,14 @@ async def api_agent_detail(request: web.Request) -> web.Response: # kiro-cli rejects unknown fields and drops the agent. data.pop("model_managed", None) data.pop("cc_model", None) + # An empty string means "no value", not the literal "": + # the key is dropped so the spec stays minimal and reads + # back through spec_str() as absent. + for field, value in text_edits.items(): + if value: + data[field] = value + else: + data.pop(field, None) f.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") # The list_agents() cache keys on a (count, newest-mtime-ns) # signature; two writes inside the same mtime granularity @@ -1148,7 +1505,13 @@ async def api_agent_detail(request: web.Request) -> web.Response: clear_list_agents_cache() state.push_refresh("agents") return web.json_response( - {"ok": True, "model": data.get("model", ""), "skills": mapped} + { + "ok": True, + "model": data.get("model", ""), + "skills": mapped, + "prompt": spec_str(data, "prompt"), + "description": spec_str(data, "description"), + } ) # ``skills`` / ``unmanaged_skills`` are computed, response-only # views of ``resources`` — never written back into the spec @@ -1170,6 +1533,12 @@ async def api_agent_detail(request: web.Request) -> web.Response: "model": spec_model(data), "skills": keys, "unmanaged_skills": unmanaged_uris, + # Whether Kiro Crew owns this spec and rewrites it on + # install. Served rather than re-derived client-side so + # the editor disables exactly the fields PATCH refuses, + # instead of keeping a second copy of the owned-file list + # in TypeScript that would drift from agent_files.py. + "managed": f.name in _managed_agent_filenames(), } ) except (json.JSONDecodeError, OSError): @@ -1178,7 +1547,9 @@ async def api_agent_detail(request: web.Request) -> web.Response: if name == "default": if request.method != "GET": return web.json_response({"error": "cannot modify built-in default agent"}, status=400) - return web.json_response({"name": "default", "model": ""}) + # No file to edit, so it is managed by definition — the editor must not + # offer fields whose PATCH the branch above already refuses. + return web.json_response({"name": "default", "model": "", "managed": True}) return web.json_response({"error": "not found"}, status=404) diff --git a/src/kiro_crew/dashboard/server.py b/src/kiro_crew/dashboard/server.py index 2d9c6c66dc7..eeba7e990f0 100644 --- a/src/kiro_crew/dashboard/server.py +++ b/src/kiro_crew/dashboard/server.py @@ -2335,6 +2335,7 @@ async def _wf_nudge_authorizer( app.router.add_get("/api/agents/detail/{name}", handlers.api_agent_detail) app.router.add_patch("/api/agents/detail/{name}", handlers.api_agent_detail) app.router.add_delete("/api/agents/detail/{name}", handlers.api_agent_detail) + app.router.add_post("/api/agents/detail", handlers.api_agent_create) # KiroCrew Agent CRUD app.router.add_get("/api/agents", handlers.api_kirocrew_agents) app.router.add_get( diff --git a/test/test_agent_template_create.py b/test/test_agent_template_create.py new file mode 100644 index 00000000000..8b252aaef1f --- /dev/null +++ b/test/test_agent_template_create.py @@ -0,0 +1,533 @@ +"""Tests for authoring agent templates from the dashboard. + +Two capabilities, both previously absent — the agents dir could only be written +by a package install, an app, or by hand: + +* CREATE — ``POST /api/agents/detail`` writes ``.json``, either from a + conservative blank baseline or as a copy of an existing template. +* EDIT — ``PATCH /api/agents/detail/{name}`` accepts ``prompt`` and + ``description``, so a created template can be corrected without hand-editing + JSON. + +The privilege surface (``tools`` / ``allowedTools`` / ``toolsSettings``) is +deliberately not writable through either verb; the tests below pin that. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest +from aiohttp import web + +from kiro_crew.agent_discovery import clear_list_agents_cache +from kiro_crew.dashboard.handlers.agents import ( + _BLANK_TEMPLATE_ALLOWED_TOOLS, + api_agent_create, + api_agent_detail, +) + + +@pytest.fixture(autouse=True) +def _no_agent_cache(): + clear_list_agents_cache() + yield + clear_list_agents_cache() + + +def _request(method: str, body: dict | list | None = None, name: str = "") -> MagicMock: + request = MagicMock(spec=web.Request) + request.method = method + request.match_info = {"name": name} + request.app = {"state": MagicMock()} + request.get = lambda key, default=None: default + + async def _json(): + if body is None: + raise json.JSONDecodeError("no body", "", 0) + return body + + request.json = _json + return request + + +async def _create(agents_dir: Path, body: dict | list | None) -> web.Response: + with patch("kiro_crew.agent.KIRO_AGENTS_DIR", agents_dir): + return await api_agent_create(_request("POST", body)) + + +async def _patch(agents_dir: Path, name: str, body: dict) -> web.Response: + with patch("kiro_crew.agent.KIRO_AGENTS_DIR", agents_dir): + return await api_agent_detail(_request("PATCH", body, name=name)) + + +async def _get(agents_dir: Path, name: str) -> web.Response: + with patch("kiro_crew.agent.KIRO_AGENTS_DIR", agents_dir): + return await api_agent_detail(_request("GET", None, name=name)) + + +def _spec(resp: web.Response) -> dict: + return json.loads(resp.body.decode("utf-8")) + + +@pytest.fixture +def agents_dir(tmp_path: Path) -> Path: + d = tmp_path / "agents" + d.mkdir() + return d + + +# ── CREATE: the blank baseline ── + + +class TestCreateBlank: + @pytest.mark.asyncio + async def test_writes_a_named_spec(self, agents_dir): + resp = await _create(agents_dir, {"name": "researcher"}) + + assert resp.status == 200 + written = json.loads((agents_dir / "researcher.json").read_text(encoding="utf-8")) + assert written["name"] == "researcher" + + @pytest.mark.asyncio + async def test_auto_approves_only_read_only_tools(self, agents_dir): + """The blank baseline's whole security story: a useful tool surface, but + nothing with a side effect is pre-approved.""" + await _create(agents_dir, {"name": "researcher"}) + + written = json.loads((agents_dir / "researcher.json").read_text(encoding="utf-8")) + assert set(written["allowedTools"]) == set(_BLANK_TEMPLATE_ALLOWED_TOOLS) + assert "execute_bash" in written["tools"] + assert "execute_bash" not in written["allowedTools"] + assert "fs_write" not in written["allowedTools"] + + @pytest.mark.asyncio + async def test_omits_kirocrew_mcp_servers(self, agents_dir): + """Kiro Crew's own MCP surface (spawn / cron / computer use) is opt-in via + a copy of ``kirocrew``, never a blank template's default.""" + await _create(agents_dir, {"name": "researcher"}) + + written = json.loads((agents_dir / "researcher.json").read_text(encoding="utf-8")) + assert not [t for t in written["tools"] if t.startswith("@kirocrew")] + + @pytest.mark.asyncio + async def test_description_and_prompt_are_stored(self, agents_dir): + await _create( + agents_dir, + {"name": "researcher", "description": "Digs through papers", "prompt": "Be rigorous."}, + ) + + written = json.loads((agents_dir / "researcher.json").read_text(encoding="utf-8")) + assert written["description"] == "Digs through papers" + assert written["prompt"] == "Be rigorous." + + @pytest.mark.asyncio + async def test_absent_optional_fields_are_omitted_not_empty(self, agents_dir): + """kiro-cli reads the spec; an empty-string prompt is not the same as no + prompt, and a minimal spec is the one least likely to be rejected.""" + await _create(agents_dir, {"name": "researcher"}) + + written = json.loads((agents_dir / "researcher.json").read_text(encoding="utf-8")) + assert "prompt" not in written + assert "description" not in written + + @pytest.mark.asyncio + async def test_tools_in_the_body_are_ignored(self, agents_dir): + """The privilege surface is not settable through create — otherwise one + call could mint a template that auto-approves everything.""" + await _create( + agents_dir, + { + "name": "researcher", + "tools": ["execute_bash"], + "allowedTools": ["execute_bash", "fs_write"], + "toolsSettings": {"execute_bash": {"deniedCommands": []}}, + }, + ) + + written = json.loads((agents_dir / "researcher.json").read_text(encoding="utf-8")) + assert "execute_bash" not in written["allowedTools"] + assert "toolsSettings" not in written + + +# ── CREATE: name validation ── + + +class TestCreateNameValidation: + @pytest.mark.asyncio + async def test_missing_name_is_rejected(self, agents_dir): + resp = await _create(agents_dir, {}) + assert resp.status == 400 + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "name", + [ + "../escape", + "sub/dir", + "back\\slash", + "..", + ".hidden", + "has space", + "-leading-dash", + "sym*bol", + ], + ) + async def test_unsafe_names_are_rejected(self, agents_dir, name): + resp = await _create(agents_dir, {"name": name}) + + assert resp.status == 400 + assert list(agents_dir.iterdir()) == [] + + @pytest.mark.asyncio + async def test_traversal_writes_nothing_outside_the_agents_dir(self, agents_dir, tmp_path): + await _create(agents_dir, {"name": "../../pwned"}) + + assert not (tmp_path / "pwned.json").exists() + assert not (tmp_path.parent / "pwned.json").exists() + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "name", ["kirocrew", "kirocrew-lite", "kirocrew-knowledge", "default"] + ) + async def test_managed_and_builtin_names_are_reserved(self, agents_dir, name): + resp = await _create(agents_dir, {"name": name}) + + assert resp.status == 400 + assert not (agents_dir / f"{name}.json").exists() + + @pytest.mark.asyncio + async def test_overlong_name_is_rejected(self, agents_dir): + resp = await _create(agents_dir, {"name": "a" * 65}) + assert resp.status == 400 + + @pytest.mark.asyncio + async def test_non_string_name_is_rejected(self, agents_dir): + resp = await _create(agents_dir, {"name": {"id": "x"}}) + assert resp.status == 400 + + @pytest.mark.asyncio + async def test_non_object_body_is_rejected(self, agents_dir): + resp = await _create(agents_dir, ["name"]) + assert resp.status == 400 + + @pytest.mark.asyncio + async def test_invalid_json_is_rejected(self, agents_dir): + resp = await _create(agents_dir, None) + assert resp.status == 400 + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "body,code", + [ + ({}, "name_required"), + ({"name": "../escape"}, "invalid_agent_name"), + ({"name": "kirocrew"}, "agent_name_reserved"), + ({"name": "a" * 65}, "name_too_long"), + ({"name": "ok", "prompt": {"bad": 1}}, "prompt_invalid"), + ({"name": "ok", "description": "d" * 501}, "description_too_long"), + ({"name": "ok", "from": 7}, "invalid_from"), + ], + ) + async def test_every_rejection_carries_a_machine_readable_code( + self, agents_dir, body, code + ): + """The dashboard branches on ``code``; the prose is advisory and localized + away, so an un-coded rejection is untranslatable by construction.""" + resp = await _create(agents_dir, body) + + assert resp.status == 400 + assert _spec(resp)["code"] == code + + +# ── CREATE: collisions ── + + +class TestCreateCollisions: + @pytest.mark.asyncio + async def test_existing_filename_conflicts(self, agents_dir): + (agents_dir / "researcher.json").write_text('{"name": "researcher"}', encoding="utf-8") + + resp = await _create(agents_dir, {"name": "researcher"}) + assert resp.status == 409 + assert _spec(resp)["code"] == "agent_template_exists" + + @pytest.mark.asyncio + async def test_name_claimed_by_a_package_spec_conflicts(self, agents_dir): + """kiro-cli resolves an agent by its ``name`` field, so a second spec + declaring a package agent's name makes which one wins a coin flip — the + filename being free is not enough.""" + (agents_dir / "somepkg-reviewer.json").write_text( + '{"name": "reviewer"}', encoding="utf-8" + ) + + resp = await _create(agents_dir, {"name": "reviewer"}) + + assert resp.status == 409 + assert not (agents_dir / "reviewer.json").exists() + + @pytest.mark.asyncio + async def test_an_unparseable_neighbour_does_not_block_creation(self, agents_dir): + (agents_dir / "broken.json").write_text("{not json", encoding="utf-8") + + resp = await _create(agents_dir, {"name": "researcher"}) + assert resp.status == 200 + + @pytest.mark.asyncio + async def test_existing_file_is_never_overwritten(self, agents_dir): + (agents_dir / "researcher.json").write_text( + '{"name": "researcher", "prompt": "original"}', encoding="utf-8" + ) + + await _create(agents_dir, {"name": "researcher", "prompt": "replacement"}) + + kept = json.loads((agents_dir / "researcher.json").read_text(encoding="utf-8")) + assert kept["prompt"] == "original" + + @pytest.mark.asyncio + async def test_a_file_appearing_after_the_scan_is_not_clobbered(self, agents_dir): + """The TOCTOU window the exclusive create closes. + + Reading the dir and writing the file are two steps. A concurrent POST for + the same name — or another tool writing the same path — lands in between, + and only the kernel can make "create only if absent" atomic. Stubbing the + scan to report an empty dir is exactly what the loser of that race sees, + so this reaches the write with the file already present. + """ + victim = agents_dir / "researcher.json" + victim.write_text('{"name": "researcher", "prompt": "original"}', encoding="utf-8") + + with patch( + "kiro_crew.dashboard.handlers.agents._read_agent_specs", return_value=[] + ): + resp = await _create(agents_dir, {"name": "researcher", "prompt": "replacement"}) + + assert resp.status == 409 + assert json.loads(victim.read_text(encoding="utf-8"))["prompt"] == "original" + + +# ── CREATE: copying an existing template ── + + +class TestCreateFromCopy: + @pytest.fixture + def source(self, agents_dir) -> Path: + path = agents_dir / "base.json" + path.write_text( + json.dumps( + { + "name": "base", + "description": "the original", + "prompt": "original prompt", + "model": "some-model", + "tools": ["execute_bash", "fs_read", "@kirocrew-core"], + "allowedTools": ["fs_read", "@kirocrew-core"], + "mcpServers": {"kirocrew-core": {"command": "mcp", "args": []}}, + "resources": ["skill://~/.kiro/skills/babysit/SKILL.md"], + "toolsSettings": {"execute_bash": {"deniedCommands": ["rm -rf /*"]}}, + } + ), + encoding="utf-8", + ) + return path + + @pytest.mark.asyncio + async def test_copy_inherits_the_privilege_surface(self, agents_dir, source): + """A copy is how a user gets a privileged tool surface: inherited from a + spec that already exists on disk, not assembled from a request body.""" + await _create(agents_dir, {"name": "clone", "from": "base"}) + + written = json.loads((agents_dir / "clone.json").read_text(encoding="utf-8")) + assert written["tools"] == ["execute_bash", "fs_read", "@kirocrew-core"] + assert written["allowedTools"] == ["fs_read", "@kirocrew-core"] + assert written["toolsSettings"]["execute_bash"]["deniedCommands"] == ["rm -rf /*"] + assert written["mcpServers"] == {"kirocrew-core": {"command": "mcp", "args": []}} + assert written["resources"] == ["skill://~/.kiro/skills/babysit/SKILL.md"] + + @pytest.mark.asyncio + async def test_copy_takes_the_new_name(self, agents_dir, source): + await _create(agents_dir, {"name": "clone", "from": "base"}) + + written = json.loads((agents_dir / "clone.json").read_text(encoding="utf-8")) + assert written["name"] == "clone" + + @pytest.mark.asyncio + async def test_supplied_prompt_overrides_the_copied_one(self, agents_dir, source): + await _create(agents_dir, {"name": "clone", "from": "base", "prompt": "mine"}) + + written = json.loads((agents_dir / "clone.json").read_text(encoding="utf-8")) + assert written["prompt"] == "mine" + + @pytest.mark.asyncio + async def test_source_is_left_untouched(self, agents_dir, source): + await _create(agents_dir, {"name": "clone", "from": "base", "prompt": "mine"}) + + original = json.loads(source.read_text(encoding="utf-8")) + assert original["name"] == "base" + assert original["prompt"] == "original prompt" + + @pytest.mark.asyncio + async def test_copy_resolves_a_package_spec_by_its_name_field(self, agents_dir): + (agents_dir / "somepkg-reviewer.json").write_text( + json.dumps({"name": "reviewer", "tools": ["fs_read"]}), encoding="utf-8" + ) + + resp = await _create(agents_dir, {"name": "myreviewer", "from": "reviewer"}) + + assert resp.status == 200 + written = json.loads((agents_dir / "myreviewer.json").read_text(encoding="utf-8")) + assert written["tools"] == ["fs_read"] + + @pytest.mark.asyncio + async def test_unknown_source_is_a_404(self, agents_dir): + resp = await _create(agents_dir, {"name": "clone", "from": "nope"}) + + assert resp.status == 404 + assert _spec(resp)["code"] == "source_template_not_found" + assert not (agents_dir / "clone.json").exists() + + @pytest.mark.asyncio + async def test_bookkeeping_keys_are_not_inherited(self, agents_dir): + """kiro-cli rejects unknown fields and then resolves no agent at all, so a + copy of an older spec must not carry Kiro Crew's sidecar keys.""" + (agents_dir / "legacy.json").write_text( + json.dumps({"name": "legacy", "model_managed": True, "cc_model": "x"}), + encoding="utf-8", + ) + + await _create(agents_dir, {"name": "clone", "from": "legacy"}) + + written = json.loads((agents_dir / "clone.json").read_text(encoding="utf-8")) + assert "model_managed" not in written + assert "cc_model" not in written + + +# ── EDIT: prompt and description ── + + +class TestPatchText: + @pytest.fixture + def template(self, agents_dir) -> Path: + path = agents_dir / "researcher.json" + path.write_text( + json.dumps({"name": "researcher", "prompt": "old", "description": "old desc"}), + encoding="utf-8", + ) + return path + + @pytest.mark.asyncio + async def test_prompt_is_written(self, agents_dir, template): + resp = await _patch(agents_dir, "researcher", {"prompt": "new prompt"}) + + assert resp.status == 200 + assert json.loads(template.read_text(encoding="utf-8"))["prompt"] == "new prompt" + assert _spec(resp)["prompt"] == "new prompt" + + @pytest.mark.asyncio + async def test_description_is_written(self, agents_dir, template): + resp = await _patch(agents_dir, "researcher", {"description": "new desc"}) + + assert resp.status == 200 + assert json.loads(template.read_text(encoding="utf-8"))["description"] == "new desc" + + @pytest.mark.asyncio + async def test_empty_value_drops_the_key(self, agents_dir, template): + await _patch(agents_dir, "researcher", {"prompt": ""}) + + assert "prompt" not in json.loads(template.read_text(encoding="utf-8")) + + @pytest.mark.asyncio + async def test_a_file_prompt_can_be_replaced_with_inline_text(self, agents_dir): + """A copied template inherits ``prompt: file://…`` from its source; the + editor has to be able to detach it.""" + path = agents_dir / "clone.json" + path.write_text( + json.dumps({"name": "clone", "prompt": "file:///somewhere/prompt.md"}), + encoding="utf-8", + ) + + await _patch(agents_dir, "clone", {"prompt": "inline now"}) + + assert json.loads(path.read_text(encoding="utf-8"))["prompt"] == "inline now" + + @pytest.mark.asyncio + async def test_non_string_prompt_is_rejected(self, agents_dir, template): + resp = await _patch(agents_dir, "researcher", {"prompt": {"file": "x"}}) + + assert resp.status == 400 + assert json.loads(template.read_text(encoding="utf-8"))["prompt"] == "old" + + @pytest.mark.asyncio + async def test_overlong_prompt_is_rejected(self, agents_dir, template): + resp = await _patch(agents_dir, "researcher", {"prompt": "x" * 100_001}) + + assert resp.status == 400 + assert json.loads(template.read_text(encoding="utf-8"))["prompt"] == "old" + + @pytest.mark.asyncio + async def test_a_rejected_combined_patch_applies_nothing(self, agents_dir, template): + """Validation runs before any mutation, so a bad prompt cannot leave a + half-applied model change behind.""" + resp = await _patch( + agents_dir, "researcher", {"model": "new-model", "prompt": {"bad": 1}} + ) + + assert resp.status == 400 + written = json.loads(template.read_text(encoding="utf-8")) + assert "model" not in written + assert written["prompt"] == "old" + + @pytest.mark.asyncio + @pytest.mark.parametrize("field", ["prompt", "description"]) + async def test_managed_specs_refuse_the_edit(self, agents_dir, field): + """Kiro Crew rewrites these fields on every install, so accepting the edit + would show the user a save that silently reverts.""" + path = agents_dir / "kirocrew.json" + path.write_text( + json.dumps({"name": "kirocrew", "prompt": "file://p.md", "description": "d"}), + encoding="utf-8", + ) + + resp = await _patch(agents_dir, "kirocrew", {field: "hijacked"}) + + assert resp.status == 400 + assert _spec(resp)["code"] == "agent_template_managed" + assert json.loads(path.read_text(encoding="utf-8"))[field] != "hijacked" + + @pytest.mark.asyncio + async def test_managed_specs_still_accept_a_model_patch(self, agents_dir): + """Only the rewritten fields are refused; the model pin is a real sidecar + -backed setting and must keep working.""" + path = agents_dir / "kirocrew.json" + path.write_text(json.dumps({"name": "kirocrew"}), encoding="utf-8") + + resp = await _patch(agents_dir, "kirocrew", {"model": "some-model"}) + + assert resp.status == 200 + assert json.loads(path.read_text(encoding="utf-8"))["model"] == "some-model" + + +# ── The managed flag the editor gates on ── + + +class TestManagedFlag: + @pytest.mark.asyncio + async def test_a_created_template_is_not_managed(self, agents_dir): + await _create(agents_dir, {"name": "researcher"}) + + resp = await _get(agents_dir, "researcher") + assert _spec(resp)["managed"] is False + + @pytest.mark.asyncio + async def test_a_kirocrew_owned_spec_is_managed(self, agents_dir): + (agents_dir / "kirocrew.json").write_text('{"name": "kirocrew"}', encoding="utf-8") + + resp = await _get(agents_dir, "kirocrew") + assert _spec(resp)["managed"] is True + + @pytest.mark.asyncio + async def test_the_builtin_default_is_managed(self, agents_dir): + resp = await _get(agents_dir, "default") + assert _spec(resp)["managed"] is True diff --git a/website/src/api/client.ts b/website/src/api/client.ts index 4c5ba5156d7..aa7336fd33a 100644 --- a/website/src/api/client.ts +++ b/website/src/api/client.ts @@ -1219,6 +1219,7 @@ export const api = { // Agents agentsInstalled: () => fetch('/api/agents/installed').then(j), agentDetail: (name: string) => fetch('/api/agents/detail/' + encodeURIComponent(name)).then(j), + agentCreate: (body: object) => fetch('/api/agents/detail', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) }).then(j), agentPatch: (name: string, body: object) => fetch('/api/agents/detail/' + encodeURIComponent(name), { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) }).then(j), agentDelete: (name: string) => fetch('/api/agents/detail/' + encodeURIComponent(name), { method: 'DELETE' }).then(j), // KiroCrew agents diff --git a/website/src/components/AgentTemplateCreateDialog.tsx b/website/src/components/AgentTemplateCreateDialog.tsx new file mode 100644 index 00000000000..0c76c866852 --- /dev/null +++ b/website/src/components/AgentTemplateCreateDialog.tsx @@ -0,0 +1,142 @@ +import { useEffect, useState } from 'react' +import { api } from '../api/client' +import { Btn, SendBtn, Input } from './ui' +import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogBody, DialogFooter } from './ui/dialog' +import SimpleSelect from './SimpleSelect' +import InfoTip from './InfoTip' + +import { i18nT } from '../i18n/t' +/** + * The one authoring surface for a kiro agent template (`~/.kiro/agents/.json`). + * + * Both entry points share it: "New template" opens it with no source, and + * "Duplicate" opens it with `initialFrom` set. They differ only in that one + * field, and a second component for the copy case would have to keep the same + * name validation, the same conflict messages, and the same tool-surface + * explainer in sync. + * + * The dialog deliberately does NOT offer `tools` / `allowedTools` / + * `deniedCommands`. Those are the privilege surface — the auto-approve list and + * the bash deny patterns — and the create endpoint refuses them from a request + * body for that reason. A blank template gets a read-only auto-approve baseline; + * copying a template inherits whatever that one was already trusted with. + */ +export default function AgentTemplateCreateDialog({ + open, + templates, + initialFrom, + onClose, + onCreated, +}: { + open: boolean + /** Names of installed templates offered as a starting point. */ + templates: string[] + /** Preselected source, for the Duplicate entry point. */ + initialFrom?: string + onClose: () => void + onCreated: (name: string) => void +}) { + const [name, setName] = useState('') + const [description, setDescription] = useState('') + const [prompt, setPrompt] = useState('') + const [from, setFrom] = useState(initialFrom || '') + const [error, setError] = useState('') + const [submitting, setSubmitting] = useState(false) + + // Reset on each open so a dismissed attempt does not resurface half-typed, + // and so Duplicate always reflects the row the user actually clicked. + useEffect(() => { + if (!open) return + setName(initialFrom ? `${initialFrom}-copy` : '') + setDescription('') + setPrompt('') + setFrom(initialFrom || '') + setError('') + setSubmitting(false) + }, [open, initialFrom]) + + const submit = async () => { + setError('') + const n = name.trim() + if (!n) { setError(i18nT('components.agentTemplateCreateDialog.name_is_required')); return } + setSubmitting(true) + try { + const body: Record = { name: n } + if (description.trim()) body.description = description.trim() + if (prompt.trim()) body.prompt = prompt + if (from) body.from = from + const r: { ok?: boolean; name?: string; error?: string } = await api.agentCreate(body) + // The server owns the rules the form cannot check (reserved names, a name + // already claimed by a package spec), so its message is shown verbatim + // rather than replaced with a guess. + if (r.error) { setError(r.error); setSubmitting(false); return } + onCreated(r.name || n) + } catch (e) { + setError(e instanceof Error ? e.message : i18nT('components.agentTemplateCreateDialog.failed_to_create_template')) + } finally { + setSubmitting(false) + } + } + + return ( + { if (!next) onClose() }}> + + + {initialFrom ? i18nT('components.agentTemplateCreateDialog.duplicate_agent_template') : i18nT('components.agentTemplateCreateDialog.new_agent_template')} + + +
+
+
+ {/* Native input associated via htmlFor+id; label-has-for's nesting requirement is a false positive. */} + {/* eslint-disable-next-line jsx-a11y/label-has-for */} + + +
+ setName(e.target.value)} autoFocus /> +
+
+
+ {i18nT('components.agentTemplateCreateDialog.start_from')} + +
+ +
+
+ {/* eslint-disable-next-line jsx-a11y/label-has-for */} + + setDescription(e.target.value)} /> +
+
+
+ {/* eslint-disable-next-line jsx-a11y/label-has-for */} + + +
+