Skip to content

tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407

Description

@rucka

Story Statement

As a maintainer installing the Pair skills into a project
I want pair update to install a skill's nested references/ sub-directory inside that skill's installed directory, with working relative links
So that the first skill that uses progressive disclosure via references/* is installed usable instead of scattered across a sibling pseudo-skill dir with two broken links

Where: the skills asset-registry copy transform — transformPath/flatten in packages/content-ops (copyDirectoryWithTransforms, rewriteLinksAfterTransform), as used by apps/pair-cli update/install/package.

Epic Context

Parent Epic: none — cross-cutting install-pipeline defect, sibling of #352 / #384 (mirror integrity).
Status: Todo
Priority: P2 (Could-Have) — latent: no dataset skill ships a references/ sub-dir today; it fires on the first one.

Status Workflow

  • Todo: Story is created but not yet refined
  • Refined: Story is detailed, estimated, and ready for development
  • In Progress: Story is actively being developed
  • Done: Story delivered and accepted

User Value

User Benefit: a skill author can use the standard Agent-Skills references/ layout (the layout the third-party agent-browser skill already uses at .claude/skills/agent-browser/references/*.md) and have it install correctly.
Business Impact: unblocks progressive disclosure for the Pair skill corpus (#313's direction) without hand-repairing the install output.
Visible UI Value: n/a (CLI/install artifact) — demonstrable as the installed tree + working links after pair update.

Rough Sizing

Story Points: M(3)
Confidence: Medium
Reasoning: the flatten transform is shared by every asset registry, so the fix needs a scoped rule (flatten only the registry's top-level entries, preserve deeper segments) plus link-rewrite coverage and fixtures; blast radius is the reason it is not a one-liner.

Initial Scope

Likely In Scope

  • Flatten only the registry's top-level entry (process/reviewpair-process-review) and preserve deeper segments (process/review/references/deep.mdpair-process-review/references/deep.md).
  • Fix the link rewrite for both directions of a nested artifact (see evidence below).
  • Fixtures/tests for a nested references/ dataset skill through install / update / package.
  • Re-point the mirror guard's documented derivation (installedArtifactPath, packages/knowledge-hub/src/tools/skill-md-mirror.ts) at the corrected mapping once fixed — it currently encodes today's flatten output faithfully.

Likely Out of Scope

  • Adding references/ sub-docs to any Pair skill (separate authoring work).
  • Changing flatten semantics for non-skill registries beyond what the scoped rule implies.

Open Questions

Acceptance Criteria

Recorded post-hoc. This story was implemented straight from Draft: it never went through /pair-process-refine-story, so it carried no Given-When-Then criteria and no risk:* label while PR #411 was being reviewed. The criteria below are derived from the story statement, the reproduction block, and what PR #411 actually delivers, written down during review round 2 of #411 so an independent reviewer has criteria of record to verify against instead of reconstructing them. The skipped Draft→Ready transition is a fact of this story's history and should be noted in the merge decision, not backdated.

AC1 — a nested sub-directory installs INSIDE the skill
Given a dataset skill at <category>/<name>/ that ships references/deep.md
When the skills registry is copied (pair update / install / package)
Then the file installs at <target>/<prefix>-<category>-<name>/references/deep.md, and no sibling <prefix>-<category>-<name>-references/ directory is created.

AC2 — both link directions resolve in the installed tree
Given SKILL.md links ./references/deep.md and deep.md links ../SKILL.md
When the skill is installed
Then both hrefs still resolve after the copy, and neither points back into the dataset source layout.

AC3 — a sub-document is content, not an installed entry
Given two skills each owning a references/ sub-directory, and an unrelated skill whose body contains the token /references
When the skills registry is copied
Then no references skill name is registered: the unrelated body, AGENTS.md and .pair/knowledge are left alone, and the sub-document's frontmatter name: is not rewritten to a path.

AC4 — no other asset registry changes behaviour
Given a registry that does not declare the bounding option
When it is copied
Then every directory separator is still flattened exactly as before, pinned by a regression-witness test.

AC5 — an unusable configuration or layout fails loudly, before any file is copied
Given a bounding depth that is not a positive integer, or a preserved tail containing ./.., or a source entry shallower than the declared depth that owns a sub-directory (next/references)
When the config is validated or the copy starts
Then the run fails with an error naming the offending path, and no partial or mis-placed install is produced.

AC6 — a mirror registry stays idempotent for nested content
Given a mirror-behaviour registry with a bounded depth, from which a references/ sub-directory has been removed at the source
When it is copied again
Then the installed nested directory is removed, and the entry's own files survive.

AC7 — the open questions are resolved by a recorded decision
Given the two Open Questions above (registry-wide redefinition vs. an explicit knob; collision detection)
When the fix is implemented
Then the choice is recorded as an ADR with the options considered, and the mirror guard's documented derivation points at the corrected mapping.

Definition of Done Expectations

Standard DoD Requirements (never detailed during refinement — this story skipped Draft→Ready; the acceptance criteria above were derived post-hoc, see the note there):

Dependencies

Story Dependencies: #238 (closed — introduced flatten/prefix + reference rewriting), #384 / PR #406 (the guard that would otherwise cement today's output).
Epic Dependencies: none.

Notes

Found during the independent code review of PR #406 (story #384) — not a defect of that PR, which faithfully mirrors the pipeline's real output and documents it.

Reproduced against the real pipeline (in-memory clone, copyDirectoryWithTransforms with the skills registry options { flatten: true, prefix: 'pair' }), dataset:

  • process/review/SKILL.md containing [deep](references/deep.md)
  • process/review/references/deep.md containing [SKILL](../SKILL.md)

Produced:

pair-process-review/SKILL.md
pair-process-review/merge.md
pair-process-review-references/deep.md        <-- sibling pseudo-skill dir, outside the skill
  • SKILL.md link rewritten to ./references/deep.mdbroken (nothing at that path).
  • deep.md back-link rewritten to ../../../.skills/process/review/SKILL.mdbroken (points into the dataset source layout, not the install target).

Consequence today: the first references/ sub-dir added to a dataset skill installs broken, and #384's guard will assert that broken layout as the expected mirror while telling the developer to run 'pair update' — i.e. the guard cements it. Fixing the pipeline is the right layer; the guard follows automatically because it composes the real transform.

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