diff --git a/.squad/agents/gnc/history.md b/.squad/agents/gnc/history.md index cbd545bef..af2c4a257 100644 --- a/.squad/agents/gnc/history.md +++ b/.squad/agents/gnc/history.md @@ -9,3 +9,6 @@ ### Node Version Requirements Node.js ≥20 required. Node 24+ enforces strict ESM resolution (no extensionless imports). cli-entry.ts has runtime check that warns about node:sqlite availability (≥22.5.0). + +### Dual-Layer ESM Fix (Issue #449) +Upgraded from single-layer session.js patch to dual-layer approach: (1) Inject `exports` field into vscode-jsonrpc@8.2.1 package.json at postinstall — this is the canonical fix that resolves ALL subpath imports at once, matching vscode-jsonrpc v9.x. (2) Keep session.js `.js` extension patch as defense-in-depth. Added `squad doctor` detection for both layers (checks vscode-jsonrpc exports field and copilot-sdk session.js import syntax). Runtime Module._resolveFilename patch in cli-entry.ts remains as Layer 3 for npx cache hits where postinstall never runs. diff --git a/.squad/decisions/inbox/gnc-node22-esm.md b/.squad/decisions/inbox/gnc-node22-esm.md new file mode 100644 index 000000000..6fa657a85 --- /dev/null +++ b/.squad/decisions/inbox/gnc-node22-esm.md @@ -0,0 +1,25 @@ +# Decision: Dual-Layer ESM Fix for vscode-jsonrpc + +**By:** GNC +**Issue:** #449 +**Date:** 2025-07-25 + +## What + +ESM module resolution for `vscode-jsonrpc` subpath imports now uses a dual-layer postinstall strategy: + +1. **Layer 1 (canonical):** Inject `exports` field into `vscode-jsonrpc@8.2.1/package.json` — fixes ALL subpath imports at once. +2. **Layer 2 (defense-in-depth):** Patch `copilot-sdk/dist/session.js` to add `.js` extension — kept as fallback. +3. **Layer 3 (runtime):** `cli-entry.ts` `Module._resolveFilename` intercept — handles npx cache hits where postinstall never runs. + +`squad doctor` now detects both Layer 1 and Layer 2 issues. + +## Why + +- vscode-jsonrpc@8.2.1 has no `exports` field; Node 22+ strict ESM rejects extensionless subpath imports. +- Patching individual files (session.js) doesn't scale — any future copilot-sdk file using `vscode-jsonrpc/node` would break. +- The exports field injection matches what vscode-jsonrpc v9.x ships, making it forward-compatible. + +## Impact + +All agents should know: if a user reports ESM import errors on Node 22/24, direct them to `squad doctor` which now detects and reports the exact issue. diff --git a/packages/squad-cli/scripts/patch-esm-imports.mjs b/packages/squad-cli/scripts/patch-esm-imports.mjs index de5bff9bc..1b399b5f7 100644 --- a/packages/squad-cli/scripts/patch-esm-imports.mjs +++ b/packages/squad-cli/scripts/patch-esm-imports.mjs @@ -1,18 +1,19 @@ #!/usr/bin/env node /** - * ESM Import Patcher for @github/copilot-sdk - * - * Patches broken ESM imports in @github/copilot-sdk for Node 24+ compatibility. - * - * Root cause: @github/copilot-sdk@0.1.32 has inconsistent ESM imports: - * - session.js imports "vscode-jsonrpc/node" (BROKEN - missing .js extension) - * - client.js imports "vscode-jsonrpc/node.js" (correct) - * - * Node 24+ enforces strict ESM resolution requiring .js extensions. - * This is a temporary workaround until copilot-sdk fixes upstream. - * - * Issue: bradygaster/squad#XXX + * ESM Import Patcher — dual-layer fix for Node 22/24+ compatibility + * + * Layer 1: Patch vscode-jsonrpc/package.json with `exports` field + * vscode-jsonrpc@8.2.1 has no `exports` field. Node 22+ strict ESM + * rejects subpath imports like 'vscode-jsonrpc/node' without it. + * Injecting the exports map from v9.x fixes ALL subpath imports at once. + * + * Layer 2: Patch @github/copilot-sdk session.js (defense-in-depth) + * copilot-sdk@0.1.32 imports 'vscode-jsonrpc/node' without .js extension. + * This layer ensures the import works even if Layer 1 somehow fails. + * + * Issue: bradygaster/squad#449 + * Upstream: https://github.com/github/copilot-sdk/issues/707 */ import { readFileSync, writeFileSync, existsSync } from 'fs'; @@ -21,57 +22,85 @@ import { fileURLToPath } from 'url'; const __dirname = dirname(fileURLToPath(import.meta.url)); +// Locations where npm workspaces / global install may place dependencies +const SEARCH_ROOTS = [ + join(__dirname, '..', 'node_modules'), // squad-cli local + join(__dirname, '..', '..', '..', 'node_modules'), // workspace root + join(__dirname, '..', '..'), // global install (sibling) +]; + /** - * Patch session.js in @github/copilot-sdk + * Layer 1 — Inject `exports` field into vscode-jsonrpc/package.json. + * This is the canonical fix: once the package has proper exports, Node's + * ESM resolver handles every subpath ('vscode-jsonrpc/node', '/browser', etc.) + * without needing per-file patches. */ -function patchCopilotSdk() { - // Try multiple possible locations (npm workspaces can hoist dependencies) - const possiblePaths = [ - // squad-cli package node_modules - join(__dirname, '..', 'node_modules', '@github', 'copilot-sdk', 'dist', 'session.js'), - // Workspace root node_modules (common with npm workspaces) - join(__dirname, '..', '..', '..', 'node_modules', '@github', 'copilot-sdk', 'dist', 'session.js'), - // Global install location (node_modules at parent of package) - join(__dirname, '..', '..', '@github', 'copilot-sdk', 'dist', 'session.js'), - ]; - - let sessionJsPath = null; - for (const path of possiblePaths) { - if (existsSync(path)) { - sessionJsPath = path; - break; +function patchVscodeJsonrpcExports() { + const exportsField = { + '.': { types: './lib/common/api.d.ts', default: './lib/node/main.js' }, + './node': { node: './lib/node/main.js', types: './lib/node/main.d.ts' }, + './node.js': { node: './lib/node/main.js', types: './lib/node/main.d.ts' }, + './browser': { types: './lib/browser/main.d.ts', browser: './lib/browser/main.js' }, + }; + + for (const root of SEARCH_ROOTS) { + const pkgPath = join(root, 'vscode-jsonrpc', 'package.json'); + if (!existsSync(pkgPath)) continue; + + try { + const raw = readFileSync(pkgPath, 'utf8'); + const pkg = JSON.parse(raw); + + if (pkg.exports) { + console.log('⏭️ vscode-jsonrpc already has exports field — skipping'); + return false; + } + + pkg.exports = exportsField; + writeFileSync(pkgPath, JSON.stringify(pkg, null, 2) + '\n', 'utf8'); + console.log('✅ Patched vscode-jsonrpc/package.json with exports field (Node 22/24+ ESM fix)'); + return true; + } catch (err) { + console.warn('⚠️ Failed to patch vscode-jsonrpc exports:', err.message); + return false; } } - if (!sessionJsPath) { - // copilot-sdk not installed (maybe optionalDependency or CI without install) - // This is fine - exit silently - return false; - } + return false; +} - try { - let content = readFileSync(sessionJsPath, 'utf8'); - - // Replace extensionless import with .js extension - const patched = content.replace( - /from\s+["']vscode-jsonrpc\/node["']/g, - 'from "vscode-jsonrpc/node.js"' - ); - - if (patched !== content) { - writeFileSync(sessionJsPath, patched, 'utf8'); - console.log('✅ Patched @github/copilot-sdk ESM imports for Node 24+ compatibility'); - return true; - } else { - // Already patched or upstream fixed +/** + * Layer 2 — Patch copilot-sdk session.js import (defense-in-depth). + * Rewrites extensionless 'vscode-jsonrpc/node' to 'vscode-jsonrpc/node.js'. + */ +function patchCopilotSdkSessionJs() { + for (const root of SEARCH_ROOTS) { + const sessionJsPath = join(root, '@github', 'copilot-sdk', 'dist', 'session.js'); + if (!existsSync(sessionJsPath)) continue; + + try { + const content = readFileSync(sessionJsPath, 'utf8'); + + const patched = content.replace( + /from\s+["']vscode-jsonrpc\/node["']/g, + 'from "vscode-jsonrpc/node.js"' + ); + + if (patched !== content) { + writeFileSync(sessionJsPath, patched, 'utf8'); + console.log('✅ Patched @github/copilot-sdk session.js ESM imports'); + return true; + } + return false; + } catch (err) { + console.warn('⚠️ Failed to patch copilot-sdk session.js:', err.message); return false; } - } catch (err) { - console.warn('⚠️ Failed to patch @github/copilot-sdk ESM imports:', err.message); - console.warn(' This may cause issues on Node 24+ if copilot-sdk loads.'); - return false; } + + return false; } -// Run patch -patchCopilotSdk(); \ No newline at end of file +// Run both layers +patchVscodeJsonrpcExports(); +patchCopilotSdkSessionJs(); \ No newline at end of file diff --git a/packages/squad-cli/src/cli/commands/doctor.ts b/packages/squad-cli/src/cli/commands/doctor.ts index d7c106779..298bac099 100644 --- a/packages/squad-cli/src/cli/commands/doctor.ts +++ b/packages/squad-cli/src/cli/commands/doctor.ts @@ -213,6 +213,101 @@ function checkDecisionsMd(squadDir: string): DoctorCheck { }; } +// ── ESM compatibility checks ──────────────────────────────────────── + +/** + * Check that vscode-jsonrpc has the `exports` field needed for Node 22/24+ + * strict ESM subpath resolution. Without it, `import('vscode-jsonrpc/node')` + * fails with ERR_PACKAGE_PATH_NOT_EXPORTED. + */ +function checkVscodeJsonrpcExports(cwd: string): DoctorCheck { + const possiblePaths = [ + path.join(cwd, 'node_modules', 'vscode-jsonrpc', 'package.json'), + path.join(cwd, 'packages', 'squad-cli', 'node_modules', 'vscode-jsonrpc', 'package.json'), + ]; + + for (const pkgPath of possiblePaths) { + if (!fileExists(pkgPath)) continue; + + const pkg = tryReadJson(pkgPath) as Record | undefined; + if (!pkg) { + return { + name: 'vscode-jsonrpc exports field', + status: 'fail', + message: 'package.json found but not valid JSON', + }; + } + + if (pkg['exports'] && typeof pkg['exports'] === 'object') { + const exports = pkg['exports'] as Record; + if (exports['./node']) { + return { + name: 'vscode-jsonrpc exports field', + status: 'pass', + message: 'exports field present with ./node subpath', + }; + } + } + + return { + name: 'vscode-jsonrpc exports field', + status: 'fail', + message: 'missing exports field — run postinstall or reinstall (see #449)', + }; + } + + return { + name: 'vscode-jsonrpc exports field', + status: 'warn', + message: 'vscode-jsonrpc not found in node_modules', + }; +} + +/** + * Check that @github/copilot-sdk session.js has the .js extension fix + * on its vscode-jsonrpc/node import (defense-in-depth behind the exports patch). + */ +function checkCopilotSdkSessionPatch(cwd: string): DoctorCheck { + const possiblePaths = [ + path.join(cwd, 'node_modules', '@github', 'copilot-sdk', 'dist', 'session.js'), + path.join(cwd, 'packages', 'squad-cli', 'node_modules', '@github', 'copilot-sdk', 'dist', 'session.js'), + ]; + + for (const sessionPath of possiblePaths) { + if (!fileExists(sessionPath)) continue; + + try { + const content = fs.readFileSync(sessionPath, 'utf8'); + + if (/from\s+["']vscode-jsonrpc\/node["']/.test(content)) { + return { + name: 'copilot-sdk session.js ESM patch', + status: 'fail', + message: 'session.js has extensionless vscode-jsonrpc/node import — run postinstall (see #449)', + }; + } + + return { + name: 'copilot-sdk session.js ESM patch', + status: 'pass', + message: 'session.js imports use .js extension', + }; + } catch { + return { + name: 'copilot-sdk session.js ESM patch', + status: 'warn', + message: 'could not read session.js', + }; + } + } + + return { + name: 'copilot-sdk session.js ESM patch', + status: 'warn', + message: '@github/copilot-sdk not found in node_modules', + }; +} + // ── public API ────────────────────────────────────────────────────── /** @@ -249,6 +344,10 @@ export async function runDoctor(cwd?: string): Promise { checks.push(checkDecisionsMd(squadDir)); } + // 10–11 ESM compatibility (Node 22/24+) + checks.push(checkVscodeJsonrpcExports(resolvedCwd)); + checks.push(checkCopilotSdkSessionPatch(resolvedCwd)); + return checks; } diff --git a/test/cli/doctor.test.ts b/test/cli/doctor.test.ts index 09380edbe..f7869dd0f 100644 --- a/test/cli/doctor.test.ts +++ b/test/cli/doctor.test.ts @@ -55,6 +55,9 @@ describe('squad doctor', () => { expect(checks.some((c: DoctorCheck) => c.name === 'agents/ directory exists' && c.status === 'pass')).toBe(true); expect(checks.some((c: DoctorCheck) => c.name === 'casting/registry.json exists' && c.status === 'pass')).toBe(true); expect(checks.some((c: DoctorCheck) => c.name === 'decisions.md exists' && c.status === 'pass')).toBe(true); + // ESM checks return 'warn' (not fail) when node_modules absent from test dir + expect(checks.some((c: DoctorCheck) => c.name === 'vscode-jsonrpc exports field')).toBe(true); + expect(checks.some((c: DoctorCheck) => c.name === 'copilot-sdk session.js ESM patch')).toBe(true); }); it('reports failures on an empty directory', async () => { @@ -62,8 +65,8 @@ describe('squad doctor', () => { const squadDirCheck = checks.find((c: DoctorCheck) => c.name === '.squad/ directory exists'); expect(squadDirCheck?.status).toBe('fail'); - // When .squad/ is missing the file checks are skipped — only one check - expect(checks.length).toBe(1); + // When .squad/ is missing the file checks are skipped — only .squad/ + 2 ESM checks + expect(checks.length).toBe(3); }); it('detects remote mode from config.json with teamRoot', async () => {