[Bug] Wire reviewer across harnesses: Kiro fs_write cap, trustedAgents, codex §12a step#405
Merged
Conversation
…er slugs, add codex §12a step Brings the shipped v2 reviewer (verification-agent) mechanism up to scratch so it actually runs everywhere and cannot write outside its lane. Three self-contained defects fixed; no change to per-unit / swarm reviewer behaviour (that depends on the unbuilt #368 per-unit iteration engine). W1 — SECURITY: the Kiro reviewer agents granted unrestricted fs_write (no allowedPaths cap), broader than the builders they review and the path-capped conductor. Cap fs_write to aidlc/spaces/** — the only place the reviewer legitimately writes (the `## Review` section appended to the primary artifact under the per-intent record dir), matching the conductor's own cap. (The original branch capped to aidlc-docs/**; that layout was retired on v2 in favour of the per-intent aidlc/spaces workspace, so the cap was repointed during the rebase.) Files: harness/{kiro,kiro-ide}/agents/aidlc-architecture-reviewer-agent.json, aidlc-product-lead-agent.json. (claude uses .md with no JSON tool-grant layer; codex .toml carries no write grant — defect is Kiro-specific.) W2 — INVOCATION: the reviewer personas were not in the conductor's subagent.trustedAgents on Kiro/Kiro-IDE, so invoking them via the subagent tool prompted/blocked. Added aidlc-architecture-reviewer-agent and aidlc-product-lead-agent to trustedAgents in harness/{kiro,kiro-ide}/agents/aidlc.json. W3 — CODEX INERT: the codex orchestrator SKILL.md had zero reviewer wiring, so a stage's reviewer: directive was silently ignored. Wired the §12a reviewer step into the gate:true flow as the first sub-step (before learnings), using codex's sub-agent idiom (spawn the agent role; harness resolves the .toml persona); renumbered subsequent steps. Codex has no STAGE-RITUAL-IS-ATOMIC line so W3(b) was skipped. Also corrected the stale "11 agent" comments in harness/codex/emit.ts to 13 (the transpose emits 13 agent TOMLs). Per the Changelog Policy, this commit bumps core/tools/aidlc-version.ts 2.1.7 → 2.1.8, the README badge, adds the ## [2.1.8] CHANGELOG heading, and includes the regenerated dist/ trees (claude/codex/kiro/kiro-ide). (Originally authored as 2.0.3 against the 2.0.2 base; re-bumped on the rebase onto v2 at 2.1.7.)
apackeer
force-pushed
the
fix/reviewer-harness-wiring
branch
from
687a3ff to
22a329b
Compare
t-acp-kiro-reviewer.serial.test.ts drives a reviewer-declaring stage (requirements-analysis, poc, reviewer: aidlc-product-lead-agent) over a live kiro-cli ACP session through the production aidlc conductor via next --stage <slug> --single, and asserts the section-12a contract on disk: the ## Review READY/NOT-READY verdict appended to requirements.md under the per-intent record (aidlc/spaces/**, the tree the reviewer's fs_write cap names), no resurrection of the retired flat aidlc-docs/ layout, and a sub-agent tool call carrying the reviewer slug. Turn shape: disk-condition cancel (poll for ## Review then session/cancel — the ACP forwarding loop never voluntarily ends), plus one bounded follow-up turn if the model ends its turn on the stage's prose-rendered questions. withState is load-bearing: without a seeded aidlc-state.md the intent record does not resolve and the engine's recordPrefix falls back to the bare intents root, so the stage writes to a shard the poll never watches (live-observed on the first run). Gated AIDLC_KIRO_ACP_LIVE=1; skips clean otherwise. Live GREEN 1/0 (5 expects, 328s). Test-only: no version bump.
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.
What & why
The v2 reviewer mechanism — an optional
reviewer:stage field that invokes a clean-room sub-agent which appends a READY/NOT-READY## Reviewsection to the artifact (stage-protocol §12a) — shipped in 2.0.0 but is inert or unsafe on some harnesses. This PR fixes three self-contained defects so the reviewer actually runs everywhere and can't write outside its lane. No change to per-unit / swarm reviewer behaviour (that depends on the unbuilt per-unit iteration engine and is deliberately left alone).The three fixes
W1 — SECURITY: cap the Kiro reviewer's
fs_writeThe Kiro reviewer agent configs granted
fs_writewith noallowedPathscap — broader than the builder agents they review, and broader than the conductor (which is path-capped). The reviewer only ever appends a## Reviewsection to the primary artifact under theaidlc/spaces/**workspace (the per-intent record dir - the flataidlc-docs/layout this PR originally capped to was retired on v2), so its write grant is now capped to match the conductor's ownfs_writecap:Applied to all four reviewer JSONs:
harness/{kiro,kiro-ide}/agents/aidlc-architecture-reviewer-agent.jsonandaidlc-product-lead-agent.json.tools/allowedTools/execute_bashare unchanged. (Kiro-specific: the Claude harness uses.mdagents with no JSON tool-grant layer, and Codex.tomlagents carry no write grant — verified.)W2 — INVOCATION: trust the reviewer slugs
The reviewer personas were missing from the conductor's
subagent.trustedAgentson Kiro / Kiro-IDE, so invoking the reviewer via thesubagenttool would prompt or block. Addedaidlc-architecture-reviewer-agentandaidlc-product-lead-agenttosubagent.trustedAgentsinharness/{kiro,kiro-ide}/agents/aidlc.json(the existingaidlc-developer-agent+aidlc-architect-agentare retained).W3 — CODEX INERT: wire the §12a reviewer step into the Codex orchestrator
The Codex orchestrator
SKILL.mdhad zero reviewer wiring while Claude and Kiro both have it, so on Codex a stage'sreviewer:directive was silently ignored. The reviewer persona configs already ship to Codex (.codex/agents/aidlc-architecture-reviewer-agent.toml+aidlc-product-lead-agent.toml), so this is a prose fix:gate: trueblock inharness/codex/skills/aidlc/SKILL.md, using Codex's sub-agent idiom (spawn the agent role — the harness resolves.codex/agents/aidlc-<role>-agent.toml, which loads its own persona viadeveloper_instructions); subsequent steps renumbered.SKILL.md, so there was nothing to mirror — skipped.11 agentcomments inharness/codex/emit.tsto13(the transpose emits 13 agent TOMLs = 11 domain + 2 reviewer personas).Version
Per the Changelog Policy this bumps
core/tools/aidlc-version.ts2.1.7 → 2.1.8, the README badge, and adds a## [2.1.8]CHANGELOG entry; the regenerateddist/trees (claude / codex / kiro / kiro-ide) ride in the same commit. (Originally authored as 2.0.3 against the 2.0.2 base; rebased onto v2 at 2.1.7 on 2026-07-03 and re-bumped.)Verification
bun run typecheck— 0 errorsbun scripts/package.ts --check— all four harness trees in sync (no dist drift)bun test tests/unit/t68-version-changelog-sync.test.ts— green (version trio agrees at 2.0.3)-P 8): smoke + unit 102 files / 2024 assertions / 0 failures. The two integration reds in the full parallel run (t72,t20) are live-SDK nondeterminism — both pass on isolated re-run and neither touches this diff.Rebase notes (2026-07-03, onto v2 @ 2.1.7)
The branch was rebased across ~20 releases (2.0.2 -> 2.1.7). Each fix was re-verified against current v2 and three adjustments were made where the codebase had moved:
aidlc-docs/no longer exists on v2 (replaced by the per-intentaidlc/spaces/**workspace), so the reviewerfs_writecap now targetsaidlc/spaces/**- identical to the conductor's own cap. Verified the## Reviewsection is appended to the primary artifact under the intent record dir (stage-protocol.md "Reviewer executes").trustedAgentsgap; both reviewer slugs are added there too.aidlc/spaces/<space>/memory/org.md; the test-run carve-out is gone per 2.1.4). One of the two stale "11 agent" comments inemit.tshad drifted to a third spot; all now read 13.Re-verified on the rebased head:
bun run typecheck0 errors;package.ts --checkclean on all four trees; t68 + t04 green; smoke+unit tier 125 files / 2242 assertions / 0 failures (stamp2026-07-03T10-14-20Z); deterministic e2e slice 31 files with the single red (t126, live Claude-SDK emitter-pairing) re-run green-alone (stamp2026-07-03T10-30-37Z) - a known agent-latency flake, not a regression.Out of scope
(deliberately not touched)
Per-unit /
for_eachreviewer behaviour and the swarm path (depend on the unbuilt per-unit iteration engine), the §12a loop determinism, and the## Reviewbuilder-slug signature bug.