Skip to content

feat: ground planning and review skills in consumer standards (V1 pilot)#291

Merged
kyle-sexton merged 17 commits into
mainfrom
feat/proactive-vs-reactive-skills
Jul 17, 2026
Merged

feat: ground planning and review skills in consumer standards (V1 pilot)#291
kyle-sexton merged 17 commits into
mainfrom
feat/proactive-vs-reactive-skills

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

No linked issue

Related

Development-workflow skills now load the consumer's standards proactively (plan time) instead of colliding with them reactively at review time. One shared, versioned standards concern contract — consumed by planning AND review — makes work get built to the criteria it will be reviewed against.

What's in here

  • Standards concern contract v1.0.0 at docs/conventions/standards/ — thin routing index with a normative presence test, three additive layers (user-global → team → personal overlays) under precedence inversion, a six-rung resolution ladder, .claude/standards.yaml concern file + JSON schema, and a normative idempotent setup-and-migration procedure with DIRECTIONAL version-delta handling.
  • Sync machinery (scripts/sync-standards-contract.sh + test-first suite): byte-identical binding copies in both pilot plugins, registered cross-plugin cluster, new standards-contract-sync CI job (drift + frontmatter/changelog/manifest bump enforcement).
  • planning 0.16.0 (rebased over main's 0.15.0 renames — the skill is now /planning:plan): /planning:plan Step 2 opens with a standards-grounding formulation input (selective section pulls, ambient-content rule, scale-gated depth, provenance naming, broken-row Boy Scout, mandatory version compare); plan template + reviewer carry the grounding surface; /planning:setup gains the standards bootstrap concern; tripwire test.
  • review 0.10.0: criteria mode resolves through the same index via the binding's ladder; Step 1's "What conventions apply?" routes through it so ALL review modes inherit; new /review:setup bootstrap skill; tripwire test.

Verification

14-row fixture matrix PASS (canary anti-theater probes both directions, setup idempotency run-twice-no-diff, overlay precedence, directional version skew, pre-existing-README confirmation gate, user-global layer incl. denied-read grace), plugin-acceptance security review PASS, claude plugin validate --strict both plugins. Full record in the PLAN below.

Follow-ups filed: #288 (fleet-wave sync generalization), #289 (wave-2 rollout inventory), #290 (pre-existing headless quality-gate preflight bug).

Approved PLAN.md (verification record included; contract slice pruned before merge)

proactive-vs-reactive-skills

Brief

TLDR

Development-workflow skills load the consumer's standards proactively (at plan/build time) instead of colliding with them reactively at review time. One shared, concern-named standards index — consumed by planning AND review — makes skills build to the criteria they'll be reviewed against.

Goal

Eliminate the rework loop where /architect-planned and /implement-built work violates consumer standards (code conventions, engineering philosophy, design-review criteria) that only surface during review. Proactive grounding becomes the DEFAULT behavior of lifecycle skills — no mode fork, no sibling skill variants — with cost governed by existing scale/blast-radius gates and progressive disclosure.

V1 (locked): the standards concern contract + a two-consumer pilot.

  1. Versioned concern contract at docs/conventions/standards/ — index schema, layers, precedence, resolution ladder.
  2. planning:architect gains a proactive standards-loading step (plan formulation grounds in the loaded criteria).
  3. review:quality-gate criteria mode resolves criteria through the SAME index (build/review symmetry).
  4. Idempotent setup bootstrap in those two plugins only.

Fleet rollout (implement, design, prototype, testing, …) follows in per-plugin waves after the pilot proves the contract.

Constraints

  • Repo-agnostic, no baked opinions (C1). Every default — including the concern folder location — is reconfigurable via re-runnable setup. Only the discovery anchor stays conventional. Plugin never names the user's org, repos, or layout.
  • Default layout (three layers, seam-2 shape): user-global ~/.claude/standards/ → team-tracked docs/standards/ → personal gitignored docs/standards/*.local.md (setup ships the .gitignore line). Team layer deliberately OUTSIDE .claude/ (verified: .claude/ writes are specially permission-guarded even under acceptEdits; reads are prompt-free anywhere in the working directory).
  • Policy precedence inversion: layers are additive; personal layers may ADD or TIGHTEN only; direct conflict → team-tracked wins. Skills state which layer contributed when a personal rule materially shapes output.
  • Progressive disclosure: root index is a thin routing map (in-scope surfaces + context clues, no content); standards files are SRP-organized (one concern per file); skills pull only sections matching the surfaces the task touches (e.g. C# conventions only when touching C#).
  • Resolution ladder (playbook-adopted): index present → use it; absent → infer from not-auto-loaded usual suspects (docs folders, ecosystem configs — never re-read auto-loaded CLAUDE.md/.claude/rules) and OFFER to persist; can't infer → ask once; else safe ecosystem default. No silent writes, ever.
  • Setup mode 2: skills usable immediately with defaults; setup optional, idempotent, re-runnable anytime; setup may offer (never force) reorganizing mixed/spread consumer standards content toward the SRP + index shape.
  • Horizontal decoupling: no cross-plugin imports. Each consuming plugin carries a synced reference/ binding copy of the contract (topic-docs/hook-utils precedent); per-plugin idempotent bootstrap — first setup writes the index, later setups validate/offer reconfigure.
  • Anti-waterfall: grounding depth rides the existing plan-scale/blast-radius gates; trivial work pays near zero. No grounding flag, no --ungrounded escape hatch.
  • Process gates: fresh-docs mandate (WebFetch current plugin docs) before any file change; per-plugin migration gate + plugin-acceptance security review; version bumps + changelog per delivery rules.

Acceptance criteria

  • docs/conventions/standards/README.md exists: index schema, three layers, precedence-inversion rule, resolution ladder, versioning — and both pilot plugins carry a synced binding copy.
  • plugins/planning/skills/architect/SKILL.md carries the proactive standards step; a plan produced for a task touching surface X cites the standards sections loaded for X; grounding depth demonstrably scales with the plan-scale tier (trivial plan → no standards fetch beyond ambient context).
  • plugins/review/skills/quality-gate criteria mode resolves criteria through the same index when present (grep confirms the binding reference, exercise confirms the load).
  • Setup in both pilot plugins bootstraps docs/standards/ idempotently in a clean non-source repo via --plugin-dir (run twice → no diff on second run).
  • Absent-index fallback: in a repo with no index, the skill infers from repo context and offers persistence — verified by exercise; zero unprompted writes.
  • No hardcoded consumer paths (grep for absolute paths / org names in changed plugin files → zero hits); claude plugin validate passes; both plugins version-bumped with changelog entries.

Captured assumptions

  • File reads need no permission approval anywhere in the working directory, all environments; .claude/ writes sit on the protected-directory prompt list (both verified against the official permissions doc, 2026-07-17).
  • standards is the umbrella term (adopted conventions become standards — research-confirmed 2026-07-17); "guidance" rejected as advisory-only connotation.
  • Auto-loaded surfaces (consumer CLAUDE.md, .claude/rules) apply ambiently and are never re-fetched by the grounding step.
  • Two consumers (one proactive, one reactive) are sufficient to validate the multi-plugin concern design before fleet rollout.

Out-of-scope

  • Dual proactive/reactive skill variants and mode flags — ruled out (playbook: depth/intensity variants are arguments, never siblings; here not even an argument).
  • Meta-setup plugin for shared plugin conventions — deferred; trigger: ≥3 shared concerns AND observed bootstrap drift/nagging across plugins.
  • Restructuring the melodic-software/standards repo itself ("am I doing the standards repo right") — separate effort, own session.
  • Fleet rollout waves beyond the two-plugin pilot — planned after pilot verdict.

Deferred questions

  • Index/contract schema detail (surface taxonomy: ecosystem × layer × stage; file format; root-index shape) → /design
  • Per-skill step placement inside architect/quality-gate bodies + binding-copy sync mechanics (extend existing sync machinery?) → /architect
  • Plugin-upgrade migration handling: inside re-runnable setup vs separate action → /design
  • Wave-2 rollout inventory + inclusion rule (which dev-workflow skills, what order) → USER-RESERVED (scope decision at wave time)

Plan

Fresh-docs verified this session (2026-07-17): skills frontmatter fields incl. disable-model-invocation / user-invocable / argument-hint (https://code.claude.com/docs/en/skills); claude plugin validate <dir> --strict, --plugin-dir session loading, ${CLAUDE_PLUGIN_ROOT} / ${CLAUDE_PLUGIN_DATA}, and pluginConfigs user-settings-only storage (https://code.claude.com/docs/en/plugins-reference). Implementation phases MUST re-fetch the pages relevant to the files they touch (repo fresh-docs mandate) before editing.

Build technique: committed work, integration risk → tracer bullet. The integration spine (contract + sync machinery, Phases 1–2) lands first and is verified end-to-end (canonical → synced copies → CI drift gate) before either consumer skill is rewired.

Phase 1: Contract home — docs/conventions/standards/ [DONE]

Materialize contract-spec.md as the canonical convention, following the marketplace convention template (docs/conventions/topic-docs/ precedent: README + CHANGELOG + schema + examples).

File Action What changes
docs/conventions/standards/README.md Create The contract: YAML frontmatter standards-contract: 1.0.0 (self-describing version — see D2); index schema + columns; external-row validation duty; standards-file rules (SRP, pure prose); personal overlays; user-global layer; .claude/standards.yaml concern file + resolution ladder; .claude/rules division-of-content seam + pointer pattern; versioning; normative "Setup and migration" section (bootstrap procedure, idempotency, run-twice-no-diff, version-delta detection + guided migration, tolerant-reader rule) — single home for the procedure both plugins implement by reference. Stress-test additions (DA): presence test is normative — an index exists iff the standards-contract frontmatter key is present; a <standards_dir>/README.md without it is pre-existing content = inference source only, and setup treats it as hand-authored, requiring explicit confirmation before any conversion (DA-F5). Resolution root = git top-level (concern file discovered there); index File paths are forward-slash repo-relative (DA-F6). Version-delta detection is DIRECTIONAL: index newer than the reading plugin's bundled contract → best-effort read, NEVER offer migration (no downgrades), message "update the plugin" (DA-F3). Setup short-circuits without interview when a conforming index at the bundled version exists (DA-F11). Size guidance: SRP + soft per-file budget, recommend splitting oversized files; grounding reads matched files selectively — sections relevant to the task, not necessarily whole files (DA-F8). The "Resolution ladder" is a clearly-delimited section skills jump to (progressive disclosure within the binding); consuming SKILL.md files carry pointer language only, never a restated ladder (DA-F9). Authoring constraint: no relative markdown links in the README — the synced copies would 404 in the offline lychee lane; backtick file names instead (DA-F12)
docs/conventions/standards/standards.schema.json Create JSON Schema for .claude/standards.yaml (standards_dir string, all keys optional) — topic-docs.schema.json / ecosystem.schema.json precedent
docs/conventions/standards/CHANGELOG.md Create 1.0.0 initial entry
docs/conventions/standards/examples/worked-index.md Create One worked consumer index (in-root rows, one external row, one .local.md overlay note)

Content authority: docs/topics/proactive-vs-reactive-skills/design/contract-spec.md + Brief constraints. Additive contract text beyond the spec is limited to D2 (version frontmatter) plus the enumerated DA additions above — each closes a stress-test gap without contradicting a RESOLVED design thread.

Sanity Check:

  • grep -c "standards-contract: 1.0.0" docs/conventions/standards/README.md returns ≥1 (frontmatter); grep -c "^## 1.0.0" docs/conventions/standards/CHANGELOG.md ≥ 1
  • jq -e . docs/conventions/standards/standards.schema.json exit 0
  • grep -cE "^## " docs/conventions/standards/README.md ≥ 6 (index schema, files, overlays, user-global, concern file, rules seam, versioning/setup present)
  • markdownlint + repo link-check lanes pass on the new files

Phase 2: Sync machinery + binding copies [DONE]

Byte-identical binding copies delivered by a dedicated sync script (hook-utils shape — see D1), registered as a cross-plugin cluster.

Pre-flight (first item): ls plugins/*/reference/ and grep -rn "standards-contract" plugins/ scripts/ .github/ — confirm no existing standards-contract.md collision and no consumer already parsing that name. Verified this session: none exist; review plugin has no skills/setup/.

File Action What changes
scripts/sync-standards-contract.sh Create Mirrors sync-hook-utils.sh shape: sync (copy docs/conventions/standards/README.md into every plugin carrying reference/standards-contract.md), --check (byte-compare), --check-bump <ref> (contract changed but a carrying plugin's manifest version didn't → fail). Beyond the hook-utils shape (DA-F4): --check-bump additionally fails when canonical content changed vs base but the standards-contract frontmatter semver did NOT change or CHANGELOG.md gained no new ## heading — the frontmatter version is what T6 migration detection reads; without this, content drifts under a frozen version forever. Opt-in by file presence — fleet waves add plugins without script edits
scripts/sync-standards-contract.test.sh Create Written first (Red) — test-first per TDD default; asserts sync/--check/--check-bump behavior incl. the frontmatter-bump + changelog-heading failure cases, against temp fixtures (check-cross-plugin-source-drift.test.sh precedent)
plugins/planning/reference/standards-contract.md Create Initial opt-in copy (byte-identical to canonical)
plugins/review/reference/standards-contract.md Create Initial opt-in copy
scripts/cross-plugin-source-registry.txt Modify Register reference/standards-contract.md cluster, comment naming the dedicated check (required — an unregistered identical cluster fails check-cross-plugin-source-drift.sh --check)
.github/workflows/ci.yml Modify New standards-contract-sync job mirroring hook-utils-sync (--check, test run, --check-bump origin/$BASE_REF); add to the final aggregate gate's needs list

Sanity Check:

  • scripts/sync-standards-contract.sh --check exit 0; bash scripts/sync-standards-contract.test.sh exit 0
  • cmp docs/conventions/standards/README.md plugins/planning/reference/standards-contract.md exit 0 (same for review copy)
  • scripts/check-cross-plugin-source-drift.sh --check exit 0 (cluster registered)
  • grep -c "standards-contract-sync" .github/workflows/ci.yml ≥ 2 (job + needs entry)

Phase 3: planning plugin — proactive grounding + setup extension [DONE]

Review: code-design

Fresh-docs re-fetch (skills page) before edits.

File Action What changes
plugins/planning/skills/architect/SKILL.md Modify New "Ground in consumer standards" subsection at the TOP of Step 2 (formulation input, not a Step 1 gate — anti-waterfall; see D3): resolve the index per the contract ladder via the binding [${CLAUDE_PLUGIN_ROOT}/reference/standards-contract.md]; match task surfaces against Applies when clues; pull ONLY matched files not already ambient (fired rules / CLAUDE.md never re-pulled; post-compaction context counts as NOT-ambient — re-resolve per task, so a second task in the same session grounds its own surfaces); depth rides the existing Step 2 plan-scale table (trivial → no fetch beyond ambient); personal-layer provenance named when a .local.md rule shapes the plan; broken external row → surface + offer fix (Boy Scout, never silent); older-index → best-effort + "index at vX, contract at vY — re-run setup"; absent index → infer from not-auto-loaded usual suspects, OFFER to persist, ask once, else safe default — zero unprompted writes. Reads matched files SELECTIVELY (task-relevant sections, DA-F8); the ladder itself is NOT restated in SKILL.md — pointer to the binding's delimited "Resolution ladder" section only (DA-F9)
plugins/planning/tests/standards-binding.test.sh (location per repo plugins/**/*.test.sh precedent) Create Tripwire contract test (DA-F7): greps assert the load-bearing markers survive future prose edits — grounding heading inside Step 2, binding reference in architect + setup SKILL.md, no restated-ladder keywords. Picked up automatically by scripts/run-plugin-tests.sh
plugins/planning/skills/architect/context/plan-template.md Modify Full template gains a "Standards grounding" element (table: surface → sections cited → layer provenance); scale guidance notes trivial/small skip
plugins/planning/skills/architect/context/plan-reviewer.md Modify Reviewer axis: "plan cites the standards sections loaded for the surfaces it touches, or states why grounding was skipped (scale tier)"
plugins/planning/skills/setup/SKILL.md Modify Second concern: standards bootstrap per the binding's normative Setup-and-migration section — read state (.claude/standards.yaml → index → inference), interview, bootstrap docs/standards/README.md skeleton + overlay ignore line, validate every index row path, version-delta detection + guided migration, optional pointer-rule generation (interactive only). Frontmatter description updated to cover both concerns. Explicit reconciliation work item (reviewer F1): the skill currently prohibits editing ignore files twice (Task step 4; What-this-skill-does-NOT-do). The overlay line ships as a setup-created <standards_dir>/.gitignore containing *.local.md — never a consumer root-.gitignore edit — and both prohibition passages are amended to scope: setup never edits an ignore file it did not create; the standards root's own bootstrap-shipped .gitignore is setup-owned. Contract README (Phase 1) words the mechanism identically
plugins/planning/skills/architect/evals/evals.json Modify Case: task touching surface X in an index-present fixture → plan cites X's sections
plugins/planning/skills/setup/evals/evals.json Modify Case: standards bootstrap idempotency (second run proposes no changes)
plugins/planning/.claude-plugin/plugin.json Modify 0.12.00.13.0
plugins/planning/CHANGELOG.md Modify 0.13.0 entry

Sanity Check:

  • grep -c "standards-contract.md" plugins/planning/skills/architect/SKILL.md ≥ 1 and same grep ≥ 1 in plugins/planning/skills/setup/SKILL.md
  • grep -c "Ground in consumer standards" plugins/planning/skills/architect/SKILL.md = 1, located inside Step 2 (Read assertion)
  • grep -c "Standards grounding" plugins/planning/skills/architect/context/plan-template.md ≥ 1
  • jq -r .version plugins/planning/.claude-plugin/plugin.json = 0.13.0; CHANGELOG head entry matches
  • grep -rniE "melodic-software|kyle" plugins/planning/skills/architect/SKILL.md plugins/planning/skills/setup/SKILL.md returns 0 hits (repo-agnostic)

Phase 4: review plugin — criteria rewire + new setup skill [DONE]

Review: code-design

Fresh-docs re-fetch (skills page) before edits.

File Action What changes
plugins/review/skills/quality-gate/context/criteria.md Modify "How to use" step 1 rewired: resolve criteria through the standards index per the contract ladder via the binding (.claude/standards.yamldocs/standards/README.md → declared/inferred location with offer-to-persist → ask once); existing REVIEW.md / docs-directory discovery becomes an inference source inside the ladder, not the primary; baseline (severity.md + agent checklists) stays the final fallback; tolerant-reader + broken-row Boy Scout lines added
plugins/review/skills/quality-gate/SKILL.md Modify One line in Shared inputs: criteria resolution goes through the plugin binding reference/standards-contract.md. DA-F1 resolution (user-approved 2026-07-17): Step 1 item 3 ("What conventions apply?") rewired through the same index resolution — one sentence, so ALL review modes (self/code/security/pr) inherit index-grounded conventions, not just criteria mode
plugins/review/skills/setup/SKILL.md Create New skill, planning:setup frontmatter shape (user-invocable: true, disable-model-invocation: true): standards bootstrap implementing the binding's normative Setup-and-migration section (same semantics as planning's — single home in the contract, no duplicated procedure prose)
plugins/review/skills/setup/evals/evals.json Create Bootstrap + idempotency cases
plugins/review/skills/quality-gate/evals/evals.json Modify Case: criteria mode in index-present fixture resolves through the index
plugins/review/tests/standards-binding.test.sh (location per repo plugins/**/*.test.sh precedent) Create Tripwire contract test (DA-F7): greps assert criteria.md's binding reference + ladder pointer and the setup skill's binding reference survive future prose edits
plugins/review/.claude-plugin/plugin.json Modify 0.9.00.10.0
plugins/review/CHANGELOG.md Modify 0.10.0 entry

