Skip to content

Comments

feat(skills): add create-prd and create-tech-spec skills#87

Merged
aryeko merged 13 commits intomainfrom
feat/prd-and-tech-spec-skills
Feb 19, 2026
Merged

feat(skills): add create-prd and create-tech-spec skills#87
aryeko merged 13 commits intomainfrom
feat/prd-and-tech-spec-skills

Conversation

@aryeko
Copy link
Owner

@aryeko aryeko commented Feb 19, 2026

Summary

  • Add two new agent skills (create-prd and create-tech-spec) that complete the planpilot product workflow chain: idea → PRD → tech spec → plans → GitHub sync
  • Update install docs (INSTALL.md, INSTALL.agent.md) and README for all three skills
  • Switch hardcoded v2.3.0 install URLs to main branch so new skills are immediately available
  • Include test artifacts (PRD, tech spec, design doc) demonstrating the full skill chain works end-to-end

Test plan

  • Ran create-prd skill end-to-end → produced docs/prd/planpilot-validate-prd.md
  • Ran create-tech-spec skill end-to-end → produced docs/specs/planpilot-validate-spec.md
  • Fed spec through roadmap-to-github-project → produced valid .plans JSON (20 items, dry-run exit 0)
  • poe docs-links passes (all markdown links valid)
  • Verify install URLs resolve after merge to main

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced two new agent skills: Create PRD and Create Tech Spec for generating structured product and technical specifications from feature ideas and codebases.
    • Added comprehensive end-to-end workflow examples demonstrating validation and synchronization use cases.
  • Documentation

    • Updated installation guides to support multiple skills installation.
    • Expanded examples with detailed workflow documentation and step-by-step guides for validation and GitHub synchronization workflows.

aryeko and others added 9 commits February 19, 2026 01:02
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevents generated specs from containing machine-specific absolute paths
by instructing the agent to use project-relative paths instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Includes design doc, implementation plan, and test outputs from
the create-prd/create-tech-spec/roadmap-to-github-project chain.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- INSTALL.md: "both" → "all three skills"
- create-tech-spec: "three" → "four" fixed closing sections

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace hardcoded v2.3.0 tag with main branch in all install URLs.
The new skills don't exist at v2.3.0; main ensures they're available
immediately. Pin to a release tag once the next version is cut.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 19, 2026

📝 Walkthrough

Walkthrough

This pull request expands the planpilot project from a single skill to a multi-skill architecture, introducing two new skills (create-prd and create-tech-spec) alongside the existing roadmap-to-github-project skill. It includes comprehensive design documentation, skill definitions, updated installation procedures, full example workflows, and supporting configuration files.

Changes

