fix(kiro): translate MCP keys, round-trip hook enabled, keep skill metadata - #2589
Merged
Conversation
…tadata
Three independent fidelity bugs in the Kiro adapters:
- MCP: the Rulesync-only kiroAutoApprove/kiroAutoBlock keys were serialized
verbatim into .kiro/settings/mcp.json, where Kiro ignores them. Translate
them onto the documented autoApprove/disabledTools fields (merging with a
natively spelled list rather than overwriting it) and lift autoApprove back
into kiroAutoApprove on import.
- Hooks: KiroIdeHookEntrySchema parsed enabled but the canonical converter
dropped it and generation hardcoded enabled: true, so importing a disabled
hook and regenerating silently reactivated it. Add enabled to the canonical
HookDefinition and round-trip it.
- Skills: KiroSkill projected frontmatter down to {name, description},
dropping license/compatibility/metadata. Carry residual keys through a new
kiro: section, following the copilot-skill residual-key spread pattern.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Only rename a genuine string array on MCP import: a hand-written "autoApprove": "all" would otherwise land in kiroAutoApprove, which is typed as a string array, producing a .rulesync/mcp.jsonc the next generate refuses to parse. - Keep an explicitly authored empty tool list instead of dropping it, so import then generate stays idempotent. - Document that kiroAutoBlock is a redundant spelling of the canonical disabledTools and has no import counterpart, so a round-trip widens its scope to every target that supports disabledTools. - Warn at generate time when a hook carries enabled: false and the target is not kiro-ide, since every other target emits it as an ordinary active hook. - Correct the claim that Kiro IDE is the only tool with an on-disk enable flag; Antigravity has one, but on the named hook group rather than the individual definition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The warning fired for events the target does not support, contradicting the "Skipped hook event(s)" message logged for the very same event, and for tool-native enabled keys inside an override block, which the tool itself honors. Look at the shared canonical block only and skip events already reported as unsupported. Also assert the full warning message in the test rather than a substring that any other warning could satisfy, and spell out in the docs that a kiroAutoBlock round-trip widens the denylist to every target supporting disabledTools. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 6, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #2408
Three verified, independent fidelity bugs in the Kiro adapters. Design gaps in the issue (the
permissions.yamlemitter, global permission scope, the universal V2+V3 agent format) are untouched, so the issue stays open.1. MCP key translation
src/types/mcp.tsdefines the Rulesync-onlykiroAutoApprove/kiroAutoBlockkeys, butKiroMcp.fromRulesyncMcpserialized every server verbatim, so those literal key names landed in.kiro/settings/mcp.jsonwhere Kiro ignores them.Per the MCP configuration docs, the real per-server fields are
autoApprove(tools that run without a confirmation prompt) anddisabledTools(tools hidden from the agent). Both now translate:kiroAutoApprove→autoApprove,kiroAutoBlock→disabledToolsautoApprove/disabledToolsnatively keeps working — the two lists are merged, not overwrittenautoApprove→kiroAutoApprove.disabledToolsis left alone because it is already a canonical Rulesync field with the same meaning, sokiroAutoBlockdeliberately has no import counterpart (the round-trip is still stable).2. Hook
enabledround-tripKiroIdeHookEntrySchemaparsedenabled, butkiroIdeHooksToCanonicalnever copied it and generation hardcodedenabled: true— importing a deliberately disabled hook and regenerating silently switched it back on.enabledis now an optional boolean on the canonicalHookDefinition, round-tripped by the Kiro IDE emitter only.trueis Kiro's default, so only an explicitfalseis written back into the canonical file; no other hook target reads or writes the field.3. Skills fidelity
KiroSkill.toRulesyncSkill/fromRulesyncSkillprojected frontmatter down to{name, description}, so an importedlicense/compatibility/metadatawas erased on the next generate. Kiro's skills docs document all three.A
kiro:section is added to the Rulesync skill frontmatter, and the adapter uses the residual-key spread pattern from PR #2587 (copilot-skill.ts) rather than enumerating known keys — so any future field a hand-writtenSKILL.mdcarries also survives the round-trip.Verification
pnpm cicheckgreen (8355 unit tests, format/lint/types, cspell/secretlint, docs-content + skill-docs sync).npx vitest run --config vitest.e2e.config.ts src/e2e/e2e-mcp.spec.ts src/e2e/e2e-hooks.spec.ts src/e2e/e2e-skills.spec.ts— 324 passed.