fix(media): resolve npx without npm_execpath on Windows (#2236)

* fix(media): resolve npx without npm_execpath on Windows

* test(media): preserve npx resolution diagnostics

* chore: refresh skills manifest

* chore: refresh skills manifest after rebase

skills-manifest.json

@@ -46,7 +46,7 @@
       "files": 10
     },
     "media-use": {
-      "hash": "389942983c2c78c2",
+      "hash": "e3f818afb0afbb34",
       "files": 122
     },
     "motion-graphics": {

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 });

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/);
 });