diff --git a/src/resource-loader.ts b/src/resource-loader.ts index 901d8e1b1..1309578e2 100644 --- a/src/resource-loader.ts +++ b/src/resource-loader.ts @@ -2,7 +2,7 @@ import { DefaultResourceLoader, sortExtensionPaths } from '@gsd/pi-coding-agent' import { createHash } from 'node:crypto' import { homedir } from 'node:os' import { chmodSync, copyFileSync, cpSync, existsSync, lstatSync, mkdirSync, openSync, closeSync, readFileSync, readlinkSync, readdirSync, rmSync, statSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs' -import { dirname, join, relative, resolve } from 'node:path' +import { basename, dirname, join, relative, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import { compareSemver } from './update-check.js' import { discoverExtensionEntryPaths } from './extension-discovery.js' @@ -287,33 +287,144 @@ function copyDirRecursive(src: string, dest: string): void { * ~/.gsd/agent/extensions/ have no ancestor node_modules, so imports of * @gsd/* packages fail. The symlink makes Node's standard resolution find * them without requiring every call site to use jiti. + * + * Layout differences by install method: + * - Source/monorepo: packageRoot/node_modules has everything → simple symlink + * - npm/bun global: deps hoisted to dirname(packageRoot), including @gsd/* → simple symlink + * - pnpm global: external deps hoisted, but @gsd/* stays in packageRoot/node_modules + * → merged directory with symlinks from both roots (#3529, #3564) */ function ensureNodeModulesSymlink(agentDir: string): void { const agentNodeModules = join(agentDir, 'node_modules') - const gsdNodeModules = join(packageRoot, 'node_modules') + const internalNodeModules = join(packageRoot, 'node_modules') + const hoistedNodeModules = dirname(packageRoot) + const isGlobalInstall = basename(hoistedNodeModules) === 'node_modules' + if (!isGlobalInstall) { + // Source/monorepo: internal node_modules has everything + reconcileSymlink(agentNodeModules, internalNodeModules) + return + } + + // Global install: check if workspace scopes (@gsd/*) are hoisted. + // npm/bun hoist everything; pnpm keeps workspace packages internal. + if (!hasMissingWorkspaceScopes(hoistedNodeModules, internalNodeModules)) { + // Everything is hoisted — simple symlink to parent node_modules + reconcileSymlink(agentNodeModules, hoistedNodeModules) + return + } + + // pnpm-style layout: create a real directory merging both roots + reconcileMergedNodeModules(agentNodeModules, hoistedNodeModules, internalNodeModules) +} + +/** Check if any @gsd* scopes exist in internal but not in hoisted node_modules */ +function hasMissingWorkspaceScopes(hoisted: string, internal: string): boolean { + if (!existsSync(internal)) return false try { - const stat = lstatSync(agentNodeModules) + for (const entry of readdirSync(internal, { withFileTypes: true })) { + if (entry.isDirectory() && entry.name.startsWith('@gsd') && + !existsSync(join(hoisted, entry.name))) { + return true + } + } + } catch { /* non-fatal */ } + return false +} +/** Ensure a symlink at `link` points to `target`, fixing stale/wrong entries */ +function reconcileSymlink(link: string, target: string): void { + try { + const stat = lstatSync(link) if (stat.isSymbolicLink()) { - const existing = readlinkSync(agentNodeModules) - // Symlink exists — verify it points to the correct, existing target - if (existing === gsdNodeModules && existsSync(agentNodeModules)) return // correct and target exists - // Stale or wrong target — remove and recreate - unlinkSync(agentNodeModules) + const existing = readlinkSync(link) + if (existing === target && existsSync(link)) return // correct and target exists + unlinkSync(link) } else { - // Real directory (not a symlink) is blocking — remove it - rmSync(agentNodeModules, { recursive: true, force: true }) + // Real directory (or merged dir from previous pnpm fix) — remove it + rmSync(link, { recursive: true, force: true }) } } catch { - // lstatSync throws if path doesn't exist — that's fine, we'll create below + // lstatSync throws if path doesn't exist — fine, we'll create below } try { - symlinkSync(gsdNodeModules, agentNodeModules, 'junction') + symlinkSync(target, link, 'junction') } catch (err) { - // This failure makes GSD non-functional — extensions can't resolve @gsd/* packages - console.error(`[gsd] WARN: Failed to symlink ${agentNodeModules} → ${gsdNodeModules}: ${err instanceof Error ? err.message : err}`) + console.error(`[gsd] WARN: Failed to symlink ${link} → ${target}: ${err instanceof Error ? err.message : err}`) + } +} + +/** + * Create a real node_modules directory containing symlinks from both the + * hoisted root (external deps) and internal root (@gsd/* workspace packages). + * Used for pnpm global installs where @gsd/* isn't hoisted. + */ +function reconcileMergedNodeModules( + agentNodeModules: string, + hoisted: string, + internal: string, +): void { + // Fast path: if already merged for this packageRoot + same directory contents, skip. + // The fingerprint includes entry names from both roots so `pnpm add/remove` triggers rebuild. + const marker = join(agentNodeModules, '.gsd-merged') + const fingerprint = mergedFingerprint(hoisted, internal) + try { + if (existsSync(marker) && readFileSync(marker, 'utf-8').trim() === fingerprint) return + } catch { /* rebuild */ } + + // Remove any existing symlink or stale merged directory + try { + const stat = lstatSync(agentNodeModules) + if (stat.isSymbolicLink()) { + unlinkSync(agentNodeModules) + } else { + rmSync(agentNodeModules, { recursive: true, force: true }) + } + } catch { /* doesn't exist */ } + + mkdirSync(agentNodeModules, { recursive: true }) + + let linkedCount = 0 + + // Symlink entries from the hoisted node_modules (external deps) + try { + for (const entry of readdirSync(hoisted, { withFileTypes: true })) { + // Skip the gsd-pi package itself and dotfiles + if (entry.name === basename(packageRoot)) continue + if (entry.name.startsWith('.')) continue + try { symlinkSync(join(hoisted, entry.name), join(agentNodeModules, entry.name)); linkedCount++ } catch { /* skip individual */ } + } + } catch (err) { + console.error(`[gsd] WARN: Failed to read hoisted node_modules at ${hoisted}: ${err instanceof Error ? err.message : err}`) + } + + // Overlay @gsd* workspace scopes from internal node_modules + try { + for (const entry of readdirSync(internal, { withFileTypes: true })) { + if (!entry.name.startsWith('@gsd')) continue + const link = join(agentNodeModules, entry.name) + try { lstatSync(link); unlinkSync(link) } catch { /* didn't exist */ } + try { symlinkSync(join(internal, entry.name), link); linkedCount++ } catch { /* skip individual */ } + } + } catch (err) { + console.error(`[gsd] WARN: Failed to read internal node_modules at ${internal}: ${err instanceof Error ? err.message : err}`) + } + + // Only stamp marker if we actually linked something — avoids caching a broken state + if (linkedCount > 0) { + try { writeFileSync(marker, fingerprint) } catch { /* non-fatal */ } + } +} + +/** Build a cache fingerprint from packageRoot + sorted entry names of both directories */ +function mergedFingerprint(hoisted: string, internal: string): string { + try { + const h = readdirSync(hoisted).sort().join(',') + const i = readdirSync(internal).sort().join(',') + return `${packageRoot}\n${h}\n${i}` + } catch { + return packageRoot // fallback: at least invalidate on version change } } diff --git a/src/tests/node-modules-symlink.test.ts b/src/tests/node-modules-symlink.test.ts index ef0bdf724..a22f10910 100644 --- a/src/tests/node-modules-symlink.test.ts +++ b/src/tests/node-modules-symlink.test.ts @@ -1,9 +1,15 @@ -import test from "node:test"; +/** + * Tests for ensureNodeModulesSymlink — covers symlink reconciliation for + * source installs (#3529) and pnpm-style merged node_modules (#3564). + */ +import { test } from "node:test"; import assert from "node:assert/strict"; -import { existsSync, lstatSync, mkdirSync, mkdtempSync, readlinkSync, rmSync, symlinkSync, unlinkSync } from "node:fs"; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, readdirSync, rmSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; +// --- Integration tests via initResources (source/monorepo path) --- + test("initResources creates node_modules symlink in agent dir", async (t) => { const { initResources } = await import("../resource-loader.ts"); const tmp = mkdtempSync(join(tmpdir(), "gsd-symlink-")); @@ -30,7 +36,6 @@ test("initResources replaces a real directory blocking node_modules with a symli const fakeAgentDir = join(tmp, "agent"); t.after(() => rmSync(tmp, { recursive: true, force: true })); - // First call to set up agent dir structure initResources(fakeAgentDir); const nodeModulesPath = join(fakeAgentDir, "node_modules"); @@ -56,7 +61,6 @@ test("initResources replaces a stale symlink with a correct one", async (t) => { const fakeAgentDir = join(tmp, "agent"); t.after(() => rmSync(tmp, { recursive: true, force: true })); - // First call to set up agent dir structure initResources(fakeAgentDir); const nodeModulesPath = join(fakeAgentDir, "node_modules"); @@ -88,7 +92,6 @@ test("initResources replaces symlink whose target was deleted", async (t) => { const correctTarget = readlinkSync(nodeModulesPath); // Create a symlink that points to a path that doesn't exist - // (simulates the case where npm upgrade moved the package location) unlinkSync(nodeModulesPath); const deadTarget = join(tmp, "old-install", "node_modules"); symlinkSync(deadTarget, nodeModulesPath); @@ -102,3 +105,135 @@ test("initResources replaces symlink whose target was deleted", async (t) => { const fixedTarget = readlinkSync(nodeModulesPath); assert.equal(fixedTarget, correctTarget, "broken symlink should be replaced with correct target"); }); + +// --- Unit tests for pnpm-style merged node_modules (#3564) --- +// These simulate the filesystem layout without going through initResources, +// since packageRoot is fixed at module load time. + +test("pnpm layout: merged node_modules contains entries from both hoisted and internal", (t) => { + // Simulate pnpm global layout: + // hoisted/node_modules/ + // yaml/ ← external dep + // @sinclair/ ← external scoped dep + // gsd-pi/ ← package root + // node_modules/ + // @gsd/ ← workspace scope (NOT hoisted) + // @gsd-build/ ← workspace scope (NOT hoisted) + const tmp = mkdtempSync(join(tmpdir(), "gsd-pnpm-merge-")); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + const hoisted = join(tmp, "node_modules"); + const pkgRoot = join(hoisted, "gsd-pi"); + const internal = join(pkgRoot, "node_modules"); + const agentNodeModules = join(tmp, "agent", "node_modules"); + + // Create hoisted entries (external deps) + mkdirSync(join(hoisted, "yaml"), { recursive: true }); + mkdirSync(join(hoisted, "@sinclair", "typebox"), { recursive: true }); + mkdirSync(join(hoisted, "@anthropic-ai", "sdk"), { recursive: true }); + mkdirSync(pkgRoot, { recursive: true }); + + // Create internal entries (workspace packages) + mkdirSync(join(internal, "@gsd", "pi-ai"), { recursive: true }); + mkdirSync(join(internal, "@gsd", "pi-coding-agent"), { recursive: true }); + mkdirSync(join(internal, "@gsd-build", "core"), { recursive: true }); + + // Create merged directory manually (simulating what reconcileMergedNodeModules does) + mkdirSync(agentNodeModules, { recursive: true }); + + // Link hoisted entries (skip gsd-pi itself and dotfiles) + for (const entry of readdirSync(hoisted, { withFileTypes: true })) { + if (entry.name === "gsd-pi" || entry.name.startsWith(".")) continue; + symlinkSync(join(hoisted, entry.name), join(agentNodeModules, entry.name)); + } + + // Overlay @gsd* workspace scopes from internal (these take precedence) + for (const entry of readdirSync(internal, { withFileTypes: true })) { + if (!entry.name.startsWith("@gsd")) continue; + const link = join(agentNodeModules, entry.name); + try { lstatSync(link); unlinkSync(link); } catch { /* didn't exist */ } + symlinkSync(join(internal, entry.name), link); + } + + // Verify: external deps resolve through hoisted symlinks + assert.ok(existsSync(join(agentNodeModules, "yaml")), "yaml should resolve"); + assert.ok(existsSync(join(agentNodeModules, "@sinclair")), "@sinclair should resolve"); + assert.ok(existsSync(join(agentNodeModules, "@anthropic-ai")), "@anthropic-ai should resolve"); + + // Verify: workspace packages resolve through internal symlinks + assert.ok(existsSync(join(agentNodeModules, "@gsd")), "@gsd should resolve"); + assert.ok(existsSync(join(agentNodeModules, "@gsd", "pi-ai")), "@gsd/pi-ai should resolve"); + assert.ok(existsSync(join(agentNodeModules, "@gsd-build")), "@gsd-build should resolve"); + + // Verify: gsd-pi itself is NOT symlinked (it's the package root, not a dep) + assert.ok(!existsSync(join(agentNodeModules, "gsd-pi")), "gsd-pi should not be in merged dir"); + + // Verify: @gsd points to internal, not hoisted (internal takes precedence) + const gsdTarget = readlinkSync(join(agentNodeModules, "@gsd")); + assert.equal(gsdTarget, join(internal, "@gsd"), "@gsd should point to internal node_modules"); +}); + +test("hasMissingWorkspaceScopes detects pnpm layout", (t) => { + const tmp = mkdtempSync(join(tmpdir(), "gsd-pnpm-detect-")); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + const hoisted = join(tmp, "hoisted"); + const internal = join(tmp, "internal"); + + // npm-style: @gsd exists in both hoisted and internal + mkdirSync(join(hoisted, "@gsd"), { recursive: true }); + mkdirSync(join(internal, "@gsd"), { recursive: true }); + + // Inline the detection logic for testing + const hasMissing = (h: string, i: string): boolean => { + if (!existsSync(i)) return false; + for (const entry of readdirSync(i, { withFileTypes: true })) { + if (entry.isDirectory() && entry.name.startsWith("@gsd") && + !existsSync(join(h, entry.name))) { + return true; + } + } + return false; + }; + + assert.equal(hasMissing(hoisted, internal), false, "npm-style: no missing scopes"); + + // pnpm-style: @gsd-build only in internal + mkdirSync(join(internal, "@gsd-build"), { recursive: true }); + assert.equal(hasMissing(hoisted, internal), true, "pnpm-style: @gsd-build missing from hoisted"); +}); + +test("merged node_modules marker uses fingerprint including directory entries", (t) => { + const tmp = mkdtempSync(join(tmpdir(), "gsd-pnpm-marker-")); + t.after(() => rmSync(tmp, { recursive: true, force: true })); + + // Simulate two directories with known entries + const hoisted = join(tmp, "hoisted"); + const internal = join(tmp, "internal"); + mkdirSync(join(hoisted, "yaml"), { recursive: true }); + mkdirSync(join(hoisted, "@sinclair"), { recursive: true }); + mkdirSync(join(internal, "@gsd"), { recursive: true }); + + // Build fingerprint the same way the production code does + const h = readdirSync(hoisted).sort().join(","); + const i = readdirSync(internal).sort().join(","); + const fakePackageRoot = "/usr/lib/node_modules/gsd-pi"; + const fingerprint = `${fakePackageRoot}\n${h}\n${i}`; + + const agentNodeModules = join(tmp, "agent", "node_modules"); + mkdirSync(agentNodeModules, { recursive: true }); + const marker = join(agentNodeModules, ".gsd-merged"); + writeFileSync(marker, fingerprint); + + // Verify fingerprint contains all three components + const stored = readFileSync(marker, "utf-8").trim(); + assert.ok(stored.includes(fakePackageRoot), "fingerprint includes packageRoot"); + assert.ok(stored.includes("@sinclair"), "fingerprint includes hoisted entries"); + assert.ok(stored.includes("@gsd"), "fingerprint includes internal entries"); + + // Verify fingerprint changes when a new package is added + mkdirSync(join(hoisted, "new-package"), { recursive: true }); + const h2 = readdirSync(hoisted).sort().join(","); + const fingerprint2 = `${fakePackageRoot}\n${h2}\n${i}`; + assert.notEqual(fingerprint, fingerprint2, "fingerprint should change when deps change"); +});