Skip to content

tech-debt: extend mirror-equality guard to per-skill SKILL.md pairs (all 36 skills) #352

Description

@rucka

Story Statement

As a Pair maintainer/reviewer
I want an automated mirror-equality guard asserting that every root .claude/skills/<prefixed>/SKILL.md equals the real pair update copy-pipeline transform of its dataset source (packages/knowledge-hub/dataset/.skills/<cat>/<name>/SKILL.md)
So that per-skill SKILL.md drift between the canonical dataset and the installed root mirror is caught by CI instead of by hand (as was done for PR #350), closing a pre-existing structural gap in the mirror-equality guard.

Where: packages/knowledge-hub test suite (runs in CI + local pnpm --filter @pair/knowledge-hub test).

Epic Context

Parent: KB / mirror integrity (Integration & Process Standardization bounded context)
Status: Refined
Priority: P2 (tech-debt, R7.2 — never blocks a PR)
Type: tech-debt

Classification

risk:yellow · cost:green — supporting-subdomain CI test-guard addition; isolated, test-only, reuses already-exported @pair/content-ops transforms; floored at yellow by KB-default service criticality (quality-model §3.1) (refinement).

Matrix — per dimension
Dimension Tier Source Note
Service/domain criticality yellow KB default (no Criticality Table) Medium default — file present, table absent (quality-model §3.1)
Change/diff risk green story scope isolated whole-file mirror-equality test reusing the real copy pipeline + existing helpers; no production behavior change
Business impact yellow subdomain class Integration & Process Standardization = Supporting; CI/test-integrity tooling around the core, not core logic
Security relevance green path heuristic test-only guard; no authn/authz/secrets/PII
Coupling balance green supporting subdomain, no new integration reuses the existing knowledge-hub→content-ops dependency within the same bounded context; introduces no new cross-context coupling

Tier = max(assessed) = risk:yellow (Service criticality / Business impact). Cost = highest detected signal = green (no cost signals). Model: KB defaults + Tag Projection (Active: risk). Confidence: high.

Acceptance Criteria

Functional Requirements (Given-When-Then)

  1. Given the dataset .skills tree and the root .claude/skills tree are in sync,
    When the guard runs,
    Then for every skill directory present in the dataset it passes, asserting root/<prefixed>/SKILL.md is byte-for-byte equal to the real copy-pipeline transform of dataset/.skills/<cat>/<name>/SKILL.md.

  2. Given a root SKILL.md whose name: frontmatter no longer matches its prefixed directory (drift),
    When the guard runs,
    Then it FAILS, naming the offending skill and showing the expected vs actual content.

  3. Given a root SKILL.md whose relative links were not depth-adjusted (../../ not bumped to ../../../) or whose /command skill-reference tokens drifted,
    When the guard runs,
    Then it FAILS for that skill (the transform composition covers link-depth + skill-reference rewrites, not only skill-name/link-path helpers).

  4. Given a dataset skill whose root mirror file is missing,
    When the guard runs,
    Then it FAILS loudly (no silent skip).

  5. Given a new skill is later added to the dataset,
    When the guard runs,
    Then it is covered automatically with no test edit (iteration is data-driven over dataset skill dirs — never a hardcoded skill count).

Business Rules

  • Directional (dataset → root), not bijective: the guard iterates dataset skill dirs and ignores root-only skills that have no dataset source (e.g. agent-browser). Root == realTransform(dataset) is asserted only for dataset-sourced skills.
  • Faithful transform, no parallel re-implementation: reproduce the exact pair update copy pipeline (copyDirectoryWithTransforms / content-ops copy transforms + collectSkillDirs), not a hand-rolled regex — a bug in the real pipeline must fail this guard, not be masked.
  • Consolidated, cross-cutting invariant → one file (analogous to assess-output-only.test.ts): a single it.each over all dataset skills, NOT one test file per skill (per the "conformance test per file not per story" ADL exception for cross-cutting invariants).
  • Gate/tooling logic lives in a tested module (per the "gate & tooling code in tested modules" ADL): any new orchestration helper is white-box-tested; the test drives the real production transform. No spawnSync of a script.
  • R7.2: tech-debt — the guard is additive and must not block any current PR beyond enforcing the invariant going forward.

Edge Cases and Error Handling

  • Root-only non-pair skill (agent-browser): ignored — not treated as drift.
  • Missing root mirror for a dataset skill: hard failure with the skill path.
  • next skill (bare top-level dir, no category): included; maps to pair-next.
  • Frontmatter name: sync: the leaf-name → prefixed-dir rename must be reflected (syncFrontmatter), and any mismatch caught.
  • Failure message ergonomics: on failure, output must identify the specific skill and give the regenerate hint (pair update), mirroring the existing skills-guide whole-file test.

Definition of Done (right-sized for a test-only tech-debt guard)

  • All acceptance criteria implemented and verified
  • Consolidated it.each guard added under packages/knowledge-hub/src/tools/, covering every dataset skill (data-driven count)
  • Guard reuses the real @pair/content-ops copy pipeline (no parallel transform re-implementation)
  • pnpm --filter @pair/knowledge-hub test green; guard demonstrably fails on an injected drift (frontmatter, link-depth, /command, missing mirror) then passes when reconciled
  • Lint/type-check pass
  • Changes confined to packages/knowledge-hub (test + optional co-located helper) — no changes to content-ops, pair-cli, templates, other skills, or .claude/skills
  • Code reviewed and merged

Story Sizing and Sprint Readiness

Final Story Points: S (2)
Confidence: Medium
Sizing Justification: Isolated, test-only. All transforms already exported by content-ops; reuses collectSkillDirs + buildDatasetSkillNameMap. Main effort is choosing the faithful transform-composition strategy and writing the drift-injection verification. Fits comfortably in one sprint.

Dependencies and Coordination

  • Prerequisite: none — @pair/content-ops already exports every needed function (copyDirectoryWithTransforms, InMemoryFileSystemService, transformPath, syncFrontmatter, rewriteLinksAfterTransform, rewriteSkillReferences, buildSkillNameMap, buildSkillLinkPathMap).
  • Related: extends the existing skills-guide mirror-equality guard (skills-guide-mirror.test.ts) to a new artifact set (per-skill SKILL.md).
  • No external/infra dependencies.

Technical Analysis

Implementation Approach

Preferred strategy — reuse the real pipeline (no re-implementation): seed an InMemoryFileSystemService from the dataset .skills tree, run the real copyDirectoryWithTransforms (flatten + prefix: 'pair', the same options pair update uses) into an in-memory destination, then assert each produced <prefixed>/SKILL.md equals the on-disk root .claude/skills/<prefixed>/SKILL.md byte-for-byte. This composes the exact copy transform — dir-rename (transformPath), frontmatter name: sync (syncFrontmatter), relative-link-depth rewrite (rewriteLinksAfterTransform, the ../../../../../ bump), and /command rewrite (rewriteSkillReferencesInFiles) — with zero parallel logic, so a real pipeline bug fails the guard.

Why the skills-guide helpers alone are insufficient (the reason this is its own story): applyKnownMirrorTransforms only chains rewriteSkillReferences + rewriteSkillLinkPaths, which assumes equal directory depth (true for skills-guide.md). Per-skill SKILL.md files change depth on copy (dataset/.skills/<cat>/<name>/.claude/skills/<prefixed>/), also triggering the link-depth rewrite and frontmatter sync — so the guard must compose the full copy transform.

Alternative (fallback): compose the pure transforms directly in a co-located helper. Rejected as primary because rewriteLinksAfterTransform is file-service-based (not a pure string fn), making the real-pipeline reuse both simpler and more faithful.

Key components: packages/knowledge-hub/src/tools/ (new consolidated guard + optional helper), reusing collectSkillDirs / buildDatasetSkillNameMap from skills-guide-mirror.ts and the @pair/content-ops copy pipeline.

Design: not required.

Technical Risks and Mitigation

Risk Impact Probability Mitigation
Re-implementing the transform diverges from real pipeline Medium Medium Drive the real copyDirectoryWithTransforms via in-memory FS; assert against real production functions only
Hardcoding skill count (stale — was "36", now 39) Low Medium Iterate over collectSkillDirs(dataset/.skills); never assert a fixed count
Root-only skills (e.g. agent-browser) misread as drift Medium High Directional dataset→root iteration; ignore root entries with no dataset source

Task Breakdown

  • T1 — Choose + prototype the faithful transform strategy: seed InMemoryFileSystemService from dataset/.skills, run real copyDirectoryWithTransforms({ flatten: true, prefix: 'pair' }), capture produced SKILL.md per prefixed dir. (AC1, Business Rule "faithful transform")
  • T2 — Add consolidated it.each guard under packages/knowledge-hub/src/tools/ iterating dataset skill dirs (via collectSkillDirs), asserting each root SKILL.md == transform output byte-for-byte; directional, ignoring root-only skills. (AC1, AC5, Business Rules)
  • T3 — Missing-mirror + name/link-depth//command drift handling: fail loudly with skill path + regenerate hint. (AC2, AC3, AC4, Edge Cases)
  • T4 — Verify the guard: inject each drift class locally (frontmatter name, link-depth, /command, deleted mirror) → guard FAILS; reconcile → guard PASSES. (DoD)
  • T5 — Confirm scope containment: git diff --name-only touches only packages/knowledge-hub/; run pnpm --filter @pair/knowledge-hub test + lint/type-check green. (DoD)

Refined via: manual how-to flow (08-how-to-refine-a-user-story) — full alignment context supplied by the delegating session; interactive grill phase not applicable in delegated refinement.
Refinement Date: 2026-07-24

Metadata

Metadata

Assignees

No one assigned

    Labels

    risk:yellowClassification: medium risk tiertech-debtTracked technical debt (living backlog, R7.2 — never blocks a PR)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions