diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 6f760cb..93d1e89 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "propulsion", - "version": "1.0.6", + "version": "1.1.0", "description": "Opinionated agentic coding workflow that guides software work from exploration and planning through execution and review.", "author": { "name": "Moon Pixels" diff --git a/package.json b/package.json index 49a74cb..7e04cf6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "propulsion", - "version": "1.0.6", + "version": "1.1.0", "main": "./index.mjs", "exports": "./index.mjs", "scripts": { diff --git a/skills/brainstorm/SKILL.md b/skills/brainstorm/SKILL.md index c0e0055..19e75b3 100644 --- a/skills/brainstorm/SKILL.md +++ b/skills/brainstorm/SKILL.md @@ -1,6 +1,5 @@ --- name: brainstorm -# prettier-ignore description: Create an approved PRD through repo inspection and interrogation. Use when scope, UX, constraints, or success criteria are unclear, or when user needs a PRD. --- @@ -10,43 +9,40 @@ Turn vague feature, UX, API, product-scope, or requirements work into an approve ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. -- If an approved `docs/propulsion/.../prd.md` already exists for this work, STOP. Enter the `plan` skill. +- If user provides an approved `docs/propulsion/.../prd.md`, STOP. Enter the `plan` skill. ## Instructions Follow these steps IN ORDER. Do NOT skip steps. -1. Load `interrogate` skill to close blocking branches and reach shared understanding before PRD writing. -2. If the request is too large, decompose it and explore only the first phase or subsystem. -3. After all blocking branches are closed and brainstorming is complete, check for relevant non-Propulsion skills and load them before writing `prd.md`. -4. Write `docs/propulsion/{yyyymmdd}-{feature-name}/prd.md` using the template in [references/prd-template.md](references/prd-template.md), including resolved decisions, project facts, and constraints from `interrogate`. -5. Compare `prd.md` against the conversation for missing decisions, facts, constraints, requested behaviours, or success criteria; update `prd.md` before approval if relevant content is missing. -6. Ask the user to review `prd.md`; treat only explicit approval, such as "approved" or "yes, proceed", as approval before entering `plan`. +1. Load `interrogate` skill and interview the user about their request. +2. Write `docs/propulsion/{yyyymmdd}-{feature-name}/prd.md` using the template in [references/prd-template.md](references/prd-template.md). +3. Maliciously sanity-check `prd.md` against the conversation and add any missing decisions, facts, constraints, behaviours, or success criteria. +4. Ask the user to review and approve `prd.md`. +5. After explicit approval, enter the `plan` skill. ## Rules These rules are MANDATORY. -- MUST close every blocking branch before writing `prd.md`. Blocking branches include anything that would change scope, UX, architecture, sequencing, or success criteria. -- BEFORE writing `prd.md`, ALWAYS check for relevant non-Propulsion skills and load them IMMEDIATELY. -- Propulsion skills and workflow MUST take precedence over any conflicting non-Propulsion skill UNLESS the user instructions state otherwise. +- ALWAYS use `interrogate` skill to reach a shared understanding BEFORE writing the PRD. +- ALWAYS use the PRD template for structure and section order. - MUST keep the PRD product-facing while recording durable implementation and testing decisions. - ENSURE the PRD includes ALL relevant decisions, even if they seem obvious or minor. -- You CAN create supporting documents such as `docs/propulsion/.../research.md` or `docs/propulsion/.../diagrams.md` if needed, but the PRD must include all durable decisions. -- DO include the supporting documents as implementation inputs in the PRD, but DO NOT rely on them for durable decisions. -- DO NOT print the PRD, or other workflow artefacts in the chat, keep them in files. -- If you cannot write files, STOP, ask the user to switch to write mode, then continue to write the PRD. +- USE supporting documents such as `docs/propulsion/.../diagrams.md` if needed. +- If you cannot write files, STOP, ask the user to enable write mode before continuing the PRD. +- NEVER print the full PRD in the chat, ONLY write it to the file. ## Completion Gate Do NOT leave this skill until ALL items are complete. -- [ ] Used `interrogate` skill to reach shared understanding and close every blocking branch. -- [ ] `prd.md` written to `docs/propulsion/.../prd.md`. -- [ ] `prd.md` sanity-checked against the conversation and updated if relevant content was missing. -- [ ] User has explicitly approved `prd.md` after self-review. +- [ ] Used `interrogate` skill to gather every last detail about the request. +- [ ] Written PRD to `docs/propulsion/.../prd.md`. +- [ ] Compared PRD against conversation and added any missing content. +- [ ] User has explicitly approved `prd.md`. ## Next Steps diff --git a/skills/brainstorm/references/prd-template.md b/skills/brainstorm/references/prd-template.md index 03d1fc0..fe8aedf 100644 --- a/skills/brainstorm/references/prd-template.md +++ b/skills/brainstorm/references/prd-template.md @@ -3,31 +3,46 @@ Write `docs/propulsion/{yyyymmdd}-{feature-name}/prd.md` using this exact section order. ```md -# +# PRD ## Problem Statement -State the user problem in user language. +State the problem in user language. + +## Solution + +Describe the proposed behaviour end-to-end from the user's perspective. ## Goals - Goal 1 - Goal 2 +## Non-Goals + +- Explicit non-goal 1 +- Explicit non-goal 2 + ## User Stories -1. As a , I want , so that -2. As a , I want , so that +| ID | User Story | +| ------ | ---------------------------------------------------- | +| US-001 | As a , I want , so that . | +| US-002 | As a , I want , so that . | -## Solution +## Functional Requirements -Describe the proposed behaviour end-to-end from the user's perspective. +| ID | Requirement | +| ------ | -------------------------------------------- | +| FR-001 | When , the system must . | +| FR-002 | The system must when . | -## Implementation Inputs +## Non-Functional Requirements -- External links or references to check out -- Business rules or constraints that must be respected -- References to tickets, docs, or other internal resources that are relevant to implementation +| ID | Category | Requirement | +| ------- | ----------- | --------------------------------------------- | +| NFR-001 | Performance | must complete within . | +| NFR-002 | Security | The system must . | ## Implementation Decisions @@ -35,6 +50,12 @@ Describe the proposed behaviour end-to-end from the user's perspective. - Data shape or API contract decisions - Interaction rules that the `plan` skill should not re-litigate +## Implementation Inputs + +- External links or references to check out +- Business rules or constraints that must be respected +- References to tickets, docs, or other internal resources that are relevant to implementation + ## Testing Decisions - What public behaviour matters @@ -43,20 +64,29 @@ Describe the proposed behaviour end-to-end from the user's perspective. ## Out Of Scope -- Explicit non-goals +- Deferred idea +- Thing that must not be implemented -## Risks / Open Questions +## Notes -- Only non-blocking risks or future questions +- Any further notes about the feature ``` ## Rules These rules are MANDATORY. -- Keep it product-facing. -- Include durable decisions, not file paths or code. -- Resolve blocking branches before writing. -- Write the PRD to the correct path using the correct shape. -- Use the first explored phase only if the request was decomposed. -- Self-review for placeholders, contradictions, and scope drift before asking for approval. +- ALWAYS follow the template structure and section order exactly as specified. +- MUST use the following non-functional requirement categories: + - Performance: Response times, throughput, and resource utilisation under normal and peak loads. + - Reliability: System availability, fault tolerance, and error recovery capabilities. + - Security: Data protection, authentication, authorisation, and compliance with relevant standards. + - Usability: User experience, accessibility, and ease of use across different user groups. + - Scalability: Ability to handle increased load and growth in users or data volume. + - Maintainability: Code quality, documentation, and ease of future modifications. + - Compatibility: Cross-platform support, browser compatibility, and integration capabilities. + - Portability: Ability to deploy across different environments and infrastructure. + - Compliance: Adherence to industry standards, regulations, and organisational policies. + - Monitoring: Observability, logging, and alerting capabilities for operational support. +- MUST ensure there's a LONG and EXTENSIVE list of user stories, functional requirements, and non-functional requirements to cover all aspects of the feature. +- DO create supporting documents with mermaid diagrams, data models, or other relevant artefacts if they help clarify the feature or implementation. diff --git a/skills/commit/SKILL.md b/skills/commit/SKILL.md index 6a49c5b..5e6e873 100644 --- a/skills/commit/SKILL.md +++ b/skills/commit/SKILL.md @@ -9,7 +9,7 @@ Create exactly one local git commit from safe current changes and report the res ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. - The current directory is inside a git repository with a writable index. diff --git a/skills/debug/SKILL.md b/skills/debug/SKILL.md index 064de92..674abd5 100644 --- a/skills/debug/SKILL.md +++ b/skills/debug/SKILL.md @@ -1,61 +1,66 @@ --- name: debug -# prettier-ignore -description: Handle concrete bug reports through direct intake, reproduce, diagnose, TDD-fix, verify, reset, and escalate loops. Use when failures need repair. +description: Handle concrete failures through intake, diagnosis, one-hypothesis fixes, review, reset, and escalation. Use when bugs or failures need repair. --- # Debug -Use when a concrete failure or bug report needs diagnosis and repair. +Diagnose concrete failures before dispatching one evidence-backed fix loop. ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. -- The work is a bug, regression, crash, incorrect output, flaky behaviour, or other concrete failure. -- If the work is feature-shaped or expected behaviour is intentionally being designed, STOP. Load `brainstorm`. +- The request is a concrete failure: bug, regression, crash, failing test/build, incorrect output, flaky behaviour, or runtime error. +- If the request is feature-shaped, product-scope work, expected-behaviour design, refactor, optimisation, or enhancement, STOP and load `brainstorm`. ## Instructions Follow these steps IN ORDER. Do NOT skip steps. -1. Create or resume `docs/propulsion/{yyyymmdd}-{bug-slug}/debug.md` using [references/debug-template.md](references/debug-template.md). -2. Load `interrogate` skill for missing user-answerable intake: report provenance, expected behaviour, actual behaviour, impact, environment, reproduction, and questions answered. -3. Record intake answers and resolved decisions from `interrogate` in `debug.md`; if expected behaviour is unknowable, stay in `debug` and ask or block. -4. Complete targeted codebase exploration in `debug.md` before reproduction, diagnosis, or fix work: relevant files, existing tests, commands, logs, ownership, and candidate boundaries only. -5. Reproduce, reduce, isolate, diagnose, reset, and escalate with [references/investigation-loop.md](references/investigation-loop.md). -6. Do not allow production-code changes until `debug.md` contains a grounded diagnosis, supporting evidence, fix constraints, and one chosen fix hypothesis. -7. Start a fresh bug-worker subagent with [references/bug-worker-prompt.md](references/bug-worker-prompt.md), then review with [references/bug-reviewer-prompt.md](references/bug-reviewer-prompt.md). -8. If review rejects the fix, send findings back with [references/bug-feedback-prompt.md](references/bug-feedback-prompt.md). Repeat until approved, reset when evidence contradicts the diagnosis, or reassess architecture and patterns before escalating after 3 failed fix loops. -9. If the fix is verified and approved, record closure in `debug.md` and finish. +1. Create or resume `docs/propulsion/{yyyymmdd}-{bug-slug}/debug.md` from [references/debug-template.md](references/debug-template.md) before diagnosis work. +2. Record report provenance, expected behaviour, actual behaviour, impact, environment, reproduction, prior attempts, and open blockers in `debug.md`. +3. Load `interrogate` ONLY when missing user-answerable intake blocks expected behaviour, reproduction, impact, or environment; record answers and resolved decisions in `debug.md`. +4. Explore only the relevant code, tests, logs, recent changes, ownership, and likely boundaries; record facts and intentional limits in `debug.md`. +5. Use [references/investigation-loop.md](references/investigation-loop.md) to reproduce, read the full error, reduce, compare working examples, isolate the first bad boundary, and test one diagnosis hypothesis at a time. +6. Gate fix dispatch until `debug.md` has grounded diagnosis evidence, the first bad state or divergence, fix constraints, a falsifier, and one chosen fix hypothesis. +7. Dispatch one fresh bug-worker with [references/bug-worker-prompt.md](references/bug-worker-prompt.md), then dispatch one fresh reviewer with [references/bug-reviewer-prompt.md](references/bug-reviewer-prompt.md). +8. If review rejects the fix, return findings to the active worker with [references/bug-feedback-prompt.md](references/bug-feedback-prompt.md) while the diagnosis still holds. +9. If verification, review, or new evidence contradicts the diagnosis, reset to investigation and record the contradicted evidence before any new fix attempt. +10. After 3 failed fix loops, reassess architecture and patterns, record the reassessment, then escalate to the user with evidence and next options. +11. Close only when fixed and verified, blocked by missing intake, no-repro after documented attempts, or escalated after the 3-loop reassessment path. ## Rules These rules are MANDATORY. -- MUST keep `debug.md` current from direct entry or resumed entry through closure. -- MUST complete intake for expected behaviour, actual behaviour, impact, environment, and reproduction before broad code inspection or fix work. -- MUST record `interrogate` intake answers and resolved decisions in `debug.md`. -- MUST record targeted codebase exploration before reproduction, diagnosis, or fix work. -- MUST stabilise reproduction, reduce the failing case, and isolate the first bad boundary or divergence before choosing a fix. -- NEVER edit production code in the main `debug` stage. -- EVERY fix attempt MUST start with a failing regression test and target one fix hypothesis. -- MUST record each failed hypothesis, diagnostic edit, fix attempt, review outcome, reset, escalation, and closure in `debug.md`. +- MUST keep `debug.md` current from entry through closure. +- MUST diagnose before fixing; NEVER make permanent production-code edits in the controller stage. +- MUST use `interrogate` only for missing user-answerable intake, not repo facts the agent can inspect. +- MUST reset instead of pushing through when evidence contradicts the diagnosis or chosen fix hypothesis. +- EVERY fix loop MUST target one chosen fix hypothesis and start with a failing regression test unless `tdd` declares no valuable test. +- MUST record failed hypotheses, blocked/no-repro status, rejected reviews, resets, failed loops, verification, escalation, and closure. ## Completion Gate Do NOT leave this skill until ALL items are complete. -- [ ] `debug.md` exists at `docs/propulsion/{yyyymmdd}-{bug-slug}/debug.md` and was created or resumed on entry. -- [ ] Intake, `interrogate` answers and resolved decisions, targeted codebase exploration, reproduction, reduction, evidence, ranked hypotheses, experiments, diagnosis gate, fix attempts, review outcomes, verification, and closure are recorded in `debug.md`. -- [ ] The bug is either closed with verified evidence or escalated after architecture and pattern reassessment following 3 failed fix loops. +- [ ] `debug.md` exists or is resumed at `docs/propulsion/{yyyymmdd}-{bug-slug}/debug.md`. +- [ ] Intake, user-answerable `interrogate` decisions if any, targeted exploration, reproduction or no-repro attempts, full error reading, reduction, evidence, hypotheses, diagnosis gate, fix loops, reviews, resets, and verification are recorded. +- [ ] Outcome is one of: fixed and verified; blocked on missing intake; no-repro with documented attempts; reset to diagnosis with contradicted evidence; review-rejected and returned to worker; escalated after 3 failed loops plus architecture and pattern reassessment. + +## Next Steps + +Once the completion gate is fully checked: + +- Return a concise status with the `debug.md` path, final outcome, checks run, and any user decision needed. ## References Use these references when you need detail. -- [references/debug-template.md](references/debug-template.md) - Living `debug.md` template for the full bug dossier. -- [references/investigation-loop.md](references/investigation-loop.md) - Core bug loop for reproduce, reduce, isolate, diagnose, reset, and escalate. +- [references/debug-template.md](references/debug-template.md) - Living `debug.md` template for the bug dossier. +- [references/investigation-loop.md](references/investigation-loop.md) - Evidence-first reproduce, reduce, isolate, diagnose, reset, and escalate loop. - [references/bug-worker-prompt.md](references/bug-worker-prompt.md) - Prompt template for one diagnosis-gated TDD fix attempt. - [references/bug-reviewer-prompt.md](references/bug-reviewer-prompt.md) - Prompt template for independent review of one bug fix attempt. -- [references/bug-feedback-prompt.md](references/bug-feedback-prompt.md) - Prompt template for feeding review findings back into the active bug fix attempt. +- [references/bug-feedback-prompt.md](references/bug-feedback-prompt.md) - Prompt template for returning review findings to the active worker. diff --git a/skills/debug/references/bug-feedback-prompt.md b/skills/debug/references/bug-feedback-prompt.md index 1f77357..dbed73a 100644 --- a/skills/debug/references/bug-feedback-prompt.md +++ b/skills/debug/references/bug-feedback-prompt.md @@ -5,7 +5,7 @@ Use this template when returning reviewer findings to the active bug-worker duri ````markdown **You are a subagent completing work in the Propulsion workflow.** -Your bug-fix attempt was reviewed independently. Treat the review as technical claims to verify, not instructions to obey blindly. +Your bug-fix attempt was reviewed independently under the `debug` skill. Treat the review as technical claims to verify, not instructions to obey blindly. ## Review Report @@ -15,13 +15,15 @@ Your bug-fix attempt was reviewed independently. Treat the review as technical c Follow these steps IN ORDER. Do NOT skip steps. -1. Read the full review report and triage every finding as `valid`, `invalid`, or `unclear`. -2. If any finding is `unclear`, STOP and report exactly what evidence is missing. -3. For each `valid` finding, make the minimal correct fix only if the diagnosis still holds. -4. If any valid finding or new evidence contradicts the diagnosis, STOP, update `debug.md`, and reset back to diagnosis. -5. Keep the regression-test-first requirement intact for any additional code change. -6. Re-run the relevant checks and update `debug.md` with the outcome. -7. Return an implementation report in the exact format defined below. +1. Read the full review report, current `debug.md`, and active diff. +2. Triage every reviewer finding as `valid`, `invalid`, or `unclear` before changing code. +3. If any finding is `unclear`, STOP and report `Status: unclear` with the missing evidence; do not change code. +4. Confirm every `valid` finding fits the active chosen fix hypothesis and does not require a new hypothesis. +5. If any valid finding or new evidence contradicts the diagnosis, chosen hypothesis, or fix constraints, STOP, update `debug.md`, and reset back to diagnosis. +6. For each valid in-scope finding, make the minimal correction within the active fix hypothesis only. +7. Keep the regression-test-first requirement intact for any additional code change; if no new test is valuable, record the `tdd` rationale and fallback proof. +8. Re-run the relevant checks and update `debug.md` with finding triage, code changes, verification outcome, and diagnosis status. +9. Return an implementation report in the exact format defined below. ## Output @@ -66,7 +68,9 @@ These rules are MANDATORY. - Triage every finding before changing code. - Do not continue coding once the diagnosis is contradicted. - Preserve the one-hypothesis, one-fix-loop discipline. -- Do not start a second fix hypothesis inside feedback handling; if the chosen fix hypothesis fails, update `debug.md` and reset back to diagnosis. +- Do not start a second fix hypothesis inside feedback handling; if a finding requires one, update `debug.md` and reset back to diagnosis. +- Do not broaden the active fix beyond reviewer findings that fit the chosen fix hypothesis. - Update `debug.md` before handing control back to `debug`. +- MUST return exactly one `Status:` field with `done`, `blocked`, or `unclear`. - Follow the output format EXACTLY as defined above. ```` diff --git a/skills/debug/references/bug-reviewer-prompt.md b/skills/debug/references/bug-reviewer-prompt.md index 43e34d9..cddd4c5 100644 --- a/skills/debug/references/bug-reviewer-prompt.md +++ b/skills/debug/references/bug-reviewer-prompt.md @@ -5,7 +5,7 @@ Use this template when starting a fresh bug-reviewer subagent for one bug-fix lo ````markdown **You are a subagent completing work in the Propulsion workflow.** -You are an implementation reviewer for one bug-fix attempt under the `debug` skill. +You are a sceptical implementation reviewer for one bug-fix attempt under the `debug` skill. ## Inputs @@ -19,11 +19,11 @@ This is the full self-review implementation report submitted by the bug-worker. ## Review Focus -- Confirm the diagnosis gate was satisfied before any production-code change. +- Confirm the diagnosis gate was satisfied before any production-code change and that it still explains the bug. - Reject if any required diagnosis evidence is missing: exact symptom, reduced reproduction or flaky classification, full error reading / error-reading conclusion, recent-change conclusion, applicable working example or explicit N/A, boundary tracing, first bad boundary or divergence / first-bad-divergence, fix constraints, chosen fix hypothesis, fail-then-pass regression proof, or reset evidence from prior failed loops. - Confirm the regression-test-first requirement was followed. - Confirm the fix matches the chosen fix hypothesis and fix constraints in `debug.md`. -- Confirm verification is sufficient for the reported bug behaviour. +- Confirm verification is sufficient for the reported bug behaviour, not just adjacent symptoms. - If the implementation weakens the diagnosis or fails verification, require a reset back to diagnosis. ## Instructions @@ -31,11 +31,13 @@ This is the full self-review implementation report submitted by the bug-worker. Follow these steps IN ORDER. Do NOT skip steps. 1. Read the current `debug.md` and implementation report in full. -2. Inspect the real code and diff, not just the report. -3. Verify the regression test failed first, then passed after the fix; `debug.md` must show the failing result before fix and passing result after fix. -4. Verify the change stays within the chosen fix hypothesis and does not hide unexplained evidence. -5. Return approval only if the diagnosis gate, regression-test-first requirement, diagnosis status, and verification all hold. -6. If anything fails, reject the attempt and state whether `debug` must reset back to diagnosis. +2. Inspect the real code, tests, and diff yourself; do not trust the worker report. +3. Verify the diagnosis gate was complete before the fix and that current evidence has not contradicted it. +4. Verify the regression test failed first for the expected bug reason, then passed after the fix; `debug.md` must show both results. If no test was written, verify `tdd` explicitly declared no valuable test and the fallback proof is sufficient. +5. Verify the change stays within the chosen fix hypothesis and fix constraints, with no symptom-only masking or unexplained evidence hidden by the patch. +6. Verify the checks run are sufficient for the reported bug behaviour and affected boundaries. +7. Return approval only if the diagnosis gate, chosen hypothesis fit, regression-test-first proof or accepted no-test rationale, diagnosis status, and verification all hold. +8. If anything fails, reject the attempt and state whether `debug` must reset back to diagnosis. ## Output @@ -88,7 +90,7 @@ These rules are MANDATORY. - ALWAYS check for relevant non-Propulsion skills and load them IMMEDIATELY. - Propulsion skills and workflow MUST take precedence over any conflicting non-Propulsion skill UNLESS the user instructions state otherwise. - Reject speculative or symptom-only fixes. -- Reject missing root-cause evidence, missing fail-then-pass proof, changes outside the chosen fix hypothesis, or permanent code changes made outside the bug-worker subagent. +- Reject missing root-cause evidence, missing fail-then-pass proof or accepted no-test rationale, changes outside the chosen fix hypothesis, or permanent code changes made outside the bug-worker subagent. - Call out missing verification or contradictory evidence explicitly. - MUST return exactly one `Status:` line with either `approved` or `rejected`. - If `Status: rejected`, MUST include at least one finding. diff --git a/skills/debug/references/bug-worker-prompt.md b/skills/debug/references/bug-worker-prompt.md index fe00be5..ff30f4e 100644 --- a/skills/debug/references/bug-worker-prompt.md +++ b/skills/debug/references/bug-worker-prompt.md @@ -5,7 +5,7 @@ Use this template when starting a fresh bug-worker subagent for one bug-fix loop ````markdown **You are a subagent completing work in the Propulsion workflow.** -You are a senior software engineer implementing one bug-fix attempt under the `debug` skill. +You are a senior software engineer implementing exactly one diagnosis-gated bug-fix attempt under the `debug` skill. ## Bug Context @@ -17,13 +17,15 @@ You are a senior software engineer implementing one bug-fix attempt under the `d Follow these steps IN ORDER. Do NOT skip steps. -1. Review the bug context and ask questions if the diagnosis gate, scope, or repo state is unclear. Do not guess. -2. Verify the full diagnosis gate evidence checklist in `debug.md`: exact symptom, reduced reproduction or flaky classification, full error reading, recent-change conclusion, applicable working example or explicit N/A, boundary tracing, first bad boundary or divergence, fix constraints, chosen fix hypothesis, and reset evidence from prior loops if any. -3. If the diagnosis gate is not satisfied, STOP and report that `debug` must return to diagnosis before a fix attempt. -4. Load the `tdd` skill NOW and follow it. -5. Implement one bug-fix attempt for the chosen fix hypothesis. -6. Update `debug.md` with the regression test, fix attempt, verification result, and any contradictory evidence. -7. Return an implementation report in the exact format defined below. +1. Review the bug context and ask questions if the diagnosis gate, chosen hypothesis, fix constraints, or repo state is unclear. Do not guess. +2. Verify the full diagnosis gate in `debug.md`: exact symptom, reduced reproduction or flaky classification, full error reading, recent-change conclusion, applicable working example or explicit N/A, boundary tracing, first bad boundary or divergence, fix constraints, chosen fix hypothesis, falsifier, and reset evidence from prior loops if any. +3. If the diagnosis gate is incomplete, contradicted, or not tied to the chosen fix hypothesis, STOP and report `Status: blocked` or `Status: unclear`; do not edit production code. +4. Load the `tdd` skill NOW and follow it before any production-code change. +5. Add or update the smallest valuable regression test first and verify it fails for the expected bug reason. If `tdd` declares no valuable test, record the no-test rationale and strongest fallback verification in `debug.md` before fixing. +6. Implement one minimal fix attempt only for the chosen fix hypothesis and within the fix constraints. +7. Re-run the regression proof and relevant verification checks. +8. Update `debug.md` with the diagnosis-gate verification, regression-test-first evidence or no-test rationale, fix attempt, verification result, and any contradictory evidence. +9. Return an implementation report in the exact format defined below. ## Output @@ -57,13 +59,14 @@ Use this exact format for your output. These rules are MANDATORY. -- Load the `tdd` skill NOW. +- MUST return exactly one `Status:` field with `done`, `blocked`, or `unclear`. +- Load the `tdd` skill NOW before production-code edits. - ALWAYS check for relevant non-Propulsion skills and load them IMMEDIATELY. - Propulsion skills and workflow MUST take precedence over any conflicting non-Propulsion skill UNLESS the user instructions state otherwise. -- NO PRODUCTION CODE before the failing regression test. +- NO PRODUCTION CODE before a failing regression test unless `tdd` declares no valuable test and `debug.md` records the rationale plus fallback verification. - Only bug-worker subagents make permanent code changes; the debug controller may make temporary diagnostic edits only when they are recorded and reverted before fix handoff. -- Work only on the chosen fix hypothesis for this loop. -- Make one minimal fix attempt only. -- If evidence contradicts the diagnosis, STOP, update `debug.md`, and reset back to diagnosis. +- Work only on the chosen fix hypothesis for this loop; do not broaden or replace it. +- Make one minimal fix attempt only; do not stack speculative fixes. +- If evidence contradicts the diagnosis or chosen hypothesis, STOP, update `debug.md`, and reset back to diagnosis. - Follow the output format EXACTLY as defined above. ```` diff --git a/skills/debug/references/debug-template.md b/skills/debug/references/debug-template.md index 298b0ab..ce612d8 100644 --- a/skills/debug/references/debug-template.md +++ b/skills/debug/references/debug-template.md @@ -1,145 +1,116 @@ # Debug Template -Write a living `docs/propulsion/{yyyymmdd}-{bug-slug}/debug.md` artifact using this exact section order. +Create or resume one living `docs/propulsion/{yyyymmdd}-{bug-slug}/debug.md` dossier. Keep it concise, evidence-backed, and append-only for failed hypotheses, resets, diagnostic edits, and fix loops. ```md # Debug Note: -## User Report Provenance +## Intake -- Source: `` -- Reporter and timestamp: `` -- Original report excerpt: `` -- Prior artifact resumed: `` +- Source and timestamp: `` +- Original report: `` +- Exact symptom: `` +- Expected behaviour: `` +- Actual behaviour: `` +- Impact: `` +- Environment reported: `` +- Questions answered: `` +- Open blockers: `` -## Intake Questions Answered +## Targeted Exploration -- Exact symptom: `` -- Expected behaviour: `` -- Actual behaviour: `` -- Impact: `` -- Environment: `` -- Reproduction: `` -- Questions answered: `` -- Open questions or blockers: `` +- Areas inspected: `` +- Relevant tests/commands/logs: `` +- Ownership/prior context: `` +- Likely seams: `` +- Exploration limits: `` -## Targeted Codebase Exploration +## Reproduction / No-Repro -- Relevant files or areas: `` -- Existing tests or commands: `` -- Ownership and prior context: `` -- Likely seams or boundaries: `` -- Exploration limits: `` - -## Reproduction - -- Status: `` -- Exact command or path: `` -- Expected behaviour: `` -- Actual behaviour: `` -- Reduced reproduction or flaky classification: `` +- Status: `` +- Exact command/path: `` +- Expected vs actual: `` +- Full output location: `` +- Reduced repro: `` +- No-repro/blocking rationale: `` ## Full Error Reading -- Full error, stack, warning, assertion, and exit code: `` -- First meaningful frame: `` -- Relevant surrounding logs or traces: `` -- Error-reading conclusion: `` - -## Environment Facts +- Complete error evidence: `` +- First meaningful frame/signal: `` +- Surrounding context: `` +- Conclusion: `` -- Revision / branch / artifact: `` -- Runtime and platform: `` -- Inputs, flags, config, and data facts: `` -- Scope: `` +## Environment And Recent Changes -## Recent Changes +- Revision/build: `` +- Runtime/platform/config/data: `` +- Working tree/staged diff: `` +- Recent commits/release delta: `` +- Change conclusion: `` -- Working tree and staged diff: `` -- Recent commits or release delta: `` -- Dependencies, config, environment, CI, and runtime drift: `` -- Recent-change conclusion: `` +## Reduction And Comparison -## Reduction And Isolation +- Smallest failing case: `` +- Variables removed/controlled: `` +- Working example: `` +- Broken vs working differences: `` +- First observed divergence: `` -- Smallest failing case found: `` -- What was removed or controlled: `` -- Good / bad comparison points: `` - -## Working Examples +## Boundary Tracing -- Working example or reference implementation: `` -- Broken versus working comparison: `` -- First observed divergence: `` +- Boundary map: `` +- Ingress/egress observations: `` +- Config/data/state propagation: `` +- First bad boundary: `` ## Diagnostic Edits -- Temporary diagnostic edits made: `` +- Temporary edits: `` - Revert status: `` -- Diagnostic edit outcome: `` - -## Boundary Tracing - -- Boundary map: `` -- Ingress observations: `` -- Egress observations: `` -- Config propagation: `` -- State at each component handoff: `` -- First bad boundary or divergence: `` +- Outcome: `` ## Evidence -- E1. `` +- E1. `` - E2. `` - E3. `` ## Hypotheses And Experiments - H1. `` - - Why plausible: `` + - Evidence for: `` - Strongest alternative: `` - - Experiment: `` - - Expected result: `` - - Actual result: `` - - Falsifier: `` - - Conclusion: `` -- H2. `` - - Why plausible: `` - - Experiment: `` + - Experiment: `` + - Expected result: `` - Actual result: `` - - Falsifier: `` + - Falsifier: `` - Conclusion: `` -- Experiment outcomes: `` +- H2. `` ## Diagnosis Gate -- First bad state or divergence: `` -- Root cause statement: `` caused `` because `` -- Falsifier: `` -- Fix constraints: `` +- First bad state/divergence: `` +- Root cause: ` caused because ` +- Falsifier: `` +- Fix constraints: `` - Gate status: `` -## Fix Hypothesis - -- Chosen fix hypothesis: `` -- Why this fix follows from the diagnosis: `` -- Loop number: `<1 | 2 | 3>` - ## Regression Test -- Test location: `` -- behaviour under test: `` -- Failing result before fix: `` -- Passing result after fix: `` +- Test location: `` +- Behaviour under test: `` +- Failing proof before fix: `` +- Passing proof after fix: `` ## Fix Attempts -- Attempt 1: `` +- Attempt 1: `` - Files changed: `` - - Verification: `` + - Verification: `` - Review result: `` - Outcome: `` -- Attempt 2: `` +- Attempt 2: `` ## Verification @@ -147,28 +118,30 @@ Write a living `docs/propulsion/{yyyymmdd}-{bug-slug}/debug.md` artifact using t - Wider regression checks: `` - Remaining unexplained evidence: `` -## Architecture And Pattern Reassessment +## Reassessment - Trigger: `` -- Failed loop summary: `` -- Architecture or pattern reassessment: `` -- Next direction before escalation: `` +- Failed loop summary: `` +- Architecture/pattern reassessment: `` +- Next direction/escalation: `` ## Closure -- Final status: `` -- User-visible resolution: `` -- Closure evidence: `` +- Final status: `` +- User-visible resolution: `` +- Closure evidence: `` - Follow-ups: `` ``` ## Rules -- `debug.md` starts or resumes in `debug` and stays the single living bug artifact for later loops. -- Capture user report provenance, exact symptom, expected behaviour, actual behaviour, environment facts, full error reading, recent changes, reduced reproduction or flaky classification, working examples, diagnostic edits, boundary tracing, first bad boundary or divergence, hypotheses, falsifiers, fix constraints, chosen fix hypothesis, experiment outcomes, fix attempts, review outcomes, and closure together in one file. -- If expected behaviour is unknowable, keep asking or block in `debug`; do not route to fixing. -- The diagnosis gate must be grounded before any production-code change. -- Record failed hypotheses, diagnostic edits, failed fix loops, resets, and escalations explicitly; do not erase them. -- After 3 failed fix loops, reassess architecture and patterns before user escalation. -- Temporary diagnostic edits must be reverted and recorded before fix handoff. -- Keep the artifact short, concrete, and evidence-backed. +- `debug.md` is the single audit trail from intake through closure. +- Reproduce before theorising; read the full error before summarising; reduce before widening search. +- If expected behaviour, reproduction, or environment is unknowable, record the blocker and do not dispatch a fix. +- Ground the diagnosis gate before any production-code change or fix dispatch. +- Use one hypothesis, one experiment, and one fix at a time; record expected experiment results before running them. +- Record flaky, no-repro, regression-window, performance, environment/config, data-dependent, concurrency, and multi-component evidence using the same sections. +- Record temporary diagnostic edits with file, purpose, tag/comment marker when relevant, observation, and revert status. +- Revert temporary diagnostic edits before fix handoff. +- Preserve failed hypotheses, contradicted evidence, rejected reviews, reset reasons, failed fix loops, and escalations. +- After 3 failed fix loops, reassess architecture and patterns before escalating. diff --git a/skills/debug/references/investigation-loop.md b/skills/debug/references/investigation-loop.md index 7e0f931..7703c60 100644 --- a/skills/debug/references/investigation-loop.md +++ b/skills/debug/references/investigation-loop.md @@ -1,83 +1,84 @@ # Investigation Loop -Use this reference to keep `debug` aligned with systematic, evidence-first bug resolution. +Use this loop to keep `debug.md` evidence-first and to stop fixes before root cause is grounded. -## Iron Law +## Loop -No fixes before grounded root-cause investigation. Do not choose a fix hypothesis, dispatch production-code work, or edit production code until the evidence explains the earliest bad state or divergence. Temporary diagnostic edits are allowed only when recorded in `debug.md`, used for investigation, and reverted before fix handoff. +1. Capture the feedback signal. -Work the loop in this order and keep `debug.md` current after each step. +- Record the exact symptom: failing command, assertion, crash, wrong output, visible behaviour, alert, or metric. +- Record expected versus actual behaviour and the user impact. +- Freeze relevant environment facts: revision, runtime, platform, flags, config, inputs, time/locale, dataset, tenant, CI/prod scope. -1. Reproduce and stabilise. +2. Reproduce or block. -- Capture the exact symptom, failing assertion, wrong output, or visible bad behaviour. -- Read the failure fully: full error, stack, warning, assertion, logs, exit code, and first meaningful frame before summarising. -- Freeze the environment facts that matter: revision, runtime, flags, config, inputs, time, locale, dataset, and scope. -- Reduce moving parts until one command, script, or exact manual path reproduces the issue, or classify it explicitly as flaky. +- Reproduce before theorising using one command, script, URL, or manual path. +- If it will not reproduce, record no-repro attempts, environment gaps, and the next needed signal before blocking or asking. +- For flaky failures, prove pass/fail variation, capture run counts, freeze seed/time/order where possible, and record what changes between runs. -2. Scan recent changes. +3. Read the failure fully. -- Check recent changes before broad code reading: working tree diff, staged diff, recent commits, dependencies, config changes, environment changes, CI changes, and runtime drift. -- Record the smallest credible good/bad window when one exists. -- Treat drift as evidence, not a diagnosis, until tied to the reproduced symptom. +- Read the complete error, stack, warning, assertion, logs, exit code, and first meaningful frame before summarising. +- Separate what the output proves from what it merely suggests. -3. Reduce the case. +4. Scan recent changes. -- Minimise setup, fixtures, services, flags, and data while preserving the same symptom. -- Prefer the smallest practical failing case. This follows the delta-debugging idea: remove variables until the bug survives in less space. -- If the symptom changes, record that you changed the problem. +- Check working tree diff, staged diff, recent commits, dependencies, config, environment, CI, runtime drift, and release delta before broad code reading. +- If a good/bad window exists, record the smallest credible window and isolate it before guessing. -4. Compare working examples. +5. Reduce the case. -- When applicable, compare against a working example, reference implementation, adjacent passing test, prior release, documented sample, or known-good trace. -- Record the first meaningful divergence between broken and working paths. -- If no useful working example exists, record why. +- Remove fixtures, services, flags, data, timing, and setup while preserving the same symptom. +- If the symptom changes, record that the problem changed and reset the reduction. +- For performance/resource failures, reduce to the threshold and boundary where the cost first diverges from a good baseline. +- For data-dependent failures, shrink to the smallest input, fixture, stored state, or tenant dataset that still fails. -5. Isolate the first bad boundary. +6. Compare with working evidence. -- Compare broken versus working inputs, environments, traces, or outputs. -- Find the first component, layer, handoff, or state transition where the signal changes from good to bad. -- Trace boundary data explicitly: ingress, egress, config propagation, and state at each component handoff. -- If a good/bad history window exists, isolate it before broad code reading. Diff debugging beats guessing. +- Compare against a passing test, adjacent feature, prior release, reference implementation, known-good trace, or good environment. +- For environment/config failures, compare runtime, flags, env, and config propagation at each boundary. +- Record the first meaningful broken-versus-working difference. -6. Diagnose with one ranked hypothesis at a time. +7. Isolate the first bad boundary. -- Keep a short list: current best hypothesis, strongest alternative, and unexplained evidence. -- Run one discriminating experiment at a time and record the expected result first. -- Trace backward from the late symptom to the earliest explainable bad state or divergence. -- Use existing logs, traces, dumps, breakpoints, logpoints, watchpoints, and debugger-led inspection before mutating logic. +- Trace ingress, egress, config propagation, data, state, and timing at each component handoff. +- For concurrency/order bugs, serialise when possible, use logpoints/watchpoints, and capture the first ordering change that turns good into bad. +- For multi-component failures, inspect each handoff until the earliest bad boundary is visible. -7. Gate the fix. +8. Hypothesize one cause. -- Do not choose a fix until the diagnosis explains the first bad state or divergence, not just the late symptom. -- Record one chosen fix hypothesis, the falsifier, and fix constraints in `debug.md`. -- If the evidence no longer fits, reset the diagnosis instead of pushing through. +- Keep one current best hypothesis plus the strongest alternative and unexplained evidence. +- Define the falsifier and one discriminating experiment before running it. +- Prefer existing logs, traces, dumps, breakpoints, logpoints, watchpoints, and debugger inspection before mutating code. -8. Verify the fix loop result. +9. Experiment once. -- If a dispatched fix attempt fails verification or contradicts the diagnosis, return to step 1 or 2 with the new evidence. -- After 3 failed fix loops, reassess architecture and patterns before user escalation, then escalate with the failed loops and reassessment summarised in `debug.md`. +- Run one experiment at a time and record expected result, actual result, and conclusion. +- Temporary diagnostic edits are allowed only for investigation; record file, purpose, tag/comment marker when relevant, observation, and revert status in `debug.md`. +- Revert temporary diagnostic edits before fix handoff. -Use these narrowing moves when the failure clearly fits one: +10. Diagnose and gate the fix. -- Flaky: prove pass/fail variation, freeze time/seed/order, and record what changes between runs before theorising. -- Regression window exists: isolate the smallest credible good/bad window and bisect it before broad code reading. -- Performance or resource failure: record the failing threshold, compare against a good baseline, and isolate the slow or wasteful boundary before proposing a fix. -- Environment or config mismatch: compare broken versus working runtime, flags, and config propagation at each boundary. -- Data-dependent failure: shrink to the smallest failing input, stored state, or fixture that still produces the same symptom. -- Concurrency or ordering bug: serialise the workload when possible, add logpoints or watchpoints, and capture the first ordering change that makes good turn bad. -- Multi-component boundary failure: inspect ingress and egress at each handoff until the first bad boundary is visible. +- Ground the diagnosis only when evidence explains the earliest bad state or divergence, not just the late symptom. +- Record root cause, falsifier, fix constraints, and one chosen fix hypothesis. +- Dispatch one fix at a time; if evidence no longer fits, reset diagnosis instead of pushing through. + +11. Reset or escalate. + +- If verification, review, or new evidence contradicts the model, return to the earliest loop step affected and record the reset reason. +- After 3 failed fix loops, reassess architecture and patterns before escalating to the user. +- Escalate with reproduced facts, failed hypotheses, experiments, fix attempts, reassessment, and the exact decision or access needed. ## Rules -- No fixes before grounded root-cause investigation. +- No permanent production-code changes in the controller before a grounded diagnosis. - Reproduce before theorising. -- Read the full error before summarising. -- Scan recent changes before widening the search. -- Reduce before widening the search. +- Read full errors before summarising. +- Scan changes and reduce before widening search. - Isolate before fixing. -- Compare against a working example or reference when applicable. -- Trace ingress, egress, config propagation, and state at each component handoff. -- One hypothesis, one experiment, one fix at a time. -- Reset immediately when verification or evidence breaks the current model. -- Reassess architecture and patterns after 3 failed fix loops before escalating to the user. +- Use one hypothesis, one experiment, and one fix at a time. +- Record expected experiment results before running experiments. +- Record, tag where relevant, and revert temporary diagnostic edits before fix handoff. +- Treat flaky, regression-window, performance, environment/config, data-dependent, concurrency, and multi-component cases as evidence patterns, not shortcuts to a fix. +- Reset when evidence breaks the current model. +- Reassess architecture and patterns after 3 failed fix loops before user escalation. diff --git a/skills/execute/SKILL.md b/skills/execute/SKILL.md index 7baf1fb..6d24a6b 100644 --- a/skills/execute/SKILL.md +++ b/skills/execute/SKILL.md @@ -1,6 +1,5 @@ --- name: execute -# prettier-ignore description: Execute a feature plan through subagent implementation and review one phase at a time. Use when current `plan.md` exists and the user wants feature implementation to start. --- @@ -10,64 +9,66 @@ Execute a feature plan one phase at a time. ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. - If no `docs/propulsion/.../plan.md` exists for this work, STOP. Load `plan`. -- If the plan is bug-oriented, STOP. Load `debug`. ## Instructions Follow these steps IN ORDER. Do NOT skip steps. -1. Review `plan.md` to understand how each phase is scoped. -2. Select the first incomplete phase in `plan.md` as the current phase. -3. Start a fresh worker subagent with the prompt in [references/worker-prompt.md](references/worker-prompt.md). -4. Wait for the worker to finish and report back. -5. If the worker reports `Status: unclear` then provide additional context or clarification to the worker. -6. If the worker reports `Status: blocked` then triage the blocker and resolve it. -7. Escalate to the user if you are unable to resolve `unclear` or `blocked` statuses from context within the plan, codebase, or tools. -8. If the worker reports `Status: done` then start a fresh implementation-reviewer subagent with the prompt in [references/reviewer-prompt.md](references/reviewer-prompt.md). -9. If the implementation-reviewer reports `Status: rejected`, send the findings back to the same worker subagent with the prompt in [references/worker-feedback-prompt.md](references/worker-feedback-prompt.md). -10. Repeat steps 4-9 until the worker reports `Status: done` and the latest implementation-reviewer reports `Status: approved`. -11. Check off the current phase acceptance criteria and phase completion in `plan.md`. -12. Repeat steps 2-11 for the next incomplete phase in `plan.md` until all phases are complete. -13. After the last phase is complete, infer and run the final repo-wide checks from the target repository's docs, scripts, and conventions before claiming the plan complete. +1. Review and select the first incomplete phase in `plan.md`. +2. Start a fresh worker subagent with the prompt in [references/worker-prompt.md](references/worker-prompt.md). +3. Wait for the worker to finish and handle the status: + - If `Status: unclear`, provide additional context or clarification. + - If `Status: blocked`, triage the blocker and resolve it. + - If you cannot resolve `unclear` or `blocked` from the plan, codebase, or tools, escalate to the user. + - If `Status: done`, continue to review. +4. Start a fresh reviewer subagent with the prompt in [references/reviewer-prompt.md](references/reviewer-prompt.md). +5. Wait for the reviewer to finish and handle the status: + - If `Status: approved`, mark the current phase complete in `plan.md`. + - If `Status: rejected`, send the findings back to the same worker subagent with the prompt in [references/worker-feedback-prompt.md](references/worker-feedback-prompt.md). +6. Repeat steps 3-5 until the worker reports `Status: done` and the latest reviewer reports `Status: approved`. +7. Repeat steps 1-6 for each incomplete phase in `plan.md`. +8. Infer and run repo-wide checks, such as tests and linters. +9. Inform the user that implementation is complete. +10. Ask whether the user has feedback on the implementation. ## Rules These rules are MANDATORY. -- DO NOT implement work from the orchestrating agent; ALWAYS use a fresh worker subagent for each phase. -- ALWAYS use a fresh implementation-reviewer subagent for each phase review. -- DO NOT use `execute` for bug diagnosis, bug-fix orchestration, or bug-fix feedback loops; those belong to `debug`. -- NEVER resolve `Status: unclear` or `Status: blocked` by guessing. If the information is not in the plan, codebase, or tools, escalate to the user for resolution. +- NEVER implement a phase without a worker subagent. +- ALWAYS user a fresh reviewer subagent for every review. +- NEVER resolve `Status: unclear` or `Status: blocked` by guessing; if the information is not in the plan, codebase, or tools, escalate to the user for resolution. - ALWAYS update `plan.md` checkboxes after each successful implementation-review cycle. -- ALWAYS use the prompts in the references when starting subagents and sending feedback. -- DO NOT directly implement user feedback once the completion gate is passed; loop back to `brainstorm` instead. -- You CAN use parallel worker subagents for different phases ONLY IF the phases are truly independent with no shared dependencies or risk of conflicts. -- DO NOT claim completion without final repo-wide checks chosen from the target repository's docs, scripts, and conventions. +- MUST infer and run relevant repo-wide checks before claiming completion. +- NEVER implement user feedback directly in `execute`, instead: + - Loop back to `brainstorm` to update the PRD. + - Move to `plan` to create or update a phase if needed. + - Return to `execute` for implementation. ## Completion Gate Do NOT leave this skill until ALL items are complete. - [ ] Every phase in `plan.md` has gone through a worker subagent and received `Status: done`. -- [ ] Every phase in `plan.md` has gone through an implementation-reviewer subagent and received `Status: approved`. -- [ ] Every phase acceptance criterion and phase completion checkbox in `plan.md` is checked off. +- [ ] Every phase in `plan.md` has gone through a reviewer subagent and received `Status: approved`. +- [ ] Every phase completion checkbox in `plan.md` is marked complete. - [ ] Final repo-wide checks pass. -- [ ] Completion evidence is provided to the user. +- [ ] Informed the user that implementation is complete and asked for feedback. ## Next Steps Once the completion gate is fully checked: -- Tell the user the plan is fully executed and ask whether they have any feedback on the implementation. -- If the user provides feedback after this completion handoff, do not implement it in `execute`; load `brainstorm` to update the PRD, then `plan` to create or update a phase, then return to `execute` for implementation. +- If no user feedback is requested or provided, STOP. Implementation is complete. +- If user requests changes or provides feedback, STOP. Loop back to `brainstorm`, then `plan`, then back to `execute` for implementation. ## References Use these references when you need detail. - [references/worker-prompt.md](references/worker-prompt.md) - Fresh worker subagent prompt. -- [references/reviewer-prompt.md](references/reviewer-prompt.md) - Fresh implementation-reviewer subagent prompt. -- [references/worker-feedback-prompt.md](references/worker-feedback-prompt.md) - Prompt for sending implementation-reviewer findings back to the worker subagent. +- [references/reviewer-prompt.md](references/reviewer-prompt.md) - Fresh reviewer subagent prompt. +- [references/worker-feedback-prompt.md](references/worker-feedback-prompt.md) - Prompt for sending reviewer findings back to the worker subagent. diff --git a/skills/execute/references/reviewer-prompt.md b/skills/execute/references/reviewer-prompt.md index 5f35739..b3e516f 100644 --- a/skills/execute/references/reviewer-prompt.md +++ b/skills/execute/references/reviewer-prompt.md @@ -5,43 +5,49 @@ Use this template when starting a fresh reviewer subagent in the `execute` skill ````markdown **You are a subagent completing work in the Propulsion workflow.** -You are an implementation reviewer. Verify that work for the current phase has been implemented correctly. +You are an implementation reviewer. -## Task Description +Review the completed phase like a senior engineer: verify the real work product against the plan, acceptance criteria, code quality, security, tests, and regression risk. -**Title**: +## Task Context - +**Current phase**: "> +**Plan document location**: `docs/propulsion/.../plan.md` ## Implementation Report -This is the full self-review implementation report submitted by the worker. **Do not trust it blindly, be sceptical and verify all claims yourself.** +This is the full implementation report submitted by the worker. **Treat it as context, not proof. Independently verify claims against `plan.md`, changed files, the current diff, and check output.** ## Review Criteria -| Category | What to verify | -| -------------------- | ------------------------------------------------------------------------------------------------------------------------- | -| Phase Scope | The implementation matches the current phase goal, likely areas, and constraints without drifting into unrelated work. | -| Acceptance Criteria | Every current-phase acceptance criterion is verified against the real implementation with explicit evidence. | -| Correctness | The code, prompts, workflow, and behaviour actually do what the phase claims, with no broken logic or contradictory text. | -| Verification Quality | Checks run are relevant, results support the claims made, and no obvious verification gap hides a real issue. | -| Regression Risk | The change does not introduce obvious breakage, dead steps, or workflow gaps in the surrounding execute loop. | +| Category | What to verify | +| ----------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Plan Alignment | The implementation matches the current phase goal, demo / verification outcome, likely areas, constraints, and implementation notes without drift. | +| Acceptance Criteria | Every current-phase acceptance criterion is evaluated by ID as `met`, `not met`, or `unclear` with evidence from the actual implementation. | +| Functional Correctness | Changed code, tooling, architecture, or behaviour satisfy the phase contract without broken logic, contradictions, or incomplete handling. | +| Tests / Verification | Relevant tests or checks were run where feasible, missing verification is reported, and tests validate observable behaviour without brittle coupling to source shape, private helpers, or incidental implementation details. | +| Maintainability / Refactoring | The work is clear, cohesive, appropriately simple, DRY, SOLID, YAGNI-aligned, and free of avoidable complexity or missed refactoring opportunities. | +| Security / Trust Boundaries | Inputs, permissions, secrets, file access, external calls, prompt boundaries, and other trust boundaries remain safe for the changed scope. | +| Performance / Reliability | The implementation avoids avoidable latency, resource waste, brittle failure modes, race conditions, and unreliable workflow states. | +| Integration / Regression Risk | Surrounding workflows, APIs, prompts, feedback loops, conventions, and existing behaviours remain compatible. | +| Output Usefulness | Rejections include actionable, evidence-backed findings a worker can verify or challenge technically. | -Flag only real issues you can support with evidence from the code, diff, prompts, plan, or verification output. +Flag only real issues supported by evidence from the plan, diff, files, checks, prompts, or documented workflow rules. ## Instructions Follow these steps IN ORDER. Do NOT skip steps. -1. Review the full task description to understand the intended scope. -2. Review the full implementation report to understand what was implemented. -3. Inspect the real implementation directly in the repo. Read the changed files, inspect the diff, and verify any claimed checks or evidence. -4. Review every acceptance criterion one by one and decide whether it is met, not met, or unclear from the available evidence. -5. Treat the worker's report as an input, not as proof. If a claim is unsupported by the code, diff, or verification evidence, do not accept it. -6. If you find an issue, report it as a severity-ranked finding with a concrete technical claim, evidence, impact, and a plausible fix direction. -7. Return `Status: rejected` if there is at least one real issue that should send the phase back to the worker. Otherwise, return `Status: approved`. +1. Read the current phase directly from `plan.md`. +2. Review the worker report as context, not proof. +3. Inspect the real implementation, including relevant changed files and the current diff. +4. Load relevant skills when needed to validate the produced work against skill-specific standards. +5. Run relevant tests or checks where feasible; if verification cannot be performed, report that clearly. +6. Use the review criteria table to evaluate the work product and every current-phase acceptance criterion by ID. +7. Report real issues as findings using `critical`, `high`, `medium`, `low`, or `nitpick`. +8. Return the implementation review report in the exact format below. ## Output @@ -52,19 +58,20 @@ Use this exact format for your output. **Status**: -**Acceptance Criteria Status** +**Acceptance Criteria Results** -- : - - Evidence: +- : + - Evidence: **Findings** -- [] +- [] - Location: - Issue: - - Impact: + - Impact: + - Evidence: - Fix: @@ -74,32 +81,40 @@ Use this exact format for your output. These rules are MANDATORY. -- DO NOT trust the worker's self-review report, verify all criteria yourself. -- ALWAYS check for relevant non-Propulsion skills and load them IMMEDIATELY. -- Propulsion skills and workflow MUST take precedence over any conflicting non-Propulsion skill UNLESS the user instructions state otherwise. -- MUST review the actual implementation, not only the report text. -- MUST inspect relevant changed files and the current diff before approving. -- MUST cover every current-phase acceptance criterion. -- MUST return exactly one `Status:` line with either `approved` or `rejected`. -- If `Status: rejected`, MUST include at least one finding. -- Findings MUST be ordered by severity, highest first. -- Findings MUST be evidence-based, actionable, and specific enough for a worker to verify or challenge technically. -- Prefer exact locations. If you cannot pinpoint a location, say so explicitly instead of guessing. -- DO NOT make code changes; review only. +- NEVER approve from the worker report alone; review the actual implementation, relevant changed files, and current diff. +- VERIFY the current phase directly from `plan.md` before assessing the work. +- ENSURE every current-phase acceptance criterion is evaluated by ID as `met`, `not met`, or `unclear`, with evidence. +- RETURN exactly one `Status:` line with either `approved` or `rejected`. +- Status CAN be `approved` only when every acceptance criterion is `met` and there are no blocking findings. +- Status MUST be `rejected` if any acceptance criterion is `not met` or `unclear`. +- TREAT `critical`, `high`, `medium`, and `low` findings as blocking. +- TREAT `nitpick` findings as non-blocking when all acceptance criteria are met and no blocking findings exist. +- DO NOT approve tests that assert implementation details in a way that would fail under behaviour-preserving refactors. +- INCLUDE at least one actionable finding when using `rejected`. +- ORDER findings by severity, highest first, with `nitpick` findings last. +- ENSURE findings are evidence-based, actionable, and specific enough for a worker to verify or challenge technically. +- NEVER make code changes; review only. +- ALWAYS follow the output structure and section order exactly as specified. ## Completion Gate Do NOT output your response until ALL items are complete. -- [ ] Current phase task description reviewed in full. -- [ ] Real implementation inspected in the repo, including relevant code and diff. -- [ ] Verification evidence and checks reviewed. -- [ ] Every acceptance criterion evaluated with evidence. -- [ ] Status set to `approved` or `rejected` according to the verified results. +- [ ] Current phase details reviewed directly from `plan.md`. +- [ ] Worker implementation report reviewed as context, not proof. +- [ ] Real implementation inspected in the repo, including relevant files and current diff. +- [ ] Relevant skills loaded when needed for validation. +- [ ] Relevant tests ran and checked. +- [ ] Every current-phase acceptance criterion evaluated by ID with evidence. +- [ ] Findings categorised with the required severity rules. +- [ ] Approval decision set to `approved` or `rejected` according to acceptance criteria and finding severity rules. +- [ ] Output implementation review report in the exact format specified. ```` ## Rules These rules are MANDATORY. -- Replace all placeholders with the actual current-phase content before dispatching the reviewer. +- MUST copy and paste the correct phase number and title from the plan. +- ALWAYS replace the plan path with the actual path for the plan being reviewed. +- ALWAYS paste the full worker implementation report into `Implementation Report` before dispatching the reviewer. diff --git a/skills/execute/references/worker-feedback-prompt.md b/skills/execute/references/worker-feedback-prompt.md index 308d137..7f76c61 100644 --- a/skills/execute/references/worker-feedback-prompt.md +++ b/skills/execute/references/worker-feedback-prompt.md @@ -1,13 +1,13 @@ -# Implementation Feedback Prompt Template +# Worker Feedback Prompt Template Use this template when returning review findings to the active worker subagent in the `execute` skill. ````markdown **You are a subagent completing work in the Propulsion workflow.** -Your implementation has been reviewed by an independent reviewer. Below is their report, including any findings and suggestions for improving your implementation. +Your work has been reviewed. Verify each review finding as a technical claim against the real plan, codebase, diff, checks, and workflow rules. Fix valid findings, reject invalid findings with evidence, and escalate unclear findings. -## Implementation Review Report +## Review Report @@ -15,16 +15,17 @@ Your implementation has been reviewed by an independent reviewer. Below is their Follow these steps IN ORDER. Do NOT skip steps. -1. Review the full implementation review report to understand the reviewer's findings and suggestions. -2. Treat each review finding as a technical claim to verify in the real codebase, not as an instruction to obey blindly. -3. Triage every finding as `valid`, `invalid`, or `unclear` before making any change. -4. If any finding remains `unclear` after inspecting the plan, codebase, diff, and available evidence, STOP and report `Status: unclear` with a request for specific additional information needed to triage the finding. -5. For every `valid` finding, make the minimal correct fix. -6. For every `invalid` finding, keep the code as-is and prepare evidence-backed pushback. -7. Load the `tdd` skill NOW if it is not already loaded, and follow the guidance when implementing the fixes. -8. Verify your implementation works as intended. -9. Self-review your implementation against the task description and acceptance criteria. -10. Return a report in the exact format defined below. +1. Review the current phase details directly from `plan.md`. +2. Review the full implementation review report. +3. Inspect the relevant plan sections, changed files, current diff, checks, and codebase context needed to evaluate the review findings. +4. Triage every review finding as `valid`, `invalid`, or `unclear` before making any code change. +5. If any finding remains `unclear` after investigation, STOP and return `Status: unclear` with the specific information needed to continue. +6. For every `invalid` finding, keep the implementation unchanged and prepare evidence-backed pushback. +7. For every `valid` finding, load the `tdd` skill, follow it, and make the minimal correct fix. +8. Load any additional recommended skills when needed to validate or fix the reviewed work. +9. Verify the implementation works and conforms to the current phase in `plan.md`. +10. Re-evaluate every current-phase acceptance criterion by ID. +11. Return your implementation report in the exact format below. ## Output @@ -35,9 +36,16 @@ Use this exact format for your output. **Status**: +**Review Feedback Triage**: + +- + - Classification: + - Resolution: + - Evidence: + **What Changed**: -- +- **Checks Run**: @@ -46,46 +54,49 @@ Use this exact format for your output. **Files Changed**: -- - -**Self-Review Findings**: - -- +- **Acceptance Criteria Status**: -- : +- : - Evidence: - -**Review Feedback Triage**: - -- - - Classification: - - Resolution: - - Evidence: ``` ## Rules These rules are MANDATORY. -- ALWAYS Triage and classify every finding into `valid`, `invalid`, or `unclear` before changing any code. -- ALWAYS ask questions if any finding is unclear. Do NOT guess or make assumptions. -- DO keep pushback technical and evidence-based. Do NOT make it personal or emotional. +- NEVER treat reviewer findings as automatically correct; verify each finding against the real implementation. +- ALWAYS triage every review finding as `valid`, `invalid`, or `unclear` before changing code. +- Status MUST be `unclear` if any finding cannot be triaged after inspecting the plan, codebase, diff, checks, and available evidence. +- Status MUST be `blocked` if a valid finding cannot be fixed because of missing access, failing tooling, contradictory requirements, or another blocker. +- Status CAN ONLY be `done` when every finding is resolved, every valid finding is fixed, every invalid finding has evidence-backed pushback, and every acceptance criterion is re-evaluated. +- DO NOT change code for invalid findings. +- DO NOT make speculative changes beyond the current phase or review findings. +- ALWAYS load and use the `tdd` skill before fixing valid findings. +- MUST verify implementation against the plan before claiming `Status: done`. +- ENSURE pushback is technical, evidence-based, and specific enough for the reviewer to verify or challenge. +- ALWAYS follow the output structure and section order exactly as specified. ## Completion Gate Do NOT output your response until ALL items are complete. +- [ ] Current phase details reviewed directly from `plan.md`. +- [ ] Full implementation review report reviewed. +- [ ] Relevant plan sections, changed files, current diff, checks, and codebase context inspected. - [ ] Every review finding triaged as `valid`, `invalid`, or `unclear` before coding. -- [ ] Every `valid` finding fixed. +- [ ] Every `valid` finding fixed using the `tdd` skill. - [ ] Every `invalid` finding answered with evidence-backed pushback. - [ ] Any unresolved `unclear` finding surfaced through `Status: unclear`. -- [ ] Relevant verification checks rerun after changes. +- [ ] Any unresolved implementation blocker surfaced through `Status: blocked`. +- [ ] Relevant verification checks rerun after changes where feasible. +- [ ] Every current-phase acceptance criterion re-evaluated by ID with evidence. +- [ ] Output implementation report in the exact format specified. ```` ## Rules These rules are MANDATORY. -- Replace all placeholders with the real implementation review report before sending this prompt. +- ALWAYS paste the full review report into `Review Report` before dispatching the worker. diff --git a/skills/execute/references/worker-prompt.md b/skills/execute/references/worker-prompt.md index ffd4e25..0aebfcd 100644 --- a/skills/execute/references/worker-prompt.md +++ b/skills/execute/references/worker-prompt.md @@ -7,22 +7,22 @@ Use this template when starting a fresh worker subagent in the `execute` skill. You are a senior software engineer implementing the current phase of work defined below. -## Task Description +## Task Context -**Title**: - - +**Current phase**: "> +**Plan document location**: `docs/propulsion/.../plan.md` ## Instructions Follow these steps IN ORDER. Do NOT skip steps. -1. Review the task description above and ask any clarifying questions if the requirements, scope, or repo state are unclear. Do not guess or make assumptions. -2. Load the `tdd` skill NOW and follow it. -3. Implement the work for this phase as defined in the task description. -4. Verify your implementation works as intended. -5. Self-review your implementation against the task description and acceptance criteria. -6. Return a report in the exact format defined below. +1. Review the current phase details in `plan.md`. +2. Load any recommended skills for this phase immediately. +3. Gather additional context from the codebase, PRD, and any relevant tools as needed. +4. Ask any clarifying questions if the requirements, scope, or repo state are unclear. +5. Load the `tdd` skill and follow it to implement the requirements. +6. Verify your implementation works and conforms to the plan. +7. Return your implementation report in the exact format below. ## Output @@ -46,13 +46,9 @@ Use this exact format for your output. - -**Self-Review Findings**: - -- - **Acceptance Criteria Status**: -- : +- : - Evidence: ``` @@ -60,24 +56,28 @@ Use this exact format for your output. These rules are MANDATORY. -- ALWAYS ask questions if anything in the task description is unclear. Do NOT guess or make assumptions. -- DO NOT write any code before fully understanding the requirements, scope, and repo state for the current phase. +- MUST start by reviewing the current phase details in `plan.md`. +- ALWAYS load any recommended skills and gather additional context before asking questions or implementing. +- ALWAYS ask questions if anything in the task is unclear, NEVER guess or make assumptions. - ALWAYS load and use the `tdd` skill. -- ALWAYS check for relevant non-Propulsion skills and load them IMMEDIATELY. -- Propulsion skills and workflow MUST take precedence over any conflicting non-Propulsion skill UNLESS the user instructions state otherwise. -- Follow the instructions and output format EXACTLY as defined above. +- MUST verify implementation against the plan before claiming `Status: done`. +- ALWAYS follow the output structure and section order exactly as specified. ## Completion Gate Do NOT output your response until ALL items are complete. -- [ ] Task description is fully understood with no unclear requirements, scope, or repo state. -- [ ] Implementation is complete and verified to work as intended. -- [ ] Self-review is complete and any issues are documented in the report. +- [ ] Reviewed the current phase details in `plan.md`. +- [ ] Loaded any recommended skills and gathered additional context. +- [ ] Asked clarifying questions for any unclear requirements, scope, or repo state. +- [ ] Followed the `tdd` skill to implement the requirements. +- [ ] Verified implementation works and conforms to the plan. +- [ ] Output the implementation report in the exact format specified. ```` ## Rules These rules are MANDATORY. -- Replace all placeholder context in the task description with the actual current-phase details before dispatching. +- MUST copy and paste the correct phase number and title from the plan. +- ALWAYS replace the plan path with the actual path for the plan being implemented. diff --git a/skills/interrogate/SKILL.md b/skills/interrogate/SKILL.md index a1c876c..880374c 100644 --- a/skills/interrogate/SKILL.md +++ b/skills/interrogate/SKILL.md @@ -1,6 +1,5 @@ --- name: interrogate -# prettier-ignore description: Manage interrogation, intake, interviews, scope clarification, requirements gathering, and shared understanding. Use when missing decisions must be resolved. --- @@ -12,11 +11,11 @@ Reach a shared understanding by exploring the project for context and interrogat Follow these steps IN ORDER. Do NOT skip steps. -1. Launch a fresh explorer subagent to inspect available project context for facts relevant to the request. +1. Gather context by launching a fresh explorer subagent to inspect the project for facts relevant to the request. 2. Interrogate the user relentlessly about every aspect of the request until a shared understanding is reached. - Ask questions one at a time, provide your recommended answer first, then 2-3 viable alternatives. - Walk down each branch of the decision tree resolving dependencies between decisions. - - There are no limits on the number of questions; keep asking until shared understanding is reached. + - There are no limits on the number of questions; keep asking until a shared understanding is reached. 3. Return a concise summary to the caller. ## Rules @@ -25,10 +24,10 @@ These rules are MANDATORY. - MUST use explorer subagent for entry exploration. - ALWAYS relentlessly interrogate the user until a shared understanding is reached. -- DO NOT limit the number of questions; keep asking until every blocking branch is closed. +- DO NOT limit the number of questions; keep asking until EVERY blocking branch is closed. - MUST ask user exactly one question at a time, provide a recommended answer, then 2-3 viable alternatives. - ALWAYS check if a question can be answered by project inspection before asking. -- MUST walk the decision tree until every blocking branch is closed by project facts or user answers. +- MUST walk the decision tree until EVERY blocking branch is closed by project facts or user answers. ## Completion Gate diff --git a/skills/interrogate/references/interrogate-protocol.md b/skills/interrogate/references/interrogate-protocol.md index 0a5ff1f..105c0c1 100644 --- a/skills/interrogate/references/interrogate-protocol.md +++ b/skills/interrogate/references/interrogate-protocol.md @@ -1,6 +1,6 @@ # Interrogate Protocol -Use this protocol when a workflow needs missing information resolved before it can proceed safely. +Use this protocol when a request needs missing information resolved before it can proceed safely. ## Entry Exploration diff --git a/skills/plan/SKILL.md b/skills/plan/SKILL.md index 1f7b445..0f5e1ae 100644 --- a/skills/plan/SKILL.md +++ b/skills/plan/SKILL.md @@ -1,6 +1,5 @@ --- name: plan -# prettier-ignore description: Create an implementation-ready feature plan from an approved PRD using phases scoped as thin vertical slices. Use when an approved `docs/propulsion/.../prd.md` exists. --- @@ -10,7 +9,7 @@ Turn an approved PRD into a phased implementation-ready plan using vertical slic ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. - If a `docs/propulsion/.../plan.md` already exists for this work, STOP. Ask the user whether to enter `execute`. - If no approved `docs/propulsion/.../prd.md` exists, STOP. Enter the `brainstorm` skill. @@ -19,46 +18,42 @@ ALL prerequisites MUST be true before following this skill. Follow these steps IN ORDER. Do NOT skip steps. -1. Review the approved `prd.md` to understand the requirements. +1. Review the approved `prd.md` to get a complete understanding of the feature. 2. Explore relevant areas of the codebase for fresh context. -3. Identify high-level durable decisions that are unlikely to change throughout implementation. -4. Identify narrow vertical slices that cut through all integration layers end-to-end. -5. Write `docs/propulsion/{yyyymmdd}-{feature-name}/plan.md` using the template in [references/plan-template.md](references/plan-template.md). -6. Start a plan review in a fresh subagent with the prompt in [references/plan-reviewer-prompt.md](references/plan-reviewer-prompt.md). -7. Review and implement feedback from the plan review. -8. Repeat steps 6 and 7 until the review returns `Status: approved`. -9. Tell the user the plan is complete and ask whether to move to `execute`. +3. Load any non-Propulsion skills relevant to the PRD and use them to help inform the plan. +4. Write `docs/propulsion/{yyyymmdd}-{feature-name}/plan.md` using the template in [references/plan-template.md](references/plan-template.md). +5. Start a fresh plan review subagent with the prompt in [references/plan-reviewer-prompt.md](references/plan-reviewer-prompt.md). +6. Review and implement feedback from the plan review. +7. Repeat steps 5 and 6 until the review returns `Status: approved`. +8. Ask the user to review and approve the plan. +9. After explicit approval, enter the `execute` skill. ## Rules These rules are MANDATORY. -- MUST output the `plan.md` in the exact shape specified in the template reference. -- DO NOT output the plan in the chat, ONLY in the `plan.md` file. -- MUST keep phases thin, ordered, and implementation-ready. -- DO NOT lose ANY information from the PRD that is relevant to implementation; lost information risks misalignment and rework. -- MUST treat `Status: approved` as the only valid approval signal for implementation readiness. -- MUST treat `findings` as fixable plan issues inside `plan`. -- MUST apply `suggestions` when they make implementation smoother. -- DO NOT invent missing product decisions. -- DO NOT auto-start `execute`; ALWAYS ask the user whether to proceed. +- NEVER skip reviewing the PRD, exploring the codebase, or loading relevant skills. +- ALWAYS use the plan template reference for structure, section order, and completion rules. +- NEVER print the full plan in the chat, ONLY write it to the file. +- MUST use ALL information from the PRD, DO NOT leave any details out even if they seem obvious or minor. +- USE `Status: approved` as the ONLY valid review approval signal. +- MUST treat review `findings` as fixable issues and `suggestions` as helpful improvements. +- NEVER invent product decisions that are not in the PRD; if a decision is missing, enter `brainstorm` to resolve it before planning. ## Completion Gate Do NOT leave this skill until ALL items are complete. -- [ ] PRD reviewed. -- [ ] Relevant codebase areas explored for fresh context. -- [ ] `plan.md` written to `docs/propulsion/.../plan.md`. -- [ ] Plan review by a subagent returns `Status: approved`. -- [ ] User asked whether to move to `execute`. +- [ ] Reviewed the PRD, codebase, and relevant skills. +- [ ] Written plan to `docs/propulsion/.../plan.md`. +- [ ] Plan reviewed by a subagent which returned `Status: approved`. +- [ ] User has explicitly approved `plan.md`. ## Next Steps Once the completion gate is fully checked: -- If the user says to proceed, enter the `execute` skill. -- If product intent is missing, enter the `brainstorm` skill. +- If `prd.md` is approved, enter the `execute` skill. ## References diff --git a/skills/plan/references/plan-reviewer-prompt.md b/skills/plan/references/plan-reviewer-prompt.md index a2a30bd..eb95b28 100644 --- a/skills/plan/references/plan-reviewer-prompt.md +++ b/skills/plan/references/plan-reviewer-prompt.md @@ -1,28 +1,33 @@ # Plan Reviewer Prompt Template -Use this template when starting plan review in a fresh subagent from the `plan` skill. +Use this template when starting a fresh plan review subagent in the `plan` skill. ````markdown **You are a subagent completing work in the Propulsion workflow.** -You are a plan document reviewer. Verify that the plan is implementation ready. +You are a plan document reviewer. Verify that the plan is implementation ready and conforms to the plan template. -**Plan document location**: `docs/propulsion/{yyyymmdd}-{feature-name}/plan.md` +**Plan document location**: `docs/propulsion/.../plan.md` **Source PRD location**: `docs/propulsion/.../prd.md` ## Review Criteria -| Category | What to verify | -| ------------------------- | --------------------------------------------------------------------------------------------------------------------- | -| Source Alignment | The plan preserves all information from the PRD within the relevant phases, no information is lost. | -| Behaviour Coverage | The end-to-end user behaviour is covered coherently; no important flow, transition, or outcome is missing. | -| Scope Control | Required work is included, and speculative or non-goal work is excluded. | -| Phase Design | Phases are thin vertical slices delivering narrow end-to-end behaviour, not horizontal layers or vague milestones. | -| Sequencing & Dependencies | Phase order is workable, dependencies are respected, and the plan will not force rework or dead ends later. | -| Phase Specificity | Each phase gives the worker enough context to act: clear goal, likely areas, constraints, and acceptance criteria. | -| Decision Hygiene | Durable decisions are captured once at the right level, with no contradictions or re-litigation points across phases. | - -Flag only issues that would cause implementation to build the wrong thing, get stuck, or need re-planning. +| Category | What to verify | +| ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Source Alignment | The plan preserves all relevant PRD decisions, implementation inputs, testing decisions, constraints, and scope boundaries. | +| Requirements Traceability | Every PRD user story, functional requirement, and non-functional requirement appears in the Requirements Coverage Matrix and is covered by at least one phase and one acceptance criterion. | +| Acceptance Criteria Coverage | Acceptance criteria use exact PRD IDs, cover the mapped requirements, are observable/testable, and are specific enough to verify implementation. | +| Vertical Slice Design | Phases are thin vertical slices delivering narrow end-to-end behaviour, not horizontal layers or vague milestones. | +| Phase Completeness | Each phase includes enough schema, backend, API, UI, validation, authorisation, testing, logging, documentation, or other relevant layer work to deliver its stated behaviour. | +| Skills Coverage | Relevant skills are recommended globally and per phase, no obviously required skill is missing, and no irrelevant skill is recommended. | +| Testing Coverage | Each phase has a testing plan that validates its acceptance criteria and covers the important public behaviours and seams. | +| Scope Control | Required work is included, speculative work is excluded, and no product behaviour is invented beyond the approved PRD. | +| Sequencing & Dependencies | Phase order is workable, dependencies are respected, and the plan will not force avoidable rework or dead ends later. | +| Phase Specificity | Each phase gives the worker enough context to act: clear goal, demo / verification outcome, likely areas, constraints, implementation notes, acceptance criteria, and testing plan. | +| Decision Hygiene | Durable decisions are captured once at the right level, with no contradictions or re-litigation points across phases. | +| Template Conformity | The plan follows the required template structure, section order, tables, and conventions. | + +Flag only issues that would cause implementation to build the wrong thing, miss required scope, get stuck, or need re-planning. ## Instructions @@ -68,28 +73,28 @@ Use this exact format for your output. These rules are MANDATORY. -- Replace the plan and PRD paths with the actual paths for the plan being reviewed. -- ALWAYS check for relevant non-Propulsion skills and load them IMMEDIATELY. -- Propulsion skills and workflow MUST take precedence over any conflicting non-Propulsion skill UNLESS the user instructions state otherwise. +- NEVER skip any part of the review criteria +- ENSURE every part of the PRD is considered in the review, even if it seems obvious or minor. +- EVERY PRD user story, functional requirement, and non-functional requirement MUST be traceable to at least one phase and one acceptance criterion in the plan. - MUST return exactly one `Status:` line with either `approved` or `rejected`. - Status CAN be `approved` if there are only suggestions but NO findings. -- If returning `Status: rejected`, MUST include at least one finding. -- Findings MUST be specific issues that would block or misdirect the next stage, not vague or general feedback. -- Suggestions are optional, but if included, MUST be specific improvements that would increase implementation readiness. -- Be specific about which phase or section each finding or suggestion relates to, and why it matters for implementation readiness. -- DO NOT update the plan document or source PRD, only review and provide feedback in this output. +- Status MUST be `rejected` if there are ANY findings. +- NEVER update the plan document or source PRD, only review and provide feedback in this output. +- ALWAYS follow the output structure and section order exactly as specified. ## Completion Gate Do NOT output your response until ALL items are complete. -- [ ] Plan reviewed against source PRD. -- [ ] Findings and suggestions are categorised according to the criteria. +- [ ] Thoroughly reviewed Plan against source PRD. +- [ ] Used the review criteria to identify issues and improvements. +- [ ] Categorised issues as findings or suggestions based on their impact on implementation readiness. - [ ] Status is set to `approved` if no findings, or `rejected` if there are any blocking issues. +- [ ] Output review report in the exact format specified. ```` ## Rules These rules are MANDATORY. -- Replace the plan and PRD paths with the actual paths for the plan being reviewed. +- ALWAYS replace the plan and PRD paths with the actual paths for the plan being reviewed. diff --git a/skills/plan/references/plan-template.md b/skills/plan/references/plan-template.md index c700049..7e66bdf 100644 --- a/skills/plan/references/plan-template.md +++ b/skills/plan/references/plan-template.md @@ -1,68 +1,164 @@ # Plan Template -Write `docs/propulsion/{yyyymmdd}-{feature-name}/plan.md` using this shape. +Write `docs/propulsion/{yyyymmdd}-{feature-name}/plan.md` using this exact section order. ```md -# Plan: +# Plan > Source PRD: `docs/propulsion/.../prd.md` -## For Agentic Coders - Use the `execute` skill to implement this plan and track progress using the checkboxes. ## Durable Decisions +List decisions that apply globally across all phases. + - Decision 1 - Decision 2 -## Phase 1: +## Relevant Skills + +List every agentic skill that may be needed while implementing this plan. + +| Skill | Required For | Details | +| -------------- | ---------------- | ------------------------------------------ | +| `` | Phase 1, Phase 3 | Declares how UI components should be used. | + +## Requirements Coverage Matrix + +Every PRD user story, functional requirement, and non-functional requirement MUST be represented here. + +Use exact IDs from the PRD. Do NOT rename, merge, or invent IDs. + +| PRD ID | Type | Covered By Phase(s) | Covered By Acceptance Criteria | Notes | +| ------- | -------------------------- | ------------------- | ------------------------------ | ----- | +| US-001 | User Story | Phase 1 | AC-001, AC-002 | | +| FR-001 | Functional Requirement | Phase 1 | AC-001 | | +| NFR-001 | Non-Functional Requirement | Phase 1 | AC-002 | | + +## Phase 1: **Status**: [ ] Phase complete -**Goal**: Concise description of the vertical slice, describing the narrow end-to-end behaviour that this phase will implement and test. +**Goal**: Describe the narrow end-to-end behaviour this phase implements. The goal MUST describe a complete user-visible, system-visible, or test-verifiable outcome. + +**Demo / Verification Outcome**: Describe exactly how someone can verify this phase is complete without inspecting implementation details. + +**Skills To Load**: + +| Skill | Why This Phase Needs It | +| -------------- | ----------------------- | +| `` | `` | + +**Likely Areas**: + +Use exact file paths only when they are durable and important. Prefer directories or modules when exact files may change. + +- `src/...` +- `tests/...` + +**Constraints**: -**Supporting Context**: Any important implementation inputs from the PRD that are relevant to this phase. +List the durable constraints this phase MUST respect. -**User Stories**: +- Constraint 1 +- Constraint 2 -1. As a , I want , so that +**Implementation Notes**: -**Likely Areas**: `src/...`, `tests/...` +Provide enough implementation context that a subagent can start work without rediscovering scope. Do NOT invent product decisions that are absent from the PRD. -**Constraints**: Durable rules that this phase must respect +- Note 1 +- Note 2 **Acceptance Criteria**: -- [ ] Criterion 1 -- [ ] Criterion 2 +Each acceptance criterion MUST reference at least one PRD user story, functional requirement, or non-functional requirement unless the criterion is purely functional and no NFR applies. -## Phase 2: +| ID | Acceptance Criterion | User Story ID(s) | Functional Requirement ID(s) | Non-Functional Requirement ID(s) | +| ------ | ------------------------------------------------------------ | ---------------- | ---------------------------- | -------------------------------- | +| AC-001 | Given , when , then . | US-001 | FR-001 | NFR-001 | +| AC-002 | Given , when , then . | US-001 | FR-002 | NFR-002 | +| AC-003 | can . | US-001 | FR-001, FR-002 | NFR-001 | + +**Testing Plan**: + +Describe the public behaviours and seams that MUST be tested for this phase. + +| Test Level | Required Coverage | +| --------------------- | ----------------- | +| Unit | | +| Feature / Integration | | +| Browser / UI | | +| Regression | | +| Manual Verification | | + +## Phase 2: **Status**: [ ] Phase complete -**Goal**: Concise description of the vertical slice, describing the narrow end-to-end behaviour that this phase will implement and test. +**Goal**: Describe the narrow end-to-end behaviour this phase implements. The goal MUST describe a complete user-visible, system-visible, or test-verifiable outcome. + +**Demo / Verification Outcome**: Describe exactly how someone can verify this phase is complete without inspecting implementation details. + +**Skills To Load**: + +| Skill | Why This Phase Needs It | +| -------------- | ----------------------- | +| `` | `` | + +**Likely Areas**: -**Supporting Context**: Any important implementation inputs from the PRD that are relevant to this phase. +Use exact file paths only when they are durable and important. Prefer directories or modules when exact files may change. -**User Stories**: +- `src/...` +- `tests/...` -1. As a , I want , so that +**Constraints**: -**Likely Areas**: `src/...`, `tests/...` +List the durable constraints this phase MUST respect. -**Constraints**: Durable rules that this phase must respect +- Constraint 1 +- Constraint 2 + +**Implementation Notes**: + +Provide enough implementation context that a subagent can start work without rediscovering scope. Do NOT invent product decisions that are absent from the PRD. + +- Note 1 +- Note 2 **Acceptance Criteria**: -- [ ] Criterion 1 -- [ ] Criterion 2 +Each acceptance criterion MUST reference at least one PRD user story, functional requirement, or non-functional requirement unless the criterion is purely functional and no NFR applies. + +| ID | Acceptance Criterion | User Story ID(s) | Functional Requirement ID(s) | Non-Functional Requirement ID(s) | +| ------ | ------------------------------------------------------------ | ---------------- | ---------------------------- | -------------------------------- | +| AC-004 | Given , when , then . | US-001 | FR-001 | NFR-001 | +| AC-005 | Given , when , then . | US-001 | FR-002 | NFR-002 | +| AC-006 | can . | US-001 | FR-001, FR-002 | NFR-001 | + +**Testing Plan**: + +Describe the public behaviours and seams that MUST be tested for this phase. + +| Test Level | Required Coverage | +| --------------------- | ----------------- | +| Unit | | +| Feature / Integration | | +| Browser / UI | | +| Regression | | +| Manual Verification | | ``` ## Rules -- Prefer many thin phases over few thick ones. -- Keep durable decisions out of the phases when they apply globally. -- Use exact file paths only when they are durable and important. -- Copy the relevant `**User Stories**:` into each phase from the approved PRD. -- Give each phase enough context that a subagent does not need to rediscover scope. +These rules are MANDATORY. + +- MUST decompose the approved PRD into thin vertical slices (tracer bullets). +- Each phase MUST cut through every integration layer needed for that behaviour. +- PREFER many thin vertical phases over few thick phases. +- ONLY identify skills relevant to each phase by their description, DO NOT load the skills now. +- ENSURE every PRD user story, functional requirement, and non-functional requirement is covered in the Requirements Coverage Matrix. +- NEVER invent product decisions, business rules, UX behaviour, or edge-case handling not present in the approved PRD. +- EVERY acceptance criterion MUST have a unique ID. diff --git a/skills/pr/SKILL.md b/skills/pr/SKILL.md index fa173ab..45efeef 100644 --- a/skills/pr/SKILL.md +++ b/skills/pr/SKILL.md @@ -9,7 +9,7 @@ Create or reuse one GitHub pull request from the current branch and report the v ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. - GitHub CLI `gh` is installed and authenticated for the target repository (may need to run outside sandbox). - The current directory is a git repository with an `origin` remote. diff --git a/skills/propulsion/SKILL.md b/skills/propulsion/SKILL.md index b4944fb..27f6869 100644 --- a/skills/propulsion/SKILL.md +++ b/skills/propulsion/SKILL.md @@ -1,7 +1,6 @@ --- name: propulsion -# prettier-ignore -description: Execute structured AI software development from planning through execution. Use when implementing or updating code, debugging issues, or starting repo software work. +description: Execute structured AI software development from planning through execution. Use when implementing or updating code, debugging issues, or starting software work. --- # Propulsion @@ -20,29 +19,36 @@ ONCE YOU ARE FOLLOWING PROPULSION WORKFLOW, DO NOT LEAVE IT UNTIL COMPLETION. DO Follow these steps IN ORDER. Do NOT skip steps. -1. Determine whether the request is software work before clarifying questions, repo scans, external actions, or downstream Propulsion stages. +1. Determine whether the request is software work. 2. If it is not software work, ignore Propulsion and respond normally. 3. If it is a concrete failure, emit `Propulsion workflow enabled, routing to debug...`, then load and follow `debug` skill. 4. If it is feature or product-scope work, emit `Propulsion workflow enabled, routing to brainstorm...`, then load and follow `brainstorm` skill. -5. Stop routing. The loaded Propulsion skill now owns the workflow stage. +5. The loaded Propulsion skill now owns the workflow stage. ## Rules These rules are MANDATORY. -- User instructions, repository rules, and `AGENTS.md` take priority, then Propulsion skills, then default system behaviour. -- Concrete failures route to `debug`: bug reports, regressions, failing tests, failing builds, runtime errors, crashes etc. -- Feature and product-scope work routes to `brainstorm`: new features, unclear scope, UX/product shaping, requirements discovery, behaviour changes, refactors, optimisations etc. -- If `propulsion` applies, route first. Do NOT reload `propulsion` or skip it because the task looks small, obvious, or familiar. -- Once following Propulsion workflow, DO NOT leave it until completion. ALWAYS follow the rules of each skill. -- ALWAYS route first. DO NOT rationalise skipping with thoughts like: "I need more context first", "I'll inspect the repo first", "This is too small for Propulsion". They are all FALSE. +- ALWAYS follow instructions in this order: + 1. User instructions are the highest priority (Direct requests or AGENTS.md, CLAUDE.md). + 2. Propulsion skills override default system behaviour. + 3. Default system behaviour is the lowest priority. +- NEVER route non-software-work request to Propulsion. +- ONLY route concrete failures to `debug` (bug reports, regressions, failing tests, failing builds, runtime errors, crashes). +- MUST route feature and product-scope work to `brainstorm` (new features, unclear scope, UX/product shaping, requirements discovery, behaviour changes, refactors, optimisations). +- ALWAYS fall back to `brainstorm` if the request is ambiguous. +- DO NOT leave a Propulsion skill until ALL completion gate items are complete. +- NEVER rationalise skipping Propulsion with thoughts like: + - "I need more context first" + - "I'll inspect the repo first" + - "This is too small for Propulsion" ## Completion Gate Do NOT leave this skill until ALL items are complete. - [ ] Determined whether the request is software work. -- [ ] Kept non-software-work chat outside Propulsion. +- [ ] Kept non-software-work outside Propulsion. - [ ] Routed concrete failures to `debug`. - [ ] Routed feature and product-scope work to `brainstorm`. - [ ] Emitted the route-specific required response before any other user-visible text. diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md index 6d48705..a47199d 100644 --- a/skills/review/SKILL.md +++ b/skills/review/SKILL.md @@ -9,7 +9,7 @@ Review a PR or ref range like a senior engineer and return a validated report. ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. - If review scope is unclear, STOP. Ask for a PR number, PR URL, or `base...head` ref range. - GitHub CLI `gh` is installed and authenticated when reviewing a PR number or PR URL. diff --git a/skills/tdd/SKILL.md b/skills/tdd/SKILL.md index 2340d7c..0f3e881 100644 --- a/skills/tdd/SKILL.md +++ b/skills/tdd/SKILL.md @@ -1,63 +1,59 @@ --- name: tdd -# prettier-ignore -description: Build observable behaviour one failing test at a time. Use when a slice changes user-facing behaviour, API contracts, or durable business logic. +description: Execute TDD red-green-refactor for behaviour changes. Use when changing observable behaviour, public contracts, or durable business logic. --- # TDD -Use red-green-refactor when a valuable behavioural test exists. +Drive behaviour changes with one failing behavioural test, minimal green code, then safe refactor. ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. -- The work includes a change to observable user-facing behaviour, a public contract, or durable business logic. -- The codebase has a test framework installed, and tests can be run locally. -- The work is not solely for CI-only changes, linting, formatting, dependency maintenance, build or development script changes, repo hygiene, or internal refactors with no behaviour change. - -If the work mixes behaviour change with tooling or maintenance updates, use `tdd` on the behaviour change ONLY. +- The task changes observable behaviour, a public contract, or durable business logic. +- A local test runner and relevant test command are available. If not, STOP and ask whether adding or fixing the test path is in scope. +- Maintenance-only work is invalid for TDD. STOP for docs/comments/prompts/spec text, styling-only UI changes, copy-only edits unless copy is the contract, config/build/dev-tool text edits, dependency bumps, generated files, data/schema migrations without logic changes, or pure refactors. +- If work mixes behaviour change with maintenance, apply TDD ONLY to the behaviour-changing slice. ## Instructions Follow these steps IN ORDER. Do NOT skip steps. -1. Choose the smallest thin vertical slice that delivers one observable behaviour end-to-end. -2. Apply the test-quality/applicability gate in [references/testing-patterns.md](references/testing-patterns.md) before writing or keeping a test. -3. If a valuable behavioural test exists, write one failing test for that behaviour through a public interface or stable seam, verify it fails for the expected reason, implement the smallest passing code, then verify it passes. -4. If no valuable behavioural test exists, document the no-test rationale, run the strongest appropriate fallback verification, implement the smallest change, then rerun fallback verification. -5. Repeat for the next behaviour until complete; refactor only while tests or fallback checks are green. Refer to [references/refactor-candidates.md](references/refactor-candidates.md). -6. For bug fixes, prefer a regression test that reproduces the bug; if none is valuable, document why and use the strongest fallback verification. +1. Choose the smallest thin vertical slice that delivers one observable behaviour end-to-end; state the interface, expected outcome, and narrowest test command. +2. Apply the gate in [references/testing-patterns.md](references/testing-patterns.md). If no valuable behavioural test exists, record the no-test rationale and strongest fallback verification before changing code. +3. Write ONE failing test through a public interface or stable seam for the next behaviour only. +4. Run the narrowest test command and confirm the test fails for the expected reason. +5. Write the minimum production code to pass; keep fixtures small and mock only real external, slow, unstable, or nondeterministic boundaries. +6. Re-run the narrowest test command and confirm green. +7. Review refactor candidates only after green using [references/refactor-candidates.md](references/refactor-candidates.md); refactor in small behaviour-preserving steps and rerun relevant checks. +8. Repeat slice by slice until the requested behaviour is complete. ## Rules These rules are MANDATORY. -- ONLY use `tdd` on observable user-visible behaviour or business logic changes. -- NO production code before a failing test WHEN a valuable behavioural test exists. -- DO NOT fabricate brittle tests when work cannot be proven through a public interface or stable seam. -- ALWAYS write ONE test at a time for ONE observable behaviour. -- ENSURE the test initially fails for the EXPECTED reason before writing production code. -- ONLY write the minimal amount of code to make the test pass. -- ALWAYS use the public interface for testing, and test through stable seams if necessary. -- NEVER write speculative, brittle, implementation-detail, or private-structure tests. -- ALWAYS document no-test rationale plus fallback verification when no valuable behavioural test exists. -- ALWAYS look for refactor opportunities AFTER the test is green. +- NEVER write production code before a failing test WHEN a valuable behavioural test exists. +- ALWAYS test observable behaviour through a public interface or stable seam. +- NEVER add source-text checks, private-structure checks, internal call choreography, broad snapshots, speculative tests, or implementation-detail tests as behavioural proof. +- DO NOT over-mock; ONLY mock real boundaries that are external, slow, unstable, nondeterministic, or too expensive for the selected test scope. +- STOP and ask if the behaviour, acceptance rule, stable seam, or relevant test command is unclear. +- NEVER refactor while red. +- ALWAYS prefer a regression test first for bug fixes. ## Completion Gate Do NOT leave this skill until ALL items are complete. - [ ] Work was implemented in thin vertical slices. -- [ ] Each slice passed the test-quality/applicability gate. -- [ ] Each testable slice started with a failing test that failed for the expected reason. -- [ ] Untestable slices documented no-test rationale and strongest appropriate fallback verification. -- [ ] Each slice was completed with passing tests or fallback checks. -- [ ] Where possible, refactors were applied after the tests were green. +- [ ] Each testable slice has red proof that failed for the expected reason, then green proof after the smallest implementation. +- [ ] Tests prove behaviour through a public interface or stable seam, with no brittle, speculative, implementation-detail, or over-mocked tests kept. +- [ ] No-test fallback rationale was documented only where no valuable behavioural test exists. +- [ ] Refactor opportunities were reviewed after green, and refactors happened only while checks were green. ## References Use these references when you need detail. -- [references/testing-patterns.md](references/testing-patterns.md) - Testing patterns for guidance on how to write effective tests. -- [references/refactor-candidates.md](references/refactor-candidates.md) - Refactor candidates to identify good opportunities for refactor after the tests are green. +- [references/testing-patterns.md](references/testing-patterns.md) - Test scope, behavioural seams, mocks, anti-patterns, fallback verification, and concise templates. +- [references/refactor-candidates.md](references/refactor-candidates.md) - Safe refactor candidates and post-green refactor gates. diff --git a/skills/tdd/references/refactor-candidates.md b/skills/tdd/references/refactor-candidates.md index 0138b67..0ff8dad 100644 --- a/skills/tdd/references/refactor-candidates.md +++ b/skills/tdd/references/refactor-candidates.md @@ -1,135 +1,114 @@ # Refactor Candidates -Use this guide after the tests are green. Make the current behaviour easier to understand, safer to change, and cheaper to extend for needs already proven by the tests. +Use this guide only after red-green is green. Refactor to remove present design pressure while preserving behaviour. -## Start Here +## When To Refactor -Refactor only when all relevant tests are passing. +Refactor when all relevant checks are green and you can name a real improvement. -Ask these gate questions before changing the structure: +Good reasons: -- Does the code or test suite show a present problem, not a hypothetical future one? -- Can I describe the refactor without changing behaviour? -- Is this the smallest change that removes the problem? -- Will the tests still verify the same behaviour through the same public interface? -- If I stop after this step, is the code already better? +- the next change is harder than it should be; +- a rule is duplicated and starting to drift; +- names hide the behaviour proven by tests; +- setup or tests are noisy because responsibilities are misplaced; +- branches or data shapes obscure the domain rule; +- a small move would reduce current risk or confusion. -If any answer is no, do less or stop. +Do one structural idea at a time. Stop when the current pain is removed. -## Good Refactor Signals - -### Duplication +## When Not To Refactor -Refactor when the same decision, transformation, or workflow appears in more than one place and the copies are starting to drift. +Do not refactor when: -Useful moves: +- tests or fallback checks are red; +- the improvement is hypothetical; +- the abstraction has one caller and no present pressure; +- you cannot describe the behaviour-preserving move; +- the code is awkward but isolated and not blocking current work; +- the refactor would expand scope beyond the requested slice. -- Extract a function for repeated logic. -- Move shared behaviour to the object or module that owns it. -- Introduce a small abstraction only after at least two real call sites need it. +Use YAGNI: reject abstractions for futures the code does not need today. -Do not refactor duplicated syntax alone. Refactor duplicated meaning. +## Signals To Spot -### Long Methods +### Duplicated Knowledge -Refactor when one method is hard to scan, mixes multiple decisions, or forces the reader to hold too much state in their head. +Look for the same decision, validation, calculation, workflow, or domain phrase in multiple places. -Useful moves: +Why it matters: copies drift and fixes land in only one place. -- Extract helpers around distinct steps. -- Name intermediate concepts so the flow reads at the domain level. -- Separate orchestration from detailed work. +Improve it by extracting the shared rule, moving it to the owner, or introducing a small abstraction only after real call sites need it. -Prefer small extractions that clarify intent. Do not split a method into many tiny helpers with no clearer names or boundaries. +### Mixed Responsibilities -### Poor Naming +Look for one function or module that validates, calculates, persists, formats, and coordinates at once. -Refactor when a name hides purpose, uses generic placeholders, or no longer matches the behaviour proven by the tests. +Why it matters: unrelated changes collide and tests need excessive setup. -Useful moves: +Improve it by separating orchestration from decisions, moving behaviour to the module with the strongest reason to change, and keeping coordinators thin. -- Rename variables, functions, types, and tests to describe the business meaning. -- Replace temporary or misleading names with names that explain the decision being made. +### Poor Names -Prefer better names before new abstractions. +Look for placeholders, abbreviations, stale names, or test names that describe mechanics instead of behaviour. -### Feature Envy +Why it matters: unclear names hide intent and make future changes slower. -Refactor when logic reaches repeatedly into another object or data structure to do work that clearly belongs there. +Improve it by renaming variables, functions, types, files, and tests to match domain meaning. -Useful moves: +### Long Or Tangled Flow -- Move behaviour closer to the data it uses. -- Replace chains of field access with a message to the owning object. +Look for deep nesting, repeated conditions, order-sensitive branches, or methods that require scrolling and memory. -This often improves cohesion and reduces knowledge spread. +Why it matters: bugs hide in paths readers cannot follow. -### Primitive Obsession +Improve it with guard clauses, extracted predicates, named steps, or split cases. Use polymorphism only after duplication makes the cases real. -Refactor when the same primitive values travel together, need repeated validation, or encode domain rules informally. - -Useful moves: - -- Introduce a small value object. -- Replace magic strings, flags, or loosely related parameters with a named concept. -- Move validation and formatting onto that concept. +### Feature Envy -Do this only when the domain concept already exists in the current behaviour. YAGNI still applies. +Look for logic that repeatedly pulls fields from another object to make decisions for it. -### Brittle Conditionals +Why it matters: behaviour lives away from the data it depends on. -Refactor when conditionals are nested, repeated, order-sensitive, or hard to extend without fear. +Improve it by moving the behaviour closer to the data or replacing field chains with messages to the owner. -Useful moves: +### Primitive Obsession -- Extract predicate functions with clear names. -- Flatten control flow with guard clauses. -- Separate distinct cases into focused helpers. -- Replace branching with polymorphism only when multiple concrete cases already exist and the branch is a real maintenance problem. +Look for strings, booleans, numbers, or loose parameter groups that repeatedly encode a domain concept. -Prefer the smallest change that makes the branch understandable today. +Why it matters: validation and meaning scatter across the codebase. -### Misplaced Responsibilities +Improve it with a small value object, enum, named type, or parameter object when the concept already has behaviour or repeated validation. -Refactor when a module has become a grab bag for unrelated work or when one change requires touching many places because ownership is unclear. +### Test Friction -Useful moves: +Look for tests that need heavy setup, many mocks, private seams, or fragile assertions to prove simple behaviour. -- Move behaviour to the module with the strongest reason to change. -- Keep one unit focused on one kind of responsibility. -- Let high-level code coordinate and low-level code perform detailed work. +Why it matters: test pain often exposes design pain. -Use SOLID as a decision tool here. If one unit has multiple unrelated reasons to change, the design is likely fighting the current behaviour. +Improve production design first when it has real value; do not add test-only seams. -## When Not To Refactor +## Safe Moves -Do not refactor just because the code looks inelegant. +Prefer small behaviour-preserving moves: -Hold off when: +- rename; +- extract function or predicate; +- inline unnecessary indirection; +- move behaviour to its owner; +- split orchestration from domain rules; +- replace magic values with named concepts; +- collapse duplicated rules; +- simplify conditionals; +- replace partial mocks with realistic fakes when it improves design pressure. -- The tests are not green yet. -- You do not understand the problem well enough to name the improvement. -- The change is driven by a guessed future requirement. -- The abstraction would serve only one caller or one code path today. -- The code is awkward but stable, isolated, and not blocking current work. -- The tests are not giving enough confidence to separate structural change from behaviour change. +Run the narrowest relevant check after each meaningful move. If a check fails, fix or revert the last refactor step before continuing. -YAGNI matters most after green. If the current tests and code do not show the need, keep the simpler structure. +## Filters -## DRY, SOLID, YAGNI As Filters +Use these filters before changing structure: - DRY: remove repeated knowledge, not every repeated line. -- SOLID: prefer clearer ownership and narrower responsibilities when the current design already shows those pressures. -- YAGNI: reject abstractions for possibilities the code does not yet need. - -Use these principles to choose less code and better boundaries, not to justify a larger rewrite. - -## Safe Refactor Rules - -- Keep refactors behaviour-preserving. -- Change one structural idea at a time. -- Run the relevant tests after each meaningful step. -- Keep tests aimed at public behaviour so they survive the refactor. -- Stop once the current pain is removed. - -The best refactor is usually the smallest one that makes the next change easier. +- SOLID: improve ownership only where current design already shows pressure. +- YAGNI: do not build for imagined futures. +- Refactor-safe tests: existing tests should still prove the same behaviour after the move. diff --git a/skills/tdd/references/testing-patterns.md b/skills/tdd/references/testing-patterns.md index 556e5ae..dc1e61c 100644 --- a/skills/tdd/references/testing-patterns.md +++ b/skills/tdd/references/testing-patterns.md @@ -1,273 +1,149 @@ # Testing Patterns -Use this reference during red-green. Choose tests that prove behaviour, not today's implementation. +Use this reference before writing or keeping a TDD test. Keep tests behavioural, refactor-safe, and cheap enough to run during red-green. -**Refactor-safe tests are the standard:** good tests keep passing when internals are rewritten but observable behaviour stays the same. Test through public interfaces or stable seams with domain meaning, and avoid assertions about private helpers, call order, source shape, or other implementation details. +## When TDD Applies -## Default Move +Use TDD when the task changes observable behaviour, a public contract, or durable business logic. -Start with the highest-level public interface that proves the behaviour cheaply. +Do not force TDD for prose-only docs, comments, prompts, formatting, config text, dependency bumps, generated files, or pure refactors with no behaviour change. Record the no-test rationale and run fallback verification instead. -- Prefer the API a caller, user, or upstream module actually uses. -- Drop to a lower seam only when the public path is too slow, too broad, or too hard to control. -- If you drop lower, choose a stable seam with business meaning, not a private helper. +If a task mixes behaviour with maintenance, TDD only the behaviour-changing slice. -```typescript -// Good: public interface, observable result -test('creates a retrievable user', async () => { - const user = await createUser({ name: 'Adam' }); - const retrievedUser = await getUser(user.id); +## Test Type Choice - assertMatches(retrievedUser, { name: 'Adam' }); -}); +Choose the highest-level test that proves the behaviour while staying quick, deterministic, and cheap. -// Bad: reaches through internals -test('calls insertUserRow', async () => { - const insertUserRowCalls = recordCalls(dbModule, 'insertUserRow'); +1. Prefer feature or integration tests first. Test the public path a caller, user, endpoint, CLI, message handler, or upstream module uses. +2. Use unit tests second for isolated important logic, especially rules with many cases, edge conditions, or awkward setup through the full path. +3. Use browser or end-to-end tests sparingly for UI interaction patterns, smoke coverage, or behaviour that lower-level tests cannot prove. - await createUser({ name: 'Adam' }); +Drop lower only when the higher-level path is slow, flaky, too broad, or expensive to control. Drop to a stable seam with domain meaning, not a private helper. - assertEqual(insertUserRowCalls.count(), 1); -}); -``` +## Red-Green Test Quality -## Strong Test Patterns +Write one failing test for one missing behaviour. -### Test observable behaviour +Good red tests: -Assert on outputs, returned state, emitted events, persisted state visible through a supported read path, or externally visible side effects. +- name the behaviour in caller language; +- fail for the expected reason before production changes; +- tell you the next smallest code change; +- assert an observable outcome; +- avoid future requirements. -- Good: `checkout(cart)` returns `confirmed`. -- Good: `publishPost()` makes the post appear in `listPublishedPosts()`. -- Bad: `checkout()` called `paymentService.charge()` exactly once. +Weak red tests: -### Test through public interfaces +- assert helper calls, call order, source text, hook names, class names, or private state; +- require large mock choreography; +- fail when internals move but behaviour stays the same; +- cover hypothetical edge cases not requested now. -Use exported functions, HTTP endpoints, CLI commands, UI interactions, message handlers, or other supported entry points. +## Refactor-Safe Tests -- Good: `login(email, password)` rejects invalid credentials. -- Bad: `validatePasswordHash()` was called from inside `login()`. +Refactor-safe tests keep passing when internals are rewritten but behaviour stays the same. -### Choose one behaviour per test +Assert through public interfaces or stable seams: -Keep each test focused on one behaviour. Multiple assertions are fine when they describe one outcome from one cause. +- returned values; +- persisted state through supported reads; +- visible UI or announced accessibility output; +- emitted domain events; +- externally visible side effects. + +Avoid private helpers, hidden fields, internal modules, source-string checks, AST shape, broad snapshots, and internal call choreography. ```typescript -test('rejects checkout when cart is empty', async () => { +// Good: public behaviour +test('rejects checkout when the cart is empty', async () => { const result = await checkout(emptyCart()); assertEqual(result.ok, false); assertEqual(result.error, 'Cart is empty'); }); -``` - -Those assertions belong together because they describe one outcome. Split the test only when failures would point to different behaviours. - -### Prefer realistic setup over elaborate stubbing -Use simple builders, fixtures, or in-memory adapters when they keep the test closer to production behaviour. +// Bad: implementation detail +test('calls validateCart before createOrder', async () => { + const calls = recordCallOrder( + cartModule, + 'validateCart', + orderModule, + 'createOrder', + ); -- Prefer `createCart({ items: [book()] })` over six mocks. -- Prefer a fake mailer that records sent messages over mocking every mailer method. + await checkout(emptyCart()); -### Mock only at unstable or expensive boundaries - -Mock payment gateways, clocks, UUID generation, network calls, file systems, or other boundaries that are slow, nondeterministic, or outside the unit of behaviour. - -Keep the logic under test real. - -```typescript -// Good: keep business logic real, isolate time -test('marks subscription expired after due date', () => { - const clock = fakeClock(new Date('2026-04-10T12:00:00Z')); - const subscription = createSubscription({ endsAt: '2026-04-09T12:00:00Z' }); - - assertEqual(isExpired(subscription, { clock }), true); + assertSequence(calls, ['validateCart', 'createOrder']); }); ``` -### Prefer stable seams when public paths are too broad - -If the true public interface needs too much setup, drop to the narrowest stable seam that still has domain meaning. - -- Good seam: `PriceCalculator.calculate(order)`. -- Weak seam: `applyDiscountStep2()`. - -The seam should still represent behaviour another part of the system could reasonably depend on. - -## Anti-Patterns - -Reject tests that only inspect source strings, private structure, implementation details, brittle snapshots, or speculative behaviour. These are not acceptable substitutes for behavioural coverage; use fallback verification instead when no valuable behavioural test exists. - -### Implementation-detail tests - -These tests fail when refactoring changes structure without changing behaviour. - -Red flags: - -- Spying on private or internal collaborators. -- Asserting call counts, call order, or which helper ran. -- Reaching into private state not exposed by the interface. -- Naming tests after how the code works rather than what it does. - -```typescript -// Bad: proves structure, not behaviour -test('sends audit event after saving', async () => { - const events = recordCallOrder(repo, 'save', auditBus, 'emit'); - - await registerUser(input); - - assertSequence(events, ['save', 'emit']); -}); -``` - -Prefer a result that matters to a caller, such as the user being created and an audit entry being visible through a supported query. - -### Source-string and private-structure tests +## Good Tests -Do not read source files as strings or inspect private modules, hidden fields, AST shape, CSS class names, hook order, folder layout, or helper presence to prove behaviour. +Good tests prove a behaviour someone depends on. -- Bad: asserting a file contains `aria-label` or calls `useMemo()`. -- Better: render the UI and query the accessible control, or verify the public API result. +- Test through exported functions, endpoints, commands, UI interactions, handlers, or stable domain seams. +- Keep setup small and realistic. +- Use multiple assertions only when they describe one outcome from one cause. +- Prefer builders, fixtures, in-memory adapters, and real collaborators when they stay cheap. +- Make failures describe broken behaviour, not changed structure. -### Brittle snapshots +Examples: -Do not use broad snapshots for behaviour changes. Snapshots that mostly capture markup, class churn, generated IDs, timestamps, or component structure fail on harmless refactors. +- `login(email, password)` rejects invalid credentials. +- `publishPost()` makes the post visible in `listPublishedPosts()`. +- Clicking `Save` shows a success message. -- Bad: snapshotting an entire page to prove a button opens a menu. -- Better: interact as a user and assert the menu content is visible. +## Bad Tests -### Over-mocking +Reject tests that prove structure instead of behaviour. -If most of the test is mock setup, the test is probably proving that the mocks agree with each other. +- `checkout()` calls `paymentService.charge()` once. +- `login()` calls `validatePasswordHash()`. +- A component contains `aria-label` as source text. +- A broad snapshot proves a button opens a menu. +- A test exists only because an edge case might matter later. -Red flags: +Replace bad tests with behavioural assertions. If none exists, do not keep a weak test; document the no-test rationale and run fallback verification. -- Mock setup is longer than the assertions. -- You mocked every collaborator "just to be safe". -- Removing one mock changes the entire test shape. -- You cannot explain which boundary the mock isolates. +## Mocks And Doubles -Prefer a more integrated test with fewer doubles. +Mock only real boundaries that are external, slow, unstable, nondeterministic, or too expensive for the selected scope. -### Incomplete mocks +Good mock targets: -Partial mocks often pass until real code touches a field the fake omitted. +- payment gateways; +- clocks and time; +- UUID/randomness; +- network calls; +- file systems; +- third-party APIs. -```typescript -// Bad: omits fields real consumers may depend on later -const paymentResponse = { - status: 'approved', - transactionId: 'txn_123', -}; - -// Better: mirror the real shape completely -const paymentResponse = { - status: 'approved', - transactionId: 'txn_123', - currency: 'USD', - createdAt: '2026-04-10T12:00:00Z', - risk: { score: 12, decision: 'approve' }, -}; -``` - -When you must mock data, mirror the real schema, not just the subset your current assertion happens to use. - -### Mocking at the wrong level - -Do not mock away the very behaviour the test is meant to prove. - -- Bad: mocking `UserRepository.create()` in a test for "creates a user". -- Better: use a real repository with an in-memory store and mock the email provider if needed. - -### Speculative coverage - -Do not write tests for behaviour you have not been asked to deliver yet. +Keep core logic real. Prefer fakes that preserve behaviour, such as an in-memory repository or mailer that records sent messages. -Red flags: +Avoid over-mocking. If mock setup dominates the test, the test likely proves mocks agree with each other. If mock data is required, mirror the real schema enough that consumers cannot accidentally rely on missing fields. -- "We might need this edge case later." -- "I am already here, so I may as well add three more tests." -- Adding tests for options, flags, or failure modes with no current requirement. - -Speculative tests calcify design and create noise during refactor. Add the next test only when the next required behaviour appears. - -### Test-only production seams +## Anti-Patterns -Do not add public methods, flags, or branches only so tests can inspect internals. +Do not keep these as behavioural proof: -- Bad: `order._debugState()` added only for tests. -- Better: assert through existing outputs, or extract a real stable seam with production value. +- source-string checks; +- private-structure checks; +- internal call counts or order; +- broad snapshots for dynamic markup; +- test-only production flags or methods; +- partial hand-waved mocks; +- tests for speculative requirements; +- mocking away the behaviour under test. ## Gate Questions -Ask these questions before keeping a test: - -1. What behaviour does this test prove for a caller or user? -2. Would the test still pass if I rewrote the internals but kept behaviour the same? -3. Am I asserting through a public interface or a stable seam with domain meaning? -4. Is every mock isolating a real boundary rather than replacing core logic? -5. Does each mock preserve the parts of reality the test depends on? -6. If this test fails, will the failure describe a broken behaviour rather than a refactor? -7. Am I adding this test because of a current requirement, not because it feels thorough? - -If any answer is "no" or "I am not sure", simplify the test before proceeding. - -If no valuable behavioural test remains, do not keep a weak test. Document why no new test was written and run the strongest appropriate fallback verification, such as an existing related test suite, typecheck, lint, build, CLI smoke check, manual reproduction, or browser check. - -## Frontend Guidance - -For UI behaviour changes, prefer user-level tests that render the UI, interact through accessible controls, and assert visible or announced outcomes. - -- Good: click "Save" and assert the success toast appears. -- Bad: assert a component state setter was called or a specific class name exists. - -For visual-only changes or UI states that are hard to cover with valuable automated tests, prefer Playwright or browser verification when available. Capture the no-test rationale and the browser checks performed. - -## Red-Green Heuristics - -When choosing the next test: - -- Start with the smallest missing behaviour that changes what a caller can observe. -- Prefer one test that opens a path forward over a "complete" matrix up front. -- Make the failure message specific enough that you know what code to write next. -- Stop once the required behaviour is covered; breadth can come later if a requirement demands it. - -Good next tests during red-green: - -- "returns `not_found` for an unknown order id" -- "applies percentage discount before tax" -- "rejects duplicate email addresses" - -Weak next tests during red-green: - -- "calls helper with transformed payload" -- "invokes parser twice" -- "supports five hypothetical invalid payloads" when only one matters now - -## Failure Smells - -Pause and rewrite the test if: - -- The assertion mentions mocks more than domain outcomes. -- The setup requires deep knowledge of internal object graphs. -- A refactor with unchanged behaviour breaks many tests. -- The only way to verify success is to inspect a private field. -- The test title starts with "calls", "uses", "sets", or "invokes" instead of a behaviour. - -## Quick Comparison - -| Prefer | Avoid | -| -------------------------------- | ------------------------------ | -| Public interfaces | Private helpers | -| Observable outcomes | Internal call assertions | -| Minimal boundary mocks | Mocking core logic | -| Complete realistic test doubles | Partial hand-waved mock shapes | -| Required next behaviour | Speculative coverage | -| Stable seams with domain meaning | Test-only seams | - -## Bottom Line +Ask before writing or keeping a test: -Strong tests survive refactor because they describe behaviour. Weak tests break when code moves because they describe implementation. +1. What behaviour does this prove for a caller or user? +2. Would it pass after an internal rewrite with the same behaviour? +3. Is the assertion through a public interface or stable domain seam? +4. Is every mock isolating a real boundary? +5. Is this the next required behaviour, not a future guess? +6. Will failure point to broken behaviour rather than changed structure? -When in doubt, test what a caller can observe, keep mocks at real boundaries, and write only the next test needed to move red to green. +If any answer is no, rewrite the test. If no valuable behavioural test remains, document why and run the strongest fallback verification: existing related tests, typecheck, lint, build, CLI smoke check, browser check, or manual reproduction. diff --git a/skills/write-skill/assets/skill-template.md b/skills/write-skill/assets/skill-template.md index f218830..6d290e2 100644 --- a/skills/write-skill/assets/skill-template.md +++ b/skills/write-skill/assets/skill-template.md @@ -14,7 +14,7 @@ description: {One-line action-oriented summary with the main use case and trigge ## Prerequisites -ALL prerequisites MUST be true before following this skill. +ALL prerequisites MUST be satisfied BEFORE following this skill. - {Only include when the skill must stop, route, or require a condition before work starts.} diff --git a/skills/write-skill/scripts/validate-skill.js b/skills/write-skill/scripts/validate-skill.js index 7aa5fd3..8de4681 100644 --- a/skills/write-skill/scripts/validate-skill.js +++ b/skills/write-skill/scripts/validate-skill.js @@ -15,7 +15,7 @@ const allowedSections = [ ]; const sectionIntroLines = { Prerequisites: - 'ALL prerequisites MUST be true before following this skill.', + 'ALL prerequisites MUST be satisfied BEFORE following this skill.', Instructions: 'Follow these steps IN ORDER. Do NOT skip steps.', Rules: 'These rules are MANDATORY.', 'Completion Gate': 'Do NOT leave this skill until ALL items are complete.', diff --git a/tests/write-skill-validator.test.js b/tests/write-skill-validator.test.js index 1b91dce..257bd32 100644 --- a/tests/write-skill-validator.test.js +++ b/tests/write-skill-validator.test.js @@ -354,7 +354,7 @@ Start with another reference explanation. expect(result.status).toBe(1); expect(result.report.errors).toContain( - 'Start ## Prerequisites with: ALL prerequisites MUST be true before following this skill.', + 'Start ## Prerequisites with: ALL prerequisites MUST be satisfied BEFORE following this skill.', ); expect(result.report.errors).toContain( 'Start ## Instructions with: Follow these steps IN ORDER. Do NOT skip steps.',