feat(media-use): classify and tag heygen provider telemetry, harden test isolation

skills-manifest.json

@@ -46,7 +46,7 @@
       "files": 10
     },
     "media-use": {
-      "hash": "f6f3af6648b1bd81",
+      "hash": "49aa5adae0800403",
       "files": 122
     },
     "motion-graphics": {

skills/media-use/audio/scripts/lib/heygen.mjs

@@ -69,6 +69,17 @@ export function heygenCredential() {
   return null;
 }
 
+// → "oauth" | "api_key" | null. Same oauth-vs-api-key check heygenAuthHeaders()
+// makes internally, exposed on its own so callers that only need to *tag* the
+// auth path (telemetry) don't have to parse headers back apart. Never throws:
+// no credential (or an expired one) is just `null`, same as a fresh resolve
+// with nothing to tag.
+export function heygenAuthMethod() {
+  const cred = heygenCredential();
+  if (!cred?.headers) return null;
+  return "Authorization" in cred.headers ? "oauth" : "api_key";
+}
+
 // → auth headers object, or throw with a fix hint.
 export function heygenAuthHeaders() {
   const cred = heygenCredential();

skills/media-use/audio/scripts/lib/heygen.test.mjs

@@ -3,7 +3,7 @@ import assert from "node:assert/strict";
 import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { heygenAuthHeaders } from "./heygen.mjs";
+import { heygenAuthHeaders, heygenAuthMethod } from "./heygen.mjs";
 
 function withCleanHeygenEnv(fn) {
   const previousApiKey = process.env.HEYGEN_API_KEY;
@@ -58,3 +58,43 @@ test("heygenAuthHeaders tags OAuth requests as CLI traffic", () => {
     }
   });
 });
