diff --git a/codev-skeleton/protocols/aspir/consult-types/spec-review.md b/codev-skeleton/protocols/aspir/consult-types/spec-review.md index b537e7cc6..73e346e00 100644 --- a/codev-skeleton/protocols/aspir/consult-types/spec-review.md +++ b/codev-skeleton/protocols/aspir/consult-types/spec-review.md @@ -32,6 +32,12 @@ If the baked decisions themselves contain contradictions (e.g., two different la - Are acceptance criteria testable? - Is terminology consistent? +5. **Structure** + - The specify prompt delivers a canonical spec template (`protocols/spir/templates/spec.md`) inline. Does the spec actually follow it? + - Required headings, in order: `## Metadata`, `## Clarifying Questions Asked`, `## Problem Statement`, `## Current State`, `## Desired State`, `## Stakeholders`, `## Success Criteria`, `## Constraints`, `## Assumptions`, `## Solution Approaches`, `## Open Questions`, `## Performance Requirements`, `## Security Considerations`, `## Test Scenarios`, `## Dependencies`, `## References`, `## Risks and Mitigation`, `## Expert Consultation`, `## Approval`, `## Notes`. + - A free-form spec that reads well but ignores the template is a **defect**, not a style preference — it usually means the builder pattern-matched an older spec in `codev/specs/` instead of the delivered template. `REQUEST_CHANGES` for a wholesale departure (most headings missing or renamed). + - A section that genuinely does not apply may be reduced to a one-line "N/A — [reason]", but the heading should remain. Do not `REQUEST_CHANGES` over one such section. + ## Verdict Format After your review, provide your verdict in exactly this format: diff --git a/codev-skeleton/protocols/aspir/prompts/review.md b/codev-skeleton/protocols/aspir/prompts/review.md index 22af9aec4..eabe1b98e 100644 --- a/codev-skeleton/protocols/aspir/prompts/review.md +++ b/codev-skeleton/protocols/aspir/prompts/review.md @@ -57,55 +57,9 @@ Compare final implementation to original specification: ### 3. Create Review Document -Create `codev/reviews/{{artifact_name}}.md`: +Create `codev/reviews/{{artifact_name}}.md`, following the template below. Use these headings and this order — do not invent your own structure, and do not pattern-match an earlier review in `codev/reviews/` that predates this template. Steps 3b and 4 below expand on the `## Consultation Feedback`, `## Architecture Updates`, and `## Lessons Learned Updates` sections; porch's review checks grep for the last two by exact heading. -```markdown -# Review: {{title}} - -## Summary -Brief description of what was implemented. - -## Spec Compliance -- [x] Requirement 1: Implemented as specified -- [x] Requirement 2: Implemented with deviation (see below) -- [x] Requirement 3: Implemented as specified - -## Deviations from Plan -- **Phase X**: [What changed and why] - -## Lessons Learned - -### What Went Well -- [Positive observation 1] -- [Positive observation 2] - -### Challenges Encountered -- [Challenge 1]: [How it was resolved] -- [Challenge 2]: [How it was resolved] - -### What Would Be Done Differently -- [Insight 1] -- [Insight 2] - -### Methodology Improvements -- [Suggested improvement to SPIR protocol] -- [Suggested improvement to tooling] - -## Technical Debt -- [Any shortcuts taken that should be addressed later] - -## Consultation Feedback - -[See instructions below] - -## Flaky Tests -- [Any pre-existing tests that were skipped as flaky during this project] -- [Include test name, file path, and observed failure mode] -- [If none: "No flaky tests encountered"] - -## Follow-up Items -- [Any items identified for future work] -``` +{{> protocols/spir/templates/review.md}} ### 3b. Include Consultation Feedback diff --git a/codev-skeleton/protocols/aspir/prompts/specify.md b/codev-skeleton/protocols/aspir/prompts/specify.md index 3716f7969..daa5feab2 100644 --- a/codev-skeleton/protocols/aspir/prompts/specify.md +++ b/codev-skeleton/protocols/aspir/prompts/specify.md @@ -90,6 +90,10 @@ After completing the spec draft, signal completion. Porch will run 3-way consult Create or update the specification file at `codev/specs/{{artifact_name}}.md`. +Follow the canonical spec template reproduced below. Use these headings, in this order — do not invent your own structure, and do not pattern-match an earlier spec in `codev/specs/` that predates this template. If a section genuinely does not apply, keep the heading and write a one-line "N/A — [reason]" rather than deleting it. + +{{> protocols/spir/templates/spec.md}} + **IMPORTANT**: Keep spec/plan/review filenames in sync: - Spec: `codev/specs/{{artifact_name}}.md` - Plan: `codev/plans/{{artifact_name}}.md` diff --git a/codev-skeleton/protocols/aspir/protocol.json b/codev-skeleton/protocols/aspir/protocol.json index fb5c4f993..eea40f1af 100644 --- a/codev-skeleton/protocols/aspir/protocol.json +++ b/codev-skeleton/protocols/aspir/protocol.json @@ -23,6 +23,16 @@ "models": ["gemini", "codex", "claude"], "parallel": true }, + "checks": { + "spec_exists": { + "command": "test -f codev/specs/${PROJECT_TITLE}.md", + "description": "Spec file must exist before the spec-approval gate" + }, + "spec_has_required_sections": { + "command": "grep -qF '## Problem Statement' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Current State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Desired State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Success Criteria' codev/specs/${PROJECT_TITLE}.md", + "description": "Spec must follow the canonical template delivered by the specify prompt (protocols/spir/templates/spec.md). Required headings: ## Problem Statement, ## Current State, ## Desired State, ## Success Criteria" + } + }, "max_iterations": 3, "on_complete": { "commit": true, diff --git a/codev-skeleton/protocols/aspir/protocol.md b/codev-skeleton/protocols/aspir/protocol.md index 18a423a30..6cc2caf97 100644 --- a/codev-skeleton/protocols/aspir/protocol.md +++ b/codev-skeleton/protocols/aspir/protocol.md @@ -21,7 +21,7 @@ This means the builder proceeds automatically from Specify → Plan → Implemen | Phases | Specify → Plan → Implement → Review | Same | | 3-way consultations | Yes, every phase | Same | | Checks (build, tests, PR) | Yes | Same | -| Prompts / templates | Full set | Same (copied from SPIR) | +| Prompts / templates | Full set | Same prompts; templates included from SPIR (no copies) | ### When to Use ASPIR @@ -83,16 +83,18 @@ codev/protocols/aspir/ │ ├── plan.md # Plan phase prompt (same as SPIR) │ ├── implement.md # Implement phase prompt (same as SPIR) │ └── review.md # Review phase prompt (same as SPIR) -├── consult-types/ -│ ├── spec-review.md # Spec consultation guide (same as SPIR) -│ ├── plan-review.md # Plan consultation guide (same as SPIR) -│ ├── impl-review.md # Impl consultation guide (same as SPIR) -│ ├── phase-review.md # Phase consultation guide (same as SPIR) -│ └── pr-review.md # PR consultation guide (same as SPIR) -└── templates/ - ├── spec.md # Spec template (same as SPIR) - ├── plan.md # Plan template (same as SPIR) - └── review.md # Review template (same as SPIR) +└── consult-types/ + ├── spec-review.md # Spec consultation guide (same as SPIR) + ├── plan-review.md # Plan consultation guide (same as SPIR) + ├── impl-review.md # Impl consultation guide (same as SPIR) + ├── phase-review.md # Phase consultation guide (same as SPIR) + └── pr-review.md # PR consultation guide (same as SPIR) ``` +ASPIR ships **no `templates/` directory**. Its phase prompts deliver SPIR's canonical +templates directly, via an include directive pointing at `protocols/spir/templates/`, so +there is exactly one copy of each template and it cannot drift between the two protocols. +(Written as a path, not as a literal include: an include directive in prose would be +expanded — and silently emptied — when this file is delivered to a builder.) + All files except `protocol.json` and `protocol.md` are identical to their SPIR counterparts. diff --git a/codev-skeleton/protocols/aspir/templates/plan.md b/codev-skeleton/protocols/aspir/templates/plan.md deleted file mode 100644 index 9da106498..000000000 --- a/codev-skeleton/protocols/aspir/templates/plan.md +++ /dev/null @@ -1,184 +0,0 @@ -# Plan: [Title] - -## Metadata -- **ID**: plan-[YYYY-MM-DD]-[short-name] -- **Status**: draft -- **Specification**: [Link to codev/specs/spec-file.md] -- **Created**: [YYYY-MM-DD] - -## Executive Summary -[Brief overview of the implementation approach chosen and why. Reference the specification's selected approach.] - -## Success Metrics -[Copy from specification and add implementation-specific metrics] -- [ ] All specification criteria met -- [ ] Test coverage >90% -- [ ] Performance benchmarks achieved -- [ ] Zero critical security issues -- [ ] Documentation complete - -## Phases (Machine Readable) - - - -```json -{ - "phases": [ - {"id": "phase_1", "title": "Phase 1 Title Here"}, - {"id": "phase_2", "title": "Phase 2 Title Here"}, - {"id": "phase_3", "title": "Phase 3 Title Here"} - ] -} -``` - -## Phase Breakdown - -### Phase 1: [Descriptive Name] -**Dependencies**: None - -#### Objectives -- [Clear, single objective for this phase] -- [What value does this phase deliver?] - -#### Deliverables -- [ ] [Specific deliverable 1] -- [ ] [Specific deliverable 2] -- [ ] [Tests for this phase] -- [ ] [Documentation updates] - -#### Implementation Details -[Specific technical approach for this phase. Include: -- Key files/modules to create or modify -- Architectural decisions -- API contracts -- Data models] - -#### Acceptance Criteria -- [ ] [Testable criterion 1] -- [ ] [Testable criterion 2] -- [ ] All tests pass -- [ ] Code review completed - -#### Test Plan -- **Unit Tests**: [What to test] -- **Integration Tests**: [What to test] -- **Manual Testing**: [Scenarios to verify] - -#### Rollback Strategy -[How to revert this phase if issues arise] - -#### Risks -- **Risk**: [Specific risk for this phase] - - **Mitigation**: [How to address] - ---- - -### Phase 2: [Descriptive Name] -**Dependencies**: Phase 1 - -[Repeat structure for each phase] - ---- - -### Phase 3: [Descriptive Name] -**Dependencies**: Phase 2 - -[Continue for all phases] - -## Dependency Map -``` -Phase 1 ──→ Phase 2 ──→ Phase 3 - ↓ - Phase 4 (optional) -``` - -## Resource Requirements -### Development Resources -- **Engineers**: [Expertise needed] -- **Environment**: [Dev/staging requirements] - -### Infrastructure -- [Database changes] -- [New services] -- [Configuration updates] -- [Monitoring additions] - -## Integration Points -### External Systems -- **System**: [Name] - - **Integration Type**: [API/Database/Message Queue] - - **Phase**: [Which phase needs this] - - **Fallback**: [What if unavailable] - -### Internal Systems -[Repeat structure] - -## Risk Analysis -### Technical Risks -| Risk | Probability | Impact | Mitigation | Owner | -|------|------------|--------|------------|-------| -| [Risk 1] | L/M/H | L/M/H | [Strategy] | [Name] | - -### Schedule Risks -| Risk | Probability | Impact | Mitigation | Owner | -|------|------------|--------|------------|-------| -| [Risk 1] | L/M/H | L/M/H | [Strategy] | [Name] | - -## Validation Checkpoints -1. **After Phase 1**: [What to validate] -2. **After Phase 2**: [What to validate] -3. **Before Production**: [Final checks] - -## Monitoring and Observability -### Metrics to Track -- [Metric 1: Description and threshold] -- [Metric 2: Description and threshold] - -### Logging Requirements -- [What to log and at what level] -- [Retention requirements] - -### Alerting -- [Alert condition and severity] -- [Who to notify] - -## Documentation Updates Required -- [ ] API documentation -- [ ] Architecture diagrams -- [ ] Runbooks -- [ ] User guides -- [ ] Configuration guides - -## Post-Implementation Tasks -- [ ] Performance validation -- [ ] Security audit -- [ ] Load testing -- [ ] User acceptance testing -- [ ] Monitoring validation - -## Expert Review -**Date**: [YYYY-MM-DD] -**Model**: [Model consulted] -**Key Feedback**: -- [Feasibility assessment] -- [Missing considerations] -- [Risk identification] -- [Alternative suggestions] - -**Plan Adjustments**: -- [How the plan was modified based on feedback] - -## Approval -- [ ] Technical Lead Review -- [ ] Engineering Manager Approval -- [ ] Resource Allocation Confirmed -- [ ] Expert AI Consultation Complete - -## Change Log -| Date | Change | Reason | Author | -|------|--------|--------|--------| -| [Date] | [What changed] | [Why] | [Who] | - -## Notes -[Additional context, assumptions, or considerations] - diff --git a/codev-skeleton/protocols/aspir/templates/review.md b/codev-skeleton/protocols/aspir/templates/review.md deleted file mode 100644 index af72f0e62..000000000 --- a/codev-skeleton/protocols/aspir/templates/review.md +++ /dev/null @@ -1,120 +0,0 @@ -# Review: [Feature/Project Name] - -## Summary - -[1-3 sentences: what was built, how many phases, net outcome.] - -## Spec Compliance - -- [x] AC1: [Description] (Phase N) -- [x] AC2: [Description] (Phase N) -- [ ] ACn: [Not met — reason] - -## Deviations from Plan - -- **Phase N**: [What changed and why] - -## Key Metrics - -- **Commits**: [N] on the branch -- **Tests**: [N] passing ([N] existing + [N] new) -- **Files created**: [list] -- **Files deleted**: [list] -- **Net LOC impact**: [+/-N lines] - -## Timelog - -All times [timezone], [date range]. - -| Time | Event | -|------|-------| -| HH:MM | First commit: [description] | -| HH:MM | [Phase/milestone] | -| — | **GATE: [gate-name]** (human approval required) | -| HH:MM | Implementation begins | -| HH:MM | Phase N complete after N iterations | -| HH:MM | **GATE: pr** | - -### Autonomous Operation - -| Period | Duration | Activity | -|--------|----------|----------| -| Spec + Plan | ~Nm | [Summary] | -| Human gate wait | ~Nh Nm | Idle — waiting for approval | -| Implementation → PR | ~Nh Nm | N phases, N consultation rounds | - -**Total wall clock** (first commit to pr): **Xh Ym** -**Total autonomous work time** (excluding gate waits): **~Xh Ym** -**Context window resets**: [N] (resumed automatically / required manual restart) - -## Consultation Iteration Summary - -[N] consultation files produced ([N] rounds x [N] models). [N] APPROVE, [N] REQUEST_CHANGES, [N] COMMENT. - -| Phase | Iters | Who Blocked | What They Caught | -|-------|-------|-------------|------------------| -| Specify | N | [Model] | [Brief description] | -| Plan | N | [Model] | [Brief description] | -| Phase 1 | N | [Model] | [Brief description] | -| Phase N | N | [Model] | [Brief description] | -| Review | N | [Model] | [Brief description] | - -**Most frequent blocker**: [Model] — blocked in N of N rounds, focused on: [pattern]. - -### Avoidable Iterations - -Iterations that could have been prevented with better builder behavior: - -1. **[Pattern]**: [Specific thing the builder should have done without needing reviewer feedback. E.g., "Run exhaustive grep before claiming all instances fixed."] - -2. **[Pattern]**: [Another avoidable iteration pattern.] - -## Consultation Feedback - -[For each phase that had consultation, summarize every reviewer's concerns and how the builder responded. Use **Addressed** (fixed), **Rebutted** (disagreed with reasoning), or **N/A** (out of scope/moot) for each concern. If all reviewers approved with no concerns: "No concerns raised — all consultations approved."] - -### [Phase] Phase (Round N) - -#### Gemini -- **Concern**: [Summary of concern] - - **Addressed**: [What was changed] - -#### Codex -- **Concern**: [Summary of concern] - - **Rebutted**: [Why current approach is correct] - -#### Claude -- No concerns raised (APPROVE) - -## Lessons Learned - -### What Went Well -- [Specific positive observation — what worked and why] - -### Challenges Encountered -- **[Challenge]**: [How it was resolved. How many iterations it cost.] - -### What Would Be Done Differently -- [Actionable improvement for future builders] - -## Architecture Updates - -[What you routed where — HOT `codev/resources/arch-critical.md` (tiny, capped, always-injected) vs COLD `codev/resources/arch.md` (reference) — or why no changes were needed.] - -- Routed: [hot | cold] — [fact/section] — [what was added/changed; note any demotion if the hot file was full] -- Or: "No architecture updates needed — [brief reason]" - -## Lessons Learned Updates - -[What you routed where — HOT `codev/resources/lessons-critical.md` (capped) vs COLD `codev/resources/lessons-learned.md` (reference) — or why no changes were needed.] - -- Routed: [hot | cold] — [category] — [lesson summary] -- Or: "No lessons learned updates needed — [brief reason]" - -## Technical Debt - -- [Any shortcuts taken or inconsistencies introduced] - -## Follow-up Items - -- [Items identified for future work, outside this spec's scope] diff --git a/codev-skeleton/protocols/aspir/templates/spec.md b/codev-skeleton/protocols/aspir/templates/spec.md deleted file mode 100644 index 4cca2177f..000000000 --- a/codev-skeleton/protocols/aspir/templates/spec.md +++ /dev/null @@ -1,154 +0,0 @@ -# Specification: [Title] - - - -## Metadata -- **ID**: spec-[YYYY-MM-DD]-[short-name] -- **Status**: draft -- **Created**: [YYYY-MM-DD] - -## Clarifying Questions Asked - -[List the questions you asked to understand the problem better and the responses received. This shows the discovery process.] - -## Problem Statement -[Clearly articulate the problem being solved. Include context about why this is important, who is affected, and what the current pain points are.] - -## Current State -[Describe how things work today. What are the limitations? What workarounds exist? Include specific examples.] - -## Desired State -[Describe the ideal solution. How should things work after implementation? What specific improvements will users see?] - -## Stakeholders -- **Primary Users**: [Who will directly use this feature?] -- **Secondary Users**: [Who else is affected?] -- **Technical Team**: [Who will implement and maintain this?] -- **Business Owners**: [Who has decision authority?] - -## Success Criteria -- [ ] [Specific, measurable criterion 1] -- [ ] [Specific, measurable criterion 2] -- [ ] [Specific, measurable criterion 3] -- [ ] All tests pass with >90% coverage -- [ ] Performance benchmarks met (specify below) -- [ ] Documentation updated - -## Constraints -### Technical Constraints -- [Existing system limitations] -- [Technology stack requirements] -- [Integration points] - -### Business Constraints -- [Timeline requirements] -- [Budget considerations] -- [Compliance requirements] - -## Assumptions -- [List assumptions being made] -- [Include dependencies on other work] -- [Note any prerequisites] - -## Solution Approaches - -### Approach 1: [Name] -**Description**: [Brief overview of this approach] - -**Pros**: -- [Advantage 1] -- [Advantage 2] - -**Cons**: -- [Disadvantage 1] -- [Disadvantage 2] - -**Estimated Complexity**: [Low/Medium/High] -**Risk Level**: [Low/Medium/High] - -### Approach 2: [Name] -[Repeat structure for additional approaches] - -[Add as many approaches as appropriate for the problem] - -## Open Questions - -### Critical (Blocks Progress) -- [ ] [Question that must be answered before proceeding] - -### Important (Affects Design) -- [ ] [Question that influences technical decisions] - -### Nice-to-Know (Optimization) -- [ ] [Question that could improve the solution] - -## Performance Requirements -- **Response Time**: [e.g., <200ms p95] -- **Throughput**: [e.g., 1000 requests/second] -- **Resource Usage**: [e.g., <500MB memory] -- **Availability**: [e.g., 99.9% uptime] - -## Security Considerations -- [Authentication requirements] -- [Authorization model] -- [Data privacy concerns] -- [Audit requirements] - -## Test Scenarios -### Functional Tests -1. [Scenario 1: Happy path] -2. [Scenario 2: Edge case] -3. [Scenario 3: Error condition] - -### Non-Functional Tests -1. [Performance test scenario] -2. [Security test scenario] -3. [Load test scenario] - -## Dependencies -- **External Services**: [List any external APIs or services] -- **Internal Systems**: [List internal dependencies] -- **Libraries/Frameworks**: [List required libraries] - -## References -- [Link to relevant documentation in codev/ref/] -- [Link to related specifications] -- [Link to architectural diagrams] -- [Link to research materials] - -## Risks and Mitigation -| Risk | Probability | Impact | Mitigation Strategy | -|------|------------|--------|-------------------| -| [Risk 1] | Low/Med/High | Low/Med/High | [How to address] | -| [Risk 2] | Low/Med/High | Low/Med/High | [How to address] | - -## Expert Consultation - -**Date**: [YYYY-MM-DD] -**Models Consulted**: [e.g., GPT-5 and Gemini Pro] -**Sections Updated**: -- [Section name]: [Brief description of change based on consultation] -- [Section name]: [Brief description of change based on consultation] - -Note: All consultation feedback has been incorporated directly into the relevant sections above. - -## Approval -- [ ] Technical Lead Review -- [ ] Product Owner Review -- [ ] Stakeholder Sign-off -- [ ] Expert AI Consultation Complete - -## Notes -[Any additional context or considerations not covered above] - diff --git a/codev-skeleton/protocols/maintain/protocol.md b/codev-skeleton/protocols/maintain/protocol.md index 7e91d118d..08199b956 100644 --- a/codev-skeleton/protocols/maintain/protocol.md +++ b/codev-skeleton/protocols/maintain/protocol.md @@ -185,52 +185,9 @@ Both must pass before moving to the review phase. ## Maintenance Run File -Each run creates `codev/maintain/NNNN.md`: +Each run creates `codev/maintain/NNNN.md`, following the template below: -```markdown -# Maintenance Run NNNN - -**Date**: YYYY-MM-DD -**Base Commit**: -**PR**: #NNN - -## Changes Since Last Run - - - -## Audit Findings - -Recorded by Step 3a (Audit documentation) as the cuts are applied — one line per cut, with its reason. The diff plus these reasons is the proposal; the architect's PR review is the gate. - -### arch.md (cold) / arch-critical.md (hot) --
: - -### lessons-learned.md (cold) / lessons-critical.md (hot) -- : - -## What Was Done - -### Dead Code Removed -- `path/to/file.ts`: `unusedFunction()` — not imported anywhere -- Removed `some-package` dependency — zero imports - -### Documentation Updated -- arch.md / arch-critical.md: Added VS Code extension section + removed old dashboard-server refs (cold); routed 1 invariant to the hot tier, demoted 1 stale entry to cold -- lessons-learned.md / lessons-critical.md: Extracted 3 lessons from reviews 653, 672 (cold); promoted 1 behavior-changer to the hot tier, refreshed its cold-doc map - -### Documentation Changes Log -| Document | Section | Action | Reason | -|----------|---------|--------|--------| -| arch.md | "Dashboard Server" | DELETED | OBSOLETE — replaced by Tower | - -## Deferred - -- Items found but not worth fixing now - -## Summary - -<2-3 sentences> -``` +{{> protocols/maintain/templates/maintenance-run.md}} Keep it factual and short. The run file documents what happened, not what might happen. diff --git a/codev-skeleton/protocols/maintain/templates/maintenance-run.md b/codev-skeleton/protocols/maintain/templates/maintenance-run.md index 189c478be..a55110215 100644 --- a/codev-skeleton/protocols/maintain/templates/maintenance-run.md +++ b/codev-skeleton/protocols/maintain/templates/maintenance-run.md @@ -8,6 +8,16 @@ +## Audit Findings + +Recorded by Step 3a (Audit documentation) as the cuts are applied — one line per cut, with its reason. The diff plus these reasons is the proposal; the architect's PR review is the gate. + +### arch.md (cold) / arch-critical.md (hot) +-
: + +### lessons-learned.md (cold) / lessons-critical.md (hot) +- : + ## What Was Done ### Dead Code Removed @@ -23,6 +33,7 @@ ### Documentation Changes Log | Document | Section | Action | Reason | |----------|---------|--------|--------| +| arch.md | "Dashboard Server" | DELETED | OBSOLETE — replaced by Tower | ## Deferred diff --git a/codev-skeleton/protocols/spir/consult-types/spec-review.md b/codev-skeleton/protocols/spir/consult-types/spec-review.md index b537e7cc6..73e346e00 100644 --- a/codev-skeleton/protocols/spir/consult-types/spec-review.md +++ b/codev-skeleton/protocols/spir/consult-types/spec-review.md @@ -32,6 +32,12 @@ If the baked decisions themselves contain contradictions (e.g., two different la - Are acceptance criteria testable? - Is terminology consistent? +5. **Structure** + - The specify prompt delivers a canonical spec template (`protocols/spir/templates/spec.md`) inline. Does the spec actually follow it? + - Required headings, in order: `## Metadata`, `## Clarifying Questions Asked`, `## Problem Statement`, `## Current State`, `## Desired State`, `## Stakeholders`, `## Success Criteria`, `## Constraints`, `## Assumptions`, `## Solution Approaches`, `## Open Questions`, `## Performance Requirements`, `## Security Considerations`, `## Test Scenarios`, `## Dependencies`, `## References`, `## Risks and Mitigation`, `## Expert Consultation`, `## Approval`, `## Notes`. + - A free-form spec that reads well but ignores the template is a **defect**, not a style preference — it usually means the builder pattern-matched an older spec in `codev/specs/` instead of the delivered template. `REQUEST_CHANGES` for a wholesale departure (most headings missing or renamed). + - A section that genuinely does not apply may be reduced to a one-line "N/A — [reason]", but the heading should remain. Do not `REQUEST_CHANGES` over one such section. + ## Verdict Format After your review, provide your verdict in exactly this format: diff --git a/codev-skeleton/protocols/spir/prompts/review.md b/codev-skeleton/protocols/spir/prompts/review.md index 22af9aec4..eabe1b98e 100644 --- a/codev-skeleton/protocols/spir/prompts/review.md +++ b/codev-skeleton/protocols/spir/prompts/review.md @@ -57,55 +57,9 @@ Compare final implementation to original specification: ### 3. Create Review Document -Create `codev/reviews/{{artifact_name}}.md`: +Create `codev/reviews/{{artifact_name}}.md`, following the template below. Use these headings and this order — do not invent your own structure, and do not pattern-match an earlier review in `codev/reviews/` that predates this template. Steps 3b and 4 below expand on the `## Consultation Feedback`, `## Architecture Updates`, and `## Lessons Learned Updates` sections; porch's review checks grep for the last two by exact heading. -```markdown -# Review: {{title}} - -## Summary -Brief description of what was implemented. - -## Spec Compliance -- [x] Requirement 1: Implemented as specified -- [x] Requirement 2: Implemented with deviation (see below) -- [x] Requirement 3: Implemented as specified - -## Deviations from Plan -- **Phase X**: [What changed and why] - -## Lessons Learned - -### What Went Well -- [Positive observation 1] -- [Positive observation 2] - -### Challenges Encountered -- [Challenge 1]: [How it was resolved] -- [Challenge 2]: [How it was resolved] - -### What Would Be Done Differently -- [Insight 1] -- [Insight 2] - -### Methodology Improvements -- [Suggested improvement to SPIR protocol] -- [Suggested improvement to tooling] - -## Technical Debt -- [Any shortcuts taken that should be addressed later] - -## Consultation Feedback - -[See instructions below] - -## Flaky Tests -- [Any pre-existing tests that were skipped as flaky during this project] -- [Include test name, file path, and observed failure mode] -- [If none: "No flaky tests encountered"] - -## Follow-up Items -- [Any items identified for future work] -``` +{{> protocols/spir/templates/review.md}} ### 3b. Include Consultation Feedback diff --git a/codev-skeleton/protocols/spir/prompts/specify.md b/codev-skeleton/protocols/spir/prompts/specify.md index 3716f7969..daa5feab2 100644 --- a/codev-skeleton/protocols/spir/prompts/specify.md +++ b/codev-skeleton/protocols/spir/prompts/specify.md @@ -90,6 +90,10 @@ After completing the spec draft, signal completion. Porch will run 3-way consult Create or update the specification file at `codev/specs/{{artifact_name}}.md`. +Follow the canonical spec template reproduced below. Use these headings, in this order — do not invent your own structure, and do not pattern-match an earlier spec in `codev/specs/` that predates this template. If a section genuinely does not apply, keep the heading and write a one-line "N/A — [reason]" rather than deleting it. + +{{> protocols/spir/templates/spec.md}} + **IMPORTANT**: Keep spec/plan/review filenames in sync: - Spec: `codev/specs/{{artifact_name}}.md` - Plan: `codev/plans/{{artifact_name}}.md` diff --git a/codev-skeleton/protocols/spir/protocol.json b/codev-skeleton/protocols/spir/protocol.json index 0041fb5e9..f6b74c5d3 100644 --- a/codev-skeleton/protocols/spir/protocol.json +++ b/codev-skeleton/protocols/spir/protocol.json @@ -24,6 +24,16 @@ "models": ["gemini", "codex", "claude"], "parallel": true }, + "checks": { + "spec_exists": { + "command": "test -f codev/specs/${PROJECT_TITLE}.md", + "description": "Spec file must exist before the spec-approval gate" + }, + "spec_has_required_sections": { + "command": "grep -qF '## Problem Statement' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Current State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Desired State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Success Criteria' codev/specs/${PROJECT_TITLE}.md", + "description": "Spec must follow the canonical template delivered by the specify prompt (protocols/spir/templates/spec.md). Required headings: ## Problem Statement, ## Current State, ## Desired State, ## Success Criteria" + } + }, "max_iterations": 3, "on_complete": { "commit": true, diff --git a/codev-skeleton/protocols/spir/templates/review.md b/codev-skeleton/protocols/spir/templates/review.md index af72f0e62..668055637 100644 --- a/codev-skeleton/protocols/spir/templates/review.md +++ b/codev-skeleton/protocols/spir/templates/review.md @@ -97,6 +97,10 @@ Iterations that could have been prevented with better builder behavior: ### What Would Be Done Differently - [Actionable improvement for future builders] +### Methodology Improvements +- [Suggested improvement to the SPIR protocol] +- [Suggested improvement to tooling] + ## Architecture Updates [What you routed where — HOT `codev/resources/arch-critical.md` (tiny, capped, always-injected) vs COLD `codev/resources/arch.md` (reference) — or why no changes were needed.] @@ -115,6 +119,11 @@ Iterations that could have been prevented with better builder behavior: - [Any shortcuts taken or inconsistencies introduced] +## Flaky Tests + +- [Pre-existing tests skipped as flaky during this project — test name, file path, observed failure mode] +- [If none: "No flaky tests encountered"] + ## Follow-up Items - [Items identified for future work, outside this spec's scope] diff --git a/codev/projects/bugfix-1279-spir-spec-template-is-dead-cod/status.yaml b/codev/projects/bugfix-1279-spir-spec-template-is-dead-cod/status.yaml new file mode 100644 index 000000000..2ac62d320 --- /dev/null +++ b/codev/projects/bugfix-1279-spir-spec-template-is-dead-cod/status.yaml @@ -0,0 +1,17 @@ +id: bugfix-1279 +title: spir-spec-template-is-dead-cod +protocol: bugfix +phase: pr +plan_phases: [] +current_plan_phase: null +gates: + pr: + status: approved + requested_at: '2026-07-28T18:08:01.883Z' + approved_at: '2026-07-29T02:22:18.544Z' +iteration: 1 +build_complete: false +history: [] +started_at: '2026-07-28T17:44:57.015Z' +updated_at: '2026-07-29T02:22:18.545Z' +pr_ready_for_human: false diff --git a/codev/protocols/aspir/consult-types/spec-review.md b/codev/protocols/aspir/consult-types/spec-review.md index b537e7cc6..73e346e00 100644 --- a/codev/protocols/aspir/consult-types/spec-review.md +++ b/codev/protocols/aspir/consult-types/spec-review.md @@ -32,6 +32,12 @@ If the baked decisions themselves contain contradictions (e.g., two different la - Are acceptance criteria testable? - Is terminology consistent? +5. **Structure** + - The specify prompt delivers a canonical spec template (`protocols/spir/templates/spec.md`) inline. Does the spec actually follow it? + - Required headings, in order: `## Metadata`, `## Clarifying Questions Asked`, `## Problem Statement`, `## Current State`, `## Desired State`, `## Stakeholders`, `## Success Criteria`, `## Constraints`, `## Assumptions`, `## Solution Approaches`, `## Open Questions`, `## Performance Requirements`, `## Security Considerations`, `## Test Scenarios`, `## Dependencies`, `## References`, `## Risks and Mitigation`, `## Expert Consultation`, `## Approval`, `## Notes`. + - A free-form spec that reads well but ignores the template is a **defect**, not a style preference — it usually means the builder pattern-matched an older spec in `codev/specs/` instead of the delivered template. `REQUEST_CHANGES` for a wholesale departure (most headings missing or renamed). + - A section that genuinely does not apply may be reduced to a one-line "N/A — [reason]", but the heading should remain. Do not `REQUEST_CHANGES` over one such section. + ## Verdict Format After your review, provide your verdict in exactly this format: diff --git a/codev/protocols/aspir/prompts/review.md b/codev/protocols/aspir/prompts/review.md index 22af9aec4..eabe1b98e 100644 --- a/codev/protocols/aspir/prompts/review.md +++ b/codev/protocols/aspir/prompts/review.md @@ -57,55 +57,9 @@ Compare final implementation to original specification: ### 3. Create Review Document -Create `codev/reviews/{{artifact_name}}.md`: +Create `codev/reviews/{{artifact_name}}.md`, following the template below. Use these headings and this order — do not invent your own structure, and do not pattern-match an earlier review in `codev/reviews/` that predates this template. Steps 3b and 4 below expand on the `## Consultation Feedback`, `## Architecture Updates`, and `## Lessons Learned Updates` sections; porch's review checks grep for the last two by exact heading. -```markdown -# Review: {{title}} - -## Summary -Brief description of what was implemented. - -## Spec Compliance -- [x] Requirement 1: Implemented as specified -- [x] Requirement 2: Implemented with deviation (see below) -- [x] Requirement 3: Implemented as specified - -## Deviations from Plan -- **Phase X**: [What changed and why] - -## Lessons Learned - -### What Went Well -- [Positive observation 1] -- [Positive observation 2] - -### Challenges Encountered -- [Challenge 1]: [How it was resolved] -- [Challenge 2]: [How it was resolved] - -### What Would Be Done Differently -- [Insight 1] -- [Insight 2] - -### Methodology Improvements -- [Suggested improvement to SPIR protocol] -- [Suggested improvement to tooling] - -## Technical Debt -- [Any shortcuts taken that should be addressed later] - -## Consultation Feedback - -[See instructions below] - -## Flaky Tests -- [Any pre-existing tests that were skipped as flaky during this project] -- [Include test name, file path, and observed failure mode] -- [If none: "No flaky tests encountered"] - -## Follow-up Items -- [Any items identified for future work] -``` +{{> protocols/spir/templates/review.md}} ### 3b. Include Consultation Feedback diff --git a/codev/protocols/aspir/prompts/specify.md b/codev/protocols/aspir/prompts/specify.md index 3716f7969..daa5feab2 100644 --- a/codev/protocols/aspir/prompts/specify.md +++ b/codev/protocols/aspir/prompts/specify.md @@ -90,6 +90,10 @@ After completing the spec draft, signal completion. Porch will run 3-way consult Create or update the specification file at `codev/specs/{{artifact_name}}.md`. +Follow the canonical spec template reproduced below. Use these headings, in this order — do not invent your own structure, and do not pattern-match an earlier spec in `codev/specs/` that predates this template. If a section genuinely does not apply, keep the heading and write a one-line "N/A — [reason]" rather than deleting it. + +{{> protocols/spir/templates/spec.md}} + **IMPORTANT**: Keep spec/plan/review filenames in sync: - Spec: `codev/specs/{{artifact_name}}.md` - Plan: `codev/plans/{{artifact_name}}.md` diff --git a/codev/protocols/aspir/protocol.json b/codev/protocols/aspir/protocol.json index fb5c4f993..eea40f1af 100644 --- a/codev/protocols/aspir/protocol.json +++ b/codev/protocols/aspir/protocol.json @@ -23,6 +23,16 @@ "models": ["gemini", "codex", "claude"], "parallel": true }, + "checks": { + "spec_exists": { + "command": "test -f codev/specs/${PROJECT_TITLE}.md", + "description": "Spec file must exist before the spec-approval gate" + }, + "spec_has_required_sections": { + "command": "grep -qF '## Problem Statement' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Current State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Desired State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Success Criteria' codev/specs/${PROJECT_TITLE}.md", + "description": "Spec must follow the canonical template delivered by the specify prompt (protocols/spir/templates/spec.md). Required headings: ## Problem Statement, ## Current State, ## Desired State, ## Success Criteria" + } + }, "max_iterations": 3, "on_complete": { "commit": true, diff --git a/codev/protocols/aspir/protocol.md b/codev/protocols/aspir/protocol.md index 18a423a30..6cc2caf97 100644 --- a/codev/protocols/aspir/protocol.md +++ b/codev/protocols/aspir/protocol.md @@ -21,7 +21,7 @@ This means the builder proceeds automatically from Specify → Plan → Implemen | Phases | Specify → Plan → Implement → Review | Same | | 3-way consultations | Yes, every phase | Same | | Checks (build, tests, PR) | Yes | Same | -| Prompts / templates | Full set | Same (copied from SPIR) | +| Prompts / templates | Full set | Same prompts; templates included from SPIR (no copies) | ### When to Use ASPIR @@ -83,16 +83,18 @@ codev/protocols/aspir/ │ ├── plan.md # Plan phase prompt (same as SPIR) │ ├── implement.md # Implement phase prompt (same as SPIR) │ └── review.md # Review phase prompt (same as SPIR) -├── consult-types/ -│ ├── spec-review.md # Spec consultation guide (same as SPIR) -│ ├── plan-review.md # Plan consultation guide (same as SPIR) -│ ├── impl-review.md # Impl consultation guide (same as SPIR) -│ ├── phase-review.md # Phase consultation guide (same as SPIR) -│ └── pr-review.md # PR consultation guide (same as SPIR) -└── templates/ - ├── spec.md # Spec template (same as SPIR) - ├── plan.md # Plan template (same as SPIR) - └── review.md # Review template (same as SPIR) +└── consult-types/ + ├── spec-review.md # Spec consultation guide (same as SPIR) + ├── plan-review.md # Plan consultation guide (same as SPIR) + ├── impl-review.md # Impl consultation guide (same as SPIR) + ├── phase-review.md # Phase consultation guide (same as SPIR) + └── pr-review.md # PR consultation guide (same as SPIR) ``` +ASPIR ships **no `templates/` directory**. Its phase prompts deliver SPIR's canonical +templates directly, via an include directive pointing at `protocols/spir/templates/`, so +there is exactly one copy of each template and it cannot drift between the two protocols. +(Written as a path, not as a literal include: an include directive in prose would be +expanded — and silently emptied — when this file is delivered to a builder.) + All files except `protocol.json` and `protocol.md` are identical to their SPIR counterparts. diff --git a/codev/protocols/aspir/templates/plan.md b/codev/protocols/aspir/templates/plan.md deleted file mode 100644 index 9da106498..000000000 --- a/codev/protocols/aspir/templates/plan.md +++ /dev/null @@ -1,184 +0,0 @@ -# Plan: [Title] - -## Metadata -- **ID**: plan-[YYYY-MM-DD]-[short-name] -- **Status**: draft -- **Specification**: [Link to codev/specs/spec-file.md] -- **Created**: [YYYY-MM-DD] - -## Executive Summary -[Brief overview of the implementation approach chosen and why. Reference the specification's selected approach.] - -## Success Metrics -[Copy from specification and add implementation-specific metrics] -- [ ] All specification criteria met -- [ ] Test coverage >90% -- [ ] Performance benchmarks achieved -- [ ] Zero critical security issues -- [ ] Documentation complete - -## Phases (Machine Readable) - - - -```json -{ - "phases": [ - {"id": "phase_1", "title": "Phase 1 Title Here"}, - {"id": "phase_2", "title": "Phase 2 Title Here"}, - {"id": "phase_3", "title": "Phase 3 Title Here"} - ] -} -``` - -## Phase Breakdown - -### Phase 1: [Descriptive Name] -**Dependencies**: None - -#### Objectives -- [Clear, single objective for this phase] -- [What value does this phase deliver?] - -#### Deliverables -- [ ] [Specific deliverable 1] -- [ ] [Specific deliverable 2] -- [ ] [Tests for this phase] -- [ ] [Documentation updates] - -#### Implementation Details -[Specific technical approach for this phase. Include: -- Key files/modules to create or modify -- Architectural decisions -- API contracts -- Data models] - -#### Acceptance Criteria -- [ ] [Testable criterion 1] -- [ ] [Testable criterion 2] -- [ ] All tests pass -- [ ] Code review completed - -#### Test Plan -- **Unit Tests**: [What to test] -- **Integration Tests**: [What to test] -- **Manual Testing**: [Scenarios to verify] - -#### Rollback Strategy -[How to revert this phase if issues arise] - -#### Risks -- **Risk**: [Specific risk for this phase] - - **Mitigation**: [How to address] - ---- - -### Phase 2: [Descriptive Name] -**Dependencies**: Phase 1 - -[Repeat structure for each phase] - ---- - -### Phase 3: [Descriptive Name] -**Dependencies**: Phase 2 - -[Continue for all phases] - -## Dependency Map -``` -Phase 1 ──→ Phase 2 ──→ Phase 3 - ↓ - Phase 4 (optional) -``` - -## Resource Requirements -### Development Resources -- **Engineers**: [Expertise needed] -- **Environment**: [Dev/staging requirements] - -### Infrastructure -- [Database changes] -- [New services] -- [Configuration updates] -- [Monitoring additions] - -## Integration Points -### External Systems -- **System**: [Name] - - **Integration Type**: [API/Database/Message Queue] - - **Phase**: [Which phase needs this] - - **Fallback**: [What if unavailable] - -### Internal Systems -[Repeat structure] - -## Risk Analysis -### Technical Risks -| Risk | Probability | Impact | Mitigation | Owner | -|------|------------|--------|------------|-------| -| [Risk 1] | L/M/H | L/M/H | [Strategy] | [Name] | - -### Schedule Risks -| Risk | Probability | Impact | Mitigation | Owner | -|------|------------|--------|------------|-------| -| [Risk 1] | L/M/H | L/M/H | [Strategy] | [Name] | - -## Validation Checkpoints -1. **After Phase 1**: [What to validate] -2. **After Phase 2**: [What to validate] -3. **Before Production**: [Final checks] - -## Monitoring and Observability -### Metrics to Track -- [Metric 1: Description and threshold] -- [Metric 2: Description and threshold] - -### Logging Requirements -- [What to log and at what level] -- [Retention requirements] - -### Alerting -- [Alert condition and severity] -- [Who to notify] - -## Documentation Updates Required -- [ ] API documentation -- [ ] Architecture diagrams -- [ ] Runbooks -- [ ] User guides -- [ ] Configuration guides - -## Post-Implementation Tasks -- [ ] Performance validation -- [ ] Security audit -- [ ] Load testing -- [ ] User acceptance testing -- [ ] Monitoring validation - -## Expert Review -**Date**: [YYYY-MM-DD] -**Model**: [Model consulted] -**Key Feedback**: -- [Feasibility assessment] -- [Missing considerations] -- [Risk identification] -- [Alternative suggestions] - -**Plan Adjustments**: -- [How the plan was modified based on feedback] - -## Approval -- [ ] Technical Lead Review -- [ ] Engineering Manager Approval -- [ ] Resource Allocation Confirmed -- [ ] Expert AI Consultation Complete - -## Change Log -| Date | Change | Reason | Author | -|------|--------|--------|--------| -| [Date] | [What changed] | [Why] | [Who] | - -## Notes -[Additional context, assumptions, or considerations] - diff --git a/codev/protocols/aspir/templates/review.md b/codev/protocols/aspir/templates/review.md deleted file mode 100644 index af72f0e62..000000000 --- a/codev/protocols/aspir/templates/review.md +++ /dev/null @@ -1,120 +0,0 @@ -# Review: [Feature/Project Name] - -## Summary - -[1-3 sentences: what was built, how many phases, net outcome.] - -## Spec Compliance - -- [x] AC1: [Description] (Phase N) -- [x] AC2: [Description] (Phase N) -- [ ] ACn: [Not met — reason] - -## Deviations from Plan - -- **Phase N**: [What changed and why] - -## Key Metrics - -- **Commits**: [N] on the branch -- **Tests**: [N] passing ([N] existing + [N] new) -- **Files created**: [list] -- **Files deleted**: [list] -- **Net LOC impact**: [+/-N lines] - -## Timelog - -All times [timezone], [date range]. - -| Time | Event | -|------|-------| -| HH:MM | First commit: [description] | -| HH:MM | [Phase/milestone] | -| — | **GATE: [gate-name]** (human approval required) | -| HH:MM | Implementation begins | -| HH:MM | Phase N complete after N iterations | -| HH:MM | **GATE: pr** | - -### Autonomous Operation - -| Period | Duration | Activity | -|--------|----------|----------| -| Spec + Plan | ~Nm | [Summary] | -| Human gate wait | ~Nh Nm | Idle — waiting for approval | -| Implementation → PR | ~Nh Nm | N phases, N consultation rounds | - -**Total wall clock** (first commit to pr): **Xh Ym** -**Total autonomous work time** (excluding gate waits): **~Xh Ym** -**Context window resets**: [N] (resumed automatically / required manual restart) - -## Consultation Iteration Summary - -[N] consultation files produced ([N] rounds x [N] models). [N] APPROVE, [N] REQUEST_CHANGES, [N] COMMENT. - -| Phase | Iters | Who Blocked | What They Caught | -|-------|-------|-------------|------------------| -| Specify | N | [Model] | [Brief description] | -| Plan | N | [Model] | [Brief description] | -| Phase 1 | N | [Model] | [Brief description] | -| Phase N | N | [Model] | [Brief description] | -| Review | N | [Model] | [Brief description] | - -**Most frequent blocker**: [Model] — blocked in N of N rounds, focused on: [pattern]. - -### Avoidable Iterations - -Iterations that could have been prevented with better builder behavior: - -1. **[Pattern]**: [Specific thing the builder should have done without needing reviewer feedback. E.g., "Run exhaustive grep before claiming all instances fixed."] - -2. **[Pattern]**: [Another avoidable iteration pattern.] - -## Consultation Feedback - -[For each phase that had consultation, summarize every reviewer's concerns and how the builder responded. Use **Addressed** (fixed), **Rebutted** (disagreed with reasoning), or **N/A** (out of scope/moot) for each concern. If all reviewers approved with no concerns: "No concerns raised — all consultations approved."] - -### [Phase] Phase (Round N) - -#### Gemini -- **Concern**: [Summary of concern] - - **Addressed**: [What was changed] - -#### Codex -- **Concern**: [Summary of concern] - - **Rebutted**: [Why current approach is correct] - -#### Claude -- No concerns raised (APPROVE) - -## Lessons Learned - -### What Went Well -- [Specific positive observation — what worked and why] - -### Challenges Encountered -- **[Challenge]**: [How it was resolved. How many iterations it cost.] - -### What Would Be Done Differently -- [Actionable improvement for future builders] - -## Architecture Updates - -[What you routed where — HOT `codev/resources/arch-critical.md` (tiny, capped, always-injected) vs COLD `codev/resources/arch.md` (reference) — or why no changes were needed.] - -- Routed: [hot | cold] — [fact/section] — [what was added/changed; note any demotion if the hot file was full] -- Or: "No architecture updates needed — [brief reason]" - -## Lessons Learned Updates - -[What you routed where — HOT `codev/resources/lessons-critical.md` (capped) vs COLD `codev/resources/lessons-learned.md` (reference) — or why no changes were needed.] - -- Routed: [hot | cold] — [category] — [lesson summary] -- Or: "No lessons learned updates needed — [brief reason]" - -## Technical Debt - -- [Any shortcuts taken or inconsistencies introduced] - -## Follow-up Items - -- [Items identified for future work, outside this spec's scope] diff --git a/codev/protocols/aspir/templates/spec.md b/codev/protocols/aspir/templates/spec.md deleted file mode 100644 index 4cca2177f..000000000 --- a/codev/protocols/aspir/templates/spec.md +++ /dev/null @@ -1,154 +0,0 @@ -# Specification: [Title] - - - -## Metadata -- **ID**: spec-[YYYY-MM-DD]-[short-name] -- **Status**: draft -- **Created**: [YYYY-MM-DD] - -## Clarifying Questions Asked - -[List the questions you asked to understand the problem better and the responses received. This shows the discovery process.] - -## Problem Statement -[Clearly articulate the problem being solved. Include context about why this is important, who is affected, and what the current pain points are.] - -## Current State -[Describe how things work today. What are the limitations? What workarounds exist? Include specific examples.] - -## Desired State -[Describe the ideal solution. How should things work after implementation? What specific improvements will users see?] - -## Stakeholders -- **Primary Users**: [Who will directly use this feature?] -- **Secondary Users**: [Who else is affected?] -- **Technical Team**: [Who will implement and maintain this?] -- **Business Owners**: [Who has decision authority?] - -## Success Criteria -- [ ] [Specific, measurable criterion 1] -- [ ] [Specific, measurable criterion 2] -- [ ] [Specific, measurable criterion 3] -- [ ] All tests pass with >90% coverage -- [ ] Performance benchmarks met (specify below) -- [ ] Documentation updated - -## Constraints -### Technical Constraints -- [Existing system limitations] -- [Technology stack requirements] -- [Integration points] - -### Business Constraints -- [Timeline requirements] -- [Budget considerations] -- [Compliance requirements] - -## Assumptions -- [List assumptions being made] -- [Include dependencies on other work] -- [Note any prerequisites] - -## Solution Approaches - -### Approach 1: [Name] -**Description**: [Brief overview of this approach] - -**Pros**: -- [Advantage 1] -- [Advantage 2] - -**Cons**: -- [Disadvantage 1] -- [Disadvantage 2] - -**Estimated Complexity**: [Low/Medium/High] -**Risk Level**: [Low/Medium/High] - -### Approach 2: [Name] -[Repeat structure for additional approaches] - -[Add as many approaches as appropriate for the problem] - -## Open Questions - -### Critical (Blocks Progress) -- [ ] [Question that must be answered before proceeding] - -### Important (Affects Design) -- [ ] [Question that influences technical decisions] - -### Nice-to-Know (Optimization) -- [ ] [Question that could improve the solution] - -## Performance Requirements -- **Response Time**: [e.g., <200ms p95] -- **Throughput**: [e.g., 1000 requests/second] -- **Resource Usage**: [e.g., <500MB memory] -- **Availability**: [e.g., 99.9% uptime] - -## Security Considerations -- [Authentication requirements] -- [Authorization model] -- [Data privacy concerns] -- [Audit requirements] - -## Test Scenarios -### Functional Tests -1. [Scenario 1: Happy path] -2. [Scenario 2: Edge case] -3. [Scenario 3: Error condition] - -### Non-Functional Tests -1. [Performance test scenario] -2. [Security test scenario] -3. [Load test scenario] - -## Dependencies -- **External Services**: [List any external APIs or services] -- **Internal Systems**: [List internal dependencies] -- **Libraries/Frameworks**: [List required libraries] - -## References -- [Link to relevant documentation in codev/ref/] -- [Link to related specifications] -- [Link to architectural diagrams] -- [Link to research materials] - -## Risks and Mitigation -| Risk | Probability | Impact | Mitigation Strategy | -|------|------------|--------|-------------------| -| [Risk 1] | Low/Med/High | Low/Med/High | [How to address] | -| [Risk 2] | Low/Med/High | Low/Med/High | [How to address] | - -## Expert Consultation - -**Date**: [YYYY-MM-DD] -**Models Consulted**: [e.g., GPT-5 and Gemini Pro] -**Sections Updated**: -- [Section name]: [Brief description of change based on consultation] -- [Section name]: [Brief description of change based on consultation] - -Note: All consultation feedback has been incorporated directly into the relevant sections above. - -## Approval -- [ ] Technical Lead Review -- [ ] Product Owner Review -- [ ] Stakeholder Sign-off -- [ ] Expert AI Consultation Complete - -## Notes -[Any additional context or considerations not covered above] - diff --git a/codev/protocols/maintain/protocol.md b/codev/protocols/maintain/protocol.md index 7e91d118d..08199b956 100644 --- a/codev/protocols/maintain/protocol.md +++ b/codev/protocols/maintain/protocol.md @@ -185,52 +185,9 @@ Both must pass before moving to the review phase. ## Maintenance Run File -Each run creates `codev/maintain/NNNN.md`: +Each run creates `codev/maintain/NNNN.md`, following the template below: -```markdown -# Maintenance Run NNNN - -**Date**: YYYY-MM-DD -**Base Commit**: -**PR**: #NNN - -## Changes Since Last Run - - - -## Audit Findings - -Recorded by Step 3a (Audit documentation) as the cuts are applied — one line per cut, with its reason. The diff plus these reasons is the proposal; the architect's PR review is the gate. - -### arch.md (cold) / arch-critical.md (hot) --
: - -### lessons-learned.md (cold) / lessons-critical.md (hot) -- : - -## What Was Done - -### Dead Code Removed -- `path/to/file.ts`: `unusedFunction()` — not imported anywhere -- Removed `some-package` dependency — zero imports - -### Documentation Updated -- arch.md / arch-critical.md: Added VS Code extension section + removed old dashboard-server refs (cold); routed 1 invariant to the hot tier, demoted 1 stale entry to cold -- lessons-learned.md / lessons-critical.md: Extracted 3 lessons from reviews 653, 672 (cold); promoted 1 behavior-changer to the hot tier, refreshed its cold-doc map - -### Documentation Changes Log -| Document | Section | Action | Reason | -|----------|---------|--------|--------| -| arch.md | "Dashboard Server" | DELETED | OBSOLETE — replaced by Tower | - -## Deferred - -- Items found but not worth fixing now - -## Summary - -<2-3 sentences> -``` +{{> protocols/maintain/templates/maintenance-run.md}} Keep it factual and short. The run file documents what happened, not what might happen. diff --git a/codev/protocols/maintain/templates/maintenance-run.md b/codev/protocols/maintain/templates/maintenance-run.md index 189c478be..a55110215 100644 --- a/codev/protocols/maintain/templates/maintenance-run.md +++ b/codev/protocols/maintain/templates/maintenance-run.md @@ -8,6 +8,16 @@ +## Audit Findings + +Recorded by Step 3a (Audit documentation) as the cuts are applied — one line per cut, with its reason. The diff plus these reasons is the proposal; the architect's PR review is the gate. + +### arch.md (cold) / arch-critical.md (hot) +-
: + +### lessons-learned.md (cold) / lessons-critical.md (hot) +- : + ## What Was Done ### Dead Code Removed @@ -23,6 +33,7 @@ ### Documentation Changes Log | Document | Section | Action | Reason | |----------|---------|--------|--------| +| arch.md | "Dashboard Server" | DELETED | OBSOLETE — replaced by Tower | ## Deferred diff --git a/codev/protocols/spir/consult-types/spec-review.md b/codev/protocols/spir/consult-types/spec-review.md index b537e7cc6..73e346e00 100644 --- a/codev/protocols/spir/consult-types/spec-review.md +++ b/codev/protocols/spir/consult-types/spec-review.md @@ -32,6 +32,12 @@ If the baked decisions themselves contain contradictions (e.g., two different la - Are acceptance criteria testable? - Is terminology consistent? +5. **Structure** + - The specify prompt delivers a canonical spec template (`protocols/spir/templates/spec.md`) inline. Does the spec actually follow it? + - Required headings, in order: `## Metadata`, `## Clarifying Questions Asked`, `## Problem Statement`, `## Current State`, `## Desired State`, `## Stakeholders`, `## Success Criteria`, `## Constraints`, `## Assumptions`, `## Solution Approaches`, `## Open Questions`, `## Performance Requirements`, `## Security Considerations`, `## Test Scenarios`, `## Dependencies`, `## References`, `## Risks and Mitigation`, `## Expert Consultation`, `## Approval`, `## Notes`. + - A free-form spec that reads well but ignores the template is a **defect**, not a style preference — it usually means the builder pattern-matched an older spec in `codev/specs/` instead of the delivered template. `REQUEST_CHANGES` for a wholesale departure (most headings missing or renamed). + - A section that genuinely does not apply may be reduced to a one-line "N/A — [reason]", but the heading should remain. Do not `REQUEST_CHANGES` over one such section. + ## Verdict Format After your review, provide your verdict in exactly this format: diff --git a/codev/protocols/spir/prompts/review.md b/codev/protocols/spir/prompts/review.md index 22af9aec4..eabe1b98e 100644 --- a/codev/protocols/spir/prompts/review.md +++ b/codev/protocols/spir/prompts/review.md @@ -57,55 +57,9 @@ Compare final implementation to original specification: ### 3. Create Review Document -Create `codev/reviews/{{artifact_name}}.md`: +Create `codev/reviews/{{artifact_name}}.md`, following the template below. Use these headings and this order — do not invent your own structure, and do not pattern-match an earlier review in `codev/reviews/` that predates this template. Steps 3b and 4 below expand on the `## Consultation Feedback`, `## Architecture Updates`, and `## Lessons Learned Updates` sections; porch's review checks grep for the last two by exact heading. -```markdown -# Review: {{title}} - -## Summary -Brief description of what was implemented. - -## Spec Compliance -- [x] Requirement 1: Implemented as specified -- [x] Requirement 2: Implemented with deviation (see below) -- [x] Requirement 3: Implemented as specified - -## Deviations from Plan -- **Phase X**: [What changed and why] - -## Lessons Learned - -### What Went Well -- [Positive observation 1] -- [Positive observation 2] - -### Challenges Encountered -- [Challenge 1]: [How it was resolved] -- [Challenge 2]: [How it was resolved] - -### What Would Be Done Differently -- [Insight 1] -- [Insight 2] - -### Methodology Improvements -- [Suggested improvement to SPIR protocol] -- [Suggested improvement to tooling] - -## Technical Debt -- [Any shortcuts taken that should be addressed later] - -## Consultation Feedback - -[See instructions below] - -## Flaky Tests -- [Any pre-existing tests that were skipped as flaky during this project] -- [Include test name, file path, and observed failure mode] -- [If none: "No flaky tests encountered"] - -## Follow-up Items -- [Any items identified for future work] -``` +{{> protocols/spir/templates/review.md}} ### 3b. Include Consultation Feedback diff --git a/codev/protocols/spir/prompts/specify.md b/codev/protocols/spir/prompts/specify.md index 3716f7969..daa5feab2 100644 --- a/codev/protocols/spir/prompts/specify.md +++ b/codev/protocols/spir/prompts/specify.md @@ -90,6 +90,10 @@ After completing the spec draft, signal completion. Porch will run 3-way consult Create or update the specification file at `codev/specs/{{artifact_name}}.md`. +Follow the canonical spec template reproduced below. Use these headings, in this order — do not invent your own structure, and do not pattern-match an earlier spec in `codev/specs/` that predates this template. If a section genuinely does not apply, keep the heading and write a one-line "N/A — [reason]" rather than deleting it. + +{{> protocols/spir/templates/spec.md}} + **IMPORTANT**: Keep spec/plan/review filenames in sync: - Spec: `codev/specs/{{artifact_name}}.md` - Plan: `codev/plans/{{artifact_name}}.md` diff --git a/codev/protocols/spir/protocol.json b/codev/protocols/spir/protocol.json index 0041fb5e9..f6b74c5d3 100644 --- a/codev/protocols/spir/protocol.json +++ b/codev/protocols/spir/protocol.json @@ -24,6 +24,16 @@ "models": ["gemini", "codex", "claude"], "parallel": true }, + "checks": { + "spec_exists": { + "command": "test -f codev/specs/${PROJECT_TITLE}.md", + "description": "Spec file must exist before the spec-approval gate" + }, + "spec_has_required_sections": { + "command": "grep -qF '## Problem Statement' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Current State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Desired State' codev/specs/${PROJECT_TITLE}.md && grep -qF '## Success Criteria' codev/specs/${PROJECT_TITLE}.md", + "description": "Spec must follow the canonical template delivered by the specify prompt (protocols/spir/templates/spec.md). Required headings: ## Problem Statement, ## Current State, ## Desired State, ## Success Criteria" + } + }, "max_iterations": 3, "on_complete": { "commit": true, diff --git a/codev/protocols/spir/templates/review.md b/codev/protocols/spir/templates/review.md index af72f0e62..668055637 100644 --- a/codev/protocols/spir/templates/review.md +++ b/codev/protocols/spir/templates/review.md @@ -97,6 +97,10 @@ Iterations that could have been prevented with better builder behavior: ### What Would Be Done Differently - [Actionable improvement for future builders] +### Methodology Improvements +- [Suggested improvement to the SPIR protocol] +- [Suggested improvement to tooling] + ## Architecture Updates [What you routed where — HOT `codev/resources/arch-critical.md` (tiny, capped, always-injected) vs COLD `codev/resources/arch.md` (reference) — or why no changes were needed.] @@ -115,6 +119,11 @@ Iterations that could have been prevented with better builder behavior: - [Any shortcuts taken or inconsistencies introduced] +## Flaky Tests + +- [Pre-existing tests skipped as flaky during this project — test name, file path, observed failure mode] +- [If none: "No flaky tests encountered"] + ## Follow-up Items - [Items identified for future work, outside this spec's scope] diff --git a/codev/resources/arch.md b/codev/resources/arch.md index edcad92c0..c9933590e 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -304,6 +304,8 @@ Framework files (protocol docs, templates) live in the package skeleton (resolve A `codev doctor` audit (`lib/framework-ref-audit.ts`) flags shell-fetch of framework files in a project's local `codev/` overrides; the shipped skeleton is guarded by a CI unit test. +**Every-template-has-a-consumer invariant (#1279).** A file under `protocols/

/templates/` is dead code unless some prompt or `protocol.md` delivers it via a `{{> }}` include — nothing else reads `templates/`. #1011 wired three of the nine shipped templates and the rest rotted unreferenced: builders never saw the spec and review templates, and the hand-rolled inline copies that stood in for them had already drifted from the canonical files. `__tests__/template-delivery.test.ts` now enforces both directions (no orphaned template, no dangling include) across both trees, with mutation checks proving the detector fails on a seeded violation. Consequence for authors: adding a template means adding its include in the same change, and ASPIR deliberately ships **no** `templates/` directory — it includes SPIR's, so there is exactly one copy of each template to keep current. + #### Supported Architect Harnesses & Conversation Resume (#929) **Supported architect harnesses** (Issue #929): claude and codex are supported as architects, selected via `.codev/config.json` (`shell.architect` / `shell.architectHarness`) — the same config-driven mechanism builders use, and the *recommended* one. **Gemini is builder-only** — the Gemini CLI is retiring (#778), so it is not offered or affirmed as an architect (its `GEMINI_SYSTEM_MD` builder surface stays); `doctor` warns if `gemini` is configured as an architect. (agy, the gemini successor, is deferred as an architect to #1063 — its only role-injection channel is a visible first user turn.) Harness auto-detection is **override-aware**: `getArchitectHarness` / `getBuilderHarness` resolve the harness from the override-aware command (`getResolvedCommands` → `cliOverrides` / `TOWER_ARCHITECT_CMD` / config), so a `--architect-cmd codex` / `TOWER_ARCHITECT_CMD=codex` / `--builder-cmd gemini` with no matching harness config still resolves the *non-claude* harness, not claude. (Before #929 it auto-detected from the raw config value only — an override launched the non-claude CLI but resolved the claude harness, re-arming the resume crash-loop below.) An explicit `shell.architectHarness` / `shell.builderHarness` still wins over auto-detection. OpenCode remains builder-only (file-based injection needs an ephemeral worktree). Codex reads project context (`AGENTS.md`) natively, so no architect context-file seam is needed; the `getArchitectFiles` seam #1059 added for gemini was removed with gemini's architect support. diff --git a/codev/state/bugfix-1279_thread.md b/codev/state/bugfix-1279_thread.md new file mode 100644 index 000000000..48b987ae5 --- /dev/null +++ b/codev/state/bugfix-1279_thread.md @@ -0,0 +1,170 @@ +# bugfix-1279 — SPIR spec template is dead code + +Issue #1279: `codev-skeleton/protocols/spir/templates/spec.md` exists but nothing +delivers or enforces it. Audit mandate: check *all* template references across +*all* protocols before fixing one. + +## Investigate — findings + +The delivery mechanism already exists. Spec 1011 introduced +`{{> }}` includes (`resolveCodevIncludes()` in `lib/skeleton.ts`), +resolved on two channels: at spawn inside `protocol.md`, and in porch's +`loadPromptFile()` for phase prompts. It was applied to 3 of the 9 shipped +templates and never generalized — nothing enforces that a `templates/*.md` file +has a consumer. + +### Full template-delivery audit (codev-skeleton/protocols) + +| Template | Delivered? | How / why not | +|---|---|---| +| `spir/templates/plan.md` | YES | `{{> }}` in `spir/prompts/plan.md` | +| `spir/templates/spec.md` | **NO** | `prompts/specify.md` describes content in prose only — the reported bug | +| `spir/templates/review.md` | **NO** | `prompts/review.md` carries a *divergent hand-rolled inline copy* | +| `aspir/templates/plan.md` | **NO** | byte-identical dup; `aspir/prompts/plan.md` includes **spir's** template | +| `aspir/templates/spec.md` | **NO** | byte-identical dup, zero consumers | +| `aspir/templates/review.md` | **NO** | byte-identical dup, zero consumers | +| `maintain/templates/maintenance-run.md` | **NO** | `maintain/protocol.md` carries a divergent inline copy | +| `experiment/templates/notes.md` | YES | `{{> }}` in `experiment/protocol.md` | +| `spike/templates/findings.md` | YES | `{{> }}` in `spike/protocol.md` | + +PIR/AIR/BUGFIX/RESEARCH ship no `templates/` dir — their prompts carry structure +inline by design. Not dead code (no orphaned file), so out of scope. + +### The drift is real, not theoretical + +Both inline copies have already diverged from the template they duplicate: + +- `spir/prompts/review.md`'s block is missing `## Key Metrics`, `## Timelog`, + `## Consultation Iteration Summary`, `## Architecture Updates`, + `## Lessons Learned Updates`; the template is missing `## Flaky Tests`. +- `maintain/protocol.md`'s block has `## Audit Findings`; the template doesn't. + +### Gate enforcement is weaker than the issue states + +The issue says the `spec-approval` gate "checks only that the spec file exists." +It checks **nothing** — `specify` has `"checks": {}` in both `spir/protocol.json` +and `aspir/protocol.json`. (`plan` has three checks; `review` has four.) + +### Root cause + +An authored artifact with no owning consumer. The `{{> }}` mechanism landed for +three templates and no invariant was written that every template must be +delivered — so six templates rot silently while builders pattern-match the +previous artifact's shape. + +## Fix shape + +1. Wire the dead-but-live templates via `{{> }}` (spec, review, maintenance-run). + Merge the drifted content into the canonical template first so nothing is lost. +2. Delete the three byte-identical `aspir/templates/*` duplicates; ASPIR already + points at SPIR's templates for plans. +3. Regression test: enumerate every `protocols/*/templates/*.md` and assert each + is reachable via a `{{> }}` include. Fails today, passes after (1)+(2). +4. Structure focus area in `consult-types/spec-review.md`. +5. Porch `specify`-phase checks (`spec_exists`, `spec_has_required_sections`). + +Mirrored in BOTH trees — PR #1278 (Spec 1252) closed **unmerged**, so the +`codev/protocols/` shadow copies still exist and are currently in sync with the +skeleton. Keeping them in sync is mandatory. + +**LOC note**: deletion-dominated. ~916 lines of the diff are the six dead +duplicate files (3 templates x 2 trees). Notified the architect. + +## Fix — done + +All five fix items landed, mirrored across `codev/` and `codev-skeleton/`. + +- `spir` + `aspir` `prompts/specify.md`: `{{> protocols/spir/templates/spec.md}}` + under `## Output`. Written as a **pure addition** (the original sentence is + untouched) — Spec 746 freezes baselines for these four files and rejects any + edit that modifies an existing line. Found this the hard way; worth knowing + before editing `specify.md` / `implement.md` / the consult-types. +- `spir` + `aspir` `prompts/review.md`: the divergent 48-line inline block + replaced by `{{> protocols/spir/templates/review.md}}`. `## Flaky Tests` and + `### Methodology Improvements` (present only in the prompt copy) merged into + the canonical template first. +- `maintain/protocol.md`: inline run-file block replaced by an include; + `## Audit Findings` and the changes-log example row merged into the template + first. Note the template was already the *superset* — it carried + `### Dependencies Cleaned`, which the inline copy had dropped, so maintain + builders have been missing that heading. +- Deleted `aspir/templates/{spec,plan,review}.md` in both trees after sha256 + byte-identity proof (per file, per tree, cross-tree, hashed from `git show + HEAD:` so in-flight edits couldn't confound it). +- `spec-review.md` consult type (both protocols): new Structure focus area. +- `specify` phase in `spir`/`aspir` `protocol.json`: `spec_exists` + + `spec_has_required_sections`, backed by new `runArtifactCheck` cases. + +### Two things worth passing on + +**The enforcement test caught my own bug.** I wrote a literal +`{{> protocols/spir/templates/.md}}` inside backticks in `aspir/protocol.md` +prose to explain the mechanism. `resolveCodevIncludes` does a blind regex +replace — it would have expanded that to empty at delivery, silently deleting +the paragraph. The dangling-include half of the test flagged it immediately. +Do not write a literal include directive in prose in any file that gets served +through the resolver. + +**Editing `protocol.json` with `json.dump` is a trap.** Round-tripping +reserialized the whole file: `→` became `→`, and every inline array +(`["gemini", "codex", "claude"]`) reflowed to multi-line. 49 lines of churn for +a 10-line change. Reverted and did a surgical string insert against a unique +anchor instead — 10 lines, exactly the intended diff. + +### Verification + +- `pnpm build`: clean. +- Full unit suite: **3787 passed / 48 skipped / 0 failed** (193 files), re-run + after the last doc edit. No flaky tests encountered, nothing skipped by me. +- End-to-end through the *real* four-tier resolver (not just the test's + synthetic call): all five includes expand, zero `{{>` residue, spec prompt + grows 4.6KB → 9.65KB with the template attached. Served from tier 2 + (`codev/`) in this repo, as expected. +- Live mutation demo: seeded an orphan template into the real skeleton, ran the + real test, watched it fail with the file named; removed it. + +### Left alone deliberately + +`codev/protocols/maintain/templates/{audit-report,lessons-learned}.md` — also +unreferenced, but local-only with no skeleton counterpart, and explicitly +preserved by an architect T8 ruling in the 1252 shadow-tree audit. Exempted in +the test with that reason in a comment rather than reversed unilaterally. +Flagged as follow-up in the PR. + +## PR + CMAP + +PR #1283. CMAP: gemini=APPROVE, codex=REQUEST_CHANGES, claude=APPROVE. + +**Codex was right and I'd overclaimed.** Its finding: my enforcement test scanned +*every* `.md` under `protocols/` for includes, so an include mentioned in a +consult-type would have counted as "this template has a consumer" — but only +`prompts/*.md` and `protocol.md` actually get `resolveCodevIncludes` run over +them. I verified the premise before acting on it (`commands/consult/index.ts` +loads consult-types with plain `readCodevFile`), and it holds: an include there +is never expanded, reaching the model as literal text while the template stays +unreachable. The test was weaker than the PR body's claim. + +Rewrote `findOrphanedTemplates` as reachability-from-delivery-roots with +transitive include-following, added `findUnresolvedIncludeSites`, and added a +mutation test reproducing Codex's exact scenario. Re-ran the live seeded-orphan +demo after tightening to confirm the mutation check still has teeth. + +Gemini's APPROVE landed in 11.6s and mostly restated my PR body — logged as +APPROVE but weighted as low-information. Claude's was substantive (independently +re-verified mirror parity on disk). + +**Self-initiated fix, no reviewer asked for it.** I'd picked the +`spec_has_required_sections` headings by reading the template rather than by +measuring anything. Checked the guess against the repo's 166 real specs (last 40, +the mature-SPIR corpus): `## Solution Approaches` is absent from **30%** of them +and `## Open Questions` from 15%. Gating on those would have failed a third of +legitimate specs and taught people to route around the gate. Narrowed the hard +check to the four headings recent practice honors 88-100% of the time; the other +two stay advisory in the consult type. Added a calibration guard test. + +**Process note worth remembering**: I ran the full suite once from the repo root +and got "224 failed" — that's vitest picking up all 325 workspace test files, not +a regression. CLAUDE.md says it: never run npm/test commands from the repo root, +run them from `packages/codev`. Package-scoped run was green throughout. + +Final: 3793 passed / 48 skipped / 0 failed. Awaiting the `pr` gate. diff --git a/packages/codev/src/__tests__/review-prompt-routing.test.ts b/packages/codev/src/__tests__/review-prompt-routing.test.ts index 4970bf095..0d0414cdd 100644 --- a/packages/codev/src/__tests__/review-prompt-routing.test.ts +++ b/packages/codev/src/__tests__/review-prompt-routing.test.ts @@ -18,7 +18,8 @@ const ROUTING_FILES = [ 'protocols/aspir/prompts/review.md', 'protocols/pir/prompts/review.md', 'protocols/spir/templates/review.md', - 'protocols/aspir/templates/review.md', + // aspir/templates/ was deleted in #1279 — ASPIR includes SPIR's templates, so + // there is one copy to keep routed rather than two that can drift apart. ]; const files: string[] = []; diff --git a/packages/codev/src/__tests__/template-delivery.test.ts b/packages/codev/src/__tests__/template-delivery.test.ts new file mode 100644 index 000000000..89e2fcc4b --- /dev/null +++ b/packages/codev/src/__tests__/template-delivery.test.ts @@ -0,0 +1,383 @@ +/** + * Issue #1279 — every protocol template must have an owning consumer. + * + * Spec 1011 introduced `{{> }}` includes (`resolveCodevIncludes`) as + * the resolver-aware way to *deliver* a framework file to a builder. It was wired + * for three of the nine shipped templates and never generalized, so six templates + * rotted unreferenced: builders never saw them, reviewers never checked against + * them, and two hand-rolled inline copies had already drifted from the templates + * they duplicated. + * + * The invariant this file enforces: a file under `protocols/

/templates/` is + * dead code unless some prompt or `protocol.md` delivers it via a `{{> }}` + * include. Both directions are checked — no orphaned template, and no dangling + * include — across BOTH trees (`codev-skeleton/` ships to adopters, `codev/` is + * this repo's own instance; the mirror convention requires parity). + * + * The last describe block is a mutation check: it seeds an unreachable template + * into a copy of the real tree and proves the detector FAILS on it. An + * enforcement test that cannot fail is decoration. + */ +import { describe, it, expect } from 'vitest'; +import { fileURLToPath } from 'node:url'; +import { dirname, resolve, join, relative, sep } from 'node:path'; +import { + cpSync, mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, readdirSync, rmSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { resolveCodevIncludes } from '../lib/skeleton.js'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +// src/__tests__ → repo root is four levels up. +const REPO_ROOT = resolve(__dirname, '../../../..'); + +/** Trees that carry a `protocols/` directory. Both must satisfy the invariant. */ +const TREES = ['codev-skeleton', 'codev'] as const; + +const INCLUDE_RE = /\{\{>\s*([^}\s]+)\s*\}\}/g; + +/** Every `.md` under the tree, as tree-relative POSIX paths (e.g. `protocols/spir/prompts/plan.md`). */ +function listMarkdown(treeRoot: string, subdir: string): string[] { + const start = join(treeRoot, subdir); + if (!existsSync(start)) return []; + const out: string[] = []; + const walk = (dir: string): void => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) walk(full); + else if (entry.name.endsWith('.md')) out.push(relative(treeRoot, full).split(sep).join('/')); + } + }; + walk(start); + return out.sort(); +} + +/** Include targets (`{{> path}}`) appearing anywhere in the tree's protocol docs. */ +function collectIncludeTargets(treeRoot: string): Map { + const targets = new Map(); + for (const rel of listMarkdown(treeRoot, 'protocols')) { + const content = readFileSync(join(treeRoot, rel), 'utf-8'); + for (const m of content.matchAll(INCLUDE_RE)) { + const target = m[1]; + const sources = targets.get(target) ?? []; + sources.push(rel); + targets.set(target, sources); + } + } + return targets; +} + +/** Template files (`protocols/

/templates/*.md`) present in the tree. */ +function listTemplates(treeRoot: string): string[] { + return listMarkdown(treeRoot, 'protocols').filter(p => /^protocols\/[^/]+\/templates\//.test(p)); +} + +/** + * The only two files whose `{{> }}` includes are actually resolved, and therefore + * the only two kinds of file that can *deliver* a template: + * + * - `protocols/

/prompts/*.md` — porch's `loadPromptFile` runs + * `resolveCodevIncludes` on phase prompts. + * - `protocols/

/protocol.md` — `resolveProtocolReference` runs it at spawn. + * + * Nothing else does. `consult-types/*.md` in particular are read via plain + * `readCodevFile` (`commands/consult/index.ts`), so an include written there is + * never expanded — it would reach the model as literal `{{> ... }}` text and + * would NOT make the template it names reachable by any builder. + */ +const DELIVERY_ROOT_RE = /^protocols\/[^/]+\/(?:prompts\/[^/]+\.md|protocol\.md)$/; + +/** Read a tree-relative path, falling back to the sibling tree (mirrors the four-tier resolver). */ +function readAcrossTrees(treeRoot: string, rel: string): string | null { + const here = join(treeRoot, rel); + if (existsSync(here)) return readFileSync(here, 'utf-8'); + for (const t of TREES) { + const there = join(REPO_ROOT, t, rel); + if (existsSync(there)) return readFileSync(there, 'utf-8'); + } + return null; +} + +/** + * Templates actually reachable by a builder: walk out from the delivery roots and + * follow includes transitively (`resolveCodevIncludes` recurses, so a template + * included by a delivered file is itself delivered). + * + * Reachability — not "is this path mentioned in some include somewhere" — is the + * real invariant. An include sitting in a consult-type, or in a template that is + * itself orphaned, delivers nothing. + */ +function reachableIncludeTargets(treeRoot: string): Set { + const reached = new Set(); + const queue = listMarkdown(treeRoot, 'protocols').filter(p => DELIVERY_ROOT_RE.test(p)); + const seenFiles = new Set(queue); + while (queue.length > 0) { + const rel = queue.shift()!; + const content = readAcrossTrees(treeRoot, rel); + if (content === null) continue; + for (const m of content.matchAll(INCLUDE_RE)) { + const target = m[1]; + reached.add(target); + if (!seenFiles.has(target)) { // recurse into the included file + seenFiles.add(target); + queue.push(target); + } + } + } + return reached; +} + +/** + * The invariant, as a pure predicate so the mutation check can run it against a + * seeded fixture. Returns templates no builder can actually receive. + */ +export function findOrphanedTemplates(treeRoot: string, exempt: readonly string[] = []): string[] { + const delivered = reachableIncludeTargets(treeRoot); + return listTemplates(treeRoot).filter(t => !delivered.has(t) && !exempt.includes(t)); +} + +/** + * Includes written where nothing resolves them — e.g. a consult-type. These are + * silent failures: the directive reaches the reader as literal text. + */ +export function findUnresolvedIncludeSites(treeRoot: string): string[] { + const bad: string[] = []; + for (const [target, sources] of collectIncludeTargets(treeRoot)) { + for (const src of sources) { + if (!DELIVERY_ROOT_RE.test(src) && !reachableIncludeTargets(treeRoot).has(src)) { + bad.push(`${src} includes ${target}, but nothing resolves includes in that file`); + } + } + } + return bad.sort(); +} + +/** + * Two local-only maintain templates in `codev/` have no skeleton counterpart and + * no consumer. They are exempt here rather than deleted: the Spec 1252 + * shadow-tree audit records an explicit architect ruling (T8) preserving them, + * and reversing that ruling is not this fix's call. Flagged as follow-up. + */ +const CODEV_LOCAL_ONLY_EXEMPT = [ + 'protocols/maintain/templates/audit-report.md', + 'protocols/maintain/templates/lessons-learned.md', +] as const; + +function exemptFor(tree: string): readonly string[] { + return tree === 'codev' ? CODEV_LOCAL_ONLY_EXEMPT : []; +} + +describe('#1279 — every protocol template has an owning consumer', () => { + it.each(TREES)('%s: no template is orphaned', (tree) => { + const orphans = findOrphanedTemplates(join(REPO_ROOT, tree), exemptFor(tree)); + expect( + orphans, + `Orphaned template(s) in ${tree}/ — no prompt or protocol.md delivers them via a ` + + `{{> path}} include, so builders never see them. Either wire them up or delete them:\n` + + orphans.map(o => ` - ${o}`).join('\n'), + ).toEqual([]); + }); + + it.each(TREES)('%s: no include points at a template that does not exist', (tree) => { + const treeRoot = join(REPO_ROOT, tree); + const dangling: string[] = []; + for (const [target, sources] of collectIncludeTargets(treeRoot)) { + // Includes resolve through the four-tier chain, so a target that is absent + // from THIS tree may still resolve from the skeleton. Only a target missing + // from both trees is genuinely dangling. + const found = TREES.some(t => existsSync(join(REPO_ROOT, t, target))); + if (!found) dangling.push(`${target} (included by ${sources.join(', ')})`); + } + expect(dangling, `Dangling include target(s) in ${tree}/ — these collapse to '' at delivery`).toEqual([]); + }); + + it.each(TREES)('%s: no include sits in a file whose includes are never resolved', (tree) => { + // Only prompts/*.md and protocol.md get resolveCodevIncludes run over them. + // An include anywhere else (a consult-type, say) is served as literal text. + expect(findUnresolvedIncludeSites(join(REPO_ROOT, tree))).toEqual([]); + }); + + it('the two trees ship the same set of templates (mirror parity, minus known local-only files)', () => { + const skeleton = listTemplates(join(REPO_ROOT, 'codev-skeleton')); + const local = listTemplates(join(REPO_ROOT, 'codev')) + .filter(t => !CODEV_LOCAL_ONLY_EXEMPT.includes(t as typeof CODEV_LOCAL_ONLY_EXEMPT[number])); + expect(local).toEqual(skeleton); + }); +}); + +describe('#1279 — the specific wirings this fix added', () => { + // Named explicitly so a future edit that drops one fails loudly here, with the + // reason, rather than only tripping the generic orphan check. + const WIRINGS: Array<[string, string]> = [ + ['protocols/spir/prompts/specify.md', 'protocols/spir/templates/spec.md'], + ['protocols/aspir/prompts/specify.md', 'protocols/spir/templates/spec.md'], + ['protocols/spir/prompts/review.md', 'protocols/spir/templates/review.md'], + ['protocols/aspir/prompts/review.md', 'protocols/spir/templates/review.md'], + ['protocols/maintain/protocol.md', 'protocols/maintain/templates/maintenance-run.md'], + // Pre-existing (Spec 1011) — guarded here too so the set stays complete. + ['protocols/spir/prompts/plan.md', 'protocols/spir/templates/plan.md'], + ['protocols/aspir/prompts/plan.md', 'protocols/spir/templates/plan.md'], + ['protocols/experiment/protocol.md', 'protocols/experiment/templates/notes.md'], + ['protocols/spike/protocol.md', 'protocols/spike/templates/findings.md'], + ]; + + it.each(TREES)('%s: each consumer delivers its template via an include', (tree) => { + for (const [consumer, template] of WIRINGS) { + const content = readFileSync(join(REPO_ROOT, tree, consumer), 'utf-8'); + expect(content, `${tree}/${consumer} must deliver ${template}`).toContain(`{{> ${template}}}`); + } + }); + + it('ASPIR ships no templates/ directory — it includes SPIR\'s, so there is one copy to drift', () => { + for (const tree of TREES) { + expect(existsSync(join(REPO_ROOT, tree, 'protocols/aspir/templates'))).toBe(false); + } + }); + + it('the specify prompt, once its includes resolve, actually carries the spec template', () => { + // Mirrors porch's loadPromptFile delivery path. Without this the builder sees + // prose only and pattern-matches whatever spec happens to be in codev/specs/. + for (const proto of ['spir', 'aspir']) { + const prompt = readFileSync( + join(REPO_ROOT, 'codev-skeleton', `protocols/${proto}/prompts/specify.md`), 'utf-8'); + const resolved = resolveCodevIncludes(prompt, REPO_ROOT); + expect(resolved).not.toContain('{{>'); // include expanded + expect(resolved).toContain('## Problem Statement'); // template content arrived + expect(resolved).toContain('## Solution Approaches'); + expect(resolved).toContain('SPEC vs PLAN BOUNDARY'); + } + }); + + it('the review prompt, once its includes resolve, carries the porch-checked headings', () => { + for (const proto of ['spir', 'aspir']) { + const prompt = readFileSync( + join(REPO_ROOT, 'codev-skeleton', `protocols/${proto}/prompts/review.md`), 'utf-8'); + const resolved = resolveCodevIncludes(prompt, REPO_ROOT); + expect(resolved).not.toContain('{{>'); + // porch's review_has_arch_updates / review_has_lessons_updates grep for these. + expect(resolved).toContain('## Architecture Updates'); + expect(resolved).toContain('## Lessons Learned Updates'); + // Content that lived only in the old inline copy must survive the swap. + expect(resolved).toContain('## Flaky Tests'); + expect(resolved).toContain('### Methodology Improvements'); + } + }); + + it('the maintain protocol, once its includes resolve, carries the run-file structure', () => { + const md = readFileSync(join(REPO_ROOT, 'codev-skeleton', 'protocols/maintain/protocol.md'), 'utf-8'); + const resolved = resolveCodevIncludes(md, REPO_ROOT); + expect(resolved).not.toContain('{{>'); + expect(resolved).toContain('# Maintenance Run NNNN'); + // Content that lived only in the old inline copy must survive the swap. + expect(resolved).toContain('## Audit Findings'); + // Content that lived only in the template must now reach the builder. + expect(resolved).toContain('### Dependencies Cleaned'); + }); +}); + +/** + * Mutation check (architect requirement, learned from Spec 1252 T12). + * + * The orphan detector above passes on the real tree. That is only meaningful if + * it would FAIL on a tree that violates the invariant — so seed a violation into + * a copy of the real tree and assert it is caught. + */ +describe('#1279 — the orphan detector actually fails on a violation', () => { + const withTreeCopy = (fn: (treeRoot: string) => void): void => { + const dir = mkdtempSync(join(tmpdir(), 'template-delivery-mutation-')); + try { + cpSync(join(REPO_ROOT, 'codev-skeleton', 'protocols'), join(dir, 'protocols'), { recursive: true }); + fn(dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }; + + it('control: the unmutated copy is clean', () => { + withTreeCopy(treeRoot => { + expect(findOrphanedTemplates(treeRoot)).toEqual([]); + }); + }); + + it('MUTATION: an unreferenced template is reported as orphaned', () => { + withTreeCopy(treeRoot => { + writeFileSync(join(treeRoot, 'protocols/spir/templates/orphan.md'), '# Nobody includes me\n'); + expect(findOrphanedTemplates(treeRoot)).toEqual(['protocols/spir/templates/orphan.md']); + }); + }); + + it('MUTATION: re-creating the deleted aspir template duplicates is reported as orphaned', () => { + // The exact regression this issue fixed — a per-protocol copy with no consumer. + withTreeCopy(treeRoot => { + mkdirSync(join(treeRoot, 'protocols/aspir/templates'), { recursive: true }); + for (const t of ['spec', 'plan', 'review']) { + cpSync(join(treeRoot, `protocols/spir/templates/${t}.md`), + join(treeRoot, `protocols/aspir/templates/${t}.md`)); + } + expect(findOrphanedTemplates(treeRoot)).toEqual([ + 'protocols/aspir/templates/plan.md', + 'protocols/aspir/templates/review.md', + 'protocols/aspir/templates/spec.md', + ]); + }); + }); + + it('MUTATION: an include in a consult-type does NOT count as a consumer', () => { + // Codex caught this on PR #1283: an earlier version of the detector counted an + // include in ANY markdown as delivery. Consult-types are read with plain + // readCodevFile — no include resolution — so a template named only there is + // still unreachable by every builder, and the directive reaches the reviewing + // model as literal text. + withTreeCopy(treeRoot => { + writeFileSync(join(treeRoot, 'protocols/spir/templates/orphan.md'), '# Nobody delivers me\n'); + const ct = join(treeRoot, 'protocols/spir/consult-types/spec-review.md'); + writeFileSync(ct, readFileSync(ct, 'utf-8') + '\n{{> protocols/spir/templates/orphan.md}}\n'); + // Still orphaned — the consult-type mention buys it nothing. + expect(findOrphanedTemplates(treeRoot)).toEqual(['protocols/spir/templates/orphan.md']); + // And the misplaced directive itself is reported. + expect(findUnresolvedIncludeSites(treeRoot)).toEqual([ + 'protocols/spir/consult-types/spec-review.md includes protocols/spir/templates/orphan.md,' + + ' but nothing resolves includes in that file', + ]); + }); + }); + + it('MUTATION: a template included only by another orphaned template stays orphaned', () => { + // Transitive reachability must start from the delivery roots, not from any + // include edge — two orphans pointing at each other are still two orphans. + withTreeCopy(treeRoot => { + writeFileSync(join(treeRoot, 'protocols/spir/templates/orphan-a.md'), + '# A\n\n{{> protocols/spir/templates/orphan-b.md}}\n'); + writeFileSync(join(treeRoot, 'protocols/spir/templates/orphan-b.md'), '# B\n'); + expect(findOrphanedTemplates(treeRoot)).toEqual([ + 'protocols/spir/templates/orphan-a.md', + 'protocols/spir/templates/orphan-b.md', + ]); + }); + }); + + it('a template included by a DELIVERED file is reachable transitively', () => { + // The positive counterpart: reachability follows include edges from a root, + // because resolveCodevIncludes recurses. + withTreeCopy(treeRoot => { + writeFileSync(join(treeRoot, 'protocols/spir/templates/nested.md'), '# Nested\n'); + const tmpl = join(treeRoot, 'protocols/spir/templates/plan.md'); // delivered by prompts/plan.md + writeFileSync(tmpl, readFileSync(tmpl, 'utf-8') + '\n{{> protocols/spir/templates/nested.md}}\n'); + expect(findOrphanedTemplates(treeRoot)).toEqual([]); + }); + }); + + it('MUTATION: unwiring the spec template (the reported bug) is reported as orphaned', () => { + withTreeCopy(treeRoot => { + const p = join(treeRoot, 'protocols/spir/prompts/specify.md'); + const stripped = readFileSync(p, 'utf-8') + .replace('{{> protocols/spir/templates/spec.md}}', ''); + writeFileSync(p, stripped); + // aspir/prompts/specify.md still includes it, so unwire that one too — + // this reproduces the pre-fix state exactly. + const a = join(treeRoot, 'protocols/aspir/prompts/specify.md'); + writeFileSync(a, readFileSync(a, 'utf-8').replace('{{> protocols/spir/templates/spec.md}}', '')); + expect(findOrphanedTemplates(treeRoot)).toEqual(['protocols/spir/templates/spec.md']); + }); + }); +}); diff --git a/packages/codev/src/commands/porch/__tests__/checks.test.ts b/packages/codev/src/commands/porch/__tests__/checks.test.ts index 36bc345a1..0f59d7a58 100644 --- a/packages/codev/src/commands/porch/__tests__/checks.test.ts +++ b/packages/codev/src/commands/porch/__tests__/checks.test.ts @@ -7,10 +7,13 @@ import { tmpdir } from 'node:os'; import { runCheck, runPhaseChecks, + runArtifactCheck, formatCheckResults, allChecksPassed, + REQUIRED_SPEC_SECTIONS, type CheckEnv, } from '../checks.js'; +import type { ArtifactResolver } from '../artifacts.js'; describe('porch check runner', () => { const cwd = tmpdir(); @@ -189,4 +192,72 @@ describe('porch check runner', () => { expect(results.every(r => r.passed)).toBe(true); }); }); + describe('spec checks (#1279 — spec-approval used to gate on nothing)', () => { + /** Minimal resolver stub: only getSpecContent matters for these two checks. */ + const resolverWithSpec = (content: string | null): ArtifactResolver => ({ + getSpecContent: () => content, + getPlanContent: () => null, + getReviewContent: () => null, + } as unknown as ArtifactResolver); + + const specFrom = (sections: readonly string[]): string => + `# Specification: Test\n\n${sections.map(h => `${h}\n\nbody\n`).join('\n')}`; + + it('spec_exists fails when the resolver finds no spec', () => { + const r = runArtifactCheck('spec_exists', 'cmd', resolverWithSpec(null), defaultEnv); + expect(r?.passed).toBe(false); + expect(r?.error).toBe('Spec not found'); + }); + + it('spec_exists passes when a spec is present', () => { + const r = runArtifactCheck('spec_exists', 'cmd', resolverWithSpec('# Anything'), defaultEnv); + expect(r?.passed).toBe(true); + }); + + it('spec_has_required_sections passes on a spec that follows the template', () => { + const r = runArtifactCheck( + 'spec_has_required_sections', 'cmd', resolverWithSpec(specFrom(REQUIRED_SPEC_SECTIONS)), defaultEnv); + expect(r?.passed).toBe(true); + expect(r?.output).toContain(String(REQUIRED_SPEC_SECTIONS.length)); + }); + + it('spec_has_required_sections fails on a free-form spec, naming what is missing', () => { + // The exact reported failure mode: a readable spec that ignores the template + // because the builder pattern-matched an earlier free-form spec. + const r = runArtifactCheck( + 'spec_has_required_sections', 'cmd', + resolverWithSpec('# Spec\n\n## Background\n\n## What We Will Build\n'), defaultEnv); + expect(r?.passed).toBe(false); + for (const h of REQUIRED_SPEC_SECTIONS) expect(r?.error).toContain(h); + }); + + it('spec_has_required_sections fails when only some sections are present', () => { + const partial = REQUIRED_SPEC_SECTIONS.slice(0, REQUIRED_SPEC_SECTIONS.length - 1); + const r = runArtifactCheck( + 'spec_has_required_sections', 'cmd', resolverWithSpec(specFrom(partial)), defaultEnv); + expect(r?.passed).toBe(false); + expect(r?.error).toContain('missing 1 required section'); + expect(r?.error).toContain(REQUIRED_SPEC_SECTIONS[REQUIRED_SPEC_SECTIONS.length - 1]); + }); + + it('does NOT require the sections a legitimate spec often omits (calibration guard)', () => { + // 30% of this repo's recent specs have no '## Solution Approaches' and 15% + // no '## Open Questions'. Gating on those would fire on good work, so they + // stay advisory in the spec-review consult type. Guard the decision. + expect(REQUIRED_SPEC_SECTIONS).not.toContain('## Solution Approaches'); + expect(REQUIRED_SPEC_SECTIONS).not.toContain('## Open Questions'); + const spec = specFrom(REQUIRED_SPEC_SECTIONS); // no Solution Approaches / Open Questions + expect(runArtifactCheck('spec_has_required_sections', 'cmd', resolverWithSpec(spec), defaultEnv)?.passed) + .toBe(true); + }); + + it('spec_has_required_sections fails when the spec is absent entirely', () => { + const r = runArtifactCheck('spec_has_required_sections', 'cmd', resolverWithSpec(null), defaultEnv); + expect(r?.passed).toBe(false); + }); + + it('an unrecognized check name still falls through to shell execution', () => { + expect(runArtifactCheck('something_else', 'cmd', resolverWithSpec('x'), defaultEnv)).toBeNull(); + }); + }); }); diff --git a/packages/codev/src/commands/porch/checks.ts b/packages/codev/src/commands/porch/checks.ts index 7480f7b1f..bad9fb959 100644 --- a/packages/codev/src/commands/porch/checks.ts +++ b/packages/codev/src/commands/porch/checks.ts @@ -124,6 +124,35 @@ export async function runCheck( }); } +/** + * Headings the `spec_has_required_sections` check requires (issue #1279). + * + * These are guaranteed by the canonical spec template + * (`protocols/spir/templates/spec.md`), which the specify prompt now delivers + * inline via a `{{> }}` include. Deliberately a core subset, not the template's + * full 20-heading list: the gate is a backstop against a wholesale departure + * from the template, not a style linter. + * + * Calibrated against this repo's 166 existing specs, restricted to the 40 most + * recent (the corpus written under mature SPIR). Absence rates there: Success + * Criteria 0%, Problem Statement 5%, Desired State 5%, Current State 12% — all + * four are near-universal in practice, so requiring them catches a template + * departure without punishing normal work. + * + * `## Solution Approaches` (30% absent) and `## Open Questions` (15%) are + * deliberately NOT required here: a spec with one obvious approach and no open + * questions is legitimate, and a hard gate that fires on a third of good specs + * trains people to route around it. Both are still checked — advisorily, where + * judgement is possible — by the Structure focus area in the `spec-review` + * consult type. + */ +export const REQUIRED_SPEC_SECTIONS = [ + '## Problem Statement', + '## Current State', + '## Desired State', + '## Success Criteria', +] as const; + /** * Try to run an artifact-dependent check programmatically via the resolver. * Returns a CheckResult if the check name is recognized, null otherwise @@ -139,6 +168,40 @@ export function runArtifactCheck( const { PROJECT_ID: projectId, PROJECT_TITLE: title } = env; switch (name) { + case 'spec_exists': { + const content = resolver.getSpecContent(projectId, title); + return { + name, + command, + passed: content !== null, + output: content !== null ? 'Spec found via resolver' : undefined, + error: content === null ? 'Spec not found' : undefined, + duration_ms: Date.now() - startTime, + }; + } + + case 'spec_has_required_sections': { + const content = resolver.getSpecContent(projectId, title); + if (content === null) { + return { name, command, passed: false, error: 'Spec not found', duration_ms: Date.now() - startTime }; + } + const missing = REQUIRED_SPEC_SECTIONS.filter(h => !content.includes(h)); + return { + name, + command, + passed: missing.length === 0, + output: missing.length === 0 + ? `Found all ${REQUIRED_SPEC_SECTIONS.length} required sections` + : undefined, + error: missing.length === 0 + ? undefined + : `Spec is missing ${missing.length} required section(s): ${missing.join(', ')}. ` + + 'The specify prompt delivers the canonical template inline — follow its headings ' + + 'rather than copying an earlier spec.', + duration_ms: Date.now() - startTime, + }; + } + case 'plan_exists': { const content = resolver.getPlanContent(projectId, title); return {