fix(add): make chosen variables actually take effect, in the CLI and the preview

packages/cli/src/commands/add.test.ts

@@ -3,7 +3,14 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "no
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import type { RegistryItem, RegistryManifest } from "@hyperframes/core";
-import { AddError, buildSnippet, parseVariableValues, remapTarget, runAdd } from "./add.js";
+import {
+  AddError,
+  buildSnippet,
+  describeInstallFailure,
+  parseVariableValues,
+  remapTarget,
+  runAdd,
+} from "./add.js";
 import { trackRegistryItemAdded } from "../telemetry/events.js";
 
 // Assert the emitted payload rather than the transport: `shouldTrack()` is
@@ -462,3 +469,46 @@ describe("variable values in the snippet", () => {
     expect(parseVariableValues(undefined)).toBeNull();
   });
 });
+
+describe("describeInstallFailure", () => {
+  it("explains a bare transport failure instead of echoing it", () => {
+    // What the user actually sees after copying a command off the catalog page.
+    // Item FILES are not cached, so a blip surfaces as node's `fetch failed`
+    // with no URL and no cause, and reads like the command was wrong.
+    const message = describeInstallFailure(new Error("fetch failed"));
+
+    expect(message).toContain("could not download the item's files");
+    expect(message).toContain("rather than a bad command");
+    expect(message).toContain("HTTPS_PROXY");
+  });
+
+  it("names the project's own registry when it is not the public one", () => {
+    // The reported failure: hyperframes.json pointed at a private host with a
+    // self-signed certificate. Telling that reader to check their connection
+    // sends them to debug the one thing that was working.
+    const message = describeInstallFailure(
+      new Error("fetch failed"),
+      "https://private.example/registry",
+    );
+
+    expect(message).toContain("https://private.example/registry");
+    expect(message).toContain("not the public registry");
+  });
+
+  it("stays quiet about the registry when it is the default one", () => {
+    const message = describeInstallFailure(
+      new Error("fetch failed"),
+      "https://raw.githubusercontent.com/heygen-com/hyperframes/main/registry",
+    );
+
+    expect(message).not.toContain("not the public registry");
+  });
+
+  it("leaves a non-transport failure exactly as it was", () => {
+    // An unsafe target or a malformed item is the caller's problem to read; a
+    // connectivity lecture there would send them to fix the wrong thing.
+    const message = describeInstallFailure(new Error('Unsafe target "../x"'));
+
+    expect(message).toBe('Install failed: Unsafe target "../x"');
+  });
+});

packages/cli/src/commands/add.ts

@@ -15,7 +15,7 @@ import { existsSync } from "node:fs";
 import { resolve, relative } from "node:path";
 import { ITEM_TYPE_DIRS, type RegistryItem } from "@hyperframes/core";
 import { c } from "../ui/colors.js";
