From ab2e7d8768d1ab60559a3697f7b2b7a2808cc02f Mon Sep 17 00:00:00 2001 From: yaojin Date: Tue, 18 Aug 2026 19:28:40 +0800 Subject: [PATCH 1/3] fix: resolve profile-installed plugins from cordis-plugin-loader Plugins installed via the plugin market (dsh-market) go into the profile's node_modules (e.g. DSH_HOME/profiles/web/node_modules), but the cordis-plugin-loader resolves packages from the app bundle's own node_modules directory. After packaging, these two paths are different, so both hot-mount and post-restart plugin loading fail with 'Cannot find package' errors. This change injects the profile's node_modules directories into the harness Node.js process via two mechanisms: 1. NODE_PATH + Module.globalPaths for CommonJS resolution 2. A custom ESM resolve hook (registered via --import) that falls back to profile node_modules when the default resolution fails The ESM hook uses a synthetic parent URL inside each profile's node_modules directory so that Node's standard resolver (including package.json exports, conditions, subpaths) handles the actual resolution correctly. Closes #73 --- build/profile-esm-resolver.mjs | 42 +++++++++++++++++ build/profile-module-paths.mjs | 49 ++++++++++++++++++++ package.json | 8 ++++ src/main/index.ts | 1 + src/main/runtime/harness-runtime.ts | 14 ++++-- test/profile-module-paths.test.ts | 72 +++++++++++++++++++++++++++++ test/runtime.test.ts | 47 +++++++++++++++++++ 7 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 build/profile-esm-resolver.mjs create mode 100644 build/profile-module-paths.mjs create mode 100644 test/profile-module-paths.test.ts diff --git a/build/profile-esm-resolver.mjs b/build/profile-esm-resolver.mjs new file mode 100644 index 00000000..b58250cc --- /dev/null +++ b/build/profile-esm-resolver.mjs @@ -0,0 +1,42 @@ +import { pathToFileURL } from 'node:url' + +let profileNodeModules = [] + +export function initialize(data) { + if (data?.profileNodeModules) { + profileNodeModules = data.profileNodeModules + } +} + +export async function resolve(specifier, context, nextResolve) { + if (profileNodeModules.length === 0) { + return nextResolve(specifier, context) + } + + const isBareSpecifier = !specifier.startsWith('.') && + !specifier.startsWith('/') && + !specifier.startsWith('file://') && + !specifier.startsWith('node:') && + !specifier.includes('://') + + if (!isBareSpecifier) { + return nextResolve(specifier, context) + } + + try { + return await nextResolve(specifier, context) + } catch (defaultError) { + for (const nodeModulesDir of profileNodeModules) { + const syntheticParent = pathToFileURL(nodeModulesDir + '/.resolve-anchor/anchor.js').href + try { + return await nextResolve(specifier, { + ...context, + parentURL: syntheticParent + }) + } catch { + // try next profile + } + } + throw defaultError + } +} diff --git a/build/profile-module-paths.mjs b/build/profile-module-paths.mjs new file mode 100644 index 00000000..0e72f46e --- /dev/null +++ b/build/profile-module-paths.mjs @@ -0,0 +1,49 @@ +import { readdirSync, existsSync } from 'node:fs' +import { createRequire, register } from 'node:module' +import { join, delimiter, dirname } from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' + +const dshHome = process.env.DSH_HOME +const __filename = fileURLToPath(import.meta.url) +const __dirname = dirname(__filename) + +function collectProfileNodeModules(home) { + if (!home) return [] + const profilesDir = join(home, 'profiles') + try { + return readdirSync(profilesDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => join(profilesDir, entry.name, 'node_modules')) + .filter((dir) => existsSync(dir)) + } catch { + return [] + } +} + +const profileNodeModules = collectProfileNodeModules(dshHome) + +if (profileNodeModules.length > 0) { + const paths = profileNodeModules.join(delimiter) + if (process.env.NODE_PATH) { + process.env.NODE_PATH = process.env.NODE_PATH + delimiter + paths + } else { + process.env.NODE_PATH = paths + } + + try { + const require = createRequire(import.meta.url) + const Module = require('node:module') + if (Module.globalPaths) { + for (const dir of profileNodeModules) { + if (!Module.globalPaths.includes(dir)) { + Module.globalPaths.push(dir) + } + } + } + } catch { + // Best effort for CJS fallback + } + + const resolverUrl = pathToFileURL(join(__dirname, 'profile-esm-resolver.mjs')).href + register(resolverUrl, { data: { profileNodeModules } }) +} diff --git a/package.json b/package.json index 07c2c546..b5e712cc 100644 --- a/package.json +++ b/package.json @@ -111,6 +111,14 @@ "from": "build/harness-node-entry.mjs", "to": "harness-node-entry.mjs" }, + { + "from": "build/profile-module-paths.mjs", + "to": "profile-module-paths.mjs" + }, + { + "from": "build/profile-esm-resolver.mjs", + "to": "profile-esm-resolver.mjs" + }, { "from": "build/dsh-desktop.patch.yml", "to": "dsh-desktop.patch.yml" diff --git a/src/main/index.ts b/src/main/index.ts index d118507e..43be14d1 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -457,6 +457,7 @@ async function bootstrap(): Promise { dshEntryPath: dshEntryPath(), nodeExecutablePath: bundledNodePath(), nodeEntryPath: harnessNodeEntryPath(), + profileModulePathsPath: desktopResourcePath('profile-module-paths.mjs'), dshPatchPath: desktopResourcePath('dsh-desktop.patch.yml'), dshHome: join(app.getPath('userData'), 'harness'), logPath: join(app.getPath('logs'), 'harness.log'), diff --git a/src/main/runtime/harness-runtime.ts b/src/main/runtime/harness-runtime.ts index 7555bdbb..db3d4f08 100644 --- a/src/main/runtime/harness-runtime.ts +++ b/src/main/runtime/harness-runtime.ts @@ -9,6 +9,7 @@ export interface HarnessRuntimeOptions { dshEntryPath: string nodeExecutablePath: string nodeEntryPath: string + profileModulePathsPath: string dshPatchPath: string dshHome: string logPath: string @@ -36,7 +37,8 @@ export function buildHarnessSpawnOptions( launchDirectory: string, dshHome: string, platform: NodeJS.Platform = process.platform, - environment: NodeJS.ProcessEnv = process.env + environment: NodeJS.ProcessEnv = process.env, + profileModulePathsPath?: string ): SpawnOptionsWithoutStdio { const { ELECTRON_RUN_AS_NODE: _runAsNode, ...parentEnvironment } = environment const pathKey = platform === 'win32' ? 'Path' : 'PATH' @@ -47,6 +49,7 @@ export function buildHarnessSpawnOptions( ...parentEnvironment, DSH_HOME: dshHome, NO_COLOR: '1', + ...(profileModulePathsPath ? { DSH_DESKTOP_PROFILE_MODULE_PATHS: profileModulePathsPath } : {}), [pathKey]: environment[pathKey] ?? environment.PATH ?? '' }, stdio: ['pipe', 'pipe', 'pipe'], @@ -58,10 +61,12 @@ export function buildNodeArguments( nodeEntryPath: string, dshEntryPath: string, port: number, - patchPath?: string + patchPath?: string, + profileModulePathsPath?: string ): string[] { return [ '--expose-internals', + ...(profileModulePathsPath ? ['--import', profileModulePathsPath] : []), nodeEntryPath, dshEntryPath, ...buildHarnessArguments(port, patchPath) @@ -121,7 +126,8 @@ export class HarnessRuntime { this.options.nodeEntryPath, this.options.dshEntryPath, port, - this.options.dshPatchPath + this.options.dshPatchPath, + this.options.profileModulePathsPath ) const startupTimeoutMs = this.options.startupTimeoutMs ?? (process.platform === 'win32' ? 120_000 : 45_000) @@ -136,7 +142,7 @@ export class HarnessRuntime { child = this.options.launchProcess( this.options.nodeExecutablePath, args, - buildHarnessSpawnOptions(launchDirectory, this.options.dshHome) + buildHarnessSpawnOptions(launchDirectory, this.options.dshHome, undefined, undefined, this.options.profileModulePathsPath) ) } catch (error) { const message = error instanceof Error ? error.message : String(error) diff --git a/test/profile-module-paths.test.ts b/test/profile-module-paths.test.ts new file mode 100644 index 00000000..7605bba3 --- /dev/null +++ b/test/profile-module-paths.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' +import { mkdir, writeFile, rm } from 'node:fs/promises' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import { pathToFileURL } from 'node:url' + +describe('Profile module paths resolution', () => { + it('resolves bare specifiers from profile node_modules via synthetic parent', async () => { + const testDir = join(tmpdir(), `dsh-desktop-resolver-${Date.now()}`) + const profileNodeModules = join(testDir, 'profiles', 'web', 'node_modules') + const packageDir = join(profileNodeModules, 'test-profile-pkg') + + await mkdir(packageDir, { recursive: true }) + await writeFile( + join(packageDir, 'package.json'), + JSON.stringify({ name: 'test-profile-pkg', version: '1.0.0', main: 'index.js', type: 'module' }) + ) + await writeFile(join(packageDir, 'index.js'), 'export const value = 42;\n') + + try { + const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href + const mod = await import(resolverUrl) + + mod.initialize({ profileNodeModules: [profileNodeModules] }) + + const fakeParent = 'file:///app/somewhere/module.js' + const nextResolve = (specifier: string, _context: unknown) => { + if (specifier.includes('test-profile-pkg')) { + return Promise.resolve({ url: pathToFileURL(join(packageDir, 'index.js')).href, shortCircuit: true }) + } + return Promise.reject(new Error(`not found: ${specifier}`)) + } + + const result = await mod.resolve( + 'test-profile-pkg', + { parentURL: fakeParent, conditions: ['import'] }, + nextResolve + ) + + expect(result.url).toContain('test-profile-pkg') + expect(result.url).toContain('index.js') + } finally { + await rm(testDir, { recursive: true, force: true }) + } + }) + + it('passes through non-bare specifiers unchanged', async () => { + const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href + const mod = await import(resolverUrl) + + mod.initialize({ profileNodeModules: ['/fake/profile/node_modules'] }) + + const fakeParent = 'file:///app/module.js' + let capturedSpecifier = '' + const nextResolve = (specifier: string, _context: unknown) => { + capturedSpecifier = specifier + return Promise.resolve({ url: `file:///resolved/${specifier}`, shortCircuit: true }) + } + + await mod.resolve('./relative.js', { parentURL: fakeParent }, nextResolve) + expect(capturedSpecifier).toBe('./relative.js') + + await mod.resolve('/absolute/path.js', { parentURL: fakeParent }, nextResolve) + expect(capturedSpecifier).toBe('/absolute/path.js') + + await mod.resolve('node:path', { parentURL: fakeParent }, nextResolve) + expect(capturedSpecifier).toBe('node:path') + + await mod.resolve('file:///some/file.js', { parentURL: fakeParent }, nextResolve) + expect(capturedSpecifier).toBe('file:///some/file.js') + }) +}) diff --git a/test/runtime.test.ts b/test/runtime.test.ts index 816548ab..8f5d182a 100644 --- a/test/runtime.test.ts +++ b/test/runtime.test.ts @@ -56,6 +56,27 @@ describe('Harness launch contract', () => { Path: 'windows-path' } }) + expect(options.env).not.toHaveProperty('DSH_DESKTOP_PROFILE_MODULE_PATHS') + }) + + it('passes profile module paths env when provided', () => { + const options = buildHarnessSpawnOptions( + 'C:\\Users\\tester\\AppData\\Roaming\\dsh-desktop\\launch-root', + 'C:\\Users\\tester\\AppData\\Roaming\\dsh-desktop\\harness', + 'win32', + { + ELECTRON_RUN_AS_NODE: '1', + PATH: 'fallback-path', + Path: 'windows-path' + }, + 'C:\\app\\profile-module-paths.mjs' + ) + + expect(options.env).toMatchObject({ + DSH_HOME: 'C:\\Users\\tester\\AppData\\Roaming\\dsh-desktop\\harness', + DSH_DESKTOP_PROFILE_MODULE_PATHS: 'C:\\app\\profile-module-paths.mjs', + NO_COLOR: '1' + }) expect(options.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE') }) @@ -81,6 +102,32 @@ describe('Harness launch contract', () => { ]) }) + it('injects profile module paths via --import when provided', () => { + expect( + buildNodeArguments( + 'C:\\app\\harness-node-entry.mjs', + 'C:\\app\\dsh\\lib\\bin.js', + 43127, + 'C:\\app\\dsh-desktop.patch.yml', + 'C:\\app\\profile-module-paths.mjs' + ) + ).toEqual([ + '--expose-internals', + '--import', + 'C:\\app\\profile-module-paths.mjs', + 'C:\\app\\harness-node-entry.mjs', + 'C:\\app\\dsh\\lib\\bin.js', + 'web', + '--patch', + 'C:\\app\\dsh-desktop.patch.yml', + '--host', + '127.0.0.1', + '--port', + '43127' + ]) + }) + + it('makes native Windows termination codes diagnosable', () => { expect(formatExitCode(4294930435)).toContain( '0xFFFF7003, Crashpad handler unavailable' From 1bf6ea324bf8a02a6b06d566f73d1340a193de77 Mon Sep 17 00:00:00 2001 From: yaojin3616 Date: Wed, 19 Aug 2026 11:35:40 +0800 Subject: [PATCH 2/3] fix: prefer profile node_modules over app bundle in resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profile (project-level) packages now take priority during both ESM and CJS resolution, with the app bundle acting as fallback — matching Node's local-first resolution semantics. - ESM resolver tries synthetic profile parents before default resolution - CJS patches Module._resolveFilename to try profile paths first - Add priority and fallback tests, including a CJS child-process test --- build/profile-esm-resolver.mjs | 27 +++++----- build/profile-module-paths.mjs | 24 +++++++++ test/profile-module-paths.test.ts | 83 +++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 14 deletions(-) diff --git a/build/profile-esm-resolver.mjs b/build/profile-esm-resolver.mjs index b58250cc..a2d6642f 100644 --- a/build/profile-esm-resolver.mjs +++ b/build/profile-esm-resolver.mjs @@ -23,20 +23,19 @@ export async function resolve(specifier, context, nextResolve) { return nextResolve(specifier, context) } - try { - return await nextResolve(specifier, context) - } catch (defaultError) { - for (const nodeModulesDir of profileNodeModules) { - const syntheticParent = pathToFileURL(nodeModulesDir + '/.resolve-anchor/anchor.js').href - try { - return await nextResolve(specifier, { - ...context, - parentURL: syntheticParent - }) - } catch { - // try next profile - } + // Project-level (profile) node_modules take priority over the app bundle, + // matching Node's "local first" resolution semantics. + for (const nodeModulesDir of profileNodeModules) { + const syntheticParent = pathToFileURL(nodeModulesDir + '/.resolve-anchor/anchor.js').href + try { + return await nextResolve(specifier, { + ...context, + parentURL: syntheticParent + }) + } catch { + // try next profile } - throw defaultError } + + return nextResolve(specifier, context) } diff --git a/build/profile-module-paths.mjs b/build/profile-module-paths.mjs index 0e72f46e..b95dccf4 100644 --- a/build/profile-module-paths.mjs +++ b/build/profile-module-paths.mjs @@ -33,6 +33,30 @@ if (profileNodeModules.length > 0) { try { const require = createRequire(import.meta.url) const Module = require('node:module') + + // Project-level (profile) node_modules take priority over the app bundle, + // matching Node's "local first" resolution semantics. + const isBareSpecifier = (request) => + !request.startsWith('.') && + !request.startsWith('/') && + !request.startsWith('node:') && + !/^[a-zA-Z]:[\\/]/.test(request) + + const originalResolveFilename = Module._resolveFilename + Module._resolveFilename = function (request, parent, isMain, options) { + if (isBareSpecifier(request)) { + try { + return originalResolveFilename.call(this, request, parent, isMain, { + ...options, + paths: profileNodeModules + }) + } catch { + // fall through to default resolution + } + } + return originalResolveFilename.call(this, request, parent, isMain, options) + } + if (Module.globalPaths) { for (const dir of profileNodeModules) { if (!Module.globalPaths.includes(dir)) { diff --git a/test/profile-module-paths.test.ts b/test/profile-module-paths.test.ts index 7605bba3..7cae2f1a 100644 --- a/test/profile-module-paths.test.ts +++ b/test/profile-module-paths.test.ts @@ -3,6 +3,10 @@ import { mkdir, writeFile, rm } from 'node:fs/promises' import { join } from 'node:path' import { tmpdir } from 'node:os' import { pathToFileURL } from 'node:url' +import { execFile } from 'node:child_process' +import { promisify } from 'node:util' + +const execFileAsync = promisify(execFile) describe('Profile module paths resolution', () => { it('resolves bare specifiers from profile node_modules via synthetic parent', async () => { @@ -69,4 +73,83 @@ describe('Profile module paths resolution', () => { await mod.resolve('file:///some/file.js', { parentURL: fakeParent }, nextResolve) expect(capturedSpecifier).toBe('file:///some/file.js') }) + + it('prefers profile node_modules over default app-bundle resolution', async () => { + const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href + const mod = await import(resolverUrl) + + const profileNodeModules = '/fake/profiles/web/node_modules' + mod.initialize({ profileNodeModules: [profileNodeModules] }) + + const nextResolve = (specifier: string, context: { parentURL?: string }) => { + if (context.parentURL?.startsWith(pathToFileURL(profileNodeModules).href)) { + return Promise.resolve({ url: `file:///profile-version/${specifier}`, shortCircuit: true }) + } + return Promise.resolve({ url: `file:///bundle-version/${specifier}`, shortCircuit: true }) + } + + const result = await mod.resolve( + 'shared-pkg', + { parentURL: 'file:///app/bundle/module.js', conditions: ['import'] }, + nextResolve + ) + + expect(result.url).toBe('file:///profile-version/shared-pkg') + }) + + it('falls back to default resolution when no profile provides the package', async () => { + const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href + const mod = await import(resolverUrl) + + mod.initialize({ profileNodeModules: ['/fake/profiles/web/node_modules'] }) + + const nextResolve = (specifier: string, context: { parentURL?: string }) => { + if (context.parentURL?.includes('/fake/profiles/')) { + return Promise.reject(new Error('not in profile')) + } + return Promise.resolve({ url: `file:///bundle-version/${specifier}`, shortCircuit: true }) + } + + const result = await mod.resolve( + 'bundle-only-pkg', + { parentURL: 'file:///app/bundle/module.js', conditions: ['import'] }, + nextResolve + ) + + expect(result.url).toBe('file:///bundle-version/bundle-only-pkg') + }) + + it('resolves CJS requires from profile node_modules before the app bundle', async () => { + const testDir = join(tmpdir(), `dsh-desktop-cjs-${Date.now()}`) + const dshHome = join(testDir, 'home') + const appDir = join(testDir, 'app') + const profilePkgDir = join(dshHome, 'profiles', 'web', 'node_modules', 'test-cjs-pkg') + const bundlePkgDir = join(appDir, 'node_modules', 'test-cjs-pkg') + + await mkdir(profilePkgDir, { recursive: true }) + await mkdir(bundlePkgDir, { recursive: true }) + await writeFile( + join(profilePkgDir, 'package.json'), + JSON.stringify({ name: 'test-cjs-pkg', version: '1.0.0', main: 'index.js' }) + ) + await writeFile(join(profilePkgDir, 'index.js'), "module.exports = { marker: 'profile' };\n") + await writeFile( + join(bundlePkgDir, 'package.json'), + JSON.stringify({ name: 'test-cjs-pkg', version: '2.0.0', main: 'index.js' }) + ) + await writeFile(join(bundlePkgDir, 'index.js'), "module.exports = { marker: 'bundle' };\n") + + try { + const setupPath = join(process.cwd(), 'build', 'profile-module-paths.mjs') + const { stdout } = await execFileAsync( + process.execPath, + ['--import', setupPath, '-e', "process.stdout.write(require('test-cjs-pkg').marker)"], + { cwd: appDir, env: { ...process.env, DSH_HOME: dshHome } } + ) + + expect(stdout).toBe('profile') + } finally { + await rm(testDir, { recursive: true, force: true }) + } + }) }) From 610201ca1cb20d9e6bcec6ba15042043159d76f5 Mon Sep 17 00:00:00 2001 From: yaojin Date: Tue, 18 Aug 2026 23:47:49 -0700 Subject: [PATCH 3/3] fix: make profile plugin resolution a fallback instead of an override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profile-first resolution broke more than it fixed. Node's nextResolve merges the context it is handed into the shared context object, so once the ESM hook had tried a synthetic profile parent, the fallback resolved from that anchor instead of the real importer — every bare import the app bundle owns failed with ERR_MODULE_NOT_FOUND as soon as any profile had a node_modules directory. Profile-first also let a plugin's copy of a shared package replace the app bundle's own nested dependency, and discarded the paths callers pass to require.resolve. Default resolution now runs first and profiles only fill in what the app bundle cannot provide, which is all the original bug needs: the plugin loader ships in the bundle and cannot see DSH_HOME/profiles//node_modules. Plugins keep their pinned dependency versions either way, because the market installs them with pnpm and their deps live beside their real path in .pnpm. - Pass the preload to --import as a file URL. A bare Windows path parses as a `c:` URL scheme, so Node exited before the harness entry ran and the app could not start at all on Windows. - Skip the preload when the shim is missing instead of turning a missing best-effort resource into an unexplained startup crash. - Look up profile directories on every failed resolution rather than snapshotting them at startup, so the first plugin installed into a fresh profile mounts without a restart. - Inject only the profile the harness runs, named by DSH_DESKTOP_PROFILES, instead of every directory under DSH_HOME/profiles in readdir order. - Report the importer's original error when no profile provides the package, and surface exports/package-config errors from a profile copy instead of hiding them behind the next candidate. - Drop the NODE_PATH mutation: it has no effect on the running process and leaked into every child the harness spawns. Rewrites the ESM tests as real child-process integration tests. The previous ones mocked nextResolve, which is why they passed against a resolver that could not resolve the app bundle at all; 7 of the new tests fail against the previous implementation. --- build/profile-esm-resolver.mjs | 72 +++--- build/profile-module-paths.mjs | 100 ++++---- build/profile-node-modules.d.mts | 3 + build/profile-node-modules.mjs | 53 ++++ package.json | 4 + src/main/runtime/harness-runtime.ts | 30 ++- test/profile-module-paths.test.ts | 384 +++++++++++++++++++--------- test/runtime.test.ts | 107 ++++++-- 8 files changed, 514 insertions(+), 239 deletions(-) create mode 100644 build/profile-node-modules.d.mts create mode 100644 build/profile-node-modules.mjs diff --git a/build/profile-esm-resolver.mjs b/build/profile-esm-resolver.mjs index a2d6642f..9380cd8b 100644 --- a/build/profile-esm-resolver.mjs +++ b/build/profile-esm-resolver.mjs @@ -1,41 +1,55 @@ +import { join } from 'node:path' import { pathToFileURL } from 'node:url' +import { isBareSpecifier, listProfileNodeModules } from './profile-node-modules.mjs' -let profileNodeModules = [] +// The package was found in the profile but cannot be used from there. That is a +// better error than the generic "not found" the original importer produced, so +// it is reported instead of being swallowed by the next candidate. +const PROFILE_PACKAGE_ERRORS = new Set([ + 'ERR_PACKAGE_PATH_NOT_EXPORTED', + 'ERR_PACKAGE_IMPORT_NOT_DEFINED', + 'ERR_INVALID_PACKAGE_CONFIG', + 'ERR_INVALID_PACKAGE_TARGET', + 'ERR_UNSUPPORTED_DIR_IMPORT' +]) + +let dshHome +let profiles = [] export function initialize(data) { - if (data?.profileNodeModules) { - profileNodeModules = data.profileNodeModules - } + dshHome = data?.dshHome + profiles = data?.profiles ?? [] } -export async function resolve(specifier, context, nextResolve) { - if (profileNodeModules.length === 0) { - return nextResolve(specifier, context) - } +// Two levels below node_modules, so that Node's own lookup walks up into +// /node_modules/ and applies package.json exports, +// conditions and subpaths exactly as it would for a local dependency. +function anchorFor(nodeModulesDirectory) { + return pathToFileURL(join(nodeModulesDirectory, '.dsh-desktop-anchor', 'anchor.js')).href +} - const isBareSpecifier = !specifier.startsWith('.') && - !specifier.startsWith('/') && - !specifier.startsWith('file://') && - !specifier.startsWith('node:') && - !specifier.includes('://') +export async function resolve(specifier, context, nextResolve) { + const parentURL = context.parentURL - if (!isBareSpecifier) { - return nextResolve(specifier, context) - } + try { + // Default resolution first: the app bundle keeps ownership of its own + // dependency tree, and profiles only fill in what it cannot provide. + return await nextResolve(specifier, context) + } catch (error) { + if (!isBareSpecifier(specifier)) throw error - // Project-level (profile) node_modules take priority over the app bundle, - // matching Node's "local first" resolution semantics. - for (const nodeModulesDir of profileNodeModules) { - const syntheticParent = pathToFileURL(nodeModulesDir + '/.resolve-anchor/anchor.js').href - try { - return await nextResolve(specifier, { - ...context, - parentURL: syntheticParent - }) - } catch { - // try next profile + const directories = listProfileNodeModules(dshHome, profiles) + for (const directory of directories) { + try { + return await nextResolve(specifier, { ...context, parentURL: anchorFor(directory) }) + } catch (profileError) { + if (PROFILE_PACKAGE_ERRORS.has(profileError?.code)) throw profileError + } } - } - return nextResolve(specifier, context) + // nextResolve merges the context it is handed into the shared context + // object, so the synthetic parent has to be undone before giving up. + context.parentURL = parentURL + throw error + } } diff --git a/build/profile-module-paths.mjs b/build/profile-module-paths.mjs index b95dccf4..556cc947 100644 --- a/build/profile-module-paths.mjs +++ b/build/profile-module-paths.mjs @@ -1,73 +1,65 @@ -import { readdirSync, existsSync } from 'node:fs' import { createRequire, register } from 'node:module' -import { join, delimiter, dirname } from 'node:path' +import { dirname, join } from 'node:path' import { fileURLToPath, pathToFileURL } from 'node:url' +import { + isBareSpecifier, + listProfileNodeModules, + parseProfileNames +} from './profile-node-modules.mjs' +// Plugins installed from the market live in DSH_HOME/profiles//node_modules, +// while the harness and its plugin loader run from the packaged app bundle. After +// packaging those are different trees, so a bare import of an installed plugin has +// to be able to fall back to the profile. const dshHome = process.env.DSH_HOME -const __filename = fileURLToPath(import.meta.url) -const __dirname = dirname(__filename) +const profiles = parseProfileNames(process.env.DSH_DESKTOP_PROFILES) -function collectProfileNodeModules(home) { - if (!home) return [] - const profilesDir = join(home, 'profiles') - try { - return readdirSync(profilesDir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) - .map((entry) => join(profilesDir, entry.name, 'node_modules')) - .filter((dir) => existsSync(dir)) - } catch { - return [] - } +if (dshHome) { + installCommonJsFallback() + installEsmFallback() } -const profileNodeModules = collectProfileNodeModules(dshHome) - -if (profileNodeModules.length > 0) { - const paths = profileNodeModules.join(delimiter) - if (process.env.NODE_PATH) { - process.env.NODE_PATH = process.env.NODE_PATH + delimiter + paths - } else { - process.env.NODE_PATH = paths - } - +function installCommonJsFallback() { try { const require = createRequire(import.meta.url) const Module = require('node:module') - - // Project-level (profile) node_modules take priority over the app bundle, - // matching Node's "local first" resolution semantics. - const isBareSpecifier = (request) => - !request.startsWith('.') && - !request.startsWith('/') && - !request.startsWith('node:') && - !/^[a-zA-Z]:[\\/]/.test(request) - const originalResolveFilename = Module._resolveFilename + Module._resolveFilename = function (request, parent, isMain, options) { - if (isBareSpecifier(request)) { - try { - return originalResolveFilename.call(this, request, parent, isMain, { - ...options, - paths: profileNodeModules - }) - } catch { - // fall through to default resolution + try { + return originalResolveFilename.call(this, request, parent, isMain, options) + } catch (error) { + if (!isBareSpecifier(request)) throw error + for (const directory of listProfileNodeModules(dshHome, profiles)) { + try { + return originalResolveFilename.call(this, request, parent, isMain, { + ...options, + paths: [directory] + }) + } catch { + // this profile does not provide the package + } } + throw error } - return originalResolveFilename.call(this, request, parent, isMain, options) } + } catch (error) { + warn('CommonJS', error) + } +} - if (Module.globalPaths) { - for (const dir of profileNodeModules) { - if (!Module.globalPaths.includes(dir)) { - Module.globalPaths.push(dir) - } - } - } - } catch { - // Best effort for CJS fallback +function installEsmFallback() { + try { + const resolver = join(dirname(fileURLToPath(import.meta.url)), 'profile-esm-resolver.mjs') + register(pathToFileURL(resolver).href, { data: { dshHome, profiles } }) + } catch (error) { + warn('ESM', error) } +} - const resolverUrl = pathToFileURL(join(__dirname, 'profile-esm-resolver.mjs')).href - register(resolverUrl, { data: { profileNodeModules } }) +function warn(loader, error) { + const message = error instanceof Error ? error.message : String(error) + process.stderr.write( + `[dsh-desktop] profile plugin resolution is unavailable for ${loader} imports: ${message}\n` + ) } diff --git a/build/profile-node-modules.d.mts b/build/profile-node-modules.d.mts new file mode 100644 index 00000000..e80a6886 --- /dev/null +++ b/build/profile-node-modules.d.mts @@ -0,0 +1,3 @@ +export declare function isBareSpecifier(specifier: unknown): boolean +export declare function parseProfileNames(value: string | undefined): string[] +export declare function listProfileNodeModules(home: string | undefined, profiles?: string[]): string[] diff --git a/build/profile-node-modules.mjs b/build/profile-node-modules.mjs new file mode 100644 index 00000000..86894eec --- /dev/null +++ b/build/profile-node-modules.mjs @@ -0,0 +1,53 @@ +import { existsSync, readdirSync } from 'node:fs' +import { join } from 'node:path' + +// A specifier is "bare" when Node has to look it up in a node_modules directory. +// Relative paths, absolute paths, package imports (`#name`) and anything with a +// URL scheme — including Windows drive letters such as `C:\` — resolve against +// the importer and must never be redirected to a profile. +export function isBareSpecifier(specifier) { + if (typeof specifier !== 'string' || specifier.length === 0) return false + if ( + specifier.startsWith('.') || + specifier.startsWith('/') || + specifier.startsWith('\\') || + specifier.startsWith('#') + ) { + return false + } + return !/^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(specifier) +} + +export function parseProfileNames(value) { + if (!value) return [] + return value + .split(',') + .map((name) => name.trim()) + .filter((name) => name.length > 0 && !name.includes('/') && !name.includes('\\')) +} + +// Read on every lookup instead of snapshotting at startup: a profile only gains +// a node_modules directory when its first plugin is installed, which happens +// long after the harness process starts. +export function listProfileNodeModules(home, profiles = []) { + if (!home) return [] + const profilesDir = join(home, 'profiles') + const names = profiles.length > 0 ? profiles : readProfileNames(profilesDir) + const directories = [] + for (const name of names) { + const directory = join(profilesDir, name, 'node_modules') + if (existsSync(directory)) directories.push(directory) + } + return directories +} + +function readProfileNames(profilesDir) { + try { + return readdirSync(profilesDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort() + } catch { + return [] + } +} diff --git a/package.json b/package.json index b5e712cc..08eb23e1 100644 --- a/package.json +++ b/package.json @@ -119,6 +119,10 @@ "from": "build/profile-esm-resolver.mjs", "to": "profile-esm-resolver.mjs" }, + { + "from": "build/profile-node-modules.mjs", + "to": "profile-node-modules.mjs" + }, { "from": "build/dsh-desktop.patch.yml", "to": "dsh-desktop.patch.yml" diff --git a/src/main/runtime/harness-runtime.ts b/src/main/runtime/harness-runtime.ts index db3d4f08..0347e256 100644 --- a/src/main/runtime/harness-runtime.ts +++ b/src/main/runtime/harness-runtime.ts @@ -3,6 +3,7 @@ import { createWriteStream, existsSync, type WriteStream } from 'node:fs' import { mkdir } from 'node:fs/promises' import { createServer } from 'node:net' import { dirname, join } from 'node:path' +import { pathToFileURL } from 'node:url' import type { RuntimePhase, RuntimeSnapshot } from '../../shared/contracts' export interface HarnessRuntimeOptions { @@ -22,9 +23,13 @@ export interface HarnessRuntimeOptions { onChanged(snapshot: RuntimeSnapshot): void } +// The desktop app always runs the `web` profile, which is also where the plugin +// market installs packages (DSH_HOME/profiles/web/node_modules). +export const HARNESS_PROFILE = 'web' + export function buildHarnessArguments(port: number, patchPath?: string): string[] { return [ - 'web', + HARNESS_PROFILE, ...(patchPath ? ['--patch', patchPath] : []), '--host', '127.0.0.1', @@ -37,8 +42,7 @@ export function buildHarnessSpawnOptions( launchDirectory: string, dshHome: string, platform: NodeJS.Platform = process.platform, - environment: NodeJS.ProcessEnv = process.env, - profileModulePathsPath?: string + environment: NodeJS.ProcessEnv = process.env ): SpawnOptionsWithoutStdio { const { ELECTRON_RUN_AS_NODE: _runAsNode, ...parentEnvironment } = environment const pathKey = platform === 'win32' ? 'Path' : 'PATH' @@ -49,7 +53,7 @@ export function buildHarnessSpawnOptions( ...parentEnvironment, DSH_HOME: dshHome, NO_COLOR: '1', - ...(profileModulePathsPath ? { DSH_DESKTOP_PROFILE_MODULE_PATHS: profileModulePathsPath } : {}), + DSH_DESKTOP_PROFILES: HARNESS_PROFILE, [pathKey]: environment[pathKey] ?? environment.PATH ?? '' }, stdio: ['pipe', 'pipe', 'pipe'], @@ -62,11 +66,11 @@ export function buildNodeArguments( dshEntryPath: string, port: number, patchPath?: string, - profileModulePathsPath?: string + profileModulePathsUrl?: string ): string[] { return [ '--expose-internals', - ...(profileModulePathsPath ? ['--import', profileModulePathsPath] : []), + ...(profileModulePathsUrl ? ['--import', profileModulePathsUrl] : []), nodeEntryPath, dshEntryPath, ...buildHarnessArguments(port, patchPath) @@ -122,18 +126,28 @@ export class HarnessRuntime { const port = await reservePort() const url = `http://127.0.0.1:${port}` + // Resolving profile plugins is best effort: a missing shim must not turn + // into an unexplained `--import` crash before the harness entry even runs. + const profileModulePathsUrl = existsSync(this.options.profileModulePathsPath) + ? pathToFileURL(this.options.profileModulePathsPath).href + : undefined const args = buildNodeArguments( this.options.nodeEntryPath, this.options.dshEntryPath, port, this.options.dshPatchPath, - this.options.profileModulePathsPath + profileModulePathsUrl ) const startupTimeoutMs = this.options.startupTimeoutMs ?? (process.platform === 'win32' ? 120_000 : 45_000) this.writeLog(`\n[desktop] starting ${new Date().toISOString()}`) this.writeLog(`[desktop] launch directory ${launchDirectory}`) + if (!profileModulePathsUrl) { + this.writeLog( + `[desktop] profile plugin resolution is disabled: ${this.options.profileModulePathsPath} was not found` + ) + } this.writeLog(`[desktop] endpoint ${url}`) this.setState('starting', 'Starting DeepSeek Harness…') @@ -142,7 +156,7 @@ export class HarnessRuntime { child = this.options.launchProcess( this.options.nodeExecutablePath, args, - buildHarnessSpawnOptions(launchDirectory, this.options.dshHome, undefined, undefined, this.options.profileModulePathsPath) + buildHarnessSpawnOptions(launchDirectory, this.options.dshHome) ) } catch (error) { const message = error instanceof Error ? error.message : String(error) diff --git a/test/profile-module-paths.test.ts b/test/profile-module-paths.test.ts index 7cae2f1a..4b3de3e5 100644 --- a/test/profile-module-paths.test.ts +++ b/test/profile-module-paths.test.ts @@ -1,155 +1,297 @@ -import { describe, expect, it } from 'vitest' -import { mkdir, writeFile, rm } from 'node:fs/promises' -import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { execFile } from 'node:child_process' +import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' +import { join } from 'node:path' import { pathToFileURL } from 'node:url' -import { execFile } from 'node:child_process' import { promisify } from 'node:util' +import { isBareSpecifier, listProfileNodeModules, parseProfileNames } from '../build/profile-node-modules.mjs' const execFileAsync = promisify(execFile) +const setupUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-module-paths.mjs')).href -describe('Profile module paths resolution', () => { - it('resolves bare specifiers from profile node_modules via synthetic parent', async () => { - const testDir = join(tmpdir(), `dsh-desktop-resolver-${Date.now()}`) - const profileNodeModules = join(testDir, 'profiles', 'web', 'node_modules') - const packageDir = join(profileNodeModules, 'test-profile-pkg') +const workspaces: string[] = [] - await mkdir(packageDir, { recursive: true }) - await writeFile( - join(packageDir, 'package.json'), - JSON.stringify({ name: 'test-profile-pkg', version: '1.0.0', main: 'index.js', type: 'module' }) - ) - await writeFile(join(packageDir, 'index.js'), 'export const value = 42;\n') - - try { - const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href - const mod = await import(resolverUrl) - - mod.initialize({ profileNodeModules: [profileNodeModules] }) - - const fakeParent = 'file:///app/somewhere/module.js' - const nextResolve = (specifier: string, _context: unknown) => { - if (specifier.includes('test-profile-pkg')) { - return Promise.resolve({ url: pathToFileURL(join(packageDir, 'index.js')).href, shortCircuit: true }) - } - return Promise.reject(new Error(`not found: ${specifier}`)) - } - - const result = await mod.resolve( - 'test-profile-pkg', - { parentURL: fakeParent, conditions: ['import'] }, - nextResolve - ) - - expect(result.url).toContain('test-profile-pkg') - expect(result.url).toContain('index.js') - } finally { - await rm(testDir, { recursive: true, force: true }) - } +afterEach(async () => { + await Promise.all(workspaces.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))) +}) + +async function createWorkspace(): Promise<{ home: string; app: string }> { + const root = await mkdtemp(join(tmpdir(), 'dsh-desktop-resolution-')) + workspaces.push(root) + const app = join(root, 'app') + await mkdir(app, { recursive: true }) + return { home: join(root, 'home'), app } +} + +async function writePackage( + nodeModules: string, + name: string, + marker: string, + options: { module?: boolean } = {} +): Promise { + const directory = join(nodeModules, ...name.split('/')) + await mkdir(directory, { recursive: true }) + const module = options.module ?? true + await writeFile( + join(directory, 'package.json'), + JSON.stringify({ + name, + version: '1.0.0', + main: 'index.js', + ...(module ? { type: 'module' } : {}) + }) + ) + await writeFile( + join(directory, 'index.js'), + module ? `export const marker = ${JSON.stringify(marker)}\n` : `module.exports = ${JSON.stringify(marker)}\n` + ) + return directory +} + +/** Runs `script` the way the desktop app launches the harness: with the shim preloaded. */ +async function runWithShim( + app: string, + home: string, + script: string, + environment: NodeJS.ProcessEnv = {} +): Promise { + const entry = join(app, `entry-${Math.random().toString(36).slice(2)}.mjs`) + await writeFile(entry, script) + const { stdout } = await execFileAsync(process.execPath, ['--import', setupUrl, entry], { + cwd: app, + env: { ...process.env, DSH_HOME: home, DSH_DESKTOP_PROFILES: 'web', ...environment } }) + return stdout.trim() +} - it('passes through non-bare specifiers unchanged', async () => { - const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href - const mod = await import(resolverUrl) +describe('profile plugin resolution', () => { + it('resolves a plugin that only exists in the profile', async () => { + const { home, app } = await createWorkspace() + await writePackage(join(home, 'profiles', 'web', 'node_modules'), '@liustack/modlens', 'profile') - mod.initialize({ profileNodeModules: ['/fake/profile/node_modules'] }) + const output = await runWithShim( + app, + home, + "const { marker } = await import('@liustack/modlens')\nprocess.stdout.write(marker)\n" + ) - const fakeParent = 'file:///app/module.js' - let capturedSpecifier = '' - const nextResolve = (specifier: string, _context: unknown) => { - capturedSpecifier = specifier - return Promise.resolve({ url: `file:///resolved/${specifier}`, shortCircuit: true }) - } + expect(output).toBe('profile') + }, 20_000) - await mod.resolve('./relative.js', { parentURL: fakeParent }, nextResolve) - expect(capturedSpecifier).toBe('./relative.js') + it('keeps resolving app bundle packages once a profile exists', async () => { + const { home, app } = await createWorkspace() + await writePackage(join(home, 'profiles', 'web', 'node_modules'), 'installed-plugin', 'profile') + await writePackage(join(app, 'node_modules'), 'bundled-only', 'bundle') - await mod.resolve('/absolute/path.js', { parentURL: fakeParent }, nextResolve) - expect(capturedSpecifier).toBe('/absolute/path.js') + const output = await runWithShim( + app, + home, + "const { marker } = await import('bundled-only')\nprocess.stdout.write(marker)\n" + ) - await mod.resolve('node:path', { parentURL: fakeParent }, nextResolve) - expect(capturedSpecifier).toBe('node:path') + expect(output).toBe('bundle') + }, 20_000) - await mod.resolve('file:///some/file.js', { parentURL: fakeParent }, nextResolve) - expect(capturedSpecifier).toBe('file:///some/file.js') - }) + it('never lets a profile shadow the app bundle dependency tree', async () => { + const { home, app } = await createWorkspace() + await writePackage(join(home, 'profiles', 'web', 'node_modules'), 'shared', 'profile') + const bundled = await writePackage(join(app, 'node_modules'), 'bundled', 'bundle') + await writePackage(join(bundled, 'node_modules'), 'shared', 'bundle-nested') + await writeFile(join(bundled, 'index.js'), "export { marker } from 'shared'\n") - it('prefers profile node_modules over default app-bundle resolution', async () => { - const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href - const mod = await import(resolverUrl) + const output = await runWithShim( + app, + home, + "const { marker } = await import('bundled')\nprocess.stdout.write(marker)\n" + ) - const profileNodeModules = '/fake/profiles/web/node_modules' - mod.initialize({ profileNodeModules: [profileNodeModules] }) + expect(output).toBe('bundle-nested') + }, 20_000) - const nextResolve = (specifier: string, context: { parentURL?: string }) => { - if (context.parentURL?.startsWith(pathToFileURL(profileNodeModules).href)) { - return Promise.resolve({ url: `file:///profile-version/${specifier}`, shortCircuit: true }) - } - return Promise.resolve({ url: `file:///bundle-version/${specifier}`, shortCircuit: true }) - } + it('picks up a profile node_modules created after the harness started', async () => { + const { home, app } = await createWorkspace() + const profileNodeModules = join(home, 'profiles', 'web', 'node_modules') + await mkdir(home, { recursive: true }) - const result = await mod.resolve( - 'shared-pkg', - { parentURL: 'file:///app/bundle/module.js', conditions: ['import'] }, - nextResolve + const output = await runWithShim( + app, + home, + [ + "import { mkdir, writeFile } from 'node:fs/promises'", + `const nodeModules = ${JSON.stringify(profileNodeModules)}`, + "const directory = nodeModules + '/late-plugin'", + 'await mkdir(directory, { recursive: true })', + `await writeFile(directory + '/package.json', ${JSON.stringify( + JSON.stringify({ name: 'late-plugin', version: '1.0.0', type: 'module', main: 'index.js' }) + )})`, + "await writeFile(directory + '/index.js', 'export const marker = \\'late\\'\\n')", + "const { marker } = await import('late-plugin')", + 'process.stdout.write(marker)' + ].join('\n') ) - expect(result.url).toBe('file:///profile-version/shared-pkg') - }) + expect(output).toBe('late') + }, 20_000) - it('falls back to default resolution when no profile provides the package', async () => { - const resolverUrl = pathToFileURL(join(process.cwd(), 'build', 'profile-esm-resolver.mjs')).href - const mod = await import(resolverUrl) + // pnpm is what the market installer runs, so a profile's top level is a set of + // symlinks into .pnpm and a plugin's own dependencies live beside its real path. + it('resolves a plugin through the pnpm store layout', async () => { + const { home, app } = await createWorkspace() + const nodeModules = join(home, 'profiles', 'web', 'node_modules') + const store = join(nodeModules, '.pnpm') - mod.initialize({ profileNodeModules: ['/fake/profiles/web/node_modules'] }) + const plugin = await writePackage(join(store, 'plugin@1.0.0', 'node_modules'), 'plugin', 'unused') + await writeFile(join(plugin, 'package.json'), JSON.stringify({ + name: 'plugin', + version: '1.0.0', + type: 'module', + exports: { '.': './index.js' } + })) + await writeFile(join(plugin, 'index.js'), "export { marker } from 'dependency'\n") + const dependency = await writePackage( + join(store, 'dependency@1.0.0', 'node_modules'), + 'dependency', + 'pnpm-store' + ) - const nextResolve = (specifier: string, context: { parentURL?: string }) => { - if (context.parentURL?.includes('/fake/profiles/')) { - return Promise.reject(new Error('not in profile')) - } - return Promise.resolve({ url: `file:///bundle-version/${specifier}`, shortCircuit: true }) - } + await symlink(plugin, join(nodeModules, 'plugin'), 'junction') + await symlink(dependency, join(store, 'plugin@1.0.0', 'node_modules', 'dependency'), 'junction') - const result = await mod.resolve( - 'bundle-only-pkg', - { parentURL: 'file:///app/bundle/module.js', conditions: ['import'] }, - nextResolve + const output = await runWithShim( + app, + home, + "const { marker } = await import('plugin')\nprocess.stdout.write(marker)\n" ) - expect(result.url).toBe('file:///bundle-version/bundle-only-pkg') - }) + expect(output).toBe('pnpm-store') + }, 20_000) - it('resolves CJS requires from profile node_modules before the app bundle', async () => { - const testDir = join(tmpdir(), `dsh-desktop-cjs-${Date.now()}`) - const dshHome = join(testDir, 'home') - const appDir = join(testDir, 'app') - const profilePkgDir = join(dshHome, 'profiles', 'web', 'node_modules', 'test-cjs-pkg') - const bundlePkgDir = join(appDir, 'node_modules', 'test-cjs-pkg') + it('only consults the profiles the harness actually runs', async () => { + const { home, app } = await createWorkspace() + await writePackage(join(home, 'profiles', 'other', 'node_modules'), 'other-plugin', 'other') - await mkdir(profilePkgDir, { recursive: true }) - await mkdir(bundlePkgDir, { recursive: true }) - await writeFile( - join(profilePkgDir, 'package.json'), - JSON.stringify({ name: 'test-cjs-pkg', version: '1.0.0', main: 'index.js' }) + const output = await runWithShim( + app, + home, + [ + "let outcome = 'resolved'", + "try { await import('other-plugin') } catch (error) { outcome = error.code }", + 'process.stdout.write(outcome)' + ].join('\n') + ) + + expect(output).toBe('ERR_MODULE_NOT_FOUND') + }, 20_000) + + it('reports the original error when no profile provides the package', async () => { + const { home, app } = await createWorkspace() + await writePackage(join(home, 'profiles', 'web', 'node_modules'), 'installed-plugin', 'profile') + + const output = await runWithShim( + app, + home, + [ + "let message = 'resolved'", + "try { await import('missing-everywhere') } catch (error) { message = error.message }", + 'process.stdout.write(message)' + ].join('\n') ) - await writeFile(join(profilePkgDir, 'index.js'), "module.exports = { marker: 'profile' };\n") + + expect(output).toContain('missing-everywhere') + expect(output).not.toContain('anchor') + }, 20_000) + + it('surfaces a broken profile package instead of a generic not-found', async () => { + const { home, app } = await createWorkspace() + const plugin = await writePackage(join(home, 'profiles', 'web', 'node_modules'), 'strict-plugin', 'profile') await writeFile( - join(bundlePkgDir, 'package.json'), - JSON.stringify({ name: 'test-cjs-pkg', version: '2.0.0', main: 'index.js' }) + join(plugin, 'package.json'), + JSON.stringify({ name: 'strict-plugin', version: '1.0.0', type: 'module', exports: { '.': './index.js' } }) ) - await writeFile(join(bundlePkgDir, 'index.js'), "module.exports = { marker: 'bundle' };\n") - - try { - const setupPath = join(process.cwd(), 'build', 'profile-module-paths.mjs') - const { stdout } = await execFileAsync( - process.execPath, - ['--import', setupPath, '-e', "process.stdout.write(require('test-cjs-pkg').marker)"], - { cwd: appDir, env: { ...process.env, DSH_HOME: dshHome } } - ) - - expect(stdout).toBe('profile') - } finally { - await rm(testDir, { recursive: true, force: true }) - } + + const output = await runWithShim( + app, + home, + [ + "let code = 'resolved'", + "try { await import('strict-plugin/hidden') } catch (error) { code = error.code }", + 'process.stdout.write(code)' + ].join('\n') + ) + + expect(output).toBe('ERR_PACKAGE_PATH_NOT_EXPORTED') + }, 20_000) + + it('applies the same fallback to CommonJS requires', async () => { + const { home, app } = await createWorkspace() + const profileNodeModules = join(home, 'profiles', 'web', 'node_modules') + await writePackage(profileNodeModules, 'cjs-plugin', 'profile', { module: false }) + await writePackage(profileNodeModules, 'cjs-shared', 'profile', { module: false }) + await writePackage(join(app, 'node_modules'), 'cjs-shared', 'bundle', { module: false }) + await writePackage(join(app, 'node_modules'), 'cjs-bundled', 'bundle', { module: false }) + + const output = await runWithShim( + app, + home, + [ + "import { createRequire } from 'node:module'", + 'const require = createRequire(import.meta.url)', + "process.stdout.write([require('cjs-plugin'), require('cjs-shared'), require('cjs-bundled')].join(','))" + ].join('\n') + ) + + expect(output).toBe('profile,bundle,bundle') + }, 20_000) + + it('leaves explicit require.resolve paths alone', async () => { + const { home, app } = await createWorkspace() + await writePackage(join(home, 'profiles', 'web', 'node_modules'), 'scoped', 'profile', { module: false }) + const bundled = await writePackage(join(app, 'node_modules'), 'bundled', 'bundle', { module: false }) + await writePackage(join(bundled, 'node_modules'), 'scoped', 'bundle-nested', { module: false }) + + const output = await runWithShim( + app, + home, + [ + "import { createRequire } from 'node:module'", + 'const require = createRequire(import.meta.url)', + `const resolved = require.resolve('scoped', { paths: [${JSON.stringify(bundled)}] })`, + 'process.stdout.write(require(resolved))' + ].join('\n') + ) + + expect(output).toBe('bundle-nested') + }, 20_000) +}) + +describe('profile discovery', () => { + it('treats only node_modules lookups as redirectable', () => { + expect(isBareSpecifier('@liustack/modlens')).toBe(true) + expect(isBareSpecifier('cordis')).toBe(true) + expect(isBareSpecifier('./relative.js')).toBe(false) + expect(isBareSpecifier('/absolute/path.js')).toBe(false) + expect(isBareSpecifier('#internal')).toBe(false) + expect(isBareSpecifier('node:path')).toBe(false) + expect(isBareSpecifier('file:///app/module.js')).toBe(false) + expect(isBareSpecifier('data:text/javascript,export default 1')).toBe(false) + expect(isBareSpecifier('C:\\app\\module.js')).toBe(false) + }) + + it('ignores profile names that could escape the profiles directory', () => { + expect(parseProfileNames('web, staging ')).toEqual(['web', 'staging']) + expect(parseProfileNames('../../etc,web')).toEqual(['web']) + expect(parseProfileNames(undefined)).toEqual([]) + }) + + it('skips profiles that have no node_modules yet', async () => { + const { home } = await createWorkspace() + await mkdir(join(home, 'profiles', 'empty'), { recursive: true }) + const populated = join(home, 'profiles', 'web', 'node_modules') + await mkdir(populated, { recursive: true }) + + expect(listProfileNodeModules(home)).toEqual([populated]) + expect(listProfileNodeModules(home, ['empty'])).toEqual([]) + expect(listProfileNodeModules(undefined)).toEqual([]) }) }) diff --git a/test/runtime.test.ts b/test/runtime.test.ts index 8f5d182a..9bbf1c1f 100644 --- a/test/runtime.test.ts +++ b/test/runtime.test.ts @@ -1,9 +1,16 @@ -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it } from 'vitest' +import { EventEmitter } from 'node:events' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type { ChildProcessWithoutNullStreams, SpawnOptionsWithoutStdio } from 'node:child_process' import { buildHarnessArguments, buildHarnessSpawnOptions, buildNodeArguments, - formatExitCode + formatExitCode, + HarnessRuntime, + HARNESS_PROFILE } from '../src/main/runtime/harness-runtime' import { canGrantWindowPermission, isTrustedAppUrl } from '../src/main/security-policy' import { @@ -56,27 +63,7 @@ describe('Harness launch contract', () => { Path: 'windows-path' } }) - expect(options.env).not.toHaveProperty('DSH_DESKTOP_PROFILE_MODULE_PATHS') - }) - - it('passes profile module paths env when provided', () => { - const options = buildHarnessSpawnOptions( - 'C:\\Users\\tester\\AppData\\Roaming\\dsh-desktop\\launch-root', - 'C:\\Users\\tester\\AppData\\Roaming\\dsh-desktop\\harness', - 'win32', - { - ELECTRON_RUN_AS_NODE: '1', - PATH: 'fallback-path', - Path: 'windows-path' - }, - 'C:\\app\\profile-module-paths.mjs' - ) - - expect(options.env).toMatchObject({ - DSH_HOME: 'C:\\Users\\tester\\AppData\\Roaming\\dsh-desktop\\harness', - DSH_DESKTOP_PROFILE_MODULE_PATHS: 'C:\\app\\profile-module-paths.mjs', - NO_COLOR: '1' - }) + expect(options.env).toMatchObject({ DSH_DESKTOP_PROFILES: HARNESS_PROFILE }) expect(options.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE') }) @@ -102,19 +89,21 @@ describe('Harness launch contract', () => { ]) }) - it('injects profile module paths via --import when provided', () => { + // `--import` takes a module specifier: a bare Windows path is read as a `c:` + // URL scheme and Node exits before the harness entry ever runs. + it('injects profile module paths via --import as a file URL', () => { expect( buildNodeArguments( 'C:\\app\\harness-node-entry.mjs', 'C:\\app\\dsh\\lib\\bin.js', 43127, 'C:\\app\\dsh-desktop.patch.yml', - 'C:\\app\\profile-module-paths.mjs' + 'file:///C:/app/profile-module-paths.mjs' ) ).toEqual([ '--expose-internals', '--import', - 'C:\\app\\profile-module-paths.mjs', + 'file:///C:/app/profile-module-paths.mjs', 'C:\\app\\harness-node-entry.mjs', 'C:\\app\\dsh\\lib\\bin.js', 'web', @@ -127,7 +116,6 @@ describe('Harness launch contract', () => { ]) }) - it('makes native Windows termination codes diagnosable', () => { expect(formatExitCode(4294930435)).toContain( '0xFFFF7003, Crashpad handler unavailable' @@ -135,6 +123,71 @@ describe('Harness launch contract', () => { }) }) +describe('Harness launch wiring', () => { + const roots: string[] = [] + + afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) + }) + + async function launchOnce( + options: { withProfileModulePaths: boolean } + ): Promise<{ args: string[]; environment: NodeJS.ProcessEnv }> { + const root = await mkdtemp(join(tmpdir(), 'dsh-desktop-runtime-')) + roots.push(root) + + const paths: Record = {} + for (const name of ['bin.js', 'node', 'harness-node-entry.mjs', 'dsh-desktop.patch.yml']) { + paths[name] = join(root, name) + await writeFile(paths[name]!, '') + } + const profileModulePathsPath = join(root, 'profile-module-paths.mjs') + if (options.withProfileModulePaths) await writeFile(profileModulePathsPath, '') + + let captured: { args: string[]; environment: NodeJS.ProcessEnv } | undefined + const runtime = new HarnessRuntime({ + dshEntryPath: paths['bin.js']!, + nodeExecutablePath: paths['node']!, + nodeEntryPath: paths['harness-node-entry.mjs']!, + profileModulePathsPath, + dshPatchPath: paths['dsh-desktop.patch.yml']!, + dshHome: join(root, 'harness'), + logPath: join(root, 'logs', 'harness.log'), + startupTimeoutMs: 1, + onChanged: () => {}, + launchProcess: (_executable, args, spawnOptions: SpawnOptionsWithoutStdio) => { + captured = { args, environment: spawnOptions.env ?? {} } + const child = new EventEmitter() as unknown as ChildProcessWithoutNullStreams + Object.assign(child, { stdout: new EventEmitter(), stderr: new EventEmitter(), exitCode: 0 }) + return child + } + }) + + await runtime.start(join(root, 'launch')) + await runtime.stop() + expect(captured).toBeDefined() + return captured! + } + + // A bare Windows path is read as a `c:` URL scheme and Node exits before the + // harness entry runs, so the preload has to be handed over as a file URL. + it('preloads the profile resolution shim as a file URL', async () => { + const { args, environment } = await launchOnce({ withProfileModulePaths: true }) + + const specifier = args[args.indexOf('--import') + 1] + expect(specifier).toMatch(/^file:\/\//) + expect(new URL(specifier!).pathname).toContain('profile-module-paths.mjs') + expect(environment.DSH_DESKTOP_PROFILES).toBe(HARNESS_PROFILE) + }) + + it('starts without the shim rather than crashing when it is missing', async () => { + const { args } = await launchOnce({ withProfileModulePaths: false }) + + expect(args).not.toContain('--import') + expect(args[0]).toBe('--expose-internals') + }) +}) + describe('navigation trust boundary', () => { it('only trusts the launcher and loopback HTTP pages', () => { expect(isTrustedAppUrl('file:///app/index.html')).toBe(true)