mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
feat(skillpack): extract copyArtifacts shared helper (T1)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
3933eb6a79
commit
ea3559239a
@@ -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,
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user