Skip to content

[Bugfix #1279] Deliver every protocol template via a {{> }} include, and enforce it - #1283

Merged
waleedkadous merged 12 commits into
mainfrom
builder/bugfix-1279
Jul 29, 2026
Merged

[Bugfix #1279] Deliver every protocol template via a {{> }} include, and enforce it#1283
waleedkadous merged 12 commits into
mainfrom
builder/bugfix-1279

Conversation

@waleedkadous

@waleedkadous waleedkadous commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

codev-skeleton/protocols/spir/templates/spec.md was dead code — no prompt delivered it, no consult type checked against it, no porch check enforced it. Builders never saw it and pattern-matched whatever spec already sat in codev/specs/.

The audit the issue asked for found this is a class problem, not a one-off: Spec 1011 built the {{> <codev-path>}} include mechanism and wired it for three of the nine shipped templates. The other six rotted.

Template Before After
spir/templates/plan.md delivered (#1011) unchanged
experiment/templates/notes.md delivered (#1011) unchanged
spike/templates/findings.md delivered (#1011) unchanged
spir/templates/spec.md dead — the reported bug delivered by prompts/specify.md
spir/templates/review.md dead — prompt carried a divergent inline copy delivered by prompts/review.md
maintain/templates/maintenance-run.md deadprotocol.md carried a divergent inline copy delivered by protocol.md
aspir/templates/spec.md dead — byte-identical dup, zero consumers deleted
aspir/templates/plan.md deadaspir/prompts/plan.md includes SPIR's deleted
aspir/templates/review.md dead — byte-identical dup, zero consumers deleted

Root Cause

An authored artifact with no owning consumer. Nothing in the codebase reads protocols/*/templates/ except a {{> }} include, and no invariant said every template must have one. So a template could be written, reviewed, merged, and then never delivered to anybody — exactly the class Spec 1252 attacked.

The drift was already real, not hypothetical. Both hand-rolled inline copies had diverged from the template they duplicated, in both directions:

  • spir/prompts/review.md's block was missing ## Key Metrics, ## Timelog, ## Consultation Iteration Summary, ## Architecture Updates, ## Lessons Learned Updates; the template was missing ## Flaky Tests and ### Methodology Improvements.
  • maintain/protocol.md's block had ## Audit Findings; the template didn't — and the template had ### Dependencies Cleaned, which the inline copy had dropped, so maintain builders have been silently missing that heading.

Everything that existed only in an inline copy was merged into the canonical template before the swap, so no content is lost.

Finding beyond the original report: spec-approval gated on nothing

The issue said the gate "checks only that the spec file exists." It checked nothing at all — the specify phase carried "checks": {} in both spir/protocol.json and aspir/protocol.json, while plan had three checks and review had four. An empty spec file, or one that ignored the template wholesale, reached the human gate with zero automated signal. That's a second gap, and it's fixed here.

Fix

  1. Deliver the three live-but-undelivered templates via {{> }}spir/aspir prompts/specify.md, spir/aspir prompts/review.md, maintain/protocol.md.
  2. Delete the six dead ASPIR duplicates. ASPIR now includes SPIR's templates directly (as aspir/prompts/plan.md already did), so there is exactly one copy of each template that can drift. aspir/protocol.md updated to match.
  3. Structure focus area in the spir/aspir spec-review consult type, with a carveout that a single genuinely-N/A section is not grounds for REQUEST_CHANGES.
  4. Two specify-phase porch checksspec_exists and spec_has_required_sections, backed by runArtifactCheck cases following the existing plan_exists / has_phases_json pattern. The required-heading list is a deliberate core subset (4 of the template's 20), calibrated against this repo's 166 real specs — see below. This is a backstop against wholesale departure, not a style linter; the failure message names exactly which headings are missing.
  5. The enforcement invarianttemplate-delivery.test.ts.

Calibrating the porch check against real specs

I picked the required headings by reading the template, then checked that guess against the repo's 166 existing specs, restricted to the 40 most recent (the corpus written under mature SPIR). Absence rates there:

Heading Absent in recent specs Required?
## Success Criteria 0% yes
## Problem Statement 5% yes
## Desired State 5% yes
## Current State 12% yes
## Open Questions 15% no — advisory only
## Solution Approaches 30% no — advisory only

Gating on ## Solution Approaches would have failed about a third of legitimate recent specs; a spec with one obvious approach is normal, especially when the architect pre-wrote it. A hard gate that fires that often trains people to route around it, which is worse than no gate. Both dropped headings are still checked by the spec-review Structure focus area, where a model can weigh whether the omission is reasonable. A calibration guard test keeps the decision from being silently reverted.

Everything is mirrored across codev/ and codev-skeleton/; the two trees are verified at zero drift.

Byte-identity proof before deletion

All six deleted files are byte-identical to their SPIR counterparts. Hashed from git show HEAD: so in-flight edits could not confound the comparison:

codev           spec    aspir=07237c2117438147  spir=07237c2117438147  IDENTICAL
codev           plan    aspir=3ddb80a3930487dd  spir=3ddb80a3930487dd  IDENTICAL
codev           review  aspir=68e0b3cdd6343794  spir=68e0b3cdd6343794  IDENTICAL
codev-skeleton  spec    aspir=07237c2117438147  spir=07237c2117438147  IDENTICAL
codev-skeleton  plan    aspir=3ddb80a3930487dd  spir=3ddb80a3930487dd  IDENTICAL
codev-skeleton  review  aspir=68e0b3cdd6343794  spir=68e0b3cdd6343794  IDENTICAL

Cross-tree (codev vs codev-skeleton) for the same three: IDENTICAL. Nothing codev-unique dies.

Test Plan

Regression test: packages/codev/src/__tests__/template-delivery.test.ts (15 tests). Enforces both directions — no orphaned template, no dangling include — across both trees, plus the specific wirings by name and the resolved-prompt content that must actually reach a builder.

Enforcement is by reachability, not by "this path is mentioned in an include somewhere" — see the Codex finding below. The detector walks out from the only two surfaces that actually resolve includes (prompts/*.md via porch's loadPromptFile, protocol.md via resolveProtocolReference) and follows include edges transitively. A companion check flags an include written anywhere else, since nothing expands it there.

Mutation-checked (per the 1252 T12 lesson — an enforcement test that can't fail is decoration). The final describe block copies the real tree and seeds seven scenarios: a clean control, a bare orphan, re-created aspir duplicates, the reported spec-template unwiring, an include placed in a consult-type, two orphaned templates including each other, and the positive transitive case. Each asserts the detector's exact output. Verified out-of-band against the live test too — seeding seeded-orphan.md into the real skeleton produces:

FAIL  template-delivery.test.ts > codev-skeleton: no template is orphaned
AssertionError: Orphaned template(s) in codev-skeleton/ — no prompt or protocol.md
delivers them via a {{> path}} include, so builders never see them. Either wire
them up or delete them:
  - protocols/spir/templates/seeded-orphan.md

The invariant already earned its keep: it caught a bug in this very PR. I wrote a literal {{> protocols/spir/templates/<name>.md}} inside backticks in aspir/protocol.md prose to explain the mechanism. resolveCodevIncludes does a blind regex replace, so it would have expanded that to empty at delivery — silently deleting the paragraph. The dangling-include check flagged it immediately.

Other tests: checks.test.ts +8 covering the two new spec checks, including the exact reported failure mode (a readable free-form spec that ignores the template). review-prompt-routing.test.ts updated for the deleted aspir template paths.

Results:

  • pnpm build: clean.
  • Full unit suite: 3793 passed / 48 skipped / 0 failed (193 files), run from packages/codev per the repo's directory-map rule. No flaky tests encountered; nothing skipped by me.
  • Porch fix-phase checks: build ✓, tests ✓.
  • End-to-end through the real four-tier resolver (not just the test's synthetic call): all five new includes expand, zero {{> residue, and the specify prompt grows 4.6KB → 9.65KB with the template attached. Served from tier 2 (codev/) in this repo, as expected.

CMAP (3-way review)

Model Verdict Confidence
Gemini (agy) APPROVE HIGH
Codex REQUEST_CHANGES → addressed HIGH
Claude APPROVE HIGH

Codex — REQUEST_CHANGES, and it was right. Its finding: the enforcement test "does not enforce the invariant described in the PR body — collectIncludeTargets() scans every *.md under protocols/, so an include mentioned in a consult-type, template, or other non-delivery markdown would mark a template as consumed even though no builder prompt or protocol.md actually delivers it."

Verified and addressed in a069dc28. I confirmed the premise against the source: commands/consult/index.ts loads consult-types with plain readCodevFile, with no resolveCodevIncludes — so an include written there is never expanded. It would reach the reviewing model as literal {{> ... }} text and leave the template unreachable by every builder, while the old detector scored it as delivered. My PR body claimed more than the test enforced.

findOrphanedTemplates now computes reachability from the two real delivery roots and follows include edges transitively. Added findUnresolvedIncludeSites (an include placed where nothing resolves it is its own silent bug) plus three tests for the tightened semantics, including a mutation reproducing Codex's exact scenario. Re-verified the live seeded-orphan demo still fails after the tightening.

Gemini's APPROVE came back in 11.6s and largely restates the PR body — I'd weight it as a low-information signal rather than independent confirmation. Claude's APPROVE followed a substantive pass (it read the diff, the new tests, and independently verified mirror parity across both trees on disk).

Post-review commits: a069dc28 (reachability fix) and dcb22f8b (check calibration — self-initiated, from measuring the check against real specs rather than from a reviewer finding).

Notes for the reviewer

Diff size. 37 files, roughly +780/−1230. 1,221 of those lines are deletions, and 916 are the six dead duplicate files (3 templates × 2 trees — codev/ shadow copies still exist because #1278 closed unmerged). The remaining deletions are the two inline template copies being replaced by one-line includes. Substantively this is a small change; it's deletion-dominated by design. Flagged to the architect before starting and endorsed to proceed within BUGFIX.

Relationship to #1280. #1280 may rewrite the prompts and reshape the templates themselves. That's orthogonal to this PR and survives it: what lands here is the wiring (templates reach builders) and the invariant (a template with no consumer fails CI). Whatever #1280 decides the templates should say, the delivery mechanism and the enforcement stay correct — and if #1280 adds a template, the test forces it to be wired in the same change.

One follow-up left deliberately undone. codev/protocols/maintain/templates/{audit-report,lessons-learned}.md are also unreferenced, but they're local-only with no skeleton counterpart and were explicitly preserved by an architect T8 ruling in the 1252 shadow-tree audit. Reversing that ruling isn't this fix's call, so they're exempted in the test with the reason in a comment. Worth a separate decision.

Spec 746 baseline interaction, for whoever edits these files next. spir/aspir prompts/specify.md (and air/prompts/implement.md, and the consult-types) are guarded by frozen pure-addition baselines. Any edit that modifies an existing line fails baked-decisions.test.ts rather than merely adding to it. This PR's specify-prompt change is structured as a strict addition for that reason.

Fixes #1279

Spec 1011 introduced the {{> <codev-path>}} include as the resolver-aware way
to deliver a framework file to a builder, but wired it for only three of the
nine shipped templates. The rest were authored artifacts with no owning
consumer: prompts/specify.md described spec content in prose and never
referenced templates/spec.md, so builders pattern-matched whatever spec
happened to be in codev/specs/ already.

Wires the three live-but-undelivered templates:

- spir + aspir prompts/specify.md now include templates/spec.md
- spir + aspir prompts/review.md now include templates/review.md, replacing a
  hand-rolled inline block
- maintain/protocol.md now includes templates/maintenance-run.md, replacing a
  hand-rolled inline block

Both inline copies had already drifted from the template they duplicated, in
both directions. Content that existed only in an inline copy is merged into the
canonical template first, so the swap loses nothing: '## Flaky Tests' and
'### Methodology Improvements' into the review template, '## Audit Findings'
and the changes-log example row into the maintenance-run template.

Deletes the six dead ASPIR template duplicates. aspir/templates/{spec,plan,
review}.md had zero consumers in either tree — aspir/prompts/plan.md already
included SPIR's plan template rather than ASPIR's own copy, so ASPIR's
templates/ directory was unreachable in full. All six are byte-identical to
their SPIR counterparts at HEAD (sha256 verified per file, per tree, and
cross-tree before deletion), so nothing codev-unique is lost. ASPIR now
includes SPIR's templates directly, leaving exactly one copy of each template
that can drift. protocol.md updated to match (no templates/ dir; ASCII tree
corrected).

Mirrored across codev/ and codev-skeleton/.
The issue reported that the spec-approval gate 'checks only that the spec file
exists'. It checked nothing at all — the specify phase carried "checks": {} in
both spir/protocol.json and aspir/protocol.json, while plan had three checks and
review four. A spec that ignored the template, or an empty spec file, reached
the human gate with no automated signal.

Adds two specify-phase checks, following the existing plan_exists /
has_phases_json pattern:

- spec_exists — the spec resolves via the artifact resolver
- spec_has_required_sections — the spec carries the six load-bearing headings
  the canonical template guarantees (Problem Statement, Current State, Desired
  State, Success Criteria, Solution Approaches, Open Questions)

Both are backed by runArtifactCheck cases so they run through the resolver
rather than a shell grep, with the protocol.json command as the fallback. The
required-heading list is a deliberate core subset, not the template's full
20-heading list: this is a backstop against a wholesale departure from the
template, not a style linter, and the failure message names exactly which
headings are missing.

Also adds a Structure focus area to the spir/aspir spec-review consult type so
reviewers check the spec against the template the specify prompt now delivers —
with the carveout that a single N/A section is not grounds for REQUEST_CHANGES.
template-delivery.test.ts enforces the invariant this bug violated: a file
under protocols/<p>/templates/ is dead code unless some prompt or protocol.md
delivers it via a {{> }} include. Both directions are checked (no orphaned
template, no dangling include) across both trees, plus the specific wirings by
name and the resolved-prompt content that must reach a builder.

The last describe block is a mutation check — an enforcement test that cannot
fail is decoration. It copies the real tree and seeds four distinct violations
(a bare orphan, re-created aspir duplicates, the reported spec-template unwiring,
and a clean control), asserting the detector reports each. Verified out-of-band
that seeding an orphan into the real skeleton fails the live test with an
actionable message naming the file.

The invariant already earned its keep: it caught a literal {{> }} directive
written into aspir/protocol.md prose during this fix, which the resolver would
have expanded to empty at delivery.

checks.test.ts covers the two new spec checks including the exact reported
failure mode (a readable free-form spec that ignores the template).

review-prompt-routing.test.ts drops the two deleted aspir template paths.
Extends the existing '#1011 Protocol & Template Delivery' subsection in arch.md
rather than opening a new one — this is the same mechanism, now with an
enforced completeness rule. Routed to the COLD tier: it is authoring guidance
for framework files, not a decision-shaping fact needed in every phase prompt,
and the hot tier is at its cap.

Also lands the builder thread.
…lude mention

Addresses Codex's REQUEST_CHANGES on PR #1283. The detector counted an include
in ANY markdown under protocols/ as proof a template had a consumer. Only two
kinds of file actually get resolveCodevIncludes run over them:

- protocols/<p>/prompts/*.md — porch's loadPromptFile
- protocols/<p>/protocol.md  — resolveProtocolReference at spawn

Verified consult-types are NOT among them: commands/consult/index.ts loads them
with plain readCodevFile, so an include written there is never expanded — it
would reach the reviewing model as literal {{> ... }} text while leaving the
template it names unreachable by every builder. The old detector would have
scored that as delivered. The claim in the PR body was therefore stronger than
what the test enforced.

findOrphanedTemplates now walks out from the delivery roots and follows include
edges transitively (resolveCodevIncludes recurses, so a template included by a
delivered file is itself delivered), and reports anything not reached.

Adds findUnresolvedIncludeSites — an include placed where nothing resolves it is
its own silent failure, now caught directly — and three tests for the tightened
semantics: an include in a consult-type does not count as a consumer, two
orphaned templates including each other stay orphaned, and a template included
by a delivered file is reachable transitively.

Re-verified the mutation check still fails on a live seeded orphan after the
tightening. 20 tests in the file; full unit suite 3793 passed / 0 failed.
… specs

The required-heading set was picked by reading the template, not by measuring
what real specs contain. Checked it against this repo's 166 existing specs,
restricted to the 40 most recent (written under mature SPIR). Absence rates:

  Solution Approaches  30%
  Open Questions       15%
  Current State        12%
  Problem Statement     5%
  Desired State         5%
  Success Criteria      0%

Gating on Solution Approaches would have failed roughly a third of legitimate
recent specs — a spec with one obvious approach is normal, especially when the
architect pre-wrote it. A hard gate that fires that often trains people to route
around it, which is worse than no gate.

Drops Solution Approaches and Open Questions from the porch check, leaving the
four headings that recent practice honors 88-100% of the time. A spec missing
all four is unambiguously off-template, which is what this backstop is for. Both
dropped headings are still checked advisorily by the Structure focus area in the
spec-review consult type, where a model can weigh whether the omission is
reasonable.

Adds a calibration guard test so the decision is not silently reverted.
@waleedkadous
waleedkadous merged commit 54118ef into main Jul 29, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SPIR spec template is dead code: no prompt, consult type, or porch check references it

1 participant