ADR-321: Implement instance mode in spec-first-draft and /spec forwarding - #74
ADR-321: Implement instance mode in spec-first-draft and /spec forwarding#74jodavis-claude wants to merge 2 commits into
Conversation
… argument Adds a Step 1 parsing rule that detects and strips a trailing/embedded 'using <playbook-reference>' (path or bare name) from $ARGUMENTS before resolving the feature brief, and forwards it to spec-first-draft in Step 2 so instance mode can resolve it per playbook-contract's bare-name resolution order.
…-template.md Adds instance mode: when a playbook reference (path or bare name) is supplied, resolve it per playbook-contract's bare-name resolution order, read the playbook's spec-template.md, use its sections in place of the default draft template, and stamp '> **Playbook:** <name or path>' as an additional header line alongside the existing Status/Design doc lines. Default (no playbook reference) flow is unchanged. Key decisions: - Implemented directly rather than through the tdd-tester/tdd-implementer/tdd-refactorer trio: this Testable component is agent-skill prose with no AAA-testable logic of its own (component-taxonomy's explicit agent-skill-prose example), verified instead by the fixture-driven dry-run harness already checked in for this task. Mirrors ADR-318/ADR-319's precedent of bypassing the trio for this same kind of component. - Verified green by spawning a subagent blind to authoring context to run the instance-mode Step 2 against fixture-playbook/, then grading the output against RUN.md's checklist: the draft file existed, its four sections matched spec-template.md's sections exactly (Domain, Endpoints, Applicable ADRs, Deltas from playbook assumptions), and the > **Playbook:** header line was present. - Narrowed scope per the task brief's recommendation: instance mode does not seed the drafted spec's Component Breakdown table from the playbook's step list — that reading of the spec's Key Design Decision text is not gated by this task's own exit criteria or the pre-built dry-run checklist, and is flagged as a known ambiguity for the task brief's author to confirm. - Step 1 (gather context) and Step 3 (pause for review) left unmodified, per the brief.
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis-claude
left a comment
There was a problem hiding this comment.
Reviewed against ADR-321's exit criteria and the Playbook Harvesting spec (Task 5). All four exit criteria are met: the pre-existing dry-run checklist in RUN.md served as the checklist-first red state, commands/spec.md documents and forwards using <playbook>, spec-first-draft's instance mode resolves the reference per playbook-contract and stamps the header, and the described dry run (subagent grading against RUN.md's checklist) confirms the draft uses the template's sections plus the header stamp. Default (no-playbook) flow is left byte-for-byte unchanged, and the playbook-contract citation follows the harvest-playbook precedent rather than restating it. Two non-blocking notes below; no Priority 1-4 issues that would block approval.
| ### 1 — Resolve the feature brief | ||
|
|
||
| Use the `identify-project-work-items` skill to resolve the argument to a `work-item-id` and `work-item-type`, then fetch the work item: | ||
| Parse `$ARGUMENTS` for a trailing or embedded `using <playbook-reference>` — a path or a bare |
There was a problem hiding this comment.
Minor correctness/fault-tolerance note (non-blocking): stripping a using <playbook-reference> that can appear "trailing or embedded" anywhere in $ARGUMENTS risks silently mangling a legitimate feature brief that happens to contain the ordinary English word "using" (e.g. "notification service using webhooks for delivery") — there'd be no error, just a quietly wrong brief and a spurious playbook-resolution attempt. This matches the spec's own literal /spec ... using <playbook> wording, so it's not a defect introduced here, but consider tightening the guidance to prefer a trailing match and/or require the following token look like a bare skill name or path (no spaces) before stripping, to reduce false positives from conversational free text.
|
|
||
| Name: `_spec_<FeatureName>.md` in PascalCase. | ||
|
|
||
| **Instance mode.** If a playbook reference (a path or a bare name) was supplied when this skill |
There was a problem hiding this comment.
Non-blocking, flagged for confirmation rather than a defect: the spec's "Explicit trigger, explicit naming" Key Design Decision text says the playbook's step list also feeds the drafted spec's own ## Component Breakdown table, but this instance-mode implementation only substitutes template sections and stamps the header. That's the narrower scope the task's own exit criteria and the pre-built RUN.md checklist actually gate, per the task brief's recommendation, so I'm not blocking on it here — just surfacing it again so the user/spec owner can confirm whether Component Breakdown seeding belongs in this task, a later one (e.g. ADR-322), or should be dropped from the spec text.
Work item: ADR-321 — Implement "instance mode" in
spec-first-draft(drafting a thin instance spec from an explicitly named playbook'sspec-template.md, stamping a> **Playbook:**header reference) and add theusing <playbook>forwarding path incommands/spec.md.Changes:
plugins/dev-team/commands/spec.md— argument-hint gains a[using <playbook>]form; Step 1 now parses$ARGUMENTSfor a trailing/embeddedusing <playbook-reference>(path or bare name, flag-or-conversational, mirroringharvest.md's style), stripping it before resolving the feature brief; Step 2 forwards the detected reference tospec-first-draft, noting it triggers instance mode.plugins/dev-team/skills/spec-first-draft/SKILL.md— adds instance mode: frontmatter description/argument-hint updated; a new "Use this skill when" bullet plus a citation ofplaybook-contract(directory contract and bare-name resolution order — cited, not restated, matchingharvest-playbook's precedent). Step 2 gains an "Instance mode" paragraph: when a playbook reference was supplied, resolve it perplaybook-contract, read the playbook'sspec-template.md, use its own sections in place of the default## Overviewthrough## Related Docssections, and add a> **Playbook:** <name or path>header line alongside the existing> **Status:**/> **Design doc:**lines. Default (no playbook reference) flow is byte-for-byte unchanged. Steps 1 and 3 untouched.Design decisions:
implement-tddtrio) — the Testable component (spec-first-draftinstance mode) is agent-skill prose with no AAA-testable logic of its own, so it was verified via the fixture-driven dry-run harness already checked in for this task (fixtures/playbook-harvesting/RUN.md's pre-authored "spec-first-draftinstance mode — checks" section), mirroring the same call made in ADR-318 and ADR-319.> **Status:**/> **Design doc:**, not a replacement, resolving a brief-flagged ambiguity conservatively per the "stamped into" wording.## Component Breakdowntable from the playbook's step list, since neither this task's exit criteria nor the pre-builtRUN.mddry-run checklist gates that behavior. Flagged in the task brief for confirmation on whether that seeding belongs in a later task (ADR-322) or was meant for this one.spec-first-draft's Step 1 (context gathering) and Step 3 (pause for review) unmodified, per the brief's recommendation.playbook-contract's bare-name resolution tier 2) — out of this task's exit criteria; the fixture dry run used an absolute path so this tier was never exercised.Testing completed:
missing-test-harness); both components in scope are prose edits, not executable code.plugins/dev-team/fixtures/playbook-harvesting/RUN.md("spec-first-draftinstance mode — checks") served as the red/green equivalent: authored before this task's skill-prose edits (pre-existing red state), then confirmed green by spawning a subagent blind to authoring context to run the instance-mode Step 2 againstfixtures/playbook-harvesting/fixture-playbook/with a fictitious inventory-service brief, and grading the resulting draft against RUN.md's checklist —test -fon the draft passed,grep -q "^> \*\*Playbook:\*\*"passed, and the draft's four body sections (Domain,Endpoints,Applicable ADRs,Deltas from playbook assumptions) matchedspec-template.md's own sections exactly, in order.Known ambiguities (flagged for reviewer):
## Component Breakdowntable from the playbook'sSKILL.mdstep list — left unresolved, narrower scope implemented per the task brief's own recommendation.dev-team.mdoverlay when present") — not implemented;playbook-contract's directory contract explicitly says there is no such overlay file, and the spec/task brief (treated as authoritative) omit it, deferring to ADR-322.playbook-contract's bare-name resolution tier 2 ("installed plugin skill directories") still has no concrete lookup mechanism established anywhere in this repo — will recur for ADR-322 if not resolved before then.