Skip to content

tech-debt: extend mirror-equality guard to all root skill artifacts (sub-docs + references) #384

Description

@rucka

Story Statement

As a maintainer of the Pair knowledge base
I want the mirror-equality guard to assert every artifact a skill directory contributes through the pair update transform, not only its SKILL.md
So that a hand-edit to a root sub-doc is caught in CI instead of drifting silently until someone regenerates and loses it

Where: packages/knowledge-hub/src/tools/skill-md-mirror.ts + its colocated test — the guard #352 delivered, extended.

Epic Context

Parent Epic: nessuna — tech-debt trasversale sull'integrità del mirror, come #352 (anch'essa senza epic). Candidata a un'epic "integrità KB" se ne verrà aperta una.
Status: Refined
Priority: P2 (Could-Have)

Status Workflow

  • Refined: Story is detailed, estimated, and ready for development
  • In Progress: Story is actively being developed
  • Done: Story delivered and accepted

Classification

risk:yellow · cost:green · coupling: not assessed — no domain artifacts

Matrix — per dimension
Dimension Tier Source Note
Service/domain criticality yellow KB default tech/risk-matrix.md declares no Criticality Table ⇒ Medium default (D21)
Change/diff risk green story scope test-only, one module + its test; no runtime path
Business impact yellow subdomain class Integration & Process Standardization (Supporting) — owns the mirror transform
Security relevance green path heuristic no authorization surface, no input handling, no secrets
Coupling balance not assessed absent no new integration; tightens an existing dataset↔root assertion

Acceptance Criteria

Functional Requirements

Given-When-Then Format:

  1. Given a skill directory in the dataset that contributes a non-SKILL.md artifact (today: process/bootstrap/assess-orchestration.md, process/review/merge-and-cascade.md, process/review/degradation-levels.md, process/implement/post-review-merge.md)
    When the guard runs
    Then each such artifact's root copy is asserted byte-for-byte against the real copy-pipeline transform of its dataset source, exactly as SKILL.md already is

  2. Given a root sub-doc hand-edited so it no longer matches its dataset source
    When the guard runs
    Then it FAILS, naming the offending artifact by its dataset-relative path and showing the compact line-level diff (diffSkillMd), not a full two-file dump

  3. Given a new artifact added to a dataset skill directory
    When the guard runs
    Then it is covered with no test edit — the case list is derived from the dataset at collection time, never hardcoded, and never a count

  4. Given the dataset artifact exists but its root copy is missing
    When the guard runs
    Then it FAILS naming the missing path and pointing at pair update, matching SKILL.md's AC4 behaviour

  5. Given a root-only file inside a skill directory with no dataset source
    When the guard runs
    Then it is NOT asserted and NOT treated as drift — the guard stays directional (dataset → root), as agent-browser already proves for SKILL.md

