Skip to content

v1.26.2 ·

Eleven reviews the gate could not see

The problem

aidlc review <instance> requirements writes requirements.review.md. The review gate looks for a findings entry keyed requirements.md. The two never meet, so a complete adversarial review — written, parsed, counted, and summarised correctly — is invisible to the gate it was run for.

The mechanism, confirmed by reading:

  • loadFindings (transition.ts:190) keys the index on file.replace('.review.md', ''), so requirements.review.md becomes the key requirements.
  • checkReviewGate (lifecycle.ts:412-418) iterates templatePhase.required_artifacts, whose names are requirements.md and design.md, and looks each up with hasOwnProperty.
  • Only full-feature declares the gate, on exactly requirements and design (full-feature.yaml:42-45). bugfix declares it empty; quick-feature, micro-task and spike declare no gates at all — including this instance, which is therefore not self-blocking.

The failure is worse than the report

The item asked whether any completed instance holds an orphaned findings file that never counted. It does — 11 files across 6 instances, every one of them a full-feature instance, and every one on requirements or design, the two phases the gate covers:

InstanceOrphaned files
app-operational-skillsdesign.review.md
cost-trackingdesign.review.md, requirements.review.md
nextjs-website-revampdesign.review.md, requirements.review.md
release-blog-traildesign.review.md, requirements.review.md
task-breakdowndesign.review.md, requirements.review.md
traceability-idsdesign.review.md, requirements.review.md

Eleven adversarial reviews were performed and none of them counted toward the gate they were run for. app-operational-skills is the sharpest case: it holds design.review.md (orphaned) and requirements.md.review.md (counted), so one instance used both spellings.

Every instance from agent-portability (2026-08-21) onward uses the .md.review.md spelling. The convention did not change in code; agents simply started typing the extension. Nothing rewarded them for it and nothing warned the ones who did not.

One correction to the item

The item states "the command's own help gives no hint that the .md is load-bearing". The help text does carry a hint — the argument is described as 'Artifact filename to review (e.g., requirements.md)' (review.ts:135). The defect is not a missing hint; it is that nothing enforces it. A hint in help text is not a boundary.

How it could be solved

The roadmap item that reported this named two directions, and warned about the second one itself.

Make the command append the extension. Typing requirements would write the file the gate already looks for, and nothing else would change. Cheap, and it leaves every file already written under the old spelling exactly as dead as it was. Eleven of them, across six completed instances, every one on a phase that a template actually gates. A fix that leaves the reported symptom standing in eleven places is not the fix.

Make the gate accept both spellings. That revives the eleven, and the item flagged the cost in the same breath: two spellings both become valid forever. Somebody reads an instance directory in a year, sees two conventions, and has to work out which one the gate honours. Nothing would ever collapse them, because both would keep working.

The two options are not alternatives. They are the two halves of one fix, and taking either alone is what makes it wrong.

So the shape that shipped is a single module owning the mapping in both directions, with the writer and the reader deriving their key from it. The writer stops producing new orphans; the reader adopts the old ones. And because the resolver strips the suffix and only then canonicalises, both spellings converge on one key by construction rather than by a compatibility branch — no call site anywhere has to know which spelling it is looking at, which is why the change is small enough to read in one sitting.

That left one real decision: which spelling wins. The doubled .md is ugly, and it won on arithmetic. It is what the gate already looks up, so the gate needed no change at all. It is what thirty-one files across thirteen instances already carry, against eleven. Choosing the prettier spelling would have moved thirty-one files in order to stop eleven from moving, and would have touched the gate as well.

A third thing got added that the item did not ask for. Typing an artifact name no template declares used to scaffold a findings file happily — a typo produced a real file that no gate would ever read, which is the same silent-success shape one step earlier in the pipeline. The command now refuses it and lists the names that would work. The allowlist deliberately admits optional artifacts and every phase in scope rather than only the current phase's required ones, because a real instance had reviewed an optional architecture diagram, and re-reviewing an earlier phase's artifact is exactly what the amendment protocol asks for. Both would have been refused by the narrower reading, and neither is a typo.

The one thing explicitly not done: nothing re-runs or re-opens the six instances holding the orphaned reviews. Their filenames are reconciled and their history stands. The reviews were real when they were written; only the gate's opinion of them was wrong.

How AIDLC solves it

A bugfix to the review-findings filename, so aidlc review writes a name the review gate can read. Eleven findings files across six completed instances in this repository had never counted toward the gate they were run for; they now do, and are renamed.

Patch, not minor: no new capability, no interface removed. aidlc review accepts everything it accepted before and now also accepts the bare artifact name. The one behaviour that becomes stricter is the AC-7 allowlist — an artifact the instance's template does not declare is refused rather than scaffolded. That could in principle break somebody reviewing an artifact no template declares, which nothing in this repository does, and the alternative was leaving a typo silently producing a file no gate reads.

Changes

  • packages/cli/src/review/findings-naming.ts — new, the single declaration site for the artifact ↔ findings filename mapping in both directions.
  • packages/cli/src/commands/review.ts — filename from the helper; legacy probe so one artifact never gets two files; artifact allowlist; canonical name in frontmatter; help text states both spellings.
  • packages/cli/src/commands/transition.tsloadFindings keys through the helper; sorted iteration; canonical spelling wins a collision; an unparseable findings file warns instead of being skipped in silence.
  • packages/cli/src/core/lifecycle.ts — the gate violation names the filename it expected and the artifacts findings do exist for.
  • packages/cli/src/doctor/migrations/findings-filename.ts — new, safe tier, renames legacy files and reports collisions rather than resolving them.
  • packages/content/skills/80-review.md — states that <artifact> carries its own .md.
  • docs/reviews-and-guidance.md — see the docs step below.
  • Two expectations updated on purpose: packages/cli/test/golden/no-pack-cli.json (new message) and AC-13's touched-file guard in packages/website/src/test/blog-deferrals.test.ts (excludes pure renames).