[#224] refactor: assess-* output-only; record-decision sole adoption writer

[rucka]

Refactoring: assess-* skills → output-only; record-decision → sole adoption writer

Story: Closes #224 (Epic #209 — Code Quality & Testing foundations)
Type: Refactor · Priority: P1 · Risk: Low (no product code; skill/docs semantics only)

Motivation

The execution-layer dogfood surfaced an ambiguity: each assess-* skill both assessed a domain and persisted the outcome (wrote its adoption file + composed /record-decision). Assessing had side effects, there were effectively two adoption writers, and assess-debt risked polluting the backlog via scan-mode/auto-conversion. This PR cleanly separates assessing (read + recommend, zero side effects) from deciding/persisting (write), with a single writer.

The contract (per the issue's "Contract: assess output → record-decision input")

• assess-* output = a proposal carrying the rendered adoption content + target (ready-to-write section/file body + which adoption file/section) plus a human-facing report. It performs no write.
• record-decision input = { content, target, decision-metadata }. Generic persister: writes content to target + records ADR/ADL. No per-domain rendering — that lives in each assess-* output.
• Orchestration = the caller wires the two (assess-* → [accept] → record-decision). In bootstrap/review the caller is the process skill; standalone it's the human/agent.

Invariant: only record-decision touches adoption files; only assess-* renders its own adoption content; the caller only orchestrates.

Changes Made

• T1 — assess-* output-only (all 10): debt, code-quality, stack, architecture, testing, ai, pm, infrastructure, observability, methodology. Removed every file/adoption write step and every compose record-decision step; kept the resolution cascade (Argument > Adoption > Assessment) and the report. Step 4 → "Render Adoption Proposal", Step 5 → "Emit Proposal" ({content, target, decision-metadata}). code-quality/debt were already read-only — annotated as output-only.
• T2 — record-decision sole writer + bootstrap rewire: record-decision documents itself as the sole adoption writer and gains a generic {content, target} persist path (Step 4 "Generic Persist"). pair-process-bootstrap Step 2.2 rewired to compose /record-decision(content, target) per accepted proposal.
• T3 — scan-mode removed: assess-debt has no $mode:scan (and never auto-creates items); explicit "no auto-creation / never blocks (R7.2)". (Remaining "Scan"/"full scan" wording refers to reading the codebase and the cascade path — not a mode.)
• T4 — review reports debt: pair-process-review Step 4.2 rewritten — new debt is surfaced in the review output, no auto-issue, PR never blocked on debt grounds. assess-stack Phase 3 rewired to persist via record-decision on approval.
• T5 — tech-debt label kept: write-issue gained a $labels arg + a note that promotion to the backlog is a deliberate, selective act (never a 100% auto-conversion).
• T6 — ADR: .pair/adoption/tech/adr/adr-009-assess-output-only.md.
• T7 — docs + conformance: skills-catalog.mdx assessment section updated to the output-only contract; conformance test added at packages/knowledge-hub/src/assess-output-only.test.ts.

Every touched skill was synced in dataset source (packages/knowledge-hub/dataset/.skills/...) and installed mirror (.claude/skills/...) in the same commit, byte-consistent under the mirror transform (name-prefix + strip trailing slash on markdown dir-links).

Files Changed (31)

• Skills (dataset + mirror): all 10 assess-*, record-decision, write-issue, process/bootstrap, process/review.
• Added: .pair/adoption/tech/adr/adr-009-assess-output-only.md, packages/knowledge-hub/src/assess-output-only.test.ts.
• Docs: apps/website/content/docs/reference/skills-catalog.mdx.

AC coverage

• AC1 every assess-* produces report/proposal, writes nothing → T1 (all 10).
• AC2 proposal carries rendered content + target; caller persists via record-decision (generic, no per-domain rendering) → T1 + T2.
• AC3 assess-debt has no $mode:scan, no auto-creation → T3.
• AC4 review reports new debt, no auto-issue, never blocks (R7.2) → T4.
• AC5 deliberate tech-debt promotion via write-issue → T5.
• AC6 ADR recorded → T6 (ADR-009).

Reviewer notes (disclosures — you cannot see the checkpoint)

• record-decision mirror DDR forward-port. The installed record-decision mirror was already stale before this PR — it lacked the DDR/domain content merged in DDR type in pair-capability-record-decision + ddr-template #245 (the mirror was never regenerated then). Because assess-* skills are output-only; record-decision is the sole adoption writer (no scan-mode) #224 edits record-decision, its mirror was regenerated from the dataset source of truth, which also forward-ports that DDR type in pair-capability-record-decision + ddr-template #245 DDR content. That's why the record-decision mirror diff (~115 lines) is larger than the dataset diff (~25 lines). This is a legitimate mirror-sync, not a assess-* skills are output-only; record-decision is the sole adoption writer (no scan-mode) #224 behavior change.
• /implement intentionally untouched (scope). The directive scoped the rewire to bootstrap + review. /implement still composes /assess-stack for new deps; persistence flows through its existing /record-decision step, and assess-stack's Composition Interface documents the output-only contract for that caller. A minor wording tidy in implement Step 2.4 is pre-existing and functionally covered — noted as a possible tiny follow-up, out of assess-* skills are output-only; record-decision is the sole adoption writer (no scan-mode) #224 scope.

Design decisions

• ADR-009 (.pair/adoption/tech/adr/adr-009-assess-output-only.md): the full record — options considered, the contract, and consequences.

Testing / Quality Gates

knowledge-hub vitest:        80/80 pass (incl. new assess-output-only.test.ts 24/24)
docs-staleness-check.js:     PASS — 35 skills, 8 commands in sync
KB broken-links:             All markdown links valid
markdownlint (touched):      PASS
turbo ts:check:              PASS (7/7)
pre-push hooks:              hygiene + docs-staleness PASS
Conformance grep:            no assess-* writes / composes record-decision; no scan-mode

Note for local test runs: @pair/content-ops must be built first (turbo run build --filter=@pair/content-ops) or 2 knowledge-hub suites fail to resolve the package entry.

Risk Assessment

Low — no product/runtime code changed. Surface is skill markdown + docs + one ADR + one conformance test. Behavior change is limited to skill orchestration semantics (persistence moves from assess-* to the caller via record-decision).

🤖 Generated with Claude Code

[rucka]

Code Review — PR #310

Review Information

PR Number: #310
Author: (via Pair implementer)
Reviewer: Independent reviewer (blind — checkpoint not consulted)
Review Date: 2026-07-12
Story/Epic: #224 (Epic #209 — Code Quality & Testing foundations)
Review Type: Refactor (skills/docs semantics; no product/runtime code)

Review Summary

Overall Assessment

• Approved
• Approved with Comments — all 6 ACs met; 1 Major doc-reconciliation + several Minors noted. The Major is a documentation-precision issue (not a functional blocker); recommend addressing but it need not gate merge. Deferred to the human merge gate.
• Request Changes
• Comment Only

Key Changes Summary

Makes all 10 assess-* skills output-only (render a {content, target, decision-metadata} proposal + report, write nothing), promotes record-decision to the sole generic adoption persister ($content/$target path), removes assess-debt scan-mode/auto-creation, rewires bootstrap + review to persist via record-decision, keeps a tech-debt $labels promotion path in write-issue, adds ADR-009 + a 24-case conformance test. Dataset + .claude/ mirror both updated.

Business Value Validation

Delivers the story thesis: assessing becomes side-effect-free, persistence explicit/gated, debt surfaced without backlog pollution (R7.2). Verified independently against AC + code, not just the PR claims.

AC Verification

AC	Verdict	Evidence
AC1 — all 10 assess-* output-only, write nothing	Met	Residual-write grep clean (no Write Adoption File step, no compose record-decision) in dataset + mirror; conformance test enforces output-only in all 10. Caveat: assess-pm indirect write via setup-pm (see M1).
AC2 — proposal carries content+target; record-decision generic persister	Met	assess-* Step 5 "Emit Proposal"; record-decision Step 4 "Generic Persist" ($content/$target, no per-domain rendering). Caveat: wording ambiguity (m2).
AC3 — assess-debt no $mode:scan, no auto-creation	Met	No $mode arg row; explicit "no $mode:scan", "no auto-creation"; test enforces.
AC4 — review reports debt, no auto-issue, never blocks	Met	review Step 4.2 rewritten: surfaced not blocked, no auto-issue, R7.2.
AC5 — deliberate tech-debt promotion via write-issue	Met	$labels arg + deliberate-promotion note.
AC6 — ADR recorded	Met	ADR-009 present, well-structured (options/decision/consequences). Caveat: naming (m4).

Quality Gates (re-run independently in worktree)

knowledge-hub vitest:   80/80 pass (incl. assess-output-only.test.ts 24/24)
docs-staleness-check:   PASS — 35 skills, 8 commands in sync
mirror byte-consistency: verified — normalized diff (name-prefix + trailing-slash) = 0 for bootstrap, record-decision, assess-debt, assess-stack, review

Issues to Address

Major

• M1 — .claude/skills/pair-capability-record-decision/SKILL.md (~"single writer of adoption files in the system") + .pair/adoption/tech/adr/adr-009-assess-output-only.md ("single adoption writer"). The PR establishes an absolute invariant — "only record-decision touches adoption files." But /pair-capability-setup-pm Step 4 ("Update Way-of-Working") writes the PM section of way-of-working.md directly (record-decision is only composed afterward in Step 5), and the new assess-pm flow routes persistence through setup-pm (not record-decision) when it is installed. Concrete case: bootstrap with setup-pm installed → PM section is written by setup-pm, contradicting ADR-009. Impact: the deliverable's headline invariant is factually false for the PM domain; a maintainer relying on "only record-decision writes adoption" is surprised by setup-pm. Root cause (setup-pm) is pre-existing and out of assess-* skills are output-only; record-decision is the sole adoption writer (no scan-mode) #224's declared scope, so this is a documentation-precision issue, not a functional defect. Recommendation: in record-decision and ADR-009 (both in scope), either soften to "sole generic adoption writer" and explicitly carve setup-pm as the one delegated exception, or route setup-pm's way-of-working write through record-decision. Cheap (one sentence); recommend pre-merge, not a hard gate.

Minor

• m2 — record-decision SKILL.md, Step 4 "Generic Persist". "Write $content verbatim to $target" conflicts with the same step's "preserving any sections of the target the content does not own (section-scoped update)". A generic persister with "no per-domain rendering" is nonetheless required to identify section boundaries to avoid clobbering siblings (bootstrap writes core/testing/AI sections of tech-stack.md via successive record-decision calls). Recommendation: clarify that $target designates the owned section and persistence is a heading-scoped merge, not a whole-file overwrite.
• m3 — .pair/knowledge/guidelines/collaboration/templates/ddr-template.md (missing). The forward-ported record-decision mirror links [DDR template](../../../.pair/.../ddr-template.md), but that file exists only in the dataset (packages/knowledge-hub/dataset/.pair/...), not in the repo-root .pair/ (which does contain adr/adl/context-map/subdomain-context templates). Dangling reference in the shipped .claude/ mirror; mitigated by record-decision's graceful degradation. Root cause is DDR type in pair-capability-record-decision + ddr-template #245's incomplete .pair/ sync of the template, surfaced by this PR's forward-port. Recommendation: sync ddr-template.md into .pair/ (follow-up; arguably outside assess-* skills are output-only; record-decision is the sole adoption writer (no scan-mode) #224 core scope).
• m4 — record-decision SKILL.md, "File naming" (ADR). Documents ADR filenames as YYYY-MM-DD-<topic>.md, but the new record and 8 of 11 existing ADRs use sequential adr-NNN-*. The doc diverges from actual repo practice. The ADR-009 filename itself correctly matches its siblings (do not rename it). Recommendation: align record-decision's documented ADR naming to the adr-NNN- convention in use.

Non-actionable (reported for transparency)

• n5 — mirror forward-port disclosure gap (bootstrap). The PR discloses the record-decision/DDR type in pair-capability-record-decision + ddr-template #245 DDR mirror forward-port but not the bootstrap mirror forward-port of map-subdomains/map-contexts reclassified process→capability (scoped, callers matrix) #246 domain-modeling content (Phase 3.5, map-subdomains/map-contexts rows, Callers Matrix). Verified this is a legitimate mirror sync: dataset bootstrap already carried Phase 3.5 (from map-subdomains/map-contexts reclassified process→capability (scoped, callers matrix) #246 cd5a5e8); the mirror was stale (last touched 410c631) and regeneration forward-ports it. Dataset == mirror confirmed. Non-actionable — the mirror must match the dataset source of truth; fixing would be wrong. Disclosure-only gap.
• n6 — DoD "relevant guideline pages" not updated. Only skills-catalog.mdx changed. Verified no guideline page contained stale scan-mode/auto-create content requiring update. Non-actionable — no guideline change was required; DoD satisfied.

Positive Feedback

• Clean, uniform application of the output-only contract across all 10 skills; dataset + mirror kept byte-consistent (verified).
• Conformance test (assess-output-only.test.ts) is a good guard: enforces output-only wording in dataset + mirror, absence of $mode, and the sole-writer/generic-persist contract.
• ADR-009 is thorough (options, contract, consequences incl. the /implement and record-decision-absent trade-offs).
• Reviewer-notes disclosures in the PR body (record-decision forward-port, /implement scope) are helpful and honest.

Risk Assessment

Low — no product/runtime code. Surface is skill markdown + docs + one ADR + one test. Behavior change limited to skill orchestration semantics.

---

Independent review per /pair-process-review. Verdict deferred to the human merge gate on M1.

[rucka]

Remediation — round 1 (commit 3358fc2)

All edits applied to BOTH the dataset source (packages/knowledge-hub/dataset/.skills/capability/record-decision/SKILL.md) and the installed mirror (.claude/skills/pair-capability-record-decision/SKILL.md); the two stay byte-consistent under the mirror transform (verified: the only diffs are skill-name prefixing and directory-link trailing slashes — no body drift).

Per finding:

• [Major] "only record-decision touches adoption files" invariant is false — /setup-pm writes the PM section of way-of-working.md directly → Softened the invariant to "sole GENERIC adoption writer" and carved /setup-pm as the one delegated exception in all four SKILL.md locations (overview, Step 4 intro, Composition output line, Composition invariant line). Also updated ADR-009 Decision, the "Sole generic adoption writer" consequence, the invariant sentence, and added a Trade-offs bullet documenting the /setup-pm exception. — record-decision/SKILL.md (dataset+mirror), .pair/adoption/tech/adr/adr-009-assess-output-only.md
• [Minor] Step 4 "Generic Persist" said write $content verbatim → reads as whole-file overwrite, clobbering sibling sections → Reworded to a heading-scoped merge into $target's owned section ("replace that section's body while preserving every sibling section the content does not own"). Applied to the overview generic-persister clause, the $content param row, the Step 4 Act line, and the Composition output line. — record-decision/SKILL.md (dataset+mirror)
• [Minor] Mirror links ddr-template.md under repo-root templates, but that file existed only in the dataset → Copied the dataset's ddr-template.md verbatim to .pair/knowledge/guidelines/collaboration/templates/ddr-template.md (byte-identical) so the mirror link resolves. — .pair/knowledge/guidelines/collaboration/templates/ddr-template.md (new)
• [Minor] Documented ADR filename YYYY-MM-DD-<topic>.md contradicts the repo's dominant sequential adr-NNN- convention → Split ADR from ADL in File naming (ADR sequential adr-NNN-, ADL date-based, DDR ddr-NNN-); added a next-sequence-number step + unbroken-sequence Verify to the "If architectural → ADR" flow; scoped the same-topic-same-date idempotency note to ADL only; fixed the naming-rationale note. adr-009 NOT renamed (already matches adr-NNN). — record-decision/SKILL.md (dataset+mirror)

Not changed (escalated): none.
Quality gates: PASS — knowledge-hub tests 80/80, check:links OK, markdownlint 0 errors, docs:staleness in sync, ts:check + hygiene PASS.
→ Re-review requested.

[rucka]

Code Review — PR #310 (Re-review, round 2)

Review Information

PR Number: #310
Author: (via Pair implementer)
Reviewer: Independent reviewer (blind — .pair/working/ not consulted)
Review Date: 2026-07-12
Story/Epic: #224 (Epic #209 — Code Quality & Testing foundations)
Review Type: Refactor (skills/docs semantics; no product/runtime code)
Head reviewed: 3358fc2 (remediation round 1)

Review Summary

Overall Assessment

• Approved — all 4 findings from round 1 verified genuinely resolved in code (not merely acknowledged); byte-consistency of record-decision dataset↔mirror holds under the mirror transform; no new issues introduced by the remediation. All 6 ACs remain met.
• Approved with Comments
• Request Changes
• Comment Only

Scope of this round

Remediation commit 3358fc2 touched exactly 4 files (verified via git show --stat): record-decision/SKILL.md (dataset + mirror), adr-009-assess-output-only.md, and the new repo-root ddr-template.md. Tight scope — no collateral to the 10 assess-* skills, bootstrap, review, write-issue, or the conformance test.

Verification of round-1 findings

#	Sev (round 1)	Claim	Verdict	Evidence
1	Major	Soften "only record-decision touches adoption" → "sole GENERIC writer" + carve /setup-pm exception (record-decision + ADR-009)	Resolved	record-decision: overview L12, Step 4 intro L105, Composition output L163, Composition invariant L164 all say "sole generic writer" + name /setup-pm as the one exception. ADR-009: Decision L34, Benefit L53, Trade-offs bullet L62 all document the /setup-pm carve-out. Softened invariant is now factually accurate (assess-* are output-only; bootstrap/review persist via record-decision; setup-pm is the only remaining direct adoption writer).
2	Minor	Replace "write $content verbatim" with heading-scoped merge preserving siblings	Resolved	Word "verbatim" removed everywhere. Heading-scoped-merge phrasing at overview L12, $content param row L21, Step 4 Act L111 ("replace that section's body while preserving every sibling section the content does not own"), Composition output L163. Consistent with the interactive path (L124 "only add/modify the affected section").
3	Minor	Copy ddr-template.md into repo-root templates so the mirror link resolves	Resolved	.pair/knowledge/guidelines/collaboration/templates/ddr-template.md present; diff vs dataset copy = 0 (byte-identical). Mirror link ../../../.pair/.../ddr-template.md from .claude/skills/pair-capability-record-decision/ now resolves.
4	Minor	Align documented ADR naming to sequential adr-NNN-; adr-009 not renamed	Resolved	File-naming split (L73-77): ADR adr-NNN- sequential, ADL YYYY-MM-DD-, DDR ddr-NNN-. Next-sequence step added to ADR flow (L83) + unbroken-sequence Verify (L85). Same-topic-same-date idempotency note scoped to ADL only (Edge Cases L173). Rationale note fixed (L193). adr-009-assess-output-only.md correctly left unrenamed (already matches adr-NNN).

Byte-consistency (dataset ↔ installed mirror, record-decision)

diff of dataset record-decision/SKILL.md vs mirror pair-capability-record-decision/SKILL.md yields only the mirror transform: (a) name: + /skill name-prefixing (/record-decision→/pair-capability-record-decision, /implement→/pair-process-implement, /review→/pair-process-review, /bootstrap→/pair-process-bootstrap), and (b) trailing-slash strip on markdown directory links (../adr/→../adr). Zero body drift — the remediated lines are identical between the two.

Conformance test integrity

Remediation did not touch assess-output-only.test.ts. Verified its record-decision assertions still hold against the reworded source: /sole writer|single writer/ matches (frontmatter description retains "the SOLE writer of adoption files"); "generic persist" present (Step 4 header). No remediation-induced test break.

Previously-accepted items (carried forward)

• n5 — bootstrap mirror forward-port disclosure gap: still non-actionable (mirror must match dataset source of truth). Unchanged.
• n6 — DoD "relevant guideline pages": still non-actionable (no guideline page required a change; only skills-catalog needed updating). Unchanged.

Risk Assessment

Low — no product/runtime code. Surface is skill markdown + one ADR + one template. Remediation is documentation-precision only.

Positive Feedback

• Every round-1 finding fixed at the point of truth (dataset) and mirrored, not merely acknowledged.
• ddr-template copied byte-identical rather than hand-retyped — no drift risk.
• ADR-naming fix is internally consistent (File naming + Step 3 flow + Edge Cases + rationale note all aligned), and correctly leaves the already-correct adr-009 filename alone.

---

Independent re-review per /pair-process-review. No remaining actionable findings — verdict Approved, carried to the human merge gate.