feat: deterministic plan-approval guard - code-generation cannot dispatch before the human approves the plan (2.5.41)

[apackeer]

Summary

Code Generation's plan-before-generation ordering is now enforced deterministically. #631 reported the inversion: code-generation-plan.md was generated after the code, together with code-summary.md, making the plan a retroactive summary instead of the input to generation.

The reported symptom itself was fixed by the 2.5.0 Step 3 hard-stop rewrite (#568), and a live reproduction against v2 at 2.5.10 (Claude SDK, composer-trimmed construction, one unit) confirmed the correct ordering: plan written -> Plan Approval menu presented with the plan on disk and no summary -> developer agent dispatched only after the recorded "Approve Plan" answer. What remained was an enforcement gap: the ordering lived only in prose. Nothing refused a developer dispatch when the plan was missing, and the stage-completion artifact guard cannot catch the inversion because it fires at completion time, when a backfilled plan already exists. Per the framework layering (determinism belongs in tools and hooks), this PR gives the ordering its deterministic twin, following the reviewer-scope guard's pattern.

What changed

New framework hook aidlc-plan-approval-guard.ts (the 14th), PreToolUse. While the workflow's Current Stage is code-generation and Construction autonomy is not granted, a dispatch targeting aidlc-developer-agent is refused (exit 2 + a redirecting stderr reason naming stage Steps 2-3) unless a unit named in the dispatch prompt - any unit, when the prompt names none - has BOTH:

• code-generation-plan.md on disk under construction/<unit>/code-generation/, and
• an answered Plan Approval [Answer]: tag in code-generation-questions.md (the Stop hook's tag grammar: blank or underscores-only is pending, so a tag reset by "Request Changes" keeps blocking until the human re-approves).

Each refusal emits a PLAN_APPROVAL_BLOCKED audit row (the 75th event type) naming the tool, target agent, and mentioned unit(s).

Carve-outs and failure posture. Autonomous Construction swarms are exempt: the autonomy grant is the human's standing approval for the batch, the swarm referee owns per-unit verification, and a deterministic block would deadlock a granted swarm on a question no one is present to answer. The hook fails open on every ambiguity (no state file, another stage, another agent or tool, malformed stdin, unreadable evidence), and AIDLC_DISABLE_PLAN_APPROVAL_GUARD=1 disables enforcement entirely - the same escape-hatch pattern as the reviewer-scope guard.

Per-harness wiring.

Harness	Registration
Claude Code	settings.json PreToolUse, matcher Task
Kiro CLI	conductor agent JSON, matcher subagent; the adapter translates the crew schema (stages[].role/prompt_template) into the core hook's Task shape
Codex	hooks.json PreToolUse target (trust-seed regenerated); the adapter forwards spawn_agent calls naming the developer agent
opencode	plugin tool.execute.before on task dispatches
Kiro IDE	no registration - its hook payloads carry no tool arguments; the SKILL documents the bound as prose-only, matching the other guards

User experience. The happy path is unchanged - the hook allows silently when the plan and answer are on disk. The change is only visible when a conductor tries to generate first: the dispatch is refused with a reason that redirects it to write the plan and present the approval question, so the human always sees the plan and its gate before any code is generated.

Tests

• New tests/unit/t248-plan-approval-guard.test.ts (20 tests): the pure decision table (block on missing plan / unanswered tag / re-armed tag; allow on approval, other stages, other agents, autonomy; word-boundary unit matching), the hook subprocess lifecycle (block + stderr contract, PLAN_APPROVAL_BLOCKED audit row, every fail-open path, the off-switch), and per-harness registration pins including the deliberate Kiro IDE absence.
• Count pins updated: hooks 13 -> 14 (t01, t02, t132 including the fourteen word map), audit events 74 -> 75 (t28, t81, t111, t239), t219 project-dir reference count, t150 codex trust-key roster; coverage registry regenerated.
• Gates on the rebased head: package.ts --check clean, typecheck clean, biome clean, smoke+unit 62 files / 4418 assertions / 0 failures. The integration tier ran fully on the pre-rebase head; its 3 unrelated reds (t66, t89, t72) reproduce identically on a pristine v2 checkout (t66/t89 broke upstream with the claim-sources sensor and are fixed on the open fix/issue-495-rules-delivery branch; t72 is a live SDK journey exceeding its 885s budget on baseline too).

Notes for reviewers

• Prompt-based unit matching is a deliberate lenient heuristic: a dispatch prompt that names no known unit falls back to "any unit approved", keeping false blocks near zero; a workflow with no plan anywhere (the reported failure) is refused outright.
• Codex's spawn_agent input schema is not fixture-pinned in this repo, so the adapter shim is shape-agnostic: it scans the serialized tool_input for the developer-agent name rather than trusting a typed field.
• Version slot 2.5.15: 2.5.12-2.5.14 are claimed by open PRs (feat(onboard): /aidlc-onboard S1 — promote core + rule route (2.5.40) #660/fix: deliver stage steering deterministically - bounded rule chunks, dispatch-boundary enforcement, absorbed reviewer checklists (2.5.33) #658, feat: GitHub Copilot harness - one dist for Copilot CLI + VS Code agent mode (2.5.46) #657/feat: move the ARS arithmetic into a deterministic ars subcommand (2.5.25) #644, feat: Cursor harness support (2.5.63) #661). Will re-bump at merge time if the slot is taken.

Fixes #631

[leandrodamascena]

Thanks for adding a deterministic guard for this ordering. I reproduced two paths that still defeat the guarantee.

1. [P1] Require every mentioned unit to be approved
   core/hooks/aidlc-plan-approval-guard.ts:173-178

The decision uses mentioned.some(approved). A dispatch such as “Generate todo-core using auth for reference” is allowed when auth is approved even if todo-core has no plan or approval. I reproduced this against the real hook; omitting the approved sibling correctly returned exit 2.

Use mentioned.every(approved), or block when any mentioned unit lacks approval. t248 does not cover this cell.

2. [P1] Support the compiled hook route
   core/hooks/aidlc-plan-approval-guard.ts:250
core/tools/aidlc.ts:943-953

The hook executes only under import.meta.main and does not export run(input). The compiled dispatcher consequently reports hook does not export run(input), while the Codex and Kiro adapters convert that non-2 result into allow.

I built the native binary and reproduced the same unapproved developer dispatch:

• Source Codex/Kiro adapters: exit 2
• Compiled Codex/Kiro adapter routes: exit 0

The binary smoke suite passes because it exercises validate-state, not this new flow-altering hook.

3. [P2] Accept only explicit Plan Approval
   core/hooks/aidlc-plan-approval-guard.ts:117-133

questionsFileApproved() treats any nonblank [Answer]: as approval. Both B. Request Changes and an unrelated answered question return true. The documented re-arm therefore depends on the conductor first resetting the tag, recreating the prose dependency this hook is intended to remove.

Parse the Plan Approval question specifically and accept only the explicit “Approve Plan” response.

4. [P2] Identify the Codex target without scanning unrelated prompt text
   harness/codex/hooks/aidlc-codex-adapter.ts:472-480

The top-level agent_type is the currently acting agent, not the spawn target, so it should not be used here. However, scanning the complete serialized tool_input still causes false positives. I reproduced a spawn targeting aidlc-quality-agent whose message mentioned “aidlc-developer-agent output”; it was incorrectly blocked as a developer dispatch.

Pin the actual spawn_agent input shape and inspect only its target field.

Non-blocking follow-ups:

• gatherUnitEvidence() accepts an empty code-generation-plan.md because it checks only existence.
• docs/guide/10-state-and-audit.md:82 says 19 event categories; the table and canonical registry contain 20.

Verification:

• 182 focused tests passed
• Typecheck passed
• Biome passed
• Package parity passed
• Native binary build passed
• PR CI is green
• Worktree remained clean

Requesting changes because the multi-unit and compiled-mode paths both allow unapproved generation.

[apackeer]

Addressed all review findings in b127dbe:

• Require every unit mentioned by a developer dispatch to have approved evidence.
• Export run(input) from the guard and route compiled Kiro execution through the binary; added native gates for the direct hook plus compiled Codex/Kiro adapter paths.
• Accept only an explicit Approve Plan answer under the Plan Approval heading.
• Read the Codex spawn target only from tool_input.agent_type, and forward prompt text only from message/items, avoiding unrelated-message false positives.
• Treat empty/whitespace-only code-generation-plan.md files as missing.
• Correct the audit taxonomy category count from 19 to 20.

Verification completed:

• bun run typecheck
• bun run lint
• bun scripts/package.ts --check
• Focused unit suite: 80 passed, 0 failed
• bun scripts/build-binaries.ts --target native
• git diff --check

[leandrodamascena]

Thanks for addressing the previous review. I verified that all four original findings are fixed, including the compiled Codex and Kiro paths. Two edge cases still undermine the deterministic guarantee.

1. [P2] Determine the target unit without treating every prompt mention as a target (core/hooks/aidlc-plan-approval-guard.ts:180-185)

The guard now requires every unit mentioned anywhere in the prompt to have approved evidence. This blocks valid dispatches when the current unit is approved but the prompt mentions another unit as context or a dependency. I reproduced this with an approved todo-core and unapproved auth: Implement todo-core using the auth contract for reference is blocked for both units, even though auth is only contextual.

The opposite bypass remains when no known unit is mentioned: ctx.units.some(approved) allows the dispatch if any unrelated unit is approved, even when the actual target may be unapproved. The target should come from a deterministic field, marker, or unambiguous artifact path rather than arbitrary mentions in prompt prose.

2. [P2] Do not require an undocumented exact Markdown heading (core/hooks/aidlc-plan-approval-guard.ts:121-141)

questionsFileApproved() recognizes approval only under a heading whose complete text is exactly Plan Approval. The stage contract requires a “Plan Approval question,” but does not prescribe that exact heading. For example, this valid representation is rejected:

## Q1: Plan Approval
A. Approve Plan
B. Request Changes
[Answer]: A. Approve Plan

I reproduced questionsFileApproved() returning false for both Q1: Plan Approval and Question 1 - Plan Approval. Either define and enforce an exact persisted format in the stage contract or make the parser recognize the question identifier within the heading.

Verification:

• All four previous findings are resolved.
• 80 focused tests passed.
• Native binary build and the new compiled gates passed.
• Package parity passed across all harnesses.
• Local typecheck and lint were unavailable because the isolated worktree lacked bun-types and biome.
• No CI checks are currently reported.
• The branch conflicts with current v2 and requires a rebase/version bump.

[apackeer]

Addressed the second-round review comments in 8678fffc:

• Replaced prompt-wide unit-name inference with an explicit first-line AIDLC-UNIT: <unit> delegation contract. Missing, unknown, or conflicting markers block; duplicate identical markers remain unambiguous; contextual dependency mentions no longer affect the target (core/hooks/aidlc-plan-approval-guard.ts:113-170, core/aidlc-common/stages/construction/code-generation.md:156-160).
• Expanded Plan Approval heading recognition to accept the documented unnumbered form plus Q1: Plan Approval and Question 1 - Plan Approval, while still requiring the explicit Approve Plan answer (core/hooks/aidlc-plan-approval-guard.ts:116-147).
• Added focused regression coverage for contextual sibling mentions, missing/unknown/conflicting markers, duplicate markers, and numbered headings (tests/unit/t263-plan-approval-guard.test.ts:99).
• Rebased onto current v2, resolved the test-number collision as t263, regenerated all harness distributions/trust hashes, and aligned the release metadata at 2.5.37 (CHANGELOG.md:4).

Validation completed:

• Focused smoke/unit/integration slice: 17 files passed, 0 failed; 294 assertions passed, 0 failed
• bun run typecheck
• bun run lint
• bun scripts/package.ts --check
• bun tests/gen-coverage-registry.ts --check
• bun scripts/build-binaries.ts --target native
• git diff --check

The new GitHub CI run is queued on this head.

[leandrodamascena]

Requesting changes for two remaining issues:

1. tests/smoke/t148-kiro-file-structure.test.ts:280 still expects only one subagent matcher, leaving CI red after the new hook adds a second one.

2. questionsFileApproved() only recognizes “Plan Approval” when it appears in a heading. The stage protocol permits ## Q1 followed by the question text and [Answer]: A. Approve Plan; this valid format is incorrectly rejected. Please either support it or explicitly require a deterministic heading in the stage contract and tests.

Focused validation: 98 tests passed; t148 reproduced with one failure.

[leandrodamascena]

Thank you for the updates. Both findings from my latest review are addressed:

• The Kiro smoke test now validates both subagent hook registrations.
• questionsFileApproved() now supports ## Q1 followed by Plan Approval, while still requiring the explicit Approve Plan answer.

I verified the focused suite locally: 40 tests / 264 assertions passed, along with typecheck and package parity. CI is also green.

No remaining blocking findings.

Non-blocking cleanup: the PR title/body still reference version 2.5.15 and describe autonomous swarms as exempt, while the current implementation and CHANGELOG use 2.5.38 and require the same approval evidence under autonomous Construction.