feat(agents): add Agent Template authoring — create, edit, clone dialog + security predicates (#2255)

[qh2244]

Problem / Motivation

Custom agent templates could only be created by hand-editing JSON under ~/.kiro/crew/agents/. The dashboard listed templates and let you pick one, but offered no way to author or change one — so a user who wanted an agent with a particular model, prompt, tool set, or MCP server had to leave the UI, find the directory, learn the schema, and get the file right by hand. A malformed field there is not reported in the UI: kiro-cli rejects the whole spec and the session silently falls back to the default agent.

Why it matters

Authoring an agent is the main way a user shapes KiroCrew to their own work, and it was the one part of the roster the dashboard could not do. The JSON route also puts the burden of the security-relevant fields on the user: allowedTools grants auto-approval, mcpServers.*.env is where a literal API token ends up pasted, and resources globs can be pointed at ~/.aws or ~/.ssh. Doing this in the UI is what makes those fields screenable at a single chokepoint rather than trusted because a human typed them.

What changed (motivation → approach → change)

Goal: create, edit, and clone agent templates from the roster, with the schema and the security rules enforced server-side rather than assumed of the author.

Approach. Two endpoints (POST /api/agents/installed, PUT /api/agents/installed/{name}) behind one validation path, plus a structured dialog that never asks the user to type JSON. Two design decisions carried most of the weight:

• Absent means preserve, present means replace. A PUT replaces the keys it carries and leaves out what it omits. The dialog then sends only the fields the user actually changed, so an edit cannot clobber a concurrent external edit to a field this user never touched — and, combined with a version check, a failed detail fetch cannot erase stored config by echoing empty fields back.
• One chokepoint per decision, not one per call site. Validation, credential screening, path screening, and bookkeeping strip all happen at single points (_persist_spec, _screen_both_bases, a whole-entry MCP sweep) rather than at each endpoint, because the recurring bug class in review was a rule applied to the site a finding named while its siblings kept the hole.

What was built. Backend: the two endpoints with owner authorization, SEL-audited denials, name validation against both the shared agent grammar and the template grammar, model validation against advertised ids, credential screening over whole MCP entries (env, headers, url, args, command), path-sensitivity screening of resources globs against both the project and HOME bases, an exactly-one-transport rule per MCP server, and an optimistic-concurrency check that returns 409 agent_template_conflict rather than losing a write. Writes stage to a temp file and publish with os.link, so a create is atomic and fails if the name already exists. Frontend: AgentTemplateCreator.tsx (create/edit/clone, tool chips with per-tool auto-approve, MCP rows, skill catalog), Create in the roster header, Edit/Clone in the inspector, and agentCreate/agentUpdate in client.ts. Security predicates that both sides need live in kiro_crew/security.py.

Tests

• test/test_agent_template_authoring.py — 190 tests. The write surface end to end: validation per field, absent-means-preserve on update, the 409 conflict path and the version token's sensitivity to a same-size same-mtime change, owner authorization on both endpoints, atomic publish and the refusal to publish truncated JSON when hard links are unavailable, credential screening scoped to values a request changes, relative-glob traversal against both bases, and the exactly-one-MCP-transport rule.
• test/test_mcp_env_credential_screen.py — 84 tests. The credential predicates: token-split key matching, literal-value shapes, ${VAR} reference allowance, and the metadata-suffix exemptions.
• website/src/test/AgentTemplateSubmitPayload.test.tsx — 17 tests. What the dialog actually sends: unchanged fields omitted, changed fields sent, a cleared field sent (clearing is a change) while a never-loaded empty field stays omitted, unmodelled MCP fields and space-containing argv surviving a round trip, and a clone carrying resources explicitly because POST has nothing to preserve against.

Each fix in review was revert-verified — the fix patched back out to confirm the paired test fails — because several tests initially passed for the wrong reason (an exception the old path also raised; a remount that reset the state under test).

Manual verification

Pod screenshot verification of the create, edit, and clone flows (below). The screenshots need a refresh: they predate the MCP transport field and the 320px stacking added during review, so they show the dialog one revision behind. Re-capturing before merge.

Screenshots

Inspector with Edit / Clone buttons (user-owned template selected)

Edit dialog — name disabled, fields pre-filled from existing template

Clone dialog — name cleared, rest pre-filled

Related Issues

Closes #2255. Subsumes the scope of #2023. Supersedes #2148, which was closed in favour of this branch.

Review of this PR surfaced a class of pre-existing unguarded mutating endpoints beyond its scope; enumerated in #4944 rather than rewritten here.

Open questions for the reviewer

Three UX items are deliberately left as product decisions rather than being decided in a review round:

1. The per-tool auto-approve state is conveyed by colour alone.
2. Escape discards a part-written draft with no confirmation.
3. Whether Clone should be offered for managed templates. This is the first-run question: a fresh install has only managed specs, so the current !managed gate hides Edit and Clone together, while the backend's 403 text tells the user to "clone it to get an editable copy".

Checklist

• Single commit with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (module-spec docs for the new write surface)
• No secrets, credentials, or internal references in the diff

[bolichen97]

Overlap notice — three PRs implement Agent Template authoring from the dashboard:

• feat: add dashboard authoring for custom Agent Templates #2023 (@RohanK6) — create-only core, closes Add dashboard authoring for custom Agent Templates #1829. Full field surface, 38 tests, 10-locale i18n + context sidecar, docs, and a reusable screenshot-capture harness.
• feat(agents): author agent templates from the dashboard #2148 (@kyleseaman) — independent design via POST /api/agents/detail, deliberately excluding tools/allowedTools/toolsSettings from the write surface, with prompt/description editing, 72 revert-verified tests, and full i18n.
• feat(agents): add Agent Template authoring — create, edit, clone dialog + security predicates (#2255) #2383 (@qh2244) — create + edit + clone in one PR, building on feat: add dashboard authoring for custom Agent Templates #2023's architecture and additionally closing the Agent Templates: edit flow + deferred dialog polish and security-predicate hoist (fast-follow to #2023) #2255 fast-follow (draft auto-commit, shield discoverability, +N overflow cue, security-predicate hoist into kiro_crew.security).

Recommendation: consolidate on #2383 as the base — widest scope (create/edit/clone + all #2255 items), the only branch without merge conflicts, and it matches what issues #1829 and #2255 asked for. However, #2383 is not yet merge-ready: CI is failing (frontend i18n gates, backend test shard, coverage gate), it ships no tests, and only the en catalog. Before merge it should adopt:

• from feat: add dashboard authoring for custom Agent Templates #2023: the backend/frontend test suites (they target the same code feat(agents): add Agent Template authoring — create, edit, clone dialog + security predicates (#2255) #2383 subsumes and should port nearly verbatim), the 10-locale i18n catalogs, the docs page, and the capture harness — @RohanK6, your review would be especially valuable given feat(agents): add Agent Template authoring — create, edit, clone dialog + security predicates (#2255) #2383 extends your design and you authored Agent Templates: edit flow + deferred dialog polish and security-predicate hoist (fast-follow to #2023) #2255;
• from feat(agents): author agent templates from the dashboard #2148: the concurrency and identity hardening (O_EXCL create, cross-spec name-conflict detection, backend-served managed flag, machine-readable error codes) — plus a maintainer decision on the privilege-surface question feat(agents): author agent templates from the dashboard #2148 raises: whether allowedTools should be authorable at create time at all. @kyleseaman, your revert-verified guard tests would materially strengthen the winner.

@kyleseaman @RohanK6 — inviting you both to review #2383 so the best parts of all three land. If maintainers prefer the original sequencing (#2023 first, #2383 rebased back into the fast-follow it started as), that also works; the main thing is that the tests and i18n coverage from the other two PRs are not lost.

[kyleseaman]

I pushed a commit onto this branch (6b90d9e47) rather than opening a competing PR — I had #2148 open for the narrower create-only version of this and yours covers strictly more, so the useful move was to bring its tests here and fix what they caught. Happy to back any of it out if you disagree with a call.

Three of the fixes are load-bearing, and they share one root cause worth stating up front: kiro-cli validates ~/.kiro/agents/*.json with serde deny_unknown_fields and rejects the entire spec on any unknown key, then silently falls back to the default agent (agent.py:migrate_agent_specs docstring, and the reason agent_state.py's sidecar exists at all). So an unknown field isn't a degraded template — it's a template that doesn't exist, while the session looks like it's running the user's agent.

1. The prompt went to customInstructions. That key appears nowhere in the repo; the kiro field is prompt. Every template created with a prompt was unloadable. The request-body alias still works, only the written key changed.

2. deniedCommands is now refused outright. Top-level it's an unknown key. Moving it under toolsSettings.execute_bash — the only place kiro-cli reads it — would have been worse, because that mechanism is retired: denial moved to the hooks.py PreToolUse gate, and agent.py:_strip_legacy_denied_commands deletes that key on every refresh specifically so a stale spec rule can't outrank Settings > Security.

There was also a mismatch inside the feature: the dialog sent the nested toolsSettings shape while _build_template_spec only read a top-level deniedCommands, so every pattern a user typed was dropped on the floor. I removed the editor along with the field rather than leave a control that can't be honoured.

3. PUT could overwrite Kiro Crew's own agent. The guard was if "--" in target.name, but no entry in OWNED_KIRO_AGENT_FILES contains a double dash — so PUT /api/agents/installed/kirocrew passed, and since _build_template_spec returns a fresh dict, it full-replaced the spec and dropped hooks (including the bash audit hook), includeMcpJson and the managed MCP block until the next install rebuild. Now refused by filename read from agent_files.

Smaller ones in the same commit: PUT now carries forward spec keys the form doesn't model, so editing a description no longer deletes hand-authored hooks/toolsSettings; create uses O_EXCL instead of exists() + os.replace, which was check-then-overwrite and clobbered the loser of a concurrent POST; create now rejects a name an existing spec already answers to via its name field (a package agent at <pkg>-reviewer.json declaring "name": "reviewer" made a new reviewer.json a coin flip); and managed stems plus the built-in default are reserved.

Your security.py predicates are good. I wrote 55 tests against them and every one passed on the first run — the token-split matching (TOKENIZER not flagged, myApiKeyPath not flagged, GITHUB_TOKEN flagged) and the anchored ${VAR} escape both hold. They were just unverified, which is why I pinned both directions: a false negative leaks a credential into the spec, a false positive teaches people to route around the check.

On the red CI, both failures were fixable and are now green locally:

• test_error_code_contract::test_baseline_is_not_stale — your added code fields improved missing_code from 58 to 56 and the ratchet fails on an un-snapshotted gain. Re-ran --update.
• Frontend Tests (9 failures) — the branch added 35 _one plural forms to ja, ko and zh-CN, which resolve to the single CLDR category other, so those forms are unreachable; three also tripped the destructive-copy check by keeping English text. I removed the 30 per locale that had an _other sibling. Full vitest is now 11,551 pass / 0 fail.

Verification on the new head: 32 endpoint tests + 55 predicate tests, all 7 schema fixes revert-verified (patched out, confirmed failing, restored). Backend 39,648 pass — the 22 remaining failures are host-environment (jq version, /proc semantics, ssh -G, a Node 16 shim) and 21 reproduce in a separate clone at main with none of this code. isort, flake8, mypy (856 files), tsc, eslint 0 errors, all 13 i18n checks, brand gate: clean.

Two things I deliberately left alone: the branch is 82 behind main and CONFLICTING, and rebasing someone else's branch felt like yours to do — say the word and I'll take it. And one test asserts the written spec contains only keys kiro-cli accepts, so the next unknown-field slip fails there instead of silently disabling every template; if you'd rather that live somewhere else, move it.

[RohanK6]

Resolved #2023 in favor of this one. Feel free to let me know if you'd like any help bringing anything worthwhile over!

[kyleseaman]

Head is now 3a3c81964. Two notes on reading the bot comments above: they update in place and currently show 51bd6e51d, which is four heads old — every BLOCKING finding in the visible GPT body was fixed in the two pushes after it. And Cross-Platform Portability, E2E, and all four Backend Tests (Windows) shards, which were red, now pass.

GPT's five BLOCKING findings from 51bd6e51d — all fixed

1. Wildcard auto-approval bypasses the gates — fixed, and worse than described. Measured on this host: may_skip_gate_now returns True for *, @* and @builtin/* but False for execute_bash and fs_write. A glob is therefore a strictly broader grant that passes the exact gate the specific names fail. allowedTools now requires exact tool names; wildcards are refused with a stable code. Revert-verified.
2. Relative resources bypass the sensitive-path screen — fixed, and this was a hole in my own previous fix, for exactly the reason named: a relative path is resolved by the agent against the agent's directory, while the validator runs in the gateway process, so file://../.ssh/id_rsa resolves somewhere harmless where it is checked and somewhere sensitive where it is used. Resolution cannot close that, so parent traversal is refused outright. Parsed with PureWindowsPath, which treats both separators on every platform — that also cleared the Cross-Platform Portability failure my first attempt caused (it hand-split on "/", which the guard greps for). Revert-verified, including backslash traversal cases.
3. MCP suggestions create invalid server specifications — fixed, and broader than the suggestion path. The backend does not require command/url and the dialog only models args, so any add-by-name produced a husk like {"args":[...]} with nothing launchable — and an unusable server invalidates the whole spec, falling the session back to the default agent. Submit now refuses husks with a visible message rather than persisting one.
4. Editing the fallback detail deletes stored configuration — fixed structurally rather than by a point patch, because this was the third round in a row of the same class. See the invariant section below.
5. MCP suggestions violate the two-button row limit — fixed by removing the row: the chips are now a single DropdownMenu, verified by counting rendered buttons in the MCP fieldset (2). Picking a name fills the input rather than adding the server, since a launchable entry needs a command the dialog cannot infer.

The invariant, instead of a fourth point fix

Findings 2, 3 and 4 above, plus round 1's static owned-keys set and round 2's args-only mcpServers, are all one defect: the dialog sent fields it had no authoritative value for, and the backend replaces what the request carries. Three rounds of patching individual instances produced a new sibling each time, so this round closed the class instead.

The dialog now records which keys the loaded record actually carried, and submit sends a field only when it was loaded or the user authored it. A field that was never loaded and is still empty is omitted, so the backend preserves it. Combined with absent-means-preserve, a partial or failed detail load is now structurally incapable of erasing stored configuration. Editing still works in both directions: a field that was loaded and then cleared is still sent, because that is how a deliberate clear is expressed — the two cases are no longer the same wire payload. Five tests pin it, and reverting to the old unconditional payload build fails them.

Edit/Clone are additionally gated on a successful detail fetch, so the list-only fallback pane stays readable but cannot open an editor over fields that merely look empty.

GPT's two advisory FINDINGs

• Function-local imports — accepted as a real nit, declined for this PR. The function-local imports in these handlers are pre-existing style in this module, and hoisting them risks the eager-boot-import problem this repo has hit before. Happy to do it here if you want it in scope.
• "code": "invalid_json" and the baseline are out of scope — rebutted on the baseline half. error-code-baseline.json is not optional: test_baseline_is_not_stale FAILS without it, because adding codes improved missing_code 53 → 51 in this file and the ratchet refuses to sit looser than the code. Reverting it turns CI red. The regenerated totals move in the tightening direction only (1404 → 1402 missing, 742 → 763 compliant).

Design Review — CONCERNS

• Clone was lossy where edit was made lossless — correct, and the best catch of the round: _carry_forward_unowned protects PUT only, and clone goes through POST with nothing on disk to preserve against. Fixed for resources, which the create endpoint does accept, so a cloned template keeps its steering globs. For hooks, includeMcpJson and toolsSettings the create endpoint cannot express them at all — it builds the spec from a whitelist — so instead of dropping them silently the dialog now names them in a warning before you save, which is the same refuse-rather-than-diverge rule applied to MCP husks and the name mismatch. Making clone fully lossless needs the create path to accept those keys (or a server-side clone endpoint); I did not add either here because it is a real scope decision, and toolsSettings.execute_bash.deniedCommands is the retired mechanism this PR deliberately refuses, so carrying it wholesale would reintroduce what the PR rejects. Both fixes revert-verified.
• "Clone it" is named as the remedy but hidden for source === 'kirocrew' — accepted, not fixed. It is a real inconsistency, but the fix is a UI affordance decision (show Clone for managed specs) and, per the point above, a clone of a managed spec would drop its hooks today. Fixing the affordance before clone is lossless would hand users a remedy that quietly produces a broken copy. Worth doing in the follow-up that makes clone lossless.
• Three PNGs committed under temp-screenshots/ — rebutted. That is this repo's documented convention for PR evidence: temp-screenshots/ is deliberately top-level and outside every packaged path (not docs/, not src/kiro_crew/**), so review images never ride into the wheel or the desktop build, and the directory is pruned periodically. Commit-SHA-pinned same-origin URLs are used precisely because branch-pinned links break on branch deletion and external hosts are camo-blocked for private repos.
• Document the PUT contract — agreed and worth doing; the replace-what-you-send / preserve-what-you-omit contract is now load-bearing for clients and should not live only in a docstring.

UX Review — CONCERNS

• MCP args input had no label — fixed; it now carries a translated aria-label ("Server arguments") rather than relying on the args… placeholder.
• PR text claims the overflow reads "+N more (type to search)" — moot: that control no longer exists, replaced by the single dropdown. The description needs correcting, which I am doing separately.
• No dirty-state guard on dismiss — accepted, not fixed. Escape or an overlay click still discards a long prompt with no confirmation, and the backend allows 100k chars, so the impact is real. It is a self-contained follow-up rather than part of this round's data-loss work.
• Name rules diverge and the failure surfaces as a raw regex — accepted, not fixed. Frontend slugify is looser than the backend pattern, so some names round-trip to a backend rejection rendered verbatim in the footer error box. Worth mirroring the rule client-side and humanizing the message.
• Untranslated strings in non-English catalogs — this is the repo's standing pattern for newly-added keys (the untranslated i18n check is report-only and currently counts 563 across 116 files), so a translation pass is a repo-wide task rather than this PR's. All keys added here are present in every catalog, so nothing renders as a missing-key fallback and the parity and key-reference gates pass.
• Shield approved-state is colour-only — accepted, not fixed; a fill or shape change alongside the colour is the right call for the same follow-up.

Verification on 3a3c81964

164 targeted backend tests, full vitest 17,223 pass, mypy 895 files, isort/flake8/tsc clean, eslint 0 errors, 16/16 i18n with I18N_BASE_REF=origin/main, dead-key ratchet green, and the portability guard reproduced locally against its own rule set.

The remaining red is not this PR

Frontend Tests (2), Frontend Coverage Merge and the Coverage Gate that cascades from them are failing on main itself — filed as #3113. main's own push run for 0f1a4d5 shows the same two jobs red. Cause: 13b16144d (#2882) reworded the catalog string that LocalStorageDebug's clear-orphans button uses as its title, but left LocalStorageDebugCoverage.test.tsx:244 matching the old wording. The failing test file and the catalog value are byte-identical between this branch and main, so both inputs are main's. This PR cannot clear those three until #3113 lands.

[kyleseaman]

Head is now 1d4e46073, rebased across 90 commits of main (0 behind, still one commit, original authorship preserved). All four BLOCKING findings from 9d5392a48 are fixed and revert-verified.

1. agents.py:2038 — unanchored resource globs expose credential files — FIXED, as an invariant rather than a third patch

Correct. if not root: return False let file://.*/* through: the literal prefix before the first wildcard is ., which contains no /, so no root was extracted and the entry was allowed. Under a home-directory workspace that sweeps dotfile directories including .ssh.

This is the third round of findings on this one screen (traversal, then this), so it now enforces a rule instead of another special case: a file:// glob must be anchored to a concrete root. An unanchored glob's meaning is decided entirely by the agent's working directory, which this validator — running in the gateway — cannot know. That is the same reasoning already applied to parent traversal: when the base is unknowable here, refuse rather than resolve.

Consequence worth stating plainly: file://**/*.md is now refused too. It was previously allowed by an explicit test, which I changed. The shape templates actually use, file://.kiro/steering/**/*.md, is anchored and still passes.

2. agents.py:2389 — unsafe existing-spec reads — FIXED

Both halves were real. A malformed spec was caught as existing = {}, which turned carry-forward into a full replace: nothing preserved because nothing was understood — the same silent erase this handler exists to prevent, reached through a parse failure instead of a partial request. The update now refuses with 409 agent_template_unreadable on an unreadable file, invalid JSON, or a non-object spec, and leaves the file byte-identical. Symlinks are refused for the additional reason you name: following one would copy whatever it points at into a world-readable agent spec.

3. agents.py:2410 — concurrent updates lose saved fields — FIXED

Real lost-update race. Existence check, read, carry-forward and write are now one serialized step under _get_config_lock(). It is an asyncio.Lock, so awaiting the executor hops inside it suspends the handler without blocking the event loop — only another config writer waits, which is the point.

The test issues two genuinely disjoint PUTs concurrently (one touching only description, one only prompt) and asserts neither resurrects the other's pre-edit value. Revert-verified: replacing the lock with if True: fails it, so the serialization is load-bearing and the race was reachable rather than theoretical.

4. AgentTemplateCreator.tsx:146 — MCP arguments containing spaces corrupted — FIXED

Real data corruption: the args array was space-joined for display and re-split on whitespace at submit, so ["--header", "User Agent"] came back as three arguments and the server received a different argv. The dialog now keeps the loaded array and sends it verbatim while the text still matches its joined form; only a field the user actually edited is re-derived by splitting. Revert-verified with a spaced-argument fixture.

Advisory FINDING — "code": "invalid_json" on pre-existing responses

Same answer as before, and it has not become less true: the error-code-baseline.json half is not optional, because test_baseline_is_not_stale fails without it once these handlers add codes. Reverting it turns CI red. The ratchet only ever moves in the tightening direction.

Rebase note (worth a look, since it touched a file this PR did not own): main refactored the inline route table into dashboard/routes/ behind register_all(app). The branch still carried the old ~680-line inline list, of which exactly two lines were this PR's (the POST and PUT). Resolution took main's register_all(app) wholesale and relocated those two registrations into routes/agents.py beside the GET they extend — server.py now contains no api_agents_installed reference at all. The 11 locale-catalog conflicts were resolved against the merge base (main's catalog, plus keys the branch added, minus keys it deleted), which lands at 45 changed lines per catalog for 41 added keys — no phantom reordering.

Gates on 1d4e46073: 204 targeted backend tests, full vitest 17,848 pass / 0 fail, mypy 942 files, isort/flake8/tsc clean, eslint 0 errors, 16/16 i18n with I18N_BASE_REF=origin/main, error-code ratchet green, portability guard reproduced locally against its own rule set. The three App.test.tsx credits-pill failures reported in earlier rounds are gone — they were local-environment only, and no longer reproduce after this rebase.

[kyleseaman]

Head is now 523d9e5b5 (rebased, 0 behind, one commit, original authorship preserved). All four GPT BLOCKING findings and Opus's BLOCKING finding from 4143a70ff are fixed and revert-verified (5/5 mutations caught).

GPT

1. agents.py:2099 — prompt paths bypass the sensitive-path screen — FIXED

Verified before fixing, because a bogus restriction on prompt would be worse than the finding. It holds: the repo's own docs show "prompt": "file:///path/to/prompt.md", and KiroCrew writes exactly that form for its managed agents (config["prompt"] = f"file://{_prompt_path()}"). So prompt is resolved and READ, making it the same credential-disclosure path as resources. It now goes through the same screen rather than a second one.

2. agents.py:1860 — collision scan permits an unbounded unsafe read — FIXED

Correct, and it was my code. _name_already_claimed read every spec with a bare read_text, in a user-writable directory shared with other tools. It now uses agent_discovery._read_agent_spec, the reader you pointed at, which caps the size through safe_read_file_bytes, resolves the link with strict=True, and refuses a sensitive resolved target.

Worth noting how this one was verified, because the first attempt was a false pass: nothing in the suite distinguished the two readers, so reverting to read_text kept everything green. The discriminator is the AppleDouble skip that only the shared reader has — an ._claimed.json sidecar declaring a name must not claim it — paired with an assertion that a genuine spec still does. Reverting the reader now fails.

3. agents.py:2073 — falsy non-strings erase stored scalar fields — FIXED

Real, and it was an interaction my own carry-forward created: if description: skipped both validation and assignment for false/0, so the key was present in the request — which presence-based carry-forward reads as authored — while absent from the built spec. Result was an erase with HTTP 200. Type-checking now happens on PRESENCE, before any truthiness test, for description, model, prompt and the customInstructions alias. The legitimate clear still works: "" is a string, validates, and omits the field.

4. agents.py:2150 — malformed MCP entries are persisted — FIXED

The whole entry shape is validated now (command/url string, args list-of-strings), not just the two fields being screened. Consistent with this PR's own premise: kiro-cli rejects the entire spec on a malformed server and falls the session back to the default agent, so persisting one breaks the template silently.

Opus — the better catch of the round

agents.py:441 — a relative dotfile resource bypasses the screen — FIXED

This is the fourth instance of one class on this screen, and Opus did the work to prove it rather than assert it: is_sensitive_path with no base resolves candidate forms against the gateway's cwd, while the path is resolved by the agent against its workspace. So file://.ssh/id_rsa has no .., matches nothing, and was accepted — while file://~/.ssh/id_rsa, file://../.ssh/id_rsa and unanchored globs were all already refused. Only the plain relative dotfile form slipped through.

Fixed by screening the relative form a second time with base_dir=$HOME — the worst case the workspace can be, and precisely the scenario this screen's own docstring names. I chose that over the suggested dotfile-directory list because the list is the thing that goes stale: it would have to name .ssh, .aws, .gnupg, then .config/gcloud, then the next one. Reusing the shared helper with an explicit base keeps one source of truth. Measured to confirm it discriminates correctly rather than just blocking dots: .ssh/id_rsa, .aws/credentials and .gnupg/secring.gpg are refused, while .kiro/steering/**/*.md — dot-leading and the PR's documented shape — still passes. The prompt screen inherits this, so a relative dotfile prompt is refused too.

Advisory — three 400 bodies omit code — FIXED. Added too_many_skills (both handlers) and name_mismatch, with tests asserting the codes. This improved missing_code 51 → 48 in this file, so the error-code baseline was re-snapshotted again (1402 → 1399 total, 763 → 789 compliant) — the same ratchet that makes reverting the baseline turn CI red.

GPT's advisory FINDINGs

• Managed specs exposed Edit — FIXED, and it was real: the auxiliary managed specs (kirocrew-knowledge, kirocrew-research, kirocrew-lite) do not report source === 'kirocrew', so a source-only check offered Edit for templates whose PUT always returns 403. The listing now carries a managed flag derived from the same _template_is_writable predicate the PUT enforces, and the menu gates on that — so the UI cannot offer an action the endpoint refuses, and adding a spec to OWNED_KIRO_AGENT_FILES updates both at once. A test pins the flag to the refusal so they cannot drift.
• Picker selections lack command/url — already addressed structurally last round: the picker now only fills the name input, it does not add a server, precisely because a launchable entry needs a command the dialog cannot infer. A husk is refused at submit with a visible message rather than persisted.
• Model validation — accepted, not fixed. Rejecting unadvertised model IDs is a real improvement but it is a behaviour change on a field the editor shares with the model picker, and it belongs with that surface rather than bolted into template validation.

Gates on 523d9e5b5: 125 targeted backend tests in the template/error-code/import-hoist set, 116 in the authoring suite alone, full vitest 17,865 pass, mypy 942 files, isort/flake8/tsc clean, eslint 0 errors, 16/16 i18n. One local App.test.tsx credits-pill failure remains on this host — it reproduces in isolation, the file is not in this diff, and CI's frontend shards are green on it.

[kyleseaman]

Head is now 89d463d75 (rebased across 12 commits, 0 behind, one commit, original authorship preserved). All five BLOCKING findings from a0a5ca798 are fixed, each revert-verified (5/5 mutations caught).

1. security.py:8404 — credential-bearing header keys bypass screening — FIXED

Correct and one-line. AUTH_HEADER tokenizes to [AUTH, HEADER], and the metadata-suffix check returns before the secret-token check, so the exemption won outright and the literal value persisted. A header value is frequently the credential itself (AUTH_HEADER=Bearer ...), so HEADER is removed from the exemption and the reason is recorded next to the set so it does not get re-added. AUTH_HEADER and AUTHORIZATION_HEADER are now flagged; the genuine metadata suffixes (TOKEN_URL, SECRET_NAME, CREDENTIAL_PATH, ...) still are not.

2. agents.py:2115 — malformed MCP transports persisted — FIXED (models: declined, with reasoning)

Two of the three parts were real and are fixed:

• A server now requires a launchable transport — command for stdio or url for http. An entry with neither cannot start, and kiro-cli rejects the whole spec on it, which silently falls the session back to the default agent.
• env is screened on presence, not on truthiness. {"env": []} previously skipped the type check entirely and was persisted as an invalid block. Same defect shape as the falsy-scalar erase from the previous round, one level down.

Fixing the transport rule invalidated three of my own earlier test fixtures that used args-only servers — the exact husk shape the rule now refuses. Two were updated to launchable entries. The third mattered more: test_create_refuses_a_base64_wrapped_credential asserted a 400 for a credential reason, and the new transport rule would have refused it first, so the test would have passed with the credential screen removed. It now carries a command so it still exercises the screen, and the credential-wiring revert-verify still fails without the fix.

On advertised-model validation: declined, and this is now the third round it has been raised, so here is the full reasoning rather than a one-liner. The dashboard's model picker and this endpoint share a source of truth (advertised_model_ids / model_is_unusable), and a template can legitimately name a model that is momentarily unadvertised — a backend that has not enumerated yet, or a model the user has entitlement to but the current probe has not returned. Refusing at write time converts a transient availability gap into a permanent authoring failure, and the spec is re-resolved at session start anyway where an unusable id already degrades to the default. If you want it enforced, the right place is the picker plus a warning on the template row, not a 400 on save — happy to do that as a follow-up, but it is a product behaviour change rather than a defect fix.

3. agents.py:2439 — update performs an unbounded config read — FIXED

Correct, and it was the second site of the same defect: I routed the collision scan through _read_agent_spec last round but left the update's own read as read_text. Now both go through the capped, symlink-resolving, sensitive-path-refusing reader, and a None result is refused rather than treated as empty. This is exactly the "audit every path reaching the same IO" case, and I only fixed one of two.

4. agents.py:2278 — path validation blocks the event loop — FIXED

Real, and self-inflicted: the screens added in earlier rounds (is_sensitive_path, path_contains_sensitive) resolve realpaths, so _build_template_spec became synchronous filesystem work on the gateway loop. On a network-backed home that stalls the loop and the heartbeat with it. Both handlers now build through discovery_executor().

5. agents.py:2279 — security denials not SEL-audited — FIXED

Correct. Most refusals from the builder are security decisions — a sensitive resource or prompt path, a literal credential in an MCP env block, a wildcard auto-approve grant — and they returned 400 with no audit record. Both handlers now emit a denied log_api_access event with the refusal reason before returning. Three tests pin it, including one asserting a successful create does not record a denial.

Advisory — function-local imports — FIXED this round. agent_files, platform.governance and tempfile are hoisted to module scope. I declined this twice on eager-import-risk grounds and was wrong both times: main's own test_no_function_local_agent_imports_remain ratchet made the kiro_crew.agent case a hard CI failure last round, and these three are leaf modules with no cycle back to the handlers — verified by importing the module fresh in a clean interpreter, which succeeds.

Advisory — "code": "invalid_json" scope. Same answer as before, unchanged because the constraint is unchanged: the error-code-baseline.json half is not optional, since test_baseline_is_not_stale fails without it once these handlers add codes. Reverting it turns CI red. The ratchet only moves in the tightening direction.

Gates on 89d463d75: 218 targeted backend tests across the authoring, credential-screen, import-hoist and error-code suites, full vitest 17,977 pass, mypy 942 files, isort/flake8/tsc clean, eslint 0 errors, 16/16 i18n, brand gate clean, portability guard reproduced locally.

One local-only failure worth naming so it is not mistaken for a regression: CrewCompanionPanelCoverage.test.tsx > closes on Escape fails in the full local suite but passes 36/36 in isolation, and the file is not in this diff. That is cross-test contamination surfaced by local full-suite composition; CI shards the frontend four ways, so it does not reproduce there. Same class as issue #3264.

[kyleseaman]

Head is now c682af78a (rebased, 0 behind, one commit, original authorship preserved). All three BLOCKING findings and the advisory from 89d463d75 are fixed, both new guards revert-verified.

1. agents.py:2537 — managed-template denials bypass SEL auditing — FIXED

Correct, and it is the same audit-every-path gap one branch over: last round I added the denied event to the validation branch and left the managed-template 403 beside it silent. It now emits a denied agent_template.update event naming the template before returning. Revert-verified.

2. agents.py:2112 — unavailable models persisted — FIXED, and I was wrong to decline it

I declined this three times on the grounds that write-time refusal would be brittle when entitlement is momentarily unknowable. That objection does not survive reading the predicate: model_is_unusable already fails open on an empty or absent advertised set, precisely so an unadvertising backend is not read as "nothing is allowed". So the brittleness I was guarding against is handled inside the thing you asked me to call. Your consequence chain — the spec claims a model, session startup withholds it, and the agent silently runs the backend default while the user believes they pinned one — is the part I never actually answered.

The check delegates to model_is_unusable rather than comparing ids, so it cannot become a second spelling of "can this account run it" and disagree with the picker or the wire.

Doing it properly required one small refactor worth flagging since it touches a path this PR does not own: the provider walk that resolves what a live session advertises lived inline inside _entitled_kiro_models, and it carries non-obvious ordering rationale (newest session first, because a session started before a plan change still holds its old snapshot). Rather than restate that walk, I extracted it as _live_advertised_model_ids(request) and had the picker call it too, so both read one source. The picker's own entitlement suite (test_api_models_entitlement, test_agent_default_model, test_agent_model_unpin — 83 tests) passes unchanged, which is the evidence the extraction is behaviour-preserving.

Three tests pin the new rule, including the fail-open case: an empty advertised set still accepts any model.

3. AgentTemplateCreator.tsx:145 — malformed shared specs crash the editor — FIXED

Real. Agent specs live in a user-writable directory shared with other tools, so a hand-edited "tools": "fs_read" reaches the detail endpoint as a string, || [] keeps the string, and the first .map blanks the dialog. tools, allowedTools and skills now require Array.isArray. I also guarded mcpServers: a string there does not throw, but Object.entries("abc") yields index/char pairs, so the dialog would have populated itself with garbage server rows instead of crashing — quieter and worse.

Advisory — remaining function-local governance import — FIXED. My hoist script last round replaced only the first occurrence; a second lived in _persist_spec. Both are gone and the module-scope import serves both.

That hoist had a consequence worth recording: three governance tests patched kiro_crew.platform.governance.sanitize_agent_config_governance, which stopped intercepting once _persist_spec bound the name at import time. They now patch the handler module's reference. I re-ran the round-1 mutation harness afterwards rather than trusting the green: removing the sanitize call from the chokepoint still fails 4 tests, so the retarget did not paper over a real break.

Gates on c682af78a: 238 targeted backend tests across the authoring, credential-screen, import-hoist, error-code and model-entitlement suites, mypy 943 files, isort/flake8/tsc clean, 16/16 i18n, brand gate clean.

On the shape of this review cycle. Nine rounds in, the finding count is falling (5 → 5 → 4 → 3) and this round produced no new defect class — two were adjacent instances of gaps I had half-fixed (audit one branch, hoist one occurrence), one was a genuine new input-validation case, and one was a finding I had wrongly declined. That reads like convergence rather than a treadmill. The recurring pattern in my own errors is worth naming plainly: when a fix applies to a class of call sites, I have repeatedly fixed the one named in the finding and left its siblings — the reader on the collision scan but not the update, the audit on validate but not managed, the hoist on one import but not two. Where a third instance of that shape shows up, the right response is to enumerate every site rather than patch the reported one.

[kyleseaman]

Head is now 61ec54068 (rebased twice this round, 0 behind, MERGEABLE, one commit, original authorship preserved). All three BLOCKING findings across both bots are fixed, each revert-verified (5/5 mutations caught).

Opus — relative glob bypasses the credential-file screen (agents.py:2054)

Confirmed and fixed, and the fix is structural rather than the one-line form prescribed.

The analysis is exactly right, including the asymmetry proof: file://~/.kube/** was refused while file://.kube/** was accepted. The IS-check got a HOME base in round 7; the CONTAINS-check sitting three lines below it kept resolving against the gateway's cwd. For a directory whose sensitive leaf is a child (.kube/config, .config/gcloud, .docker/config.json, .local/share/kiro-cli), the IS-check cannot fire and the CONTAINS-check matched nothing.

This is the third finding in this one screen (round 5 invariant, round 7 relative dotfile IS-check, now the relative CONTAINS-check), and all three are the same defect: a call site choosing its own base. So instead of adding base_dir to the one call named, base selection is now owned by a single helper that both directions route through:

def _screen_both_bases(expanded, check):
    if not os.path.isabs(expanded) and check(expanded, base_dir=os.path.expanduser("~")):
        return True
    return check(expanded)

_path_escapes_or_is_sensitive and the glob's CONTAINS branch both call it. A future third direction cannot reintroduce the gap by omission, which is the actual failure mode here — the two checks sat adjacent, looked symmetric, and disagreed about the base.

Measured rather than assumed, before and after. All four dirs named are refused in relative form, the absolute forms stay refused, the round-7 cases (.ssh, .aws, .gnupg) stay refused, and the shapes templates legitimately use are not over-blocked: .kiro/steering/**/*.md, .kiro/steering/**, docs/**/*.md, src/**/*.py, ~/projects/notes/** all still pass. A fix that traded the bypass for a broken feature would have been worse than the bug; 14 refuse-cases and 6 allow-cases are pinned as tests.

The revert-verify includes a regression guard for round 7: reverting the IS-direction to a bare is_sensitive_path also fails, so neither half can silently come undone.

Candidate 2 dropped — agreed, and for the reason given: the shipped dialog sends prompt, and no consumer sends customInstructions: "".

GPT — user templates can collide with the app-owned namespace (agents.py:1801)

Confirmed by reading the deletion path, and fixed. I verified the chain rather than taking it on the description: _deregister_agents(app) computes _safe_link_name(app + "/") → "calendar--", then unlinks every entry in ~/.kiro/agents/ matching that prefix and ending .json. It checks neither ownership nor whether the entry is the app's own symlink. So a user template named calendar--assistant is silently deleted when the calendar app is disabled. spawn_sdk reads the same prefix as proof of which agents an app may run, so a name in that namespace is also an authority claim.

-- is now refused at creation. One implementation note worth stating because it changed a status code: I first put the rule in _build_template_spec, which is shared by create and update and runs before the managed-template check — so an app-owned spec started returning 400 (bad name) instead of 403 (managed), losing the accurate reason and the denial audit event added last round. An existing test caught it. The rule now lives in _reserved_namespace_error on the create path only, which is the only path where a name is chosen (update refuses a body name differing from the URL, so it cannot rename). Single hyphens remain legal.

GPT — MCP headers bypass schema and credential validation (agents.py:2240)

Correct, and it is the same class-of-sites gap as the base-resolution one: env was screened and headers — where a remote server's bearer token actually goes — was not. Rather than add a parallel screen, _validate_mcp_env now takes a field argument and serves both blocks, so there is one place a credential token can be forgotten instead of two. headers is type-checked as an object on presence (same falsy-non-dict reasoning as env) and screened for literal secrets, with ${VAR} references still allowed.

I measured the existing key screen against ten realistic credential-bearing header names before deciding whether it sufficed: nine were already flagged (Authorization, X-Api-Key, X-Auth-Token, Proxy-Authorization, X-Access-Token, ...) with zero false positives on benign ones (Accept, Content-Type, User-Agent, X-Request-Id, Accept-Encoding). The single miss was Cookie, which shares no token with any existing entry and is a session credential, so COOKIE was added to MCP_ENV_SECRET_KEY_TOKENS. That predicate is shared, so I ran the wider security surface — 1,589 tests across the credential-screen, sensitive-path and MCP suites — to confirm the added token breaks nothing.

Advisory — "code": "invalid_json" scope

Unchanged answer, because the constraint is unchanged: the error-code-baseline.json half is not optional, since test_baseline_is_not_stale fails without it once these handlers add codes. Reverting it turns CI red.

Relevant this round: main and this branch both tightened that baseline, producing the rebase conflict that briefly made the PR CONFLICTING (which is also why no pull_request workflows dispatched for 3cd325ed5 — GitHub cannot compute a merge for a conflicting PR). Resolved by regenerating from the merged tree rather than picking a side: missing_code 1404 → 1394, _compliant 729 → 784, three lines changed, no phantom churn. The ratchet only moves in the tightening direction.

Gates on 61ec54068

239 targeted backend tests plus 1,589 security-surface tests, mypy 941 files, isort/flake8/tsc clean, eslint 0 errors, 16/16 i18n, brand gate clean, npm ci re-run after both rebases.

Ten rounds in, the finding trajectory is 5 → 5 → 4 → 3 → 3, and the two bots now agree on the same span. Both of this round's security findings were consequences of my own earlier screens, and the pattern I named last round held: a rule applied to one call site while its sibling kept the old behaviour. Both fixes this round centralise the decision instead of duplicating it, which is the only form that stops the sequence.

[kyleseaman]

Head is now c82921f59 (rebased, 0 behind, MERGEABLE, one commit, original authorship preserved). Both BLOCKING findings are fixed and revert-verified (2/2 mutations caught).

MCP URL and argument credentials bypass screening (agents.py:2261)

Correct, and this is the fourth round of the same defect — so it is fixed as an invariant rather than by adding the two fields you named.

The sequence: round 8 screened env, round 10 added headers after you found it unscreened, and now url and args. Each round I closed the channel that was reported and left the others, because the screen was a list of field names and every new credential-carrying field was a separate thing to remember.

_mcp_entry_credential_error now walks every string value in a server entry — recursively, through nested lists and dicts — and screens each one with the shared value predicate. command is excluded because it is a filesystem path with its own sensitivity check and a path is not a credential. A field nobody has enumerated yet, or one kiro-cli adds later, is covered on arrival. The per-key screens for env and headers stay, because those catch a credential named by its key (env.GITHUB_TOKEN) where the value alone is unremarkable; the sweep catches one recognisable by its value in any field.

Measured both directions before shipping, because a generic sweep over args risks over-blocking legitimate entries. Refused: URL userinfo, a token in args, a token nested deeper in args, and a credential in an invented field the validator has no rule for. Still accepted: npx -y @scope/mcp-server, uvx --from /opt/tools/pkg --verbose, a plain remote URL, a URL with a port and path, ${GITHUB_TOKEN} in args, benign headers, and non-string values. Zero holes, zero over-blocks across 14 cases.

One correction to my own work, surfaced by the revert-verify. I first added a dedicated _URL_USERINFO_RE for https://user:secret@host, on the theory that userinfo hides the secret in structure rather than in a recognisable value shape. Reverting that regex alone did not fail its test — the shared credential patterns already recognise that shape. So the regex was unreachable, and I deleted it rather than keep a second spelling of a question the canonical patterns already answer, free to drift from them. Note the first version of that test also passed for the wrong reason (its password was itself credential-shaped, so the generic sweep caught it); the fixture and the test's docstring now say what is actually being pinned.

Filesystem stat blocks the event loop (agents.py:2615)

Correct, and self-inflicted in the same way the previous offload finding was. When I serialised the update path under the config lock in round 6, the existence check and the writability check stayed on the loop while only the read was offloaded. On an unavailable or network-backed home, that is_file() stat stalls every other gateway task including the heartbeat — and it does so while holding the config lock, so it blocks other writers too.

Both now run in the discovery executor, folded into a single _precheck hop rather than two so the round-trip cost does not double. The 403/404 ordering is unchanged, so the managed-template refusal and its audit event still fire as before.

Pinned by thread identity rather than wall-clock timing: the loop runs in the main thread, so observing the writability check on a worker thread proves it is off the loop. A duration assertion would have been a flake.

Gates on c82921f59

248 targeted backend tests (authoring, credential-screen, import-hoist, error-code), mypy 941 files, isort/flake8 clean, brand gate clean, error-code ratchet passing.

Also cleared this round: the Backend Tests (Windows) (1) failure on the previous head was test_auto_research_handlers_coverage.py::TestWatchdogLoop::test_expired_trust_forces_reauthorization, which is not in this 27-file diff and passed on re-run against the identical tree. Worth noting main's own tip is intermittently red on that shard too, though on a different test (test_session_keepalive), so the two are separate flakes rather than one shared cause.

Eleven rounds: 5 → 5 → 4 → 3 → 3 → 2. Both findings this round were consequences of my own earlier changes, and both are now fixed at the level that makes the class unreachable — one screen that walks every value, one executor hop that owns every stat on that path — rather than at the level of the two fields and one call named.

[iamwhatever]

🤖 Kiro Crew [operator: iamwhatever]: This PR has been inactive for 5+ days. I reviewed the blockers but they require your input:

• Merge conflict — the branch has diverged from main and needs a rebase/conflict resolution
• Multi-contributor design overlap — comments from bolichen97, kyleseaman, and RohanK6 indicate ongoing discussion about which PR (feat: add dashboard authoring for custom Agent Templates #2023 vs this one vs feat(agents): author agent templates from the dashboard #2148) is the canonical approach for Agent Template authoring. The design direction needs author alignment before automation can proceed.
• Scale — at 4470 lines across 27 files introducing a complete new subsystem (backend endpoints + frontend dialog + security predicates + tests), this exceeds what the pipeline can safely drive without author guidance

When you've addressed these, the pipeline will re-assess on its next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[kyleseaman]

Rebased onto current main and pushed as fa2c24963 — 0 behind, MERGEABLE, one commit, original authorship preserved.

The branch had fallen 679 commits behind, so this was a substantial re-anchor rather than a fast-forward. 14 conflicts:

• error-code-baseline.json — both sides tightened the ratchet. Regenerated from the merged tree rather than picking a side (--update), so the number reflects reality: 3 lines changed, no phantom churn.
• agents.py — import union only. Main added _AGENT_NAME_RE from kiro_crew.validation; this branch added five security imports.
• AgentsPage.tsx — the real one. Main restructured the pane into a mobile list/detail layout (useListDetailView, showList/showDetail, ListDetailBack) and added an svh viewport fix. Took main's structure wholesale and kept only this branch's actual contribution to that span: flex items-center gap-1.5 on the filter row, which the create button beside the search input needs. Verified the two concerns still compose — select() calls main's openDetail() before awaiting, and this branch's detailComplete gating still sets on both the fetch path and the auto-open effect, so Edit/Clone stay closed on a list-row fallback.
• 11 locale catalogs — main added skillsMultiSelect where this branch added agentTemplateCreator, as sibling keys sharing a closing brace. Union merged with main's keys first, +44/-1 per catalog, no reordering.

The rebase surfaced a real defect worth calling out. Main's shared agent-name grammar (_AGENT_NAME_RE) and this branch's _TEMPLATE_NAME_RE disagree, and I measured the gap rather than assuming it was cosmetic: six name shapes were creatable here and then rejected everywhere the shared grammar is enforced — release.bot, my.agent, my.agent.v2, and any name ending in -, _ or .. The result was a template that exists on disk but cannot be selected as an agent. Names must now satisfy both grammars: this one stays narrower on case (the spec filename derives from the name, so uppercase is still refused), and the shared one owns separators. Nine tests pin it in both directions, including that ordinary names (my-agent, under_score, a-b_c) still pass.

Also translated the strings this branch adds — they were English in all 11 non-English catalogs, which the diff-scoped changed-passthrough gate fails on with zero tolerance: 34 agentTemplateCreator keys plus 5 pages.agentsPage keys, ×11 locales. The Korean style gate then caught a genuine error in my own translation: {{names}}을 needs both allomorphs (을(를)) because the correct particle depends on the preceding word's final consonant, which is unknowable behind an interpolation (style/ko.md §2.1). Fixed; all 11 locale style suites pass.

Gates on fa2c24963: mypy 1003 files, black/flake8/isort/brand clean, error-code ratchet passing, 261 targeted backend tests, tsc clean, eslint 0 errors, i18n 18/18 PASS, and the full frontend suite green across CI-parity shards (1,433 files, ~22.3k tests).

Prior rounds' fixes were revert-verified again after the rebase to confirm they survived it — the round-10 relative-glob HOME-base screen and the round-11 whole-entry MCP credential sweep both still fail when mutated out (3/3 caught).

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of e5c57c60d5d718db198548901accdf3af3ddd2dc via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

Every addition names its harm, but the dialog re-spells the model/skills writes PATCH /api/agents/detail already owns, and the two paths already disagree.

What this change ships

Intent: let a user create, edit, and clone agent templates from the dashboard instead of hand-editing JSON — an ADDITION.

1. Create button + structured template dialog in the roster (new POST endpoint) — justified
2. Edit for user-owned templates; omitted fields preserved, not erased (new PUT endpoint) — justified
3. Clone pre-fills the dialog, naming the fields a copy cannot carry — justified
4. Managed templates hide Edit/Clone, derived from the server's own refusal predicate — justified
5. Literal credentials, sensitive paths, wildcard auto-approve refused and SEL-audited — justified (agent-ceiling boundary)
6. Template model checked against advertised ids via the shared resolver — justified (Model selection invariant)
7. Concurrent external edit answered with 409 instead of a silent overwrite — justified
8. Credential predicates exported from security.py — one consumer, but delegation to the canonical patterns is the point; justified
9. Dialog also sets model and skills, which the existing detail PATCH already owns — duplicate of src/kiro_crew/dashboard/handlers/agents.py:1172
10. Three screenshots the description calls outdated committed under temp-screenshots/ — rides along

Watch

• Second spelling of "set a template's model/skills": the PATCH path maintains the agent_state.set_model_managed sidecar (handlers/agents.py:1336-1343); the new PUT writes model and never touches it, so a model pinned in the dialog remains "managed" and re-syncable by a shipped-default bump. Counted consumers of the surviving PATCH: AgentsPage.tsx:505 (model), AgentSkillsEditor.tsx:84 (skills) — both UIs persist alongside the dialog, so the divergence is live, not hypothetical.
• The description says "The screenshots need a refresh… Re-capturing before merge," yet the stale captures are committed into the tree; replace or drop them before merge.

Subtractions

• Replace the exemptions apparatus (exemptions list, _stored_at, the in-lock re-validation loop in _prepare) by running _build_template_spec inside the config lock against the authoritative in-lock read — _prepare already enumerates and reads the agents directory under that same lock, so the pre-lock screening read stops being a separate truth to reconcile.

[FIRST-PRINCIPLES-REVIEWED] e5c57c6

[kyleseaman]

Head is now fe102fbe2 (rebased onto current main, 0 behind, one commit, original authorship preserved). Every blocking finding from both reviewers is fixed, plus all three First Principles subtractions.

GPT — new MCP rows always lack command/url, so Add/Create always errors (AgentTemplateCreator.tsx:277)

Correct, and it is the most consequential thing in this round — a functional break I introduced. Filed as a FINDING rather than BLOCKING, but it made the feature's Add-MCP-server affordance completely dead.

The chain: round 8 added a server-side launchable-transport requirement (an entry with neither command nor url cannot start, and kiro-cli rejects the whole spec on it, silently falling the session back to the default agent). I never added a matching field, so the dialog could only ever submit {name, args}. My own code comment admitted it — "The dialog does not collect a command, so adding one by bare name cannot produce a launchable entry" — which describes an affordance that cannot succeed. The configured-server picker does not rescue it either: it only fills the name input.

The dialog now collects the transport. One field, because command (stdio) and url (http) are the two halves of the same required value: an http(s):// value is recorded as url, anything else as command. Both submit paths carry it — the added row and the row typed but never added (that second path was its own silent hole). Three tests pin it, and reverting either derivation fails them (2/2 mutations caught).

GPT — MCP controls collapse at 320px (AgentTemplateCreator.tsx:550, BLOCKING)

Fixed, and the transport field would have made it worse — four controls on one row. The row is now flex-col and only becomes sm:flex-row at the breakpoint, with the fixed args width applying as sm:w-[140px] instead of unconditionally, plus min-w-0 on the flexible inputs so the name field cannot be squeezed to nothing.

First Principles — customInstructions alias has no sender (BLOCK)

Correct; verified independently before removing it. Grepped the whole tree: the only occurrences were the handler branch, its comment, and its own tests. The shipped dialog sends prompt. A public request-body alias must be honoured indefinitely once released, and this one was bought for nothing.

Removed: the or body.get("customInstructions", "") branch, the scalar-validation entry, and the test that existed only to exercise the alias. The test asserting customInstructions never reaches the written spec stays — that one pins the real invariant (kiro-cli's deny_unknown_fields rejects the entire spec on an unknown key), which is unrelated to accepting the alias inbound.

First Principles — subtractions (all three applied)

• Four constants made private. MCP_ENV_SECRET_VALUE_RE, MCP_ENV_METADATA_SUFFIXES, MCP_ENV_SECRET_KEY_TOKENS, MCP_ENV_SECRET_TOKEN_PAIRS had zero production consumers outside security.py. Worth noting the review's own point was borne out: the only outside references were tests searching the RE directly, which is exactly the misuse exporting it invites. They now reference the private name, so the coverage survives without the module advertising a footgun.
• _managed_agent_filenames() inlined. It returned OWNED_KIRO_AGENT_FILES verbatim for two internal call sites — a name without a decision. Both now use the constant, which still satisfies the docstring's actual point (read through agent_files rather than duplicating literals).

The invalid_json scope finding — I was wrong, and it is now reverted

Both reviewers flagged this, and I had rebutted it across five rounds on the grounds that reverting turns CI red. That rebuttal was wrong, and I only found out by measuring instead of restating it.

What I had been claiming: test_baseline_is_not_stale fails without the baseline half. What actually happens: the failing gate is test_no_new_error_response_without_a_code, a per-file ratchet that refuses an increase in missing_code and says in its own message not to regenerate the baseline. Reverting the handlers alone pushes agents.py from 48 back to 58 and trips it — which is what I kept observing and mis-attributing.

The part I never checked: main's own baseline for that file is 58. My branch is what lowered it to 48. So reverting both halves together is not raising a baseline to accommodate anything — it restores main's recorded number, and the ratchet then compares 58 against 58 and passes. Measured: the error-code suite is 6/6 green after the revert, and the agents.py entry is byte-identical to main's.

Ten handlers this PR does not own are back to main's response shape — api_agent_config, api_default_agent, the four capability install/uninstall handlers, _mutate_agent_package, api_agent_detail, api_kirocrew_agents_create, api_kirocrew_agent_update. This PR's own two endpoints keep their codes, which is the part that was in scope. The only remaining baseline delta against main is the _compliant total, which moves because the new endpoints add coded responses.

Also noted from the First Principles "Watch" list

The dialog's model edit does duplicate the existing PATCH spelling, and the divergence named is real: api_agent_detail's PATCH records the pick via agent_state.set_model_managed(...) while the PUT writes spec["model"] with no sidecar touch, so the same user action produces different freeze-against-default-bump behaviour depending on which control was used. I have not changed that in this round — it is a behavioural decision about which semantics are correct for a template edit, and it deserves its own change rather than being folded into a review round. Flagging it explicitly so it is not lost.

Gates on fe102fbe2

257 targeted backend tests, mypy 1008 files, black/flake8/isort/brand clean, error-code ratchet 6/6, tsc clean, eslint 0 errors, i18n 18/18 PASS, all 11 locale style suites (83 tests), and 680 tests across the agent-page, template-payload, roster, capabilities and i18n specs. Two new catalog keys added across all 12 locales plus the regenerated pseudolocale.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[kyleseaman]

Head is now 6af21084d (rebased, 0 behind, one commit, original authorship preserved). This round answers both blocking GPT findings on fe102fbe2 and the Design and UX CONCERNS, which were still owed a written disposition.

GPT — PUT can overwrite concurrent external edits (agents.py:2779, BLOCKING)

Correct, and it survived my own round-6 fix. That round serialized read-modify-write under _get_config_lock(), which stops two dashboard PUTs from clobbering each other. An external writer — kiro-cli, an editor, another tool — holds no such lock, so a save landing after _prepare read was replaced wholesale and lost.

The prescribed fix was "revert the PUT endpoint until it can conditionally replace the exact version read." I have implemented the condition rather than the revert, because the finding's own Fix line names conditional replacement as the requirement and that closes the vector without deleting the feature:

• _prepare stamps the file's (st_mtime_ns, st_size) before reading, so a write landing mid-read cannot be mistaken for the version parsed.
• _replace_spec_atomically takes that stamp and re-checks it immediately before the rename, discarding its temp file and returning agent_template_conflict when it no longer matches.
• The handler surfaces that as 409 with its own code, not a 500 — the file changed, the server did not fail — and SEL-audits the refusal.

Being straight about the limit: the stamp re-check to os.replace window cannot be closed against a writer that honours no lock, because the other party is not participating in any protocol. What this changes is the failure mode — from silently discarding someone's work to refusing with a retryable conflict, which is the guarantee optimistic concurrency actually offers. If you want the stronger property, it needs a lock both writers respect, and that is a change to kiro-cli's side of the contract, not this endpoint's.

Two tests pin it: an external write injected into the read window is refused with the external value intact on disk, and an undisturbed edit still writes. Both fail when either half of the mechanism is removed (2/2 mutations caught).

GPT — Clone silently omits supported fields (AgentTemplateCreator.tsx:45, BLOCKING)

Correct, and it corrects my own working notes. I had been treating the kiro spec key set as name/description/model/prompt/tools/allowedTools/mcpServers/resources/includeMcpJson/hooks/toolsSettings. Verified before acting: keyboardShortcut and welcomeMessage are documented agent fields in this repo's own docs/reference/kiro-cli/custom-agents/configuration-reference.md, and toolAliases is written into specs by Kiro Crew's Connections pass. The create path's allowlist carries none of them, so a clone dropped them with no warning.

All three are now in CLONE_UNCOPYABLE_KEYS and on the AgentDetail interface so the existing warning can detect them. That keeps the established design decision — clone names what it cannot copy rather than dropping it quietly — instead of pretending the copy is faithful.

Design — PUT skips lift_and_strip_bookkeeping

Correct and fixed. The helper's own docstring requires every writer of a kiro agent spec to call it, and #2570 named the other three; this was a fourth writer that did not. Carry-forward copies unowned keys verbatim, so a model_managed/cc_model acquired after boot would survive an edit — and kiro-cli rejects the whole spec on an unknown field, silently falling the session back to the default agent.

It now runs at _persist_spec, the same chokepoint the governance sanitize uses, so create and update are both covered and a future writer cannot reach the file without it. Already off the loop there, so the sync helper is called directly.

Design — the credential screen re-judged stored values

Correct and fixed; this one made the feature unusable for a real population. The dialog re-submits loaded mcpServers verbatim, so a template hand-authored with a literal token 400d on any edit — a description-only change included — with no in-dialog way to move the secret out. Hand-authored templates are exactly what this PR exists to replace.

The screen now judges what the request changes: a value byte-identical to the stored spec at the same location is exempt, because it is not one this request introduces. The protective invariant is unchanged and tested in both directions — a rotated literal is still refused, a credential on a server absent from the stored spec is still refused, and create (which has no stored spec) still screens everything. The stored read used for this is deliberately not load-bearing: the authoritative read for carry-forward stays inside the config lock, so this cannot reintroduce a lost update.

Fixing it surfaced a duplicate kiro_agents_dir_path() resolution per request, now resolved once so the screening read and the authoritative read cannot disagree about which file they mean.

Design — new public surface undocumented

Fixed. docs/system-specs/modules/learn-cron-dashboard.md now documents POST/PUT /api/agents/installed, the absent-means-preserve contract, every refusal class with its audit behaviour, the two chokepoint passes, and the managed listing field. Docs lint passes.

UX — MCP add path can never succeed on create

Independently found the same dead end GPT did, and prescribed either hiding the add row on create or collecting a command/URL. Round 12 took the second option, which is the one that keeps the capability: the row now has a transport field, http(s):// routing to url and anything else to command.

UX — remaining items

• edit/clone untranslated in 11 locales: not reproduced as described on this branch — the menu labels resolve through the localized edit_template/clone_template keys, which this PR added and translated in all 12 catalogs. If the reviewer is pointing at a different pair of keys still reading English, name the exact key path and I will translate it; the i18n gate's diff-scoped changed-passthrough check passes at 18/18, which it would not if a key this PR touched were English in a non-Latin locale.
• Auto-approve state is colour-only: legitimate accessibility finding, not fixed in this round. It is a visual-design change to a shipped chip (adding a filled shield or an "auto" suffix), and I would rather it be a deliberate design choice than a reviewer-round guess.
• Escape discards an authored template: legitimate, not fixed. A dirty-guard confirm is a real interaction addition and belongs with the item above.
• Clone for managed templates (suggestion): the sharpest observation in the review — a fresh install has only managed specs, so the !listed.managed gate hides Edit and Clone from first-time users, while the backend's own 403 copy tells them to "Clone it to get an editable copy", a path the UI never offers. That is a product decision about first-run behaviour, so I am flagging rather than deciding it.
• title on a disabled Radix item is unreachable: correct (pointer-events are suppressed), so the "Details could not be loaded" hint never renders. Grouped with the two UI items above.

Gates on 6af21084d

181 targeted backend tests (268 across the wider agent/credential/error-code suites before the rebase), mypy 1008 files, black/flake8/isort/brand/docs-lint clean, error-code ratchet green, tsc clean, eslint 0 errors, i18n 18/18, and 631 tests across the template-payload, agents-page and i18n specs. Seven mutations revert-verified this round (3 Design, 2 concurrency, 2 MCP transport).

One process note: black reformatted three regions of pre-existing code in agents.py that I had not touched. Reverted those three hunks — the file is in the known-unformatted baseline, so formatting it is not required and would have added unrelated churn to the diff.

[kyleseaman]

Head is now d2e4b04c2, 0 behind main. Two of the three blocking findings on 6af21084d are fixed; the third I am rebutting with a measurement, because implementing it as prescribed measurably breaks a guarantee an earlier round of this same review established.

Worker thread races the loop-owned session map (agents.py:2782) — FIXED

Correct, and a regression I introduced in round 13. _live_advertised_model_ids walks the live-session map, which the event loop owns and mutates. Before round 13 it was evaluated as an argument on the loop; wrapping the build in _read_then_build moved it inside the executor, so a concurrent session removal makes a worker thread iterate a mutating dict — RuntimeError, HTTP 500.

Resolved on the loop and captured before entering the executor, as prescribed. Pinned by thread identity: the lookup must run on the loop thread, and reverting the hoist fails that test.

Pending selection leaves the previous template editable (AgentsPage.tsx:870) — FIXED

Correct. select() cleared the destructive confirmDelete synchronously but left detailComplete set from the previously loaded template, so while B's detail was in flight the Edit/Clone buttons stayed live and acted on A — after the user had already asked for B. setDetailComplete(false) now happens synchronously alongside the other resets, and is set again only when a detail load actually lands.

Preliminary read can overwrite a concurrent external save (agents.py:2779) — REBUTTED, with a narrower fix applied

I implemented the prescribed fix first and it broke an existing guarantee. Reporting the measurement rather than the conclusion:

Stamping the version at the earliest read and refusing any change since makes two lock-serialized dashboard PUTs conflict with each other. test_two_disjoint_updates_both_survive (added in round 6, for GPT's own lost-update finding) went 200 → 409: both requests stamp before taking the lock, the first writes, and the second then sees a changed file and refuses — when the config lock plus the in-lock re-read is exactly the mechanism that lets both survive. Trading a real, tested guarantee for a narrower one is not a fix.

The underlying reason the prescription cannot work here: the server cannot distinguish "the client authored this against the version it loaded" from "the client authored it against a newer version", because the client sends no version. Server-side read timing is a proxy for client intent, and it is wrong in both directions — it rejects serialized writes that are safe, and it still cannot catch a client that loaded the file minutes earlier. The real fix is an If-Match/version on the request, which this endpoint does not yet accept and which is a contract change to the dialog as well.

What the pre-lock read is actually relied upon for is narrower than the finding assumes: the credential-screen exemption, which asserts "this value is identical to what is stored". That claim can genuinely go stale, and staleness there is security-relevant — a rotated secret could be written back unscreened. So:

• The builder now records every value it exempted, with its path.
• _prepare re-validates each exemption against the authoritative in-lock read and returns agent_template_conflict if any no longer matches.
• The write-time CAS stamp is taken inside the lock, catching a save landing during the write window.

The result is precise: rotating a secret in the window between the two reads is refused with a 409 and the rotated value survives on disk, while an edit that exempts nothing — a description change — cannot be turned into a spurious conflict. Both directions are tested, and all three mechanisms fail their paired test when patched out (3/3 mutations caught).

What remains unguarded, stated plainly: a disjoint external edit landing between the screening read and the in-lock read is absorbed rather than refused — the in-lock read sees it, carry-forward preserves the fields the client did not author, and the client's own fields win. That is the same semantics two dashboard PUTs already have, and closing it needs the client-supplied version above, not tighter server-side timing. Happy to add If-Match as a follow-up if you want the stronger property; it is a deliberate API addition rather than something to slip into a review round.

Gates on d2e4b04c2

268 targeted backend tests (178 in the authoring suite), mypy 1008 files, black/flake8/isort clean, error-code ratchet green, tsc clean, eslint 0 errors, 34 tests across the agents-page and template-payload specs.

[kyleseaman]

Head is now 2f73a74b7 (rebased, 0 behind). All three blocking findings on d2e4b04c2 are fixed and revert-verified (3/3 mutations caught).

Removing a tool retained its auto-approval (AgentTemplateCreator.tsx:353) — correct, and the most serious of the three. allowedTools IS the auto-approve list, so an entry left behind after its tool is removed is a live privilege, not dead state: remove an approved tool, re-add it later, and confirmation-skipping silently returns without the user re-granting it. removeTool now drops the grant in the same handler.

Dialog reset retained the previous MCP command (AgentTemplateCreator.tsx:196) — correct, and my own round-12 regression. I added mcpCommand state with the transport fix and wired it into addMcpServer's reset but not the dialog-open reset, so cancelling with a command typed carried it into the next template.

Exclusive create published an incomplete file (agents.py:2053) — correct. os.open(O_EXCL) then writing publishes the path before the bytes exist, so a crash mid-write leaves truncated JSON under a name kiro-cli will load, and an unparsable spec takes the whole agent down rather than failing one request. Content is now staged in a temp file and published with a single os.link, which is atomic and fails outright when the name exists — so both properties hold at once and the file is never visible partially written. Filesystems without hardlinks fall back to the previous direct create, keeping exclusivity and losing only crash-atomicity, which is better than refusing to create a template there.

Two of my three tests were false passes on the first attempt, worth recording since the corrections are the interesting part:

• The atomicity test raised OSError mid-write — which proves nothing, because the old direct path unlinked on a caught exception. The real vulnerability is a crash, where no cleanup runs. Rewritten to assert the observable property instead: while the bytes are being written, the published name must not exist. That discriminates the two implementations without needing a crash.
• The mcpCommand test unmounted and re-rendered, so fresh useState was empty regardless of the reset effect — vacuous. Rewritten to re-drive the same mounted component through the effect by changing editTarget, one of its deps.

My first two mutations were also aimed wrongly (one landed after staging had already happened), so the harness reported false passes on correct code until the mutation targeted which implementation the call site uses.

Gates on 2f73a74b7: 187 targeted backend tests, mypy 1008 files, black/flake8/isort clean, error-code ratchet green, tsc clean, eslint 0 errors, 13 template-payload specs.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[kyleseaman]

Head is now d2e20250e (rebased, 0 behind). All three blocking findings on 2f73a74b7 are fixed.

Template mutations lack owner authorization (agents.py:2680, :2821)

Correct and fixed. Both handlers install agent tools and MCP server commands into ~/.kiro/agents/, which every session on the machine resolves against — so the write surface is global even though the request is per-session. Both now refuse a non-owner with 403 owner_required before the body is parsed, so a non-owner cannot even exercise the validators, and the denial is SEL-audited in the mcp_apps.py shape.

Two things worth stating rather than burying:

The gate broke 166 of my own tests, which is the point. The test request fixture had never constructed an owner request. I made the fixture a genuine owner request (request["app"] == "", request["user"] == "local-app", no configured owner_id) rather than patching the check out, so every existing test now exercises the real gate, and added explicit non-owner refusal tests for both endpoints.

The same gap exists on ten pre-existing endpoints in this file and I have deliberately not touched them: api_agent_config's PUT (which writes the same spec files), api_default_agent, four capability install/uninstall handlers, _mutate_agent_package, api_agent_detail's DELETE, and the two api_kirocrew_agent* handlers. Changing ten unrelated handlers' authorization inside a feature PR would be unreviewable and outside its stated scope, so it is filed as #4944 with the full list. Flagging it explicitly because gating the new endpoints while api_agent_config stays open leaves the boundary inconsistent, and that inconsistency is now tracked rather than invisible.

Fallback can publish truncated JSON (agents.py:2126)

Correct, and it reverses my own round-15 judgment. I had kept a direct-write fallback for filesystems without hardlinks, reasoning it was "better than refusing to create a template at all". That was the wrong trade: a refused request is recoverable, a truncated spec on disk is not — kiro-cli rejects the whole spec and the session silently falls back to the default agent. The fallback now returns agent_template_write_failed and logs why.

That made _write_spec_exclusive_direct unreachable, so it is deleted rather than left as dead code.

Windows update drops owner-only ACL (agents.py:2056)

Correct, and it caught a path I had missed. Round 15 chmod'd the staged file on the create path only. The update path stages via mkstemp and replaces, and the replacement inherits the staged file's permissions — so an edit silently widened access on a spec that was restricted when created. On Windows the staged file also inherits the parent directory ACL, which chmod does not touch at all.

Both writers now call platform_compat.restrict_to_owner on the staged file before publishing, which is the repo's own cross-platform helper for exactly this. Two tests assert the restriction happens on create and on update.

Gates on d2e20250e

191 targeted backend tests (185 in the authoring suite), mypy 1008 files, black/flake8/isort clean, error-code ratchet green.

[kyleseaman]

Head is now 7e7c1dd0f (rebased, 0 behind). All three blocking findings on d2e20250e are fixed and revert-verified (4/4 mutations caught across the two pushes).

Unchanged stale fields overwrite newer edits (AgentTemplateCreator.tsx:339)

Correct, and it is the right answer to something I had called unsolvable. In round 14 I argued the server cannot distinguish "the client authored this against the version it loaded" from "against a newer one", and that closing it needed a client-supplied If-Match. That was true of the server, and I stopped there instead of asking what the client could do. Sending only changed fields closes the same vector with no API change — a better fix than the one I proposed.

An edit now submits a field only when the user changed it. The comparison is against the values the record loaded, normalised into the shape submit builds, so it is like-for-like. Combined with absent-means-preserve, a field this user never touched is omitted and an external edit to it survives.

The subtle half is handled as the finding asked: clearing is a change, so an emptied field is still sent, while a field that never loaded and is still empty stays omitted — which preserves the round-5 protection against a failed detail fetch erasing config. Three tests pin the three cases (nothing touched sends only name; one edit sends only that field; a cleared field is sent).

Two existing tests failed, and they were right to. They asserted an untouched mcpServers block is re-sent verbatim — the old transport of the "unmodelled fields survive a round trip" guarantee. Under the new rule the block is omitted and those fields survive server-side instead, with nothing to clobber. I rewrote both to assert the new mechanism and kept the original protection where it still applies: when the block genuinely changes, an untouched entry must still carry its command and its space-containing argv verbatim. Not weakened to green.

One bug of my own, caught by those tests: submit maps the auto model sentinel to '', but I stored the raw 'auto' as the loaded value, so an untouched model always compared unequal and leaked into every payload. Both sides are now normalised identically.

Coarse timestamps allow lost concurrent writes (agents.py:2045)

Correct, and a real weakness in my own round-13 token. st_mtime_ns reports nanoseconds but many filesystems only STORE coarse timestamps — ext3 and some network mounts at one second, FAT at two — so a same-size save inside one tick compared equal on mtime and size and the conflict check waved the overwrite through. The token is now (st_mtime_ns, st_size, sha256). It costs one extra read of a file this handler already reads, off the loop, which is cheap against silently losing an edit.

The test forces exactly that collision rather than hoping for it: same byte length, then os.utime restoring the original mtime, with assertions that mtime and size really are equal before asserting the token still differs — so it cannot pass for the wrong reason.

Mixed MCP transports are persisted (agents.py:2577)

Correct. My rule was "at least one" transport, so both command and url passed. stdio and http are alternatives rather than a pair, and kiro-cli refuses the whole spec on an unusable server, silently falling the session back to the default agent. Now exactly one, with the both-set case refused explicitly.

Gates on 7e7c1dd0f

196 targeted backend tests, mypy 1008 files, black/flake8/isort clean, error-code ratchet green, tsc clean, eslint 0 errors, 17 template-payload specs and 639 tests across the agents-page and i18n suites.

Still open and deliberately not fixed autonomously, unchanged from the round-13 disposition: the colour-only auto-approve state, an Escape-discards-draft guard, and whether Clone should be offered for managed templates (the first-run question, since a fresh install has only managed specs).

[bolichen97]

Scope coordination note with #5161: both branches edit the agent mutators in dashboard/handlers/agents.py (api_agent_config, api_agent_detail, create/update/capability routes) and their owner-authorization tests. This PR's distinct scope is Agent Template authoring (create/edit/clone) plus its credential/resource security predicates; #5161's residual scope is broad owner-gating for still-uncovered MCP/config routes.

Please preserve the authoring and security-predicate behavior here, but rebase onto the shared owner-helper baseline and avoid carrying a second copy of the agent mutation gates already merged via #5011/#6245. That keeps template semantics separate from the residual authorization sweep.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #8307 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8307: MERGE_DISCUSSION. Two open PRs are independently building 'copy a template so you can edit it', with different naming models (invisible auto-named fork vs explicit Clone), different validation stories (no screening vs a single screened chokepoint that refuses managed specs), and overlapping hunks in handlers/agents.py, AgentsPage.tsx and 12 locale catalogs. Both can ship, but the authoring model and the write chokepoint should be agreed before either lands. Files: src/kiro_crew/dashboard/handlers/agents.py, website/src/pages/AgentsPage.tsx, website/src/i18n/locales/zh-CN.json.
• This PR is PARTIALLY_COVERED with PR #5011. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #2383: REBASE. Only the incidental owner-gate implementation is now redundant; the create/edit/clone surface and the security predicates remain entirely absent from main. Rebase onto the shared owner helper and drop the inline copy rather than closing anything. Files: src/kiro_crew/dashboard/handlers/agents.py.
• This PR is PARTIALLY_COVERED with PR #6245. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #2383: REBASE. The gate the PR wrote by hand is now a shared helper with a different error code; align on rebase. Nothing about the feature itself is superseded. Files: test/test_agent_config_owner_gate_invariant.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[bolichen97]

@qh2244 the 2026-09-08 open-PR audit found two open PRs that touch the same code as this one.

#8307 (@xuejinT) — same feature, incompatible design. Shared: src/kiro_crew/dashboard/handlers/agents.py, src/kiro_crew/dashboard/routes/agents.py, src/kiro_crew/agent_discovery.py, src/kiro_crew/agent_state.py, website/src/api/client.ts, website/src/pages/AgentsPage.tsx, website/src/components/AgentSkillsEditor.tsx, and the locale catalogs. Both reinvent the same sub-mechanisms: a _TEMPLATE_NAME_RE, a declared-name-plus-filename collision scan, an exclusive create (_write_spec_exclusive here, _write_spec_file there), and one writer chokepoint calling sanitize_agent_config_governance and lift_and_strip_bookkeeping. The scopes differ: this PR adds create-from-scratch plus full field authoring (prompt, tools, allowedTools, mcpServers, resources, skills) and Clone; #8307 adds no create and no field editors, only fork-on-edit, publish and reset. #8307 is further along and already rebased onto the post-split src/kiro_crew/security/ package, while this branch still appends to the deleted src/kiro_crew/security.py and is behind main with conflicts. Suggestion: land #8307 first, then port this PR's validator, credential predicates and dialog onto its fork/publish model, so the roster does not get two competing copy paths.

#6307 (@kyleseaman) — one real conflict. Both rewrite the same lines of _entitled_kiro_models: this PR moves the available_models walk out into _live_advertised_model_ids, #6307 replaces it in place with a direct provider.available_models() call. #6307 also inserts a route immediately after the same add_get("/api/agents/installed") line this PR inserts after. Whoever lands second should keep the direct call inside the extracted helper.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.