[#238] feat: fence-aware skill ref rewrite + idempotent flatten/prefix

.pair/adoption/decision-log/2026-07-08-test-file-colocation-multi-module.md

@@ -0,0 +1,40 @@
+# Decision: Test co-location for multi-module tests — root-of-call-chain, not a standalone file
+
+## Date
+
+2026-07-08
+
+## Status
+
+Active
+
+## Category
+
+Convention Adoption
+
+## Context
+
+The existing co-location rule (`file-structure.md`: "Always co-locate test files with implementation") only covers the 1:1 case — one module, one sibling test file. It has no guidance for a test that exercises more than one module together (e.g. an integration test driving two collaborating command handlers end-to-end). Found in #238: `apps/pair-cli/src/commands/update/idempotent-skill-registry.test.ts` tested both `update/handler.ts`'s `handleUpdateCommand` and `install/handler.ts`'s `handleInstallCommand` from a standalone file matching neither, alongside the two directories' own pre-existing `handler.test.ts` siblings.
+
+## Decision
+
+A test spanning multiple modules is placed in the test file already co-located with the *root* module of the call chain it verifies — the module whose exported entry point the test is actually asserting on, even if the test also drives other modules as setup or as part of the same flow. No standalone integration-test file is created for the combination.
+
+Applied to `idempotent-skill-registry.test.ts`: its `update`-focused cases (idempotency across repeated runs, prefix change, removed-skill reference) moved into `update/handler.test.ts` (root: `handleUpdateCommand`); its `install`-only cases (external KB via `--source`, name collision) moved into `install/handler.test.ts` (root: `handleInstallCommand`). The standalone file was deleted.
+
+This does not apply to end-to-end/page-level tests (named after the user flow or page they exercise, e.g. `landing.e2e.test.ts`) or content/asset-validation tests (named after the asset they validate, e.g. `agents-md.test.ts`, which has no single source module to co-locate against) — both are pre-existing, distinct categories this decision leaves untouched.
+
+## Alternatives Considered
+
+- **Keep standalone integration-test files, named after the scenario**: Rejected. Scales into a parallel "integration tests" file tree that duplicates or drifts from the sibling `handler.test.ts` files already covering the same root modules, and breaks the "one root module → one test file" discoverability the co-location rule exists for.
+- **One `integration.test.ts` per command directory**: Rejected. Adds a second file per directory with no clearer ownership than the existing `handler.test.ts` — the root-of-call-chain rule gives an unambiguous single destination without adding a new file-naming pattern.
+
+## Consequences
+
+- `file-structure.md`'s Co-location Rules section gains an explicit rule for the multi-module case, closing the gap this decision was prompted by.
+- Future integration-style tests in this codebase (and any project adopting this KB) have an unambiguous placement rule instead of ad hoc naming per author.
+
+## Adoption Impact
+
+- `packages/knowledge-hub/dataset/.pair/knowledge/guidelines/code-design/code-organization/file-structure.md` (+ root mirror `.pair/knowledge/guidelines/code-design/code-organization/file-structure.md`): Co-location Rules section extended with the multi-module case and its two exceptions.
+- `apps/pair-cli/src/commands/update/idempotent-skill-registry.test.ts`: deleted; its cases moved into `update/handler.test.ts` and `install/handler.test.ts`.

.pair/knowledge/guidelines/code-design/code-organization/file-structure.md

@@ -48,6 +48,10 @@ UserProfile.stories.tsx (if using Storybook)
 
 ```text
 
+**Tests spanning multiple modules**: when a test exercises more than one implementation file together (e.g. an integration test driving two collaborating functions end-to-end), it does not get its own standalone file — it goes in the test file already co-located with the *root* module of that call chain, the one whose exported entry point the test is actually verifying. A test that seeds its fixture through module B but asserts on module A's behavior belongs in `A.test.ts`, not a new `A-and-b-integration.test.ts`. This keeps the co-location rule intact (one root module → one test file) instead of a proliferation of ad hoc integration-test files that duplicate coverage or drift out of sync with the modules they actually exercise.
+
+This does not apply to two other, already-common test categories that are correctly named after what they validate rather than a source module: end-to-end/page-level tests (e.g. `landing.e2e.test.ts`, testing a user flow across many files by design) and content/asset-validation tests (e.g. asserting on a generated markdown file's content, where there is no single source module to co-locate against).
+
 **Types**: Co-locate types when feature-specific:
 ```
 

apps/pair-cli/src/commands/install/handler.test.ts

@@ -775,3 +775,102 @@ describe('install — Bug 5: skill ref rewrite with agents-before-skills order',
     expect(agentsContent).not.toMatch(/(?<![a-z-])\/implement(?![a-z-])/)
   })
 })
