From d81647f9c72282abd5e41978350fecafe013b9da Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 30 Jul 2026 15:58:43 -0400 Subject: [PATCH] Deliver knowledge a skill cites from another domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Knowledge is copied per domain, so a skill could only ever cite its own domain's files. Four skills cite `testing-layers.md`, which lives in `testing`: `coding-guidelines`, `pr-guidelines`, `pr-readiness-check`, and `perps-review-pr`. Six citation sites in total, none of which could resolve for any consumer on any operator — the skill installed fine and the reference dangled. The installer now scans a skill for `knowledge/.md` citations its own domain does not satisfy, and copies the file in from whichever domain owns it. Existing citations start working unchanged; no new syntax, and nothing declares a dependency in frontmatter, so the context an agent must load stays bounded. Both the base skill and the applicable repo overlay are scanned. The overlay is merged into the emitted body, and three of the four cases cite from an overlay rather than from `skill.md` — scanning only the base fixed one of four. Resolution is by filename, which is unambiguous today: ten knowledge files, no name shared across domains. If that stops being true the reference is genuinely ambiguous, so the install fails and names the candidates rather than picking one. Verified against the real corpus: 15/15 knowledge citations resolve after install, from 9/15. Both new tests fail against main's installer. --- test/cli.test.mjs | 88 ++++++++++++++++++++++++++++++++++++++++++++++- tools/install | 57 ++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/test/cli.test.mjs b/test/cli.test.mjs index e4829a10..d36aa14d 100644 --- a/test/cli.test.mjs +++ b/test/cli.test.mjs @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -240,3 +240,89 @@ describe('managed skill pruning', () => { assert.equal(existsSync(stale), true); }); }); + +describe('cross-domain knowledge references', () => { + let root; + let source; + let target; + + function seed(domain, skill, body, knowledgeFiles = {}) { + const dir = path.join(source, 'domains', domain, 'skills', skill); + mkdirSync(dir, { recursive: true }); + writeFileSync( + path.join(dir, 'skill.md'), + ['---', `name: ${skill}`, `description: ${skill}`, 'maturity: stable', '---', body].join('\n'), + ); + for (const [file, contents] of Object.entries(knowledgeFiles)) { + const kdir = path.join(source, 'domains', domain, 'knowledge'); + mkdirSync(kdir, { recursive: true }); + writeFileSync(path.join(kdir, file), contents); + } + return dir; + } + + before(() => { + root = mkdtempSync(path.join(os.tmpdir(), 'mms-xdomain-')); + source = path.join(root, 'source'); + target = path.join(root, 'target'); + mkdirSync(path.join(source, 'tools'), { recursive: true }); + symlinkSync(INSTALL, path.join(source, 'tools', 'install')); + mkdirSync(target, { recursive: true }); + + // `testing` owns the file; `coding` cites it. Knowledge is delivered per domain, so + // before this the citation could not resolve for any consumer on any operator. + seed('testing', 'unit-testing', 'Body.', { 'testing-layers.md': '# Layers\n' }); + seed('coding', 'guidelines', 'Read [layers](knowledge/testing-layers.md) first.'); + }); + + after(() => { + rmSync(root, { recursive: true, force: true }); + }); + + test('a skill receives knowledge it cites from another domain', () => { + const result = spawnSync( + 'bash', + [INSTALL, '--target', target, '--repo', 'core', '--source', source], + { encoding: 'utf8' }, + ); + assert.equal(result.status, 0, result.stderr); + for (const base of ['.claude/skills', '.cursor/rules', '.agents/skills']) { + assert.ok( + existsSync(path.join(target, base, 'mms-guidelines', 'knowledge', 'testing-layers.md')), + `${base}: cross-domain knowledge not delivered`, + ); + } + }); + + test('the owning domain still gets its own knowledge', () => { + assert.ok( + existsSync(path.join(target, '.claude/skills', 'mms-unit-testing', 'knowledge', 'testing-layers.md')), + ); + }); + + test('an ambiguous filename fails rather than picking one', () => { + const clash = mkdtempSync(path.join(os.tmpdir(), 'mms-clash-')); + const clashSource = path.join(clash, 'source'); + const clashTarget = path.join(clash, 'target'); + mkdirSync(path.join(clashSource, 'tools'), { recursive: true }); + symlinkSync(INSTALL, path.join(clashSource, 'tools', 'install')); + mkdirSync(clashTarget, { recursive: true }); + + const saved = source; + source = clashSource; + // Two domains ship the same filename; a third cites it by name alone. + seed('testing', 'a', 'Body.', { 'shared.md': '# one\n' }); + seed('perps', 'b', 'Body.', { 'shared.md': '# two\n' }); + seed('coding', 'c', 'Read [x](knowledge/shared.md).'); + source = saved; + + const result = spawnSync( + 'bash', + [INSTALL, '--target', clashTarget, '--repo', 'core', '--source', clashSource], + { encoding: 'utf8' }, + ); + assert.notEqual(result.status, 0, 'ambiguous reference should fail the install'); + assert.match(result.stderr, /exists in more than one domain/u); + rmSync(clash, { recursive: true, force: true }); + }); +}); diff --git a/tools/install b/tools/install index e39404aa..72e1585c 100755 --- a/tools/install +++ b/tools/install @@ -364,10 +364,41 @@ copy_bundle_dirs() { done } +# Resolve a knowledge filename that the skill's own domain does not provide. +# +# Knowledge is delivered per domain, so a skill could only ever cite its own domain's +# files. Several legitimately cite another domain's — four skills across `coding`, +# `perps`, and `pr-workflow` reference `testing-layers.md`, which lives in `testing` — +# and those references could not resolve for any consumer on any operator. +# +# Filenames are unique across domains today. If that stops being true the reference is +# genuinely ambiguous, so this fails loudly rather than picking one. +resolve_foreign_knowledge() { + local domains_root="$1" want="$2" self_domain="$3" + local found=() candidate + + while IFS= read -r candidate; do + [[ "$candidate" == "$domains_root/$self_domain/knowledge/$want" ]] && continue + found+=("$candidate") + done < <(find "$domains_root" -path "*/knowledge/$want" -type f 2>/dev/null | sort) + + if (( ${#found[@]} > 1 )); then + echo "Error: knowledge file '$want' exists in more than one domain:" >&2 + printf ' %s\n' "${found[@]}" >&2 + echo "Cross-domain references resolve by filename, so this is ambiguous. Rename one." >&2 + return 1 + fi + + (( ${#found[@]} == 1 )) && printf '%s' "${found[0]}" + return 0 +} + copy_domain_knowledge() { local skill_dir="$1" dest_dir="$2" label="$3" local domain_dir; domain_dir=$(cd "$skill_dir/../.." && pwd) local knowledge_dir="$domain_dir/knowledge" + local domains_root; domains_root=$(cd "$domain_dir/.." && pwd) + local self_domain; self_domain=$(basename "$domain_dir") if [[ -d "$knowledge_dir" ]]; then action "$label/knowledge/" @@ -382,6 +413,32 @@ copy_domain_knowledge() { fi rm -rf "$dest_dir/knowledge" fi + + # Deliver knowledge cited from another domain, so the reference resolves where it is read. + # Scan the base skill AND the applicable repo overlay: the overlay is merged into the + # emitted body, so a citation there is just as load-bearing, and three of the four + # real cases live in an overlay rather than in skill.md. + local sources=("$skill_dir/skill.md") + [[ -f "$skill_dir/repos/${REPO}.md" ]] && sources+=("$skill_dir/repos/${REPO}.md") + local existing=() + local candidate_src + for candidate_src in "${sources[@]}"; do + [[ -f "$candidate_src" ]] && existing+=("$candidate_src") + done + (( ${#existing[@]} > 0 )) || return 0 + local cited source_path + while IFS= read -r cited; do + [[ -n "$cited" ]] || continue + [[ -f "$dest_dir/knowledge/$cited" ]] && continue + source_path=$(resolve_foreign_knowledge "$domains_root" "$cited" "$self_domain") || return 1 + [[ -n "$source_path" ]] || continue + if $DRY_RUN; then + action "$label/knowledge/$cited (cross-domain)" + continue + fi + mkdir -p "$dest_dir/knowledge" + cp "$source_path" "$dest_dir/knowledge/$cited" + done < <(grep -ohE 'knowledge/[A-Za-z0-9._-]+\.md' "${existing[@]}" 2>/dev/null | sed 's|^knowledge/||' | sort -u) } copy_project_bundles() {