+
+test("heygenAuthMethod returns api_key for an env API key, without tagging headers", () => {
+  withCleanHeygenEnv(() => {
+    process.env.HEYGEN_API_KEY = "hg_test";
+    assert.equal(heygenAuthMethod(), "api_key");
+  });
+});
+
+test("heygenAuthMethod returns oauth for a live OAuth credential", () => {
+  withCleanHeygenEnv(() => {
+    const dir = mkdtempSync(join(tmpdir(), "heygen-cred-"));
+    try {
+      process.env.HEYGEN_CONFIG_DIR = dir;
+      writeFileSync(
+        join(dir, "credentials"),
+        JSON.stringify({
+          oauth: {
+            access_token: "at_test",
+            expires_at: "2099-01-01T00:00:00Z",
+          },
+        }),
+      );
+      assert.equal(heygenAuthMethod(), "oauth");
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});
+
+test("heygenAuthMethod returns null with no credential at all", () => {
+  withCleanHeygenEnv(() => {
+    const dir = mkdtempSync(join(tmpdir(), "heygen-cred-"));
+    try {
+      process.env.HEYGEN_CONFIG_DIR = dir; // no credentials file written
+      assert.equal(heygenAuthMethod(), null);
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});

skills/media-use/scripts/lib/heygen-cli.mjs

@@ -1,3 +1,5 @@
+import { track } from "./telemetry.mjs";
+
 // v0.3.0 is the first CLI that can use an OAuth session; v0.1.x/0.2.x reject it
 // ("heygen-cli can't use OAuth yet"), and OAuth is what the free-usage path
 // needs — so anything below this can't authenticate for free usage at all.
@@ -20,6 +22,14 @@ const ACTIONABLE_MESSAGES = new Set([
 ]);
 
 export function classifyHeygenError(err) {
+  return classifyHeygenErrorResult(err).message;
+}
+
+export function classifyHeygenErrorCode(err) {
+  return classifyHeygenErrorResult(err).code;
+}
+
+function classifyHeygenErrorResult(err) {
   const detail = heygenErrorDetail(err);
   const text = [err?.stderr, err?.stdout, err?.message, detail]
     .map((value) => textOf(value))
@@ -33,7 +43,7 @@ export function classifyHeygenError(err) {
   // embeds the `heygen ...` command line — sending users to reinstall a CLI they
   // just ran successfully. Keep this narrow.
   if (err?.code === "ENOENT" || lower.includes("command not found")) {
-    return HEYGEN_NOT_FOUND_MESSAGE;
+    return { code: "not_found", message: HEYGEN_NOT_FOUND_MESSAGE };
   }
 
   if (
@@ -50,24 +60,63 @@ export function classifyHeygenError(err) {
     lower.includes("auth required") ||
     lower.includes("authentication required")
   ) {
-    return HEYGEN_NOT_AUTHENTICATED_MESSAGE;
+    return { code: "not_authenticated", message: HEYGEN_NOT_AUTHENTICATED_MESSAGE };
   }
 
   const version = firstSemver(text);
   if (version && versionLessThan(version, HEYGEN_MIN_VERSION)) {
-    return HEYGEN_OUTDATED_MESSAGE;
+    return { code: "outdated", message: HEYGEN_OUTDATED_MESSAGE };
   }
 
-  return detail;
+  if (
+    lower.includes("rate limit") ||
+    lower.includes("quota") ||
+    lower.includes("insufficient credit") ||
+    lower.includes("too many requests") ||
+    lower.includes("throttled") ||
+    /\b429\b/.test(lower)
+  ) {
+    return { code: "rate_limited", message: detail };
+  }
+
+  return { code: "other", message: detail };
 }
 
-export function reportHeygenFailure(err, context) {
-  const message = classifyHeygenError(err);
+// reportHeygenFailure's callers (voice-provider.mjs, heygen-search.mjs) are
+// synchronous and several layers below the CLI's process.exit() calls, so
+// they can't await this tracking call themselves. Stash each attempt's
+// promise here so a caller closer to exit (resolve.mjs) can join it first —
+// same "awaited so a short-lived run flushes it" discipline telemetry.mjs's
+// track() already documents, just reachable from a sync call site.
+const pendingFailureTracking = new Set();
+
+export function reportHeygenFailure(err, context, trackEvent = track) {
+  const { code, message } = classifyHeygenErrorResult(err);
   if (ACTIONABLE_MESSAGES.has(message)) {
     console.error(message);
   } else {
     console.error(`media-use: \`${context}\` failed: ${message}`);
   }
+  try {
+    const tracked = Promise.resolve(
+      trackEvent("media_use_provider_error", { provider: "heygen", reason: code }),
+    ).catch(() => {});
+    pendingFailureTracking.add(tracked);
+    void tracked.finally(() => pendingFailureTracking.delete(tracked));
+    return tracked;
+  } catch {
+    // Telemetry must never affect the provider failure path.
+    return Promise.resolve();
+  }
+}
+
+// Awaits every provider-error track fired since the last flush, so a caller
+// about to process.exit() doesn't orphan one mid-request (both are separate,
+// non-keepalive HTTP connections with no ordering guarantee otherwise).
+// Never rejects: each tracked promise already swallows its own failure.
+export async function flushHeygenFailureTracking() {
+  if (pendingFailureTracking.size === 0) return;
+  await Promise.all(pendingFailureTracking);
 }
 
 export function firstSemver(text) {

skills/media-use/scripts/lib/heygen-cli.test.mjs

@@ -1,12 +1,32 @@
 import { strict as assert } from "node:assert";
+import { spawnSync } from "node:child_process";
 import { test } from "node:test";
 import {
   classifyHeygenError,
+  classifyHeygenErrorCode,
+  flushHeygenFailureTracking,
   HEYGEN_NOT_AUTHENTICATED_MESSAGE,
   HEYGEN_NOT_FOUND_MESSAGE,
   HEYGEN_OUTDATED_MESSAGE,
+  reportHeygenFailure,
 } from "./heygen-cli.mjs";
 
+function captureFailureReport(err, context, trackEvent) {
+  const originalError = console.error;
+  const stderrCalls = [];
+  console.error = (...args) => stderrCalls.push(args);
+  try {
+    if (trackEvent) {
+      reportHeygenFailure(err, context, trackEvent);
+    } else {
+      reportHeygenFailure(err, context);
+    }
+  } finally {
+    console.error = originalError;
+  }
+  return stderrCalls;
+}
+
 test("classifies ENOENT-style missing heygen errors with install instructions", () => {
   const message = classifyHeygenError({ code: "ENOENT", message: "spawn heygen ENOENT" });
 
@@ -64,3 +84,184 @@ test("passes through unrelated errors", () => {
 
   assert.equal(message, "rate limit exceeded");
 });
+
+test("classifies existing HeyGen failures with stable reason codes", () => {
+  assert.equal(classifyHeygenErrorCode({ code: "ENOENT" }), "not_found");
+  assert.equal(
+    classifyHeygenErrorCode({ stderr: Buffer.from("HTTP 401 Unauthorized") }),
+    "not_authenticated",
+  );
+  assert.equal(
+    classifyHeygenErrorCode({ stderr: Buffer.from("heygen v0.1.5 is unsupported") }),
+    "outdated",
+  );
+  assert.equal(classifyHeygenErrorCode({ stderr: Buffer.from("provider unavailable") }), "other");
+});
+
+test("classifies rate-limit text case-insensitively", () => {
+  assert.equal(
+    classifyHeygenErrorCode({ stderr: Buffer.from("RATE LIMIT exceeded") }),
+    "rate_limited",
+  );
+});
+
+test("classifies quota and insufficient-credit errors as rate limited", () => {
+  for (const detail of ["Quota exhausted", "INSUFFICIENT CREDIT remaining"]) {
+    assert.equal(classifyHeygenErrorCode({ stderr: Buffer.from(detail) }), "rate_limited");
+  }
+});
+
+test("classifies the literal 429 reason phrase and throttling language as rate limited", () => {
+  for (const detail of ["Too Many Requests", "Error: throttled by upstream, retry later"]) {
+    assert.equal(classifyHeygenErrorCode({ stderr: Buffer.from(detail) }), "rate_limited");
+  }
+});
+
+test("does not misclassify unrelated errors that share a word with the new phrasing", () => {
+  // Shares "too many" with "too many requests" but is a distinct failure (fd
+  // exhaustion, not a rate limit) — the match must require the full phrase.
+  assert.equal(
+    classifyHeygenErrorCode({ stderr: Buffer.from("Too many open file descriptors") }),
+    "other",
+  );
+});
+
+test("classifies a bare 429 as rate limited without matching request IDs", () => {
+  assert.equal(
+    classifyHeygenErrorCode({ stderr: Buffer.from("HTTP 429 Too Many Requests") }),
+    "rate_limited",
+  );
+  assert.equal(
+    classifyHeygenErrorCode({ stderr: Buffer.from("request req-429abc failed") }),
+    "other",
+  );
+});
+
+test("tracks not-found failures without changing actionable output", () => {
+  const trackingCalls = [];
+  const stderrCalls = captureFailureReport({ code: "ENOENT" }, "heygen asset search", (...args) =>
+    trackingCalls.push(args),
+  );
+
+  assert.deepEqual(stderrCalls, [[HEYGEN_NOT_FOUND_MESSAGE]]);
+  assert.deepEqual(trackingCalls, [
+    ["media_use_provider_error", { provider: "heygen", reason: "not_found" }],
+  ]);
+});
+
+test("tracks generic failures without including raw detail", () => {
+  const trackingCalls = [];
+  const stderrCalls = captureFailureReport(
+    { stderr: Buffer.from("private provider detail") },
+    "heygen asset search",
+    (...args) => trackingCalls.push(args),
+  );
+
+  assert.deepEqual(stderrCalls, [
+    ["media-use: `heygen asset search` failed: private provider detail"],
+  ]);
+  assert.deepEqual(trackingCalls, [
+    ["media_use_provider_error", { provider: "heygen", reason: "other" }],
+  ]);
+});
+
+test("keeps failure output observable when telemetry is opted out", () => {
+  const previousOptOut = process.env.HYPERFRAMES_NO_TELEMETRY;
+  process.env.HYPERFRAMES_NO_TELEMETRY = "1";
+  try {
+    const stderrCalls = captureFailureReport({ code: "ENOENT" }, "heygen asset search");
+
+    assert.deepEqual(stderrCalls, [[HEYGEN_NOT_FOUND_MESSAGE]]);
+  } finally {
+    if (previousOptOut === undefined) {
+      delete process.env.HYPERFRAMES_NO_TELEMETRY;
+    } else {
+      process.env.HYPERFRAMES_NO_TELEMETRY = previousOptOut;
+    }
+  }
+});
+
+test("keeps failure output observable when tracking throws synchronously", () => {
+  const originalError = console.error;
+  const stderrCalls = [];
+  let thrown;
+  console.error = (...args) => stderrCalls.push(args);
+  try {
+    try {
+      reportHeygenFailure({ code: "ENOENT" }, "heygen asset search", () => {
+        throw new Error("tracking failed");
+      });
+    } catch (err) {
+      thrown = err;
+    }
+  } finally {
+    console.error = originalError;
+  }
+
+  assert.deepEqual(stderrCalls, [[HEYGEN_NOT_FOUND_MESSAGE]]);
+  assert.equal(thrown, undefined);
+});
+
+test("does not leave rejected tracking promises unhandled", () => {
+  const moduleUrl = new URL("./heygen-cli.mjs", import.meta.url).href;
+  const script = `
+    import { reportHeygenFailure } from ${JSON.stringify(moduleUrl)};
+    reportHeygenFailure(
+      { stderr: "provider unavailable" },
+      "heygen asset search",
+      () => Promise.reject(new Error("tracking failed")),
+    );
+    await new Promise((resolve) => setImmediate(resolve));
+  `;
+  const child = spawnSync(
+    process.execPath,
+    ["--unhandled-rejections=strict", "--input-type=module", "--eval", script],
+    { encoding: "utf8", timeout: 5000 },
+  );
+
+  assert.equal(child.error, undefined);
+  assert.equal(child.signal, null);
+  assert.equal(child.status, 0, child.stderr);
+  assert.equal(child.stderr, "media-use: `heygen asset search` failed: provider unavailable\n");
+});
+
+test("flushHeygenFailureTracking waits for a pending report before resolving", async () => {
+  const events = [];
+  let releaseTrack;
+  const gate = new Promise((resolve) => {
+    releaseTrack = resolve;
+  });
+
+  // Mirrors the real call sites (voice-provider.mjs, heygen-search.mjs):
+  // fire-and-forget, the return value is never awaited by the caller.
+  reportHeygenFailure({ code: "ENOENT" }, "heygen voice speech", () =>
+    gate.then(() => {
+      events.push("track-settled");
+    }),
+  );
+
+  const flushed = flushHeygenFailureTracking().then(() => {
+    events.push("flush-resolved");
+  });
+
+  // Let several pending microtasks drain before releasing the gate, so this
+  // proves flush is genuinely still waiting on the tracked promise -- not
+  // merely that it hasn't had a tick yet.
+  await Promise.resolve();
+  await Promise.resolve();
+  await Promise.resolve();
+  assert.deepEqual(events, [], "flush must not resolve while the tracked promise is still pending");
+
+  releaseTrack();
+  await flushed;
+
+  assert.deepEqual(
+    events,
+    ["track-settled", "flush-resolved"],
+    "flush must resolve only after the pending track settles, in that order",
+  );
+});
+
+test("flushHeygenFailureTracking resolves immediately when nothing is pending", async () => {
+  await flushHeygenFailureTracking();
+});