ADR-317: Add playbook-contract knowledge skill - #62
Conversation
Defines the normative two-layer playbook directory contract (required SKILL.md + spec-template.md; optional dev-team.md overlay and scripts), TODO marker semantics with mandatory manual fallbacks, vendor-neutrality rules with compliant/violating step-prose examples, the Method marker convention (format, lifecycle, post-harvest target shape), and bare-name playbook resolution order.
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis-claude
left a comment
jodavis-claude
left a comment
There was a problem hiding this comment.
First-pass review of ADR-317 (playbook-contract knowledge skill). Overall this is a solid, faithful implementation of the task brief and the spec's Key Design Decisions -- all three exit criteria are present (two-layer directory contract with verbatim tree diagram, TODO marker semantics with mandatory manual fallbacks, Method marker format/lifecycle/post-harvest shape, and bare-name resolution order), and the file closely follows the three cited precedent skills' structure (component-taxonomy, tdd-practices, missing-test-harness). One clarity issue flagged inline on the vendor-neutrality "Violating" example -- see comment. No other Priority 1-4 issues found; this is a documentation-only change so most async/security/performance criteria don't apply.
… quoted bad step prose Addresses PR #62 review comment: the Violating bullet previously led with the litmus question instead of a concrete example of violating step prose, unlike the Compliant bullet which quotes a complete step. Now both bullets quote step prose first, with the litmus question explained as the diagnostic used to catch the violation.
jodavis-claude
left a comment
jodavis-claude
left a comment
There was a problem hiding this comment.
Sign-off review — ADR-317
Prior review thread: the one open thread (Priority 4, documentation clarity on the
Violating vendor-neutrality example) was addressed satisfactorily — the fix in commit
911f5fd matches the suggested wording exactly, leading with the quoted bad step prose
("Run the usual deploy skill against the pipeline to bring the service up.") before
explaining the litmus question, mirroring the Compliant bullet's shape. Verified against
the diff and resolved.
New issues in the modified region: none. The fix commit is a scoped 12-line change
(6 insertions, 6 deletions) confined to the reported bullet; no other files touched.
Requirements check: re-verified plugins/dev-team/skills/playbook-contract/SKILL.md
against the three ADR-317 exit criteria and the spec's Key Design Decisions —
the two-layer directory contract (verbatim tree diagram), TODO marker semantics with
mandatory manual fallbacks, vendor-neutrality rules with compliant/violating examples,
the Method marker format/lifecycle/post-harvest shape, and the bare-name resolution
order (project-local → installed plugin skill directories → ask the user) are all
present and consistent with _spec_PlaybookHarvesting.md.
Sign-off: approved.
Review feedback: the "Use this skill when" bullet described playbooks narrowly as capturing "how to build a family of similar components," but a playbook can capture any reusable process discovered while implementing a spec (a testing strategy, a documentation format, etc.), not only component-building. Reworded to list component-family construction as one example among several rather than the sole use case.
Review feedback: no dev-team artifacts should live in the playbook directory contract until a real need for one proves itself. Removed the dev-team.md optional overlay from the directory tree diagram and its explanatory bullet, dropped the resulting "two-layer" framing from the section heading and frontmatter description (only the required core plus optional scripts/assets remain), and reworded the intro paragraph to state plainly that the contract has no dev-team-specific overlay file.
The overlay concept was dropped from playbook-contract per PR #62 review feedback (no dev-team artifacts in playbooks until a real need proves itself). Updates the spec's design decisions, directory tree diagrams, Component Breakdown, Open Questions, and task exit criteria (including retroactive updates to the already-completed Task 1) to match. Closes #64. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNYTYW974in8rcD7cWd3Ca
3 participants
Work item
ADR-317: Author the
playbook-contractknowledge skill — the normative, vendor-neutral definition of the two-layer playbook directory contract, the Method marker convention, and bare-name playbook resolution — so thatharvest-playbook,spec-first-draftinstance mode, andspec-task-breakdownplaybook seeding (later tasks) all cite one shared source instead of drifting apart.Changes
plugins/dev-team/skills/playbook-contract/SKILL.md(new) — a knowledge skill defining:SKILL.md+spec-template.md; optionaldev-team.mdoverlay and supporting scripts)> [!NOTE]/**Method:**format, persistent-until-harvest lifecycle, post-harvest provenance-link shape).claude/skills/<name>/→ installed plugin skill directories → ask the user)Design decisions
src/Common/*from service-three (TODO: extract into shared package)") verbatim, for consistency with what later tasks will validate against.dev-team.mdoverlay's purpose with recommended (not mandatory) heading guidance, per the spec's Open Questions section deferring hardening of that schema to real use.component-taxonomy,tdd-practices,missing-test-harness).plugin.jsonedit needed — skills are auto-discovered fromskills/<name>/SKILL.md.Wrappercomponent in scope; no consumer skills (harvest-playbook,spec-first-draft,spec-discussion,spec-task-breakdown,final-sign-off) were touched — those are later tasks (ADR-319 through ADR-322).Testing completed
Wrappercomponent ("definitional prose, no procedure," no dependencies) percomponent-taxonomy— visual inspection only, consistent with the task brief and the spec's Component Breakdown table..featurefiles or E2E harness exist anywhere in this repo (confirmed via search), and permissing-test-harnessa harness must not be invented where none exists.