+
+describe('#238: flatten+prefix pipeline for external KB and collision detection', () => {
+  const moduleDir = '/project'
+  const datasetSrc = `${moduleDir}/packages/knowledge-hub/dataset`
+
+  test('AC5: external KB installed via local --source path applies the same flatten/prefix/rewrite pipeline', async () => {
+    const externalKbPath = '/external/kb'
+    const fs = new InMemoryFileSystemService(
+      {
+        [`${moduleDir}/package.json`]: JSON.stringify({
+          name: 'test-project',
+          version: '0.1.0',
+        }),
+        [`${moduleDir}/config.json`]: JSON.stringify({
+          asset_registries: {
+            skills: {
+              source: '.skills',
+              behavior: 'mirror',
+              flatten: true,
+              prefix: 'ext',
+              description: 'Agent skills',
+              targets: [{ path: '.claude/skills/', mode: 'canonical' }],
+            },
+            agents: {
+              source: 'AGENTS.md',
+              behavior: 'mirror',
+              description: 'AI agents guidance',
+              targets: [{ path: 'AGENTS.md', mode: 'canonical' }],
+            },
+          },
+        }),
+        // Minimal nested external KB — standard `.skills/<type>/<name>/` layout
+        [`${externalKbPath}/AGENTS.md`]: '# AGENTS\n\nRun /next for the external KB.\n',
+        [`${externalKbPath}/.skills/catalog/next/SKILL.md`]: '---\nname: next\n---\n# /next\n',
+      },
+      moduleDir,
+      moduleDir,
+    )
+
+    const installConfig: InstallCommandConfig = {
+      command: 'install',
+      resolution: 'local',
+      path: externalKbPath,
+      offline: true,
+      kb: true,
+    }
+    await handleInstallCommand(installConfig, fs)
+
+    // Flattened + prefixed exactly like the official dataset pipeline
+    await expect(
+      fs.exists(`${moduleDir}/.claude/skills/ext-catalog-next/SKILL.md`),
+    ).resolves.toBe(true)
+    const skillContent = await fs.readFile(`${moduleDir}/.claude/skills/ext-catalog-next/SKILL.md`)
+    expect(skillContent).toContain('name: ext-catalog-next')
+
+    // Cross-registry rewrite applied without any source-side restructuring
+    const agentsContent = await fs.readFile(`${moduleDir}/AGENTS.md`)
+    expect(agentsContent).toContain('/ext-catalog-next')
+  })
+
+  test('name collision after flattening fails install with an explicit error', async () => {
+    const fs = new InMemoryFileSystemService(
+      {
+        [`${moduleDir}/package.json`]: JSON.stringify({
+          name: 'test-project',
+          version: '0.1.0',
+        }),
+        [`${moduleDir}/packages/knowledge-hub/package.json`]: JSON.stringify({
+          name: '@pair/knowledge-hub',
+        }),
+        [`${moduleDir}/config.json`]: JSON.stringify({
+          asset_registries: {
+            skills: {
+              source: '.skills',
+              behavior: 'mirror',
+              flatten: true,
+              description: 'Agent skills',
+              targets: [{ path: '.claude/skills/', mode: 'canonical' }],
+            },
+          },
+        }),
+        // Two distinct source paths that flatten to the same target name
+        [`${datasetSrc}/.skills/a/b/SKILL.md`]: '# Skill 1',
+        [`${datasetSrc}/.skills/a-b/SKILL.md`]: '# Skill 2',
+      },
+      moduleDir,
+      moduleDir,
+    )
+
+    const installConfig: InstallCommandConfig = {
+      command: 'install',
+      resolution: 'default',
+      kb: true,
+      offline: false,
+    }
+
+    await expect(handleInstallCommand(installConfig, fs)).rejects.toThrow(/collision/i)
+  })
+})

apps/pair-cli/src/commands/install/handler.ts

@@ -14,7 +14,7 @@ import {
   doCopyAndUpdateLinks,
   buildCopyOptions,
   postCopyOps,
-  applySkillRefsToNonSkillRegistries,
+  reconcileSkillNameRegistry,
   resolveEffectiveDatasetRoot,
   writeProjectLlmsTxt,
   type RegistryConfig,
@@ -273,9 +273,8 @@ async function executeInstall(context: InstallContext): Promise<void> {
 
   const { results, skillNameMap } = await installAllRegistries(context)
 
-  if (skillNameMap.size > 0) {
-    await applySkillRefsToNonSkillRegistries({ fs, baseTarget, pushLog }, registries, skillNameMap)
-  }
+  await reconcileSkillNameRegistry({ fs, baseTarget, pushLog }, registries, skillNameMap)
+
   if (options?.linkStyle) {
     await applyLinkTransformation(fs, { linkStyle: options.linkStyle }, pushLog, 'install')
   }