Sanity Check:

  • grep -c "standards-contract.md" plugins/review/skills/quality-gate/context/criteria.md ≥ 1; test -f plugins/review/skills/setup/SKILL.md
  • grep -c "disable-model-invocation: true" plugins/review/skills/setup/SKILL.md = 1
  • jq -r .version plugins/review/.claude-plugin/plugin.json = 0.10.0; CHANGELOG head entry matches
  • grep -rniE "melodic-software|kyle" plugins/review/skills/ returns 0 hits in changed files

Phase 5: Verification, exercise, and gates [DONE]

Review: security

Runbook results distill into this PLAN's verification notes; raw transcripts stay in .work/proactive-vs-reactive-skills/.

  1. Mechanical: claude plugin validate plugins/planning --strict and claude plugin validate plugins/review --strict exit 0; full CI lanes green (incl. new standards-contract-sync, cross-plugin-source-drift, link-check).
  2. Fixture exercises (acceptance-criteria seam, T7): clean temp non-source repos via claude --plugin-dir
    • index-present: task touching surface X → plan cites X's sections; criteria mode resolves the same rows (build/review symmetry probe)
    • index-absent: skill infers from repo context, OFFERS persistence, zero unprompted writes (git status --porcelain empty after a declined offer)
    • mixed/external-row: external row routes; a deliberately broken row is surfaced with an offered fix, never silent
    • setup idempotency: bootstrap run twice → git diff --stat empty on the second run (both plugins' setup)
    • trivial-scale: trivial task → no standards fetch beyond ambient (grounding-depth gate probe)
    • personal overlay (reviewer F3): fixture with a .local.md that ADDS one rule and directly CONFLICTS on another → added rule applied with layer provenance named; conflicting rule loses to team (precedence inversion probe)
    • user-global layer (reviewer F2/F3): fixture with ~/.claude/standards/ populated (temp HOME) → layer discovered and applied; empirically record whether the outside-cwd Read prompts — the Brief's verified prompt-free fact covers the working directory only. If it prompts, STOP at the user-approval gate: accepted-cost note in the contract README vs design change is the user's call
    • canary rules (DA-F2 — anti-theater probe, REQUIRED in every grounding fixture): each fixture standard carries a distinctive counter-default rule the model would not produce unprompted (e.g. "repository pattern forbidden; use direct DbContext"). Assert the produced plan's CONTENT complies — not merely that sections are cited; the trivial-scale fixture's plan must NOT mention the canary. Review side: a fixture diff violating the canary must be flagged by the grounded criteria resolution. Citation-only compliance = FAIL
    • version skew (DA-F3): fixture index at contract 1.1.0 (simulated) read by a plugin bundling 1.0.0 → best-effort read + "update the plugin" message, NO migration offer, no nagging loop between the two setups
    • pre-existing README (DA-F5): fixture with a hand-authored docs/standards/README.md lacking the standards-contract key → treated as inference source, setup requires explicit confirmation before conversion, never overwrites
  3. Playbook gates: per-plugin migration gate + plugin-acceptance security review (no code execution added beyond the sync script — reviewed; no egress; no secrets; cache-isolation respected: skills reference only ${CLAUDE_PLUGIN_ROOT} paths).

Sanity Check:

  • Both claude plugin validate --strict runs exit 0
  • Every fixture-exercise row above recorded with PASS + evidence pointer in this PLAN before merge
  • grep -rn "docs/standards" plugins/planning plugins/review --include="*.md" -l hits only files that present the path as the documented default, never a hardcoded absolute/org path
  • git diff --name-only origin/main -- plugins/ | xargs grep -lniE "melodic-software|kyle|D:\\\\|/Users/|C:\\\\" returns 0 hits (sweeps ALL changed plugin files incl. evals and context files — reviewer F6)

Phase 5 verification record (2026-07-17)

Raw transcripts: .work/proactive-vs-reactive-skills/fixtures/ (memory tier). Probes ran headless
(claude -p --plugin-dir … --setting-sources project --add-dir <worktree> --permission-mode acceptEdits)
in clean temp non-source git repos.

Fixture row Probe(s) Verdict Evidence
Mechanical gates PASS claude plugin validate --strict exit 0 both plugins; sync --check 0; drift check 0; tripwires 9/9 + 8/8; shellcheck/actionlint/markdownlint clean
index-present (plan side) p01 PASS plan cites csharp.md sections; canary honored (direct DbContext, repository pattern refused); violating untracked file flagged
index-present (review symmetry) p02 PASS criteria resolve via binding ladder: csharp row + external row loaded, testing row skipped (selective); canary direct hit
canary anti-theater (review verdict) p03 PASS code mode flags repository pattern CRITICAL citing csharp.md (Step 1.3 rewire — all modes inherit)
index-absent p04 PASS infers from docs/CONTRIBUTING.md; assumption surfaced; zero standards writes
mixed/external + broken row p05 (re-confirmed p15/p15b) PASS broken architecture row surfaced + fix offered, never silent; plan proceeds non-blocked
trivial-scale p06 PASS abbreviated plan, no standards fetch, zero canary mention
setup idempotency (planning) p07/p08 PASS run 1 writes skeleton index @1.0.0 + setup-owned .gitignore; run 2 short-circuits — run twice, no diff
setup idempotency (review) p09/p10 PASS same semantics via /review:setup
personal overlay p11 PASS ADD (mutation testing) applied w/ personal provenance; CONFLICT (test-after) loses to team test-first
user-global layer p15 + p15b PASS denied read handled per contract (not an error, layer contributes nothing, reported); readable run: glob discovery, SEC-CANARY applied w/ user-global provenance
version skew (setup) p12 PASS newer index → zero writes, no migration offer, "update the review plugin", no nag loop
version skew (grounding) p13 rerun PASS after prose fix (explicit frontmatter compare in grounding step): skew surfaced, best-effort, "update the planning plugin"
pre-existing README p14 PASS presence test fails → confirmation gate holds; blanket pre-acceptance judged insufficient for conversion; nothing written
security review sub-agent PASS plugin-acceptance criteria: no code-exec surface added beyond CI-side sync script, no MCP, no secrets, ${CLAUDE_PLUGIN_ROOT}-only references, no egress; verdict PASS, zero blocking findings

STOP gate (reviewer F2) FIRED and resolved: outside-cwd reads are permission-gated (empirical:
headless denials on plugin-root and ~/.claude/standards/ paths). User accepted the cost
(2026-07-17); contract README's User-global layer section now carries the accepted-cost note
(denied read = layer contributes nothing, never an error) — synced to both bindings.

Probe-environment notes (not defects): headless slash invocation of quality-gate dies silently on
its gh pr list preflight permission denial (pre-existing plugin behavior; prose invocation used
for p02); plugin-root context files are unreadable to headless sessions without --add-dir.

Blast radius

MEDIUM-HIGH. Triggers matched: new convention + enforcement mechanism (CI drift job), infrastructure change (ci.yml), architecture decision spanning multiple plugins (new cross-plugin contract). ~20 files, two plugin version bumps, one new CI job. Reversible via git revert pre-merge; convention becomes load-bearing only after consumers adopt. Formal stress-test (Step 4 /devils-advocate) REQUIRED.

Stress-test summary

Two fresh-context passes, both verified against actual repo files before application:

  1. Plan-reviewer (Step 3): 1 CRITICAL (planning:setup's double ignore-file prohibition vs the shipped overlay ignore line — reconciled via setup-owned <standards_dir>/.gitignore + scoped prohibition), 2 IMPORTANT (user-global outside-cwd read-prompt unverified → Phase 5 empirical probe + approval gate; overlay/user-global layers unexercised → fixture rows added), 5 SUGGESTIONS (precedent citation corrected, changelog grep fixed, concrete diff-driven grep sweep, D4 basis restated with named residual, compaction/multi-task ambient guard added). All folded in.
  2. Devils-advocate (Step 4): 1 CRITICAL — the pilot as Brief-scoped cannot fully validate build/review symmetry because quality-gate's criteria mode is reference-only and the verdict-issuing modes (self/code/security/pr) never route through it; resolution is a user scope decision at approval (Option A: also rewire quality-gate Step 1 item 3 "What conventions apply?" through the index — one sentence, all modes inherit; Option B: keep Brief scope, downgrade the symmetry claim to "shared resolution ladder" and push verdict grounding to wave 2). 3 HIGH folded in: grounding-theater probes (canary-rule fixtures), directional version-skew semantics (never downgrade, "update the plugin"), frontmatter-semver + changelog enforcement in --check-bump. 5 MEDIUM folded in: normative index-presence test, resolution root + forward-slash paths, tripwire *.test.sh markers (evals are NOT CI-run — named gap), size guidance + selective section reads, delimited-ladder pointer discipline. 2 LOW folded in: setup short-circuit when conformant, no relative links in the canonical README. 1 accepted cost: lockstep manifest bumps at fleet scale (D1, generalization deferred with trigger).

No research-iterate round needed: findings were contract-text and probe-design gaps, not contested external claims; zero external-evidence disputes survived verification.

Execution shape

Sequential recommended: 1 → 2 → 3 → 4 → 5.

Phases 3 and 4 are file-disjoint (different plugins) and parallel-capable (~200 LOC each — above the material threshold), but both are judgment-heavy skill-prose edits that depend on the full design context; parallel worker briefs would have to duplicate that context, and drift between the two consumers is precisely the failure mode this topic exists to remove. Cost note: sequential forgoes ~1 phase of wall-clock saving; 2 parallel agents would roughly double token spend on the heaviest phases. Fallback unaffected (already sequential).

Phase Surface Basis
1 main-session contract prose; single source for everything downstream; judgment-heavy
2 main-session small scripts + CI wiring; test-first; tightly coupled to Phase 1 output
3 main-session judgment-heavy skill prose; full design context required
4 main-session same; symmetry with Phase 3 wording matters
5 main-session + fresh-context sub-agents for fixture exercises exercises need clean context to be honest probes; security review dispatches per playbook

Decisions made (gate-passed)

Decision What it changes in the plan Basis (evidence)
D1 [EXEC-SHAPE] Sync = dedicated script (hook-utils shape: sync/--check/--check-bump, opt-in by file presence) + registry entry + CI job — not a validate-plugin-contracts.mjs extension Phase 2 file list The byte-identical precedents read this session are hook-utils and artifact-protocol — NOT topic-docs, whose reference/topic-docs.md copies are per-plugin tailored bindings with no sync (Brief's "topic-docs precedent" citation corrected, reviewer F4). sync-hook-utils.sh scales by presence-glob (fleet waves add no script edits) and --check-bump mechanically enforces the version-bump acceptance criterion; the mjs lifecycleProtocolCopies alternative hardcodes the plugin list and has no bump check. Named cost accepted (DA-F10): every canonical edit forces manifest+changelog bumps in ALL carrying plugins — fine at 2, friction at fleet scale. Deferred (trigger: first fleet wave): generalize the two clone-shaped sync scripts into one src/glob-parameterized script and revisit bump tooling
D2 [EXEC-SHAPE] Contract README carries standards-contract: <semver> frontmatter Phase 1 README T6 (design, RESOLVED) requires setup to compare "index frontmatter vs bundled contract"; the bundled copy must be self-describing offline — CHANGELOG isn't synced. Mirrors the index's own key
D3 [EXEC-SHAPE] Grounding step placed at top of architect Step 2, not Step 1 Phase 3 SKILL.md edit Step 2 is where formulation inputs gather; a Step 1 gate would read as blocking (anti-waterfall constraint C in Brief); acceptance criterion binds grounding to plan formulation output
D4 (superseded at approval) Quality-gate rewire = criteria.md + Shared-inputs line + Step 1 item 3 Phase 4 scope Originally confined to criteria mode per Brief; devils-advocate showed verdict-issuing modes never route through criteria.md (grounding would materialize only on explicit criteria invocation). User approved the one-sentence Step 1.3 extension (DA-F1 Option A, 2026-07-17) so all modes inherit
D5 [EXEC-SHAPE] Bootstrap procedure lives normatively in the contract README; both setup skills implement by binding reference Phase 1 README section; Phases 3–4 setup prose stays thin Single-home rule (topic-docs convention, read this session); T6 makes migration semantics contract semantics; prevents 2→fleet procedure drift
D6 [EXEC-SHAPE] Binding filename reference/standards-contract.md Phases 2–4 references reference/standards.md would collide semantically with consumer standards files; artifact-protocol precedent names the copy after the contract content. Cheap to rename before merge — flag if you prefer standards.md
D7 [EXEC-SHAPE] Version bumps minor: planning → 0.13.0, review → 0.10.0 Phases 3–4 Additive features (new step, new skill), no breaking change to existing behavior
D8 [EXEC-SHAPE] Test seam = script test (test-first) + registered CI drift job + eval cases + fixture-repo exercises; no unit seams inside skill prose Phases 2, 5 T7 (design, directional) fixes the seam at the highest level; *.test.sh + evals.json are the repo's two existing test surfaces (read this session); prose changes get exercise-based verification with a documented test-after carve-out

Open questions

  • DA-F1 RESOLVED (user-approved 2026-07-17): Option A — quality-gate Step 1 item 3 rewired through the index; all review modes inherit grounded conventions. Phase 4 SKILL.md row carries the work item.
  • Wave-2 rollout inventory stays USER-RESERVED (Brief); it inherits the named residuals: sync-script generalization + bump tooling deferred to the first fleet wave (D1).

Handoff to implementation

User-approval gates

  • Any deviation from contract-spec.md content discovered mid-write (beyond D2) — stop and surface.
  • Fixture-exercise FAIL that suggests contract-schema change (not just prose fix) — stop; contract edits post-approval re-open design.
  • Security-review finding at Phase 5 — stop and surface.
  • User-global-layer exercise shows an outside-cwd Read permission prompt — stop; accepted-cost note vs design change is the user's call (reviewer F2).

Execution shape ([EXEC-SHAPE] tagged)

Sequential 1→5, main-session (table above). Phase 2 script work is test-first. PLAN.md status tags advance per phase; PLAN edits main-session only.

Mechanical work

  • Commit boundary per phase (Conventional Commits; squash-merge PR titled per repo rule). Phase 1+2 may share one commit if landed together (structural spine), 3 and 4 each their own.
  • Verification checkpoint per phase = its Sanity Check block, run before the phase commit.
  • Fresh-docs fetch at the top of Phases 3 and 4 (skills page) and before any ci.yml edit (hooks/settings pages not needed).
  • Sequential fallback: n/a (already sequential).

🤖 Generated with Claude Code

kyle-sexton and others added 10 commits July 17, 2026 10:22
Interview contract for making development-workflow skills load consumer
standards proactively (plan/build time) instead of colliding with them
at review time. Locks: proactive-by-default (no mode fork), `standards`
concern with three-layer resolution (user-global / team-tracked /
personal overlay, team wins conflicts), progressive-disclosure index,
and a two-consumer V1 pilot (planning:architect + review:quality-gate).
Schema detail deferred to /design; step placement to /architect.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ding

Five-phase plan approved after fresh-context plan review and devils-advocate
stress-test: contract home at docs/conventions/standards/, hook-utils-shape
sync machinery with frontmatter-bump enforcement, proactive grounding step in
planning:architect, quality-gate criteria rewire extended to Step 1.3 at
approval (DA-F1 Option A), setup bootstrap in both pilot plugins, and a
canary-rule fixture matrix guarding against grounding theater.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Canonical convention home docs/conventions/standards/: contract README
(index schema, layers, precedence inversion, resolution ladder, normative
setup-and-migration), concern-file JSON schema, changelog, worked index
example. PLAN Phase 1 marked DONE.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e 2)

Test-first sync script (sync/--check/--check-bump with frontmatter-semver
and changelog-heading enforcement), byte-identical binding copies in the
planning and review plugins, registry entry, and a standards-contract-sync
CI job wired into the aggregate gate. PLAN Phase 2 marked DONE.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Architect Step 2 opens with a standards-grounding formulation input
resolving the consumer index through the synced standards-contract
binding; plan template and reviewer gain the grounding element and
citation axis; setup gains the standards bootstrap concern with a scoped
ignore-file prohibition; tripwire test + eval cases; planning 0.13.0.
PLAN Phase 3 marked DONE.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ase 4)

Criteria mode and Step 1 conventions routing now resolve through the
synced standards-contract binding so all review modes inherit
index-grounded conventions; new idempotent /review:setup standards
bootstrap; tripwire test + eval cases; review 0.10.0. PLAN Phase 4
marked DONE.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fourteen-row fixture exercise matrix PASS (canary anti-theater, setup
idempotency, overlay precedence, directional version skew, pre-existing
README gate, user-global layer); plugin-acceptance security review PASS;
both plugins validate --strict. STOP-gate resolution: accepted-cost note
for outside-cwd user-global reads added to the contract and resynced;
architect grounding step now mandates the frontmatter version compare.
PLAN Phase 5 marked DONE with the verification record.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Plan and verification record live in the PR description; durable outcome
is the standards convention at docs/conventions/standards/; follow-ups
graduated to issues #288, #289, #290.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI runs scripts/sync-standards-contract.sh directly; 100644 mode made the
standards-contract-sync job fail with exit 126 on Linux runners.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 486b1370dc

ℹ️ 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".

Comment thread .github/workflows/ci.yml
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 49s —— View job


PR Review — feat: ground planning and review skills in consumer standards (V1 pilot)

  • Load tools
  • Read changed files and diff
  • Review contract/convention files
  • Review sync machinery and CI
  • Review plugin skill changes
  • Review tests
  • Post findings

Summary

Well-structured PR. The contract spec, sync machinery, tripwire tests, and CI wiring are coherent and follow the established hook-utils shape closely. Security posture is clean — no code execution from plugin content, no egress, no secrets, ${CLAUDE_PLUGIN_ROOT}-only references in consuming skills. The Codex-flagged executable-bit issue (P1) is confirmed fixed in 360d884.

A few items below worth attention before merge.


Findings

1. --check-bump changelog gate checks heading count, not version entry

File: scripts/sync-standards-contract.sh

base_headings=$(git show "$base:$changelog" 2>/dev/null | grep -c '^## ' || true)
head_headings=$(grep -c '^## ' "$changelog" || true)
if [[ "${head_headings:-0}" -le "${base_headings:-0}" ]]; then

The check verifies the total count of ## headings increased, not that a heading specifically matching the bumped version was added. A maintainer who adds any ## heading (e.g. a comment section) would pass without a real version entry. Not a code bug, but a process gap if someone edits the contract while adding unrelated changelog structure.

Mitigation: the frontmatter semver check in condition (b) gates content-under-a-frozen-version, so the two checks compose reasonably. Low risk in practice but worth noting.

2. standards.schema.json $id hardcodes the main branch

File: docs/conventions/standards/standards.schema.json

"$id": "https://github.com/melodic-software/claude-code-plugins/blob/main/docs/conventions/standards/standards.schema.json"

The $id references the main branch path. If the file is ever moved or the branch renamed, this becomes stale (though JSON Schema $id is a logical identifier, not a dereferenced URL, so there's no runtime breakage). Consistent with how topic-docs.schema.json is structured in this repo, so this is a known pattern — just flagging it for awareness.

3. --check-bump test suite has no compound-failure case

File: scripts/sync-standards-contract.test.sh

The suite tests each --check-bump failure condition (stale frontmatter, stale changelog, stale manifest) individually. There's no case exercising all three failing simultaneously — confirming all three STALE * messages are emitted and stale=1 accumulates correctly. Low risk since each branch is independently tested, but a compound fixture would lock the all-failures-report-before-exit behavior.

4. head_headings empty-string edge in changelog check

File: scripts/sync-standards-contract.sh

head_headings=$(grep -c '^## ' "$changelog" || true)

If $changelog doesn't exist at HEAD, grep exits 2 (file not found) and outputs nothing to stdout; || true absorbs the error and head_headings is the empty string. ${head_headings:-0} then gives "0". If the contract file changed but the changelog was never created, 0 -le 0 (base also had no headings) → STALE CHANGELOG fires correctly. If base had 1 heading and HEAD has none, it also fires correctly. The edge is subtle but the :-0 guard handles it. Confirmed correct — documenting here for the next reader of the script.

5. review/skills/setup/evals/evals.json missing absent-index fallback case for the grounding step

File: plugins/review/skills/setup/evals/evals.json

The three eval cases cover: bootstrap (new repo), idempotency (second run), and version skew (newer index). There's no eval for the pre-existing hand-authored README path that fails the presence test, requiring explicit confirmation. The PLAN's fixture matrix (row p14) covered this via probe exercise, and evals aren't CI-run per the PLAN's noted carve-out — flagging for completeness if the eval suite is ever extended.


Strengths

  • Contract single-home discipline is well-enforced. The reference/standards-contract.md binding copy, the pointer-only pattern in SKILL.md files, the tripwire tests that grep for restated-ladder phrases (present at the resolution root, exit without interview), and the --check-bump script that gates content-without-version-bump all compose into a strong drift-prevention ring.

  • Tripwire tests are targeted without being brittle. They assert load-bearing structural markers (heading inside Step 2, binding reference present, pointer phrase exists) rather than full-content diffs. A prose rewrite that preserves the contract structure keeps passing; a structural drift that would break the contract fails immediately.

  • CI wiring faithfully mirrors hook-utils-sync. The standards-contract-sync job structure (--check → test run → --check-bump on PRs only) is identical in shape and the job is correctly added to ci-status's needs list. fetch-depth: 0 on checkout ensures the --check-bump git ref lookup works.

  • --check-bump skips new plugins correctly. The [[ -z "$base_version" ]] guard prevents false-positives for plugins being introduced in the same PR as a contract change — the initial copy already carries the new content and doesn't need a "bump" since it has no prior version.

  • Precedence-inversion semantics are clearly normalized. The contract, both plugin setup skills, and the worked example all state the same rule (additive/tighten-only; team wins on direct conflict; provenance named). The tripwire tests won't catch prose drift here, but the single-home policy limits the blast radius to updating just the contract README.

  • No restated ladder in consuming SKILL.md files. The architect SKILL.md carries a pointer to the binding's Resolution ladder section — not a restated copy. The review tripwire test actively rejects restatement. Fix this →


Verdict

  • Ready to proceed — no blocking findings
  • Needs fixes — findings 1 and 3 are worth addressing before merge (low complexity); 2, 4, and 5 are informational

Recommended before merge: Add the compound-failure test case (#3) since it locks behavior that's currently exercised only implicitly. Finding #1 (changelog count vs. version-entry check) is a low-risk process gap that can be deferred if there's no appetite to add a version-extract check now.

--check-bump's changelog condition now requires a '## <version>' heading
matching the bumped standards-contract frontmatter instead of a raw
heading-count increase, and the test suite gains a compound-failure case
asserting all three STALE conditions report in one run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Claude-review findings triaged (evidence-checked against the diff):

# Finding Class Action
1 Changelog gate counts headings, not a version-matched entry VALID Fixed in 2a00197--check-bump now requires a ## <version> heading matching the bumped standards-contract frontmatter
2 standards.schema.json $id hardcodes blob/main INFORMATIONAL Matches the repo's existing topic-docs.schema.json pattern; $id is a logical identifier — no change
3 No compound-failure test for --check-bump VALID Fixed in 2a00197 — new scenario asserts all three STALE conditions report in one run (suite 11/11)
4 head_headings empty-string edge INFORMATIONAL Reviewer confirmed correct; that code path was replaced by the version-matched check in 2a00197
5 Review setup evals lack the pre-existing-README case VALID (defer) Covered by the PLAN's p14 fixture exercise; evals are not CI-run (documented carve-out). Will ride the next evals extension

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a001971da

ℹ️ 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".

Comment thread docs/conventions/standards/README.md
Comment thread docs/conventions/standards/README.md Outdated
kyle-sexton and others added 2 commits July 17, 2026 15:52
…ctive-skills

# Conflicts:
#	plugins/planning/.claude-plugin/plugin.json
#	plugins/planning/CHANGELOG.md
Review findings: the resolution ladder now names the personal-layer
discovery step every rung performs (overlay glob + user-global read,
applied per precedence), and a pre-existing standards-root .gitignore is
consumer-owned — setup verifies the overlay line and surfaces a missing
one without writing. Bindings resynced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d68c03f56

ℹ️ 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".

Comment thread docs/conventions/standards/README.md Outdated
Comment thread plugins/planning/skills/architect/context/plan-template.md Outdated
kyle-sexton and others added 2 commits July 17, 2026 15:59
Review findings: the bootstrap now runs the committed-root git
check-ignore guard before writing the skeleton (topic-docs precedent),
and the plan template's scale note grounds small plans with a brief
citation instead of skipping — only trivial scale skips, matching the
architect step, the eval, and the build/review-symmetry goal. Bindings
resynced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ctive-skills

# Conflicts:
#	plugins/planning/.claude-plugin/plugin.json
#	plugins/planning/skills/setup/SKILL.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cc5bc62b7

ℹ️ 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".

Comment thread docs/conventions/standards/README.md
Review finding: a File value containing a subdirectory was ambiguous
between in-root and repo-relative external resolution. Rows now resolve
in-root first, then from the resolution root; a both-exist collision is
surfaced (skills best-effort the in-root file, setup validation flags it
and offers a rename). Bindings resynced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f678595a5

ℹ️ 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".

Comment thread docs/conventions/standards/README.md
Comment thread scripts/sync-standards-contract.sh Outdated
Review findings: setup's bootstrap now applies the git check-ignore
guard to .claude/standards.yaml before persisting a relocation (an
ignored concern file strands future clones on the default root), and
--check-bump treats standards.schema.json as contract surface so a
schema-only change requires the same version, changelog, and plugin
bumps (12-scenario suite green). Bindings resynced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

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


PR Review — feat: ground planning and review skills in consumer standards (V1 pilot)

  • Read changed files and diff
  • Review contract/convention files (docs/conventions/standards/)
  • Review sync machinery and CI (scripts + CI yml)
  • Review plugin skill changes (planning + review)
  • Review tests and evals
  • Post findings

Summary

Well-constructed PR that successfully closes out all findings from the previous multi-round review. The iterative loop (claude-review → Codex → kyle-sexton → Codex → kyle-sexton — 9 rounds) converged on a solid result. All prior blocking findings are resolved; this pass found no new blocking issues.


Verification of prior findings

Prior finding Status
P1 Codex: scripts committed non-executable Fixed in 360d884 — all four scripts are 100755
#1 (claude): changelog gate counted headings, not version entry Fixed in 2a00197--check-bump now requires ## <semver> matching the bumped frontmatter
#3 (claude): no compound-failure test case Fixed in 2a00197 — scenario 9/9 asserts all three STALE messages emit before exit
Codex P2: personal layers missing from Resolution ladder Fixed in 0d68c03 — "Personal layers (every rung)" step now normative
Codex P2: pre-existing .gitignore branch unspecified Fixed in 0d68c03 — consumer-owned file: verify + surface, no write
Codex P2: no git check-ignore guard for ignored roots Fixed in ab97562 — guard runs before skeleton write
Codex P2: small plans skipped grounding incorrectly Fixed in ab97562 — plan-template.md now: trivial=skip, small=brief, medium/large=full
Codex P2: disambiguation of external vs in-root row paths Fixed in 4f67859 — deterministic lookup documented, ambiguity flagged to consumer
Codex P2: git check-ignore guard missing for .claude/standards.yaml relocation Fixed in f3e1898 — same guard applied to concern file before persisting
Codex P2: schema-only changes bypass --check-bump Fixed in f3e1898 — diff gate now covers standards.schema.json; test case 12/12

New findings

1. New-plugin + schema-only edge in --check-bump — informational

File: scripts/sync-standards-contract.sh:87-100

When a schema-only change lands in the same PR as a brand-new carrying plugin (absent at base), the STALE VERSION check is correctly skipped for the new plugin via [[ -z "$base_version" ]]. If all current carrying plugins are also new, STALE VERSION fires for none of them. STALE CONTRACT VERSION still fires (content changed under a frozen frontmatter), so the gate isn't silently bypassed — but the new-plugin case produces a slightly reduced error set. This is a hypothetical edge (a schema-only change paired with a new plugin introducing the contract for the first time), the STALE CONTRACT VERSION guard stops it, and the test suite covers schema-only changes with existing plugins. No action needed; documenting for the next reader of the script.

2. Review setup evals lack the ignored-concern-file guard path — informational

File: plugins/review/skills/setup/evals/evals.json

The three review setup evals cover bootstrap, idempotency, and version skew. They don't exercise the git check-ignore -v guard on .claude/standards.yaml (contract fixed in f3e1898). That path was validated by the fixture exercise (analogous to planning setup eval #3 which does cover the ignored-root case). Evals are not CI-run per the PLAN's documented carve-out; the guard is normative in the contract and tested in planning. Flagged for the next evals extension wave.


Strengths

  • Contract single-home is enforced end-to-end. The contract README is the normative authority; both setup skills implement it by pointer reference. The tripwire tests (9 assertions in planning, 8 in review) actively reject restated-ladder phrases from skill prose. The --check-bump gate rejects content drift under a frozen frontmatter version. These four mechanisms compose into a drift-prevention ring that survives future prose edits.

  • --check-bump version-entry check (fixed in 2a00197) is the right semantics. The previous heading-count approach would have allowed any ## section to satisfy the gate. The current regex (^## ${head_contract_re}( |$)) ties the changelog entry to the exact bumped version, so the check is a meaningful invariant.

  • Compound failure test (scenario 9) correctly exercises all three STALE * conditions in one run and asserts all three messages appear before the single exit — confirming stale=1 accumulates without an early bail.

  • Schema-only gate (scenario 12) correctly triggers STALE CONTRACT VERSION when only standards.schema.json changes, enforcing that schema changes are versioned even when the README prose is unchanged. The diff gate (git diff --quiet "$base" -- "$src" "$schema") covers both surfaces.

  • Tripwire test placement discipline. Tests live under plugins/*/tests/ which scripts/run-plugin-tests.sh picks up automatically. Existing CI gate (plugin-gate) runs them. No manual registration needed for new plugins in future waves.

  • New-plugin exemption in --check-bump is correctly scoped. The [[ -z "$base_version" ]] guard fires only when a plugin manifest is absent at the base ref — a new plugin whose initial copy already carries the updated contract content. Scenario 11 covers this case; it correctly passes where the stale-manifest scenarios fail.

  • Version bumps are correct. planning: 0.15.0 → 0.16.0 (additive new step + concern in setup, no breaking change); review: 0.9.0 → 0.10.0 (additive new skill + criteria rewire). Binding copies match the canonical byte-for-byte (cmp confirmed).

  • Security posture is clean. No code execution surface added to consumer-facing plugin content. CI-side sync script is infrastructure-only. ${CLAUDE_PLUGIN_ROOT} used consistently for all plugin-relative paths. No MCP, no secrets, no egress.


Verdict

  • Ready to proceed — no blocking findings
  • Needs fixes

Both observations above are informational. The PR is ready to merge.

@kyle-sexton
kyle-sexton merged commit 1579461 into main Jul 17, 2026
20 checks passed
@kyle-sexton
kyle-sexton deleted the feat/proactive-vs-reactive-skills branch July 17, 2026 20:40
kyle-sexton added a commit that referenced this pull request Jul 19, 2026
## Summary

First slice of wave #318 (epic #313): the five doctrine-update findings
from the fleet audit, fixed in the docs rather than the plugins.

1. **Kill-switch reconciliation** — verified already landed (playbook
security review requires the per-hook userConfig boolean; the
generalization mapping routes `HOOK_<OLD>_ENABLED` → userConfig). No
edit; recorded here so the finding closes.
2. **Convention registry** gains the repository-standards row
(`docs/conventions/standards/` landed with #291, never registered).
3. **Topic-docs contract** — the "`.claude/observability/` is the sole
sanctioned generated surface under `.claude/`" claim now scopes to the
contract itself; the platform's native subagent `memory: project|local`
roots (`.claude/agent-memory[-local]/`) are Claude Code's to govern, not
the contract's to forbid.
4. **planning 0.21.0** — `domain-driven-design` manifest dependency
downgraded to presence-gated collaboration (design boundary:
`dependencies` are for genuinely-broken-without; every planning skill
works standalone). All five invocation sites gated with stated
fallbacks.
5. **Seam-phrasing owner doc** —
`docs/conventions/seam-phrasing/README.md` (gate + fallback + ownership
framing); registry row flips from "Unowned".

Remaining #318 slices: registry single-home pointer fixes (session-flow,
code-tidying), the kindle-dedrm update-flow security restructure, the
two mutation-on-bare-invocation fixes, and the naming-grammar rename
matrix (breaking — prepared as explicit per-name decisions, not
blanket-renamed).

## Verification

markdownlint (docs + planning), `typos`, `validate-plugins.sh`, contract
gate (33 setup skills): green locally.

## Related

- Part of #318
- Part of #313

No linked issue: doctrine slice of #318 — the issue closes with the
wave's final slice.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant