refactor(core,cli,engine): apply /simplify findings on getVariables PR

- core/runtime/getVariables.ts: collapse the noisy three-step type-guard
  re-cast into a single `Record<string, unknown>` narrow with early-continue
  guards. Same behaviour, ~6 lines shorter.
- cli/commands/render.ts: separate VariablesParseError from UI strings.
  parseVariablesArg now returns a kind-discriminated error
  (`conflict | read-error | parse-error | shape-error`) and the wrapper
  resolveVariablesArg owns the title/message mapping via
  `variablesErrorMessage`. Keeps the parser pure of presentation strings.
- cli/commands/render.test.ts: lift the `await import("./render.js")` into
  a `beforeAll`, add a typed `expectErr` helper, assert on the structured
  error kind instead of message-string regexes. Same coverage, less noise.
- engine/services/frameCapture.ts: replace the `as unknown as { ... }`
  double-cast with a single named `WindowWithVariables` alias inside the
  page closure.

All affected suites green (core getVariables 9, cli render 12, cli
dockerRunArgs 13).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

@@ -1,4 +1,4 @@
-import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
 
 const producerState = vi.hoisted(() => ({
   createdJobs: [] as Array<Record<string, unknown>>,
@@ -133,21 +133,31 @@ describe("renderLocal browser GPU config", () => {
 });
 
 describe("parseVariablesArg", () => {
-  it("returns undefined when neither flag is set", async () => {
-    const { parseVariablesArg } = await import("./render.js");
+  let parseVariablesArg: typeof import("./render.js").parseVariablesArg;
+
+  beforeAll(async () => {
+    ({ parseVariablesArg } = await import("./render.js"));
+  });
+
+  function expectErr<T extends { kind: string }>(
+    result: import("./render.js").VariablesParseResult,
+  ): T {
+    if (result.ok) throw new Error(`expected error, got ${JSON.stringify(result.value)}`);
+    return result.error as T;
+  }
+
+  it("returns undefined when neither flag is set", () => {
     expect(parseVariablesArg(undefined, undefined)).toEqual({ ok: true, value: undefined });
   });
 
-  it("parses inline JSON object", async () => {
-    const { parseVariablesArg } = await import("./render.js");
+  it("parses inline JSON object", () => {
     expect(parseVariablesArg('{"title":"Hello","n":3}', undefined)).toEqual({
       ok: true,
       value: { title: "Hello", n: 3 },
     });
   });
 
-  it("parses file JSON via injected reader", async () => {
-    const { parseVariablesArg } = await import("./render.js");
+  it("parses file JSON via injected reader", () => {
     const fakeReader = (path: string) => {
       if (path === "vars.json") return '{"theme":"dark"}';
       throw new Error("unexpected path");
@@ -158,43 +168,40 @@ describe("parseVariablesArg", () => {
     });
   });
 
-  it("rejects when both flags are set", async () => {
-    const { parseVariablesArg } = await import("./render.js");
-    const result = parseVariablesArg('{"a":1}', "vars.json");
-    expect(result.ok).toBe(false);
-    if (!result.ok) expect(result.title).toMatch(/Conflicting/);
+  it("rejects when both flags are set", () => {
+    const err = expectErr(parseVariablesArg('{"a":1}', "vars.json"));
+    expect(err).toEqual({ kind: "conflict" });
   });
 
-  it("rejects unparseable JSON with a source-aware title", async () => {
-    const { parseVariablesArg } = await import("./render.js");
-    const inlineFail = parseVariablesArg("{not json", undefined);
-    expect(inlineFail.ok).toBe(false);
-    if (!inlineFail.ok) expect(inlineFail.title).toBe("Invalid JSON in --variables");
-
-    const fileFail = parseVariablesArg(undefined, "x", () => "{not json");
-    expect(fileFail.ok).toBe(false);
-    if (!fileFail.ok) expect(fileFail.title).toBe("Invalid JSON in --variables-file");
-  });
-
-  it("rejects non-object payloads (array, string, null)", async () => {
-    const { parseVariablesArg } = await import("./render.js");
-    for (const payload of ["[1,2]", '"hello"', "null", "42"]) {
-      const result = parseVariablesArg(payload, undefined);
-      expect(result.ok).toBe(false);
-      if (!result.ok) expect(result.title).toBe("Invalid variables payload");
-    }
-  });
-
-  it("surfaces filesystem errors from --variables-file", async () => {
-    const { parseVariablesArg } = await import("./render.js");
-    const result = parseVariablesArg(undefined, "missing.json", () => {
-      throw new Error("ENOENT: no such file");
+  it("rejects unparseable JSON with a source-aware kind", () => {
+    expect(expectErr(parseVariablesArg("{not json", undefined))).toMatchObject({
+      kind: "parse-error",
+      source: "inline",
     });
-    expect(result.ok).toBe(false);
-    if (!result.ok) {
-      expect(result.title).toBe("Could not read --variables-file");
-      expect(result.message).toMatch(/missing\.json/);
-      expect(result.message).toMatch(/ENOENT/);
+    expect(expectErr(parseVariablesArg(undefined, "x", () => "{not json"))).toMatchObject({
+      kind: "parse-error",
+      source: "file",
+    });
+  });
+
+  it("rejects non-object payloads (array, string, null, number)", () => {
+    for (const payload of ["[1,2]", '"hello"', "null", "42"]) {
+      expect(expectErr(parseVariablesArg(payload, undefined))).toEqual({ kind: "shape-error" });
     }
   });
+
+  it("surfaces filesystem errors from --variables-file", () => {
+    const err = expectErr<{
+      kind: "read-error";
+      path: string;
+      cause: string;
+    }>(
+      parseVariablesArg(undefined, "missing.json", () => {
+        throw new Error("ENOENT: no such file");
+      }),
+    );
+    expect(err.kind).toBe("read-error");
+    expect(err.path).toBe("missing.json");
+    expect(err.cause).toMatch(/ENOENT/);
+  });
 });

packages/cli/src/commands/render.ts

@@ -398,14 +398,21 @@ interface RenderOptions {
   variables?: Record<string, unknown>;
 }
 
+export type VariablesParseError =
+  | { kind: "conflict" }
+  | { kind: "read-error"; path: string; cause: string }
+  | { kind: "parse-error"; source: "inline" | "file"; cause: string }
+  | { kind: "shape-error" };
+
 export type VariablesParseResult =
   | { ok: true; value: Record<string, unknown> | undefined }
-  | { ok: false; title: string; message: string };
+  | { ok: false; error: VariablesParseError };
 
 /**
  * Pure parser for `--variables` / `--variables-file` flag pair. Splits out
  * from `resolveVariablesArg` so validation paths are unit-testable without
- * triggering `process.exit`.
+ * triggering `process.exit`. Reports failures via a structured `kind`
+ * discriminant so the side-effecting wrapper owns all UI strings.
  */
 export function parseVariablesArg(
   inline: string | undefined,
@@ -413,11 +420,7 @@ export function parseVariablesArg(
   readFile: (path: string) => string = (p) => readFileSync(resolve(p), "utf8"),
 ): VariablesParseResult {
   if (inline != null && filePath != null) {
-    return {
-      ok: false,
-      title: "Conflicting variables flags",
-      message: "Use either --variables or --variables-file, not both.",
-    };
+    return { ok: false, error: { kind: "conflict" } };
   }
   let raw: string | undefined;
   let source: "inline" | "file" | undefined;
@@ -429,11 +432,13 @@ export function parseVariablesArg(
       raw = readFile(filePath);
       source = "file";
     } catch (error: unknown) {
-      const message = error instanceof Error ? error.message : String(error);
       return {
         ok: false,
-        title: "Could not read --variables-file",
-        message: `${filePath}: ${message}`,
+        error: {
+          kind: "read-error",
+          path: filePath,
+          cause: error instanceof Error ? error.message : String(error),
+        },
       };
     }
   }
@@ -443,23 +448,49 @@ export function parseVariablesArg(
   try {
     parsed = JSON.parse(raw);
   } catch (error: unknown) {
-    const message = error instanceof Error ? error.message : String(error);
     return {
       ok: false,
-      title: source === "file" ? "Invalid JSON in --variables-file" : "Invalid JSON in --variables",
-      message,
+      error: {
+        kind: "parse-error",
+        source: source ?? "inline",
+        cause: error instanceof Error ? error.message : String(error),
+      },
     };
   }
   if (parsed == null || typeof parsed !== "object" || Array.isArray(parsed)) {
-    return {
-      ok: false,
-      title: "Invalid variables payload",
-      message: 'Variables must be a JSON object (e.g. {"title":"Hello"}).',
-    };
+    return { ok: false, error: { kind: "shape-error" } };
   }
   return { ok: true, value: parsed as Record<string, unknown> };
 }
 
+function variablesErrorMessage(error: VariablesParseError): { title: string; message: string } {
+  switch (error.kind) {
+    case "conflict":
+      return {
+        title: "Conflicting variables flags",
+        message: "Use either --variables or --variables-file, not both.",
+      };
+    case "read-error":
+      return {
+        title: "Could not read --variables-file",
+        message: `${error.path}: ${error.cause}`,
+      };
+    case "parse-error":
+      return {
+        title:
+          error.source === "file"
+            ? "Invalid JSON in --variables-file"
+            : "Invalid JSON in --variables",
+        message: error.cause,
+      };
+    case "shape-error":
+      return {
+        title: "Invalid variables payload",
+        message: 'Variables must be a JSON object (e.g. {"title":"Hello"}).',
+      };
+  }
+}
+
 /**
  * Resolve `--variables` / `--variables-file` into a plain object, or
  * `undefined` when neither flag is set. Exits the process with a friendly
@@ -471,7 +502,8 @@ export function resolveVariablesArg(
 ): Record<string, unknown> | undefined {
   const result = parseVariablesArg(inline, filePath);
   if (!result.ok) {
-    errorBox(result.title, result.message);
+    const { title, message } = variablesErrorMessage(result.error);
+    errorBox(title, message);
     process.exit(1);
   }
   return result.value;

packages/core/src/runtime/getVariables.ts

@@ -38,14 +38,10 @@ function readDeclaredDefaults(root: Element | null): Record<string, unknown> {
 
   const out: Record<string, unknown> = {};
   for (const entry of parsed) {
-    if (
-      entry &&
-      typeof entry === "object" &&
-      typeof (entry as { id?: unknown }).id === "string" &&
-      "default" in entry
-    ) {
-      out[(entry as { id: string }).id] = (entry as { default: unknown }).default;
-    }
+    if (!entry || typeof entry !== "object") continue;
+    const e = entry as Record<string, unknown>;
+    if (typeof e.id !== "string" || !("default" in e)) continue;
+    out[e.id] = e.default;
   }
   return out;
 }

packages/engine/src/services/frameCapture.ts

@@ -162,9 +162,9 @@ export async function createCaptureSession(
   if (options.variables && Object.keys(options.variables).length > 0) {
     const variablesJson = JSON.stringify(options.variables);
     await page.evaluateOnNewDocument((json: string) => {
+      type WindowWithVariables = Window & { __hfVariables?: Record<string, unknown> };
       try {
-        (window as unknown as { __hfVariables?: Record<string, unknown> }).__hfVariables =
-          JSON.parse(json);
+        (window as WindowWithVariables).__hfVariables = JSON.parse(json);
       } catch {
         // The CLI validated the JSON before this point — a parse failure here
         // means the page swapped JSON.parse, which is the page's problem.