fix(sync): link skills through the project's node_modules (#68)

* test(sync): cover stable skill link targets

* fix(sync): link skills through the project's node_modules

import.meta.url resolves to the real path, which under pnpm is a hashed
node_modules/.pnpm/<hash>/ directory. Links into it dangle once a
reinstall changes the hash.

Author: natemoo-re

.changeset/sync-stable-links.md

@@ -0,0 +1,5 @@
+---
+'@bomb.sh/tools': patch
+---
+
+Fixes `bsh sync` creating skill symlinks that break after reinstalling dependencies with pnpm

src/commands/sync.test.ts

@@ -1,8 +1,8 @@
-import { lstat, readlink } from 'node:fs/promises';
+import { lstat, readlink, realpath, symlink } from 'node:fs/promises';
 import { fileURLToPath } from 'node:url';
 import { describe, it, expect } from 'vitest';
 import { createFixture, createMocks } from '../test-utils/index.ts';
-import { copySkills, findParentPackage, updateAgentsMd } from './sync.ts';
+import { copySkills, findParentPackage, resolveSkillsSource, updateAgentsMd } from './sync.ts';
 
 describe('copySkills', () => {
 	it('symlinks each skill into the destination', async () => {
@@ -82,6 +82,34 @@ describe('copySkills', () => {
 			]),
 		);
 	});
+
+	it('prunes stale links that target the real path of the source', async () => {
+		const fixture = await createFixture({
+			store: {
+				skills: {
+					build: { 'SKILL.md': '---\nname: build\ndescription: Build.\n---\n' },
+				},
+			},
+			project: {
+				tools: ({ symlink }) => symlink('../store'),
+				skills: {},
+			},
+		});
+		// Left behind by an older sync that linked through the real path.
+		await symlink(
+			`${await realpath(fileURLToPath(new URL('store/skills/', fixture.root)))}/removed`,
+			fileURLToPath(new URL('project/skills/removed', fixture.root)),
+		);
+
+		await copySkills({
+			source: new URL('project/tools/skills/', fixture.root),
+			dest: new URL('project/skills/', fixture.root),
+		});
+
+		await expect(
+			lstat(fileURLToPath(new URL('project/skills/removed', fixture.root))),
+		).rejects.toThrow();
+	});
 });
 
 describe('updateAgentsMd', () => {
@@ -104,6 +132,30 @@ describe('updateAgentsMd', () => {
 	});
 });
 
+describe('resolveSkillsSource', () => {
+	it('prefers the project node_modules path over the real store path', async () => {
+		const fixture = await createFixture({
+			store: { tools: { skills: { build: { 'SKILL.md': '' } } } },
+			project: {
+				'node_modules/@bomb.sh/tools': ({ symlink }) => symlink('../../../store/tools'),
+			},
+		});
+		const root = new URL('project/', fixture.root);
+		const fallback = new URL('store/tools/skills/', fixture.root);
+
+		expect((await resolveSkillsSource(root, fallback)).href).toBe(
+			new URL('node_modules/@bomb.sh/tools/skills/', root).href,
+		);
+	});
+
+	it('falls back when the project has no linked @bomb.sh/tools', async () => {
+		const fixture = await createFixture({ project: {} });
+		const fallback = new URL('store/tools/skills/', fixture.root);
+
+		expect(await resolveSkillsSource(new URL('project/', fixture.root), fallback)).toBe(fallback);
+	});
+});
+
 describe('findParentPackage', () => {
 	it('resolves the project root from INIT_CWD, not from this package', async () => {
 		const fixture = await createFixture({

src/commands/sync.ts

@@ -1,4 +1,4 @@
-import { readlink, rm, symlink } from 'node:fs/promises';
+import { readlink, realpath, rm, symlink } from 'node:fs/promises';
 import { findPackageJSON } from 'node:module';
 import { cwd, env, platform } from 'node:process';
 import { fileURLToPath, pathToFileURL } from 'node:url';
@@ -22,7 +22,7 @@ export async function sync(_ctx: CommandContext): Promise<void> {
 	}
 
 	const root = new URL('./', pathToFileURL(parentPkg));
-	const source = new URL('../../skills/', import.meta.url);
+	const source = await resolveSkillsSource(root, new URL('../../skills/', import.meta.url));
 
 	if (!(await hfs.isDirectory(source))) {
 		console.error('Could not locate bundled skills directory.');
@@ -36,6 +36,17 @@ export async function sync(_ctx: CommandContext): Promise<void> {
 	console.info(`Synced ${skills.length} skills to skills/`);
 }
 
+/**
+ * Prefer linking through the project's `node_modules/@bomb.sh/tools` over
+ * `import.meta.url`, which resolves to the real path. Under pnpm that is a
+ * versioned `node_modules/.pnpm/<hash>/` directory, so links into it dangle
+ * once a reinstall changes the hash.
+ */
+export async function resolveSkillsSource(root: URL, fallback: URL): Promise<URL> {
+	const linked = new URL('node_modules/@bomb.sh/tools/skills/', root);
+	return (await hfs.isDirectory(linked)) ? linked : fallback;
+}
+
 interface SkillInfo {
 	name: string;
 	description: string;
@@ -89,14 +100,17 @@ async function pruneStaleLinks(options: {
 	const { dest, source, keep } = options;
 	if (!(await hfs.isDirectory(dest))) return;
 
+	// Links from older syncs may target the real path rather than `source`.
+	const sources = [source.href, `${pathToFileURL(await realpath(source)).href}/`];
+
 	for await (const entry of hfs.list(dest)) {
 		if (!entry.isSymlink) continue;
 		if (keep.has(entry.name)) continue;
 
 		const linkPath = fileURLToPath(new URL(entry.name, dest));
 		try {
-			const target = await readlink(linkPath);
-			if (resolveLinkTarget(dest, target).href.startsWith(source.href)) {
+			const { href } = resolveLinkTarget(dest, await readlink(linkPath));
+			if (sources.some((s) => href.startsWith(s))) {
 				await hfs.deleteAll(linkPath);
 			}
 		} catch {