[US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring

[rucka]

PR Information

Story: #407 · Type: Bug fix · Priority: Medium (P2) · Assignee: @rucka
Classification: risk:yellow · cost:green · coupling not assessed (excluded from the max, D21)

Matrix — per dimension (review-time, D22)

Dimension	Tier	Source	Note
Service/domain criticality	yellow	KB default (no Criticality Table in tech/risk-matrix.md)	the install pipeline is how the product is delivered, but it is not a runtime service
Change/diff risk	yellow	diff footprint	~500 lines across 3 packages, in a transform shared by 5 asset registries; behaviour change is opt-in and pinned by a regression witness
Business impact	yellow	supporting subdomain (skills distribution)	a mis-install breaks skill loading for every consumer of pair update; latent today
Security relevance	green	path heuristic + round-1 review	no auth/secrets/network path; the change ADDS a traversal guard on the preserved tail
Coupling balance	not assessed	/pair-capability-assess-coupling not run in this pass	excluded from the tier max (D21); the coupling argument for the chosen option is in ADR-020

Tier = max(yellow, yellow, yellow, green) = risk:yellow, equal to the label already applied. Cost is computed but not projected as a tag (tech/risk-matrix.md → Active: risk). Confidence: high (diff read in full).

Derived acceptance criteria are recorded on #407 (the story skipped Draft→Ready — see the note there).

Summary

What Changed

flattenPath replaced every directory separator with a hyphen, so a skill's nested directory installed as a sibling pseudo-skill: process/review/references/deep.md landed at pair-process-review-references/deep.md, outside the skill, with both of its relative links dead. The first skill to use the standard Agent-Skills references/ progressive-disclosure layout would install unusable.

Three layers, all in this PR:

1. Placement — flattenDepth bounds flattening to the registry's entry granularity: the first N segments are joined, deeper ones stay a real sub-path. The skills registry's entries are two segments (process/review), so a third segment is content of that skill.
2. Links — the link rewriter rebases a target through any directory the same copy moved, not only the file's own. A sub-doc's ../SKILL.md points at its parent, which also moved; before this it fell through to the source-root fallback and came out as ../../../source/process/review/SKILL.md, a path back into the dataset layout.
3. Wiring — the option is read from config.json, validated, and threaded RegistryConfig → SyncOptions → TransformOpts → transformPath, so pair update/install/package actually honour it. "flattenDepth": 2 is declared on the skills registry.

Why This Change

The layout is not hypothetical: the third-party agent-browser skill already ships references/*.md, and #313's direction is progressive disclosure for the pair corpus. The defect is latent only because no dataset skill uses it yet — and #384's mirror guard would otherwise cement the broken layout as the expected mirror while telling the developer to run pair update.

Story Context

As a maintainer installing the Pair skills I want pair update to install a skill's nested references/ inside that skill's directory, with working relative links So that the first skill using progressive disclosure installs usable.

DoD coverage (#407's Definition of Done Expectations): functionality implemented ✅ · automated tests incl. an end-to-end nested-references/ fixture through the copy pipeline and through pair update ✅ · documentation updated (skill-authoring convention) ✅ · mirror guard (#384) green and its derivation re-pointed at the corrected mapping ✅.

Changes Made

Implementation Details

• Why not simply fix flattenPath for everyone: flattening every separator is correct for a registry whose entries are single-segment. Making the bounded form the default would silently change behaviour for the four other asset registries, none of which has a defect. Hence an opt-in depth, plus a regression witness test pinning the unbounded result so it cannot drift.

• prefixPath already prefixed only the top-level segment, so a preserved sub-path lands under the prefixed entry with no extra work: process-review/references → pair-process-review/references.

• Entry vs. content. Once flatten is bounded, a sub-directory below the entry appears in the copy's directory mapping next to real entries. It is content, not a skill, so isRegistryEntryPath excludes it from the skill-name map, the skill-link-path map and the frontmatter name: sync. Without that, references is registered as a skill name mapped to one arbitrary skill's sub-dir and the /references token in an unrelated skill's body is rewritten to it — a cross-skill corruption decided by directory iteration order.

• Fails loudly, in both boundaries. flattenDepth is validated as a positive integer at the CLI boundary and flattenPath throws on an invalid value, because the pre-existing silent degradation went in the direction that reintroduces the bug (0/-1 full-flattened; 1.5 flattened nothing). flattenPath also refuses a ./.. segment in the preserved tail: the unbounded form was traversal-safe by construction, a preserved tail joined onto the destination root is not.

• One exclusion, stated and enforced (review round 2). flattenDepth assumes every entry sits at the SAME depth, and .skills/ does not: next/ is a one-segment entry beside 39 two-segment ones, so next/references would have the shape of a real entry and install as the sibling pair-next-references/ — the whole defect back for that one skill, with the skill-name map leaking references into AGENTS.md and .pair/knowledge too. The shape is unrepresentable, not merely unhandled, so copyDirectoryWithTransforms rejects it before copying anything (a directory shallower than flattenDepth that holds files directly AND owns a sub-directory). A category directory is told apart from an entry by "holds files directly" — no SKILL.md knowledge, per ADR-020's coupling argument. Recorded as an ADR-020 trade-off and as the one exclusion in nested-sub-documents.md.

• Most specific move wins (review round 2). resolveAbsoluteTarget now asks rebaseWithinMovedDirs BEFORE the own-directory rebase. movedDirs contains the file's own directory, so the coarser stage was subsumed; asking it first let the less specific match win and left a FORWARD link into a sub-directory that moved elsewhere (./references/deep.md under an unbounded flatten) pointing at nothing. Both directions are now asserted in the same test.

• Mirror cleanup follows the sub-path (review round 2). Top-level-only cleanup was correct while one source sub-directory mapped to one top-level target directory; under a bounded flatten it maps to a sub-PATH, so a references/ removed from the source stayed installed forever. Cleanup descends into a bounded entry, gated on flattenDepth being present so the unbounded path is byte-for-byte unchanged.

• One name, one home. The option is flattenDepth end to end; TransformOpts moved next to transformPath (its only consumer) and copy/copy-types.ts re-exports it, so buildSkillNameMap/buildSkillLinkPathMap can declare the full type instead of a narrower one that lied about reading the depth.

Files Changed

Transform + copy pipeline (packages/content-ops)

• Modified: ops/naming-transforms.ts (flattenDepth, isRegistryEntryPath, isValidFlattenDepth — the predicate now shared with the CLI boundary —, TransformOpts, the two assertions, thrown as the layer's typed IO_ERROR), ops/link-rewriter.ts (movedDirs + rebaseWithinMovedDirs, longest-match single pass; two doc corrections), ops/copy/copy-directory-transforms.ts (buildTransformOpts, isEntryDir), ops/copy/copy-types.ts (re-export), ops/SyncOptions.ts, index.ts, ops/skill-reference-rewriter.ts
• Added: ops/copy/layout-validation.ts — the three source-layout validators + collectDirShapes, extracted from the copy module (pure shape analysis, changes for a different reason)
• Tests: ops/naming-transforms.test.ts (+bounded-depth, invalid-depth, traversal incl. the single-segment symmetry, isRegistryEntryPath), ops/copy/copy-directory-transforms.test.ts (+5 end-to-end incl. the two-skills cross-rewrite case), ops/skill-reference-rewriter.test.ts (+2 map-scoping cases)

CLI wiring (apps/pair-cli)

• Modified: config.json ("flattenDepth": 2 on skills), src/registry/resolver.ts, src/registry/validation.ts (validateFlattenDepthField), src/registry/operations.ts (buildCopyOptions)
• Tests: src/registry/{resolver,validation,operations}.test.ts, src/commands/update/handler.test.ts (+3 CLI-level cases through pair update)

Mirror guards (packages/knowledge-hub)

• Modified: src/tools/skill-md-mirror.ts (SKILL_COPY_OPTS + module docstring re-pointed at the corrected mapping — in scope per the story), src/tools/skills-guide-mirror.ts (reuses that single constant instead of re-typing the knobs), src/tools/skill-md-mirror.test.ts (pin test now covers flattenDepth), src/tools/skills-conformance-check.ts (+ its test — new entrypoint-depth check)

Decision + documentation

• Added: .pair/adoption/tech/adr/adr-020-bounded-flatten-depth-entry-granularity.md; skill-conventions/nested-sub-documents.md (dataset source + root mirror)
• Modified: .pair/adoption/tech/adr/adr-005-skills-infrastructure.md (amended; its behavior: "mirror" corrected to "overwrite"), skill-conventions/README.md (index row, both copies), .pair/llms.txt (regenerated), apps/website/content/docs/reference/{configuration,skill-management}.mdx

Regenerated mirror

• Modified: .claude/skills/pair-process-plan-epics/SKILL.md — one link, regenerated output of the corrected rewriter (plan-epics → map-subdomains now rebases through the moved sibling instead of re-rooting). Not a hand edit.

Testing

Test Coverage

Test-first, per the project's bug workflow. Every behavioural change was written RED first and verified RED against branch code before the fix:

• the placement cases (process-review-references where process-review/references was expected);
• the back-link case (../../../source/process/review/SKILL.md where ../SKILL.md was expected);
• the four entry-vs-content cases — reproduced from the reviewer's exact dataset (two skills each owning a references/ dir), confirmed to produce the cross-skill rewrite and the path-as-name frontmatter before the fix.

Round 2 added, same discipline:

• the shallow-entry reproduction (next + next/references + a second skill whose body contains /references) — RED as a mis-install at pair-next-references/, now a loud rejection, plus the positive case (a one-segment entry with no sub-dir installs beside two-segment ones, the real .skills/ shape today);
• the FORWARD link under an unbounded flatten, added to the existing sibling test — RED as ./references/deep.md;
• mirror + bounded idempotency across three runs, with the nested source dir removed before the third;
• the depth assertion for an empty path, the typed IO_ERROR name, and isValidFlattenDepth as a unit.

Coverage shape: unit on flattenPath/transformPath/isRegistryEntryPath (incl. the unbounded regression witness and the invalid/traversal rejections); end-to-end through copyPathOps; CLI-level through handleUpdateCommand — with the negative case pinned too (no flattenDepth ⇒ the same dataset still installs the pre-#407 sibling layout, so the option is provably the thing doing the work).

Test Results

Stated without absolute test counts on purpose: they go stale on the next commit and the body is the merge-gate evidence artifact (round-4 finding). Reproduce with pnpm quality-gate.

@pair/content-ops     ✅ all green (49 test files, 0 todo)
@pair/pair-cli        ✅ all green (72 test files)
@pair/knowledge-hub   ✅ all green (22 test files — mirror + skills-guide guards)
ts:check / lint       ✅ clean (all three packages)
prettier:check        ✅ clean on every file in this diff (two pre-existing drifted files on `main` still fail in `@pair/pair-cli` — see the note under Quality Assurance; the root `quality-gate` runs `prettier:fix`, which is why a local gate run looks green)
hygiene:check         ✅ PASS
skills:conformance    ✅ PASS — 40 skills (now incl. entrypoint depth)
docs:staleness        ✅ PASS — 40 skills, 8 commands in sync
dup:check             ✅ exit 0 (18 pre-existing clones, none in touched files)
markdownlint          ✅ clean on every touched .md

Quality Assurance

Review Areas

• Is flattenDepth: 2 the right expression of "entry granularity"? A numeric depth is generic but positional — it encodes a fact about the registry's layout in the caller and must be updated in step if a category level is ever added or removed. The sharper alternative ("the directory containing SKILL.md is the entry") was rejected on coupling, not correctness. The full argument, with preserveNested and registry-wide depth-1 as the other two rejected options, is on record in ADR-020 rather than in this body.
• Two flatten semantics now coexist (bounded for skills, unbounded elsewhere). Deliberate and pinned by test; recorded as a Trade-off in ADR-020.
• markdownlint/prettier note: apps/pair-cli/src/commands/kb-info/version-check*.test.ts carry pre-existing prettier drift on main. The repo-wide pre-push formatter touches them on every push; they are deliberately not in this diff.

Documentation

Documentation Updates

• Technical documentation (ADR): .pair/adoption/tech/adr/adr-020-bounded-flatten-depth-entry-granularity.md — new, the bounded-flatten decision with the four options considered; amends ADR-005 §Key Design Choices Collaborative Knowledge Base #2 and its Flatten/prefix rationale bullet, which are edited in place so ADR-005 no longer reads as authority for the superseded semantic. Trade-offs record the two limitations the model carries: the duplicated depth number, and that every entry must sit at the declared depth — with the too-shallow rule stated AS ENFORCED (it covers a category directory with a file of its own, not only a registry-root skill) and the nested-mirror-cleanup descent recorded as an ACCEPTED RESIDUAL (forward-compatibility: it needs behavior: 'mirror', and skills is overwrite).
• Authoring convention (KB guideline): skill-conventions/nested-sub-documents.md — new, in BOTH copies (.pair/knowledge/… and its packages/knowledge-hub/dataset/ source, byte-identical per the mirror-equality guard): the layout, how it installs, that relative links work in both directions, that a sub-doc is content (no prefix, no name: sync, not invocable), and the two exclusions (too shallow — incl. a category directory with a file of its own — and too deep, with the exact error text). Indexed by a row in that directory's README.md, both copies.
• Code documentation: the three resolution stages of resolveAbsoluteTarget, the scope note on rebaseWithinMovedDir, and the mirror-cleanup granularity docstring were all corrected to match the post-change behaviour rather than the pre-tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407 one.
• User documentation (website reference): apps/website/content/docs/reference/configuration.mdx — a flattenDepth row in the Registry Fields table, a new Bounded Flattening subsection under Transform Configuration (bounded vs. unbounded mapping for the same source, plus the entry/content and fail-loudly rules), and "flattenDepth": 2 in the Skills Registry Example, whose stale "behavior": "mirror" is corrected to "overwrite" to match config.json. Also reference/skill-management.mdx — the flatten step, its code snippet and the naming table now show the bounded form. Round-4 finding: this is the maintained public reference for the registry schema, so an external KB author authoring <category>/<name> entries with a references/ sub-dir could not discover the remedy. The earlier "n/a — registry config, described in ADR-020" was wrong: an ADR under .pair/adoption/tech/ is internal decision history, not user documentation.
• Generated KB index: .pair/llms.txt regenerated for the new guideline (and the pre-existing coupling-balance.md gap it exposed). Uncovered drift class: nothing gates .pair/llms.txt — docs:staleness covers skills/commands only, and the mirror-equality guard has no dataset source for a generated file. Two independent misses suggest a gate story of its own — filed as tech-debt: no gate covers .pair/llms.txt drift — the generated KB index goes stale silently #416, not fixed here.

Knowledge Sharing

• Technical decision recorded: ADR-020 (bounded flatten depth, entry granularity, opt-in) — amends ADR-005 §Key Design Choices Collaborative Knowledge Base #2 and its Flatten/prefix rationale, which documented flatten as full flattening. ADR-005 was edited in place so it no longer reads as authority for the superseded semantic. Numbering: adr-018 is claimed by two open branches (PR state flow (gate≠review) + pair review as required check #234, Code host separate from PM tool (WoW override) — GitHub + Linear reference case #236); 019 is left free for whichever renumbers.
• Authoring convention documented: skill-conventions/nested-sub-documents.md — a skill at the standard <category>/<skill-name> depth may ship a nested references/, how it installs, that relative links work in both directions (so the author must not pre-compensate), that a sub-doc is content (no prefix, no name: sync, not invocable), and the two exclusions (too shallow / too deep). Authoring rule 1 ("only the entry directory holds SKILL.md") is now enforced by skills:conformance, not just stated. Written generic/portable per that directory's Scope note, so the cross-reference runs one way only, from ADR-020.

Dependencies & Related Work

• ⚠️ Behaviourally coupled to [US-384] chore: mirror-equality guard widened to every dataset skill artifact #406 (story tech-debt: extend mirror-equality guard to all root skill artifacts (sub-docs + references) #384). That PR has two tests asserting the current broken mapping, by name: 'flattens a nested sub-dir into its own prefixed dir, as the pipeline does today (#407)' and 'rewrites a nested reference file too (flattened into its own prefixed dir)'. They were written knowing tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407 would invert them. Once both PRs are on main, those two assertions must be updated in the cascade rebase — they are not conflicts (different files), so nothing flags them automatically. Recorded here because file-level disjointness analysis does not catch a behavioural collision.
• Otherwise file-disjoint from the open PRs. [US-279] feat: pair-cli scaffold-kb — external KB repo + release script (reuses pair package) #391 touches content-ops but only file-system-service.ts and the in-memory-fs test utils — no overlap with ops/.

Closes #407

🤖 Generated with Claude Code

---

Round 3 (018fb287, bf76aa60) — two defects this PR had introduced

[Major] The layout guard was asymmetric. Round 2 rejected an entry shallower than flattenDepth and let the deeper case through silently: capability/sub/foo/SKILL.md installed at pair-capability-sub/foo/SKILL.md — a pseudo-entry directory with no SKILL.md at its root, invisible to the skill loader. Two further defects followed silently, because isRegistryEntryPath reports false for it: the frontmatter name: stayed unsynced, and no entry reached skillNameMap, leaving a /foo reference in an unrelated skill dangling.

This was a regression this PR introduced, not an uncovered edge — full flattening produced a perfectly usable pair-capability-sub-foo/ for the same source. And neither CI gate catches it: skills:conformance only walks <cat>/<sub>/SKILL.md so the file is never read, and the #384 mirror guard derives the installed path from the same transformPath, so it compares equal and passes.

validateNoDeepEntry uses the shape data already collected, with no SKILL.md knowledge (ADR-020's coupling argument): a directory deeper than flattenDepth is content iff its nearest ancestor at that depth also holds files directly. So process/review/references still passes; capability/sub/foo fails. Injection-verified — with the guard disabled the test resolves instead of rejecting, and skillNameMap comes out with one entry instead of two.

[Minor] The bounded form was less traversal-safe than the unbounded one it replaces. The guard checked only the preserved tail, and the argument that the head is safe ("every separator becomes a hyphen") fails at flattenDepth === 1, where the head is a single un-joined segment: flattenPath('../evil', 1) returned '../evil'. Every segment is now validated. Not reachable through the CLI today, which is why it was a hole rather than a live bug.

Fixture split — one entry depth per property it can prove

Discovered while rebasing onto the merged #406, and worth recording because it is easy to get backwards:

• The link-depth rewrite is observable only with a one-segment entry (demo/ is 2 levels below the dataset root, pair-demo/ is 3 below the mirror root). With a two-segment entry both sides are 3 levels, the rewriter correctly leaves the link alone, and the assertion proves nothing. I moved that fixture to two segments mid-rebase and silently destroyed its purpose; it is back on one, with the reason in the test.
• The nested references/ case needs a two-segment entry, because a one-segment entry owning a sub-directory is refused by the shallow-entry guard. It has its own catalog/nested entry.

Also inverted #406's two assertions that pinned the old mapping. They said so by name — "as the pipeline does today (#407)" — and were written knowing this PR would invert them.

Verification: pnpm quality-gate green at that head (counts deliberately not quoted — see Test Results).

Round 4 (8b357176) — the public reference, and the rule as enforced

11 findings (1 Major + 8 Minor + 2 Questions), all resolved, none escalated.

[Major] flattenDepth shipped undocumented outside this repo. The website's reference/configuration.mdx documents every other registry field and never mentioned it, its Transform Configuration explained flattening as unbounded only, and its Skills Registry Example diverged from config.json. All three fixed (plus the stale behavior value there and in ADR-005 §5, which skill-md-mirror.ts pin-tests as overwrite), and reference/skill-management.mdx — which reproduced the old flattenPath — brought in step.

The guard's rule is now documented as it is ENFORCED, not as the case that motivated it. validateNoShallowEntryWithSubdir rejects any directory shallower than flattenDepth that holds files directly and owns a sub-directory — which includes a category directory with a README, not only a registry-root skill. That is intentional (entry-vs-category is decided by "holds files directly", so no SKILL.md knowledge enters a transform four registries share) but was under-documented and its remediation gave wrong advice for that shape. ADR-020 and nested-sub-documents.md now state the broader rule, the error message names the extra way out (…or move/remove the file(s) held directly by '<dir>') and no longer interpolates flattenDepth as the child's depth, and a test pins the category-file shape so the rule cannot silently narrow.

Two silent-divergence classes closed, both of the kind this PR elsewhere fails loudly on:

• { flatten: false, flattenDepth: 2 } applied an unbounded path transform with bounded entry classification (a references/ silently dropped from the skill-name map and the frontmatter sync). Unreachable via the CLI, reachable through the exported copyDirectoryWithTransforms; buildTransformOpts now drops the depth when flatten is false — one source of truth for "bounded?" — pinned by a test.
• skills:conformance now fails on any SKILL.md below the entry depth. process/review/references/SKILL.md is correctly-shaped content for the copy-time guards (telling it apart there needs the marker-file knowledge ADR-020 refuses), so it installed silently non-invocable — authoring rule 1 of the new convention was stated but unenforced. Static corpus knowledge is the right layer; ENTRY_DEPTH is pinned to SKILL_COPY_OPTS.flattenDepth by test.

Traversal guard is now symmetric by SHAPE. The docstring claimed the unbounded form is safe by construction ("every separator becomes a hyphen") — true only with a separator: flattenPath('..') returned '..'. Round 3 made the bounded form stricter than the unbounded default four registries use; the guard now applies to any single-segment result in both forms, so neither is LESS safe than the other (they are deliberately not equally strict — a preserved tail is live where a hyphenated segment is inert; round-5 correction). Not reachable through the pipeline (dirname(relative(...)) never yields ./..) — defensive depth and doc accuracy.

Answered (Questions):

• Is the nested mirror cleanup live? No — it needs behavior: 'mirror' and skills is overwrite. Relabelled in code as forward-compatibility (it no longer reads as fixing a live defect) and recorded as an ACCEPTED RESIDUAL in ADR-020: a references/ deleted from the dataset stays installed, consistent with the pre-existing overwrite semantics for a whole skill directory. AC6 is met as written for a mirror registry.
• Is the unenforced authoring rule 1 separable? It was closed here instead — a few lines in the gate, and it is the same silent-hole class round 3 closed for the neighbouring shape.

Also: .pair/llms.txt regenerated (uncovered drift class flagged above); the DR-1 suggestion taken — collectDirShapes + the three validators extracted to ops/copy/layout-validation.ts (copy module back to ~400 lines, one summary sentence); the misattributed test comment moved onto the back-link test it actually describes; the stale "#407 installs it wrongly" caveats in skill-md-mirror.ts corrected (they contradicted the shipped behaviour after the #384 rebase).

Round 5 (109b18a6) — documentation says what the code enforces

6 findings (5 Minor + 1 Questions), all resolved, none escalated. Documentation/message wording only — one new test, no behaviour change.

The too-deep error's second way out is now qualified. …or give '<ancestor>' files of its own made the guard pass by reclassifying the offender as CONTENT — which, for a registry whose entries carry an entrypoint file, produces exactly the non-invocable install the same message cites as the reason to reject. In this repo skills:conformance catches the follow-on shape; a @pair/content-ops consumer has no such gate, so the message now says …IF '<dir>' is meant to be CONTENT of it — note that an entrypoint file inside content installs as content, not as an entry, i.e. with exactly the symptoms above, pinned by a test so it cannot regress to the bare advice. (layout-validation.ts, copy-directory-transforms.test.ts)

"Every entry must sit at that same depth" was an overstatement of the enforced rule — and this repo is the counter-example: .skills/next is a one-segment entry beside 39 two-segment ones under flattenDepth: 2 and installs fine; a shallower entry aborts the copy only once it also owns a sub-directory. The website's Bounded Flattening bullet (the terse one — it contradicted its own trailing clause) now states the enforced rule and says a mixed-depth corpus is workable, so an external KB author with that shape does not conclude the option is unusable for them. nested-sub-documents.md's heading and lead reworded in step, both copies. (configuration.mdx, nested-sub-documents.md ×2)

The flattenPath docstring's symmetry claim was half false. "Never MORE strict than the unbounded form" is contradicted by the code on purpose: with a preserved tail every segment is validated, so flattenPath('a/../b', 1) throws while the unbounded call returns the inert 'a-..-b', and a .. among joined entry segments stays unchecked in both. Restated as the real invariant — each form is checked exactly where its own output could carry a live ./.., so neither is less safe — with an explicit "do not restore symmetry by weakening the tail check", which is precisely how round 3's hole was born. ADR-020's Benefits bullet and the test comment corrected in step. (naming-transforms.ts + test, adr-020)

The tutorial's team config.json examples now declare flattenDepth: 2. tutorials/managing-ai-artifacts.mdx is the copy-paste path for an org authoring its own KB package and teaches the <category>/<name> layout; the examples were correct only until a skill ships a references/. Both examples updated (correct for the tree shown, future-proof) plus one sentence linking Bounded Flattening. (managing-ai-artifacts.mdx)

Answered (Questions) — a sub-document must not be linked from OUTSIDE its skill. buildSkillLinkPathMap maps entrypoints only, and this PR chains it onto the non-skill registries, so a KB/AGENTS.md link to ../.skills/<cat>/<name>/references/deep.md would be left pointing into the uninstalled source tree. Unreachable today; the convention now states it as authoring rule 5, with per-file mappings named as the follow-up if a direct deep link ever becomes necessary. No code change — confirming the documented contract, in the direction the convention had not covered.

Also: the Test Results prettier:check line reworded to what reproduces (two pre-existing drifted files on main fail in @pair/pair-cli; the root quality-gate runs prettier:fix, which is why a local gate run reads green), and the round-4 "neither less nor more safe" phrasing above corrected to match the invariant.

[rucka]

Verdict

risk:yellow · cost:green — CHANGES-REQUESTED — the transform is fixed and verified, but flattenDepth never reaches the copy pipeline from configuration, so pair update still mis-installs a nested references/ — while commit 724ea0d says Closes #407.

PR: [#411] · Author: @rucka · Reviewer: independent reviewer (Claude Opus 5) · Date: 2026-07-31 · Story: [US-407] · Type: bug

Classification matrix — per dimension

Dimension	Tier	Source	Note
Service/domain criticality	yellow	install/distribution pipeline	content-ops/ops is shared by every asset registry
Change/diff risk	yellow	8 files, +255/-13	opt-in param + one behaviour change in the shared link rewriter
Business impact	yellow	supporting subdomain (install artifacts)	latent for placement; the link-rewrite half fixed a live dead link
Security relevance	yellow	manual read (/pair-capability-assess-security not run separately)	preserved .. segment can escape the destination root — unreachable today (finding 5)
Coupling balance	yellow	manual read (/pair-capability-assess-coupling not run separately)	a second flatten implementation lives in pair-cli/registry/layout.ts and didn't learn the rule

Tier = max(assessed) = yellow, confirming the story's refinement-time risk:yellow (no raise, no drift note). Cost = highest detected signal = green. Review value is a floor (D17): confirm or raise, never lower.

Assessments

Security — Input validation

Verdict: yellow — the only new input is flattenDepth, and it is unvalidated at every boundary.

Details

• flattenDepth is typed number with no runtime validation. apps/pair-cli/src/registry/validation.ts has validators for flatten and prefix but none for flattenDepth (and rejects no unknown keys), so a config typo passes silently.
• flattenPath silently degrades on 0, negatives and fractions instead of erroring — see finding 6.
• No network/user input is touched by the diff; all path segments originate from a filesystem walk (collectFiles).

Security — Output handling

Verdict: yellow — the preserved tail is written into a filesystem path without containment checks.

Details

• flattenPath('process/review/../../../../etc', 2) → process-review/../../../../etc; posix.join('/dest/target', …) (as copy-directory-transforms.ts:93 does) → /etc. Verified by direct execution against branch code.
• The pre-change unbounded form was traversal-safe by construction. Not reachable from collectFiles today (no real directory is named ..), but flattenPath/transformPath are public exports (packages/content-ops/src/index.ts:58). Finding 5.

Security — Authentication

Verdict: green — no authentication surface in the diff.

Details

Pure path/string transforms plus local filesystem copy. No credentials, tokens, or remote calls.

Security — Authorization

Verdict: green — no access-control surface in the diff.

Details

No authorization decision is made or changed. Write targets remain the registry's configured targets.

Security — Introduced vulnerabilities

Verdict: yellow — [1 introduced (not exploitable via any current call path), 0 pre-existing]

Details

Severity	Category	File:location	Introduced / pre-existing	Recommendation
P3	A01 Path traversal (defence-in-depth)	packages/content-ops/src/ops/naming-transforms.ts:32-38	introduced	Reject/normalize .. in the preserved tail; assert containment

No red introduced finding, so the introduced-red-security rule (#227/AC4) does not fire. The CHANGES-REQUESTED verdict comes from the unmet AC and the missing ADR, not from security.

Cost

Verdict: cost:green — no cost signal in the diff.

Details

Signal	Class	Provider	Note
Managed service / infra change	green	n/a	none — local filesystem copy only
AI/LLM invocation	green	n/a	none added
CI minutes	green	GitHub	+11 unit tests in an existing suite (~ms)

Architecture (Coupling)

Verdict: yellow — one concept, two implementations, and only one of them learned the new rule.

Details

• packages/content-ops/src/ops/naming-transforms.ts (flattenPath/transformPath) is the source of truth the copy pipeline and the tech-debt: extend mirror-equality guard to per-skill SKILL.md pairs (all 36 skills) #352 mirror guard both compose — good.
• apps/pair-cli/src/registry/layout.ts:105 applyPrefixFlatten (and the reverse transformTargetToSource, line ~277) is an independent re-implementation of flatten/prefix used for expected-target-name computation and target→source mapping. It has no notion of depth, so once flattenDepth is actually enabled the two disagree for any nested path. Integration strength: contract/shared-knowledge; distance: same repo, same team; volatility: low. Balance is tolerable, but the duplication is what makes finding 1 easy to get half-right.
• The movedDirs threading (rewriteLinksAfterTransform → rewriteLinksInFile → computeNewHref → resolveAbsoluteTarget) adds an optional parameter across four layers of one module — acceptable, built once per batch, and inert when absent.

Details

Findings by severity

Critical (must fix before merge)

none

Major (should fix before merge)

• apps/pair-cli/src/registry/resolver.ts:60-72 + apps/pair-cli/src/registry/operations.ts:150-162 + apps/pair-cli/config.json:31-46 — flattenDepth never reaches the copy pipeline from configuration. normalizeRegistryConfig does not read raw['flattenDepth'], RegistryConfig does not declare it, and buildCopyOptions does not set it — so pair update / install / package always run with flattenDepth: undefined, i.e. unbounded flatten, i.e. the nested dir still installs as the sibling pair-process-review-references/. Adding "flattenDepth": 2 to config.json today would be silently dropped (no validator, no unknown-key rejection). Story tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407's statement — "pair update install a skill's nested references/ inside that skill's installed directory" — is therefore not met at the only layer the story names, while commit 724ea0d carries Closes #407. The PR's own stated reason for deferring the wiring ("turning it on while back-links are broken would trade one broken layout for another") no longer holds: the back-link half is in this same PR. Fix: read + validate flattenDepth (positive integer) in resolver.ts/validation.ts, add it to RegistryConfig and buildCopyOptions, declare "flattenDepth": 2 on the skills registry, and update SKILL_COPY_OPTS + module docstring in packages/knowledge-hub/src/tools/skill-md-mirror.ts:38 in the same change (the story lists this re-pointing as in scope) — plus one CLI-level fixture through update/package, as the story's scope asks. If you judge the wiring large enough for its own story, file it, retitle this PR, and drop Closes #407 from the commit — do not merge a Closes on an unmet AC.
• packages/content-ops/src/ops/skill-reference-rewriter.ts:180-193 (+:236-249) — a nested content directory is registered as a skill name, corrupting installed content on exactly the layout this PR enables. Reproduced against branch code: dataset process/review/{SKILL.md,references/deep.md} + capability/grill/{SKILL.md,references/deep.md} with flattenDepth: 2 yields skillNameMap = [["references","pair-capability-grill/references"],["review","pair-process-review"],["grill","pair-capability-grill"]], and the `/references` token inside pair-process-review's installed SKILL.md is rewritten to `/pair-capability-grill/references` — a cross-skill rewrite, last-writer-wins on directory iteration order. Related: copy-directory-transforms.ts:144-151 runs syncFrontmatter({ from: 'references', to: 'pair-process-review/references' }) on the sub-doc, so a sub-doc with name: frontmatter gets a path as its name. Fix: in buildSkillNameMap/buildSkillLinkPathMap (and the frontmatter sync) skip any originalSubDir deeper than the entry granularity — those are content, not skills — and add a test with two skills each owning a references/ dir.
• packages/content-ops/src/ops/naming-transforms.ts:11-38 — missing ADR/ADL — story tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407's Open Questions state the choice ("redefine flatten registry-wide (depth-1 only) or gain an explicit flattenDepth/preserveNested knob") is ADR-worthy, and .pair/adoption/tech/adr/adr-005-skills-infrastructure.md §Key Design Choices Collaborative Knowledge Base #2 / §Rationale documents flatten as full flattening because "AI tools expect skills in flat directory structures". This PR changes that semantic for the skills registry and records the reasoning only in the PR body and code comments; no ADR or ADL exists (grep flatten over adoption/tech/adr hits only ADR-005; there is no adl/ directory). Per CLAUDE.md ("architectural/project decisions MUST be recorded as ADR or ADL") and /pair-process-review Step 2.3, a new technical decision without an ADR is a HALT/CHANGES-REQUESTED condition. Fix: /pair-capability-record-decision — amend or supersede ADR-005, recording the numeric-depth form chosen, the alternatives rejected (preserveNested, "the dir containing SKILL.md is the entry"), and the opt-in default.
• PR description + title (#411) — materially contradicts head 724ea0d. The title still says "(placement half)"; the body's "⚠️ This PR delivers half the story, deliberately" section states the back-link "is still mangled to ../../../source/process/review/SKILL.md", that it is "recorded as an it.todo", and that "Merging this PR does not close tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407". None of that is true of the pushed head: the back-link is fixed (copy-directory-transforms.test.ts "keeps a nested sub-doc back-link pointing at its own skill (tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407)"), grep -rn 'it\.todo' packages/content-ops/src returns nothing, and commit 3 says Closes #407. "Files Changed" omits link-rewriter.ts (+61/-6, the largest behavioural change) and .claude/skills/pair-process-plan-epics/SKILL.md; "Test Results" says 641 tests / 1 todo where commit 3 says 644 / 0. The PR body is the review artifact of record — a reviewer, or a later resumed session, draws false conclusions about what shipped. Fix: rewrite the description against head per pr-template.md, fix the title, and make the closure semantics match the delivered scope.

Minor (consider)

• packages/content-ops/src/ops/naming-transforms.ts:32-38 — a .. segment beyond maxDepth is preserved verbatim, so the result can escape the destination root: flattenPath('process/review/../../../../etc', 2) → process-review/../../../../etc, and posix.join('/dest/target', …) → /etc (verified). The unbounded form was traversal-safe by construction. Not reachable from collectFiles today, but flattenPath/transformPath are public exports (index.ts:58). Reject/normalize ./.. in the preserved tail, or assert join(destPath, transformed) stays under destPath, with a test.
• packages/content-ops/src/ops/naming-transforms.ts:31 — invalid depths degrade silently, in the direction that reintroduces the bug: 0 → process-review-references (full flatten), -1 → same, 1.5 → a/b/c/d (no flattening at all). A config typo therefore fails silently. Validate at the CLI boundary (positive integer) and/or throw on a non-positive-integer maxDepth; add cases for 0, negative, fractional.
• packages/content-ops/src/ops/link-rewriter.ts:79-84 — the "Known scope boundary" paragraph on rebaseWithinMovedDir is now wrong: a target in a different transformed sibling dir no longer "still falls through to reRootTarget" (it goes through the new rebaseWithinMovedDirs first), and "not needed by any skill in the corpus today" is falsified by this very PR (plan-epics → map-subdomains). Update it to describe the two-stage resolution.
• packages/content-ops/src/ops/link-rewriter.ts:156-172 — the JSDoc "Computes the new href for a relative link after the file has moved. Returns null if the link should not be rewritten…" now sits immediately above splitRewritableHref, which has its own JSDoc right below it, while computeNewHref (line 174) is left undocumented. Two stacked doc blocks, the first describing a different function. Move it back onto computeNewHref.
• packages/content-ops/src/ops/link-rewriter.ts:143-154 — rebaseWithinMovedDirs copies and re-sorts movedDirs on every link of every file (O(F·L·D log D)), although the array is already built once per batch at line 306. Sort once in rewriteLinksAfterTransform and pass the sorted array.
• packages/content-ops/src/ops/naming-transforms.ts:27 vs :62 — one concept, two names: the option is flattenDepth in SyncOptions/TransformOpts/transformPath, the parameter is maxDepth in flattenPath, and it is positional (flattenPath('process/review/references', 2)) while the sibling transformPath takes an options object — a bare magic number at call sites. Name it flattenDepth throughout, or document the alias once.
• packages/content-ops/src/ops/skill-reference-rewriter.ts:181 and :240 — buildSkillNameMap/buildSkillLinkPathMap declare transformOpts: { flatten?: boolean; prefix?: string } yet forward it to transformPath, which now also reads flattenDepth. It works only because the caller happens to pass the wider TransformOpts object through unchanged; the declared type lies about what the function consumes, and any refactor that destructures and rebuilds the object silently drops the depth. Type both as TransformOpts.
• Docs — .pair/knowledge/guidelines/technical-standards/ai-development/skill-conventions/README.md (absent change) — story tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407's DoD requires "Documentation updated (skill-authoring convention: nested references/ supported)". Nothing under skill-conventions/ (nor ADR-005) states that a skill may ship a nested references/ sub-dir or how it installs, so the convention this PR exists to enable is undiscoverable to a skill author. Add a short section, cross-referenced from ADR-005, once the wiring lands.

Questions

• packages/content-ops/src/ops/naming-transforms.ts:62 — is flattenDepth: 2 the right expression of "entry granularity"? The PR flags this as worth challenging, and I agree it is the open design point: a numeric depth encodes a registry-layout fact in the caller and silently mis-flattens if a category level is ever added or removed. Were preserveNested: true (flatten only the entry) or "the directory containing SKILL.md is the entry" considered and rejected on record? Please capture the answer in the ADR from the third Major finding.
• Cross-PR (behavioural collision with [US-384] chore: mirror-equality guard widened to every dataset skill artifact #406, still OPEN) — [US-384] chore: mirror-equality guard widened to every dataset skill artifact #406 carries two tests asserting today's mapping by name ('flattens a nested sub-dir into its own prefixed dir, as the pipeline does today (#407)', 'rewrites a nested reference file too (flattened into its own prefixed dir)'); its nested back-link assertion changes under this PR's link-rewriter fix regardless of flattenDepth. The two PRs are file-disjoint, so nothing will flag it. Nothing to change in this PR — noted so the human merge gate sequences the two and updates [US-384] chore: mirror-equality guard widened to every dataset skill artifact #406's assertions in the cascade rebase. (Resolves after merge.)

Positive feedback

• Test-first, and the tests are diagnostic rather than decorative. The "regression witness" pinning the unbounded default is the right instinct for a shared transform, and the sibling case asserts the back-link is rewritten (../pair-process-review/SKILL.md), not merely left alone — which is what makes the second commit's claim ("the defect was not limited to nested references/") checkable.
• The link-rewriter half fixes a live, shipped defect, and I verified the regeneration is complete. Replaying the branch's copyDirectoryWithTransforms over the real packages/knowledge-hub/dataset/.skills tree reproduces every installed .md under .claude/skills/ byte-for-byte — SKILL.md files and the sub-docs (merge-and-cascade.md, degradation-levels.md, post-review-merge.md, assess-orchestration.md) that the tech-debt: extend mirror-equality guard to per-skill SKILL.md pairs (all 36 skills) #352 guard deliberately does not assert. So pair-process-plan-epics' ../../../.skills/capability/map-subdomains/SKILL.md really was a dead path in installed projects, ../pair-capability-map-subdomains/SKILL.md is the correct replacement, and nothing else in the corpus is left stale.
• The change leans on existing invariants instead of duplicating them: prefixPath's top-level-only prefixing and cleanupStaleTransformedEntries' transformedDir.split('/')[0] both remain correct under the new sub-path form, and the code says so rather than re-deriving it.
• movedDirs is built once per batch and inert when absent — the opt-in shape keeps every other asset registry on exactly the old code path, which is the right call for a transform shared by all of them.

Functionality & requirements (AC coverage)

Story #407 is an unrefined (Todo) tech-debt story with no Given-When-Then AC; its Story Statement + "Likely In Scope" + DoD are the effective criteria.

• Nested dir installs inside the skill — at library level (copyPathOps with flattenDepth: 2), with a passing end-to-end test.
• Forward link works (./references/deep.md) — no rewrite needed, asserted.
• Back-link works (../SKILL.md) — fixed and asserted, in both the "moved together" and "became a sibling" directions.
• "pair update … installs … inside that skill's installed directory" — not met: the option is unreachable from config.json/pair-cli (Major 1). The user-facing behaviour of pair update for placement is byte-identical to before.
• In scope: fixtures/tests "through install / update / package" — only through the content-ops library, not the CLI.
• In scope: re-point the mirror guard's documented derivation (skill-md-mirror.ts) — not done (consistent with the config not being wired, so it is the same gap).
• DoD: documentation updated (skill-authoring convention) — not done.
• DoD: mirror guard still green — verified independently against branch code (all installed artifacts match).
• Error handling: unchanged paths keep their fallbacks; rebaseWithinMovedDirs returns null so existing behaviour survives.
• Edge cases covered by tests: entry-depth path, shallower-than-depth path, more than one level below the entry, unbounded default. Missing: 0/negative/fractional depth (Minor 6), two skills each with a references/ dir (Major 2).

Testing & quality gates

• Adequate unit coverage on the pure transform (8 new cases) and one end-to-end case per direction through copyPathOps (3 new cases).
• Edge + error scenarios: invalid depth values untested; the skill-name-map / frontmatter-sync interaction with a nested dir untested (and broken — Major 2); no CLI-layer test, which is exactly where the wiring gap hides (Major 1).
• Quality gates (run independently in a detached review worktree): content-ops vitest 626/626 tests pass; tsc --noEmit -p packages/content-ops clean. Two content-ops suites (http/download-manager, http/resume-manager) failed to load in my worktree for lack of a built dist — an artifact of my read-only setup, not of the branch. Mirror-equality re-derived from branch code: no drift across all installed .md. I could not run the full pnpm quality-gate (no install in the review worktree); the PR's green claim is plausible, but its stated counts (641 tests / 1 todo) do not match head (644 / 0) — see Major 4.

Adoption compliance

• Degradation level: 2 — /pair-capability-verify-adoption treated as available in-repo, /pair-capability-assess-stack not needed (no new dependency; package.json untouched).
• Dependencies match tech-stack.md — no new dependency, no version change.
• Patterns: the change respects architecture.md's tool-package boundary (transform logic stays in content-ops), but leaves the duplicate flatten implementation in pair-cli/registry/layout.ts unaware of the new rule — see the Coupling assessment.
• ADRs present for new technical decisions — NO. ADR-005's documented flatten semantics are modified without an ADR/ADL, and the story itself flags the question as ADR-worthy. This is the process HALT condition of Step 2.3 (Major 3).

Tech debt

Surfaced, never blocking on debt grounds alone:

1. Duplicate flatten/prefix knowledge — apps/pair-cli/src/registry/layout.ts applyPrefixFlatten + transformTargetToSource vs content-ops/naming-transforms. Impact: high (a correctness divergence the moment flattenDepth is enabled); effort: medium. Worth deliberate promotion via /pair-capability-write-issue --label tech-debt if not folded into the wiring fix.
2. Mirror guard covers only SKILL.md (tech-debt: extend mirror-equality guard to all root skill artifacts (sub-docs + references) #384/[US-384] chore: mirror-equality guard widened to every dataset skill artifact #406 in flight) — my probe shows sub-docs are in fact byte-equal today, so widening the guard would lock in a property that already holds. Already tracked.
3. flattenPath positional numeric parameter vs transformPath's options object — small API inconsistency (Minor 10).

Documentation

• Code docs: unusually good — every new branch carries a "why", with the tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407 defect spelled out at each layer.
• Architecture docs: ADR-005 not amended (Major 3).
• User/authoring docs: skill-conventions/ says nothing about a nested references/ sub-dir (Minor 12).
• PR description: contradicts head (Major 4).

Performance & deployment

• No hot-path regression of consequence: link rewriting gains one extra pass over movedDirs per unresolved link; the per-link re-sort is wasteful but bounded (~42 entries for the skills registry) — Minor 9.
• Deployment/rollback: pure opt-in. With flattenDepth omitted (today's config) behaviour is byte-identical for placement, and the link-rewriter change is a strict improvement whose full output I verified against the committed corpus. Rollback = revert; no migration, no state.

---

Independence note: reviewed from story #407, the PR diff/description, and the branch code only, in a detached throwaway worktree at origin/chore/US-407-preserve-nested-references-subdir (724ea0d). .pair/working/ was not read. Findings 2, 5, 6 and the mirror-equality verification were reproduced by executing branch code, not inferred.

[rucka]

In reply to the first review.

Escalating to human — non-convergence, round 6

Re-review of head 109b18a6 (no new commit since round 5) surfaced 6 further findings, all Minor. This flush supersedes the round-3 escalate-flush comment (now marked outdated/minimized) and summarizes the full cycle to date.

Rounds so far (all in .pair/working/reviews/407.md)

• Round 1 — 13 findings (4 Major + 8 Minor + 1 Questions), all resolved, none escalated.
• Round 2 — 9 findings (1 Major + 8 Minor), all resolved, none escalated.
• Round 3 — 7 findings (1 Major + 5 Minor + 1 Questions), escalated to human (design question on the shape-inference guard). Subsequently fixed out of band.
• Round 4 — 11 findings (1 Major + 8 Minor + 2 Questions), all resolved, none escalated.
• Round 5 — 6 findings (5 Minor + 1 Questions), all resolved, none escalated.
• Round 6 (this one) — 6 further findings surfaced on re-review of the SAME head 109b18a6, all Minor. Escalating rather than auto-fixing per this run's instruction.

Still-open actionable findings (round 6)

1. [Minor] packages/knowledge-hub/src/tools/skill-md-mirror.ts:196-202 (installedArtifactPath docstring, paragraph 2) — still asserts the superseded UNBOUNDED semantic in the present tense with a worked example (… → pair-process-review-references/deep.md), directly contradicted by the very next paragraph's #407-bounded result for the same source path. Verified: transformPath(dir, SKILL_COPY_OPTS) with flattenDepth: 2 returns pair-process-review/references, not the flattened form. Missed by the round-4 grep sweep because the paragraph never says "tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407".
   → Recommendation: merge the two paragraphs, or mark the unbounded example as historical (e.g. "Before tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407 … Since tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407 the mapping is BOUNDED: …"). Docs only.

2. [Minor] apps/pair-cli/src/registry/resolver.ts:81-84 (normalizeRegistryConfig) — raw['flattenDepth'] != null && {...} drops an explicit JSON null before it reaches validateFlattenDepthField, so flattenDepth: null normalizes to "unbounded" and pair update silently reinstalls the tech-debt: pair update mis-installs a skill's nested references/ subdir (flattened out of the skill, both links broken) #407 sibling-dir defect for a registry meant to be bounded. 0/-1/1.5/'2' are all correctly rejected — null is the one hole.
   → Recommendation: raw['flattenDepth'] !== undefined && {...}; add a null test case to resolver.test.ts/validation.test.ts.

3. [Minor] PR [US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring #411 body → ## Documentation → "Generated KB index" bullet (line 137) — defers the .pair/llms.txt drift-detection gap without citing tech-debt: no gate covers .pair/llms.txt drift — the generated KB index goes stale silently #416, which was opened at 17:22 UTC (before the round-5 commit at 17:34 UTC) and describes this exact gap.
   → Recommendation: cite tech-debt: no gate covers .pair/llms.txt drift — the generated KB index goes stale silently #416 in the bullet.

4. [Minor] .pair/adoption/tech/adr/adr-020-bounded-flatten-depth-entry-granularity.md:70 (Trade-offs bold lead-in) — "Every entry must sit at the declared depth; both deviations abort the copy" is the last surviving copy of the absolute/overstated wording; round 5 reworded the two sibling docs (website bullet, nested-sub-documents.md) to the enforced (narrower) rule but left the ADR of record stating the false absolute form.
   → Recommendation: align with the two reworded copies, e.g. "A deviation from the declared depth aborts the copy once it becomes ambiguous."

5. [Minor] apps/website/content/docs/reference/configuration.mdx:174-181 (Validation → What Gets Validated) — doesn't list the new flattenDepth validation rule (positive integer; requires flatten: true) among the enumerated transform-config checks. Discoverability only — the rule is stated two sections up in Registry Fields — but the round-4 Major on this same page was about exactly this class of omission.
   → Recommendation: extend the existing transform bullet to mention flattenDepth.

6. [Minor] packages/content-ops/src/ops/copy/layout-validation.ts:157-163 (validateNoDeepEntry docstring, round-5 addition) — misattributes its audience as "a consumer of this package has no such gate", but @pair/content-ops is "private": true with no external consumers; the real reachable audience is a project whose own config.json declares a bounded registry, copied by the published pair CLI (apps/pair-cli).
   → Recommendation: reword to name the real audience.

None of these individually rise to a Major/design-level disagreement; escalating per this run's framing (non-convergence risk) rather than auto-fixing.

Convention for continuation

Any further rework or re-review on this story — including manual, out-of-band rounds — should be appended to the working log .pair/working/reviews/407.md, NOT posted as a new standalone PR comment. The next orchestrated run on this story continues the same cycle; its convergence will synthesize ONE final remediation comment and minimize this flush (and any other intermediate comments, excluding the first-review comment above).

Note: this working log is an UNTRACKED file living only in the persistent authoring worktree ../pair-worktrees/407. That worktree must be PRESERVED until merge — if it is pruned/recreated the audit trail of rounds 1-6 is lost (this flush and the first-review comment remain on the PR and still prevent a duplicate first review on the next run, but the per-finding detail would not be recoverable).

Not merged. Human decision requested on how to proceed (bundle-fix vs. continue escalating).

[rucka]

Remediation — round 4

Open findings: 0 of the six raised this round. Two of the round-3 carry-overs are also gone (see below); the rest are unchanged and were explicitly out of scope for this round.

Head: 7db5d1da. Gate: 22 + 12 tasks green.

#	Finding	What changed
1	installedArtifactPath docstring described the pre-#407 unbounded mapping in the present tense	The stale example is removed rather than re-tensed: the docstring now states the bounded behaviour only, so there is no sentence a reader can mistake for current.
2	flattenDepth: null was silently dropped instead of rejected	!== undefined (was != null), so an explicit null reaches validateFlattenDepthField and is rejected by name instead of degrading into the unbounded flatten this story removed. New test asserts the key survives normalization and the value stays null.
3	The KB-index bullet flagged a drift class without citing the issue	Cites #416 (tech-debt: no gate covers .pair/llms.txt drift), which exists and is open.
4	ADR-020's lead-in claimed every depth deviation aborts the copy	Reworded to match what the two guards actually do: a deviation aborts once the shape becomes ambiguous, and neither guard fires on depth alone — each needs its second condition, which the sub-bullets state. The duplicated trailing clause is gone.
5	flattenDepth missing from the docs' validation list	Added to the existing transform bullet: a positive integer, and only alongside flatten: true.
6	The docstring's "consumer of this package" named the wrong audience	Reworded: a project whose own config.json declares a bounded registry has no such gate, because skills:conformance is corpus-specific to this repository rather than part of the published package.

Round-3 carry-overs, re-checked rather than assumed: .pair/llms.txt does now carry the nested-sub-documents.md row, and ADR-020 no longer has an open Question about normalizing next into a category. The remaining three (the guard message's depth interpolation, adr-005's behavior wording, the DR-1 module-split suggestion) are untouched — they were listed as not-new for this round and none of them changes behaviour.