fix(skills): make the carve CLI work against the published core, and honour its own group invariant

skills-manifest.json

@@ -26,8 +26,8 @@
       "files": 121
     },
     "hyperframes-audio": {
-      "hash": "819ab4e70d0f1cc9",
-      "files": 6
+      "hash": "0b5e0ac8d731e1d2",
+      "files": 7
     },
     "hyperframes-cli": {
       "hash": "3fa884269c43d7df",

skills/hyperframes-audio/SKILL.md

@@ -261,6 +261,16 @@ A `sources` list naming two or more plain clip ids instead of a group is caught
 by the `audio_carve_ungrouped_sources` lint rule — it still works, but it is the
 version that silently rots when a clip is added.
 
+**The bed must not be a member of the group it carves against.** A group id in
+`sources` resolves to every current member on every analysis, so a bed inside
+that group is handed itself as a voice and carved against its own content —
+which is the "never carve a track against itself" rule arriving one re-analysis
+later, after a first pass that looked correct. Give the bed its own group
+(`music`) and the narration its own (`voiceover`). `carve.mjs` refuses to write
+the group form when it detects this and records clip ids instead, so the lint
+rule points at the arrangement rather than the CLI quietly producing a
+self-carve.
+
 **One knob.** `strength` is 0..1 and derives everything: how deep to cut, how
 many bands, how wide, how far to favour intelligibility over raw voice energy,
 how far the level may drop, how far under the voice to aim. Those six move

skills/hyperframes-audio/scripts/carve.mjs

@@ -26,7 +26,7 @@
 
 import { execFileSync } from "node:child_process";
 import { createRequire } from "node:module";
-import { readFileSync, writeFileSync } from "node:fs";
+import { readFileSync, realpathSync, writeFileSync } from "node:fs";
 import { dirname, resolve } from "node:path";
 import { pathToFileURL } from "node:url";
 
@@ -81,28 +81,114 @@ function fail(message, code = 1) {
  * the skill was installed — a sibling of the composition is what has the
  * dependency.
  */
-async function loadCore(fromDir) {
+export async function loadCore(fromDir) {
   const require = createRequire(pathToFileURL(resolve(fromDir, "package.json")));
-  const load = (subpath) => {
-    const file = require.resolve(`@hyperframes/core/${subpath}`);
-    return import(pathToFileURL(file).href);
+
+  /*
+   * Two constraints at once, and satisfying either alone is broken:
+   *
+   *   1. Anchored at the PROJECT, not at this script. This file lives wherever
+   *      the skill was installed, which has no @hyperframes/core; the
+   *      composition's project is what holds the dependency. So a bare
+   *      `import("@hyperframes/core/audio-carve")` from here cannot work — bare
+   *      specifiers resolve relative to the importing module.
+   *   2. Honouring the package's export CONDITIONS. `require.resolve` asks for
+   *      "require"/"node". The workspace manifest declares `node`, so this
+   *      resolved fine inside the monorepo — but the PUBLISHED manifest carries
+   *      only `import` + `types`, so every consumer of the released package got
+   *      ERR_PACKAGE_PATH_NOT_EXPORTED for a package that ships the file. That
+   *      is the audience this skill is shipped to, so the script was broken
+   *      everywhere except where it was developed.
+   *
+   * Keep the project anchor; fall back to the package's declared `import`
+   * target when no require-resolvable condition exists.
+   */
+  const load = async (subpath) => {
+    const spec = `@hyperframes/core/${subpath}`;
+    try {
+      return await import(pathToFileURL(require.resolve(spec)).href);
+    } catch (error) {
+      if (error?.code !== "ERR_PACKAGE_PATH_NOT_EXPORTED") throw error;
+      // `./package.json` is exported by every manifest, so this always resolves
+      // and gives us the package root without guessing at node_modules layout.
+      const pkgPath = require.resolve("@hyperframes/core/package.json");
+      const pkg = JSON.parse(readFileSync(pkgPath, "utf-8"));
+      const entry = pkg.exports?.[`./${subpath}`];
+      const target = typeof entry === "string" ? entry : (entry?.import ?? entry?.default ?? null);
+      if (!target) {
+        fail(
+          `@hyperframes/core does not export ./${subpath}\n` +
+            `  found at: ${pkgPath} (version ${pkg.version})\n` +
+            `  update it:  npm i -D @hyperframes/core`,
+        );
+      }
+      return import(pathToFileURL(resolve(dirname(pkgPath), target)).href);
+    }
   };
+
   try {
     return {
       carve: await load("audio-carve"),
       fx: await load("audio-fx"),
     };
   } catch (error) {
     fail(
-      `cannot resolve @hyperframes/core from ${fromDir}\n` +
-        `  install or update it:  npm i -D @hyperframes/core\n` +
-        `  (the audio-carve export needs a version that ships the carve analysis)\n` +
+      `cannot load @hyperframes/core from ${fromDir}\n` +
+        `  is it installed there?  npm i -D @hyperframes/core\n` +
         `  or point at one:   --core <dir containing node_modules/@hyperframes/core>\n` +
-        `  (${error.message})`,
+        `  (${error.code ?? "error"}: ${error.message.split("\n")[0]})`,
     );
   }
 }
 
+/**
+ * The `sources` a carve should record for these voices, on this bed.
+ *
+ * SKILL.md states the invariant: "A carve against more than one clip id is
+ * wrong. Group the clips and carve against the group." Naming the group lets
+ * `resolveCarveSourceIds` resolve membership at analysis time, so a voice added
+ * later is covered without editing `sources` — whereas a list of clip ids rots
+ * silently the moment a fourth narration clip appears. The lint rule
+ * `audio_carve_ungrouped_sources` enforces exactly this.
+ *
+ * This script was writing clip ids unconditionally, so it violated its own
+ * skill's invariant and tripped its own lint rule on every run. When every
+ * voice shares one group, record the group. Mixed or ungrouped voices keep
+ * their ids, and the lint rule then correctly tells the author to group them.
+ *
+ * The bed has to be part of the decision, because the group form resolves
+ * LATER and wider than it looks. If the bed is itself a member of the voices'
+ * group, `resolveCarveSourceIds` expands that id to every current member on the
+ * next analysis — including the bed — and the bed ends up carved against
+ * itself, which SKILL.md calls a bug rather than a mix choice. This run cannot
+ * see it: `main()` sums the voice list it detected and never round-trips
+ * through group resolution, so the first pass is correct and only the next
+ * re-analysis in Studio is wrong. So decline the group form there and fall back
+ * to clip ids, which is exactly the case `audio_carve_ungrouped_sources` exists
+ * to put in front of the author.
+ *
+ * Only an `<audio>` bed can trip it: group membership is audio-only
+ * (`audioGroupOf`), so `data-audio-group` on a `<video>` bed is ignored by core
+ * and expanding a group can never pull it in.
+ */
+export function carveSources(voices, bed) {
+  const group = sharedVoiceGroup(voices);
+  return group && !bedInVoiceGroup(voices, bed) ? [group] : voices.map((v) => v.id);
+}
+
+/** The one group every voice belongs to, or null if they do not share exactly one. */
+function sharedVoiceGroup(voices) {
+  const groups = voices.map((v) => attrOf(v.tag, "data-audio-group"));
+  const first = groups[0];
+  return Boolean(first) && groups.every((g) => g === first) ? first : null;
+}
+
+/** The case above: naming this group would put the bed in its own source list. */
+export function bedInVoiceGroup(voices, bed) {
+  const group = sharedVoiceGroup(voices);
+  return Boolean(group) && bed?.kind === "audio" && attrOf(bed.tag, "data-audio-group") === group;
+}
+
 /** Mono float PCM for one media file, via ffmpeg. */
 function decode(path) {
   let raw;
@@ -354,7 +440,16 @@ async function main() {
     : [];
   const lanes = [...carriedLanes, ...carvedLanes];
 
-  const settings = { enabled: true, sources: voices.map((v) => v.id), strength: args.strength };
+  const settings = { enabled: true, sources: carveSources(voices, bedEl), strength: args.strength };
+  // Say why the group form was declined, or the lint rule tells the author to
+  // group clips they have already grouped.
+  if (bedInVoiceGroup(voices, bedEl)) {
+    process.stderr.write(
+      `note   bed ${bedEl.id} is in group "${attrOf(bedTag, "data-audio-group")}" with the voices,\n` +
+        `       so sources are clip ids: naming that group would carve the bed\n` +
+        `       against itself on the next analysis. Move the bed to its own group.\n`,
+    );
+  }
   const written =
     ` data-fx-carve="${escapeAttr(JSON.stringify(settings))}"` +
     ` data-fx-chain="${escapeAttr(fxApi.serializeAudioFxChain(chain))}"` +
@@ -389,4 +484,21 @@ async function main() {
   process.stdout.write(`wrote ${args.comp} (id="${bedEl.id}")\n`);
 }
 
-await main();
+// Only run as a CLI. Guarded so the pure helpers above can be unit-tested by
+// importing this module (`skills/**/*.test.mjs`, run by `bun run test:skills`).
+//
+// realpath both sides: on macOS /tmp → /private/tmp, and node resolves the main
+// module's symlinks in import.meta.url while argv[1] keeps the invoked spelling —
+// a raw compare silently skips main() when invoked through any symlinked path.
+function isMainModule(importMetaUrl) {
+  if (!process.argv[1]) return false;
+  try {
+    return pathToFileURL(realpathSync(process.argv[1])).href === importMetaUrl;
+  } catch {
+    return false;
+  }
+}
+
+if (isMainModule(import.meta.url)) {
+  await main();
+}

skills/hyperframes-audio/scripts/carve.test.mjs

@@ -0,0 +1,144 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import { execFileSync } from "node:child_process";
+import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+import { carveSources, loadCore } from "./carve.mjs";
+
+const SCRIPTS_DIR = dirname(fileURLToPath(import.meta.url));
+
+/** A voice as `detectTracks` yields it: an id plus its raw opening tag. */
+function voice(id, group) {
+  const attr = group === undefined ? "" : ` data-audio-group="${group}"`;
+  return { id, tag: `<audio id="${id}"${attr} src="${id}.wav"></audio>` };
+}
+
+/** A bed as `mediaElements` yields it: `kind` is what decides group membership. */
+function bed(id, group, kind = "audio") {
+  const attr = group === undefined ? "" : ` data-audio-group="${group}"`;
+  return { id, kind, tag: `<${kind} id="${id}"${attr} src="${id}.mp3"></${kind}>` };
+}
+
+async function inTempDir(run) {
+  const dir = mkdtempSync(join(tmpdir(), "carve-test-"));
+  try {
+    return await run(dir);
+  } finally {
+    rmSync(dir, { recursive: true, force: true });
+  }
+}
+
+test("voices sharing one group carve against the group, not their ids", () => {
+  // SKILL.md's invariant: "A carve against more than one clip id is wrong.
+  // Group the clips and carve against the group." Naming the group lets
+  // membership resolve at analysis time, so a voice added later is covered.
+  const voices = [voice("vo1", "voiceover"), voice("vo2", "voiceover"), voice("vo3", "voiceover")];
+  assert.deepEqual(carveSources(voices), ["voiceover"]);
+});
+
+test("a single grouped voice still records the group", () => {
+  assert.deepEqual(carveSources([voice("vo1", "voiceover")]), ["voiceover"]);
+});
+
+test("ungrouped voices keep their ids, so the lint rule can still say so", () => {
+  // Not silently inventing a group: `audio_carve_ungrouped_sources` is the
+  // right signal here, and it needs the ids to fire on.
+  assert.deepEqual(carveSources([voice("vo1"), voice("vo2")]), ["vo1", "vo2"]);
+});
+
+test("voices in DIFFERENT groups keep their ids", () => {
+  // One carve cannot name two groups, and picking either would silently drop
+  // the other's members from the analysis.
+  const voices = [voice("vo1", "narration"), voice("vo2", "interview")];
+  assert.deepEqual(carveSources(voices), ["vo1", "vo2"]);
+});
+
+test("a partially grouped set keeps its ids", () => {
+  const voices = [voice("vo1", "voiceover"), voice("vo2")];
+  assert.deepEqual(carveSources(voices), ["vo1", "vo2"]);
+});
+
+test("an empty group attribute is not a group", () => {
+  assert.deepEqual(carveSources([voice("vo1", ""), voice("vo2", "")]), ["vo1", "vo2"]);
+});
+
+test("a bed inside the voices' group keeps clip ids, so it is never its own source", () => {
+  // `resolveCarveSourceIds` expands a group to every CURRENT member, and it gets
+  // no host element to exclude — so naming a group the bed belongs to puts the
+  // bed in its own voice list on the next analysis, and it is carved against
+  // itself. This run cannot see it (main sums the detected voices directly), so
+  // the check has to happen here.
+  const voices = [voice("vo1", "mix"), voice("vo2", "mix")];
+  assert.deepEqual(carveSources(voices, bed("bgm", "mix")), ["vo1", "vo2"]);
+});
+
+test("a bed in a DIFFERENT group leaves the group form alone", () => {
+  const voices = [voice("vo1", "voiceover"), voice("vo2", "voiceover")];
+  assert.deepEqual(carveSources(voices, bed("bgm", "music")), ["voiceover"]);
+});
+
+test("an ungrouped bed leaves the group form alone", () => {
+  assert.deepEqual(carveSources([voice("vo1", "voiceover")], bed("bgm")), ["voiceover"]);
+});
+
+test("a VIDEO bed is immune — group membership is audio-only", () => {
+  // `data-audio-group` on a <video> is ignored by core, so expanding the group
+  // can never pull this bed in and declining would be a false positive.
+  const voices = [voice("vo1", "mix"), voice("vo2", "mix")];
+  assert.deepEqual(carveSources(voices, bed("clip", "mix", "video")), ["mix"]);
+});
+
+test("the CLI still runs when invoked through a symlinked path", () => {
+  // argv[1] keeps the invoked spelling while import.meta.url is the realpath, so
+  // a raw compare in the entry guard skips main() and the CLI exits 0 having
+  // written nothing. macOS /tmp -> /private/tmp reaches this with no symlink of
+  // one's own; so does any skill install placed behind a link.
+  return inTempDir((dir) => {
+    const link = join(dir, "scripts-link");
+    symlinkSync(SCRIPTS_DIR, link, "junction");
+    let status = 0;
+    let stderr = "";
+    try {
+      execFileSync(process.execPath, [join(link, "carve.mjs")], { encoding: "utf-8" });
+    } catch (error) {
+      status = error.status;
+      stderr = String(error.stderr);
+    }
+    // Reaching parseArgs is the proof main() ran at all: no args is an error, and
+    // the broken guard's symptom is a silent exit 0 with no output.
+    assert.equal(status, 1, `expected the usage error, got status ${status}`);
+    assert.match(stderr, /--comp is required/);
+  });
+});
+
+test("loadCore honours an import-only export map, as the published core has", () => {
+  // The published manifest carries only `import` + `types` for these subpaths, so
+  // `require.resolve` cannot resolve them at all and the fallback has to read the
+  // export map itself. A fixture pins that without depending on npm.
+  return inTempDir(async (dir) => {
+    const pkgDir = join(dir, "node_modules", "@hyperframes", "core");
+    mkdirSync(join(pkgDir, "dist"), { recursive: true });
+    writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "fixture-project" }));
+    writeFileSync(
+      join(pkgDir, "package.json"),
+      JSON.stringify({
+        name: "@hyperframes/core",
+        version: "0.0.0-fixture",
+        type: "module",
+        exports: {
+          "./package.json": "./package.json",
+          "./audio-carve": { import: "./dist/audioCarve.js" },
+          "./audio-fx": { import: "./dist/audioFx.js" },
+        },
+      }),
+    );
+    writeFileSync(join(pkgDir, "dist", "audioCarve.js"), 'export const marker = "carve";\n');
+    writeFileSync(join(pkgDir, "dist", "audioFx.js"), 'export const marker = "fx";\n');
+
+    const core = await loadCore(dir);
+    assert.equal(core.carve.marker, "carve");
+    assert.equal(core.fx.marker, "fx");
+  });
+});