diff --git a/packages/deploy/src/extract-agent.ts b/packages/deploy/src/extract-agent.ts index 7c050630..b3d30c00 100644 --- a/packages/deploy/src/extract-agent.ts +++ b/packages/deploy/src/extract-agent.ts @@ -13,7 +13,8 @@ import { assertReadableFile, extractDefaultExport, packageNodePaths, - preserveLocalImportMetaUrlPlugin + preserveLocalImportMetaUrlPlugin, + withUnresolvedImportHint } from './persona-source.js'; /** @@ -107,6 +108,8 @@ export async function extractAgentSpec(onEventPath: string): Promise { + throw withUnresolvedImportHint(err, onEventPath); }); const mod = await import(pathToFileURL(compiledPath).href); diff --git a/packages/deploy/src/persona-source.test.ts b/packages/deploy/src/persona-source.test.ts new file mode 100644 index 00000000..168717bc --- /dev/null +++ b/packages/deploy/src/persona-source.test.ts @@ -0,0 +1,152 @@ +import assert from 'node:assert/strict'; +import { cp, mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'; +import { execFileSync } from 'node:child_process'; +import { createRequire } from 'node:module'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +import { nodeModulesChain, packageNodePaths, withUnresolvedImportHint } from './persona-source.js'; + +const require = createRequire(import.meta.url); +const HERE = path.dirname(fileURLToPath(import.meta.url)); + +test('nodeModulesChain treats an ancestor node_modules as a search root', () => { + const chain = nodeModulesChain('/prefix/node_modules/@agentworkforce/deploy/dist'); + assert.ok( + chain.includes('/prefix/node_modules'), + `installed tree root missing from ${JSON.stringify(chain)}` + ); + assert.ok( + !chain.some((dir) => dir.endsWith('node_modules/node_modules')), + `doubled node_modules segment in ${JSON.stringify(chain)}` + ); +}); + +test('packageNodePaths searches the persona tree before the CLI tree', () => { + const paths = packageNodePaths('/repo/customer-success/app-signal/persona.ts'); + assert.equal(paths[0], path.join('/repo/customer-success/app-signal', 'node_modules')); + assert.equal(new Set(paths).size, paths.length, 'duplicate entries'); + assert.ok( + paths.includes(nodeModulesChain(HERE).find((dir) => dir.endsWith('node_modules')) as string), + 'this package install root missing' + ); +}); + +test('withUnresolvedImportHint names the missing package and where to install it', () => { + const hinted = withUnresolvedImportHint( + new Error( + 'Build failed with 1 error:\npersona.ts:2:30: ERROR: Could not resolve "@agentworkforce/turn-kit"' + ), + '/repo/customer-success/app-signal/persona.ts' + ); + assert.ok(hinted instanceof Error); + assert.match(hinted.message, /npm install @agentworkforce\/turn-kit/); + assert.match(hinted.message, /\/repo\/customer-success\/app-signal/); +}); + +test('withUnresolvedImportHint installs the package, not the deep import path', () => { + const hinted = withUnresolvedImportHint( + new Error( + [ + 'ERROR: Could not resolve "@relayfile/adapter-core/triggers"', + 'ERROR: Could not resolve "lodash/fp"' + ].join('\n') + ), + '/repo/persona.ts' + ) as Error; + assert.match(hinted.message, /npm install @relayfile\/adapter-core lodash$/m); +}); + +test('withUnresolvedImportHint quotes shell metacharacters', () => { + const hinted = withUnresolvedImportHint( + new Error(String.raw`ERROR: Could not resolve "evil; touch pwned"`), + "/repo/it's a dir/persona.ts" + ) as Error; + const command = hinted.message.split('\n').at(-1) as string; + assert.equal(command, String.raw` cd '/repo/it'\''s a dir' && npm install 'evil; touch pwned'`); + // Round-trip through a shell to prove the injected command cannot run. + assert.deepEqual( + execFileSync('/bin/sh', ['-c', `set -- ${command.trim().split('npm install ')[1]}; printf '%s' "$1"`], { + encoding: 'utf8' + }), + 'evil; touch pwned' + ); +}); + +test('withUnresolvedImportHint ignores relative imports', () => { + const message = 'ERROR: Could not resolve "./missing.js"'; + const hinted = withUnresolvedImportHint(new Error(message), '/repo/persona.ts'); + assert.equal((hinted as Error).message, message); +}); + +/** + * The shipped failure: a globally installed CLI compiling a persona in a repo + * that has no `node_modules`. Reproduced by copying the built package into an + * installed layout so `import.meta.url` inside persona-source resolves the way + * it does for a real `npm i -g agentworkforce`. + */ +test('authored persona resolves a CLI-tree package with no project node_modules', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'agentworkforce-install-')); + const originalCwd = process.cwd(); + try { + const installRoot = path.join(root, 'prefix', 'node_modules'); + const deployDir = path.join(installRoot, '@agentworkforce', 'deploy'); + await mkdir(deployDir, { recursive: true }); + await cp(HERE, path.join(deployDir, 'dist'), { recursive: true }); + await writeFile( + path.join(deployDir, 'package.json'), + JSON.stringify({ name: '@agentworkforce/deploy', type: 'module' }), + 'utf8' + ); + // esbuild is a real dependency of the copied package; link it into the + // fake tree so the copy can load at all. + await symlink( + path.dirname(require.resolve('esbuild/package.json')), + path.join(installRoot, 'esbuild') + ); + + const kitDir = path.join(installRoot, '@agentworkforce', 'persona-kit'); + await mkdir(kitDir, { recursive: true }); + await writeFile( + path.join(kitDir, 'package.json'), + JSON.stringify({ name: '@agentworkforce/persona-kit', type: 'module', main: 'index.js' }), + 'utf8' + ); + await writeFile( + path.join(kitDir, 'index.js'), + 'export function definePersona(input) { return input; }\n', + 'utf8' + ); + + // The persona lives outside the install tree and has no node_modules. + const personaDir = path.join(root, 'watchdog-agents', 'customer-success', 'app-signal'); + await mkdir(personaDir, { recursive: true }); + const personaPath = path.join(personaDir, 'persona.ts'); + await writeFile( + personaPath, + [ + "import { definePersona } from '@agentworkforce/persona-kit';", + '', + "export default definePersona({ id: 'app-signal', onEvent: './agent.ts' });", + '' + ].join('\n'), + 'utf8' + ); + + const installed = (await import( + pathToFileURL(path.join(deployDir, 'dist', 'persona-source.js')).href + )) as typeof import('./persona-source.js'); + + // The user's cwd is their own repo, which has no node_modules either — + // without this the test's cwd (this monorepo) resolves the import and the + // assertion holds even against the broken implementation. + process.chdir(personaDir); + const result = await installed.loadPersonaSourceFile(personaPath); + assert.deepEqual(result.persona, { id: 'app-signal', onEvent: './agent.ts' }); + } finally { + process.chdir(originalCwd); + await rm(root, { recursive: true, force: true }); + } +}); diff --git a/packages/deploy/src/persona-source.ts b/packages/deploy/src/persona-source.ts index 73b133b2..c4d6b348 100644 --- a/packages/deploy/src/persona-source.ts +++ b/packages/deploy/src/persona-source.ts @@ -2,7 +2,7 @@ import { randomUUID } from 'node:crypto'; import { builtinModules } from 'node:module'; import { mkdtemp, readFile, rm, stat } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { dirname, join, resolve } from 'node:path'; +import { basename, dirname, join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { build, type Loader, type Plugin } from 'esbuild'; @@ -74,6 +74,8 @@ export async function loadPersonaSourceFile( resolveExtensions: RESOLVE_EXTENSIONS, plugins: [preserveLocalImportMetaUrlPlugin()], nodePaths: packageNodePaths(absInput) + }).catch((err) => { + throw withUnresolvedImportHint(err, absInput); }); const mod = await import(pathToFileURL(compiledPath).href); @@ -106,14 +108,112 @@ function extensionOf(inputPath: string): string { return idx === -1 ? '' : normalized.slice(idx); } +/** + * esbuild `nodePaths` fallbacks for compiling authored `.ts`/`.js` personas + * and agents. + * + * An authored persona lives in the user's own repo, which on a fresh install + * usually has no `node_modules` at all — the CLI is installed globally, so + * `@agentworkforce/*` exists only inside the CLI's own install tree. esbuild + * resolves bare imports by walking up from the importer, so without these + * fallbacks `import { definePersona } from '@agentworkforce/persona-kit'` + * fails with "Could not resolve". + * + * Fallbacks are the full `node_modules` lookup chains of (in order) the + * persona file, this module (the installed `@agentworkforce/deploy`), and the + * cwd. The chain is computed the way Node's own resolver does it, including + * the case a naive `join(dir, 'node_modules')` gets wrong: when an ancestor + * *is* `node_modules`, that directory is itself the search root. Missing that + * produced `/node_modules/node_modules` — a path that only exists in + * a monorepo checkout, which is why every installed CLI failed here while the + * dev layout worked. + * + * Order matters: the persona's own dependencies win over the CLI's copies, + * and every entry is only consulted after normal resolution has failed. + */ export function packageNodePaths(absInput: string): string[] { const here = dirname(fileURLToPath(import.meta.url)); - return [ - join(dirname(absInput), 'node_modules'), - join(here, '..', 'node_modules'), - join(here, '..', '..', '..', 'node_modules'), - join(process.cwd(), 'node_modules') - ]; + const seen = new Set(); + const paths: string[] = []; + for (const root of [dirname(resolve(absInput)), here, process.cwd()]) { + for (const dir of nodeModulesChain(root)) { + if (seen.has(dir)) continue; + seen.add(dir); + paths.push(dir); + } + } + return paths; +} + +/** Every `node_modules` directory Node would search from `fromDir` upward. */ +export function nodeModulesChain(fromDir: string): string[] { + const chain: string[] = []; + let dir = resolve(fromDir); + for (;;) { + chain.push(basename(dir) === 'node_modules' ? dir : join(dir, 'node_modules')); + const parent = dirname(dir); + if (parent === dir) break; + dir = parent; + } + return chain; +} + +/** + * Turn esbuild's bare "Could not resolve" into an actionable install hint. + * + * After `packageNodePaths`, an `@agentworkforce/*` import the CLI ships + * resolves on its own; what reaches here is a package neither the project nor + * the CLI has (a persona kit the CLI does not depend on, a third-party SDK the + * handler imports). Naming it plus the directory to install it in is the + * difference between a dead end and a one-line fix. + */ +export function withUnresolvedImportHint(error: unknown, absInput: string): unknown { + if (!(error instanceof Error)) return error; + const missing = unresolvedSpecifiers(error.message); + if (missing.length === 0) return error; + + const projectDir = dirname(resolve(absInput)); + const install = missing.map(shellQuote).join(' '); + error.message = [ + error.message, + `Install the missing package${missing.length > 1 ? 's' : ''} where the persona lives:`, + ` cd ${shellQuote(projectDir)} && npm install ${install}` + ].join('\n'); + return error; +} + +function unresolvedSpecifiers(message: string): string[] { + const found = new Set(); + for (const match of message.matchAll(/Could not resolve "([^"]+)"/g)) { + const specifier = match[1]; + // Relative/absolute imports are authoring mistakes, not missing installs. + if (specifier.startsWith('.') || specifier.startsWith('/')) continue; + found.add(packageNameOf(specifier)); + } + return [...found]; +} + +/** + * The package to install for an import specifier. A deep import + * (`@relayfile/adapter-core/triggers`, `lodash/fp`) must be reduced to its + * package name — npm reads the full specifier as a local directory or a git + * remote and would install the wrong thing, or nothing. + */ +function packageNameOf(specifier: string): string { + const segments = specifier.split('/'); + return specifier.startsWith('@') ? segments.slice(0, 2).join('/') : segments[0]; +} + +/** + * Quote a value for the copy-pasteable `cd … && npm install …` hint. Both the + * path and the specifiers originate in the user's own source, but the hint is + * written to be pasted into a shell, so a specifier like `x; rm -rf ~` must not + * survive as two commands. Ordinary paths and package names are left bare so + * the common message stays readable. + */ +function shellQuote(value: string): string { + if (value.length > 0 && /^[\w@./:+-]+$/.test(value)) return value; + return `'${value.split("'").join(`'\\''`)}'`; } export function preserveLocalImportMetaUrlPlugin(): Plugin {