fix(sync): macos symlink + multiple sync runs

.changeset/sync-symlink-trailing-slash.md

@@ -0,0 +1,5 @@
+---
+"@bomb.sh/tools": patch
+---
+
+Fixes `bsh sync` crashing when creating skill symlinks, and makes re-running it safe. It no longer errors or removes already-synced skills.

src/commands/sync.test.ts

@@ -0,0 +1,56 @@
+import { lstat, readlink } from 'node:fs/promises';
+import { fileURLToPath } from 'node:url';
+import { describe, it, expect } from 'vitest';
+import { createFixture } from '../test-utils/index.ts';
+import { copySkills } from './sync.ts';
+
+describe('copySkills', () => {
+	it('symlinks each skill into the destination', async () => {
+		const fixture = await createFixture({
+			'source-skills': {
+				build: {
+					'SKILL.md': '---\nname: build\ndescription: Build the project.\n---\nbody',
+				},
+			},
+		});
+
+		const source = new URL('source-skills/', fixture.root);
+		const dest = new URL('project/skills/', fixture.root);
+
+		const skills = await copySkills({ source, dest });
+
+		expect(skills).toEqual([{ name: 'build', description: 'Build the project.' }]);
+
+		// The destination entry must be a symlink, not a copy.
+		const linkPath = fileURLToPath(new URL('build', dest));
+		expect((await lstat(linkPath)).isSymbolicLink()).toBe(true);
+
+		// And it must resolve back to the source skill.
+		expect(await readlink(linkPath)).toBe('../../source-skills/build');
+
+		// Reading through the link reaches the real file.
+		expect(await fixture.text('project/skills/build/SKILL.md')).toContain('name: build');
+	});
+
+	it('is idempotent and never touches the source on re-sync', async () => {
+		const fixture = await createFixture({
+			'source-skills': {
+				build: {
+					'SKILL.md': '---\nname: build\ndescription: Build the project.\n---\nbody',
+				},
+			},
+		});
+
+		const source = new URL('source-skills/', fixture.root);
+		const dest = new URL('project/skills/', fixture.root);
+
+		const first = await copySkills({ source, dest });
+		// Re-running must not throw and must yield the same result.
+		const second = await copySkills({ source, dest });
+
+		expect(second).toEqual(first);
+		// The source skill files must survive a re-sync.
+		expect(await fixture.text('source-skills/build/SKILL.md')).toContain('name: build');
+		expect(await fixture.text('project/skills/build/SKILL.md')).toContain('name: build');
+	});
+});

src/commands/sync.ts

@@ -1,4 +1,4 @@
-import { readlink, symlink } from 'node:fs/promises';
+import { readlink, rm, symlink } from 'node:fs/promises';
 import { findPackageJSON } from 'node:module';
 import { dirname, isAbsolute, relative, resolve } from 'node:path';
 import { platform } from 'node:process';
@@ -41,7 +41,7 @@ interface SkillInfo {
 	description: string;
 }
 
-async function copySkills(options: { source: URL; dest: URL }): Promise<SkillInfo[]> {
+export async function copySkills(options: { source: URL; dest: URL }): Promise<SkillInfo[]> {
 	const { source, dest } = options;
 	const skills: SkillInfo[] = [];
 
@@ -60,12 +60,15 @@ async function copySkills(options: { source: URL; dest: URL }): Promise<SkillInf
 
 	for (const name of keep) {
 		const srcDir = new URL(`${name}/`, source);
-		const destDir = new URL(`${name}/`, dest);
 
-		await hfs.deleteAll(destDir);
+		// Use a path without a trailing slash. macOS rejects a trailing-slash link
+		// path with ENOENT, and `rm` on a trailing-slash directory symlink follows
+		// the link and deletes the source rather than unlinking the symlink itself.
+		const linkPath = resolve(destDirPath, name);
+		await rm(linkPath, { recursive: true, force: true });
 
 		const target = relative(destDirPath, fileURLToPath(srcDir));
-		await symlink(target, fileURLToPath(destDir), linkType);
+		await symlink(target, linkPath, linkType);
 
 		const content = await hfs.text(new URL('SKILL.md', srcDir));
 		if (content) {