diff --git a/skills/01-brainstorm/references/premise-challenge.md b/skills/01-brainstorm/references/premise-challenge.md index 81def2b..3f880d0 100644 --- a/skills/01-brainstorm/references/premise-challenge.md +++ b/skills/01-brainstorm/references/premise-challenge.md @@ -8,6 +8,11 @@ Before proposing solutions, challenge the premises. Run this after the diagnosti 2. **What happens if we do nothing?** Real pain point or hypothetical one? 3. **What existing code already partially solves this?** Map existing patterns, utilities, and flows that could be reused. 4. **If the deliverable is a new artifact** (CLI binary, library, package, container image, mobile app): **how will users get it?** Code without distribution is code nobody can use. +5. **External resource signal — verify intent before scope.** When the user mentions an existing resource ("I already have X", "I wrote a skill for Y", "we use Z elsewhere"), do not assume it means "include X in this project." It may mean the opposite: the problem is already solved and should be excluded. Reverse-verify with one question: "You mentioned X — do you want it incorporated here, or is it noting that X already handles this so we don't need to?" A wrong assumption here propagates through every downstream stage (plan, work, review) and surfaces only at merge — three gates that all trust the upstream. + +## Failure mode + +Skipping this check when the user references an external resource leads to scope creep in the opposite direction assumed: incorporating something the user considered already-handled. The later it surfaces, the costlier the rollback. ## Output format diff --git a/skills/04-review/SKILL.md b/skills/04-review/SKILL.md index b9c9225..945abce 100644 --- a/skills/04-review/SKILL.md +++ b/skills/04-review/SKILL.md @@ -24,7 +24,7 @@ See [shared pipeline instructions](../references/pipeline-config.md) for model r - Extract keywords → `grep -rl "tags:.*keyword" docs/solutions/ ~/.pi/agent/docs/solutions/` - Read **frontmatter** only (first 15 lines) of matches → score by severity + tag relevance - Fully read top 3 candidates -7. **Spec axis:** when a plan artifact exists, compare diff against it — report **missing** requirements, **scope creep** (unrequested behaviour), and **wrong implementation** (looks done but isn't). Skip if no plan. +7. **Spec axis:** when a plan artifact exists, compare diff against it — report **missing** requirements, **scope creep** (unrequested behaviour), and **wrong implementation** (looks done but isn't). Also **trace back** to the user's original wording (brainstorm scope) to catch directional misunderstandings the plan itself encoded. Skip if no plan. 8. Produce structured findings using `references/findings-schema.md` 9. **Autofixable findings:** apply and re-review (max 3 iterations) diff --git a/skills/04-review/references/reviewer-selection.md b/skills/04-review/references/reviewer-selection.md index c2bf99f..c41fc59 100644 --- a/skills/04-review/references/reviewer-selection.md +++ b/skills/04-review/references/reviewer-selection.md @@ -24,4 +24,4 @@ All reviewers evaluate changes across five axes: correctness, readability, archi - **performance-reviewer**: Triggered when query, cache, database, or streaming files change. Reviews for N+1, unnecessary allocation, missing indexes. - **integration-reviewer**: Triggered when CI/CD, Docker, package.json, or config files change. Reviews for dependency conflicts, build breakage, deployment issues. - **thoroughness-reviewer**: Triggered for large diffs (5+ files or 300+ lines). Reviews for incomplete refactors, missed callers, inconsistent changes. -- **spec-reviewer**: Triggered when a plan artifact exists. Reviews the diff against the plan — reports **missing** requirements, **scope creep** (behaviour in the diff not asked for), and **wrong implementation** (requirements that look done but aren't). Skip when no plan artifact is found. +- **spec-reviewer**: Triggered when a plan artifact exists. Reviews the diff against the plan — reports **missing** requirements, **scope creep** (behaviour in the diff not asked for), and **wrong implementation** (requirements that look done but aren't). Skip when no plan artifact is found. Additionally, **trace back to the user's original wording**: re-read the brainstorm artifact's scope statements and confirm the plan interpreted them correctly — a plan that faithfully implements a *misunderstood* requirement passes missing/scope-creep checks but still ships the wrong thing. Flag directional misunderstandings even if the diff matches the plan. diff --git a/tests/skill-contracts.test.ts b/tests/skill-contracts.test.ts index 9deb18e..9f2999a 100644 --- a/tests/skill-contracts.test.ts +++ b/tests/skill-contracts.test.ts @@ -367,9 +367,23 @@ describe("skill package contracts", () => { expect(content).toContain("Spec") expect(content).toContain("missing") expect(content).toContain("scope creep") + // Spec axis traces back to original wording, not just plan-vs-diff + expect(content).toContain("trace back") // reviewer-selection.md documents the spec-reviewer persona expect(reviewerSelection).toContain("spec-reviewer") expect(reviewerSelection).toContain("plan artifact") + expect(reviewerSelection).toContain("directional misunderstanding") + }) + + test("01-brainstorm premise-challenge verifies external-resource intent", () => { + const premise = readFileSync( + path.join(repoRoot, "skills", "01-brainstorm", "references", "premise-challenge.md"), + "utf8", + ) + + // External resource signal must be reverse-verified before scoping + expect(premise).toContain("External resource signal") + expect(premise).toContain("incorporated") }) test("out-of-scope knowledge base records rejected/already-built requests", () => {