awslabs / awslabs/aidlc-workflows

[Bug]: On Kiro the argument-reading guards ship unregistered — kiro-ide has no manifests, and kiro registers them in the CLI 2.x embedded form

Open
#764 1 comment 0 reactions 0 assignees View on GitHub
v2-launch
Dominant language
TypeScript
Stars
4.7k
Forks
853
Avg merge
1d 21h
Merged PRs (30d)
165

Description

Follow-up to #763, which measured that Kiro IDE 1.x and Kiro CLI v3 **do** deliver tool arguments to PreToolUse hooks. That premise is what the current wiring rests on, so this is the wiring half, filed separately because it is a design decision rather than a factual correction.

### What ships today

Verified on `4569754e` (v2.6.2):

- `harness/kiro-ide/hooks/` ships **eight** standalone `.json` manifests: `continue-workflow`, `enforce-approval-gate`, `log-subagent`, `rebuild-stage-graph`, `record-human-turn`, `session-start`, `sync-workflow-state`, `write-audit-log`. There is **no** manifest for `review-freeze`, `plan-approval-guard`, or `reviewer-scope`.
- Their bodies do ship: `dist/kiro-ide/.kiro/hooks/aidlc-review-freeze.ts` and `aidlc-plan-approval-guard.ts` are present and unreferenced.
- `harness/kiro/hooks/` (CLI) ships **zero** `.json` manifests — seven legacy `.kiro.hook` files and the adapter. The three guards are registered instead inside the agent configs' embedded `hooks` (`harness/kiro/agents/*.json`).

### The stated reason

`core/hooks/aidlc-reviewer-scope.ts:30-31`:

> Kiro IDE ships no registration: its hook payloads carry no tool inputs, so a pre-tool matcher has nothing to inspect there.

That is a deliberate, documented decision, not an oversight — and #763 shows the premise no longer holds on the surfaces I can test.

### What I measured in a fork

I ported the two matcher-based guards to a `kiro-ide` shell as standalone manifests plus two adapter verbs, and ran it on Kiro IDE 1.0.309 and Kiro CLI v3 in parallel workspaces. All **ten** manifests in that shell fired on both surfaces (checked through `.aidlc-hooks-health/*.last`), the two ported ones included.

One defect surfaced in my own port, and it is worth knowing before anyone repeats it: the dispatched agent's identity is in the **tool name** (`subagent_aidlc-developer-agent`), not in the arguments — there is no `agent_type` or `name` key. My adapter derived `subagent_type` from those two keys, so it passed `""` and `aidlc-plan-approval-guard.ts` returned early at its `subagent_type !== "aidlc-developer-agent"` check. The hook fired and enforced nothing. Deriving from the tool name fixes it; I verified the flip through the body's own pure function (`evaluatePlanApprovalDispatch` returns `block: false` for `""` and `block: true` for the derived name, and still allows an approved unit).

### What I did not measure

- Whether the **embedded** `hooks` in `harness/kiro/agents/*.json` still register on Kiro CLI v3. The Kiro docs say the mechanism moved (`features/hooks.md`: "From CLI 2.x - Hooks moved from embedded fields in agent config to standalone files. Run `kiro-cli agent migrate` to auto-convert"). If embedded registration is legacy-only, the same three guards may be unregistered on CLI v3 as well, which would make this broader than `kiro-ide`. I have not tested a stock CLI shell to confirm either way.
- Whether Kiro offers an input-rewrite channel (the separate question raised at the end of #763).

### The decision I am asking for

1. Should `review-freeze` and `plan-approval-guard` get `kiro-ide` manifests? Both are matcher-based and carry no persona argument, so this is a straight port.
2. Does the `kiro` harness need standalone manifests for CLI v3, or is the embedded form still honoured there?
3. `reviewer-scope` is not a straight port. Its CLI registration is per-persona (embedded in the reviewer agents' own configs, which is what makes every call arriving through it the reviewer's). The unified harness has no per-persona hook registration, so a global manifest would need to identify the reviewer from the tool name instead of from `agent_type`.

I am not asking for a specific patch — my fork already carries (1) and lives without (3). What would help most is knowing whether upstream wants this in, so that a fork does not have to keep diverging here.

Contributor guide

Open the contributing guide

Research direction

Start with core/hooks/aidlc-reviewer-scope.ts, the manifests under harness/kiro-ide/hooks/, the embedded hooks in harness/kiro/agents/*.json, and features/hooks.md. Confirm registration behavior on stock Kiro IDE and CLI v3, then document the upstream decision for the two matcher-based guards, CLI manifests, and reviewer-scope handling.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
cli, tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.