diff --git a/skills-manifest.json b/skills-manifest.json index a0384101e1..b4e8c78d8a 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -46,7 +46,7 @@ "files": 10 }, "media-use": { - "hash": "389942983c2c78c2", + "hash": "e3f818afb0afbb34", "files": 122 }, "motion-graphics": { diff --git a/skills/media-use/audio/scripts/lib/tts.mjs b/skills/media-use/audio/scripts/lib/tts.mjs index 1b61e58fcd..b708594744 100644 --- a/skills/media-use/audio/scripts/lib/tts.mjs +++ b/skills/media-use/audio/scripts/lib/tts.mjs @@ -128,6 +128,17 @@ export function resolveNpxCliFromNpmExecPath( return pathExists(npxCliPath) ? npxCliPath : null; } +export function resolveNpxCliPath( + npmExecPath = process.env.npm_execpath, + nodeExecPath = process.env.npm_node_execpath || process.execPath, + pathExists = existsSync, +) { + const fromNpm = resolveNpxCliFromNpmExecPath(npmExecPath, pathExists); + if (fromNpm) return fromNpm; + const besideNode = join(dirname(nodeExecPath), "node_modules", "npm", "bin", "npx-cli.js"); + return pathExists(besideNode) ? besideNode : null; +} + export function resolveSpawnCommand( cmd, args, @@ -143,10 +154,11 @@ export function resolveSpawnCommand( // On Windows, npx resolves to npx.cmd, which Node cannot execute directly. // Avoid `shell:true` and the .cmd shim entirely by invoking npm's JS CLI with // node, preserving request-provided values as argv data instead of shell text. - const npxCliPath = resolveNpxCliFromNpmExecPath(env.npm_execpath, pathExists); + const nodeExecPath = env.npm_node_execpath || process.execPath; + const npxCliPath = resolveNpxCliPath(env.npm_execpath, nodeExecPath, pathExists); if (!npxCliPath) return null; return { - cmd: env.npm_node_execpath || process.execPath, + cmd: nodeExecPath, args: [npxCliPath, ...args.map((arg) => String(arg))], opts: { stdio: "ignore", windowsHide: true, ...opts }, }; @@ -174,17 +186,17 @@ export function spawnP( const resolved = resolveSpawnCommand(cmd, args, opts, platform, env, pathExists); if (!resolved) { // resolveSpawnCommand only returns null for the npx-on-win32 case where - // npm_execpath isn't set (e.g. audio.mjs invoked directly with `node`, not - // through npm/npx). Without this, every call silently returns status:-1 and - // stdio:"ignore" hides why — callers just report "TTS failed - omitted" for - // every line. Surface the real reason once so it's diagnosable. + // neither npm's configured CLI nor the beside-node fallback exists. Without + // this, every call silently returns status:-1 and stdio:"ignore" hides why. if (!_warnedNpxResolution) { _warnedNpxResolution = true; + const reason = env.npm_execpath + ? `npm_execpath (${env.npm_execpath}) and the beside-node npm fallback could not be found` + : "npm_execpath is unset and the beside-node npm fallback could not be found"; console.error( - `[media-use] Cannot run "${cmd}" on Windows: npm_execpath is not set, so the ` + - `npx JS CLI can't be located. This happens when this script is run directly with ` + - `\`node\` instead of through npm/npx. Every "${cmd}" call is being skipped. ` + - `Fix: run via \`npx\`/\`npm run\`, or export npm_execpath pointing at your npm-cli.js.`, + `[media-use] Cannot run "${cmd}" on Windows: ${reason}. ` + + `Every "${cmd}" call is being skipped. Install npm with Node, or run via ` + + `\`npx\`/\`npm run\` with a valid npm_execpath.`, ); } return Promise.resolve({ status: -1 }); diff --git a/skills/media-use/audio/scripts/lib/tts.spawn.test.mjs b/skills/media-use/audio/scripts/lib/tts.spawn.test.mjs index 9262f7395d..b8e6b49a9b 100644 --- a/skills/media-use/audio/scripts/lib/tts.spawn.test.mjs +++ b/skills/media-use/audio/scripts/lib/tts.spawn.test.mjs @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { EventEmitter } from "node:events"; import { resolveNpxCliFromNpmExecPath, + resolveNpxCliPath, resolveSpawnCommand, spawnP, _resetNpxResolutionWarnForTests, @@ -33,6 +34,15 @@ test("resolveNpxCliFromNpmExecPath finds npx-cli next to npm-cli", () => { assert.equal(resolveNpxCliFromNpmExecPath(envWithNpxCli.npm_execpath, pathExists), npxCliPath); }); +test("resolveNpxCliPath finds npx-cli beside node when npm_execpath is unset", () => { + const node = "C:/Program Files/nodejs/node.exe"; + const expected = "C:/Program Files/nodejs/node_modules/npm/bin/npx-cli.js"; + assert.equal( + resolveNpxCliPath(undefined, node, (path) => path === expected), + expected, + ); +}); + test("resolveSpawnCommand routes npx through node+npx-cli on win32 without shell:true", () => { const resolved = resolveSpawnCommand( "npx", @@ -101,43 +111,48 @@ test("spawnP does not enable shell for non-npx commands even on win32", async () assert.equal(captured[0].opts.shell, undefined); }); -// Regression: win32 + npx with npm_execpath unset can't locate the npx JS CLI, -// so resolveSpawnCommand returns null and spawnP short-circuits. Previously it -// returned {status:-1} silently — every TTS line just dropped as "TTS failed - -// omitted" with no hint. Now it must surface a clear one-time diagnostic naming -// npm_execpath, while still returning {status:-1} without spawning anything. -test("spawnP surfaces a clear diagnostic (once) when npx can't be resolved on win32", async () => { +test("spawnP resolves npx beside node when npm_execpath is unset on win32", async () => { + _resetNpxResolutionWarnForTests(); + const captured = []; + const node = "C:/Program Files/nodejs/node.exe"; + const npxCli = "C:/Program Files/nodejs/node_modules/npm/bin/npx-cli.js"; + const result = await spawnP( + "npx", + ["hyperframes", "tts"], + {}, + "win32", + fakeSpawn(captured), + { npm_node_execpath: node }, + (path) => path === npxCli, + ); + assert.equal(result.status, 0); + assert.equal(captured.length, 1); + assert.equal(captured[0].cmd, node); + assert.deepEqual(captured[0].args, [npxCli, "hyperframes", "tts"]); +}); + +test("spawnP warns once with an accurate diagnostic when neither npx path exists", async () => { _resetNpxResolutionWarnForTests(); const errors = []; const originalError = console.error; - console.error = (msg) => errors.push(msg); - const captured = []; - const emptyEnv = {}; // no npm_execpath + console.error = (message) => errors.push(String(message)); try { - const r1 = await spawnP( - "npx", - ["hyperframes", "tts"], - {}, - "win32", - fakeSpawn(captured), - emptyEnv, - () => false, + const env = { npm_execpath: "C:/missing/npm-cli.js", npm_node_execpath: "C:/node/node.exe" }; + const missing = () => false; + assert.equal( + (await spawnP("npx", ["hyperframes", "tts"], {}, "win32", fakeSpawn([]), env, missing)) + .status, + -1, ); - const r2 = await spawnP( - "npx", - ["hyperframes", "tts"], - {}, - "win32", - fakeSpawn(captured), - emptyEnv, - () => false, + assert.equal( + (await spawnP("npx", ["hyperframes", "tts"], {}, "win32", fakeSpawn([]), env, missing)) + .status, + -1, ); - assert.equal(r1.status, -1); - assert.equal(r2.status, -1); - assert.equal(captured.length, 0, "must not spawn anything when resolution fails"); - assert.equal(errors.length, 1, "diagnostic is emitted once for a batch, not per line"); - assert.match(errors[0], /npm_execpath/); } finally { console.error = originalError; } + assert.equal(errors.length, 1); + assert.match(errors[0], /npm_execpath \(C:\/missing\/npm-cli\.js\)/); + assert.doesNotMatch(errors[0], /npm_execpath is not set/); });