Cohort / File(s) Summary
Configuration & Git Rules
.gitignore
Added negation pattern to re-include examples/**/planpilot.json files under version control.
Root Documentation
README.md
Updated for multi-skill architecture: revised example sync command paths, updated installation instructions to cover all three skills via loop-based installation, changed section header to plural, and updated skill URLs from v2.3.0 to main branch.
Design Documents
docs/plans/2026-02-19-prd-and-tech-spec-skills-design.md, docs/plans/2026-02-19-prd-and-tech-spec-skills.md
Introduced comprehensive design and implementation documentation for two new skills, including input/output schemas, frontmatter definitions, workflow steps, file naming conventions, artifact locations, and orchestration guidance.
Skill Definitions
skills/create-prd/SKILL.md, skills/create-tech-spec/SKILL.md
Defined two new skills: create-prd guides PRD generation from feature ideas with discovery questions and nine required sections; create-tech-spec generates technical specifications from PRDs with codebase analysis, dynamic sections, and mermaid diagrams. Both include activation gates, workflow steps, and completion checklists.
Skill Installation Guides
skills/INSTALL.md, skills/INSTALL.agent.md
Expanded installation documentation from single-skill to multi-skill pattern: all three skills installed via separate downloads and directory structures, URLs updated to main branch, added verification steps for all skills, and curl/wget fallback handling.
Examples Overview & Sync Workflow
examples/README.md, examples/sync-workflow/README.md, examples/sync-workflow/planpilot.json
Reorganized examples documentation with concise quick-start and sync workflow section; added new sync workflow example with User Authentication epic, including complete plan files and GitHub synchronization configuration.
Full Workflow Example
examples/full-workflow/README.md, examples/full-workflow/planpilot.json, examples/full-workflow/plans/*.json, examples/full-workflow/prd/planpilot-validate-prd.md, examples/full-workflow/spec/planpilot-validate-spec.md
Introduced comprehensive end-to-end example for planpilot validation feature: updated config paths, added hierarchical plan structure (epics, stories, tasks) with 15+ planning items, included full PRD and technical specification documents covering offline validation workflow, five validation layers, error reporting, and CLI integration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Three skills now hop where once was one,
A patchwork quilt of work to run—
PRDs crafted, specs designed, roadmaps made,
Through activation gates our plans are laid.
With examples blooming, docs in bloom,
We've filled the warren's planning room! 🌱

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding two new agent skills (create-prd and create-tech-spec) that are central to this PR.
Description check ✅ Passed The PR description covers all key template sections: comprehensive Summary explaining objectives, detailed Changes covering skills and documentation updates, complete Test plan with verification results, and clear Breaking changes statement.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/prd-and-tech-spec-skills

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

aryeko and others added 4 commits February 19, 2026 03:00
- Move existing sync example to examples/sync-workflow/
- Move generated test artifacts to examples/full-workflow/
- Add READMEs describing each workflow and its artifacts
- Update README.md paths for new example location

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix README dry-run sample sync map path to match actual tool output
  format (/absolute/path/to/...)
- Reorder skill listing in INSTALL.agent.md to workflow order
  (create-prd → create-tech-spec → roadmap-to-github-project)
- Normalize `author` placeholder to `<user or auto-detected>  # optional`
  in both SKILL.md frontmatter examples
- Clarify `author` is optional in create-prd completion criteria
- Extend create-prd next-steps hint to include roadmap-to-github-project

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a config so the full-workflow example can be run offline:

  planpilot sync --config examples/full-workflow/planpilot.json --dry-run

Dry-run verified: 20 items (1 epic, 4 stories, 15 tasks), exit 0.
Updates README with dry-run command under Reproducing section.
Adds !examples/**/planpilot.json negation to .gitignore so example
configs can be committed without force-adding.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aryeko aryeko merged commit 57848f5 into main Feb 19, 2026
12 of 13 checks passed
@aryeko aryeko deleted the feat/prd-and-tech-spec-skills branch February 19, 2026 06:33
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
skills/INSTALL.md (1)

142-145: ⚠️ Potential issue | 🟡 Minor

Stale singular "skill" after expanding to three-skill installation.

📝 Proposed fix
-**Restart your agent** (start a new session) so it discovers the newly installed skill. The skill won't be available until the agent re-scans `~/.agents/skills/`.
+**Restart your agent** (start a new session) so it discovers the newly installed skills. The skills won't be available until the agent re-scans `~/.agents/skills/`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/INSTALL.md` around lines 142 - 145, The documentation still uses the
singular word "skill" in the restart step after the installer was expanded to
install up to three skills; update the phrasing in the "Restart your agent"
section so it consistently uses the plural "skills" (and any nearby occurrences
of "skill" in that paragraph) and ensure the path reference `~/.agents/skills/`
remains unchanged to reflect multiple installed skills.
🧹 Nitpick comments (2)
skills/INSTALL.agent.md (1)

77-108: Skill installation order in Step 5 differs from the workflow-order used in README and INSTALL.md.

README and INSTALL.md present skills in workflow order (create-prdcreate-tech-specroadmap-to-github-project), but Step 5 here installs roadmap-to-github-project first. Users cross-referencing the two install docs may find this inconsistency confusing.

♻️ Proposed reorder to match workflow order
-Install `roadmap-to-github-project`:
-
-```bash
-mkdir -p ~/.agents/skills/roadmap-to-github-project
-```
-
-```bash
-curl -fsSL "https://raw.githubusercontent.com/aryeko/planpilot/main/skills/roadmap-to-github-project/SKILL.md" \
-  -o ~/.agents/skills/roadmap-to-github-project/SKILL.md
-```
-
 Install `create-prd`:
 ...
 Install `create-tech-spec`:
 ...
