From 01c4aae5c730cb8f21770001c5900ae29f085856 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 28 Jun 2026 06:29:08 -0700 Subject: [PATCH 1/3] agents|refactor: Move the harness overlay loader to a shared module The overlay loader that supplies each harness's tool-name mapping and frontmatter defaults now lives in its own module, shared by the skill and subagent deploy paths. --- packages/agents/src/commands/install.ts | 5 +-- packages/agents/src/commands/sync.ts | 4 +-- .../src/lib/__tests__/harness-overlay.test.ts | 33 +++++++++++++++++++ .../lib/__tests__/subagent-transform.test.ts | 32 ++---------------- packages/agents/src/lib/harness-overlay.ts | 22 +++++++++++++ packages/agents/src/lib/subagent-transform.ts | 21 ------------ 6 files changed, 62 insertions(+), 55 deletions(-) create mode 100644 packages/agents/src/lib/__tests__/harness-overlay.test.ts create mode 100644 packages/agents/src/lib/harness-overlay.ts diff --git a/packages/agents/src/commands/install.ts b/packages/agents/src/commands/install.ts index 292f572d..abe838c6 100644 --- a/packages/agents/src/commands/install.ts +++ b/packages/agents/src/commands/install.ts @@ -6,6 +6,7 @@ import { readDeploy } from '../lib/deploy-frontmatter.ts'; import { expandIncludes } from '../lib/directive-expander.ts'; import { pruneOrphanedEntries } from '../lib/entry-remover.ts'; import { HARNESSES, resolveHarnessIds, resolveHarnessPaths } from '../lib/harness.js'; +import { loadHarnessOverlay } from '../lib/harness-overlay.ts'; import { checkSymlinkSafety, copyItem, linkItem, removeItem, unlinkIfSymlink } from '../lib/installer.ts'; import { computeContentHash, @@ -23,7 +24,7 @@ import { } from '../lib/marker-injector.js'; import { rewritePathsInDirectory, rewritePathsInFile } from '../lib/path-rewriter.js'; import { renderPromptsYml } from '../lib/prompts-yml.ts'; -import { loadSubagentOverlay, renderSubagentForHarness } from '../lib/subagent-transform.ts'; +import { renderSubagentForHarness } from '../lib/subagent-transform.ts'; import { loadToolMapping, rewriteToolNames } from '../lib/tool-name-rewriter.js'; import { isEnoent, isMissingFile } from '../lib/type-guards.ts'; import type { @@ -87,7 +88,7 @@ export async function installCommand( // Load the harness overlay once per harness. The raw YAML feeds the frontmatter merger (subagents only); // the parsed `_tools:` mapping feeds the body-text placeholder rewriter (subagents and skills). const harnessConfig = HARNESSES[harnessId]; - const overlayYaml = await loadSubagentOverlay(contentDir, harnessConfig); + const overlayYaml = await loadHarnessOverlay(contentDir, harnessConfig); const toolMapping = loadToolMapping(overlayYaml); // Install skills (shared + harness-specific) diff --git a/packages/agents/src/commands/sync.ts b/packages/agents/src/commands/sync.ts index 74012229..628db15f 100644 --- a/packages/agents/src/commands/sync.ts +++ b/packages/agents/src/commands/sync.ts @@ -10,6 +10,7 @@ import { resolveContentDir } from '../lib/content-resolver.ts'; import { resolveClosure } from '../lib/dependency-resolver.ts'; import { readFileOrEmpty, writeIfChanged } from '../lib/fs-helpers.ts'; import { HARNESSES, resolveHarnessIds, resolveHarnessPaths } from '../lib/harness.ts'; +import { loadHarnessOverlay } from '../lib/harness-overlay.ts'; import { renderPromptsYml } from '../lib/prompts-yml.ts'; import { parseRulebookFile } from '../lib/rulebook-schema.ts'; import { extractRulebookSkillSlug, renderSkillFile, resolveSkillName } from '../lib/rulebook-skill.ts'; @@ -21,7 +22,6 @@ import { type ResolvedSubagent, type SubagentDeployContext, } from '../lib/subagent-deploy.ts'; -import { loadSubagentOverlay } from '../lib/subagent-transform.ts'; import { loadToolMapping } from '../lib/tool-name-rewriter.ts'; import { isEnoent, isMissingFile } from '../lib/type-guards.ts'; import type { HarnessId, InstallOptions } from '../lib/types.ts'; @@ -706,7 +706,7 @@ async function resolveSubagentTarget( contentDir: string, ): Promise { const harnessConfig = HARNESSES[harnessId]; - const overlayYaml = await loadSubagentOverlay(contentDir, harnessConfig); + const overlayYaml = await loadHarnessOverlay(contentDir, harnessConfig); return { subagentsDir: resolveHarnessPaths(harnessId, projectRoot).subagentsDir, deployContext: { diff --git a/packages/agents/src/lib/__tests__/harness-overlay.test.ts b/packages/agents/src/lib/__tests__/harness-overlay.test.ts new file mode 100644 index 00000000..76e0c93c --- /dev/null +++ b/packages/agents/src/lib/__tests__/harness-overlay.test.ts @@ -0,0 +1,33 @@ +import { mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { HARNESSES } from '../harness.ts'; +import { loadHarnessOverlay } from '../harness-overlay.ts'; + +const OVERLAY = '_tools:\n Read: Read\n'; + +describe(loadHarnessOverlay, () => { + let contentDir: string; + + beforeEach(async () => { + contentDir = path.join(tmpdir(), `agents-test-overlay-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await mkdir(path.join(contentDir, 'subagents', '_data'), { recursive: true }); + }); + + afterEach(async () => { + await rm(contentDir, { recursive: true, force: true }); + }); + + it('reads the harness overlay file from subagents/_data', async () => { + await writeFile(path.join(contentDir, 'subagents', '_data', 'claude.yaml'), OVERLAY, 'utf8'); + + expect(await loadHarnessOverlay(contentDir, HARNESSES.claude)).toBe(OVERLAY); + }); + + it('returns an empty string when the overlay file is absent', async () => { + expect(await loadHarnessOverlay(contentDir, HARNESSES.rovodev)).toBe(''); + }); +}); diff --git a/packages/agents/src/lib/__tests__/subagent-transform.test.ts b/packages/agents/src/lib/__tests__/subagent-transform.test.ts index ab94942f..207ff7aa 100644 --- a/packages/agents/src/lib/__tests__/subagent-transform.test.ts +++ b/packages/agents/src/lib/__tests__/subagent-transform.test.ts @@ -1,14 +1,9 @@ -import { mkdir, rm, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import path from 'node:path'; - import { unindent } from '@williamthorsen/toolbelt.strings/candidate'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { mergeFrontmatter } from '../frontmatter-merger.ts'; -import { HARNESSES } from '../harness.ts'; import { rewriteMarkdownPaths, rewriteTemplateVariables } from '../path-rewriter.ts'; -import { loadSubagentOverlay, renderSubagentForHarness } from '../subagent-transform.ts'; +import { renderSubagentForHarness } from '../subagent-transform.ts'; import { loadToolMapping, rewriteToolNames, ToolNameRewriteError } from '../tool-name-rewriter.ts'; const SOURCE = unindent` @@ -42,29 +37,6 @@ const ROVODEV_OVERLAY = unindent` `; -describe(loadSubagentOverlay, () => { - let contentDir: string; - - beforeEach(async () => { - contentDir = path.join(tmpdir(), `agents-test-overlay-${Date.now()}-${Math.random().toString(36).slice(2)}`); - await mkdir(path.join(contentDir, 'subagents', '_data'), { recursive: true }); - }); - - afterEach(async () => { - await rm(contentDir, { recursive: true, force: true }); - }); - - it('reads the harness overlay file from subagents/_data', async () => { - await writeFile(path.join(contentDir, 'subagents', '_data', 'claude.yaml'), CLAUDE_OVERLAY, 'utf8'); - - expect(await loadSubagentOverlay(contentDir, HARNESSES.claude)).toBe(CLAUDE_OVERLAY); - }); - - it('returns an empty string when the overlay file is absent', async () => { - expect(await loadSubagentOverlay(contentDir, HARNESSES.rovodev)).toBe(''); - }); -}); - describe(renderSubagentForHarness, () => { it('merges _defaults, rewrites the tool placeholder, and expands {harness_home_dir} for claude', () => { const output = renderSubagentForHarness(SOURCE, { diff --git a/packages/agents/src/lib/harness-overlay.ts b/packages/agents/src/lib/harness-overlay.ts new file mode 100644 index 00000000..7fc189f4 --- /dev/null +++ b/packages/agents/src/lib/harness-overlay.ts @@ -0,0 +1,22 @@ +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; + +import { isEnoent } from './type-guards.ts'; +import type { HarnessConfig } from './types.ts'; + +/** + * Reads a harness's overlay YAML, returning an empty string when the file is absent. The overlay lives under + * `subagents/_data/` but is harness-level: it carries both the subagent frontmatter `_defaults`/per-agent merge and the + * shared `{tool:NAME}` mapping, so the skill and subagent deploy paths load it alike. + */ +export async function loadHarnessOverlay(contentDir: string, harnessConfig: HarnessConfig): Promise { + const overlayPath = path.join(contentDir, 'subagents', '_data', harnessConfig.frontmatterFile); + try { + return await readFile(overlayPath, 'utf8'); + } catch (error: unknown) { + if (!isEnoent(error)) { + throw error; + } + return ''; + } +} diff --git a/packages/agents/src/lib/subagent-transform.ts b/packages/agents/src/lib/subagent-transform.ts index f5570da8..0adb019e 100644 --- a/packages/agents/src/lib/subagent-transform.ts +++ b/packages/agents/src/lib/subagent-transform.ts @@ -1,27 +1,6 @@ -import { readFile } from 'node:fs/promises'; -import path from 'node:path'; - import { mergeFrontmatter } from './frontmatter-merger.ts'; import { rewriteMarkdownPaths, rewriteTemplateVariables } from './path-rewriter.ts'; import { rewriteToolNames } from './tool-name-rewriter.ts'; -import { isEnoent } from './type-guards.ts'; -import type { HarnessConfig } from './types.ts'; - -/** - * Reads the subagent overlay YAML for a harness, returning an empty string when the file does not exist. - * Shared by `install` and `sync` so both apply the same `_defaults`/per-agent merge and `{tool:NAME}` mapping. - */ -export async function loadSubagentOverlay(contentDir: string, harnessConfig: HarnessConfig): Promise { - const overlayPath = path.join(contentDir, 'subagents', '_data', harnessConfig.frontmatterFile); - try { - return await readFile(overlayPath, 'utf8'); - } catch (error: unknown) { - if (!isEnoent(error)) { - throw error; - } - return ''; - } -} /** The harness-specific inputs a subagent render depends on, resolved once per harness by the caller. */ export interface SubagentRenderContext { From a68190c60b11c9435c346259c6d6d5c4a65c6524 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 28 Jun 2026 06:37:39 -0700 Subject: [PATCH 2/3] agents|refactor: Extract the skill transform into a shared module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill-directory transform — include expansion, tool-name rewriting, and relative-link rewriting — moves into one shared module. Installation renders declared skills through it, so the same transform can be reused by other deploy paths instead of being reachable only from the install path. --- packages/agents/src/commands/install.ts | 120 ++++-------------- .../src/lib/__tests__/skill-transform.test.ts | 114 +++++++++++++++++ packages/agents/src/lib/skill-transform.ts | 92 ++++++++++++++ 3 files changed, 230 insertions(+), 96 deletions(-) create mode 100644 packages/agents/src/lib/__tests__/skill-transform.test.ts create mode 100644 packages/agents/src/lib/skill-transform.ts diff --git a/packages/agents/src/commands/install.ts b/packages/agents/src/commands/install.ts index abe838c6..c2495700 100644 --- a/packages/agents/src/commands/install.ts +++ b/packages/agents/src/commands/install.ts @@ -22,8 +22,9 @@ import { injectMarkersInDirectory, injectProvenanceMarker, } from '../lib/marker-injector.js'; -import { rewritePathsInDirectory, rewritePathsInFile } from '../lib/path-rewriter.js'; +import { rewritePathsInFile } from '../lib/path-rewriter.js'; import { renderPromptsYml } from '../lib/prompts-yml.ts'; +import { type RenderedSkillEntry, renderSkillDirectory } from '../lib/skill-transform.ts'; import { renderSubagentForHarness } from '../lib/subagent-transform.ts'; import { loadToolMapping, rewriteToolNames } from '../lib/tool-name-rewriter.js'; import { isEnoent, isMissingFile } from '../lib/type-guards.ts'; @@ -307,18 +308,21 @@ async function installSkillEntry( toolMapping: ReadonlyMap, label = '', ): Promise { - // Eagerly resolves include directives at source-tree level. Run before the dry-run gate so missing targets, cycles, - // and out-of-tree references surface even when no files are written. - // Directory entries traverse their tree and cache each expanded `.md` file's content; - // file entries expand the file directly. - // After expansion, applies the tool-name rewriter in-memory so the cached content carries the final body text the - // write phase will emit — no second disk pass, no read-back-from-disk. + // Eagerly render the skill's final body before the dry-run gate, so missing include targets, cycles, out-of-tree + // references, and unmapped tool placeholders surface even when no files are written. Directory entries render the + // whole tree (include expansion, tool-name + link/template rewriting) into the exact bytes the write phase emits; + // single-file `.md` entries expand and tool-rewrite directly. const srcStats = await stat(srcPath); let expandedFileContent: string | undefined; - let expandedDirContents: ReadonlyMap | undefined; + let renderedDir: ReadonlyArray | undefined; if (srcStats.isDirectory()) { - const rawExpanded = await preExpandSkillDirectory(srcPath, contentDir); - expandedDirContents = rewriteToolNamesInExpansionMap(rawExpanded, contentDir, toolMapping); + renderedDir = await renderSkillDirectory(srcPath, path.basename(destPath), { + contentDir, + toolMapping, + pathPrefix: skillsPrefix, + homeDir, + harnessId, + }); } else if (srcPath.endsWith('.md')) { const expanded = await expandIncludes(srcPath, contentDir); expandedFileContent = rewriteToolNames(expanded, toolMapping, relativeFromContent(contentDir, srcPath)); @@ -340,12 +344,9 @@ async function installSkillEntry( } if (srcStats.isDirectory()) { - // Per-file walk: Writes expanded `.md` files from the cache populated during the pre-expand pass, - // mirrors the directory structure to the destination, and copies non-`.md` files plainly. - // The `_partials/` exclusion is applied during the walk. - // The cache is non-undefined here because srcStats.isDirectory() implies the directory branch above ran. - if (expandedDirContents === undefined) { - throw new Error(`Invariant violation: expandedDirContents undefined for directory ${srcPath}`); + // The rendered tree is non-undefined here because srcStats.isDirectory() implies the directory branch above ran. + if (renderedDir === undefined) { + throw new Error(`Invariant violation: renderedDir undefined for directory ${srcPath}`); } // Clean-write directories CodeAssembly previously installed: remove the prior copy so files deleted from the // source skill don't survive in the destination. Gated on prior ownership (a manifest entry exists) so a @@ -353,9 +354,7 @@ async function installSkillEntry( if (existingEntry) { await removeItem(destPath); } - await writeExpandedSkillDir(srcPath, destPath, expandedDirContents); - const skillsDestDir = path.dirname(destPath); - await rewritePathsInDirectory(destPath, skillsDestDir, skillsPrefix, homeDir, harnessId); + await writeRenderedSkillDir(destPath, renderedDir); await injectMarkersInDirectory(destPath, (fileRelPath) => buildSourceUrl(`${sourceRelativeRoot}/${fileRelPath}`)); } else if (srcPath.endsWith('.md') && expandedFileContent !== undefined) { // Single-file `.md` skill entries: write the previously expanded content directly. @@ -377,67 +376,14 @@ async function installSkillEntry( } /** - * Eagerly walks a skill source directory, runs `expandIncludes` on each `.md` file to surface include errors before - * any file is written, and returns a map keyed by absolute source path with the expanded content. - * `_partials/` directories are skipped because their contents are referenced through includes, not installed. - * The returned map is consumed by `writeExpandedSkillDir` so each `.md` file is expanded once per install. - */ -async function preExpandSkillDirectory(srcDir: string, contentDir: string): Promise> { - const expandedBySrcPath = new Map(); - await collectExpansions(srcDir, contentDir, expandedBySrcPath); - return expandedBySrcPath; -} - -async function collectExpansions( - srcDir: string, - contentDir: string, - expandedBySrcPath: Map, -): Promise { - const entries = await readdir(srcDir); - for (const entry of entries) { - if (entry === '_partials' || entry.startsWith('.')) { - continue; - } - const fullPath = path.join(srcDir, entry); - const info = await stat(fullPath); - if (info.isDirectory()) { - await collectExpansions(fullPath, contentDir, expandedBySrcPath); - } else if (entry.endsWith('.md')) { - expandedBySrcPath.set(fullPath, await expandIncludes(fullPath, contentDir)); - } - } -} - -/** - * Recursively writes a skill source directory to the destination. `.md` files are read from the pre-computed expansion - * cache (populated by `preExpandSkillDirectory`); non-`.md` files are copied verbatim. - * `_partials/` subdirectories are skipped at any depth — their contents are include targets, not installed artifacts. + * Writes a rendered skill directory to `destDir`: markdown entries are written from their transformed content, asset + * entries are copied verbatim from source. Each entry's parent directory is created as needed. */ -async function writeExpandedSkillDir( - srcDir: string, - destDir: string, - expandedBySrcPath: ReadonlyMap, -): Promise { - await mkdir(destDir, { recursive: true }); - const entries = await readdir(srcDir); +async function writeRenderedSkillDir(destDir: string, entries: ReadonlyArray): Promise { for (const entry of entries) { - if (entry === '_partials' || entry.startsWith('.')) { - continue; - } - const srcPath = path.join(srcDir, entry); - const destPath = path.join(destDir, entry); - const info = await stat(srcPath); - if (info.isDirectory()) { - await writeExpandedSkillDir(srcPath, destPath, expandedBySrcPath); - } else if (entry.endsWith('.md')) { - const expanded = expandedBySrcPath.get(srcPath); - if (expanded === undefined) { - throw new Error(`Invariant violation: pre-expand cache missing entry for ${srcPath}`); - } - await writeFile(destPath, expanded, 'utf8'); - } else { - await copyItem(srcPath, destPath); - } + const destPath = path.join(destDir, entry.relPath); + await mkdir(path.dirname(destPath), { recursive: true }); + await (entry.kind === 'markdown' ? writeFile(destPath, entry.content, 'utf8') : copyItem(entry.srcPath, destPath)); } } @@ -859,21 +805,3 @@ async function installHarnessGuidance( function relativeFromContent(contentDir: string, srcPath: string): string { return path.relative(contentDir, srcPath).split(path.sep).join('/'); } - -/** - * Returns a new map mirroring `rawExpanded`, with every value processed through the tool-name rewriter. - * The map is preserved by-reference (same keys, same iteration order); only the string values change. Each entry's - * `contextLabel` is its content-relative POSIX path, so an unmapped placeholder surfaces a usable file reference. - */ -function rewriteToolNamesInExpansionMap( - rawExpanded: ReadonlyMap, - contentDir: string, - toolMapping: ReadonlyMap, -): Map { - const rewritten = new Map(); - for (const [absSrcPath, content] of rawExpanded) { - const label = relativeFromContent(contentDir, absSrcPath); - rewritten.set(absSrcPath, rewriteToolNames(content, toolMapping, label)); - } - return rewritten; -} diff --git a/packages/agents/src/lib/__tests__/skill-transform.test.ts b/packages/agents/src/lib/__tests__/skill-transform.test.ts new file mode 100644 index 00000000..c04dd0e0 --- /dev/null +++ b/packages/agents/src/lib/__tests__/skill-transform.test.ts @@ -0,0 +1,114 @@ +import { mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { DirectiveExpansionError } from '../directive-expander.ts'; +import { type RenderedSkillEntry, renderSkillDirectory, type SkillDeployContext } from '../skill-transform.ts'; +import { ToolNameRewriteError } from '../tool-name-rewriter.ts'; + +const TOOL_MAPPING = new Map([['Read', 'open_files']]); + +describe(renderSkillDirectory, () => { + let contentDir: string; + let skillDir: string; + + beforeEach(async () => { + const stamp = `${Date.now()}-${Math.random().toString(36).slice(2)}`; + contentDir = path.join(tmpdir(), `agents-test-st-${stamp}`); + skillDir = path.join(contentDir, 'skills', 'demo'); + await mkdir(skillDir, { recursive: true }); + }); + + afterEach(async () => { + await rm(contentDir, { recursive: true, force: true }); + }); + + it('expands includes, rewrites tool placeholders, and expands template variables in SKILL.md', async () => { + await writeSkill({ + 'SKILL.md': + '# Demo\n\n\n\nUse the {tool:Read} tool at `{harness_home_dir}/x`.\n', + '_partials/frag.md': 'Shared fragment.\n', + }); + + const entries = await renderSkillDirectory(skillDir, 'demo', context()); + + const content = markdownContent(entries, 'SKILL.md'); + expect(content).toContain('Shared fragment.'); + expect(content).toContain('Use the open_files tool'); + expect(content).toContain('~/.claude/x'); + expect(content).not.toContain('{tool:Read}'); + expect(content).not.toContain('include:'); + // The partial is an include target, never a deployed entry. + expect(entries.some((entry) => entry.relPath.startsWith('_partials'))).toBe(false); + }); + + it('rewrites a bare-relative link in a nested .md against the skill slug and prefix', async () => { + await writeSkill({ 'SKILL.md': '# Demo\n', 'reference/guide.md': 'See [the data](../data/table.csv).\n' }); + + const entries = await renderSkillDirectory(skillDir, 'demo', context()); + + expect(markdownContent(entries, 'reference/guide.md')).toContain( + '[the data](~/.claude/skills/demo/data/table.csv)', + ); + }); + + it('returns non-.md files as assets pointing at the source path', async () => { + await writeSkill({ 'SKILL.md': '# Demo\n', 'data/table.csv': 'a,b\n1,2\n' }); + + const entries = await renderSkillDirectory(skillDir, 'demo', context()); + + expect(entries.find((entry) => entry.relPath === 'data/table.csv')).toEqual({ + kind: 'asset', + relPath: 'data/table.csv', + srcPath: path.join(skillDir, 'data', 'table.csv'), + }); + }); + + it('throws a file/line-anchored error for an unmapped tool placeholder', async () => { + await writeSkill({ 'SKILL.md': '# Demo\n\nUse {tool:Bash}.\n' }); + + await expect(renderSkillDirectory(skillDir, 'demo', context())).rejects.toThrow(ToolNameRewriteError); + await expect(renderSkillDirectory(skillDir, 'demo', context())).rejects.toThrow(/skills\/demo\/SKILL\.md:3/); + }); + + it('throws on a broken include directive', async () => { + await writeSkill({ 'SKILL.md': '# Demo\n\n\n' }); + + await expect(renderSkillDirectory(skillDir, 'demo', context())).rejects.toThrow(DirectiveExpansionError); + }); + + // region | Helpers + + function context(overrides: Partial = {}): SkillDeployContext { + return { + contentDir, + toolMapping: TOOL_MAPPING, + pathPrefix: '.claude/skills', + homeDir: '.claude', + harnessId: 'claude', + ...overrides, + }; + } + + /** Writes files into the demo skill directory from a relative-path → content map. */ + async function writeSkill(files: Record): Promise { + for (const [rel, content] of Object.entries(files)) { + const full = path.join(skillDir, rel); + await mkdir(path.dirname(full), { recursive: true }); + await writeFile(full, content, 'utf8'); + } + } + + /** Returns the transformed content of the markdown entry at relPath, failing if it is absent or an asset. */ + function markdownContent(entries: ReadonlyArray, relPath: string): string { + const entry = entries.find((candidate) => candidate.relPath === relPath); + if (entry?.kind !== 'markdown') { + throw new Error(`Expected a markdown entry at ${relPath}, got ${entry?.kind ?? 'nothing'}`); + } + return entry.content; + } + + // endregion | Helpers +}); diff --git a/packages/agents/src/lib/skill-transform.ts b/packages/agents/src/lib/skill-transform.ts new file mode 100644 index 00000000..f49f85c4 --- /dev/null +++ b/packages/agents/src/lib/skill-transform.ts @@ -0,0 +1,92 @@ +import { readdir } from 'node:fs/promises'; +import path from 'node:path'; + +import { expandIncludes } from './directive-expander.ts'; +import { rewriteMarkdownPaths, rewriteTemplateVariables } from './path-rewriter.ts'; +import { rewriteToolNames } from './tool-name-rewriter.ts'; + +/** The per-harness inputs a declared-skill render depends on, resolved once per harness by the caller. */ +export interface SkillDeployContext { + /** Library content root, used to resolve include directives and to label unmapped-tool errors. */ + readonly contentDir: string; + /** Canonical → harness tool-name mapping for the `{tool:NAME}` body-text rewriter. */ + readonly toolMapping: ReadonlyMap; + /** Harness-relative prefix under which `~/`-prefixed Markdown link targets are built (e.g. `.claude/skills`). */ + readonly pathPrefix: string; + /** Harness home segment that `{harness_home_dir}` tokens expand to (e.g. `.claude`). */ + readonly homeDir: string; + /** Harness identifier that `{harness_id}` tokens expand to (e.g. `claude`). */ + readonly harnessId: string; +} + +/** + * One file of a rendered skill directory, keyed by its POSIX path relative to the skill's destination root. A `markdown` + * entry carries its fully transformed text; an `asset` entry carries its source path for the caller to copy verbatim. + */ +export type RenderedSkillEntry = + | { readonly kind: 'markdown'; readonly relPath: string; readonly content: string } + | { readonly kind: 'asset'; readonly relPath: string; readonly srcPath: string }; + +/** + * Renders a declared skill's directory for one harness: every `.md` file is include-expanded, tool-name-rewritten, and + * link/template-rewritten in install's order; non-`.md` files are returned as assets to copy verbatim. Pure read — no + * writes and no ownership/provenance marking — so `install` and `sync` each compose their own write strategy and markers + * around the single shared transform. Throws (with a file:line anchor) on a broken include or an unmapped `{tool:NAME}` + * placeholder. + * + * `slug` anchors link rewriting: a relative Markdown link resolves against `/` under `pathPrefix`, matching + * how the deployed skill sits at `//`. `_partials/` directories and dotfiles are skipped at every + * depth — partials are include targets, never deployed artifacts. + */ +export async function renderSkillDirectory( + srcDir: string, + slug: string, + context: SkillDeployContext, +): Promise> { + const entries: Array = []; + await collectEntries(srcDir, '', slug, context, entries); + return entries; +} + +// region | Helpers + +/** Recursively walks `dir`, skipping `_partials/` and dotfiles, accumulating rendered entries keyed by relative path. */ +async function collectEntries( + dir: string, + relDir: string, + slug: string, + context: SkillDeployContext, + out: Array, +): Promise { + for (const entry of await readdir(dir, { withFileTypes: true })) { + if (entry.name === '_partials' || entry.name.startsWith('.')) { + continue; + } + const srcPath = path.join(dir, entry.name); + const relPath = relDir === '' ? entry.name : `${relDir}/${entry.name}`; + if (entry.isDirectory()) { + await collectEntries(srcPath, relPath, slug, context, out); + } else if (entry.name.endsWith('.md')) { + out.push({ kind: 'markdown', relPath, content: await renderMarkdown(srcPath, relPath, slug, context) }); + } else { + out.push({ kind: 'asset', relPath, srcPath }); + } + } +} + +/** Applies the skill `.md` transform chain: include expansion, tool-name rewrite, link rewrite, template expansion. */ +async function renderMarkdown( + srcPath: string, + relPath: string, + slug: string, + context: SkillDeployContext, +): Promise { + const { contentDir, toolMapping, pathPrefix, homeDir, harnessId } = context; + const contextLabel = path.relative(contentDir, srcPath).split(path.sep).join('/'); + const expanded = await expandIncludes(srcPath, contentDir); + const toolRewritten = rewriteToolNames(expanded, toolMapping, contextLabel); + const pathRewritten = rewriteMarkdownPaths(toolRewritten, `${slug}/${relPath}`, pathPrefix); + return rewriteTemplateVariables(pathRewritten, homeDir, harnessId); +} + +// endregion | Helpers From 548dfeee533a21f9dfc608f28e74e6d9a90b436b Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 28 Jun 2026 06:48:30 -0700 Subject: [PATCH 3/3] agents|feat: Apply skill transforms when sync deploys declared skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declared skills deployed by `sync` now have their `` directives expanded, their bare-relative Markdown links rewritten to absolute targets, and their `{tool:…}` placeholders rewritten to each target harness's tool names — the same transforms `install` applies. A skill that uses these constructs deploys identically whether delivered by `sync` or `install`. A broken include or an unmapped placeholder now fails the `sync` run, `--dry-run` included, before any file is written, rather than deploying a broken skill. --- .../src/commands/__tests__/sync.test.ts | 39 +++++++ packages/agents/src/commands/sync.ts | 94 ++++++++++++++-- .../src/lib/__tests__/skill-deploy.test.ts | 104 ++++++++++++++---- packages/agents/src/lib/harness-overlay.ts | 6 +- packages/agents/src/lib/skill-deploy.ts | 71 ++++++------ packages/agents/src/lib/skill-transform.ts | 9 +- 6 files changed, 246 insertions(+), 77 deletions(-) diff --git a/packages/agents/src/commands/__tests__/sync.test.ts b/packages/agents/src/commands/__tests__/sync.test.ts index 41d8410d..e6c64d21 100644 --- a/packages/agents/src/commands/__tests__/sync.test.ts +++ b/packages/agents/src/commands/__tests__/sync.test.ts @@ -535,6 +535,45 @@ describe(syncCommand, () => { expect(existsSync(path.dirname(skillPath('people-report')))).toBe(false); }); + + it('applies include expansion and tool-name and link rewriting when deploying a declared skill', async () => { + await mkdir(path.join(contentDir, 'subagents', '_data'), { recursive: true }); + await writeFile( + path.join(contentDir, 'subagents', '_data', 'claude.yaml'), + '_tools:\n Read: open_files\n', + 'utf8', + ); + const skillDir = path.join(contentDir, 'skills', 'demo'); + await mkdir(path.join(skillDir, '_partials'), { recursive: true }); + await writeFile( + path.join(skillDir, 'SKILL.md'), + '---\nname: demo\ndeploy: declared\n---\n\n\n\nUse {tool:Read}. See [guide](./guide.md).\n', + 'utf8', + ); + await writeFile(path.join(skillDir, '_partials', 'frag.md'), 'Shared fragment.\n', 'utf8'); + await writeFile(path.join(skillDir, 'guide.md'), '# Guide\n', 'utf8'); + await declareSkills('demo'); + + await syncCommand(makeOptions(), projectRoot, contentDir); + + const skill = await readFile(skillPath('demo'), 'utf8'); + expect(skill).toContain('Shared fragment.'); + expect(skill).toContain('Use open_files.'); + expect(skill).toContain('[guide](~/.claude/skills/demo/guide.md)'); + expect(skill).not.toContain('{tool:Read}'); + expect(existsSync(path.join(projectRoot, '.claude', 'skills', 'demo', '_partials'))).toBe(false); + }); + + it('fails before writing when a declared skill has an unmapped tool placeholder, dry-run included', async () => { + await writeLibrarySkill('demo', { body: 'Use {tool:Read}.' }); + await declareSkills('demo'); + + await expect(syncCommand(makeOptions({ dryRun: true }), projectRoot, contentDir)).rejects.toThrow( + /Unmapped tool name "Read" in skills\/demo\/SKILL\.md/, + ); + await expect(syncCommand(makeOptions(), projectRoot, contentDir)).rejects.toThrow(/Unmapped tool name "Read"/); + expect(existsSync(skillPath('demo'))).toBe(false); + }); }); describe('declared subagents', () => { diff --git a/packages/agents/src/commands/sync.ts b/packages/agents/src/commands/sync.ts index 628db15f..ceb78e2c 100644 --- a/packages/agents/src/commands/sync.ts +++ b/packages/agents/src/commands/sync.ts @@ -16,6 +16,7 @@ import { parseRulebookFile } from '../lib/rulebook-schema.ts'; import { extractRulebookSkillSlug, renderSkillFile, resolveSkillName } from '../lib/rulebook-skill.ts'; import { extractInstalledSlugs, injectRulebook, removeRulebook } from '../lib/sentinel-inliner.ts'; import { deploySkill, resolveDeclaredSkill, type ResolvedSkill } from '../lib/skill-deploy.ts'; +import { renderSkillDirectory, type SkillDeployContext } from '../lib/skill-transform.ts'; import { deploySubagent, resolveDeclaredSubagent, @@ -45,6 +46,12 @@ interface HarnessSubagentTarget { readonly deployContext: SubagentDeployContext; } +/** One targeted harness's project-local skills dir paired with the per-harness inputs the skill transform needs. */ +interface HarnessSkillTarget { + readonly skillsDir: string; + readonly deployContext: SkillDeployContext; +} + /** The one per-domain difference: the base dir to resolve and deploy under, and the file that hosts ambient blocks. */ export interface SyncDomain { readonly baseDir: string; @@ -166,10 +173,16 @@ async function reconcileDomain( assertNoCrossNamespaceCollisions([...desiredSkillDirs.values()], declaredSkillSet); // Skill delivery targets project-local harness skills dirs, gated by detection (or `--harness`). Passing - // `projectRoot` as the base is what keeps the skills project-scoped, and keeps tests out of the real home dir. - const harnessSkillDirs = resolveHarnessIds(options.harness, domain.baseDir).map( - (harnessId) => resolveHarnessPaths(harnessId, domain.baseDir).skillsDir, + // `projectRoot` as the base is what keeps the skills project-scoped, and keeps tests out of the real home dir. Each + // target carries the per-harness skill-transform inputs so declared-skill deployment applies include expansion and + // tool-name/link rewriting. `harnessSkillDirs` is the plain dir list the rulebook-skill passes and orphan scans use, + // which need no transform context. + const harnessSkillTargets = await Promise.all( + resolveHarnessIds(options.harness, domain.baseDir).map((harnessId) => + resolveSkillTarget(harnessId, domain.baseDir, contentDir), + ), ); + const harnessSkillDirs = harnessSkillTargets.map((target) => target.skillsDir); // Subagent delivery targets each harness's project-local subagents dir, loading that harness's overlay and tool // mapping so the deploy applies the same transform `install` would. Resolved separately from skills because the @@ -225,6 +238,11 @@ async function reconcileDomain( collectOwnedTargets(harnessSkillDirs, resolved, resolvedSkills, harnessSubagentTargets, resolvedSubagents), ); + // Render every declared skill against every targeted harness up front, so a broken include or an unmapped tool + // placeholder fails the whole run — dry-run included — before any file is written. The rendered output is discarded + // here; `deploySkill` re-renders it at write time. + await assertDeclaredSkillsRender(harnessSkillTargets, resolvedSkills); + if (options.dryRun) { reportDryRun({ ambientHostName: path.basename(ambientHostPath), @@ -290,14 +308,7 @@ async function reconcileDomain( // Reconcile declared skills per targeted harness, independently of the rulebook-skill pass above: retract owned // declared-skill dirs no longer declared, then deploy each declared skill into `//`. - for (const { skillsDir, orphans } of declaredSkillOrphansByDir) { - for (const dir of orphans) { - await rm(path.join(skillsDir, dir), { recursive: true, force: true }); - } - for (const skill of resolvedSkills) { - await deploySkill(skill, path.join(skillsDir, skill.slug)); - } - } + await reconcileDeclaredSkills(harnessSkillTargets, declaredSkillOrphansByDir, resolvedSkills); // Reconcile declared subagents per targeted harness, independently of the skill passes: retract owned subagent // files no longer declared, then deploy each declared subagent as `/.md` with the harness @@ -594,6 +605,43 @@ async function reconcileDeclaredSubagents( } } +/** + * Retracts owned declared-skill dirs no longer declared, then deploys each declared skill into every targeted harness's + * skills dir with that harness's transform applied. Orphans were computed against the pre-write filesystem, so + * retracting before writing lets a slug freed in one dir be re-created in the same sync rather than clobbered. + */ +async function reconcileDeclaredSkills( + targets: ReadonlyArray, + orphansByDir: ReadonlyArray<{ skillsDir: string; orphans: ReadonlyArray }>, + resolvedSkills: ReadonlyArray, +): Promise { + for (const target of targets) { + const orphans = orphansByDir.find((entry) => entry.skillsDir === target.skillsDir)?.orphans ?? []; + for (const dir of orphans) { + await rm(path.join(target.skillsDir, dir), { recursive: true, force: true }); + } + for (const skill of resolvedSkills) { + await deploySkill(skill, path.join(target.skillsDir, skill.slug), target.deployContext); + } + } +} + +/** + * Renders every declared skill against every targeted harness, discarding the output, so a broken include or an + * unmapped tool placeholder throws before any file is written. The deploy pass re-renders at write time; this pass + * exists only to fail the run closed, including under `--dry-run`. + */ +async function assertDeclaredSkillsRender( + targets: ReadonlyArray, + resolvedSkills: ReadonlyArray, +): Promise { + for (const target of targets) { + for (const skill of resolvedSkills) { + await renderSkillDirectory(skill.srcDir, skill.slug, target.deployContext); + } + } +} + /** * Regenerates the home-domain Rovo Dev `prompts.yml` so home-deployed skills appear in its available-skills list. As a * pure projection of the on-disk skills dir, it also drops any just-retracted skill. A no-op for the repo domain (which @@ -719,4 +767,28 @@ async function resolveSubagentTarget( }; } +/** + * Resolves one harness's project-local skills dir together with the per-harness inputs the skill transform needs: the + * tool-name mapping (from the harness overlay), the link-rewrite prefix, the home-dir segment, and the harness id. + * Passing `projectRoot` as the base keeps delivery project-scoped. + */ +async function resolveSkillTarget( + harnessId: HarnessId, + projectRoot: string, + contentDir: string, +): Promise { + const harnessConfig = HARNESSES[harnessId]; + const overlayYaml = await loadHarnessOverlay(contentDir, harnessConfig); + return { + skillsDir: resolveHarnessPaths(harnessId, projectRoot).skillsDir, + deployContext: { + contentDir, + toolMapping: loadToolMapping(overlayYaml), + pathPrefix: `${harnessConfig.homeDir}/${harnessConfig.skillsDirName}`, + homeDir: harnessConfig.homeDir, + harnessId: harnessConfig.id, + }, + }; +} + // endregion | Helpers diff --git a/packages/agents/src/lib/__tests__/skill-deploy.test.ts b/packages/agents/src/lib/__tests__/skill-deploy.test.ts index 0ee86993..62e9f016 100644 --- a/packages/agents/src/lib/__tests__/skill-deploy.test.ts +++ b/packages/agents/src/lib/__tests__/skill-deploy.test.ts @@ -5,7 +5,11 @@ import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { expandIncludes } from '../directive-expander.ts'; +import { rewriteMarkdownPaths, rewriteTemplateVariables } from '../path-rewriter.ts'; import { deploySkill, resolveDeclaredSkill } from '../skill-deploy.ts'; +import { type SkillDeployContext } from '../skill-transform.ts'; +import { rewriteToolNames } from '../tool-name-rewriter.ts'; describe(resolveDeclaredSkill, () => { let librarySkillsDir: string; @@ -71,71 +75,129 @@ describe(deploySkill, () => { await rm(destParent, { recursive: true, force: true }); }); - /** Writes a library skill directory from a map of relative file paths to contents. */ - async function writeLibrarySkill(slug: string, files: Record): Promise { - for (const [relPath, content] of Object.entries(files)) { - const full = path.join(librarySkillsDir, slug, relPath); - await mkdir(path.dirname(full), { recursive: true }); - await writeFile(full, content, 'utf8'); - } - } - it('writes the SKILL.md with the ownership marker into the destination', async () => { await writeLibrarySkill('people-report', { 'SKILL.md': '---\nname: people-report\n---\n\n# People report\n' }); - const skill = resolvedSkill('people-report'); const destDir = path.join(destParent, 'people-report'); - await deploySkill(skill, destDir); + await deploySkill(resolvedSkill('people-report'), destDir, context()); const deployed = await readFile(path.join(destDir, 'SKILL.md'), 'utf8'); expect(deployed).toContain(''); expect(deployed).toContain('# People report'); }); - it('re-deploys an unchanged single-file skill without rewriting SKILL.md', async () => { + it('re-deploys an unchanged skill without rewriting SKILL.md', async () => { await writeLibrarySkill('people-report', { 'SKILL.md': '---\nname: people-report\n---\n\n# People report\n' }); - const skill = resolvedSkill('people-report'); const destDir = path.join(destParent, 'people-report'); - await deploySkill(skill, destDir); + await deploySkill(resolvedSkill('people-report'), destDir, context()); const firstMtime = statSync(path.join(destDir, 'SKILL.md')).mtimeMs; - await deploySkill(skill, destDir); + await deploySkill(resolvedSkill('people-report'), destDir, context()); expect(statSync(path.join(destDir, 'SKILL.md')).mtimeMs).toBe(firstMtime); }); - it('mirrors auxiliary files and subdirectories verbatim, marking only the root SKILL.md', async () => { + it('mirrors auxiliary assets verbatim and marks only the root SKILL.md', async () => { await writeLibrarySkill('multi', { 'SKILL.md': '---\nname: multi\n---\n\n# Multi\n', 'reference.md': '# Reference\n', 'data/table.csv': 'a,b\n1,2\n', }); - const skill = resolvedSkill('multi'); const destDir = path.join(destParent, 'multi'); - await deploySkill(skill, destDir); + await deploySkill(resolvedSkill('multi'), destDir, context()); expect(await readFile(path.join(destDir, 'reference.md'), 'utf8')).toBe('# Reference\n'); expect(await readFile(path.join(destDir, 'data', 'table.csv'), 'utf8')).toBe('a,b\n1,2\n'); - // The marker belongs only on the root SKILL.md. expect(await readFile(path.join(destDir, 'reference.md'), 'utf8')).not.toContain('codeassembly-skill'); }); it('removes a destination file the source no longer carries on re-deploy', async () => { await writeLibrarySkill('multi', { 'SKILL.md': '---\nname: multi\n---\n\n# Multi\n', 'stale.md': 'old\n' }); const destDir = path.join(destParent, 'multi'); - await deploySkill(resolvedSkill('multi'), destDir); + await deploySkill(resolvedSkill('multi'), destDir, context()); expect(existsSync(path.join(destDir, 'stale.md'))).toBe(true); await rm(path.join(librarySkillsDir, 'multi', 'stale.md')); - await deploySkill(resolvedSkill('multi'), destDir); + await deploySkill(resolvedSkill('multi'), destDir, context()); expect(existsSync(path.join(destDir, 'stale.md'))).toBe(false); expect(existsSync(path.join(destDir, 'SKILL.md'))).toBe(true); }); + it('expands includes and rewrites tool placeholders and bare-relative links in deployed .md files', async () => { + await writeLibrarySkill('demo', { + 'SKILL.md': + '---\nname: demo\n---\n\n\n\nUse {tool:Read}. See [guide](./reference/guide.md).\n', + '_partials/frag.md': 'Shared fragment.\n', + 'reference/guide.md': 'Back to [home](../SKILL.md). Run `{harness_home_dir}/x`.\n', + }); + const destDir = path.join(destParent, 'demo'); + + await deploySkill(resolvedSkill('demo'), destDir, context(new Map([['Read', 'open_files']]))); + + const skillMd = await readFile(path.join(destDir, 'SKILL.md'), 'utf8'); + expect(skillMd).toContain('Shared fragment.'); + expect(skillMd).toContain('Use open_files.'); + expect(skillMd).toContain('[guide](~/.claude/skills/demo/reference/guide.md)'); + expect(skillMd).not.toContain('{tool:Read}'); + const guide = await readFile(path.join(destDir, 'reference', 'guide.md'), 'utf8'); + expect(guide).toContain('[home](~/.claude/skills/demo/SKILL.md)'); + expect(guide).toContain('~/.claude/x'); + // The partial is an include target, never deployed. + expect(existsSync(path.join(destDir, '_partials'))).toBe(false); + }); + + it('deploys the same body install composes from the shared transform steps, markers aside', async () => { + const toolMapping = new Map([['Read', 'open_files']]); + await writeLibrarySkill('demo', { + 'SKILL.md': + '---\nname: demo\n---\n\nUse {tool:Read}. See [guide](./reference/guide.md). Run `{harness_home_dir}/x`.\n', + 'reference/guide.md': '# Guide\n', + }); + const destDir = path.join(destParent, 'demo'); + + await deploySkill(resolvedSkill('demo'), destDir, context(toolMapping)); + + // Recompose install's pipeline for SKILL.md independently: expand → tools → markdown paths → template vars. + const expanded = await expandIncludes(path.join(librarySkillsDir, 'demo', 'SKILL.md'), librarySkillsDir); + const tooled = rewriteToolNames(expanded, toolMapping, 'demo/SKILL.md'); + const pathed = rewriteMarkdownPaths(tooled, 'demo/SKILL.md', '.claude/skills'); + const expected = rewriteTemplateVariables(pathed, '.claude', 'claude'); + + const deployed = await readFile(path.join(destDir, 'SKILL.md'), 'utf8'); + const withoutMarker = deployed + .split('\n') + .filter((line) => !line.includes('codeassembly-skill:')) + .join('\n'); + expect(withoutMarker).toBe(expected); + }); + + // region | Helpers + + function context(toolMapping: ReadonlyMap = new Map()): SkillDeployContext { + return { + contentDir: librarySkillsDir, + toolMapping, + pathPrefix: '.claude/skills', + homeDir: '.claude', + harnessId: 'claude', + }; + } + + /** Writes a library skill directory from a map of relative file paths to contents. */ + async function writeLibrarySkill(slug: string, files: Record): Promise { + for (const [relPath, content] of Object.entries(files)) { + const full = path.join(librarySkillsDir, slug, relPath); + await mkdir(path.dirname(full), { recursive: true }); + await writeFile(full, content, 'utf8'); + } + } + /** Builds a ResolvedSkill without the deploy-field check, so deploySkill tests can use minimal fixtures. */ function resolvedSkill(slug: string): { slug: string; srcDir: string } { return { slug, srcDir: path.join(librarySkillsDir, slug) }; } + + // endregion | Helpers }); diff --git a/packages/agents/src/lib/harness-overlay.ts b/packages/agents/src/lib/harness-overlay.ts index 7fc189f4..cf3cf6b8 100644 --- a/packages/agents/src/lib/harness-overlay.ts +++ b/packages/agents/src/lib/harness-overlay.ts @@ -5,9 +5,9 @@ import { isEnoent } from './type-guards.ts'; import type { HarnessConfig } from './types.ts'; /** - * Reads a harness's overlay YAML, returning an empty string when the file is absent. The overlay lives under - * `subagents/_data/` but is harness-level: it carries both the subagent frontmatter `_defaults`/per-agent merge and the - * shared `{tool:NAME}` mapping, so the skill and subagent deploy paths load it alike. + * Reads a harness's overlay YAML, returning an empty string when the file is absent. Though it lives under + * `subagents/_data/`, the overlay is harness-level: it carries both the subagent frontmatter `_defaults`/per-agent merge + * and the shared `{tool:NAME}` mapping. */ export async function loadHarnessOverlay(contentDir: string, harnessConfig: HarnessConfig): Promise { const overlayPath = path.join(contentDir, 'subagents', '_data', harnessConfig.frontmatterFile); diff --git a/packages/agents/src/lib/skill-deploy.ts b/packages/agents/src/lib/skill-deploy.ts index 2485e0a7..577262a9 100644 --- a/packages/agents/src/lib/skill-deploy.ts +++ b/packages/agents/src/lib/skill-deploy.ts @@ -4,6 +4,7 @@ import path from 'node:path'; import { makeArtifactMarker } from './artifact-marker.ts'; import { readDeploy } from './deploy-frontmatter.ts'; import { writeIfChanged } from './fs-helpers.ts'; +import { renderSkillDirectory, type SkillDeployContext } from './skill-transform.ts'; import { isEnoent, isMissingFile } from './type-guards.ts'; const skillMarker = makeArtifactMarker('skill'); @@ -46,26 +47,28 @@ export async function resolveDeclaredSkill(slug: string, librarySkillsDir: strin } /** - * Materializes a resolved skill into `destDir/`, mirroring its library directory verbatim and stamping the - * declared-skill ownership marker into the deployed root `SKILL.md`. - * The mirror is byte-stable: Unchanged files are left untouched and destination files the source no longer carries - * are removed) — so re-running sync on an unchanged skill makes no filesystem changes. - * Verbatim only: no include expansion or path rewriting (deferred to the first skill that needs it). + * Materializes a resolved skill into `destDir/` for one harness: every `.md` file is include-expanded and + * tool-name/link/template-rewritten through the shared skill transform, non-`.md` files are mirrored verbatim, and the + * declared-skill ownership marker is stamped into the deployed root `SKILL.md`. + * The write is byte-stable: unchanged files are left untouched, and destination files the source no longer carries — + * along with any directory left empty by their removal — are pruned, so re-deploying an unchanged skill makes no + * filesystem change. */ -export async function deploySkill(skill: ResolvedSkill, destDir: string): Promise { +export async function deploySkill(skill: ResolvedSkill, destDir: string, context: SkillDeployContext): Promise { + const entries = await renderSkillDirectory(skill.srcDir, skill.slug, context); await mkdir(destDir, { recursive: true }); - const srcEntries = await readdir(skill.srcDir, { withFileTypes: true }); - await removeOrphans(destDir, srcEntries); - for (const entry of srcEntries) { - const srcPath = path.join(skill.srcDir, entry.name); - const destPath = path.join(destDir, entry.name); - if (entry.isDirectory()) { - await copySubtree(srcPath, destPath); - } else if (entry.name === 'SKILL.md') { - await writeIfChanged(destPath, skillMarker.injectMarker(await readFile(srcPath, 'utf8'), skill.slug)); + const expectedFiles = new Set(entries.map((entry) => entry.relPath)); + await pruneOrphans(destDir, '', expectedFiles); + + for (const entry of entries) { + const destPath = path.join(destDir, entry.relPath); + await mkdir(path.dirname(destPath), { recursive: true }); + if (entry.kind === 'markdown') { + const body = entry.relPath === 'SKILL.md' ? skillMarker.injectMarker(entry.content, skill.slug) : entry.content; + await writeIfChanged(destPath, body); } else { - await copyFileIfChanged(srcPath, destPath); + await copyFileIfChanged(entry.srcPath, destPath); } } } @@ -83,17 +86,21 @@ async function copyFileIfChanged(srcPath: string, destPath: string): Promise { - await mkdir(destDir, { recursive: true }); - const srcEntries = await readdir(srcDir, { withFileTypes: true }); - await removeOrphans(destDir, srcEntries); - - for (const entry of srcEntries) { - const srcPath = path.join(srcDir, entry.name); - const destPath = path.join(destDir, entry.name); - await (entry.isDirectory() ? copySubtree(srcPath, destPath) : copyFileIfChanged(srcPath, destPath)); + * Removes every destination file absent from `expectedFiles`, then any directory left empty by those removals, so a + * skill's dropped files — and the directories that held them — do not linger across re-deploys. + */ +async function pruneOrphans(destDir: string, relDir: string, expectedFiles: ReadonlySet): Promise { + for (const entry of await readdir(path.join(destDir, relDir), { withFileTypes: true })) { + const rel = relDir === '' ? entry.name : `${relDir}/${entry.name}`; + const absPath = path.join(destDir, rel); + if (entry.isDirectory()) { + await pruneOrphans(destDir, rel, expectedFiles); + if ((await readdir(absPath)).length === 0) { + await rm(absPath, { recursive: true, force: true }); + } + } else if (!expectedFiles.has(rel)) { + await rm(absPath, { force: true }); + } } } @@ -109,14 +116,4 @@ async function readFileOrUndefined(filePath: string): Promise): Promise { - const srcNames = new Set(srcEntries.map((entry) => entry.name)); - for (const destEntry of await readdir(destDir, { withFileTypes: true })) { - if (!srcNames.has(destEntry.name)) { - await rm(path.join(destDir, destEntry.name), { recursive: true, force: true }); - } - } -} - // endregion | Helpers diff --git a/packages/agents/src/lib/skill-transform.ts b/packages/agents/src/lib/skill-transform.ts index f49f85c4..5d9ab2ce 100644 --- a/packages/agents/src/lib/skill-transform.ts +++ b/packages/agents/src/lib/skill-transform.ts @@ -28,11 +28,10 @@ export type RenderedSkillEntry = | { readonly kind: 'asset'; readonly relPath: string; readonly srcPath: string }; /** - * Renders a declared skill's directory for one harness: every `.md` file is include-expanded, tool-name-rewritten, and - * link/template-rewritten in install's order; non-`.md` files are returned as assets to copy verbatim. Pure read — no - * writes and no ownership/provenance marking — so `install` and `sync` each compose their own write strategy and markers - * around the single shared transform. Throws (with a file:line anchor) on a broken include or an unmapped `{tool:NAME}` - * placeholder. + * Renders a declared skill's directory for one harness: Every `.md` file is include-expanded, then tool-name-rewritten, + * then link/template-rewritten; non-`.md` files are returned as assets to copy verbatim. + * Read-only; the caller composes its own write strategy and markers around the transform. + * Throws (with a file:line anchor) on a broken include or an unmapped `{tool:NAME}` placeholder. * * `slug` anchors link rewriting: a relative Markdown link resolves against `/` under `pathPrefix`, matching * how the deployed skill sits at `//`. `_partials/` directories and dotfiles are skipped at every