Business Rules

  • Test-only, R7.2: the guard never blocks a PR by itself; it fails the test suite, which the pre-merge gate reads.
  • No re-implementation of the transform: the expected content comes from composing the real pair update copy pipeline (buildInstalledSkillMd's sibling for non-SKILL.md artifacts), never a second copy of the flatten/prefix/link-rewrite rules — the finding tech-debt: extend mirror-equality guard to per-skill SKILL.md pairs (all 36 skills) #352 already fixed for SKILL.md.
  • SKILL_COPY_OPTS stays pinned to apps/pair-cli/config.json's skills registry, so a registry change fails with correct attribution instead of silently computing the wrong root path.
  • Data-driven, never counted: no assertion may encode how many artifacts exist. Today there are 4; PR [US-230] feat: pair-process-brainstorm — 3 phases, parametrized ($root, orientation) #387 adds 3 more (process/brainstorm/{parametrization,resume,degradation}.md) and must require no change here.

Edge Cases and Error Handling

  • Invalid Input: a dataset skill dir containing a non-markdown file (e.g. a stray .DS_Store) — the enumeration must not assert it as a transformed artifact.
  • System Errors: an unreadable root file is reported as missing/unreadable with its path, never swallowed into a pass.
  • Boundary Conditions: a skill dir with SKILL.md only (the majority) contributes no extra cases and must not produce empty/vacuous ones.
  • Exceptional Scenarios: nested subdirectories under a skill dir (references/* — none exist today) must be enumerated recursively, so the guard is ready for the first one instead of silently ignoring it.

Definition of Done Checklist

  • All acceptance criteria implemented and verified
  • Guard is data-driven from the dataset (no hardcoded artifact list, no count assertion)
  • Failure messages name the artifact and show the compact diff
  • Drift injection executed: each drift class (content, missing root copy, link-depth rewrite, skill-reference rewrite) is proven to turn the guard RED, then green when reconciled — a guard that cannot fail is worse than none
  • Unit tests colocated in skill-md-mirror.test.ts; full @pair/knowledge-hub suite green
  • pnpm quality-gate green (this file is inside the prettier/lint gate)
  • PR follows pr-template.md

Dependencies

Notes

Origin: round-3 escalation of PR #377 (#352). The guard deliberately scoped itself to SKILL.md; the residual gap was filed as four near-duplicate issues (#379, #380, #383, #384) and deduplicated on 2026-07-26 keeping this one, as the widest in scope.

Task Breakdown

  • T-1: Enumerate every transformed artifact per dataset skill dir (recursive, markdown-only)
  • T-2: Extend the expected-content builder to non-SKILL.md artifacts via the real pipeline
  • T-3: Data-driven assertions per artifact, with named failures + compact diff
  • T-4: Directionality + missing-root-copy cases
  • T-5: Drift-injection proofs for every drift class
  • T-6: Reconcile any real drift the new guard surfaces on main

Dependency Graph

T-1 ── T-2 ── T-3 ──┬── T-4
                    └── T-5 ── T-6

AC Coverage

AC Tasks
AC-1 (every artifact asserted) T-1, T-2, T-3
AC-2 (named failure + diff) T-3, T-5
AC-3 (data-driven, no count) T-1, T-3
AC-4 (missing root copy) T-4, T-5
AC-5 (directional, root-only ignored) T-4

T-1: Enumerate transformed artifacts per skill dir

Priority: P0 | Estimated Hours: 2h | Bounded Context: Integration & Process Standardization

Summary: Derive, from the dataset, the full list of artifacts each skill dir contributes — recursively, so a future references/ subdir is covered.

Type: Feature Implementation

Description: readSkillsDatasetFromDisk already walks the dataset; extend the derived view so each skill dir yields its non-SKILL.md markdown artifacts alongside it. Exclude non-markdown files. Return dataset-relative paths — they are the identity used in failure messages and as the test case name.

T-2: Expected content through the real copy pipeline

Priority: P0 | Estimated Hours: 3h | Bounded Context: Integration & Process Standardization

Summary: Compose the actual pair update transform for these artifacts instead of re-deriving flatten/prefix/link-rewrite rules.

Description: buildInstalledSkillMd returns only SKILL.md. Extend it (or add a sibling that shares the same pipeline invocation) so the temp-dir copy result is read back for every artifact. The link-depth rewrite and the /command/pair-* rewrite must come from the pipeline, not from a local reimplementation. Keep the existing 120s beforeAll timeout headroom — this runs real temp-dir I/O for the whole dataset.

T-3: Data-driven per-artifact assertions

Priority: P0 | Estimated Hours: 2h | Bounded Context: Integration & Process Standardization

Summary: One case per dataset artifact, generated from T-1's list; failures name the artifact and show diffSkillMd.

Description: Mirror the existing it.each(skillDirs) shape. No count assertions anywhere. Reuse assertRootSkillMdMatches (or generalise its name) so the production module — not the test — owns the assertion, keeping the drift-injection tests and the real guard on one code path.

T-4: Directionality and missing-copy cases

Priority: P1 | Estimated Hours: 1h | Bounded Context: Integration & Process Standardization

Summary: A root-only artifact is not drift; a missing root copy fails loudly pointing at pair update.

T-5: Drift-injection proofs

Priority: P0 | Estimated Hours: 2h | Bounded Context: Integration & Process Standardization

Summary: Prove the guard fails on each drift class and passes when reconciled.

Description: Synthetic mini-dataset through the same real pipeline, then corrupt the transformed output per class: content edit, link-depth regression, skill-reference regression, missing file. Each must throw; reconciled must pass. This is the DoD item that separates a guard from a decoration.

T-6: Reconcile drift surfaced on main

Priority: P0 | Estimated Hours: 1h | Bounded Context: Integration & Process Standardization

Summary: The new guard may find real pre-existing drift in the 4 current sub-docs. Resolve each per case — decide which side is right — never by blind regeneration.

Description: #352 hit exactly this: three drifted SKILL.md mirrors, and in one case the ROOT held the correct content while the dataset was stale, so regenerating would have silently reverted a fix. Diff each of the 4 artifacts, decide direction with the git history, then regenerate.

Metadata

Metadata

Assignees

Labels

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

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions