From dfa6fedbcdaf8478b4be99d907d88fd079d9bc0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sun, 5 Jul 2026 21:29:04 -0400 Subject: [PATCH] fix(hyperframes-media): surface a clear error when npx can't be resolved on Windows (#1961) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, resolveSpawnCommand routes `npx` through node + npm's npx-cli.js (avoiding the un-spawnable npx.cmd), locating that CLI via npm_execpath. When the script is run directly with `node audio.mjs` instead of through npm/npx, npm_execpath is unset, so resolution returns null and spawnP short-circuited to `{status:-1}` — silently. With stdio:"ignore" hiding everything, callers just reported "TTS failed - omitted" for every single line, giving no hint that the real cause was an unresolvable npx. Debugging required reading the source. Fix: when spawnP hits that null-resolution path, emit a clear one-time diagnostic naming npm_execpath and the remedy (run via npx/npm, or export npm_execpath) before returning {status:-1}. One-shot latch so a batch of lines logs it once, not per line. Behavior is otherwise unchanged — still returns {status:-1} and spawns nothing. Test: new tts.spawn.test.mjs case — two consecutive win32 npx calls with npm_execpath unset both return {status:-1}, nothing is spawned, and the diagnostic (mentioning npm_execpath) is emitted exactly once. Existing spawn tests unchanged (7/7 pass). --- skills-manifest.json | 2 +- skills/hyperframes-media/scripts/lib/tts.mjs | 25 +++++++++- .../scripts/lib/tts.spawn.test.mjs | 48 ++++++++++++++++++- 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/skills-manifest.json b/skills-manifest.json index 0bb6f9c13..1a48a1f65 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -42,7 +42,7 @@ "files": 3 }, "hyperframes-media": { - "hash": "60f73ecb91858ea3", + "hash": "56190192ead45ed1", "files": 47 }, "hyperframes-registry": { diff --git a/skills/hyperframes-media/scripts/lib/tts.mjs b/skills/hyperframes-media/scripts/lib/tts.mjs index afe353e67..c41965b38 100644 --- a/skills/hyperframes-media/scripts/lib/tts.mjs +++ b/skills/hyperframes-media/scripts/lib/tts.mjs @@ -155,6 +155,13 @@ export function resolveSpawnCommand( // `platform`/`spawnFn` params (default process.platform / the real spawn) // exist so tests can exercise the win32 branch without mocking node:child_process // (its ESM exports are non-configurable, so mock.method can't patch it). +// One-shot so a whole batch of TTS lines doesn't repeat the same diagnostic. +let _warnedNpxResolution = false; +/** Test-only: reset the one-shot npx-resolution warning latch. */ +export function _resetNpxResolutionWarnForTests() { + _warnedNpxResolution = false; +} + export function spawnP( cmd, args, @@ -165,7 +172,23 @@ export function spawnP( pathExists = existsSync, ) { const resolved = resolveSpawnCommand(cmd, args, opts, platform, env, pathExists); - if (!resolved) return Promise.resolve({ status: -1 }); + 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. + if (!_warnedNpxResolution) { + _warnedNpxResolution = true; + console.error( + `[hyperframes-media] 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.`, + ); + } + return Promise.resolve({ status: -1 }); + } return new Promise((resolve) => { const p = spawnFn(resolved.cmd, resolved.args, resolved.opts); p.on("exit", (code) => resolve({ status: code ?? -1 })); diff --git a/skills/hyperframes-media/scripts/lib/tts.spawn.test.mjs b/skills/hyperframes-media/scripts/lib/tts.spawn.test.mjs index f7a6680ea..9262f7395 100644 --- a/skills/hyperframes-media/scripts/lib/tts.spawn.test.mjs +++ b/skills/hyperframes-media/scripts/lib/tts.spawn.test.mjs @@ -1,7 +1,12 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { EventEmitter } from "node:events"; -import { resolveNpxCliFromNpmExecPath, resolveSpawnCommand, spawnP } from "./tts.mjs"; +import { + resolveNpxCliFromNpmExecPath, + resolveSpawnCommand, + spawnP, + _resetNpxResolutionWarnForTests, +} from "./tts.mjs"; // Regression: on Windows, npx resolves to npx.cmd, which spawn() cannot exec // without shell:true — it fails ENOENT, silently swallowed as ok:false by the @@ -95,3 +100,44 @@ test("spawnP does not enable shell for non-npx commands even on win32", async () assert.deepEqual(captured[0].args, ["-c", "pass"]); 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 () => { + _resetNpxResolutionWarnForTests(); + const errors = []; + const originalError = console.error; + console.error = (msg) => errors.push(msg); + const captured = []; + const emptyEnv = {}; // no npm_execpath + try { + const r1 = await spawnP( + "npx", + ["hyperframes", "tts"], + {}, + "win32", + fakeSpawn(captured), + emptyEnv, + () => false, + ); + const r2 = await spawnP( + "npx", + ["hyperframes", "tts"], + {}, + "win32", + fakeSpawn(captured), + emptyEnv, + () => false, + ); + 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; + } +});