fix(kiro-ide): make hooks work in Kiro IDE (USER_PROMPT env, relative paths, audit-tail gating)#471
Conversation
|
Reviewed this in depth. The adapter half is a solid fix for a real problem: the USER_PROMPT discovery is well evidenced, and the env-var parse plus relative-path resolution genuinely revives the audit logger and sensors in the IDE. The concerns below are all in the new payload-free audit-tail gating added to the two shared core hooks, which reconciles from the last 1.
|
|
Consolidating where this PR stands, since v2 has moved under it. The two existing reviews stand in full: @apackeer's four findings (2026-07-01) and @leandrodamascena's CHANGES_REQUESTED + three inline comments are all unaddressed - findings 1-3 remain merge blockers (they corrupt live workflow state). New items accrued since those reviews:
With the earlier findings 1-3 fixed, finding 4 + the inline items folded per point 4, and points 1-3/5-6 above, this is a merge - the USER_PROMPT root-cause work is exactly what we want in. Happy to help with the rebase; the mint/block re-integration is the only non-mechanical piece. If you're short on time, say the word and we'll carry it over the line keeping your commits as the base. |
|
I've pushed the hardening in 4e245f7 (findings) + 55beefe (filesystem debug toggle). All four findings are addressed with scoped changes — no structural rework in this PR. I've also run the fixes through 2 Kiro IDE workflow runs; every finding is now confirmed holding in the debug trace, not just unit-verified. Finding 1 — state resurrection (aidlc-sync-statusline.ts, IDE audit-sync branch). Made forward-only. It now exits without writing when: Status != Running, Current Stage is none/empty, the audit slug already matches state, or the audit's latest stage is already completed/skipped. So a finalize/approve is never undone by the next shell command. Live-confirmed: across ~30+ hook cycles and multiple transitions (intent-capture → requirements-analysis → user-stories), Current Stage only ever moved forward. Trace shows repeated ide-audit-sync auditSlug="user-stories" current="user-stories" exiting at the current === auditSlug guard with no set-status follow-up. Finding 2 — single-stage rows (latestStartedStageSlug, aidlc-lib.ts). Now filters Workflow: single-stage: blocks so a synthetic --single stage-runner row can't be mistaken for the live stage. Finding 3 — recompile-forever (aidlc-runtime-compile.ts, IDE path). Added an mtime idempotency guard: if runtime-graph.json is at least as new as the newest audit shard, the tail hasn't changed since the last compile → skip. A real new transition bumps a shard's mtime past the graph and re-enables the compile. This bounds the case where a lingering transition (e.g. after WORKFLOW_COMPLETED) would otherwise recompile on every subsequent shell command. Live-confirmed: debug log shows runtime-compile skip: graph newer than audit (idempotent) graphMtime=… newestShard=… firing (guard hit), while genuine transitions (STAGE_AWAITING_APPROVAL at gate-start) still recompiled correctly. Finding 4 — path extraction (extractWrittenPath, aidlc-kiro-adapter.ts). Now trims trailing whitespace and strips the str_replace (N occurrences) suffix. When a write-class tool yields no extractable path, the adapter records a visible recordHookDrop instead of a silent no-op. Live-confirmed: every artifact write (stories.md, personas.md, memory.md, etc.) resolved cleanly to an absolute path and emitted ARTIFACT_UPDATED; zero hook-drops in the run. Smaller items: CHANGELOG reworded; the kiro-ide harness guide's sync-statusline row corrected (shell event, not spec); doctor no longer false-FAILs on the debug-only health dir; and the probe findings are now committed at Debug toggle (55beefe): added touch aidlc/.aidlc-hook-debug as a filesystem alternative to AIDLC_HOOK_DEBUG=1 — takes effect on the next hook fire with no IDE restart (the IDE spawns hook subprocesses, so an env var would need a restart). Off by default, zero overhead on a normal run. This is what produced the traces above. |
d2f2c9f to
1dc217a
Compare
apackeer
left a comment
There was a problem hiding this comment.
LGTM. Everything from the consolidated comment (2026-07-05) is addressed on the rebased head 1dc217ab; I verified each item and ran checks against it locally.
Verified on this head:
- Rebase + re-bump: merge-base is the current v2 tip
242953ec; version file, CHANGELOG heading, and README badge agree at 2.2.11 (t68 green). - mint/block re-integrated on the USER_PROMPT-era adapter: project dir from
process.cwd(), deadkiro.cwdsource dropped, full carve-out chain (autonomous mode, off-switch, open-gate predicate,humanActedSinceGate) and the exit-2 contract intact. The 2s stdin-race removal made the CHANGELOG too. - t188 renamed to t213 with the coverage registry regenerated.
- log-subagent finished properly:
extractAgentIdentity()recovers the delegate from the**Reviewer:**/**Agent:**first line and forwards the result text aslast_assistant_message(which the core hook already reads). Going further and making the marker a persona-level Output Contract in both reviewer agents plus stage-protocol is the right call - it turns a workaround into a contract. S1-S3 pin it. toolSuccess === falseguarded explicitly in audit-and-sensors, with absent-field falling through rather than dropping (the right defensive choice). T1/T2 pin both directions.- Child spawns use
process.execPathrather than barebun, consistent with the change that landed on v2 after your branch was cut - attentive rebase, appreciated. - My local verification:
bun scripts/package.ts --checkclean on this head (all four dist trees in sync); unit slice--filter "t213|t68|gen-coverage"3/3 files green, 67 assertions, 0 failures.
Minors - please pick these up in a follow-up PR, none block this merge:
- Doctor's fresh-vs-drift check counts only
.lastfiles as hook-fired content, so a workspace where the adapter fires but every write drops (akiro-adapter.dropspresent, no heartbeat yet) reads "Hook heartbeats: not yet fired" (PASS) where it should flag. Counting.dropsas fired-content closes it; alternatively fold it into #516, which already tracks doctor surfacing drop contents. - The runtime-compile mtime idempotency guard (finding 3) is the one review fix without a pinning test. A cheap shape: fire
runtime-compiletwice with the debug marker on and assert the second run logs the idempotent skip (or that the graph mtime is unchanged). docs/reference/kiro-ide-hook-payload.mdbreaks that directory'sNN-topic.mdnaming convention and nothing links to it yet - worth a number and an index entry.- In
aidlc-architecture-reviewer-agent.mdthe new Output Contract section was inserted mid-list, so the last two Key Principles bullets now sit under the Output Contract heading. The product-lead edit appended cleanly; same treatment here.
Merge-sequencing heads-up (coordination, not a fault of this PR): 2.2.11 is currently claimed by four open PRs (#471, #508, #510, #535), and the t213 test slot by three (#471, #508, #535 - different filenames so no git conflict, but two t213s would land in the tree unless the later merger renames). Whichever merges second re-bumps its version and renames its test file per the usual convention; next genuinely free test slot is t218+ given #537/#538 claim t216/t217.
Kiro IDE delivers hook context through the USER_PROMPT environment variable (not stdin, which it opens but never writes), and leaves toolArgs empty. The IDE adapter now reads USER_PROMPT, recovers the written file path from the tool-result prose, and drives the command/payload-free hooks off the audit trail. - audit-logger, sensor-fire, runtime-compile, sync-statusline now do real work on Kiro IDE; write paths resolved relative to workspace root. - runtime-compile and sync-statusline gate on the audit tail (payload-free). - sync-statusline ide-audit-sync is forward-only (never rewinds Current Stage): skips when not Running, pointer none, or audit slug already completed/skipped. - runtime-compile IDE path adds an mtime idempotency guard so a lingering transition does not recompile on every subsequent shell command. - extractWrittenPath tolerates trailing newlines, strips a str_replace " (N occurrences)" suffix, and records a visible hook-drop when a write-class tool yields no extractable path. - latestStartedStageSlug ignores synthetic --single stage-runner rows. - opt-in hook debug log: AIDLC_HOOK_DEBUG=1 or touch aidlc/.aidlc-hook-debug. Kiro CLI, Claude Code, and Codex behaviour unchanged (new branches gated behind the IDE-only ide-audit-sync marker and the opt-in debug env var).
Review fixes for leandrodamascena: - docs/guide/13-customization.md: quote the statusline command and the bun-tools permission glob in both the JSON example and the prose, and note why the * stays outside the quotes - docs/reference/14-claude-features.md: quote the statusline, session-start, and session-end command examples - rename the regression test to t219-claude-project-dir-quoting.test.ts (t199 already shipped on v2 from #492; t218 landed with #471's kiro-ide hook adapter) and update the t40 comment
… (2.2.14) (#522) * fix: quote $CLAUDE_PROJECT_DIR in Claude hook and statusline commands (#519) (2.2.19) * review: quote CLAUDE_PROJECT_DIR in docs examples, rename test to t219 Review fixes for leandrodamascena: - docs/guide/13-customization.md: quote the statusline command and the bun-tools permission glob in both the JSON example and the prose, and note why the * stays outside the quotes - docs/reference/14-claude-features.md: quote the statusline, session-start, and session-end command examples - rename the regression test to t219-claude-project-dir-quoting.test.ts (t199 already shipped on v2 from #492; t218 landed with #471's kiro-ide hook adapter) and update the t40 comment
Kiro IDE's hooks were effectively dead for everything that depended on what the agent just did — the audit logger, sensor firing, runtime-graph recompile, and statusline sync all silently no-op'd. Root cause, verified live inside a Kiro IDE session: the IDE delivers hook context through the
USER_PROMPTenvironment variable (not stdin, which it opens but never writes), and it leaves the tool arguments empty. The old adapter waited on stdin, timed out, and forwarded an empty payload. This PR reworks the IDE adapter to readUSER_PROMPT, recover the written file path from the tool-result text (resolved to absolute), and drive the command/payload-free hooks off the audit trail. The Kiro CLI, Claude Code, and Codex harnesses are untouched — they use a separate adapter and a working stdin channel, and the shared-core additions are inert without the IDE markers/flag. Bumps to 2.1.5.Summary
1. Read hook context from
USER_PROMPTinstead of stdinThe IDE never writes stdin (it hangs), and delivers
{toolName, toolArgs, toolResult, toolSuccess}via theUSER_PROMPTenv var. The IDE adapter now parses that env var and drops the dead stdin read + 2s timeout.Files changed
harness/kiro-ide/hooks/aidlc-kiro-adapter.ts→ readsUSER_PROMPT, removes stdin race2. Recover the written file path from tool-result text (resolved to absolute)
toolArgsis always empty in the IDE, so the file path is scraped from the tool-result prose (Created the X file./Replaced text in X/Appended the text to the X file.). The IDE reports it relative to the workspace root, so the adapter resolves it to absolute — fixingaudit-logger, which was comparing a relative path against an absolute record root and dropping every write.Files changed
harness/kiro-ide/hooks/aidlc-kiro-adapter.ts→extractWrittenPath()+isAbsolute/resolveagainst project dircore/hooks/aidlc-audit-logger.ts→ debug instrumentation at each exit gate (path-gate verdict)3. Payload-free gating for runtime-compile and sync-statusline
The IDE surfaces neither the shell command nor the task payload, so both hooks now gate on the audit tail via an
ide-audit-syncmarker.runtime-compileskips the command filter and compiles on a real transition;sync-statuslinederivesCurrent Stagefrom the latestSTAGE_STARTEDand is rewired from the never-firingspecevent ontoshell.Files changed
core/hooks/aidlc-runtime-compile.ts→ide-audit-syncbypasses command filter, gates on audit tailcore/hooks/aidlc-sync-statusline.ts→ audit-tail branch vialatestStartedStageSlug()core/tools/aidlc-lib.ts→ newlatestStartedStageSlug()shared helperharness/kiro-ide/hooks/aidlc-sync-statusline.kiro.hook→ triggerspec→shell4. Opt-in hook debug log
AIDLC_HOOK_DEBUG=1makes every hook append its decision path to<record>/.aidlc-hooks-health/hook-debug.log. Off by default — no log, no overhead.Files changed
core/tools/aidlc-lib.ts→hookDebug()+hookDebugEnabled()(flag-gated)core/hooks/aidlc-audit-logger.ts,core/hooks/aidlc-runtime-compile.ts,core/hooks/aidlc-sync-statusline.ts→ debug trace callsharness/kiro-ide/hooks/aidlc-kiro-adapter.ts→ adapter-level debug trace5. Tests, docs, versioning
New headless coverage driven by
USER_PROMPTenv fixtures (relative-path resolution, runtime-compile audit-tail compile, opt-in debug gate); IDE harness doc updated; 2.1.5 CHANGELOG entry.Files changed
tests/unit/t188-kiro-ide-hook-adapter.test.ts→ new IDE adapter test suite (14 cases)tests/unit/gen-coverage-registry.test.ts→ register t188 in the none→cli settests/.coverage-registry.json→ regenerateddocs/guide/harnesses/kiro-ide.md→ env-var hook mechanism + debug-flag sectioncore/tools/aidlc-version.ts→ 2.1.5README.md→ version badge 2.1.5CHANGELOG.md→ 2.1.5 entrydist/**→ regenerated (drift-checked)Testing
t188IDE adapter suite (14 cases) — all pass; drives the adapter withUSER_PROMPTenv fixtures, no live IDE needed.Current Stagesyncs.@anthropic-ai/claude-agent-sdkmodule-not-found failures (t19, t140, t142), unrelated to this change.