-import { installItem, resolveItemsByTag } from "../registry/index.js";
+import { DEFAULT_REGISTRY_URL, installItem, resolveItemsByTag } from "../registry/index.js";
 import { resolveItemWithDependencies } from "../registry/resolver.js";
 import {
   gateRegistryItemsCompatibility,
@@ -140,6 +140,8 @@ export interface RunAddResult {
   installed: string[];
   snippet: string;
   clipboardCopied: boolean;
+  /** Variable ids whose default was baked into an installed component. */
+  variablesApplied: string[];
   warnings: string[];
 }
 
@@ -181,22 +183,75 @@ async function installAll(
   destDir: string,
   baseUrl: string | undefined,
   force: boolean,
-): Promise<{ written: string[]; preserved: string[] }> {
+  requestedName: string,
+  variableValues: Record<string, unknown> | null,
+): Promise<{
+  written: string[];
+  preserved: string[];
+  variablesApplied: string[];
+  variablesUnknown: string[];
+  variablesInvalid: { id: string; reason: string }[];
+}> {
   const written: string[] = [];
   const preserved: string[] = [];
+  let variablesApplied: string[] = [];
+  let variablesUnknown: string[] = [];
+  let variablesInvalid: { id: string; reason: string }[] = [];
   try {
     for (const planItem of installPlan) {
-      const result = await installItem(planItem, { destDir, baseUrl, force });
+      const result = await installItem(planItem, {
+        destDir,
+        baseUrl,
+        force,
+        // Only the item the user named. A dependency dragged in behind it never
+        // declared these variables and must not be rewritten by them.
+        variableValues: planItem.name === requestedName ? variableValues : null,
+      });
       written.push(...result.written);
       preserved.push(...result.preserved);
+      if (planItem.name === requestedName) {
+        variablesApplied = result.variablesApplied;
+        variablesUnknown = result.variablesUnknown;
+        variablesInvalid = result.variablesInvalid;
+      }
     }
   } catch (err) {
-    throw new AddError(
-      `Install failed: ${err instanceof Error ? err.message : String(err)}`,
-      "install-failed",
-    );
+    throw new AddError(describeInstallFailure(err, baseUrl), "install-failed");
   }
-  return { written, preserved };
+  return { written, preserved, variablesApplied, variablesUnknown, variablesInvalid };
+}
+
+/**
+ * Turn a transport failure into something a reader can act on.
+ *
+ * Item FILES are not cached (only manifests are), so a network blip surfaces
+ * here as node's bare `fetch failed` with no URL, no cause and no suggestion.
+ * That is what a user sees after copying a command off the catalog page, and
+ * it reads like the command was wrong rather than the network.
+ */
+export function describeInstallFailure(err: unknown, registry?: string): string {
+  const message = err instanceof Error ? err.message : String(err);
+  const cause = err instanceof Error && err.cause instanceof Error ? err.cause.message : "";
+  const transport =
+    /fetch failed|ENOTFOUND|ECONNRESET|ECONNREFUSED|ETIMEDOUT|EAI_AGAIN|socket hang up|aborted/i;
+  if (!transport.test(`${message} ${cause}`)) return `Install failed: ${message}`;
+
+  // Name the registry first. A project that set `registry` in hyperframes.json
+  // points at a private host, and when that host is down the failure has
+  // nothing to do with the user's connection -- telling them to check their
+  // network sends them to debug the one thing that is working.
+  const custom =
+    registry && !registry.startsWith(DEFAULT_REGISTRY_URL)
+      ? `\n  This project's hyperframes.json sets registry to ${registry}, so that is the host ` +
+        "being contacted, not the public registry. If it is down or private, that is the failure."
+      : "";
+  return (
+    `Install failed: could not download the item's files.\n  ${message}` +
+    "\n  Item files are not cached, so every install fetches them. This is usually the " +
+    "registry host or the network rather than a bad command." +
+    custom +
+    "\n  Retry, or set HTTPS_PROXY if you are behind a proxy."
+  );
 }
 
 export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
@@ -244,12 +299,16 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
   }));
 
   // 5. Install — dependencies first, requested item last.
-  const { written, preserved } = await installAll(
-    installPlan,
-    projectDir,
-    config.registry,
-    opts.force ?? false,
-  );
+  const variableValues = parseVariableValues(opts.vars);
+  const { written, preserved, variablesApplied, variablesUnknown, variablesInvalid } =
+    await installAll(
+      installPlan,
+      projectDir,
+      config.registry,
+      opts.force ?? false,
+      item.name,
+      variableValues,
+    );
 
   // Report what landed, not what was asked for: a failed install throws above,
   // and the bulk `add <tag>` path re-enters here per item, so this one place
@@ -269,9 +328,16 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
     itemForInstall.files.find((f) => f.type === "hyperframes:composition") ??
     itemForInstall.files[0];
   const snippetTargetRel = primaryFile?.target ?? "";
-  const snippet = buildSnippet(item, snippetTargetRel, parseVariableValues(opts.vars));
+  const snippet = buildSnippet(item, snippetTargetRel, variableValues);
   const clipboardCopied = !opts.skipClipboard && snippet ? copyToClipboard(snippet) : false;
 
