Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion skills-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"files": 3
},
"hyperframes-media": {
"hash": "60f73ecb91858ea3",
"hash": "56190192ead45ed1",
"files": 47
},
"hyperframes-registry": {
Expand Down
25 changes: 24 additions & 1 deletion skills/hyperframes-media/scripts/lib/tts.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 }));
Expand Down
48 changes: 47 additions & 1 deletion skills/hyperframes-media/scripts/lib/tts.spawn.test.mjs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
}
});
Loading