diff --git a/.changeset/5-decision-coverage-xml-bodies.md b/.changeset/5-decision-coverage-xml-bodies.md new file mode 100644 index 000000000..3d771642e --- /dev/null +++ b/.changeset/5-decision-coverage-xml-bodies.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 5 +--- +**`check.decision-coverage-plan` gate now recognises D-NN citations inside ``, ``, ``, and `` XML tag bodies** — `extractPlanSections()` previously searched only front-matter (`must_haves`, `truths`, `objective`) and body lines under designated markdown headings. The `gsd-planner` spec directs agents to cite decision IDs inside `` bodies; those citations were invisible to the gate, causing plans that correctly followed the spec to report `passed: false` with all decisions uncovered. `extractXmlTagBodies()` now extracts inner text from the four canonical planner XML tags via a narrow regex (no parser library), and that text is appended to the `designated` search string. Self-closing tags and non-canonical tags are safely ignored. (#5) diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index d58483461..8df18de8c 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -64,6 +64,7 @@ The orchestrator provides user decisions in `` tags from `/gsd:d **Self-check before returning:** For each plan, verify: - [ ] Every locked decision (D-01, D-02, etc.) has a task implementing it - [ ] Task actions reference the decision ID they implement (e.g., "per D-03") + (The decision-coverage gate `check.decision-coverage-plan` reads D-NN citations from ``, ``, ``, and `` tag bodies, as well as markdown headings and front-matter `must_haves`/`truths`/`objective` keys — citing D-NN in any of these locations counts toward coverage.) - [ ] No task implements a deferred idea - [ ] Discretion areas are handled reasonably @@ -519,66 +520,7 @@ Wave numbers are pre-computed during planning. Execute-phase reads `wave` direct ## Interface Context for Executors -**Key insight:** "The difference between handing a contractor blueprints versus telling them 'build me a house.'" - -When creating plans that depend on existing code or create new interfaces consumed by other plans: - -### For plans that USE existing code: -After determining `files_modified`, extract the key interfaces/types/exports from the codebase that executors will need: - -```bash -# Extract type definitions, interfaces, and exports from relevant files -grep -n "export\\|interface\\|type\\|class\\|function" {relevant_source_files} 2>/dev/null | head -50 -``` - -Embed these in the plan's `` section as an `` block: - -```xml - - - - -From src/types/user.ts: -```typescript -export interface User { - id: string; - email: string; - name: string; - createdAt: Date; -} -``` - -From src/api/auth.ts: -```typescript -export function validateToken(token: string): Promise; -export function createSession(user: User): Promise; -``` - -``` - -### For plans that CREATE new interfaces: -If this plan creates types/interfaces that later plans depend on, include a "Wave 0" skeleton step: - -```xml - - Task 0: Write interface contracts - src/types/newFeature.ts - Create type definitions that downstream plans will implement against. These are the contracts — implementation comes in later tasks. - File exists with exported types, no implementation - Interface file committed, types exported - -``` - -### When to include interfaces: -- Plan touches files that import from other modules → extract those module's exports -- Plan creates a new API endpoint → extract the request/response types -- Plan modifies a component → extract its props interface -- Plan depends on a previous plan's output → extract the types from that plan's files_modified - -### When to skip: -- Plan is self-contained (creates everything from scratch, no imports) -- Plan is pure configuration (no code interfaces involved) -- Level 0 discovery (all patterns already established) +See `get-shit-done/references/planner-interface-context.md` for the full interface extraction guide. ## Context Section Rules diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 103f7edfd..5347b334e 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -226,6 +226,7 @@ "planner-gap-closure.md", "planner-graphify-auto-update.md", "planner-human-verify-mode.md", + "planner-interface-context.md", "planner-mvp-mode.md", "planner-reviews.md", "planner-revision.md", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index e92660080..9b1f3fa30 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -262,7 +262,7 @@ Full roster at `get-shit-done/workflows/*.md`. Workflows are thin orchestrators --- -## References (61 shipped) +## References (62 shipped) Full roster at `get-shit-done/references/*.md`. References are shared knowledge documents that workflows and agents `@-reference`. The groupings below match [`docs/ARCHITECTURE.md`](ARCHITECTURE.md#references-get-shit-donereferencesmd) — core, workflow, thinking-model clusters, and the modular planner decomposition. @@ -353,11 +353,12 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t | `planner-mvp-mode.md` | Vertical-slice planning rules for MVP mode. | | `planner-human-verify-mode.md` | Rules for `workflow.human_verify_mode = end-of-phase`: suppress `checkpoint:human-verify` task emission and route deferred items via ``. | | `planner-graphify-auto-update.md` | How `load_graph_context` surfaces `.last-build-status.json` auto-update state (running / failed / stale head) alongside the existing staleness annotation. Opt-in via `graphify.auto_update` (#3347). | +| `planner-interface-context.md` | Interface context rules for executors — how to extract key interfaces/types/exports from existing code and document new interfaces that downstream plans will consume. | | `skeleton-template.md` | SKELETON.md template emitted for new-project Walking Skeleton (Phase 1 + `--mvp`). | | `user-story-template.md` | User story format for MVP planning — "As a / I want to / So that" structured fields. | | `spidr-splitting.md` | SPIDR splitting decomposition rules for handling large user stories in MVP mode. | -> **Subdirectory:** `get-shit-done/references/few-shot-examples/` contains additional few-shot examples (`plan-checker.md`, `verifier.md`) that are referenced from specific agents. These are not counted in the 61 top-level references. +> **Subdirectory:** `get-shit-done/references/few-shot-examples/` contains additional few-shot examples (`plan-checker.md`, `verifier.md`) that are referenced from specific agents. These are not counted in the 62 top-level references. --- diff --git a/get-shit-done/references/planner-interface-context.md b/get-shit-done/references/planner-interface-context.md new file mode 100644 index 000000000..0a04bcba5 --- /dev/null +++ b/get-shit-done/references/planner-interface-context.md @@ -0,0 +1,62 @@ +# Interface Context for Executors + +**Key insight:** "The difference between handing a contractor blueprints versus telling them 'build me a house.'" + +When creating plans that depend on existing code or create new interfaces consumed by other plans: + +## For plans that USE existing code: +After determining `files_modified`, extract the key interfaces/types/exports from the codebase that executors will need: + +```bash +# Extract type definitions, interfaces, and exports from relevant files +grep -n "export\\|interface\\|type\\|class\\|function" {relevant_source_files} 2>/dev/null | head -50 +``` + +Embed these in the plan's `` section as an `` block: + +```xml + + + + +From src/types/user.ts: +```typescript +export interface User { + id: string; + email: string; + name: string; + createdAt: Date; +} +``` + +From src/api/auth.ts: +```typescript +export function validateToken(token: string): Promise; +export function createSession(user: User): Promise; +``` + +``` + +## For plans that CREATE new interfaces: +If this plan creates types/interfaces that later plans depend on, include a "Wave 0" skeleton step: + +```xml + + Task 0: Write interface contracts + src/types/newFeature.ts + Create type definitions that downstream plans will implement against. These are the contracts — implementation comes in later tasks. + File exists with exported types, no implementation + Interface file committed, types exported + +``` + +## When to include interfaces: +- Plan touches files that import from other modules → extract those module's exports +- Plan creates a new API endpoint → extract the request/response types +- Plan modifies a component → extract its props interface +- Plan depends on a previous plan's output → extract the types from that plan's files_modified + +## When to skip: +- Plan is self-contained (creates everything from scratch, no imports) +- Plan is pure configuration (no code interfaces involved) +- Level 0 discovery (all patterns already established) diff --git a/sdk/src/query/check-decision-coverage.test.ts b/sdk/src/query/check-decision-coverage.test.ts index a44af3f44..5d24b11cf 100644 --- a/sdk/src/query/check-decision-coverage.test.ts +++ b/sdk/src/query/check-decision-coverage.test.ts @@ -517,3 +517,197 @@ describe('config-type validation (review F16)', () => { expect(warnings.some((w) => /context_coverage_gate.*invalid type/.test(w))).toBe(true); }); }); + +// ─── XML tag body parsing (issue #5) ────────────────────────────────────── + +/** + * Regression tests for issue #5: the translation gate must parse decision IDs + * from , , , and tag bodies. + * + * The gsd-planner spec (agents/gsd-planner.md line 66) says: + * "Task actions reference the decision ID they implement (e.g., 'per D-03')" + * Plans produced by that spec put D-NN citations inside bodies, + * which the gate was blind to before this fix. + * + * Maintainer acceptance criteria (verbatim from issue #5): + * "Gate parses tag bodies for decision ID citations; regression test + * with XML-tag plan body covers all decision IDs." + */ +describe('XML tag body citation parsing (issue #5)', () => { + /** Five locked decisions, D-01..D-05. */ + const FIVE_DECISIONS = ` +### Architecture +- **D-01:** Use strict TypeScript everywhere +- **D-02:** Prefer functional composition over class inheritance +- **D-03:** All public APIs must have JSDoc +- **D-04:** Errors are typed, never thrown as plain strings +- **D-05:** Async functions always return explicit Promises +`; + + /** + * Plan whose D-NN citations are ONLY inside XML tag bodies — + * no front-matter must_haves/truths/objective entries. + */ + const XML_PLAN = `--- +phase: 17 +plan: 1 +type: implementation +wave: 1 +depends_on: [] +files_modified: [] +autonomous: true +must_haves: + - thing-a +objective: | + Build the foo. +--- + +Implement foo per D-01 and D-02 design + + + + Apply D-03 mitigation to the bar module + + + Refactor per D-04 — ensures D-05 invariant + + +`; + + it('RED: gate is blind to D-NN citations inside XML tag bodies (before fix)', async () => { + // This test is expected to FAIL on origin/main (gate returns covered: 0). + // After fix it should pass (covered: 5). + await setupPhase(FIVE_DECISIONS, { '17-01-PLAN.md': XML_PLAN }); + + const result = await checkDecisionCoveragePlan([phaseDir, contextPath], tmp); + + // GREEN assertion (what we want AFTER the fix): + expect(result.data.passed).toBe(true); + expect(result.data.covered).toBe(5); + expect(result.data.uncovered).toEqual([]); + expect(result.data.total).toBe(5); + }); + + it('does NOT count D-NN inside a non-canonical XML tag like ', async () => { + // is not one of the four canonical tags; must not count. + const planWithComment = `--- +phase: 17 +plan: 1 +type: implementation +wave: 1 +depends_on: [] +files_modified: [] +autonomous: true +must_haves: + - thing-a +--- + +D-01 not implemented yet +`; + await setupPhase( + ` +### Cat +- **D-01:** A trackable decision six words or more +`, + { '17-01-PLAN.md': planWithComment }, + ); + + const result = await checkDecisionCoveragePlan([phaseDir, contextPath], tmp); + expect(result.data.passed).toBe(false); + expect(result.data.uncovered.map((u: { id: string }) => u.id)).toContain('D-01'); + }); + + it('does NOT count D-NN mentioned in regular prose outside any XML tag', async () => { + // Prose outside designated headings and XML tags must not count. + const planWithProse = `--- +phase: 17 +plan: 1 +type: implementation +wave: 1 +depends_on: [] +files_modified: [] +autonomous: true +must_haves: + - thing-a +--- + +This section mentions D-01 in plain prose under an undesignated heading. + +## Design Notes + +D-01 appears here too but this heading is not designated. +`; + await setupPhase( + ` +### Cat +- **D-01:** A trackable decision six words or more +`, + { '17-01-PLAN.md': planWithProse }, + ); + + const result = await checkDecisionCoveragePlan([phaseDir, contextPath], tmp); + expect(result.data.passed).toBe(false); + expect(result.data.uncovered.map((u: { id: string }) => u.id)).toContain('D-01'); + }); + + it('does not crash on self-closing tags', async () => { + const planWithSelfClosing = `--- +phase: 17 +plan: 1 +type: implementation +wave: 1 +depends_on: [] +files_modified: [] +autonomous: true +must_haves: + - thing-a +--- + + + + + + +`; + await setupPhase( + ` +### Cat +- **D-01:** A trackable decision six words or more +`, + { '17-01-PLAN.md': planWithSelfClosing }, + ); + + // Should not throw; D-01 is uncovered (no citation in self-closing tag) + const result = await checkDecisionCoveragePlan([phaseDir, contextPath], tmp); + expect(result.data.passed).toBe(false); + expect(result.data.uncovered.map((u: { id: string }) => u.id)).toContain('D-01'); + }); + + it('counts D-NN in tag body', async () => { + const planWithObjective = `--- +phase: 17 +plan: 1 +type: implementation +wave: 1 +depends_on: [] +files_modified: [] +autonomous: true +must_haves: + - thing-a +--- + +Implement D-01 as the core type system +`; + await setupPhase( + ` +### Cat +- **D-01:** A trackable decision six words or more +`, + { '17-01-PLAN.md': planWithObjective }, + ); + + const result = await checkDecisionCoveragePlan([phaseDir, contextPath], tmp); + expect(result.data.passed).toBe(true); + expect(result.data.covered).toBe(1); + }); +}); diff --git a/sdk/src/query/check-decision-coverage.ts b/sdk/src/query/check-decision-coverage.ts index d92fc1269..2227f0a6a 100644 --- a/sdk/src/query/check-decision-coverage.ts +++ b/sdk/src/query/check-decision-coverage.ts @@ -157,6 +157,21 @@ interface PlanSections { const DESIGNATED_HEADINGS_RE = /^#{1,6}\s+(?:must[_ ]haves?|truths?|tasks?|objective)\b/i; +/** + * Extracts text from the four canonical XML tags that gsd-planner emits for + * decision citations: , , , . + * + * Uses a deliberately narrow regex (D2) — no XML parser library — because the + * planner's XML convention is a project-internal convention, not formal XML. + * Self-closing tags (e.g., ) are harmlessly skipped (no capture group + * match). Non-canonical tags (e.g., ) are NOT matched. + * + * Reference: agents/gsd-planner.md line 66: + * "Task actions reference the decision ID they implement (e.g., 'per D-03')" + */ +const XML_DECISION_TAGS_RE = + /<(?:objective|tasks?|action)(?:\s[^>]*)?>([\s\S]*?)<\/(?:objective|tasks?|action)>/gi; + /** Strip HTML comments AND fenced code blocks from `text`. */ function stripCommentsAndFences(text: string): string { return text @@ -165,6 +180,19 @@ function stripCommentsAndFences(text: string): string { .replace(/~~~[\s\S]*?~~~/g, ' '); } +/** + * Extract the inner text of all canonical XML tag bodies from `text`. + * Returns a concatenated string of all matched inner bodies, or '' if none. + * Called on the cleaned (comments+fences stripped) plan body. + */ +function extractXmlTagBodies(text: string): string { + const parts: string[] = []; + for (const m of text.matchAll(XML_DECISION_TAGS_RE)) { + if (m[1]) parts.push(m[1]); + } + return parts.join('\n'); +} + /** Extract a YAML block scalar (key followed by indented continuation lines). */ function extractYamlBlock(frontmatter: string, key: string): string { const re = new RegExp(`^${key}\\s*:(.*)$`, 'm'); @@ -214,7 +242,14 @@ function extractPlanSections(planContent: string): PlanSections { if (inDesignated) bodyParts.push(line); } - return { designated: [...fmParts, bodyParts.join('\n')].join('\n\n') }; + // Also include the inner text of canonical XML tag bodies (, , + // , ). The planner spec (agents/gsd-planner.md line 66) directs + // agents to cite D-NN inside bodies; the gate must honour those citations. + // extractXmlTagBodies is called on the full cleaned content (not just the body + // portion) so that blocks at the top of the document are also caught. + const xmlParts = extractXmlTagBodies(cleaned); + + return { designated: [...fmParts, bodyParts.join('\n'), xmlParts].join('\n\n') }; } async function loadPlanSections(phaseDir: string): Promise {