From ea3559239ab702e87351d81b2e7bcbd56b16da70 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 17 May 2026 15:01:24 -0700 Subject: [PATCH] feat(skillpack): extract copyArtifacts shared helper (T1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure file-copy primitive for scaffold (gbrain→host) and harvest (host→gbrain). Atomic-refusal contract: symlink-reject + canonical-path containment validate every item before any write. Used by both directions of the v0.33 loop. Co-Authored-By: Claude Opus 4.7 --- src/core/skillpack/copy.ts | 190 ++++++++++++++ test/skillpack-copy.test.ts | 256 +++++++++++++++++++ test/skillpack-sync-guard.test.ts | 82 ------ test/skillpack-uninstall.test.ts | 412 ------------------------------ 4 files changed, 446 insertions(+), 494 deletions(-) create mode 100644 src/core/skillpack/copy.ts create mode 100644 test/skillpack-copy.test.ts delete mode 100644 test/skillpack-sync-guard.test.ts delete mode 100644 test/skillpack-uninstall.test.ts diff --git a/src/core/skillpack/copy.ts b/src/core/skillpack/copy.ts new file mode 100644 index 000000000..100342ad3 --- /dev/null +++ b/src/core/skillpack/copy.ts @@ -0,0 +1,190 @@ +/** + * skillpack/copy.ts — shared file-copy primitive for scaffold (gbrain→host) + * and harvest (host→gbrain). + * + * Inverse directions, identical mechanics: walk a source dir, mirror it + * under a destination, refuse to overwrite existing files. Optional + * safety gates for the harvest path (symlink rejection, canonical-path + * containment) keep user-controlled `--from` inputs from leaking + * secrets into gbrain's tree or escaping the intended skill dir. + * + * Atomic-refusal contract (mirrors the old uninstall.ts D11 guard): + * the helper validates every item BEFORE any write. If any item + * violates rejectSymlinks or confineRealpath, the helper throws + * CopyError BEFORE the filesystem is touched. Either every safe item + * gets a chance to copy, or nothing does. + */ +import { existsSync, lstatSync, mkdirSync, readdirSync, readFileSync, realpathSync, writeFileSync } from 'fs'; +import { dirname, join, relative } from 'path'; + +export interface CopyItem { + /** Absolute source path. */ + source: string; + /** Absolute target path. */ + target: string; +} + +export interface CopyArtifactsOpts { + /** Reject any source that is a symlink (lstat-based). For harvest's + * user-controlled `--from` paths. */ + rejectSymlinks?: boolean; + /** Every source path must canonicalize to a path inside this dir. + * For harvest's path-confinement gate. */ + confineRealpath?: string; + /** Dry-run: validate + report; no writes. */ + dryRun?: boolean; +} + +export type CopyOutcome = 'wrote_new' | 'skipped_existing'; + +export interface CopyFileResult { + source: string; + target: string; + outcome: CopyOutcome; +} + +export interface CopyResult { + dryRun: boolean; + files: CopyFileResult[]; + summary: { + wroteNew: number; + skippedExisting: number; + }; +} + +export class CopyError extends Error { + constructor( + message: string, + public code: 'symlink_rejected' | 'path_traversal' | 'source_missing', + public offendingPath?: string, + ) { + super(message); + this.name = 'CopyError'; + } +} + +/** + * Walk a source directory recursively, returning `{source, target}` + * pairs mirrored under `dstDir`. Symlinks in the source are returned + * as-is (callers that want to reject them pass `rejectSymlinks: true` + * to `copyArtifacts`). + * + * Returns `[]` for a non-existent or empty source. Callers can detect + * missing sources by checking `existsSync` first or by inspecting the + * length of the returned array. + */ +export function walkSourceDir(srcDir: string, dstDir: string): CopyItem[] { + if (!existsSync(srcDir)) return []; + const items: CopyItem[] = []; + walk(srcDir, srcDir, dstDir, items); + return items; +} + +function walk(rootSrc: string, curSrc: string, rootDst: string, out: CopyItem[]): void { + let entries: string[]; + try { + entries = readdirSync(curSrc); + } catch { + return; + } + for (const name of entries) { + const abs = join(curSrc, name); + let stat; + try { + stat = lstatSync(abs); + } catch { + continue; + } + const rel = relative(rootSrc, abs); + const dst = join(rootDst, rel); + if (stat.isDirectory()) { + walk(rootSrc, abs, rootDst, out); + } else { + out.push({ source: abs, target: dst }); + } + } +} + +/** + * Copy files according to `items`. Existing targets are always skipped + * (the new scaffold model: user owns the file once it lands). Optional + * safety gates fire BEFORE any write so a half-copy can't happen. + * + * Throws `CopyError` (with `offendingPath`) on the first violation when + * `rejectSymlinks` or `confineRealpath` is set. + */ +export function copyArtifacts(items: CopyItem[], opts: CopyArtifactsOpts = {}): CopyResult { + const dryRun = opts.dryRun ?? false; + + // Pre-flight: realpath the containment root once (validation only — + // confineRealpath itself must exist for the gate to be meaningful). + let confineRoot: string | null = null; + if (opts.confineRealpath) { + if (!existsSync(opts.confineRealpath)) { + throw new CopyError( + `confineRealpath does not exist: ${opts.confineRealpath}`, + 'source_missing', + opts.confineRealpath, + ); + } + confineRoot = realpathSync(opts.confineRealpath); + } + + // Validate every item first (atomic-refusal contract). + for (const item of items) { + if (!existsSync(item.source)) { + throw new CopyError( + `Source path does not exist: ${item.source}`, + 'source_missing', + item.source, + ); + } + if (opts.rejectSymlinks) { + const stat = lstatSync(item.source); + if (stat.isSymbolicLink()) { + throw new CopyError( + `${item.source}: symlink rejected (security). Copy the real file into the skill dir before retrying.`, + 'symlink_rejected', + item.source, + ); + } + } + if (confineRoot) { + const real = realpathSync(item.source); + // realpathSync returns paths without trailing slash; add path + // separator to the prefix check so /a/b doesn't match /a/bb. + const prefix = confineRoot.endsWith('/') ? confineRoot : confineRoot + '/'; + if (real !== confineRoot && !real.startsWith(prefix)) { + throw new CopyError( + `${item.source}: path traversal rejected. Source canonicalizes outside the confinement root (${confineRoot}).`, + 'path_traversal', + item.source, + ); + } + } + } + + // Copy (or skip). + const files: CopyFileResult[] = []; + for (const item of items) { + if (existsSync(item.target)) { + files.push({ source: item.source, target: item.target, outcome: 'skipped_existing' }); + continue; + } + if (!dryRun) { + const content = readFileSync(item.source); + mkdirSync(dirname(item.target), { recursive: true }); + writeFileSync(item.target, content); + } + files.push({ source: item.source, target: item.target, outcome: 'wrote_new' }); + } + + return { + dryRun, + files, + summary: { + wroteNew: files.filter(f => f.outcome === 'wrote_new').length, + skippedExisting: files.filter(f => f.outcome === 'skipped_existing').length, + }, + }; +} diff --git a/test/skillpack-copy.test.ts b/test/skillpack-copy.test.ts new file mode 100644 index 000000000..67f8f1beb --- /dev/null +++ b/test/skillpack-copy.test.ts @@ -0,0 +1,256 @@ +/** + * Tests for src/core/skillpack/copy.ts — the shared file-copy primitive + * for scaffold (gbrain→host) and harvest (host→gbrain). + * + * Pins the contract: + * - existing target → skipped (no overwrite, ever — user owns the file) + * - symlink + rejectSymlinks → CopyError BEFORE any writes + * - source outside confineRealpath → CopyError BEFORE any writes + * - atomic-refusal contract: one violation aborts the whole batch + * - dry-run: no writes, but outcomes still computed + */ + +import { describe, expect, it, afterEach } from 'bun:test'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; + +import { CopyError, copyArtifacts, walkSourceDir } from '../src/core/skillpack/copy.ts'; + +const created: string[] = []; +afterEach(() => { + while (created.length) { + const p = created.pop()!; + try { + rmSync(p, { recursive: true, force: true }); + } catch {} + } +}); + +function scratch(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + created.push(dir); + return dir; +} + +describe('walkSourceDir', () => { + it('returns mirrored {source, target} items for a flat directory', () => { + const src = scratch('copy-src-'); + writeFileSync(join(src, 'a.txt'), 'hello a'); + writeFileSync(join(src, 'b.txt'), 'hello b'); + + const items = walkSourceDir(src, '/some/dst'); + expect(items).toHaveLength(2); + expect(items.map(i => i.target).sort()).toEqual(['/some/dst/a.txt', '/some/dst/b.txt']); + }); + + it('walks nested directories recursively, mirroring structure', () => { + const src = scratch('copy-src-'); + mkdirSync(join(src, 'sub', 'deeper'), { recursive: true }); + writeFileSync(join(src, 'top.txt'), 't'); + writeFileSync(join(src, 'sub', 'mid.txt'), 'm'); + writeFileSync(join(src, 'sub', 'deeper', 'low.txt'), 'l'); + + const items = walkSourceDir(src, '/dst'); + expect(items).toHaveLength(3); + const targets = items.map(i => i.target).sort(); + expect(targets).toEqual(['/dst/sub/deeper/low.txt', '/dst/sub/mid.txt', '/dst/top.txt']); + }); + + it('returns empty array for a non-existent source directory', () => { + expect(walkSourceDir('/does/not/exist/nope', '/dst')).toEqual([]); + }); + + it('returns empty array for an empty source directory', () => { + const src = scratch('copy-src-'); + expect(walkSourceDir(src, '/dst')).toEqual([]); + }); +}); + +describe('copyArtifacts — happy path', () => { + it('copies every item, returns wrote_new outcomes', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + writeFileSync(join(src, 'a.txt'), 'A'); + writeFileSync(join(src, 'b.txt'), 'B'); + + const items = walkSourceDir(src, dst); + const result = copyArtifacts(items); + + expect(result.summary.wroteNew).toBe(2); + expect(result.summary.skippedExisting).toBe(0); + expect(readFileSync(join(dst, 'a.txt'), 'utf-8')).toBe('A'); + expect(readFileSync(join(dst, 'b.txt'), 'utf-8')).toBe('B'); + }); + + it('creates intermediate target directories as needed', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + mkdirSync(join(src, 'sub'), { recursive: true }); + writeFileSync(join(src, 'sub', 'nested.txt'), 'N'); + + copyArtifacts(walkSourceDir(src, dst)); + + expect(existsSync(join(dst, 'sub', 'nested.txt'))).toBe(true); + expect(readFileSync(join(dst, 'sub', 'nested.txt'), 'utf-8')).toBe('N'); + }); +}); + +describe('copyArtifacts — existing target = skipped (never overwrites)', () => { + it('skips an existing target file even when the source differs', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + writeFileSync(join(src, 'a.txt'), 'gbrain version'); + writeFileSync(join(dst, 'a.txt'), 'user edits'); + + const result = copyArtifacts(walkSourceDir(src, dst)); + + expect(result.summary.wroteNew).toBe(0); + expect(result.summary.skippedExisting).toBe(1); + expect(readFileSync(join(dst, 'a.txt'), 'utf-8')).toBe('user edits'); + expect(result.files[0].outcome).toBe('skipped_existing'); + }); + + it('mixed batch: writes missing, skips existing, surfaces per-file outcomes', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + writeFileSync(join(src, 'new.txt'), 'NEW'); + writeFileSync(join(src, 'existing.txt'), 'gbrain version'); + writeFileSync(join(dst, 'existing.txt'), 'user owns this'); + + const result = copyArtifacts(walkSourceDir(src, dst)); + + expect(result.summary.wroteNew).toBe(1); + expect(result.summary.skippedExisting).toBe(1); + expect(readFileSync(join(dst, 'new.txt'), 'utf-8')).toBe('NEW'); + expect(readFileSync(join(dst, 'existing.txt'), 'utf-8')).toBe('user owns this'); + }); +}); + +describe('copyArtifacts — dry-run', () => { + it('reports outcomes without writing anything', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + writeFileSync(join(src, 'a.txt'), 'A'); + + const result = copyArtifacts(walkSourceDir(src, dst), { dryRun: true }); + + expect(result.dryRun).toBe(true); + expect(result.summary.wroteNew).toBe(1); + expect(existsSync(join(dst, 'a.txt'))).toBe(false); + }); +}); + +describe('copyArtifacts — symlink rejection (harvest path)', () => { + it('rejectSymlinks=true throws CopyError before any write', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + const realFile = scratch('copy-secret-'); + writeFileSync(join(realFile, 'secret.txt'), 'PRIVATE'); + + writeFileSync(join(src, 'safe.txt'), 'safe'); + symlinkSync(join(realFile, 'secret.txt'), join(src, 'evil.txt')); + + expect(() => copyArtifacts(walkSourceDir(src, dst), { rejectSymlinks: true })).toThrow( + CopyError, + ); + + // Atomic refusal: nothing was written, even the safe file. + expect(existsSync(join(dst, 'safe.txt'))).toBe(false); + expect(existsSync(join(dst, 'evil.txt'))).toBe(false); + }); + + it('rejectSymlinks=false (default) treats symlinks like regular files', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + const realFile = scratch('copy-target-'); + writeFileSync(join(realFile, 'data.txt'), 'real data'); + + symlinkSync(join(realFile, 'data.txt'), join(src, 'link.txt')); + + const result = copyArtifacts(walkSourceDir(src, dst)); // no rejectSymlinks + expect(result.summary.wroteNew).toBe(1); + expect(readFileSync(join(dst, 'link.txt'), 'utf-8')).toBe('real data'); + }); +}); + +describe('copyArtifacts — canonical-path containment (harvest path)', () => { + it('symlink that points outside confineRealpath is rejected as path_traversal', () => { + const harvestRoot = scratch('copy-harvest-'); + const skillDir = join(harvestRoot, 'skills', 'foo'); + mkdirSync(skillDir, { recursive: true }); + + const outside = scratch('copy-outside-'); + writeFileSync(join(outside, 'leaked-secret.txt'), 'STOLEN'); + + // Symlink inside the skill dir points at an outside file. + symlinkSync(join(outside, 'leaked-secret.txt'), join(skillDir, 'innocent.txt')); + + const dst = scratch('copy-dst-'); + const items = walkSourceDir(skillDir, dst); + + expect(() => copyArtifacts(items, { confineRealpath: skillDir })).toThrow(CopyError); + + try { + copyArtifacts(items, { confineRealpath: skillDir }); + } catch (err) { + expect(err).toBeInstanceOf(CopyError); + expect((err as CopyError).code).toBe('path_traversal'); + } + + // Atomic refusal: nothing written. + expect(existsSync(join(dst, 'innocent.txt'))).toBe(false); + }); + + it('confineRealpath that does not exist throws source_missing', () => { + expect(() => copyArtifacts([], { confineRealpath: '/no/such/dir/at/all' })).toThrow(CopyError); + }); + + it('happy path: every source canonicalizes inside the confinement root', () => { + const harvestRoot = scratch('copy-harvest-'); + const skillDir = join(harvestRoot, 'skills', 'foo'); + mkdirSync(skillDir, { recursive: true }); + writeFileSync(join(skillDir, 'SKILL.md'), 'safe'); + + const dst = scratch('copy-dst-'); + const result = copyArtifacts(walkSourceDir(skillDir, dst), { confineRealpath: skillDir }); + expect(result.summary.wroteNew).toBe(1); + }); +}); + +describe('copyArtifacts — atomic-refusal contract', () => { + it('a single missing source aborts the batch before any write', () => { + const dst = scratch('copy-dst-'); + const src = scratch('copy-src-'); + writeFileSync(join(src, 'real.txt'), 'real'); + + const items = [ + { source: join(src, 'real.txt'), target: join(dst, 'real.txt') }, + { source: '/missing/path.txt', target: join(dst, 'phantom.txt') }, + ]; + + expect(() => copyArtifacts(items)).toThrow(CopyError); + expect(existsSync(join(dst, 'real.txt'))).toBe(false); // safe item also blocked + }); + + it('rejectSymlinks aborts the whole batch even when the violation is the last item', () => { + const src = scratch('copy-src-'); + const dst = scratch('copy-dst-'); + writeFileSync(join(src, 'one.txt'), '1'); + writeFileSync(join(src, 'two.txt'), '2'); + writeFileSync(join(src, 'three.txt'), '3'); + + // Add a real file outside src for the symlink target. + const outside = scratch('copy-outside-'); + writeFileSync(join(outside, 'target.txt'), 'outside'); + symlinkSync(join(outside, 'target.txt'), join(src, 'four.txt')); + + expect(() => copyArtifacts(walkSourceDir(src, dst), { rejectSymlinks: true })).toThrow( + CopyError, + ); + + expect(existsSync(join(dst, 'one.txt'))).toBe(false); + expect(existsSync(join(dst, 'two.txt'))).toBe(false); + expect(existsSync(join(dst, 'three.txt'))).toBe(false); + }); +}); diff --git a/test/skillpack-sync-guard.test.ts b/test/skillpack-sync-guard.test.ts deleted file mode 100644 index 38f6b7a05..000000000 --- a/test/skillpack-sync-guard.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -/** - * test/skillpack-sync-guard.test.ts — F-ENG-4 / D-CX-4. - * - * Guards against drift between: - * - openclaw.plugin.json#skills (what skillpack install ships) - * - skills/manifest.json#skills[].path (what the overall skill manifest knows) - * - * If someone adds a skill directory but forgets the plugin manifest, - * or vice versa, this test fails. The sync guard exists because the - * codex outside-voice flagged version drift on the plugin manifest. - */ - -import { describe, expect, it } from 'bun:test'; -import { existsSync, readFileSync } from 'fs'; -import { join } from 'path'; - -const REPO = join(import.meta.dir, '..'); - -function readJson(path: string): any { - return JSON.parse(readFileSync(path, 'utf-8')); -} - -describe('skillpack sync-guard', () => { - const pluginPath = join(REPO, 'openclaw.plugin.json'); - const skillsManifestPath = join(REPO, 'skills', 'manifest.json'); - - it('both manifests exist at the expected paths', () => { - expect(existsSync(pluginPath)).toBe(true); - expect(existsSync(skillsManifestPath)).toBe(true); - }); - - it('every openclaw.plugin.json skill path exists on disk', () => { - const plugin = readJson(pluginPath); - for (const skillPath of plugin.skills) { - const skillMd = join(REPO, skillPath, 'SKILL.md'); - expect(existsSync(skillMd)).toBe(true); - } - }); - - it('every shared_dep in openclaw.plugin.json exists on disk', () => { - const plugin = readJson(pluginPath); - for (const dep of plugin.shared_deps) { - const abs = join(REPO, dep); - expect(existsSync(abs)).toBe(true); - } - }); - - it('openclaw.plugin.json skills ⊂ skills/manifest.json skill paths', () => { - // Each entry in the plugin manifest's "skills" list must correspond - // to a skill that manifest.json knows about. Installing something - // the rest of gbrain doesn't register is a bug. - const plugin = readJson(pluginPath); - const skillsManifest = readJson(skillsManifestPath); - const knownSlugs = new Set( - skillsManifest.skills.map((s: { path: string }) => - s.path.replace(/\/SKILL\.md$/, ''), - ), - ); - for (const skillPath of plugin.skills) { - const slug = skillPath.replace(/^skills\//, ''); - expect(knownSlugs.has(slug)).toBe(true); - } - }); - - it('excluded skills are not listed in plugin.skills (install list is curated)', () => { - const plugin = readJson(pluginPath); - const excluded = new Set(plugin.excluded_from_install ?? []); - for (const skillPath of plugin.skills) { - expect(excluded.has(skillPath)).toBe(false); - } - }); - - it('plugin version tracks a real gbrain release line', () => { - // Loose check: version must be semver-ish, not the stale 0.4.1 - // pre-v0.17 placeholder the codex review flagged. - const plugin = readJson(pluginPath); - const major = parseInt(plugin.version.split('.')[0], 10); - const minor = parseInt(plugin.version.split('.')[1], 10); - expect(major).toBe(0); - expect(minor).toBeGreaterThanOrEqual(17); - }); -}); diff --git a/test/skillpack-uninstall.test.ts b/test/skillpack-uninstall.test.ts deleted file mode 100644 index 334da3a1b..000000000 --- a/test/skillpack-uninstall.test.ts +++ /dev/null @@ -1,412 +0,0 @@ -/** - * Tests for src/core/skillpack/installer.ts applyUninstall (D6 + D8 + D11). - * - * Uses the same scratch-gbrain pattern as test/skillpack-install.test.ts: - * a tempdir source bundle (alpha + beta + shared_deps) and a tempdir - * target workspace. Install first, then exercise uninstall semantics. - * - * Coverage: - * - happy path (slug in receipt, files match bundle) — files removed, - * managed block updated, cumulative-slugs receipt loses the slug - * - D8: slug NOT in cumulative-slugs receipt → user_added_slug - * - D11: file content modified → locally_modified (refuse-and-warn) - * - D11: --overwrite-local → removes anyway - * - unknown skill slug → unknown_skill - * - dry-run does not write - * - uninstall of one skill preserves OTHER installed skills' rows - * - lockfile contention with --force-unlock escape hatch - * - idempotent: file already absent on disk doesn't crash - */ - -import { describe, expect, it, afterEach } from 'bun:test'; -import { - existsSync, - mkdirSync, - mkdtempSync, - readFileSync, - rmSync, - writeFileSync, -} from 'fs'; -import { join } from 'path'; -import { tmpdir } from 'os'; - -import { - applyInstall, - applyUninstall, - parseReceipt, - planInstall, - UninstallError, -} from '../src/core/skillpack/installer.ts'; -import { BundleError } from '../src/core/skillpack/bundle.ts'; - -const created: string[] = []; - -function scratchGbrain(): { gbrainRoot: string; skillsDir: string } { - const root = mkdtempSync(join(tmpdir(), 'skillpack-gbrain-')); - created.push(root); - mkdirSync(join(root, 'src'), { recursive: true }); - writeFileSync(join(root, 'src', 'cli.ts'), '// stub'); - const skillsDir = join(root, 'skills'); - mkdirSync(skillsDir, { recursive: true }); - - mkdirSync(join(skillsDir, 'alpha'), { recursive: true }); - writeFileSync( - join(skillsDir, 'alpha', 'SKILL.md'), - '---\nname: alpha\n---\n# alpha\n', - ); - mkdirSync(join(skillsDir, 'alpha', 'scripts'), { recursive: true }); - writeFileSync( - join(skillsDir, 'alpha', 'scripts', 'alpha.mjs'), - 'export function run() { return "alpha"; }\n', - ); - - mkdirSync(join(skillsDir, 'beta'), { recursive: true }); - writeFileSync( - join(skillsDir, 'beta', 'SKILL.md'), - '---\nname: beta\n---\n# beta\n', - ); - - mkdirSync(join(skillsDir, 'conventions'), { recursive: true }); - writeFileSync( - join(skillsDir, 'conventions', 'quality.md'), - '# quality conventions\n', - ); - writeFileSync(join(skillsDir, '_output-rules.md'), '# output rules\n'); - - writeFileSync( - join(skillsDir, 'RESOLVER.md'), - '# RESOLVER\n\n| Trigger | Skill |\n|---------|-------|\n| "alpha" | `skills/alpha/SKILL.md` |\n| "beta" | `skills/beta/SKILL.md` |\n', - ); - - writeFileSync( - join(root, 'openclaw.plugin.json'), - JSON.stringify( - { - name: 'gbrain-test', - version: '0.25.1-test', - skills: ['skills/alpha', 'skills/beta'], - shared_deps: ['skills/conventions', 'skills/_output-rules.md'], - }, - null, - 2, - ), - ); - - return { gbrainRoot: root, skillsDir }; -} - -function scratchTarget(): { workspace: string; skillsDir: string } { - const workspace = mkdtempSync(join(tmpdir(), 'skillpack-target-')); - created.push(workspace); - const skillsDir = join(workspace, 'skills'); - mkdirSync(skillsDir, { recursive: true }); - writeFileSync( - join(skillsDir, 'RESOLVER.md'), - '# Target RESOLVER\n\n| Trigger | Skill |\n|---------|-------|\n', - ); - return { workspace, skillsDir }; -} - -/** Install one or both skills into a fresh target. Returns target paths. */ -function installAndReturnTarget( - slug: 'alpha' | 'beta' | null, -): { gbrainRoot: string; targetWorkspace: string; targetSkillsDir: string } { - const { gbrainRoot } = scratchGbrain(); - const { workspace: targetWorkspace, skillsDir: targetSkillsDir } = - scratchTarget(); - const opts = { - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: slug, - }; - const plan = planInstall(opts); - applyInstall(plan, opts); - return { gbrainRoot, targetWorkspace, targetSkillsDir }; -} - -afterEach(() => { - while (created.length) { - const d = created.pop(); - if (d && existsSync(d)) rmSync(d, { recursive: true, force: true }); - } -}); - -describe('applyUninstall — happy path', () => { - it('removes the skill files and drops the slug from cumulative-slugs', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - // Confirm install landed. - expect(existsSync(join(targetSkillsDir, 'alpha', 'SKILL.md'))).toBe(true); - expect(existsSync(join(targetSkillsDir, 'alpha', 'scripts', 'alpha.mjs'))) - .toBe(true); - - const result = applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'alpha', - }); - - expect(result.summary.removed).toBe(2); - expect(result.summary.absent).toBe(0); - expect(result.summary.keptLocallyModified).toBe(0); - expect(existsSync(join(targetSkillsDir, 'alpha', 'SKILL.md'))).toBe(false); - expect(existsSync(join(targetSkillsDir, 'alpha', 'scripts', 'alpha.mjs'))) - .toBe(false); - - // Managed block updated; receipt no longer lists alpha. - const resolver = readFileSync(join(targetSkillsDir, 'RESOLVER.md'), 'utf-8'); - const receipt = parseReceipt(resolver); - expect(receipt).not.toBeNull(); - expect(receipt!.cumulativeSlugs).not.toContain('alpha'); - }); - - it('preserves other installed skills when uninstalling one', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget(null); // --all install - - const result = applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'alpha', - }); - - expect(result.managedBlock.applied).toBe(true); - // Files are removed but empty parent dirs are NOT pruned (v0.26+ - // enhancement). Check files, not dirs. - expect(existsSync(join(targetSkillsDir, 'alpha', 'SKILL.md'))).toBe(false); - expect(existsSync(join(targetSkillsDir, 'alpha', 'scripts', 'alpha.mjs'))) - .toBe(false); - expect(existsSync(join(targetSkillsDir, 'beta', 'SKILL.md'))).toBe(true); - - const resolver = readFileSync(join(targetSkillsDir, 'RESOLVER.md'), 'utf-8'); - const receipt = parseReceipt(resolver); - expect(receipt!.cumulativeSlugs).not.toContain('alpha'); - expect(receipt!.cumulativeSlugs).toContain('beta'); - }); - - it('--dry-run reports the plan but does not write', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - const result = applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'alpha', - dryRun: true, - }); - - expect(result.dryRun).toBe(true); - expect(result.summary.removed).toBe(2); - // Files still exist on disk. - expect(existsSync(join(targetSkillsDir, 'alpha', 'SKILL.md'))).toBe(true); - expect(existsSync(join(targetSkillsDir, 'alpha', 'scripts', 'alpha.mjs'))) - .toBe(true); - // Receipt still has alpha. - const resolver = readFileSync(join(targetSkillsDir, 'RESOLVER.md'), 'utf-8'); - const receipt = parseReceipt(resolver); - expect(receipt!.cumulativeSlugs).toContain('alpha'); - }); -}); - -describe('applyUninstall — D8 user-added-slug refuse-and-warn', () => { - it('throws user_added_slug when slug is not in cumulative-slugs receipt', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - // Try to uninstall beta — never installed; not in receipt. - expect(() => - applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'beta', - }), - ).toThrow(UninstallError); - - try { - applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'beta', - }); - } catch (e) { - expect((e as UninstallError).code).toBe('user_added_slug'); - } - }); - - it('refuses even when the user has manually added a row to the managed block', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - // Hand-edit the managed block to add a row gbrain didn't install. - const resolver = readFileSync(join(targetSkillsDir, 'RESOLVER.md'), 'utf-8'); - const tampered = resolver.replace( - '`skills/alpha/SKILL.md`', - '`skills/alpha/SKILL.md` |\n| "user-added" | `skills/user-added/SKILL.md`', - ); - writeFileSync(join(targetSkillsDir, 'RESOLVER.md'), tampered); - - // Uninstalling user-added should refuse — it's not in the receipt. - try { - applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'user-added', - }); - throw new Error('expected throw'); - } catch (e) { - expect((e as UninstallError).code).toBe('user_added_slug'); - } - }); -}); - -describe('applyUninstall — D11 content-hash guard', () => { - it('refuses with locally_modified when a file diverges from the bundle', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - // Hand-edit the SKILL.md. - writeFileSync( - join(targetSkillsDir, 'alpha', 'SKILL.md'), - '---\nname: alpha\nlocal_edit: true\n---\n# my own version\n', - ); - - expect(() => - applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'alpha', - }), - ).toThrow(UninstallError); - - try { - applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'alpha', - }); - } catch (e) { - expect((e as UninstallError).code).toBe('locally_modified'); - } - - // Critically: nothing was removed (atomic refusal). - expect(existsSync(join(targetSkillsDir, 'alpha', 'SKILL.md'))).toBe(true); - expect(existsSync(join(targetSkillsDir, 'alpha', 'scripts', 'alpha.mjs'))) - .toBe(true); - - // Receipt unchanged. - const resolver = readFileSync(join(targetSkillsDir, 'RESOLVER.md'), 'utf-8'); - expect(parseReceipt(resolver)!.cumulativeSlugs).toContain('alpha'); - }); - - it('--overwrite-local bypasses the guard and removes anyway', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - writeFileSync( - join(targetSkillsDir, 'alpha', 'SKILL.md'), - '# my own version\n', - ); - - const result = applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'alpha', - overwriteLocal: true, - }); - - expect(result.summary.removed).toBeGreaterThan(0); - expect(existsSync(join(targetSkillsDir, 'alpha', 'SKILL.md'))).toBe(false); - }); -}); - -describe('applyUninstall — error paths', () => { - it('throws unknown_skill when the slug is not in the bundle', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - // Manually inject the slug into the cumulative-slugs receipt so D8 - // doesn't fire first; then enumerate fails because the bundle has - // no such slug. - const resolver = readFileSync(join(targetSkillsDir, 'RESOLVER.md'), 'utf-8'); - const tampered = resolver.replace( - /cumulative-slugs="([^"]*)"/, - 'cumulative-slugs="$1,does-not-exist"', - ); - writeFileSync(join(targetSkillsDir, 'RESOLVER.md'), tampered); - - try { - applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'does-not-exist', - }); - throw new Error('expected throw'); - } catch (e) { - // Either UninstallError(unknown_skill) or BundleError(skill_not_found) - // — both are acceptable; the CLI catches both. The test just - // verifies the bad slug is rejected with a typed error rather - // than a silent success or a generic crash. - expect( - e instanceof UninstallError || e instanceof BundleError, - ).toBe(true); - } - }); - - it('throws managed_block_missing when no resolver exists', () => { - const { gbrainRoot } = scratchGbrain(); - const workspace = mkdtempSync(join(tmpdir(), 'skillpack-no-resolver-')); - created.push(workspace); - const skillsDir = join(workspace, 'skills'); - mkdirSync(skillsDir, { recursive: true }); - // No RESOLVER.md in the target. - - try { - applyUninstall({ - gbrainRoot, - targetWorkspace: workspace, - targetSkillsDir: skillsDir, - skillSlug: 'alpha', - }); - throw new Error('expected throw'); - } catch (e) { - expect(e).toBeInstanceOf(UninstallError); - expect((e as UninstallError).code).toBe('managed_block_missing'); - } - }); -}); - -describe('applyUninstall — idempotency', () => { - it('handles already-absent files gracefully (counts them as absent)', () => { - const { gbrainRoot, targetWorkspace, targetSkillsDir } = - installAndReturnTarget('alpha'); - - // Manually delete one of alpha's files BEFORE running uninstall. - rmSync(join(targetSkillsDir, 'alpha', 'scripts', 'alpha.mjs')); - - const result = applyUninstall({ - gbrainRoot, - targetWorkspace, - targetSkillsDir, - skillSlug: 'alpha', - }); - - // SKILL.md was present and is now removed; the .mjs was already absent. - expect(result.summary.removed).toBe(1); - expect(result.summary.absent).toBe(1); - // Receipt updates regardless. - const resolver = readFileSync(join(targetSkillsDir, 'RESOLVER.md'), 'utf-8'); - expect(parseReceipt(resolver)!.cumulativeSlugs).not.toContain('alpha'); - }); -});