+Install `roadmap-to-github-project`:
+...
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/INSTALL.agent.md` around lines 77 - 108, Step 5 installs skills in the
wrong order; reorder the three installation blocks so they match the workflow
order used elsewhere (create-prd → create-tech-spec →
roadmap-to-github-project). Locate the three installation sections that
reference the SKILL.md downloads for create-prd, create-tech-spec, and
roadmap-to-github-project and move the roadmap-to-github-project block so it
appears after create-tech-spec; ensure the mkdir and curl commands for each
skill remain paired and unchanged.
docs/plans/2026-02-19-prd-and-tech-spec-skills-design.md (1)

193-196: Minor: future-tense note is already completed in this PR.

Line 196 says "INSTALL.md and INSTALL.agent.md will be updated to include all three skills" — those updates are part of this same PR. Consider updating to past tense for historical accuracy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-02-19-prd-and-tech-spec-skills-design.md` around lines 193 -
196, Update the sentence "INSTALL.md and INSTALL.agent.md will be updated to
include all three skills" to past tense for accuracy (e.g., "INSTALL.md and
INSTALL.agent.md have been updated to include all three skills" or "were
updated") so the document reflects that the changes are included in this PR;
locate that exact phrase in the section referencing SKILL.md and replace with
the past-tense wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/plans/2026-02-19-prd-and-tech-spec-skills.md`:
- Around line 1-15: The document starts with an H1 then jumps to "### Task 1"
(and subsequent "### Task N") which violates heading increment rules; change all
task headings from H3 (### Task 1 … ### Task 6) to H2 (## Task 1 … ## Task 6) so
the sequence goes H1 → H2 → H3 if subheadings are needed, and apply this same
change for every "### Task" heading in the file (including both create-prd and
create-tech-spec SKILL descriptions mentioned in the plan).

In `@examples/full-workflow/prd/planpilot-validate-prd.md`:
- Around line 135-145: The diagram currently shows four layers (nodes B–E) but
the text and epics describe five validation layers; split the combined
"Dependency & Uniqueness Check" into two distinct Layer 4 and Layer 5 nodes so
the Mermaid graph matches the doc. Update the nodes referenced (A, B, C, D, E,
F) so the flow becomes A -> B -> C -> D -> E -> F -> G (or similarly insert a
new node between current E and F), rename labels to explicitly show "Layer 1:
JSON Syntax Validation", "Layer 2: Schema Validation", "Layer 3: Hierarchy
Validation", "Layer 4: Dependency Cycle Detection", "Layer 5: ID Uniqueness
Check", then final node as "Validation Report"; also ensure the style lines that
target node names are updated to include the new node name(s).

In `@examples/full-workflow/spec/planpilot-validate-spec.md`:
- Line 243: The fenced code blocks for the PASS and FAIL validation report
examples (the blocks containing the text "planpilot - validation report") are
missing a language specifier; update both fences to use ```text so they read
```text followed by the report content and closing ```, ensuring MD040 is
satisfied for the examples in planpilot-validate-spec.md.
- Around line 417-422: The spec currently contradicts itself about handling
--config and --plan-dir: the "CLI argument parsing" section says "--plan-dir
takes precedence" while the "Risks & Mitigations" section demands an error when
both are provided; pick one behavior and make the document consistent. Decide
whether to (A) keep the simpler precedence rule (make --plan-dir override
--config) and remove the "Error message if both provided" note from the Risks &
Mitigations, or (B) enforce mutual exclusivity (emit an error when both flags
are supplied) and change the "CLI argument parsing" line to state they are
mutually exclusive; update all references to --config and --plan-dir accordingly
so the spec has a single, unambiguous rule.
- Around line 286-288: The example output incorrectly states "(4 items in
cycle)" for the cycle string "task-1 → task-2 → task-3 → task-1"; update the
parenthetical to the correct distinct item count "(3 items in cycle)" so the
example matches TR5.2 semantics and counts only unique items in the cycle (e.g.,
change the parenthetical after "task-1 → task-2 → task-3 → task-1" to "(3 items
in cycle)").
- Around line 114-144: The spec is inconsistent about where CycleDetector lives
(diagram places CycleDetector in Core Layer alongside PlanValidator/PlanLoader,
while the "Extension Point" and Open Tasks say to add it to the validate
command/CLI). Decide and unify placement: either move CycleDetector into the
Core (core/plan/) and update the "Extension Point" and Open Tasks to say the
validate command invokes core.plan.CycleDetector (referencing CycleDetector,
PlanValidator, PlanLoader, ValidateCmd), or move it to the CLI
(cli/commands/validate.py) and update the diagram to remove CycleDetector from
the Core and show ValidateCmd calling a CLI-local CycleDetector; update all
mentions (diagram, "Extension Point", and Open Tasks) to match the chosen
location and ensure the invocation flow references the correct symbol names
(ValidateCmd -> PlanLoader -> PlanValidator -> CycleDetector or ValidateCmd ->
CycleDetector as per chosen design).
- Around line 101-103: Add a new TR8.3 row to the TR8 behavioral contract
documenting the missing exit code: state "Exit code 1 if an unexpected/unhandled
error occurs" and map it to "R8.3 | Return 1 from command handler (any
unexpected exceptions not matching ConfigError, PlanLoadError,
PlanValidationError)"; place it alongside TR8.1 and TR8.2 so the table now lists
0, 3, and 1 and thereby makes the command handler's user-visible exit-code
guarantees complete and consistent with the command handler behavior described
in app.py.

