refactor(agents): render the template pane unconditionally and remove the standalone Agent Templates page - #9936
Conversation
|
Intent: Remove the standalone Agent Templates page now that #8307 put template editing inside each agent's editor, leaving one place to manage a template. |
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound consolidation with a real upgrade story, but retiring the only guarded template-delete path leaves reference integrity unenforced. Watch
Suggestions
[DESIGN-REVIEWED] 4b3a6c6 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of UX-Verdict: CONCERNS The now-always-on Template pane ships with its source chip stranded over the explainer copy in both committed screenshots — first-time readers see it as broken. Watch
Evidence gaps
[UX-REVIEWED] 4b3a6c6 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All verification done. Producing the review. First-Principles-Verdict: CONCERNS Template delete now rests on an unverified kiro-cli claim: that a missing spec "falls back to the default agent spec" instead of failing session start. What this change shipsInventory (10 items) — 10 justifiedIntent: make the in-agent Template pane the only template UI — on upgrades too — and delete the duplicate standalone page. FIX (of a two-doors duplication left half-finished by #8307).
More items exist below the cap (65 i18n keys pruned across 13 locales, two capture harnesses deleted, baselines regenerated, screenshots committed per the Counts run: Watch
[FIRST-PRINCIPLES-REVIEWED] 4b3a6c6 |
4b319d7 to
5b92a7d
Compare
5b92a7d to
ebd49a7
Compare
ebd49a7 to
cea9b1b
Compare
span=4659357888e5
Disposition: fixed in cea9b1b.
|
span=48bcd838f698
|
span=b028d969143a
|
span=c58c60f99e9e
|
span=0fca2def5eb3
|
span=ea1ae43ec34b
|
span=25d648f6b07f
|
span=85ab384fdef7
|
span=2f6082500a24
Disposition: fixed in c53c1f2.
|
span=063a9298bf87
|
span=e002a7fcabdc
|
… the standalone Agent Templates page Deletes the agent_template_pane flag end-to-end (a stale materialized false in existing configs made a default flip inert), renders the in-agent Template pane unconditionally, removes the standalone page, prunes retired nav pins, and deletes the retired surfaces' server routes (sessions/context, spawn clear, agent-detail DELETE).
c53c1f2 to
4b3a6c6
Compare
span=abc80c2c9464
Disposition: rebutted (not a defect) — the finding's failure chain does not occur; the doc is corrected in 4b3a6c6 to state the real behavior.
For a human override if this recurs: /ai-review override gpt 4b3a6c6: on-disk deletion of a bound template cannot break sessions — kiro-cli silently falls back to the default agent spec; degradation is documented in agents.md and the dashboard spec, and the design owner accepted retiring the guarded route. |
span=76a4e6fdfdbe
Disposition: fixed in 4b3a6c6 — the second Clears-when branch is now satisfied in full.
|
span=76f35a762ba7
|
span=ea808e6d2ee8
|
span=a2d25b627c3d
|
span=ecdbdbb9a664
|
span=d2b3475bf17b
|
span=e2683a38363f
Disposition: fixed in 4b3a6c6.
|
span=92c862fa3095
Disposition: fixed in 4b3a6c6.
|
span=a5a12402e0cd
Disposition: rebutted (confirmed by direct test) — the finding's Clears-when is met: the fallback was exercised on kiro-cli itself.
|
span=c7911bd9b25a
Disposition: rebutted — the finding's second Clears-when branch is met: the owner explicitly accepts dangling bindings as tolerable, and the failure mode is documented.
|
span=9cd4ef2b49cf
|
span=03bfe4b1b1a4
|
span=8271f286c82e
|
Problem / Motivation
The dashboard had two places for an agent's template. The standalone Agent Templates page (a Capabilities tab) showed a template's definition, and #8307 added the same definition inside each agent's Template pane — but behind the
agent_template_paneflag, which defaulted off. So for most users the in-agent pane never rendered, and the standalone page was still the only template UI.Why it matters
Two doors to the same thing is a navigation tax, and the page carries ~1,100 lines that must be kept in sync with the pane that mirrors it. But the page can't simply be deleted while its replacement is dark — that would strand template viewing/editing (exactly what the first review round on this branch caught). And flipping the flag's default is not enough either: older builds materialized
agent_template_pane: falseinto every savedconfig.json, so a new default never reaches an existing install (the second round's design blocker).What changed (motivation → approach → change)
Delete the flag entirely, render the pane unconditionally, then remove the page — one coherent change so there is never a window with no template UI, on fresh installs and upgrades alike.
agent_template_paneis removed from the config dataclass, the parse,to_dict(),config-baseline.json(regenerated for only this diff's entries), and the frontend (useAgentTemplatePane.tsdeleted;KiroCrewAgentsPage.tsximportsAgentTemplateDetailstatically and renders it whenever the Template pane is open — no lazy chunk, so no chunk-load failure path). The pane sits inside aretryOnlyErrorBoundary: a render crash shows a contained Try Again in the pane — no dashboard blank-out, and no /chat hand-off that would discard the sheet's unsaved edits. A stale materializedfalsein an existing config is a retired key: ignored at load, dropped on the next save, never warned about (CONFIG_RESERVED_TOP_KEYS), pinned bytest_retired_key_is_ignored_dropped_and_never_warned. No migration needed — nothing reads the key anymore.AgentsPage.tsxand its 7 suites, the Capabilitiestemplatestab, the pinnable-surface entry, and repoints the two chat "Manage agents" footers at?tab=crews./agentsalready redirects. The page's default-template picker goes with it — the same control survives in the create form and the chat agent dropdown.readNavPinnednow drops retired surface ids before the cap applies (navPinned.retiredIds.test.ts).GET /api/sessions/context(plus the now-consumerlesscontext_info()),DELETE /api/spawn, and theDELETE /api/agents/detail/{name}branch with its reference-guard test. Template delete is done by removing the JSON file on disk.pages.agentsPage.*i18n keys plus theirpluralKeys.json/en.context.jsonleftovers, drops the stale destructive-confirm exemption, deletes the two capture harnesses that targeted the removed tab (capture-agent-templates.mjs,capture-agent-skills.mjs— the pane harness covers their subjects), and updatesagents.md, the feature map, and the dashboard spec.flowchart LR subgraph Before A1[open agent]:::ctx --> F1{agent_template_pane?}:::removed F1 -->|off, the default| P1[picker only]:::removed F1 -->|on| P2[full definition]:::ctx A2[Agent Templates page]:::removed --> D1[full definition]:::ctx end subgraph After A3[open agent]:::ctx --> P3[Template pane: full definition, always]:::added end classDef added fill:#DCFCE7,stroke:#16A34A,color:#14532D,stroke-width:2px classDef removed fill:#FEE2E2,stroke:#DC2626,color:#7F1D1D,stroke-dasharray:4 3 classDef ctx fill:#E0F2FE,stroke:#0284C7,color:#0C4A6E linkStyle 0,1,2,3 stroke:#DC2626,stroke-dasharray:4 3 linkStyle 4 stroke:#16A34A,stroke-width:2px🟩 added · 🟥 removed · 🟦 unchanged
The flag and the standalone page are both gone; every agent's Template pane shows the full definition, for existing installs too.
Tests
test_retired_key_is_ignored_dropped_and_never_warnedpins the upgrade path: a config carrying a staleagent_template_pane: falseloads with no field, no round-trip, and no unrecognized-key warning.navPinned.retiredIds.test.tspins the ghost-pin prune: a retired pin is dropped before the 5-slot cap so remaining pin controls stay usable.test_config_baseline.pyparity passes with the row removed. The page's 7 frontend suites and the DELETE-route guard test are deleted with their subjects;test_api_server, owner-auth, messaging-coverage, and the session suites are updated for the removed routes and pass (798 Python tests green across the touched suites; 191 affected vitest tests green).Manual verification
npx tsc -bclean;i18n-check+check-i18n-keysclean;flake8/isortclean on the changed Python. The capture harness (capture-agent-template-pane.mjs) runs the real built SPA with no flag stub and its self-checks pass — the pane renders unconditionally.Screenshots / video
The in-agent Template pane, now always on — it renders the full definition (model, skills, tools, auto-approved, MCP servers, system prompt, guardrails) inline, which is what makes the standalone page redundant.
Built-in template (
kirocrew, the default agent):A custom template (
atlas):The two frames are committed under
temp-screenshots/agent-template-pane/(the repo’s screenshot convention;cleanup-temp-screenshots.ymlprunes them on schedule). Captured with the repo'scapture-agent-template-pane.mjsharness (real built SPA, fixture-stubbed API, no flag stub). The removed standalone tab's "after" is its absence from the Capabilities rail; the pane above is the replacement users get instead.Related Issues
no linked issue: completes the #8307 in-agent template pane by making it unconditional, then removing the page it replaces.
Checklist