ADR-318: Build the skill dry-run harness for Playbook Harvesting

[jodavis-claude]

Work item

ADR-318: Build the reusable, checked-in fixtures and documented run procedure — the "skill
dry-run harness" — that will later verify the three Testable prose components introduced by
the Playbook Harvesting feature (harvest-playbook, spec-first-draft instance mode,
spec-task-breakdown playbook seeding), per the missing-test-harness rule that a harness
must be built deliberately rather than improvised mid-task. Requires Task 1 (ADR-317).

Changes

All new files under plugins/dev-team/fixtures/playbook-harvesting/:

• fixture-spec.md — mini-spec fixture with > [!NOTE] / **Method:** markers and a small
  ## Planned Implementation section
• exemplar-repo-1/, exemplar-repo-2/ — checked-in per-commit snapshots (no .git
  directories) for two sibling service instances (orders-service, billing-service) stamped
  from the same template: template stamp, strip/replace, and a divergent choice (JSON vs.
  plain-text logging), each driven by a commits.json manifest
• materialize.py — replays an exemplar's commits.json manifest into a throwaway git repo on
  demand (idempotent)
• test_materialize.py — pytest unit tests for materialize.py (14 tests)
• template-output/ — pristine template-stamp output for strip/replace derivation by diff
• fixture-playbook/ — hand-authored playbook conforming to playbook-contract's shape
  (SKILL.md, spec-template.md, dev-team.md, service-yaml-schema.md)
• fixture-instance-spec.md — hand-authored instance spec carrying a > **Playbook:** header
  pointing at fixture-playbook/
• interview-answer-key.md — scripted Q&A for the harvest interview, including the divergence
  resolution and the "no answer — proceed with your recommendation" fallback rule
• RUN.md — the documented run procedure: fixture contents, on-demand (not CI) invocation
  model, materialize commands, the clean-session validating-subagent procedure, mechanical
  assertions as commands per target component, and re-run instructions

Design decisions

• Fixture directory location: shared plugins/dev-team/fixtures/playbook-harvesting/
  (not colocated under harvest-playbook/), since none of the three consuming skills exist
  yet in this worktree and more than one skill consumes subsets of the fixture set.
• Interview answer key format: a Markdown table (question theme → scripted answer).
• Mechanical assertions: kept inline in RUN.md under each component's checklist item
  rather than as separate scripts, to avoid inventing a new script-execution convention.
