SK-697: Add agent asset support for Kiro client - #206
Conversation
Installs agents to ~/.kiro/agents/ in two formats simultaneously:
- {name}.md — IDE + CLI v3 (YAML frontmatter + system prompt body)
- {name}.json — CLI v2 default (JSON with prompt field)
Field routing: known v2-only fields (allowedTools, toolAliases, toolsSettings,
hooks, includeMcpJson, keyboardShortcut) are excluded from .md; permissions is
excluded from .json (CLI v2 has no permissions model). Unknown fields pass
through to both formats for forward compatibility — verified by live harness
test against kiro-cli v2 and v3 engines.
Separate third-party yaml import into its own group (gci). Replace sb.WriteString(string(out)) with sb.Write(out) (mirror).
1. writeCLIFormat: guard reserved keys (name, prompt, description) in the
[agent.kiro] passthrough loop so they cannot overwrite Asset metadata
or zip content
2. permissions: tighten type guard from !isList||len>0 to isList&&len>0 so
a TOML table ([agent.kiro.permissions]) is rejected instead of silently
producing the wrong schema {rules:{key:val}}
3. model: write with %q (same as description) so values containing ": " do
not produce invalid YAML
4. readAgentContent: fallback tries canonical "AGENT.md" (was lowercase
"agent.md"); error now names both files tried, mirroring
opencode/handlers/rule.go
5. Promote mdV2Only and mdExplicitFields to package-level vars (same pattern
as hook event maps in every other handler)
- Accept []map[string]any permissions from TOML decode - Reject table-shaped permissions with guidance - Reserve "default" agent name to protect hooks config - Verify both agent files, propagate YAML errors
|
Claude finished @detkin's task in 2m 25s —— View job Solid, well-documented PR — the review fixes (permissions decode normalization, reserved [MEDIUM] No black-box integration test exercising agent install through the Kiro clientLocation: Issue: The Suggestion: Add an agent case to the Kiro integration test suite that installs a real agent bundle via the client for global + repo scope and asserts both [LOW] Known fields with an unexpected type are silently dropped from the
|
- Black-box agent install/uninstall tests via Kiro client - Mistyped known fields fall through to pass-through - Clearer error when default prompt file is missing
|
Claude finished @detkin's task in 2m 45s —— View job PR Review — SK-697: Kiro agent asset supportThe four issues from the prior review are all correctly addressed — silent-type-drops now fall through to pass-through ( [LOW] A
|
- Never pass name/description through from [agent.kiro] - Fix stale scope comment in integration test
|
Claude finished @detkin's task in 1m 37s —— View job PR Review — SK-697: Kiro agent asset supportAll findings from the previous review rounds are correctly resolved — silent type-drops fall through to pass-through, [LOW] A
|
|
Claude finished @detkin's task in 1m 33s —— View job PR Review — SK-697: Kiro agent asset supportAll findings from the four prior review rounds are correctly resolved and the latest commit closes the last one — Verification of prior findings against the current code:
Review complete. No issues found. |
2 participants
Summary
Mirrors #204 (thanks @tmack8001!) onto a maintainer branch and fixes the issues found in review. Trevor's three commits are preserved as-is; the fixes land on top.
type = "agent"support to the Kiro client with dual-write output:{name}.md(IDE / CLI v3, YAML frontmatter) and{name}.json(CLI v2) in.kiro/agents/— see SK-697 Add agent asset support for Kiro client #204 for the full design and live-harness validation[[agent.kiro.permissions]]insidemap[string]anyas[]map[string]any, not[]any, so permissions were silently dropped from the.mdoutput on the real TOML parse path; both shapes are now normalized (covered by a real-TOML round-trip test)[agent.kiro.permissions]with guidance instead of silently dropping itdefault—.kiro/agents/default.jsonis the sx-managed Kiro CLI hooks config; install rejects the name and remove never deletes that fileVerifyInstallednow requires both output files so a missing half gets repairedyaml.Marshal(correct YAML escaping) and marshal errors propagate instead of silently dropping fieldsCloses #204.
Security implications of changes have been considered