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) <noreply@anthropic.com>
This commit is contained in:
TÂCHES 2026-03-21 09:19:43 -06:00 committed by GitHub
parent 182e4a5f85
commit 8c228b8dbb
2 changed files with 137 additions and 11 deletions

View file

@ -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'))
}

View file

@ -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 });
}
});