• ADR-317 sequencing: playbook-contract (this task's stated prerequisite) hasn't landed
  in this worktree yet. The Method-marker format and playbook-directory contract were authored
  directly from _spec_PlaybookHarvesting.md's own definitions; flagging for cross-check
  against the real playbook-contract SKILL.md once ADR-317 lands.
• Divergent choice + answer key designed together: exemplar repos diverge on logging format
  specifically so the answer key has a concrete conflict to resolve.
• TDD scope call: only materialize.py's git-replay logic is AAA-testable; drove it through
  genuine red/green TDD directly rather than spawning the tdd trio, since the rest of the
  component is hand-authored fixture content with no natural red/green cycle.
• No E2E scenarios: no E2E harness exists in this repo, and this task's deliverable has no
  runtime behavior of its own to exercise end-to-end.
• Filed a follow-up GitHub issue (_doc_Projects.md plugin layout table is missing skills/ and fixtures/ #65) noting _doc_Projects.md's plugin
  layout table is missing skills/ and the new fixtures/ directory — out of scope here.

Testing completed

• python3 -m pytest plugins/dev-team/fixtures/playbook-harvesting/test_materialize.py — 14
  tests, all passing
• Full repo suite python3 -m pytest plugins/dev-team — 192 passed

[github-actions]

build-and-test: Python test results

Status: ✅ Passed

Test log

[jodavis-claude]

Reviewed ADR-318 (skill dry-run harness fixtures for Playbook Harvesting) against the task brief's 9 exit criteria and CONTRIBUTING.md. All 9 exit criteria are satisfied and the fixture set is internally consistent (verified: no committed .git dirs, Method-marker count matches RUN.md's claim, vendor-neutrality grep passes on fixture-playbook/, the TODO fallback RUN.md references is actually present, materialize.py's 14 unit tests pass). One correctness/fault-tolerance gap found in materialize.py — see inline comment. Everything else is a minor, non-blocking note.

[jodavis-claude]

Sign-off review for ADR-318 fix pass (commit 89807b5).

Prior thread (materialize.py correctness/fault tolerance): resolved. _validate_entry_shape() now validates each manifest entry is a dict with string dir/message keys before load_manifest/validate_snapshots_exist dereference them, raising a friendly MaterializeError instead of an uncaught KeyError/TypeError traceback. Re-ran the reporter's exact repro (echo '[{"message": "missing dir key"}]' > commits.json) — now prints Error: .../commits.json entry {...} is missing required key 'dir' and exits 1, no traceback. 5 new tests in TestMaterializeErrorHandling cover the new branches (non-dict entry, missing dir, missing message, non-string dir, non-string message), follow the test_<module>_<scenario>_<expected_result> naming convention and AAA structure from CONTRIBUTING.md.

Scan of modified files (materialize.py, test_materialize.py) for new issues: none found.

• Fault tolerance: _validate_entry_shape is called from load_manifest before validate_snapshots_exist/materialize ever dereference entry["dir"]/entry["message"], so all downstream accesses are now guarded. CalledProcessError is still caught explicitly in main(), not swallowed.
• Subprocess calls retain explicit timeout= per CONTRIBUTING.md.
• No security or performance concerns in this fixture/CLI script.
• No documentation changes needed — this is a bug fix within the already-documented materialize.py contract, not a design change.

Verification run:

• python3 -m pytest plugins/dev-team/fixtures/playbook-harvesting/test_materialize.py -q — 19 passed
• python3 -m pytest plugins/dev-team -q — 197 passed

All prior review threads resolved, no new issues found. Approving for hand-off.

[jodavis-claude]

Sign-off review for ADR-318 (skill dry-run harness fixtures for Playbook Harvesting), commit 89807b5 — same commit as the prior sign-off pass; no new commits since.

Prior thread (materialize.py manifest-entry validation, discussion #discussion_r3576274601): already resolved. Independently re-verified: _validate_entry_shape() is called from load_manifest() for every manifest entry before validate_snapshots_exist()/materialize() dereference entry["dir"]/entry["message"], raising a friendly MaterializeError (caught in main() and printed as Error: ...) for a non-dict entry, a missing dir/message key, or a non-string value — closing the original gap (uncaught KeyError/TypeError traceback). Re-ran the full suite: python3 -m pytest plugins/dev-team/fixtures/playbook-harvesting/test_materialize.py -q → 19 passed. No action needed; thread stays resolved.

Modified files since the last review pass: only materialize.py and test_materialize.py changed (in commit 89807b5, already covered above). All other fixture files were reviewed in the prior pass against commit a59c443 and are unchanged since. The 5 new tests in TestMaterializeErrorHandling follow this repo's test_<module>_<scenario>_<expected_result> naming convention and AAA structure (CONTRIBUTING.md), use tmp_path per-function fixtures, and _run_materialize's subprocess.run call carries a timeout=30. No new issues found.

Exit criteria: all 9 criteria from the task brief remain satisfied (fixture mini-spec with Method markers, two exemplar repos with scripted git history and no nested .git dirs, template-output dir, fixture-playbook conforming to the spec's contract description, fixture-instance-spec with the Playbook header, interview answer key with the fallback rule, RUN.md's documented run procedure with mechanical assertions as commands, re-run instructions, and the on-demand invocation model linking ADR-334).

CI: build-and-test SUCCESS, gate SUCCESS, devcontainer-validate SKIPPED (expected, no devcontainer changes).

No open threads, no new Priority 1-4 issues. Approving for hand-off.