In `@README.md`:
- Around line 133-136: The fenced code block containing "Fetch and follow
instructions from
https://raw.githubusercontent.com/aryeko/planpilot/main/skills/INSTALL.agent.md"
lacks a language specifier and triggers markdownlint MD040; update that fenced
block to include a language (e.g., add "text" after the opening ``` so the block
starts with ```text) to satisfy the linter and preserve the content formatting.

In `@skills/create-prd/SKILL.md`:
- Around line 54-61: The schema table marks `tags` as optional but the
Completion Criteria requires `tags` alongside `title`, `status`, and `created`,
causing conflicting instructions; update the schema table row for `tags` to mark
it as required (change the "Required" column for `tags` from `no` to `yes`) so
the table aligns with the Completion Criteria and the agent will include `tags`
in generated PRDs.

In `@skills/create-tech-spec/SKILL.md`:
- Around line 55-63: The frontmatter schema currently marks the `tags` field as
optional but completion criterion 2 requires it; either make `tags` required in
the frontmatter schema table (change its "Required" value to `yes` for `tags`)
or update completion criterion 2 to remove `tags` as a mandatory field so both
agree; locate the `tags` row in the frontmatter schema table and the text
referenced as "completion criterion 2" and update one of them so the schema and
criteria are consistent.

---

Outside diff comments:
In `@skills/INSTALL.md`:
- Around line 142-145: The documentation still uses the singular word "skill" in
the restart step after the installer was expanded to install up to three skills;
update the phrasing in the "Restart your agent" section so it consistently uses
the plural "skills" (and any nearby occurrences of "skill" in that paragraph)
and ensure the path reference `~/.agents/skills/` remains unchanged to reflect
multiple installed skills.

---

Nitpick comments:
In `@docs/plans/2026-02-19-prd-and-tech-spec-skills-design.md`:
- Around line 193-196: Update the sentence "INSTALL.md and INSTALL.agent.md will
be updated to include all three skills" to past tense for accuracy (e.g.,
"INSTALL.md and INSTALL.agent.md have been updated to include all three skills"
or "were updated") so the document reflects that the changes are included in
this PR; locate that exact phrase in the section referencing SKILL.md and
replace with the past-tense wording.

In `@skills/INSTALL.agent.md`:
- Around line 77-108: Step 5 installs skills in the wrong order; reorder the
three installation blocks so they match the workflow order used elsewhere
(create-prd → create-tech-spec → roadmap-to-github-project). Locate the three
installation sections that reference the SKILL.md downloads for create-prd,
create-tech-spec, and roadmap-to-github-project and move the
roadmap-to-github-project block so it appears after create-tech-spec; ensure the
mkdir and curl commands for each skill remain paired and unchanged.

Comment on lines +1 to +15
# PRD and Tech Spec Skills Implementation Plan

> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.

**Goal:** Create two new SKILL.md files (`create-prd` and `create-tech-spec`) and update installation docs to include them.

**Architecture:** Two independent skills following the same pattern as `roadmap-to-github-project`. Each skill is a standalone SKILL.md with frontmatter, workflow, and section definitions. Installation docs updated to cover all three skills.

**Tech Stack:** Markdown SKILL.md files, YAML frontmatter, mermaid diagrams

**Design doc:** `docs/plans/2026-02-19-prd-and-tech-spec-skills-design.md`

---

### Task 1: Create `create-prd` SKILL.md
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix heading level skip (H1 → H3).

