[#314] feat: wire template-override resolution into skills

[rucka]

Summary

What Changed

Adds ONE shared "Template Resolution" instruction (.pair/knowledge/skill-conventions/template-resolution.md, dataset + .claude mirror) — a plain file-existence check on .pair/adoption/tech/templates/<name>-template.md, falling back to the KB path unchanged when no override exists — and wires a pointer to it into every SKILL.md that links a collaboration template. Also fixes apps/website/content/docs/customization/templates.mdx to list all shipped templates and describe the mechanism accurately (was previously claiming behavior that didn't exist), and resolves the orphaned branch-template.md (now consumed by /pair-capability-publish-pr for branch-format parsing).

Why This Change

templates.mdx already documented an override mechanism ("the AI reads adoption files first") that no skill actually implemented — every SKILL.md hardcoded a relative link straight to the KB default. A project that followed the docs and dropped a custom template into .pair/adoption/tech/templates/ would have it silently ignored. This closes that doc/behavior gap with one shared, DRY instruction (not 35 copies of the same logic) and keeps the common case (no override) byte-identical to today's behavior.

Story Context

User Story: #314 — "Wire template-override resolution into skills (docs already claim it works — it doesn't)"
Acceptance Criteria:

1. No override present → resolves to KB default exactly as today (idempotent, zero behavior change). ✅ covered by conformance test.
2. Override present → skill resolves and uses it, verified for a process-composed template (pr-template.md via /pair-process-implement) and a capability-composed one (adr-template.md via /pair-capability-record-decision). ✅
3. One shared instruction, not duplicated across every skill. ✅ skill-conventions/template-resolution.md, referenced by pointer.
4. templates.mdx accuracy — lists all shipped templates, mechanism described correctly, orphan resolved. ✅

Changes Made

Implementation Details

• New shared instruction packages/knowledge-hub/dataset/.pair/knowledge/skill-conventions/template-resolution.md (+ root mirror .pair/knowledge/skill-conventions/template-resolution.md): file-existence check on .pair/adoption/tech/templates/<name>-template.md, KB path fallback, override wins whole-file (no merge).
• Registered in skill-conventions/README.md.
• Pointer added to all 16 template-linking SKILL.md (dataset .skills/ sources + regenerated .claude/ mirror) at their template-use sites — see deviation note below on the count.
• record-decision wired in the same commit — its earlier mutex with [#224] refactor: assess-* output-only; record-decision sole adoption writer #310 is cleared ([#224] refactor: assess-* output-only; record-decision sole adoption writer #310 merged to main), so ADR/ADL/DDR/analysis-log resolution is included here rather than deferred.
• apps/website/content/docs/customization/templates.mdx rewritten: 19 shipped templates listed (was 14), mechanism corrected to the plain file-existence check (dropped the inaccurate "AI reads adoption first" framing), ## Templates reframed as an audit-trail listing (not a trigger — the file-existence check on disk is authoritative), branch-template.md orphan resolved (documented as consumed by /pair-capability-publish-pr for branch-format parsing).
• Root (.claude/) regenerated via node apps/pair-cli/dist/cli.js update --offline --source packages/knowledge-hub/dataset — verified byte-identical re-run (clean git status after re-running), keeping dataset↔root mirror consistent per project convention.

Files Changed

• Added: packages/knowledge-hub/dataset/.pair/knowledge/skill-conventions/template-resolution.md, mirrored root .pair/knowledge/skill-conventions/template-resolution.md, packages/knowledge-hub/src/conformance/template-resolution.test.ts.
• Modified: 16 dataset SKILL.md files + their .claude/skills/** mirrors (32 files total), skill-conventions/README.md (×2, dataset+root), apps/website/content/docs/customization/templates.mdx.

Testing

Test Coverage

• Unit/Conformance Tests: new packages/knowledge-hub/src/conformance/template-resolution.test.ts — 15 tests (test-first, confirmed RED before the instruction file/pointers existed, GREEN after): asserts every template-linking SKILL.md (dataset + mirror) carries the resolution pointer, and the override-wins/idempotent-fallback semantics of the shared instruction text.

Test Results

knowledge-hub vitest:  364/364 passing (incl. 15 new conformance tests)
check:links:           all markdown links valid (dataset + root)
ts:check (turbo):      8/8 packages passing
mdlint:check (turbo):  6/6 packages passing
Mirror consistency:    `pair update --offline` re-run produces a clean git status (byte-identical)

Testing Strategy

• Idempotent-default path: verified the conformance suite asserts "no override present ⇒ unchanged KB path" as the default resolution outcome.
• Override-wins path: verified for one process-composed template (pr-template.md, consumed by /pair-process-implement → /pair-capability-publish-pr) and one capability-composed template (adr-template.md, consumed by /pair-capability-record-decision) per AC2's explicit ask.
• Dry-run for reviewer: to verify manually, drop a file at .pair/adoption/tech/templates/pr-template.md and confirm /pair-capability-publish-pr's resolution step picks it up; remove it and confirm it falls back to the KB default unchanged.

Reviewer Guide

Two deliberate deviations from the story/orchestrator's stated counts — please confirm you're OK with these

1. 16 skills wired, not 15. The story's count (14 template-linking skills + record-decision = 15) predates /pair-capability-publish-pr, which merged later (pair-process-implement composes checkpoint + publish-pr via subagent (resume from checkpoint) #256/[#256] feat: implement composes checkpoint + publish-pr via handoff-only subagent #359). publish-pr factually links pr-/commit-/branch-/checkpoint-templates and is where the pr-template AC2 resolution actually happens in practice (/pair-process-implement composes publish-pr for its PR step). Excluding it would leave the single most important template consumer unwired and fail AC2 in spirit. The conformance test asserts the count-agnostic invariant "every template-linking SKILL.md carries the pointer" (currently ≥15) rather than hardcoding 15, so it's robust to this and future additions.

2. 19 shipped templates listed, not 17. At story refinement (2026-07-13) the KB shipped 17 template files; by now it ships 19 (*-template.md, excluding README). The doc previously listed only 14 (already stale against the 17 at refinement time); this PR adds all 5 that were missing (checkpoint-, manual-test-case-, manual-test-report-, context-map-, subdomain-context-template.md) to reach the current 19.

3. record-decision (T3) included, per the orchestrator's explicit directive that [#224] refactor: assess-* output-only; record-decision sole adoption writer #310 (which held an earlier mutex on this file) is merged — confirmed via git log before touching it.

Review Focus Areas

1. Confirm the shared-reference pattern (skill-conventions/<topic>.md + a one-line pointer at each use site) matches the existing convention (e.g. way-of-working-pm-resolution.md) rather than introducing a new pattern.
2. Confirm the file-existence check is described consistently between the shared instruction and templates.mdx (single source of truth: the files on disk, not the ## Templates prose).
3. Spot-check 2-3 of the 16 wired SKILL.md files (dataset + mirror) to confirm the pointer reads naturally at its use site and doesn't duplicate existing adoption-check prose.

Dependencies & Related Work

Related PRs

• Builds on: [#256] feat: implement composes checkpoint + publish-pr via handoff-only subagent #359 (/pair-capability-publish-pr merged, clearing the count baseline used here), [#224] refactor: assess-* output-only; record-decision sole adoption writer #310 (merged — cleared the record-decision/SKILL.md mutex referenced in the orchestrator directive).

Follow-up Work

• None identified — this closes the doc/behavior gap for template overrides as scoped.

Refs: #314

[rucka]

Code Review — PR #367 (Independent)

Review Information

PR Number: #367
Author: rucka
Reviewer: Independent reviewer (/pair-process-review)
Review Date: 2026-07-23
Story/Epic: #314 — Wire template-override resolution into skills
Review Type: Feature (skill-instruction + docs)
Classification: risk:red · cost:green (confirmed — no raise; docs/instruction-only diff, business-impact red keeps tier)

Review Summary

Overall Assessment

• Approved
• Approved with Comments
• Request Changes — two Major gaps re-open the exact doc/behavior mismatch this story exists to close
• Comment Only

Key Changes Summary

Adds one shared skill-conventions/template-resolution.md (dataset + root mirror) — a plain file-existence check on .pair/adoption/tech/templates/<name>-template.md, KB fallback unchanged — and wires a one-line pointer into 16 template-linking SKILL.md (dataset + .claude mirror). Rewrites templates.mdx (19 templates, accurate mechanism, branch-template orphan resolved). New 15-test conformance suite.

Business Value Validation

The core value — a project's PR/commit/story/ADR overrides are actually honored instead of silently ignored — is delivered for the directly-linked template sites. Two consumption paths still silently use the KB default (see Major findings), which is the precise anti-goal in the story's "So that" clause.

Code Review Checklist — Findings

Major Issues (should fix before merge)

• packages/knowledge-hub/dataset/.skills/process/review/merge-and-cascade.md:14 + .skills/process/implement/post-review-merge.md:15 (+ .claude mirrors) — The commit-template links at these two merge-commit-drafting sites are NOT wired to override-first resolution (no pointer). A project that overrides commit-template.md gets the override at per-task commits (implement Step 2.7.5, wired) but the merge/squash commit message (/pair-process-review Phase 6 and /pair-process-implement Phase 4) still uses the unmodified KB default. This is exactly the "skill quietly using the unmodified KB default" the story's So-that clause forbids, applied to a first-class, commonly-overridden template (the doc's own Step-2 example is commit-template). The conformance test does not catch this because collectSkillFiles only walks SKILL.md, never these auxiliary instruction files — so the DoD item "grep confirms zero skills still hardcode a template link without going through the resolution instruction" is not actually satisfied. Fix in this PR: add the one-line resolution pointer at both commit-template use-sites (dataset + mirror).

• apps/website/content/docs/customization/templates.mdx:72-73,87 — The doc lists context-map-template.md and subdomain-context-template.md with specific "Used by" skills, and line 87 makes a blanket claim that every listed template (all 19) resolves override-first via the file-existence check. But no SKILL.md and no resolution-pointer-carrying file links these two — they are consumed only through the context-map-maintenance.md guideline (architecture/design-patterns/context-map-maintenance.md:23,47,69,70), which hardcodes the KB path with no resolution pointer. A project dropping .pair/adoption/tech/templates/context-map-template.md would be silently ignored — re-introducing, for 2 of 19 templates, the very doc/behavior mismatch this story is chartered to eliminate. This violates AC5 ("the override mechanism's description matches the actual implemented resolution logic"). Fix in this PR (low cost): either add the resolution pointer to context-map-maintenance.md's two template-link sites so these overrides resolve, or narrow the doc so it does not imply guideline-consumed templates are override-resolvable.

Minor Issues (consider addressing)

• packages/knowledge-hub/src/conformance/template-resolution.test.ts:25-29 + src/tools/skills-conformance-check.ts:316 — The conformance invariant is structurally blind to non-SKILL.md instruction files (the root cause that lets the Major Setup & Project Management Integration #1 gap pass CI). Broaden the template-link scan to all .md under .skills/** (and the mirror), so auxiliary composed files are held to the same pointer requirement. Overlaps with Major Setup & Project Management Integration #1's fix.

Questions

• AC4 / DoD "reflected in the KB guideline that defines it" — the ## Templates audit-trail semantics are documented in template-resolution.md and templates.mdx; confirm no separate way-of-working structural guideline still describes that section as a resolution trigger. (Grep found no stale claim; treating AC4 as met.)

AC Verification

AC	Verdict	Evidence
AC1 — no override ⇒ KB default unchanged (idempotent)	Met	Convention file states zero-behavior-change/idempotent; conformance asserts it
AC2 — override wins, verified process- + capability-composed	Met (for wired sites)	implement/publish-pr (pr+commit+branch) and record-decision (adr/adl/ddr/analysis-log) carry pointer, dataset + mirror
AC3 — single shared instruction, referenced (not duplicated)	Met	16 SKILL.md reference one template-resolution.md; registered in README
AC4 — ## Templates = audit trail, not trigger	Met	Documented in convention + mdx
AC5 — mdx lists all shipped templates, mechanism accurate, orphan resolved	Partially met	All 19 listed and branch-template orphan resolved (publish-pr consumes it), BUT blanket "all overridable" claim inaccurate for context-map/subdomain-context (Major #2)

Testing Review

New conformance suite (15 tests) asserts the convention content (file-existence check, override-wins, idempotent fallback, audit-trail, edge cases) and pointer presence across every SKILL.md (dataset + mirror) + the 3 AC2 consumers. All assertions verified satisfied by inspection (suite GREEN, consistent with PR claim). Gap: coverage excludes auxiliary composed .md files (Minor).

Security Review

No security surface. Docs/instruction change plus one test file; no runtime code, inputs, secrets, or auth. Verdict: green, 0 findings.

Adoption Compliance

Dataset↔root mirror consistent: template-resolution.md byte-identical in both; root README carries the new row; all mirror SKILL.md pointers present. Relative link paths resolve in both dataset (../../../ → dataset root) and mirror (../../../ → repo root). Consistent with the existing shared-reference convention (way-of-working-pm-resolution.md).

Risk Assessment

Risk	Impact	Probability	Mitigation
Commit-template override silently ignored at merge-commit	Med	Certain when a team overrides commit-template + squashes	Wire the 2 aux merge sites (Major #1)
Doc overpromises override for 2 guideline-only templates	Med	Certain when a team overrides those templates	Wire guideline or narrow doc (Major #2)

Decision

CHANGES-REQUESTED. Both Majors are small, in-scope, and fixable in this PR; no new story needed. Everything else (shared DRY instruction, 16-skill wiring, mirror consistency, mdx completeness, branch-template orphan resolution, conformance suite) is solid and matches the ACs.

[rucka]

Remediation — round 1 (commit 21c90d4)

Per finding:

• [Major] commit-template merge-commit sites not wired to override-first (merge-and-cascade.md:14 + post-review-merge.md:15) → added the (resolve override-first — [template resolution](…)) pointer at the commit-template use-site in both aux files, dataset + .claude mirror. A project that overrides commit-template.md now gets its override at the merge/squash commit drafted in /pair-process-review Phase 6 and /pair-process-implement Phase 4, not just per-task commits — packages/knowledge-hub/dataset/.skills/process/review/merge-and-cascade.md, packages/knowledge-hub/dataset/.skills/process/implement/post-review-merge.md, .claude/skills/pair-process-review/merge-and-cascade.md, .claude/skills/pair-process-implement/post-review-merge.md
• [Major] context-map-template & subdomain-context-template overrides silently ignored (context-map-maintenance.md:23,47) → added the resolution pointer at both template-link use-sites in the guideline (dataset + root), so a project dropping .pair/adoption/tech/templates/context-map-template.md or subdomain-context-template.md is now honored. This makes the templates.mdx blanket "all 19 resolve override-first" claim (line 87) true for these 2 of 19 (AC5) — chose the "make overrides resolve" option over narrowing the doc — packages/knowledge-hub/dataset/.pair/knowledge/guidelines/architecture/design-patterns/context-map-maintenance.md, .pair/knowledge/guidelines/architecture/design-patterns/context-map-maintenance.md
• [Minor] conformance scan structurally blind to non-SKILL.md instruction files (template-resolution.test.ts:25-29 + skills-conformance-check.ts:316) → added collectSkillMarkdownFiles() (recursive walk of all .md, not just SKILL.md) to the tested production module; the dataset + mirror pointer-invariant now scans disclosed aux files. This is the root cause that let Major Setup & Project Management Integration #1 pass CI green; the broadened scan is now green with the Major Setup & Project Management Integration #1 fix in place (and confirms no other aux file has the gap). Kept a separate SKILL.md-only count for the ">=15 wired skills" invariant — packages/knowledge-hub/src/tools/skills-conformance-check.ts, packages/knowledge-hub/src/conformance/template-resolution.test.ts
• [Questions] AC4 — no other guideline claims way-of-working ## Templates is the resolution trigger → confirmed. Grep for resolution trigger / ## Templates across .pair, dataset, and website content: the only "resolution trigger" mentions are in template-resolution.md correctly negating it; the ### Templates headings in how-to 10/11 and decision-frameworks README are reference lists, not trigger claims. No action needed.

Not changed (escalated): none
Quality gates: PASS — knowledge-hub vitest 364/364 (incl. 15 template-resolution), check:links (dataset + root), skills:conformance 39/39, ts:check (turbo, 8 pkgs), mdlint clean; changed TS files prettier-clean (pre-existing drift in refine-story-draft-to-ready.test.ts / CLAUDE.md is not from this story and not staged).
→ Re-review requested.

[rucka]

Code Review — PR #367 (Independent re-review)

Review Information

PR Number: #367
Author: rucka
Reviewer: Independent reviewer (/pair-process-review)
Review Date: 2026-07-23
Story/Epic: #314 — Wire template-override resolution into skills
Review Type: Feature (skill-instruction + docs)
Classification: risk:red · cost:green (confirmed — no raise; docs/instruction-only diff, business-impact red keeps tier)

Review Summary

Overall Assessment

• Approved
• Approved with Comments — both prior Majors genuinely resolved; only Minor doc/test-precision nits remain, none blocking
• Request Changes
• Comment Only

Key Changes Summary

One shared skill-conventions/template-resolution.md (dataset + root mirror, byte-identical) — a plain file-existence check on .pair/adoption/tech/templates/<name>-template.md, KB fallback unchanged — wired via a one-line pointer into every file that links a collaboration template: 16 SKILL.md (dataset + .claude mirror), 2 auxiliary composed files (merge-and-cascade.md, post-review-merge.md), and the context-map-maintenance.md guideline. templates.mdx rewritten (19 templates, accurate file-existence mechanism, branch-template orphan resolved). New 15-test conformance suite + broadened collectSkillMarkdownFiles scanner.

Business Value Validation

Core value delivered and now complete: a project's PR/commit/story/ADR/context-map overrides are honored instead of silently ignored, across every site that links a template. Verified: all 19 shipped templates have at least one pointer-carrying consumer, so the doc's blanket "override-first" claim is substantively true.

Verification of Prior Findings

Prior finding	Status	Evidence
[Major] commit-template not wired at merge-commit sites (merge-and-cascade.md, post-review-merge.md + mirrors)	RESOLVED	Pointer added at merge-and-cascade.md:14 and post-review-merge.md:15, dataset + .claude mirror. Merge/squash commit now resolves override-first.
[Major] context-map-template & subdomain-context-template overrides silently ignored (guideline, no pointer)	RESOLVED	Pointer added at context-map-maintenance.md:23,47 (dataset + root). Both overrides now resolve; templates.mdx:87 blanket claim now true for these 2 of 19.
[Minor] conformance scan blind to non-SKILL.md files	RESOLVED	New collectSkillMarkdownFiles() walks all .md under .skills; test's pointer invariant now scans aux files. Zero offenders across dataset + mirror.
[Questions] AC4 — no other guideline claims ## Templates is the resolution trigger	CONFIRMED	## Templates documented as audit trail in convention + mdx; no stale trigger claim found.

AC Verification

AC	Verdict	Evidence
AC1 — no override ⇒ KB default unchanged (idempotent)	Met	Convention states zero-behavior-change/idempotent; conformance asserts fallback path
AC2 — override wins, verified process- + capability-composed	Met	implement/publish-pr (pr+commit+branch+checkpoint), record-decision (adr/adl/ddr/analysis-log) carry pointer, dataset + mirror; AC2 consumer tests green
AC3 — single shared instruction, referenced not duplicated	Met	One template-resolution.md; 16 SKILL.md + 2 aux + 1 guideline reference it; registered in README
AC4 — ## Templates = audit trail, not trigger	Met	Documented in convention + mdx
AC5 — mdx lists all shipped templates, mechanism accurate, orphan resolved	Met	19 listed (matches KB corpus exactly); file-existence mechanism accurate; branch-template consumed by /pair-capability-publish-pr. Minor "Used by" attribution imprecision on 2 rows (see below) — does not affect resolution

Code Review Checklist — Findings

Minor Issues (consider addressing)

• apps/website/content/docs/customization/templates.mdx:72-73 — The "Used by" cells attribute context-map-template.md to /pair-capability-record-decision (DDR flow) and subdomain-context-template.md to /pair-capability-map-subdomains / /pair-capability-map-contexts. But none of those SKILL.md files link either template — record-decision's DDR flow creates a minimal context-map.md inline (SKILL.md:145) without reading context-map-template.md, and map-*'s SKILL.md link only subdomain-template.md/bounded-context-template.md. Both templates are consumed solely via the context-map-maintenance.md guideline (which the map-contexts half of line 72 already correctly notes with "via context-map maintenance"). Impact: a team auditing whether record-decision honors a context-map-template override would be misled — it doesn't use that template. Override resolution itself is unaffected (guideline path carries the pointer). Recommendation: attribute both rows to the context-map-maintenance guideline and drop/qualify the record-decision (DDR flow) claim for context-map-template.

• context-map-maintenance.md:69-70 (dataset + root) — The "Related Documents" footer links context-map-template.md and subdomain-context-template.md without the resolution pointer, while the file's own use-sites (23, 47) carry it. Not a behavior gap (footer is navigational; resolution happens at read-sites), and leaving it bare is defensible since a pointer belongs at use-sites, not a see-also list — but it is internally inconsistent and leaves the prior finding's cited lines 69-70 untouched. Recommendation: accept as navigation-only, or add a one-line note for consistency.

• .pair/knowledge/skill-conventions/README.md:11 (dataset + root) — The new row hardcodes "16 skills", a count not enforced by any conformance gate (checkProseCounts/checkCatalogCounts cover only next/SKILL.md + the 3 onboarding prose files). If a future template-linking skill is added, this figure goes stale silently. Recommendation: drop the hard count (e.g. "all template-linking skills") or extend the gate.

• packages/knowledge-hub/src/conformance/template-resolution.test.ts — The broadened pointer invariant scans DATASET_SKILLS + MIRROR (.claude/skills) but not the guidelines corpus (.pair/knowledge/guidelines), where the resolved Major Collaborative Knowledge Base #2 fix now lives (context-map-maintenance.md). A regression that removed that guideline's pointer would pass CI green — the same class of blind spot as the (now-fixed) aux-file gap, one corpus over. Recommendation: extend the scan to guideline template-link sites, or explicitly accept guidelines as out-of-charter for this invariant.

Positive Feedback

• DRY shared-instruction pattern matches the existing way-of-working-pm-resolution.md convention rather than inventing a new mechanism.
• Dataset↔root mirror is byte-identical for the convention file; SKILL.md mirror diffs are exactly the expected pair update transforms (name/slash-command prefixing, link-path normalization) — the pointer lines themselves are identical and all relative targets resolve.
• Every one of the 19 shipped templates now has a pointer-carrying consumer; the doc's blanket override-first claim is substantively true.
• Conformance suite is test-first (RED→GREEN) and asserts the count-agnostic ">=15 SKILL.md" invariant, robust to future additions.

Security Review

No security surface. Docs/instruction change plus one test file; no runtime code, inputs, secrets, or auth. Verdict: green, 0 findings.

Adoption Compliance

Dataset↔root mirror consistent; new README row present in both; every template-linking file (16 SKILL.md + 2 aux + 1 guideline) carries the pointer, dataset + mirror. Consistent with the shared-reference convention.

Decision

APPROVED WITH COMMENTS. Both prior Majors are genuinely resolved (not merely acknowledged): the merge-commit sites and the context-map guideline now resolve override-first, dataset + mirror; the conformance scanner was broadened to catch the root-cause blind spot. All five ACs are met. The four remaining items are Minor doc/test-precision nits — none re-open the doc/behavior mismatch this story exists to close. Human merge gate decides whether to fold the Minors into this PR or defer.

[rucka]

Remediation — round 2 (commit 922fba3)

Per finding (all Minor):

• [Minor] Wrong "Used by" attribution for context-map / subdomain-context templates (templates.mdx:72-73) → verified in code: /record-decision creates context-map.md inline (Step 4, "offer to create a minimal context-map.md") and never reads the template; map-contexts/map-subdomains link only subdomain-/bounded-context-template, not these two. Re-attributed both rows to the context-map-maintenance guideline (its sole consumer) and dropped the false "record-decision (DDR flow)" claim. — apps/website/content/docs/customization/templates.mdx
• [Minor] Footer template links missing the resolution pointer (context-map-maintenance.md:69-70) → added the same override-first pointer the use-sites (~23, ~47) carry, to both "Related Documents" links. Intra-file consistency now zero-gap. — .pair/knowledge/guidelines/architecture/design-patterns/context-map-maintenance.md (+ dataset mirror; root regenerated via pair update)
• [Minor] Hardcoded "16 skills" can go stale (no gate) (skill-conventions/README.md:11) → replaced the fixed count with the non-numeric "all template-linking skills". — .pair/knowledge/skill-conventions/README.md (+ dataset mirror)
• [Minor] Conformance scan blind to the guidelines corpus (template-resolution.test.ts) → extended the pointer invariant to the guidelines tree (dataset source + generated root) via the generic .md walker. New describe block: (a) no template-linking guideline may drop the pointer, (b) a guard asserting context-map-maintenance.md is actually reached by the scan — so a regression that removes the Major Collaborative Knowledge Base #2 pointer now fails the test. +4 tests (368 total; template-resolution.test.ts now 19). Verified only context-map-maintenance.md matches the collaboration/templates/<name>-template.md link form in the corpus → no false positives. — packages/knowledge-hub/src/conformance/template-resolution.test.ts

Not changed (escalated): none.

Mirror: dataset↔root regenerated with pair update --offline (not hand-edited); byte-consistent.

Quality gates: PASS — knowledge-hub vitest 368/368 (incl. template-resolution 19); check:links (dataset + root) clean; markdownlint clean; ts:check (turbo, 8 pkgs) clean; skills:conformance 39/39; docs:staleness in sync.

→ Re-review requested.

[rucka]

Code Review — Independent Re-Review

PR: #367 · Story: #314 (wire template-override resolution into skills) · Reviewer: Independent reviewer (blind) · Date: 2026-07-23 · Type: Feature + bundled orchestration chore

Overall Assessment

• Approved — Ready to merge

All four Minor findings from the prior review are genuinely resolved (not merely acknowledged), every AC of #314 is met by the diff, the bundled orchestration change is logically coherent and tested, and there is no regression. Full suite: 368/368 passing (after building @pair/content-ops), check:links green on dataset + root, tsc clean, dataset↔root byte-consistent.

Block A — Prior Minors verified resolved

1. templates.mdx attribution (RESOLVED). context-map-template.md and subdomain-context-template.md are now attributed to the context-map maintenance guideline, not to record-decision/DDR or map-* directly. Verified in code: record-decision/SKILL.md writes context-map.md inline (creates a minimal file, Step 4) and never reads the template; the only files referencing these two templates are context-map-maintenance.md and the templates README.md. The mdx link /docs/reference/guidelines-catalog resolves to a real route.
2. context-map-maintenance.md footer (RESOLVED). Both Related-Documents links carry the resolve override-first — [template resolution] pointer, consistent with the two in-body use-sites (§1 Challenge the Glossary, §What moves).
3. skill-conventions/README.md count (RESOLVED). Hardcoded "16 skills" replaced with the non-numeric "all template-linking skills".
4. Conformance test blind-spot (RESOLVED — closes prior Major Collaborative Knowledge Base #2). template-resolution.test.ts now scans the guidelines corpus (dataset + generated root) via the new generic collectSkillMarkdownFiles walker, with an explicit guard asserting context-map-maintenance.md is actually reached by the scan. Dropping the pointer from that guideline now fails the test. 19 tests, all green.

Block B — Bundled orchestration change (implement-batch)

Logic is coherent and preserves all return contracts:

• First review (round===0) posts the report to the PR; re-reviews return findings only and are instructed not to post — and stay blind to the working log (.pair/working/reviews/<id>.md), receiving prior findings via prompt. Independence preserved.
• Fix rounds append to the working log instead of per-round PR comments.
• Convergence (actionable.length===0 with remediated) posts ONE synthesized remediation comment then deletes the log; a clean first review (remediated===false) posts no synthesis — first-review comment stands alone.
• Escalation flushes the log to the PR (kept, not deleted).
• Return statuses ready-for-merge / escalate / failed-fix unchanged.

Tests cover the five required behaviors (first-review-posts / re-review-silent / fix-logs / synthesis / clean-first-review-no-synthesis); 12/12 workflow tests pass.

Findings

All findings are Minor and scoped to Block B (the bundled orchestration chore, .claude/workflows/implement-batch.js). None block merge.

Minor 1 — Escalation-flush path is untested

Location: .claude/workflows/implement-batch.js:295-299, 322-326
Description: The if (remediated) flush-to-PR branch on both escalation exits (MAX_FIX_ROUNDS / needsHumanDecision) has no dedicated test. The five convergence-path behaviors are covered, but a regression that broke the escalation flush (e.g. deleting the log instead of keeping it, or not posting) would pass CI silently.
Recommendation: Add a test asserting that on escalation a flush: agent runs and the log is NOT deleted.

Minor 2 — Design-disagreement escalation doesn't pass open findings to the flush agent

Location: .claude/workflows/implement-batch.js:322-326
Description: The MAX_FIX_ROUNDS escalation passes ${JSON.stringify(actionable)} to the flush agent, but the fix.needsHumanDecision escalation relies solely on the working log. If the disagreeing fix agent set needsHumanDecision without writing the round to the log, the flush comment could be thin. The human still has the first-review comment, so impact is limited to comment richness.
Recommendation: Pass prevFindings (the open decision) to the disagreement-flush prompt for symmetry.

Minor 3 — Orphaned working log on failed-fix

Location: .claude/workflows/implement-batch.js:317
Description: If a fix agent returns null (!fix → failed-fix) after a prior round already created .pair/working/reviews/<id>.md, the log is neither flushed nor deleted — it lingers as an orphaned working file.
Recommendation: On failed-fix, either flush the log to the PR or note it in the return payload so the orchestrator can clean up.

By-design / non-actionable observations (transparency)

• templates.mdx lists 19 templates, not the 17 named in the story/brief. The KB ships 19 *-template.md files today (verified via ls); AC5's intent is "list all shipped templates" and "17" was the stale count at refinement (2026-07-13). Listing 19 is the correct, current fulfilment of AC5, not a defect. Disposition: By design — matches the authoritative on-disk template count.
• 16 skills wired, not 15. /pair-capability-publish-pr (merged post-refinement, [#256] feat: implement composes checkpoint + publish-pr via handoff-only subagent #359) is a genuine template consumer (pr-/commit-/branch-/checkpoint-templates) and is where the AC2 pr-template resolution actually happens; excluding it would fail AC2 in spirit. The conformance invariant is count-agnostic (>= 15). Disposition: By design — count baseline shifted after refinement.

AC verification (all met)

• AC1 idempotent no-override → KB default: instruction + test. ✓
• AC2 override wins, process-composed (pr-template via /implement→/publish-pr) + capability-composed (adr-template via /record-decision): both carry the pointer, both tested. ✓
• AC3 single shared instruction, file-existence check, no ## Templates parsing: template-resolution.md + pointer at all 16 use-sites. ✓
• AC4 ## Templates = audit trail not trigger: instruction + mdx Step 3 + test. ✓
• AC5 mdx lists all shipped templates, accurate mechanism, branch-template orphan resolved (genuinely consumed by /publish-pr for branch-format parsing — verified). ✓

Verification evidence

• node --test implement-batch.test.mjs → 12/12 pass
• vitest run (knowledge-hub) → 368/368 pass (5 initial file-load failures were an unbuilt @pair/content-ops in the fresh worktree, unrelated to the diff; green after pnpm --filter @pair/content-ops build)
• check:links → all links valid (dataset + root)
• tsc --noEmit → clean
• dataset↔root: template-resolution.md and context-map-maintenance.md byte-identical; skill pointer lines identical dataset vs .claude mirror; git status clean

Independent blind re-review — did not read .pair/working/.