fix(hooks): emit Kiro v1 hook documents#2095
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the Kiro hook integration to emit Kiro IDE/CLI 1.0’s v1 hook document schema ({"version":"v1","hooks":[...]}) instead of the legacy when/then format, and allows Kiro-targeted packages to provide native v1 hook arrays that APM can rewrite/deploy.
Changes:
- Canonicalizes Kiro event names to v1 PascalCase trigger names and updates casing expectations.
- Adds a Kiro-only parser opt-in for native v1
hooks: [...]documents and normalizes them into the internal event-map shape. - Updates unit tests and documentation to assert and describe the v1 output contract and native v1 input support.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/integration/hook_integrator.py |
Updates Kiro event mapping to v1 triggers and adds _parse_hook_json(..., allow_kiro_v1=...) support. |
src/apm_cli/integration/kiro_hook_integrator.py |
Implements v1 document rendering, matcher/action translation, and native-v1 normalization on the Kiro path. |
tests/unit/integration/test_kiro_target.py |
Replaces legacy-schema assertions with exact v1 documents and adds regression coverage for native v1 + trigger mapping. |
docs/src/content/docs/integrations/ide-tool-integration.md |
Documents Kiro v1 output and native v1 input acceptance at the integration overview level. |
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md |
Documents Kiro v1 authoring/output behavior and trigger passthrough for Kiro-only events. |
packages/apm-guide/.apm/skills/apm-usage/package-authoring.md |
Syncs agent-facing package-authoring guidance with the Kiro v1 hook document shape. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Low
| prompt = action.get("prompt") | ||
| if action.get("type") == "askAgent" or isinstance(prompt, str): | ||
| if action.get("type") in {"agent", "askAgent"} or isinstance(prompt, str): | ||
| prompt_text = prompt if isinstance(prompt, str) else action.get("command") | ||
| if isinstance(prompt_text, str) and prompt_text.strip(): | ||
| return {"type": "askAgent", "prompt": prompt_text} | ||
| return {"type": "agent", "prompt": prompt_text} |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Spec Guardian:
|
| Panel | Signal | Shocked | New B | New R | New N |
|---|---|---|---|---|---|
| Swagger / OpenAPI editor | ship_with_followups | 8/10 | 0 | 3 | 0 |
| OCI distribution editor | ship_with_followups | 8/10 | 0 | 3 | 0 |
| Package-manager contract editor | ship_with_followups | 7/10 | 0 | 4 | 0 |
| Web-platform architect | ship_with_followups | 7/10 | 0 | 3 | 0 |
B = new critical findings, R = new recommended findings, N = new nits. Counts are advisory signal only. The maintainer ships.
Convergent themes
- T1 -- Schema-version source: all four panels flagged the generic phrase "current registered schema" as undefined. Folded by making
req-tg-006explicitly require the Kiro v1 hook schema. - T2 -- Timeout structure: three panels asked where
timeoutlives. Folded by specifying an optional numerictimeoutinside a commandaction. - T3 -- Scope discipline: target-specific layering and future schema evolution are useful follow-ups, not requirements for this Kiro v1 correction.
Defer to a later amendment
- Define cross-target malformed native-input and unknown-field policy after the portable contract is designed.
- Revisit future target-schema version discovery when a registry or machine-readable target contract exists.
Linter notes (2 checks noted)
- [7] The checklist's mechanical heading-slug algorithm reports existing numbered-heading links because it strips punctuation differently from the documentation renderer; this amendment adds no heading links.
- [11] This PR also modifies Python implementation and tests, so the general code-review and CI lint paths apply in parallel.
Checks 1-6 and 9-10 pass; check 8 is not applicable because the spec has no Mermaid blocks. The full spec-conformance suite passes (131 passed, 2 skipped), orphan alignment reports 98 requirements, generated artifacts are clean, and Mode B delegates to orphan alignment for this spec-concurrent change.
Full per-panel findings
Swagger / OpenAPI editor -- shocked_meter 8/10
No critical issues. Recommended defining the schema-version source, locating timeout structurally, and keeping generic requirements separate from Kiro details.
OCI distribution editor -- shocked_meter 8/10
No critical issues. Recommended defining the schema-version source and timeout bounds; integrity-domain suggestions were outside this hook-schema change.
Package-manager contract editor -- shocked_meter 7/10
No critical issues. Recommended future malformed-input and unknown-field policy. Those cross-target contracts are deferred rather than inferred in this bug fix.
Web-platform architect -- shocked_meter 7/10
No critical issues. Recommended avoiding coupling generic normative language to Kiro version churn and making timeout structural. Both points were addressed by the scoped wording.
This panel is advisory. Re-apply the spec-review label to run it again.
Move timeout to the hook entry, preserve native v1 metadata, and add an empirical install regression test. Addresses review panel findings on Kiro runtime correctness and integration coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 2 | Trigger canonicalization folded; architecture remains target-local. |
| CLI Logging Expert | 0 | 1 | 1 | Native normalization now emits debug evidence. |
| DevX UX Expert | 0 | 1 | 2 | Empirical install coverage now protects the user promise. |
| Supply Chain Security Expert | 0 | 0 | 2 | Path containment and command rewriting remain intact. |
| OSS Growth Hacker | 0 | 0 | 2 | Changelog and a native Kiro example now lead with usable value. |
| Doc Writer | 2 | 0 | 0 | Runtime hierarchy and native field preservation corrected. |
| Test Coverage Expert | 0 | 1 | 0 | Real apm install --target kiro regression test committed. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Recommendation
All in-scope findings from this pass are folded on 5f8a6b11e. Focused evidence is 373 passed and 2 skipped; the mutation break changes generated version to v0 and makes the empirical install test fail on the exact runtime contract. Ship after CI confirms this SHA.
Full per-persona findings
- Runtime correctness: timeout moved from
actionto the hook entry; nativedescription,timeout, andenabledare preserved. - Architecture: native trigger aliases are canonicalized through the Kiro event map.
- Diagnostics: native v1 normalization logs file and hook count at debug level.
- Documentation: spec, producer guide, integration guide, package-authoring skill, and changelog now match implementation.
- Coverage:
tests/integration/test_kiro_hook_install_e2e.pyexecutes the real local install flow and preserves unrelated configuration. - Auth and performance personas were inactive because those surfaces are unchanged.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Clarify hook-level fields and log unsupported native actions. Addresses final panel polish findings without changing the runtime contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bound native v1 support to command and agent actions and warn when a file contributes neither. Addresses the final documentation panel findings with regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | Target adapter remains bounded and explicit. |
| CLI Logging Expert | 0 | 0 | 0 | Debug and warning paths match the install contract. |
| DevX UX Expert | 0 | 0 | 0 | Real install behavior and non-destructive configuration are proven. |
| Supply Chain Security Expert | 0 | 0 | 0 | Existing containment and rewrite guards remain intact. |
| OSS Growth Hacker | 0 | 0 | 0 | Changelog and runnable example are user-facing. |
| Doc Writer | 0 | 0 | 0 | Spec, docs, and implementation agree on supported v1 actions. |
| Test Coverage Expert | 0 | 0 | 0 | Integration and mutation-break gates are independently reproducible. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Recommendation
Ship now. Exact-SHA CI is green: https://github.com/microsoft/apm/actions/runs/29063433020. Local relevant tests report 145 passed and 2 skipped, and the full lint chain is silent. Mutation proofs: changing generated version from v1 to v0 fails the install e2e assertion; downgrading the unsupported-action warning to debug fails its regression test.
Full per-persona findings
No current-SHA findings remain. Auth and performance personas were inactive because those surfaces are unchanged.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Resolve release-note and OpenAPM revision-history conflicts while preserving both mainline Antigravity edits and the Kiro v1 contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Update integration helpers added on main to assert the Kiro v1 matcher, action, and document contracts rather than removed legacy symbols. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fold panel follow-ups by exercising command and agent actions through the real install flow, clarifying internal transport fields and warnings, and synchronizing user and agent documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fold the final documentation panel follow-ups by linking the overview to the canonical authoring guide and showing a portable matcher input. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | Target adapter remains bounded and explicit. |
| CLI Logging Expert | 0 | 0 | 0 | Debug and warning paths are consistent and actionable. |
| DevX UX Expert | 0 | 0 | 0 | Install behavior is unchanged except for correct Kiro output. |
| Supply Chain Security Expert | 0 | 0 | 0 | Containment, fail-closed parsing, sanitization, and 0600 modes remain intact. |
| OSS Growth Hacker | 0 | 0 | 0 | User-benefit release notes and a runnable example support adoption. |
| Doc Writer | 0 | 0 | 0 | Canonical docs, target matrix, package guidance, and spec agree. |
| Test Coverage Expert | 0 | 0 | 0 | The real install path is defended at e2e tier. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Recommendation
Ship now. Exact-SHA CI is green: https://github.com/microsoft/apm/actions/runs/29114188271. bash scripts/test-integration.sh reports 10206 passed, 172 skipped, 16 deselected, and 2 expected xfails. The full canonical lint chain is silent.
Regression-trap evidence
tests/integration/test_kiro_hook_install_e2e.py::test_install_transforms_kiro_v1_hook_and_preserves_unrelated_config executes the real subprocess install flow and validates command and agent v1 documents, script deployment, and unrelated-file preservation. Changing _kiro_hook_document from version: v1 to version: v0 makes that exact test fail; restoring the guard returns it to green.
Mergeability
a9cecce3507199095053b98874d98c30f0df4bb2 is mergeable with all checks green; required maintainer review remains pending.
Full per-persona findings
No current findings. Auth and performance personas were inactive because those surfaces are unchanged.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
# Conflicts: # docs/src/content/docs/specs/openapm-v0.1.md
Re-scope the Kiro hook target as an exact-Kiro v1 integrator. Native Kiro v1 documents are now read and re-emitted directly into Kiro-native results instead of round-tripping through the Claude-shaped event map via internal _kiro_* transport keys. - Remove _normalize_kiro_v1 and _with_kiro_hook_fields (side channels). - _kiro_action_from_action is a pure Kiro-native action converter. - _kiro_hook_document takes explicit native fields (name/trigger/matcher/ description/timeout/enabled), no popping of namespaced keys. - Native path rewrites each command via _rewrite_command_for_target (same security primitive the portable path uses), dedups scripts. - Portable path keeps _rewrite_hooks_data (in-place path rewrite of APM's declared input; identical shape in/out, no vendor intermediate). - Value-safe diagnostics unchanged (filenames/counts only). Update the runtime-coverage unit tests to the new helper signatures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f98f5a49-17a9-4ac9-a4ff-dd2995780839
Consolidate the three Unreleased "Fixed" blocks into one per Keep a Changelog, and record `closes #2071` on the Kiro hooks entry. Apply ruff formatting to the refactored kiro_hook_integrator module. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f98f5a49-17a9-4ac9-a4ff-dd2995780839
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 2 | Clean resolve-then-write refactor; _ResolvedKiroHook is a proper Kiro-native value (not a foreign IR); dual-resolver dispatch and _rewrite_hooks_data reuse are architecturally sound. |
| CLI Logging Expert | 0 | 0 | 2 | Value-safe diagnostics confirmed; all log/display paths use filenames and counts only, no secrets leak. |
| DevX UX Expert | 0 | 0 | 3 | No required changes; three ergonomics nits on warning wording, filename-canonicalization orphans, and matcher-syntax clarity. |
| Supply Chain Security Expert | 0 | 0 | 1 | All five security controls preserved through the refactor; no path-traversal, permission, or validation regression detected. |
| OSS Growth Hacker | 0 | 1 | 1 | Docs surfaces solid; CHANGELOG entry undersells the win -- reframe as 'Kiro hooks now work with Kiro 1.0'. |
| Doc Writer | 0 | 1 | 2 | Docs accurately describe the shipped Kiro v1 contract; no stale internals, spec/CHANGELOG correct. Minor precision nits only. |
| Test Coverage Expert | 0 | 0 | 1 | All critical hook surfaces defended: e2e regression trap, native v1 field preservation, prompt->agent, unsupported-action skip, event-map canonicalization covered; ship. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [OSS Growth Hacker] Reword the CHANGELOG entry to lead with the interop-fix headline ('Kiro hooks now emit the official v1 runtime shape...') and demote matchers/timeouts/toggles to supporting detail. -- Current wording buries the adoption-unblocking story. The user-facing win is broken-to-working, not a feature list. This shapes release-note framing.
- [Doc Writer] Fix the hooks-and-commands guide:
PreTaskExecandPostTaskExecare NOT native-only -- both are reachable from portable input via_HOOK_EVENT_MAP. Correct the Kiro-only set toPostFileCreate,PostFileSave,PostFileDelete, andSessionStart. -- Mislabeling can mislead producers into authoring native v1 shape when a portable event already reaches the same trigger, undermining the portable-by-manifest promise. - [Python Architect] Remove the unused
source_actionfield from_ResolvedKiroHookor add a reserved-for-future comment naming its planned consumer. -- Dead data on a value object is a minor code-smell; removing it now avoids confusion for future contributors reading the dataclass. - [Test Coverage Expert] Add a parametrize entry for
type:"agent"input alongside the existingtype:"askAgent"test on_kiro_action_from_action. -- Both inputs enter the same branch, so coverage is not at risk, but an explicit parametrize locks the passthrough contract for the nativetype:"agent"alias. - [CLI Logging Expert] Log
hook_file.name(actual filename) instead ofhook_file_dir.name(parent directory) in the debug path of_resolve_hooks_for_file. -- When multiple hook files share a parent directory, the current log line is ambiguous under--verbosedebugging.
Architecture
classDiagram
direction TB
class BaseIntegrator {
+check_collision(target_path, rel_path, managed, force)
}
class HookIntegrator {
+HOOK_COMMAND_KEYS tuple
+find_hook_files(install_path) list
+_parse_hook_json(hook_file, allow_kiro_v1) dict
+_rewrite_hooks_data(data, pkg_path, target) tuple
+_rewrite_command_for_target(cmd, pkg_path, target) tuple
+_summarize_command(entry) str
+_get_package_name(pkg_info) str
}
class IntegrationResult {
<<Dataclass>>
+files_integrated int
+files_skipped int
+target_paths list
+files_adopted int
+display_payloads list
}
class HookIntegrationResult {
+hooks_integrated property
}
class _MergeHookConfig {
<<Dataclass frozen>>
+config_filename str
+target_key str
+event_container_key str
}
class _ResolvedKiroHook {
<<ValueObject>>
+trigger str
+action dict
+source_action dict
+name str
+matcher str
+description str
+timeout int
+enabled bool
}
BaseIntegrator <|-- HookIntegrator
IntegrationResult <|-- HookIntegrationResult
HookIntegrator *-- _MergeHookConfig : merge targets
HookIntegrator ..> _ResolvedKiroHook : resolves into
HookIntegrator ..> HookIntegrationResult : returns
_ResolvedKiroHook ..> HookIntegrationResult : serialized via _write_resolved_hooks
note for _ResolvedKiroHook "Kiro-native value, NOT a foreign IR.\nCollect-then-render with\n_write_resolved_hooks."
class _ResolvedKiroHook:::touched
class HookIntegrator:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["integrate_kiro_hooks\nkiro_hook_integrator.py:406"] --> B["I/O find_hook_files\nhook_integrator.py"]
B --> C["_filter_hook_files_for_target\nhook_file_routing.py"]
C --> D["I/O _parse_hook_json allow_kiro_v1=True\nhook_integrator.py:636"]
D --> E{"_resolve_hooks_for_file\nhooks is list vs dict?"}
E -->|"Native v1 list"| F["_resolve_native_v1_hooks\n_kiro_action_from_action per entry"]
E -->|"Portable event dict"| G["I/O _rewrite_hooks_data\nhook_integrator.py:816\ndeep copy + path rewrite"]
F --> H{"action type?"}
H -->|"command"| I["I/O _rewrite_command_for_target\nper command path"]
H -->|"agent"| J["passthrough"]
I --> K["list of _ResolvedKiroHook"]
J --> K
G --> L["_resolve_portable_hooks\nevent map + _kiro_action_from_action"]
L --> K
K --> M["_write_resolved_hooks"]
M --> N{"check_collision\nbase_integrator.py"}
N -->|"No collision"| O["FS atomic_write_text\n0o600 permissions"]
N -->|"Collision"| P["files_skipped"]
O --> Q["FS _copy_scripts\ncopy_deployed_hook_bundle"]
Q --> R["HookIntegrationResult"]
Recommendation
Ship this PR as-is. All nine panelists signal clean, with zero blocking or required findings and full E2E regression coverage proving the v1 contract holds. The two recommended follow-ups -- CHANGELOG reframing and docs trigger-accuracy fix -- are worth addressing before the next release cut but do not affect runtime correctness or security. Track them as a post-merge polish pass or fold into the release-prep PR.
Full per-persona findings
Python Architect
- [nit]
_ResolvedKiroHook.source_actionis stored but never read anywhere atsrc/apm_cli/integration/kiro_hook_integrator.py:52
source_action is a required field set in both resolvers but no code path reads it. Dead data on a value object.
Suggested: Remove source_action from _ResolvedKiroHook and its two construction sites, or add a reserved-for-future comment. - [nit]
_ResolvedKiroHookcould be frozen for value-object safety atsrc/apm_cli/integration/kiro_hook_integrator.py:41
Created once, consumed read-only. @DataClass(frozen=True) enforces the value-object contract, consistent with _MergeHookConfig. Pure polish.
Suggested: Change @DataClass to @DataClass(frozen=True) on _ResolvedKiroHook.
CLI Logging Expert
- [nit] Debug log uses parent-directory name instead of the hook filename at
src/apm_cli/integration/kiro_hook_integrator.py:383
_resolve_hooks_for_file logs hook_file_dir.name (parent dir, often 'hooks') rather than the actual filename; ambiguous when multiple hook files share a directory.
Suggested: Log hook_file.name (thread the path into _resolve_hooks_for_file or log in the caller). - [nit] Warning for unsupported actions could hint at the two supported types at
src/apm_cli/integration/kiro_hook_integrator.py:481
The warning is value-safe but would be more self-contained if it named what IS supported.
Suggested: Append: (supported: type "command" or type "agent").
DevX UX Expert
- [nit] Unsupported-action warning names the problem but not the fix at
src/apm_cli/integration/kiro_hook_integrator.py:481
Appending 'supported action types are command and agent' would give the author a one-read next action. - [nit] Event-name canonicalization (camelCase->PascalCase) changes Kiro filenames; re-install leaves orphaned old-format files at
CHANGELOG.md:24
Existing Kiro consumers who re-install get new v1 files alongside orphaned old files. A one-line cleanup note would prevent confusion. - [nit] Native example uses regex matcher while portable example uses glob, unremarked at
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md:101
A brief parenthetical noting native Kiro v1 matchers are regex and portable matchers are glob would prevent author surprise.
Supply Chain Security Expert
- [nit] Native v1 trigger passthrough could use a brief inline comment noting the safety contract at
src/apm_cli/integration/kiro_hook_integrator.py:182
_KIRO_EVENT_MAP.get(trigger,trigger) passes unrecognized triggers through unchanged; safe because _safe_hook_slug sanitizes for filename, ensure_path_within bounds output, and the trigger is data-only. A one-line comment documenting this chain aids future reviewers.
OSS Growth Hacker
- [recommended] CHANGELOG entry frames the fix as a feature list instead of the adoption-unblocking headline at
CHANGELOG.md:24
Current entry leads with capabilities. The user-facing story is a broken-to-working interop transition. Lead with the interop fix, then list capabilities as supporting detail.
Suggested: Reword to lead with: 'Kiro hooks now emit the official v1 runtime shape that Kiro 1.0 reads natively, replacing the superseded schema.' then list matchers/timeouts/toggles/native-v1 input as supporting detail. (closes [BUG] Kiro hook integrator emits the legacy pre-1.0 hook schema #2071) (fix(hooks): emit Kiro v1 hook documents #2095) - [nit] The hooks-and-commands Kiro v1 PostFileSave+ruff example is excellent story material -- consider a callout box at
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md:93
Wrapping it in a :::tip callout would improve scan-to-value time.
Doc Writer
- [recommended] 'Native-only triggers such as PreTaskExec, PostTaskExec ...' mislabels two triggers that are also reachable from portable input at
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md:89
_HOOK_EVENT_MAP['kiro'] maps portable PreTaskExecution->PreTaskExec and PostTaskExecution->PostTaskExec, so those are NOT native-only; only PostFileCreate/PostFileSave/PostFileDelete/SessionStart have no portable alias.
Suggested: Reword: 'Kiro-native triggers such as PreTaskExec and PostTaskExec pass through; PostFileCreate, PostFileSave, PostFileDelete, and SessionStart are Kiro-only and require the native v1 shape.' - [nit] Pass-through trigger enumeration omits SessionStart at
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md:90
The kiro event map includes SessionStart alongside PostFile* triggers; adding it completes the Kiro-only set.
Suggested: Add SessionStart to the enumerated Kiro-only triggers. - [nit] Native regex matcher vs portable glob matcher juxtaposed without a note at
docs/src/content/docs/producer/author-primitives/hooks-and-commands.md:101
Both accurate about APM behavior, but a one-line note on how Kiro interprets the matcher would help.
Test Coverage Expert
- [nit] Native Kiro v1 agent action passthrough (type:"agent" input) has no direct unit test on _kiro_action_from_action at
tests/integration/test_integration_runtime_coverage.py:127
The PR adds type:"agent" to the accepted set alongside askAgent. The unit test only sends type:"askAgent" as input. Both enter the same branch so severity is nit, but a parametrize entry with type:"agent" input would lock the passthrough contract.
Suggested: Add: _kiro_action_from_action({"type":"agent","prompt":"Do X"}, command_keys=("command",)) == {"type":"agent","prompt":"Do X"}
Proof (passed):tests/integration/test_integration_runtime_coverage.py::TestKiroActionFromAction::test_ask_agent_type-- proves: askAgent converts to Kiro v1 agent action, but not native type:agent passthrough [multi-harness-support]
Auth Expert -- inactive
PR #2095 touches only Kiro hook integrator, tests, docs, spec, CHANGELOG, and conformance artifacts -- no auth-surface files modified.
Performance Expert -- inactive
PR touches only Kiro hook integrator, docs, specs, conformance, and tests -- no cache, transport, resolve, pipeline, or materialization hot-path files.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Address in-scope apm-review-panel findings on PR #2095: - kiro_hook_integrator: drop the unused _ResolvedKiroHook.source_action field and freeze the dataclass (value-object safety); document the native-trigger passthrough safety contract; log the hook filename instead of the parent directory; name the two supported action types in the unsupported-actions warning. - docs(hooks-and-commands): correct the Kiro-only trigger set (PreTaskExec/PostTaskExec pass through from portable input; only PostFileCreate/PostFileSave/PostFileDelete/SessionStart are Kiro-only) and note that Kiro matchers are regex, unlike portable glob matchers. - CHANGELOG: reframe the Fixed entry to lead with the v1 interop fix. - tests: add native type:"agent" passthrough coverage for _kiro_action_from_action. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f98f5a49-17a9-4ac9-a4ff-dd2995780839
APM Spec Guardian:
|
| Panel | Verdict | Shocked | New B | New R | New N |
|---|---|---|---|---|---|
| Swagger / OpenAPI Editor | ship_with_followups | 7/10 | 0 | 1 | 2 |
| OCI Distribution Editor | ship_with_followups | 7/10 | 0 | 2 | 1 |
| W3C TAG Architect | ship_with_followups | 7/10 | 0 | 3 | 1 |
| Package-Manager Registry-Contract Editor | ship_with_followups | 7/10 | 0 | 3 | 2 |
B = new blocking findings, R = new recommended, N = new nits.
Counts are signal strength, not gates. The maintainer ships.
Convergent themes (flagged by 2+ panels)
- T1 -- Canonical JSON serialization absent; deployed_file_hashes (req-lk-012) will diverge across implementations producing semantically equivalent but byte-different hook output (supporting: oci-rec-r1-1, pkg-rec-r1-1, pkg-rec-r1-3)
- T2 -- Version-boundary defense missing; behavior on unrecognized native Kiro version (e.g. v2) is undefined with no diagnostic or reject obligation (supporting: tag-rec-r1-2, pkg-rec-r1-2)
Fold now (1 item)
- [F1 / standalone] req-tg-006 -- [FOLDED in this revision] Normalize mid-paragraph subject shift: replace
A Kiro consumerwithThe consumerso req-tg-006 uses a consistent subject binding throughout, matching the spec's house style for requirement paragraphs.
Success criterion:grep -c 'A Kiro consumer' in req-tg-006 paragraph returns 0; all MUST/MUST-NOT sentences in req-tg-006 use 'The consumer' as subject.
Defer to v0.1.1 (12 items)
- [F2 / T1] req-tg-006 -- Append a canonical-serialization sentence to req-tg-006: the consumer MUST serialize the Kiro v1 hook document as UTF-8 JSON with declaration key order matching the schema, two-space indentation, no trailing comma, and a single trailing newline (U+000A). Parallels req-tg-005's YAML byte-determinism clause.
- [F3 / T2] req-tg-006 -- Add a version-boundary defensive sentence: the consumer MUST NOT emit a version field value other than
v1in this specification version; on encountering a native input document whose version !=v1, the consumer MUST surface a diagnostic naming the unrecognized version and MUST NOT deploy the hook output. Follows the workspaces/nest-mode defensive-MUST pattern. - [F4 / standalone] Appendix A -- Pin the Kiro hook action type discriminator in the Appendix A JSON Schema section with a oneOf constraining the two shapes: {type:command, command:string} and {type:agent, prompt:string}. Reference the sub-schema from req-tg-006 prose so conformance testing can reject malformed action payloads.
- [F5 / standalone] req-tg-006 -- Append a parenthetical to the clause applying path-rewriting and deployment constraints naming req-tg-002 deploy-root confinement, req-sc-002 path-traversal rejection, and req-sc-009 executable approval gate. No new normative statement added; the parenthetical makes the existing fail-closed inference chain explicit and grep-verifiable.
- [F6 / standalone] req-tg-006 -- Append a non-normative editorial note paralleling the one on req-tg-005 acknowledging the concrete Kiro v1 vocabulary is pinned this revision and MAY relocate to the Target Registry companion once cross-target hook-schema registration exists.
- [F7 / standalone] Section 8.5 -- Add a one-sentence introductory note at the top of Section 8.5 naming the per-target normative-block pattern and its eventual relocation to the Target Registry companion. Non-normative framing only.
- [F8 / T1] req-tg-006 -- Clarify the preservation fidelity model: state whether hook field values are preserved byte-for-byte from the portable input (no re-serialization except path rewriting) OR re-serialized through the canonical JSON form defined by F2. The choice determines whether deployed_file_hashes are implementation-invariant.
- [F9 / standalone] req-tg-006 -- Audit the two MUST-NOT prohibitions in req-tg-006 that restate the same obligation; if confirmed redundant, consolidate into one MUST-NOT, decrement normative MUST count by 1, and reconcile all three count sites.
- [F10 / standalone] req-tg-006 -- Rephrase
MUST use the Kiro v1 hook schemaso the normative claim is anchored locally (name the structural elements inline) rather than solely back-referencing an external definition. - [F11 / standalone] req-tg-006 -- Future editorial pass: split req-tg-006's bundled MUST obligations into individually citable req-ids. The spec style elsewhere prefers one principal obligation per id. Not actionable in this amendment; track for next patch.
- [F12 / standalone] req-tg-006 -- Add an editorial cross-reference noting Kiro is auto-detected via the Target Registry companion (Section 4.2.1 auto-detectable taxonomy) rather than selected via a manifest
target: kirodeclaration. - [F13 / standalone] Appendix D -- Add a brief editorial note to Appendix D documenting the concurrent-amendment reconciliation protocol so future editors can replicate the 0.1.13 pattern without re-deriving it.
Linter handoff: After F1 lands: re-run ASCII check (check 1) on the edited paragraph; confirm normative count remains 99 (94 MUST, 5 SHOULD) across Section 1.3, Appendix C trailer, and Appendix D row 0.1.13 -- F1 changes no normative text so count MUST NOT shift. Check 7 (relative-anchor resolution) pre-existing false-positives on numbered-heading slugs are known; req-tg-006's own anchor resolves correctly; no new breakage expected. Check 11 notes .py files modified -- these belong to apm-review-panel running in parallel on the same combined code+spec PR; not a spec-guardian concern. [Post-fold verification: ASCII OK; count sites all 99/94/5; conformance gate 42 passed.]
Full per-panel findings
Swagger / OpenAPI Editor -- shocked_meter 7/10, confidence high
Summary: Solid editorial work adding req-tg-006 for Kiro v1 hook output. The new requirement is well-structured with clear required/optional field separation and proper RFC 2119 keyword use. The single recommended gap (action discriminator shapes not pinned in the JSON Schema appendix) is a completeness improvement, not a conformance break. None blocking. Ship with the discriminator follow-up tracked.
New recommended findings (1)
- [sw-rec-r1-1] 8.5 -- The action object is MUST-present per req-tg-006, but its internal discriminator shapes (command vs agent) are not normatively enumerated in the schema appendix. A Producer could emit {type:command} with no command string and still satisfy the MUST. Without a normative oneOf, conformance testing cannot reject malformed action payloads.
Recommended fix: Pin the action type discriminator in Appendix A with a oneOf for command(+command string) and agent(+prompt string); reference the sub-schema from req-tg-006 prose.
New nit findings (2)
- [sw-nit-r1-1] Two MUST-NOT prohibitions in req-tg-006 restate the same obligation; one can be dropped without normative loss.
- [sw-nit-r1-2] The phrase
MUST use the Kiro v1 hook schemaback-references an external definition rather than making the paragraph self-contained; rephrase so the normative claim is anchored locally.
Preserved strengths confirmed
- Count consistency 99/94/5 verified across Sec 1.3, Appendix C trailer, Appendix D.
- Anchor uniqueness: req-tg-006 sole new anchor, no collision.
- Monotonic numbering after req-tg-005, no renumbering.
- Consumer conformance-class assignment correct.
- Cross-references resolve.
OCI Distribution Editor -- shocked_meter 7/10, confidence high
Summary: Well-scoped addition that correctly forbids the superseded schema and mandates v1 output. Two recommended follow-ups: specify canonical JSON serialization for reproducible content hashes, and make the fail-closed constraint chain explicit via cross-references.
New recommended findings (2)
- [oci-rec-r1-1] req-tg-006 -- req-tg-006 introduces a JSON document but does not specify canonical serialization; two conformant implementations may produce semantically equivalent but byte-different JSON, causing req-lk-012 SHA-256 divergence. req-tg-005 solved the equivalent YAML problem.
Recommended fix: Add a canonical-serialization sentence (UTF-8, declaration key order, two-space indent, single trailing newline) OR defer to the Target Registry companion with an explicit editorial note. - [oci-rec-r1-2] req-tg-006 -- The path-rewriting/deployment-constraints clause is security load-bearing but does not name the constraint anchors (req-tg-002, req-sc-002, req-sc-009); the fail-closed chain relies on inference.
Recommended fix: Append a parenthetical naming req-tg-002, req-sc-002, req-sc-009 to make the fail-closed chain explicit without adding a new normative statement.
New nit findings (1)
- [oci-nit-r1-1] req-tg-006 bundles ~7 distinct MUST obligations under one id; the spec style elsewhere prefers one principal obligation per id for citability. Note for a future split.
Preserved strengths confirmed
- Fail-closed extraction controls and hash-anchored trust intact.
- Executable approval gate still covers hooks generically.
- Mirror-tolerance unaffected.
- Count sites consistent at 99 (94 MUST, 5 SHOULD).
W3C TAG Architect -- shocked_meter 7/10, confidence high
Summary: Well-scoped, internally consistent target-specific requirement following the req-tg-005 precedent. Main gaps: missing forward-compat editorial note and an unbounded MUST-accept-native version boundary; both addressable with minimal text; nothing breaks a conformant implementation.
New recommended findings (3)
- [tag-rec-r1-1] req-tg-006 -- req-tg-006 hardcodes the literal v1 and a concrete Kiro field vocabulary but, unlike req-tg-005, carries no forward-compatibility editorial note, leaving evolution intent unstated.
Recommended fix: Append an editorial note paralleling req-tg-005 acknowledging the concrete v1 vocabulary is pinned this revision and MAY relocate to the Target Registry companion. - [tag-rec-r1-2] req-tg-006 -- The MUST-accept-native-input clause does not bound the obligation to the pinned version; behavior on a native Kiro v2 document (reject/warn/best-effort) is undefined.
Recommended fix: Add a sentence: a native document whose version != v1 is out of scope; the consumer MUST emit a diagnostic naming the unrecognized version and MAY reject deployment. - [tag-rec-r1-3] Section 8.5 -- With req-tg-005 and req-tg-006 the spec now has two target-specific normative blocks framed as exceptions; this emerging pattern would benefit from an explicit layering paragraph so future targets have a template.
Recommended fix: Add a one-sentence note at the top of Section 8.5 naming the per-target normative-block pattern and its eventual relocation to the Target Registry companion.
New nit findings (1)
- [tag-nit-r1-1] Mid-paragraph subject shifts from
The consumertoA Kiro consumer; use consistent subject binding.
Preserved strengths confirmed
- Appendix C index carries the new requirement correctly.
- Count reconciliation consistent (99 = 94 MUST + 5 SHOULD).
- x- vendor-extension namespace intact.
- Amendment process unaffected.
Package-Manager Registry-Contract Editor -- shocked_meter 7/10, confidence high
Summary: Structurally sound and the count reconciliation is correct, but it lacks a canonical-serialization clause for emitted JSON (the primary determinism gap for deployed_file_hashes), a defensive MUST-NOT for unrecognized version values, and clarity on native-input preservation fidelity. All recommended-level; none breaks a conformant implementation today given a single reference implementation.
New recommended findings (3)
- [pkg-rec-r1-1] req-tg-006 -- req-tg-006 mandates structural fields but does not pin canonical JSON serialization order/whitespace; since these files participate in deployed_file_hashes (req-lk-012), key-order/indent/trailing-newline differences across implementations produce divergent lockfile hashes.
Recommended fix: Append a canonical-serialization MUST OR defer with a defensive editorial note pointing at the reference implementation formatting. - [pkg-rec-r1-2] req-tg-006 -- req-tg-006 hard-pins version v1 with no reserved-slot/forward-compat defensive pair; an early consumer that silently accepts a future v2 could produce undefined deployment behavior.
Recommended fix: Add: a consumer MUST NOT emit a version other than v1 in this spec version, and MUST surface a diagnostic and MUST NOT deploy on an unrecognized native version. - [pkg-rec-r1-3] req-tg-006 --
preserve their supported names/descriptions/triggers/...does not pin the fidelity model (byte-for-byte round-trip vs semantic re-serialization); the choice directly affects deployed_file_hashes determinism.
Recommended fix: Clarify either byte-for-byte deploy (no re-serialization except path rewriting) OR re-serialize through the canonical form so deployed hashes are implementation-invariant.
New nit findings (2)
- [pkg-nit-r1-1] req-tg-006 references Kiro as a target but Section 4.2.1 canonical target set does not include kiro; add an editorial cross-reference noting Kiro is auto-detected via the Target Registry companion. -- fix: add the auto-detection cross-reference or add kiro to the canonical set if explicit selection is intended.
- [pkg-nit-r1-2] Appendix D row 0.1.13 concurrent-amendment reconciliation pattern has recurred; an editorial note on the reconciliation protocol would help future implementers.
Preserved strengths confirmed
- Lockfile hash determinism framework intact and governs Kiro hook files via deployed_file_hashes.
- Conformance-class separation maintained (consumer-only).
- Count reconciliation consistent at 99 (94 MUST, 5 SHOULD).
- Reserved-slot discipline undisturbed.
This panel is advisory. It does not block merge. Re-apply the spec-review label after addressing feedback to re-run.
Fold the sole fold_now item from the apm-spec-guardian editorial-patch panel: replace the mid-paragraph subject 'A Kiro consumer' with 'The consumer' in req-tg-006 so the requirement uses a consistent subject throughout. Prose-only; no normative surface or count change (still 99 / 94 MUST / 5 SHOULD across all three count sites). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f98f5a49-17a9-4ac9-a4ff-dd2995780839
PreTaskExec passes through from portable input via _HOOK_EVENT_MAP, so it is not Kiro-only. Replace the example with genuinely Kiro-only triggers (PostFileSave / SessionStart) to match the corrected sibling doc (hooks-and-commands.md) and keep the apm-guide usage resource in sync with the docs corpus. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f98f5a49-17a9-4ac9-a4ff-dd2995780839
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 1 | Architecture is sound: clean module boundary, correct BaseIntegrator delegation, frozen value-object, pure converters, no side channels. Pass-1 folds verified correct. |
| Cli Logging Expert | 0 | 0 | 0 | Output UX is clean; value-safe diagnostics verified (no command/prompt/env/secret interpolation); pass-1 folds applied; ASCII-only confirmed. |
| Devx Ux Expert | 0 | 0 | 0 | Clean DevX surface. Emitted Kiro v1 shape matches docs and integration-test assertions. Warning message is actionable. No new UX concerns; ship it. |
| Supply Chain Security Expert | 0 | 0 | 0 | Path-traversal confinement, script-copy hardening, fail-closed extraction, value-safe diagnostics, no exec at install time, 0o600 perms all verified. |
| Oss Growth Hacker | 0 | 0 | 0 | Positioning surface is clean. CHANGELOG leads with the interop win; docs give a cold Kiro user a runnable example within 30 seconds. |
| Test Coverage Expert | 0 | 0 | 3 | Kiro v1 hook surface is defended at e2e and integration-with-fixtures tiers; native agent passthrough gap folded; ship. |
| Doc Writer | 0 | 0 | 0 | Docs accurate to Kiro v1 code; the one apm-guide sync gap (PreTaskExec mislabel) is resolved on this head. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 2 follow-ups
- [Python Architect] Add concrete type annotations to the two params in
_write_resolved_hooks(diagnostics,display_payloads) atkiro_hook_integrator.py:254. -- Inherited pattern; cosmetic only, improves grep-ability and IDE support. - [Test Coverage Expert] Add a direct unit test for the
_dedup_scriptshelper. -- Currently exercised only transitively via integration paths; a focused parametrize test would lock its dedup semantics.
Architecture
classDiagram
direction LR
class BaseIntegrator {
<<Base>>
+check_collision(target, rel, managed, force)
+find_files_by_glob(path, pattern)
}
class HookIntegrator {
<<Strategy>>
+HOOK_COMMAND_KEYS tuple
+integrate_hooks_for_target(pkg, root, target)
+_rewrite_command_for_target(cmd, pkg, name, key)
+_rewrite_hooks_data(data, pkg, name, key)
+_parse_hook_json(hook_file, allow_kiro_v1)
+_get_package_name(pkg_info, root)
+_summarize_command(entry)
}
class _ResolvedKiroHook {
<<ValueObject>>
+trigger str
+action dict
+name str or None
+matcher str or None
+description str or None
+timeout int or float or None
+enabled bool or None
}
class HookIntegrationResult {
<<ValueObject>>
+files_integrated int
+files_skipped int
+target_paths list
+scripts_copied int
+files_adopted int
+display_payloads list
}
class IntegrationResult {
<<Base>>
+files_integrated int
+files_updated int
+files_skipped int
+target_paths list
}
BaseIntegrator <|-- HookIntegrator
IntegrationResult <|-- HookIntegrationResult
HookIntegrator ..> _ResolvedKiroHook : builds via helpers
HookIntegrator ..> HookIntegrationResult : returns
note for _ResolvedKiroHook "frozen=True; built once, read-only"
flowchart TD
A["HookIntegrator.integrate_hooks_for_target"] -->|target.name == kiro| B["integrate_kiro_hooks"]
B --> C{".kiro/ exists?"}
C -->|No| D["Return empty result"]
C -->|Yes| E["find + filter hook files"]
E --> G["_parse_hook_json(allow_kiro_v1=True)"]
G --> H{"data.hooks is list? (native v1)"}
H -->|Yes| I["_resolve_native_v1_hooks"]
H -->|No| K["_resolve_portable_hooks"]
I --> L["_kiro_action_from_action per hook"]
K --> L
L --> O["_write_resolved_hooks"]
O --> P{"byte-identical existing?"}
P -->|Yes| Q["files_adopted, chmod 0o600"]
P -->|No| R["check_collision"]
R -->|Clear| T["atomic_write_text + chmod 0o600"]
O --> V["copy scripts (ensure_path_within)"]
Recommendation
Ship now. Zero blocking findings, zero open recommended findings, two affirming PASSED test-evidence rows (e2e + integration-with-fixtures), and green CI-mirror lint on the final head 8363824af. The two remaining nits (type annotations on _write_resolved_hooks, a direct _dedup_scripts unit test) are low-signal optional improvements that do not gate merge -- track them as post-merge follow-ups.
Full per-persona findings
Python Architect
- [nit] Two parameters in
_write_resolved_hookslack concrete type annotations atkiro_hook_integrator.py:254
diagnosticsanddisplay_payloads: listcould be typed asDiagnosticCollector | Noneandlist[dict]to match the precision of the other parameters. Low-signal because the pattern is inherited from existing HookIntegrator call sites, but worth tightening for grep-ability and IDE support.
Suggested:diagnostics: DiagnosticCollector | None,display_payloads: list[dict].
Cli Logging Expert
No findings. Value-safe diagnostics verified: _log.debug interpolates only hook counts + filename; _log.warning interpolates only the filename; _emit_hook_event_diagnostics sanitizes event names. Zero diagnostic strings interpolate command content, agent prompts, env vars, or secrets. ASCII-only confirmed at byte level.
Devx Ux Expert
No findings. _kiro_hook_document produces {"version":"v1","hooks":[...]} with exactly the Kiro-native fields; the e2e test asserts this contract. Warning names the file and the supported action types. Docs example uses correct JSON-escaped regex (\\.py$) and accurately classifies Kiro-only triggers.
Supply Chain Security Expert
No findings. Verified: path-traversal confinement (_safe_hook_slug strips separators; ensure_path_within backstop); script-copy hardening (ensure_path_within on every copy in native and portable paths); fail-closed extraction (_parse_hook_json returns None on bad shape; isinstance guards on every field); value-safe diagnostics (basenames + counts only); no subprocess/os.system/exec/eval at install time; 0o600 on generated files; pass-1 safety-contract comment applied.
Oss Growth Hacker
No findings. CHANGELOG leads with the user-facing interop win; docs give a cold Kiro user a runnable Ruff lint-on-save example; the ide-tool-integration -> hooks-and-commands cross-reference chain aids discoverability.
Test Coverage Expert
- [nit]
_dedup_scriptshelper has no direct unit test atsrc/apm_cli/integration/kiro_hook_integrator.py
A new pure helper that removes duplicate (source, target_rel) pairs; exercised transitively by integration tests but has no focused unit test. Below the critical-promise threshold (internal helper, not user-visible), so does not warrant blocking or recommended severity.
Suggested: Add a 3-case parametrize test (empty, no duplicates, with duplicates).
Proof (missing at):tests/integration/test_integration_runtime_coverage.py::TestDedupScripts::test_removes_duplicate_script_pairs-- proves: Script dedup preserves order and removes exact-match duplicates [devx] - [nit] E2E test confirms v1 shape and portable-to-native transform; coverage sufficient at floor tier
test_install_transforms_kiro_v1_hook_and_preserves_unrelated_configexercises the real CLI via subprocess, asserts the emitted JSON matches native Kiro v1, asserts both command and agent action types are produced, and asserts unrelated user-owned hook files are preserved.
Proof (passed):tests/integration/test_kiro_hook_install_e2e.py::test_install_transforms_kiro_v1_hook_and_preserves_unrelated_config-- proves: A Kiro-targeted package install materializes a hook file in native Kiro v1 format that Kiro actually accepts [multi-harness-support,vendor-neutral]
uv run --extra dev python -m pytest tests/integration/test_kiro_hook_install_e2e.py -q => 1 passed in 1.44s - [nit] Native v1 document passthrough (with all optional fields) proven at integration-with-fixtures tier
test_kiro_hooks_accept_native_v1_documentsexercises a native v1 document with name/description/trigger/matcher/timeout/enabled/action and verifies passthrough with path-rewriting only on the command.
Proof (passed):tests/unit/integration/test_kiro_target.py::test_kiro_hooks_accept_native_v1_documents-- proves: Native Kiro v1 hook documents pass through APM install preserving all fields [multi-harness-support,vendor-neutral]
Doc Writer
No open findings on this head. The one sync gap raised against 12018598e -- packages/apm-guide/.apm/skills/apm-usage/package-authoring.md labeling PreTaskExec as Kiro-only, contradicting the corrected sibling doc and _HOOK_EVENT_MAP -- was resolved by the final-fold commit 8363824af (replaced with genuinely Kiro-only triggers PostFileSave / SessionStart). All other doc claims (native v1 shape, PascalCase trigger map, matcher field, action type, hook-level timeout, preserved description/timeout/enabled, askAgent -> agent, zero-supported-actions warning, regex-vs-glob matcher note, cross-links) verified accurate against code.
Auth Expert -- inactive
PR touches only integration hook files (hook_integrator.py, kiro_hook_integrator.py), tests, docs, spec, and conformance artifacts -- no token management, credential resolution, AuthResolver, HostInfo, AuthContext, or remote-host authentication surface.
Performance Expert -- inactive
PR touches only integration hook emitters, tests, docs, spec, and conformance artifacts -- none of these are on any package-manager performance hot path (no resolution, lockfile, download, cache, git transport, or materialization code is affected).
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
fix(hooks): emit Kiro v1 hook documents
TL;DR
Kiro hook installation now emits the current v1
hooksarray schema instead of the legacywhen/thenformat. Portable event names, matchers, command timeouts, prompt actions, and native Kiro v1 inputs are consumed directly into Kiro-native documents without changing other target integrations. This removes the manual migration step Kiro 1.0 previously required.Note
Closes #2071. The broader design of a new portable
agentaction remains outside this bug fix; existingaskAgentinputs now render as Kiro v1agentactions.Problem (WHY)
when/thendocuments as legacy and does not run them as current v1 hooks.patterns, andrunCommandfields do not match Kiro v1's PascalCasetrigger,matcher, andactioncontract.hooksfield.These are the concrete failure modes and acceptance criteria documented in #2071.
Approach (WHAT)
{ "version": "v1", "hooks": [...] }.hooksarrays directly into Kiro-native documents on the Kiro path -- no foreign (Claude-shaped) event-map round-trip and no_kiro_*side-channel keys.Implementation (HOW)
src/apm_cli/integration/hook_integrator.pysrc/apm_cli/integration/kiro_hook_integrator.py_ResolvedKiroHookvalue (each field already final) and serializes one v1 document per hook, preserving script rewriting, collision checks, permissions, and display payloads. Native v1 is read and re-emitted directly; the portable path reuses_rewrite_hooks_datafor in-place script-path rewriting.tests/unit/integration/test_kiro_target.pytests/integration/test_integration_runtime_coverage.pydocs/src/content/docs/integrations/ide-tool-integration.mddocs/src/content/docs/producer/author-primitives/hooks-and-commands.mdpackages/apm-guide/.apm/skills/apm-usage/package-authoring.mddocs/src/content/docs/specs/openapm-v0.1.mdreq-tg-006.docs/public/specs/manifests/openapm-v0.1.requirements.yml/tests/spec_conformance/test_manifest_reqs.pyDiagrams
Legend: Portable and native hook inputs each resolve into Kiro-native hooks, which one writer serializes to a Kiro v1 file. Native input is consumed directly -- there is no vendor-shaped intermediate.
flowchart LR subgraph Input[Package hook input] P[Copilot or Claude event map] N[Kiro v1 hooks array] end subgraph Transform[Kiro target transform] Parse[_parse_hook_json allow_kiro_v1] RP[_resolve_portable_hooks] RN[_resolve_native_v1_hooks] W[_write_resolved_hooks] end subgraph Output[Kiro runtime] File[".kiro/hooks/*.json v1"] end P --> Parse N --> Parse Parse --> RP Parse --> RN RP --> W RN --> W W --> File classDef new stroke-dasharray: 5 5; class RN,W new;Trade-offs
askAgentauthoring path and rejected a new cross-target primitive design in this bug-fix scope. Unbounded Kiro agent actions are preserved as agent actions, never lowered into bounded portable loops.Benefits
version: "v1"and exactly one entry inhooks.PreToolUse,PostToolUse,UserPromptSubmit,Stop, task, file, and session events reach Kiro with canonical trigger names.name,description,matcher,timeout, andenabledexactly.Validation
Focused Kiro suites (
tests/unit/integration/test_kiro_target.py,tests/integration/test_kiro_hook_install_e2e.py,tests/integration/test_integration_runtime_coverage.py):195 passed.Full relevant suite (
uv run --extra dev pytest tests/unit tests/integration):11940 passed, 239 skipped, 2 xfailed.CI lint mirror
Hermetic install e2e (
uv run --extra dev pytest -q tests/integration/test_kiro_hook_install_e2e.py):1 passed. Mutation-break proof: temporarily restoring the legacy{when, then:{runCommand}}schema in_kiro_hook_documentmade that exact subprocess install test fail at the generated document's v1-contract assertion ({'then': {'runCommand': ...}, 'when': 'PreToolUse'} == {'version': 'v1', 'hooks': [...]}); restoring the production guard returned it to green. The mutation was not committed.uv run --extra dev pytest tests/spec_conformance -q:132 passed, 2 skipped.Scenario Evidence
tests/integration/test_kiro_hook_install_e2e.py::test_install_transforms_kiro_v1_hook_and_preserves_unrelated_config(mutation-proven regression trap for #2071)tests/unit/integration/test_kiro_target.py::test_kiro_hooks_accept_native_v1_documentstests/unit/integration/test_kiro_target.py::test_kiro_v1_event_map_uses_canonical_trigger_namestests/unit/integration/test_kiro_target.py::test_kiro_hooks_convert_prompt_actions_to_ask_agentHow to test
PreToolUsecommand hook into a project containing.kiro/; confirm the generated file hasversion: "v1"and ahooksarray.trigger, optionalmatcher, andaction, with nowhen,then, orrunCommandfields.PreTaskExec; confirm the trigger and name are preserved and referenced scripts are copied under.kiro/hooks/<package>/.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com