+  for (const { id, reason } of variablesInvalid) {
+    warnings.push(`--vars ${id} ignored: ${reason}`);
+  }
+  if (variablesUnknown.length > 0) {
+    warnings.push(`--vars ignored (not declared by ${item.name}): ${variablesUnknown.join(", ")}`);
+  }
+
   return {
     ok: true,
     name: item.name,
@@ -282,6 +348,7 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
     installed: installPlan.map((planItem) => planItem.name),
     snippet,
     clipboardCopied,
+    variablesApplied,
     warnings,
   };
 }
@@ -374,6 +441,12 @@ export default defineCommand({
       for (const file of result.written) {
         console.log(`  ${c.dim(relative(projectDir, file))}`);
       }
+      if (result.variablesApplied.length > 0) {
+        // Say it out loud. A component's values are baked into the file rather
+        // than shown in the snippet, so without this the command looks
+        // identical whether --vars worked or was thrown away.
+        console.log(`  ${c.dim(`variables applied: ${result.variablesApplied.join(", ")}`)}`);
+      }
       if (result.snippet) {
         console.log("");
         console.log(c.dim("Include snippet:"));

packages/cli/src/commands/transcribe.test.ts

@@ -48,8 +48,16 @@ describe("transcribe command", () => {
   it("explicit run exits non-zero and is NOT reported as a command failure", async () => {
     const { dir, input } = dummyAudio();
     dirs.push(dir);
-    await transcribeCmd.run!({ args: { input, json: true, optional: false } } as never);
+    // Pin the engine. `auto` picks Parakeet whenever parakeet-mlx happens to be
+    // installed, and only the whisper path is mocked here -- so on those
+    // machines this test used to shell out to a real ASR binary, fail with
+    // "Parakeet did not produce output", and land in the generic failure branch
+    // instead of the soft-skip it is asserting.
+    await transcribeCmd.run!({
+      args: { input, json: true, optional: false, engine: "whisper" },
+    } as never);
 
+    expect(transcribeMock).toHaveBeenCalled();
     expect(consumeCommandResult().exitCode).toBe(1);
     expect(trackTranscribeUnavailable).toHaveBeenCalledWith({ optional: false });
     expect(trackCommandFailure).not.toHaveBeenCalled();
@@ -58,8 +66,14 @@ describe("transcribe command", () => {
   it("--optional skips cleanly with exit 0", async () => {
     const { dir, input } = dummyAudio();
     dirs.push(dir);
-    await transcribeCmd.run!({ args: { input, json: true, optional: true } } as never);
+    await transcribeCmd.run!({
+      args: { input, json: true, optional: true, engine: "whisper" },
+    } as never);
 
+    // Asserting the mock ran is what keeps this honest: without it the test
+    // passes on a machine with no Parakeet and silently tests nothing on one
+    // that has it.
+    expect(transcribeMock).toHaveBeenCalled();
     expect(consumeCommandResult().exitCode).toBe(0);
     expect(trackTranscribeUnavailable).toHaveBeenCalledWith({ optional: true });
     expect(trackCommandFailure).not.toHaveBeenCalled();

packages/cli/src/registry/index.ts

@@ -1,21 +1,8 @@
-export {
-  DEFAULT_REGISTRY_URL,
-  fetchRegistryManifest,
-  fetchItemManifest,
-  fetchItemFile,
-} from "./remote.js";
+// Only what other modules actually import. Everything else in this folder is
+// reached through its own module, so re-exporting it here just creates surface
+// that has to be kept working without anyone depending on it.
+export { DEFAULT_REGISTRY_URL } from "./remote.js";
 
-export {
-  listRegistryItems,
-  loadAllItems,
-  resolveItem,
-  resolveItemsByTag,
-  type ResolveOptions,
-} from "./resolver.js";
+export { listRegistryItems, loadAllItems, resolveItemsByTag } from "./resolver.js";
 
-export {
-  installItem,
-  assertSafeTarget,
-  type InstallOptions,
-  type InstallResult,
-} from "./installer.js";
+export { installItem } from "./installer.js";