The document opens with an H1 at line 1 then jumps directly to H3 (### Task 1) at line 15, skipping H2. This violates MD001 (heading-increment) and is correctly flagged by markdownlint.

📝 Proposed fix
-### Task 1: Create `create-prd` SKILL.md
+## Task 1: Create `create-prd` SKILL.md

Apply the same change to all subsequent ### Task N: headings (Tasks 2–6).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# PRD and Tech Spec Skills Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
**Goal:** Create two new SKILL.md files (`create-prd` and `create-tech-spec`) and update installation docs to include them.
**Architecture:** Two independent skills following the same pattern as `roadmap-to-github-project`. Each skill is a standalone SKILL.md with frontmatter, workflow, and section definitions. Installation docs updated to cover all three skills.
**Tech Stack:** Markdown SKILL.md files, YAML frontmatter, mermaid diagrams
**Design doc:** `docs/plans/2026-02-19-prd-and-tech-spec-skills-design.md`
---
### Task 1: Create `create-prd` SKILL.md
# PRD and Tech Spec Skills Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
**Goal:** Create two new SKILL.md files (`create-prd` and `create-tech-spec`) and update installation docs to include them.
**Architecture:** Two independent skills following the same pattern as `roadmap-to-github-project`. Each skill is a standalone SKILL.md with frontmatter, workflow, and section definitions. Installation docs updated to cover all three skills.
**Tech Stack:** Markdown SKILL.md files, YAML frontmatter, mermaid diagrams
**Design doc:** `docs/plans/2026-02-19-prd-and-tech-spec-skills-design.md`
---
## Task 1: Create `create-prd` SKILL.md
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-02-19-prd-and-tech-spec-skills.md` around lines 1 - 15, The
document starts with an H1 then jumps to "### Task 1" (and subsequent "### Task
N") which violates heading increment rules; change all task headings from H3
(### Task 1 … ### Task 6) to H2 (## Task 1 … ## Task 6) so the sequence goes H1
→ H2 → H3 if subheadings are needed, and apply this same change for every "###
Task" heading in the file (including both create-prd and create-tech-spec SKILL
descriptions mentioned in the plan).

Comment on lines +135 to +145
```mermaid
graph LR
A["Plan Files<br/>JSON"] -->|Layer 1| B["JSON Syntax<br/>Validation"]
B -->|Layer 2| C["Schema<br/>Validation"]
C -->|Layer 3| D["Hierarchy<br/>Validation"]
D -->|Layer 4| E["Dependency &<br/>Uniqueness Check"]
E --> F["Validation<br/>Report"]

style A fill:#e1f5ff
style F fill:#c8e6c9
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Diagram labels 4 layers but the text consistently describes five.

Throughout the document (Overview, Requirements R5.1/R6.1, success metrics) and the associated epics.json, the design is described as "five-layer validation" with dependency cycle detection and ID uniqueness as distinct Layer 4 and Layer 5. The diagram combines them into a single "Layer 4" box, which contradicts that description and could mislead an implementer reading the diagram in isolation.

🛠 Proposed fix — split into 5 labeled layers
-    D -->|Layer 4| E["Dependency &<br/>Uniqueness Check"]
-    E --> F["Validation<br/>Report"]
+    D -->|Layer 4| E["Dependency<br/>Cycle Check"]
+    E -->|Layer 5| G["ID Uniqueness<br/>Check"]
+    G --> F["Validation<br/>Report"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/full-workflow/prd/planpilot-validate-prd.md` around lines 135 - 145,
The diagram currently shows four layers (nodes B–E) but the text and epics
describe five validation layers; split the combined "Dependency & Uniqueness
Check" into two distinct Layer 4 and Layer 5 nodes so the Mermaid graph matches
the doc. Update the nodes referenced (A, B, C, D, E, F) so the flow becomes A ->
B -> C -> D -> E -> F -> G (or similarly insert a new node between current E and
F), rename labels to explicitly show "Layer 1: JSON Syntax Validation", "Layer
2: Schema Validation", "Layer 3: Hierarchy Validation", "Layer 4: Dependency
Cycle Detection", "Layer 5: ID Uniqueness Check", then final node as "Validation
Report"; also ensure the style lines that target node names are updated to
include the new node name(s).

Comment on lines +101 to +103
| **TR8.1** | Exit code 0 if all validations pass | R8.1 | Return 0 from command handler |
| **TR8.2** | Exit code 3 if any validation fails | R8.2 | Return 3 from command handler (matches ConfigError, PlanLoadError, PlanValidationError mapping in app.py) |
| **TR9.1** | Perform validation without GitHub authentication | R9.1 | No auth resolver, no network calls; local I/O only |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Exit code 1 missing from TR8 — behavioral contract gap.

The command handler signature (lines 230–237) documents three exit codes (0, 3, and 1 for unexpected errors), but TR8 only formalizes exit codes 0 and 3. Per the coding guidelines, behavioral contracts including all exit codes must be documented as user-visible guarantees.

Add a TR8.3 entry:

📝 Proposed addition to TR8
 | **TR8.2** | Exit code 3 if any validation fails | R8.2 | Return 3 from command handler (matches ConfigError, PlanLoadError, PlanValidationError mapping in app.py) |
+| **TR8.3** | Exit code 1 for unexpected/unhandled errors | — | Return 1 from command handler on unexpected exceptions (e.g., I/O errors, internal bugs); distinguishable from validation failures in CI/CD |

As per coding guidelines: "Behavioral contracts (exit codes, discovery semantics, dry-run semantics) must be documented as user-visible guarantees."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| **TR8.1** | Exit code 0 if all validations pass | R8.1 | Return 0 from command handler |
| **TR8.2** | Exit code 3 if any validation fails | R8.2 | Return 3 from command handler (matches ConfigError, PlanLoadError, PlanValidationError mapping in app.py) |
| **TR9.1** | Perform validation without GitHub authentication | R9.1 | No auth resolver, no network calls; local I/O only |
| **TR8.1** | Exit code 0 if all validations pass | R8.1 | Return 0 from command handler |
| **TR8.2** | Exit code 3 if any validation fails | R8.2 | Return 3 from command handler (matches ConfigError, PlanLoadError, PlanValidationError mapping in app.py) |
| **TR8.3** | Exit code 1 for unexpected/unhandled errors || Return 1 from command handler on unexpected exceptions (e.g., I/O errors, internal bugs); distinguishable from validation failures in CI/CD |
| **TR9.1** | Perform validation without GitHub authentication | R9.1 | No auth resolver, no network calls; local I/O only |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/full-workflow/spec/planpilot-validate-spec.md` around lines 101 -
103, Add a new TR8.3 row to the TR8 behavioral contract documenting the missing
exit code: state "Exit code 1 if an unexpected/unhandled error occurs" and map
it to "R8.3 | Return 1 from command handler (any unexpected exceptions not
matching ConfigError, PlanLoadError, PlanValidationError)"; place it alongside
TR8.1 and TR8.2 so the table now lists 0, 3, and 1 and thereby makes the command
handler's user-visible exit-code guarantees complete and consistent with the
command handler behavior described in app.py.

Comment on lines +114 to +144
```mermaid
graph TB
subgraph CLI["CLI Layer (cli/commands/)"]
ValidateCmd["validate.py<br/>Command Handler"]
Parser["CLI Parser<br/>(--config, --plan-dir)"]
Reporter["Report Formatter<br/>(output, exit code)"]
end

subgraph Core["Core Layer (core/plan/)"]
PlanLoader["PlanLoader<br/>Split/unified JSON parsing"]
PlanValidator["PlanValidator<br/>Hierarchy + field validation"]
CycleDetector["CycleDetector<br/>DFS-based cycle detection"]
end

subgraph Contracts["Contracts (core/contracts/)"]
Models["PlanItem, Plan<br/>PlanItemType, etc."]
end

Parser -->|args| ValidateCmd
ValidateCmd -->|load| PlanLoader
PlanLoader -->|parsed items| PlanValidator
PlanValidator -->|validate| CycleDetector
CycleDetector -->|errors| Reporter

PlanLoader -->|schema| Models
PlanValidator -->|models| Models

style CLI fill:#bbdefb
style Core fill:#c8e6c9
style Contracts fill:#fff3e0
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

CycleDetector layer placement is inconsistent across the spec.

The component diagram places CycleDetector inside the Core Layer (core/plan/ subgraph, lines 122–126), but the "Extension Point" section (line 186) and Open Tasks (line 519) both say to add it "to validate command" — which is the CLI layer (cli/commands/validate.py). These are different architectural decisions with different implications for reusability and testability, and the spec needs to resolve this before implementation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/full-workflow/spec/planpilot-validate-spec.md` around lines 114 -
144, The spec is inconsistent about where CycleDetector lives (diagram places
CycleDetector in Core Layer alongside PlanValidator/PlanLoader, while the
"Extension Point" and Open Tasks say to add it to the validate command/CLI).
Decide and unify placement: either move CycleDetector into the Core (core/plan/)
and update the "Extension Point" and Open Tasks to say the validate command
invokes core.plan.CycleDetector (referencing CycleDetector, PlanValidator,
PlanLoader, ValidateCmd), or move it to the CLI (cli/commands/validate.py) and
update the diagram to remove CycleDetector from the Core and show ValidateCmd
calling a CLI-local CycleDetector; update all mentions (diagram, "Extension
Point", and Open Tasks) to match the chosen location and ensure the invocation
flow references the correct symbol names (ValidateCmd -> PlanLoader ->
PlanValidator -> CycleDetector or ValidateCmd -> CycleDetector as per chosen
design).


Human-readable format printed to stdout:

```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fenced code blocks missing language specifier (MD040).

Both the PASS and FAIL validation report examples (lines 243 and 263) use bare fenced code blocks. Specify text to satisfy MD040.

📝 Proposed fix
-```
+```text
 planpilot - validation report

Apply to both blocks at lines 243 and 263.

Also applies to: 263-263

🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 243-243: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/full-workflow/spec/planpilot-validate-spec.md` at line 243, The
fenced code blocks for the PASS and FAIL validation report examples (the blocks
containing the text "planpilot - validation report") are missing a language
specifier; update both fences to use ```text so they read ```text followed by
the report content and closing ```, ensuring MD040 is satisfied for the examples
in planpilot-validate-spec.md.

Comment on lines +286 to +288
Dependency Cycle Errors:
- Cycle: task-1 → task-2 → task-3 → task-1 (4 items in cycle)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incorrect cycle item count in example output.

task-1 → task-2 → task-3 → task-1 involves 3 distinct items, not 4. The parenthetical (4 items in cycle) counts the repeated start node, which contradicts the format described in TR5.2 (item1 → item2 → item3 → item1 = 3-item cycle). This example will confuse implementers and end users.

📝 Proposed fix
-  - Cycle: task-1 → task-2 → task-3 → task-1 (4 items in cycle)
+  - Cycle: task-1 → task-2 → task-3 → task-1 (3 items in cycle)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Dependency Cycle Errors:
- Cycle: task-1 → task-2 → task-3 → task-1 (4 items in cycle)
Dependency Cycle Errors:
- Cycle: task-1 → task-2 → task-3 → task-1 (3 items in cycle)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/full-workflow/spec/planpilot-validate-spec.md` around lines 286 -
288, The example output incorrectly states "(4 items in cycle)" for the cycle
string "task-1 → task-2 → task-3 → task-1"; update the parenthetical to the
correct distinct item count "(3 items in cycle)" so the example matches TR5.2
semantics and counts only unique items in the cycle (e.g., change the
parenthetical after "task-1 → task-2 → task-3 → task-1" to "(3 items in
cycle)").

Comment on lines +417 to +422
3. CLI argument parsing:
- --config alone: load config
- --plan-dir alone: scan directory
- --config and --plan-dir: --plan-dir takes precedence
- Neither: search defaults, report error if not found
- --verbose: enable debug logging (info level logs)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Contradictory behavior when both --config and --plan-dir are provided.

The test strategy at line 421 specifies --plan-dir takes precedence when both flags are supplied, but the Risks & Mitigations section (line 479) states an error message should be emitted when both are provided. These are mutually exclusive behaviors — the spec must commit to one before implementation.

Recommendation: pick precedence (simpler, mirrors common CLI conventions) and remove the "Error message if both provided" note from the risk table, or vice versa.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/full-workflow/spec/planpilot-validate-spec.md` around lines 417 -
422, The spec currently contradicts itself about handling --config and
--plan-dir: the "CLI argument parsing" section says "--plan-dir takes
precedence" while the "Risks & Mitigations" section demands an error when both
are provided; pick one behavior and make the document consistent. Decide whether
to (A) keep the simpler precedence rule (make --plan-dir override --config) and
remove the "Error message if both provided" note from the Risks & Mitigations,
or (B) enforce mutual exclusivity (emit an error when both flags are supplied)
and change the "CLI argument parsing" line to state they are mutually exclusive;
update all references to --config and --plan-dir accordingly so the spec has a
single, unambiguous rule.

Comment on lines 133 to 136

```
Fetch and follow instructions from https://raw.githubusercontent.com/aryeko/planpilot/v2.3.0/skills/INSTALL.agent.md
Fetch and follow instructions from https://raw.githubusercontent.com/aryeko/planpilot/main/skills/INSTALL.agent.md
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fenced code block missing language specifier — markdownlint MD040 lint failure.

The pipeline reports MD040 at line 134. Adding text fixes it:

📝 Proposed fix
-```
+```text
 Fetch and follow instructions from https://raw.githubusercontent.com/aryeko/planpilot/main/skills/INSTALL.agent.md
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion

🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 134-134: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 133 - 136, The fenced code block containing "Fetch
and follow instructions from
https://raw.githubusercontent.com/aryeko/planpilot/main/skills/INSTALL.agent.md"
lacks a language specifier and triggers markdownlint MD040; update that fenced
block to include a language (e.g., add "text" after the opening ``` so the block
starts with ```text) to satisfy the linter and preserve the content formatting.

Comment on lines +54 to +61
| Field | Type | Required | Description |
|-------|------|----------|-------------|
| `title` | string | yes | Feature or capability name |
| `status` | enum | yes | `draft`, `review`, or `approved` |
| `created` | string | yes | ISO 8601 date (YYYY-MM-DD) |
| `author` | string | no | User name or email; can be auto-detected from git config |
| `tags` | array | no | Feature categorization tags for discoverability |

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

tags is marked optional in the schema table but required in the Completion Criteria — contradictory agent instructions.

Line 60 marks tags as no (not required), but Completion Criteria item 2 (line 202) explicitly lists tags as a mandatory field alongside title, status, and created. An agent following the schema table will omit tags, then fail the completion check, creating an unnecessary correction loop.

🛠 Proposed fix — mark `tags` as required in the schema table
-| `tags` | array | no | Feature categorization tags for discoverability |
+| `tags` | array | yes | Feature categorization tags for discoverability |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| Field | Type | Required | Description |
|-------|------|----------|-------------|
| `title` | string | yes | Feature or capability name |
| `status` | enum | yes | `draft`, `review`, or `approved` |
| `created` | string | yes | ISO 8601 date (YYYY-MM-DD) |
| `author` | string | no | User name or email; can be auto-detected from git config |
| `tags` | array | no | Feature categorization tags for discoverability |
| Field | Type | Required | Description |
|-------|------|----------|-------------|
| `title` | string | yes | Feature or capability name |
| `status` | enum | yes | `draft`, `review`, or `approved` |
| `created` | string | yes | ISO 8601 date (YYYY-MM-DD) |
| `author` | string | no | User name or email; can be auto-detected from git config |
| `tags` | array | yes | Feature categorization tags for discoverability |
🧰 Tools
🪛 LanguageTool

[style] ~59-~59: It’s more common nowadays to write this noun as one word.
Context: ...YYY-MM-DD) | | author | string | no | User name or email; can be auto-detected from git...

(RECOMMENDED_COMPOUNDS)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/create-prd/SKILL.md` around lines 54 - 61, The schema table marks
`tags` as optional but the Completion Criteria requires `tags` alongside
`title`, `status`, and `created`, causing conflicting instructions; update the
schema table row for `tags` to mark it as required (change the "Required" column
for `tags` from `no` to `yes`) so the table aligns with the Completion Criteria
and the agent will include `tags` in generated PRDs.

Comment on lines +55 to +63
| Field | Type | Required | Description |
|-------|------|----------|-------------|
| `title` | string | yes | Feature name with "— Technical Specification" suffix |
| `status` | enum | yes | `draft`, `review`, or `approved` |
| `created` | string | yes | ISO 8601 date (YYYY-MM-DD) |
| `prd` | string | yes | Path to source PRD (e.g., `docs/prd/user-auth-prd.md`) |
| `author` | string | no | User name or auto-detected from git config |
| `tags` | array | no | Technical categorization tags (e.g., `[backend, database, api]`) |

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

tags optional in schema but required in completion criteria — pick one.

The frontmatter schema table (Line 62) marks tags as optional (no), but completion criterion 2 (Line 343) lists tags as a mandatory frontmatter field. An agent following the completion criteria strictly would reject specs that omit tags, contradicting the schema.

📝 Proposed fix to align completion criteria with the schema
-2. YAML frontmatter includes `title`, `status` (draft), `created` (today's date), `prd` (path to source PRD), and `tags`
+2. YAML frontmatter includes `title`, `status` (draft), `created` (today's date), and `prd` (path to source PRD); `author` and `tags` are optional
🧰 Tools
🪛 LanguageTool

[style] ~61-~61: It’s more common nowadays to write this noun as one word.
Context: ...h-prd.md) | | author| string | no | User name or auto-detected from git config | |t...

(RECOMMENDED_COMPOUNDS)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/create-tech-spec/SKILL.md` around lines 55 - 63, The frontmatter
schema currently marks the `tags` field as optional but completion criterion 2
requires it; either make `tags` required in the frontmatter schema table (change
its "Required" value to `yes` for `tags`) or update completion criterion 2 to
remove `tags` as a mandatory field so both agree; locate the `tags` row in the
frontmatter schema table and the text referenced as "completion criterion 2" and
update one of them so the schema and criteria are consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant