Merge pull request #3564 from Tibsfox/fix/node-modules-symlink-target
fix(cli): resolve hoisted node_modules for global installs
This commit is contained in:
Jeremy McSpadden
GitHub
commit
c900e1004a
2 changed files with 265 additions and 19 deletions
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue