From 8c228b8dbb9f910c3d90b459751e0fdde47569af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=82CHES?= Date: Sat, 21 Mar 2026 09:19:43 -0600 Subject: [PATCH] fix: robust node_modules symlink handling to prevent extension loading failures (#1762) The ensureNodeModulesSymlink function silently failed when: a real directory existed instead of a symlink, the symlink target moved after npm upgrade, or the symlink pointed to a deleted location. All three cases left extensions unable to resolve @gsd/* packages, making GSD completely non-functional. Three fixes: 1. Use lstatSync to detect real directories vs symlinks and handle each 2. Verify the symlink target actually exists before considering it valid 3. Log a warning on symlinkSync failure instead of silently swallowing 4. Move ensureNodeModulesSymlink before the early-return version check so it runs on EVERY launch, not just during resource syncs Closes #1688 Co-authored-by: Claude Opus 4.6 (1M context) --- src/resource-loader.ts | 32 ++++--- src/tests/node-modules-symlink.test.ts | 116 +++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 src/tests/node-modules-symlink.test.ts diff --git a/src/resource-loader.ts b/src/resource-loader.ts index f2b80a176..97327d50c 100644 --- a/src/resource-loader.ts +++ b/src/resource-loader.ts @@ -271,17 +271,27 @@ function ensureNodeModulesSymlink(agentDir: string): void { const gsdNodeModules = join(packageRoot, 'node_modules') try { - const existing = readlinkSync(agentNodeModules) - if (existing === gsdNodeModules) return // already correct - unlinkSync(agentNodeModules) + const stat = lstatSync(agentNodeModules) + + 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) + } else { + // Real directory (not a symlink) is blocking — remove it + rmSync(agentNodeModules, { recursive: true, force: true }) + } } catch { - // readlinkSync throws if path doesn't exist or isn't a symlink — both are fine + // lstatSync throws if path doesn't exist — that's fine, we'll create below } try { symlinkSync(gsdNodeModules, agentNodeModules, 'junction') - } catch { - // Non-fatal — worst case, extensions fall back to NODE_PATH via jiti + } 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}`) } } @@ -359,6 +369,11 @@ export function initResources(agentDir: string): void { // up even when the version/hash match causes the full sync to be skipped. pruneRemovedBundledExtensions(manifest, agentDir) + // Ensure ~/.gsd/agent/node_modules symlinks to GSD's node_modules on EVERY + // launch, not just during resource syncs. A stale/broken symlink makes ALL + // extensions fail to resolve @gsd/* packages, rendering GSD non-functional. + ensureNodeModulesSymlink(agentDir) + // Skip the full copy when both version AND content fingerprint match. // Version-only checks miss same-version content changes (npm link dev workflow, // hotfixes within a release). The content hash catches those at ~1ms cost. @@ -386,11 +401,6 @@ export function initResources(agentDir: string): void { // overwrite them (covers extensions, agents, and skills in one walk). makeTreeWritable(agentDir) - // Ensure ~/.gsd/agent/node_modules symlinks to GSD's node_modules so that - // native ESM import() calls from synced extension files can resolve @gsd/* - // packages via ancestor directory lookup. NODE_PATH only applies to CJS/jiti. - ensureNodeModulesSymlink(agentDir) - writeManagedResourceManifest(agentDir) ensureRegistryEntries(join(agentDir, 'extensions')) } diff --git a/src/tests/node-modules-symlink.test.ts b/src/tests/node-modules-symlink.test.ts new file mode 100644 index 000000000..a3c13f4cb --- /dev/null +++ b/src/tests/node-modules-symlink.test.ts @@ -0,0 +1,116 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, readlinkSync, rmSync, symlinkSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +test("initResources creates node_modules symlink in agent dir", async () => { + const { initResources } = await import("../resource-loader.ts"); + const tmp = mkdtempSync(join(tmpdir(), "gsd-symlink-")); + const fakeAgentDir = join(tmp, "agent"); + + try { + initResources(fakeAgentDir); + + const nodeModulesPath = join(fakeAgentDir, "node_modules"); + // Use lstatSync instead of existsSync — existsSync follows the symlink and + // returns false for dangling symlinks (e.g. in worktrees without node_modules) + let stat; + try { + stat = lstatSync(nodeModulesPath); + } catch { + assert.fail("node_modules symlink should exist after initResources"); + } + assert.equal(stat.isSymbolicLink(), true, "node_modules should be a symlink, not a real directory"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); + +test("initResources replaces a real directory blocking node_modules with a symlink", async () => { + const { initResources } = await import("../resource-loader.ts"); + const tmp = mkdtempSync(join(tmpdir(), "gsd-symlink-realdir-")); + const fakeAgentDir = join(tmp, "agent"); + + try { + // First call to set up agent dir structure + initResources(fakeAgentDir); + + const nodeModulesPath = join(fakeAgentDir, "node_modules"); + + // Remove the symlink and replace with a real directory + rmSync(nodeModulesPath, { recursive: true, force: true }); + mkdirSync(nodeModulesPath, { recursive: true }); + + const statBefore = lstatSync(nodeModulesPath); + assert.equal(statBefore.isSymbolicLink(), false, "should be a real directory before fix"); + assert.equal(statBefore.isDirectory(), true, "should be a real directory before fix"); + + // Second call should replace the real directory with a symlink + initResources(fakeAgentDir); + + const statAfter = lstatSync(nodeModulesPath); + assert.equal(statAfter.isSymbolicLink(), true, "real directory should be replaced with symlink"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); + +test("initResources replaces a stale symlink with a correct one", async () => { + const { initResources } = await import("../resource-loader.ts"); + const tmp = mkdtempSync(join(tmpdir(), "gsd-symlink-stale-")); + const fakeAgentDir = join(tmp, "agent"); + + try { + // First call to set up agent dir structure + initResources(fakeAgentDir); + + const nodeModulesPath = join(fakeAgentDir, "node_modules"); + const correctTarget = readlinkSync(nodeModulesPath); + + // Remove and replace with a stale symlink pointing to a non-existent path + rmSync(nodeModulesPath, { force: true }); + symlinkSync("/tmp/nonexistent-gsd-node-modules-" + Date.now(), nodeModulesPath); + + const staleTarget = readlinkSync(nodeModulesPath); + assert.notEqual(staleTarget, correctTarget, "stale symlink should point elsewhere"); + + // Second call should fix the stale symlink + initResources(fakeAgentDir); + + const fixedTarget = readlinkSync(nodeModulesPath); + assert.equal(fixedTarget, correctTarget, "stale symlink should be replaced with correct target"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); + +test("initResources replaces symlink whose target was deleted", async () => { + const { initResources } = await import("../resource-loader.ts"); + const tmp = mkdtempSync(join(tmpdir(), "gsd-symlink-missing-")); + const fakeAgentDir = join(tmp, "agent"); + + try { + initResources(fakeAgentDir); + + const nodeModulesPath = join(fakeAgentDir, "node_modules"); + 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) + rmSync(nodeModulesPath, { force: true }); + const deadTarget = join(tmp, "old-install", "node_modules"); + symlinkSync(deadTarget, nodeModulesPath); + + // The symlink itself exists but its target doesn't + assert.equal(lstatSync(nodeModulesPath).isSymbolicLink(), true); + assert.equal(existsSync(deadTarget), false, "dead target should not exist"); + + initResources(fakeAgentDir); + + const fixedTarget = readlinkSync(nodeModulesPath); + assert.equal(fixedTarget, correctTarget, "broken symlink should be replaced with correct target"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +});