fix(core,cli): parse figma 403 body, batch asset fetch, fix NO_TOKEN box

Extends the scope+retry work from the figma bug-bash (valid report: 9-bugs-with-repros; the skill-not-used report was discarded). - 403-body parse (bug 4): figma returns 403 {"err":"Invalid token"} for bad PATs (NOT 401), and 403 {"err":"Invalid scope(s)… requires X"} for missing scopes. get() now reads the body: "Invalid token" reclassifies to BAD_TOKEN with re-mint advice; a scope body surfaces figma's own diagnosis verbatim; else falls back to the endpoint's scope hint. Reads both err and message (variables endpoint uses message). One fix, honest messages for bugs 1/4/9. - Batch asset fetch (requested): figma asset accepts multiple refs (space-separated or comma-joined) of one file and renders them in a SINGLE /v1/images call via new client.renderNodes — figma's documented per-minute rate-limit workaround. runAssetImport delegates to runAssetImportMany; cache-checks per node, batches only the misses, one index.md regen. - NO_TOKEN box (bug 8): errorBox indented only the first hint line, mangling the numbered setup list. Indent every line; single-line hints unchanged. Verified live: 3 refs -> 3 imports -> 1 request; bad token -> BAD_TOKEN not scope advice. Client suite 22, cli figma 33. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-03 12:54:29 +00:00 · 2026-07-09 15:14:57 -07:00
parent 1bb7688347
commit 4fc699fee6
8 changed files with 400 additions and 84 deletions
@@ -3,7 +3,7 @@ import { describe, expect, it, afterEach } from "vitest";
 import { mkdtempSync, readFileSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { runAssetImport, type AssetImportDeps } from "./asset.js";
+import { runAssetImport, runAssetImportMany, type AssetImportDeps } from "./asset.js";
 import type { FigmaClient } from "@hyperframes/core/figma";

 const dirs: string[] = [];
@@ -17,8 +17,9 @@ afterEach(() => {
 });

 function fakeClient(overrides: Partial<FigmaClient> = {}): FigmaClient {
-  return {
+  const client: FigmaClient = {
    renderNode: () => Promise.resolve({ url: "https://cdn.example/a", ext: "png" }),
+    renderNodes: () => Promise.resolve([]),
    imageFills: () => Promise.resolve(new Map()),
    variables: () => Promise.resolve({ variables: {}, variableCollections: {} }),
    styles: () => Promise.resolve([]),
@@ -26,6 +27,19 @@ function fakeClient(overrides: Partial<FigmaClient> = {}): FigmaClient {
    fileVersion: () => Promise.resolve({ version: "7", lastModified: "2026-07-01" }),
    ...overrides,
  };
+  // Default renderNodes delegates to renderNode (honoring any override) so
+  // existing single-node tests keep controlling behavior via renderNode.
+  if (!overrides.renderNodes) {
+    client.renderNodes = (fileKey, nodeIds, opts) =>
+      Promise.all(
+        nodeIds.map((nodeId) =>
+          client
+            .renderNode({ fileKey, nodeId }, opts)
+            .then((r) => ({ nodeId, url: r.url, ext: r.ext })),
+        ),
+      );
+  }
+  return client;
 }

 const PNG_BYTES = new Uint8Array([0x89, 0x50, 0x4e, 0x47]);
@@ -134,6 +148,39 @@ describe("runAssetImport", () => {
    expect(index).not.toContain("image_002");
  });

+  it("batches many nodes into ONE renderNodes call and freezes each", async () => {
+    const dir = scratch();
+    let renderNodesCalls = 0;
+    let batchSize = 0;
+    const batchClient = fakeClient({
+      renderNodes: (fileKey, nodeIds, opts) => {
+        renderNodesCalls += 1;
+        batchSize = nodeIds.length;
+        return Promise.resolve(
+          nodeIds.map((nodeId) => ({ nodeId, url: `https://cdn/${nodeId}`, ext: opts.format })),
+        );
+      },
+    });
+    const results = await runAssetImportMany(
+      ["KEY:1-2", "KEY:3-4", "KEY:5-6"],
+      { format: "png" },
+      deps(dir, { client: batchClient }),
+    );
+    expect(results).toHaveLength(3);
+    expect(results.every((r) => !r.reused)).toBe(true);
+    expect(renderNodesCalls).toBe(1); // one REST call for all three
+    expect(batchSize).toBe(3);
+    // distinct frozen files, all recorded
+    expect(new Set(results.map((r) => r.record.id)).size).toBe(3);
+  });
+
+  it("splits comma-joined refs and rejects a cross-file batch", async () => {
+    const dir = scratch();
+    await expect(
+      runAssetImportMany(["KEY:1-2", "OTHER:3-4"], { format: "png" }, deps(dir)),
+    ).rejects.toThrow(/share a fileKey/);
+  });
+
  it("reuses against ANY matching tuple, not just the oldest row", async () => {
    const dir = scratch();
    await runAssetImport("KEY:1-2", { format: "svg" }, deps(dir)); // image_001 (svg)
@@ -9,6 +9,7 @@ import {
  appendRecord,
  buildAssetSnippet,
  createFigmaClient,
+  FigmaClientError,
  findAllByFigmaNode,
  freezeBytes,
  nextId,
@@ -49,56 +50,68 @@ export interface AssetImportResult {
  reused: boolean;
 }

-export async function runAssetImport(
-  refInput: string,
-  opts: AssetImportOptions,
-  deps: AssetImportDeps,
-): Promise<AssetImportResult> {
+function requireNodeRef(refInput: string): { fileKey: string; nodeId: string } {
  const ref = parseFigmaRef(refInput);
  if (!ref.nodeId)
    throw new Error(
      `ref "${refInput}" has no node id — share a link with ?node-id=… or use fileKey:nodeId`,
    );
+  return { fileKey: ref.fileKey, nodeId: ref.nodeId };
+}

-  const { version } = await deps.client.fileVersion(ref.fileKey);
-  const description = normalizeMeta(opts.description);
-  const entity = normalizeMeta(opts.entity);
-
-  // Cache key per spec §5: fileKey:nodeId:format:scale:version → reuse.
-  // Check EVERY row for the node (a node can legitimately have several
-  // format/scale/version tuples — the oldest-row shortcut minted duplicates
-  // forever once a second tuple existed). Unspecified scale is canonically 1
-  // on both sides (figma's default). Reuse also requires the frozen file to
-  // still exist — a deleted file falls through to re-import.
-  const existing = findAllByFigmaNode(deps.projectDir, ref.fileKey, ref.nodeId).find(
+/** Cache hit per spec §5 (fileKey:nodeId:format:scale:version). Check EVERY
+ * row for the node — a node can carry several format/scale/version tuples,
+ * and the oldest-row shortcut minted duplicates forever. Reuse requires the
+ * frozen file to still exist; a deleted file falls through to re-import.
+ * Metadata supplied on a re-import upserts rather than being discarded. */
+function reuseExisting(
+  fileKey: string,
+  nodeId: string,
+  opts: AssetImportOptions,
+  version: string,
+  deps: AssetImportDeps,
+  description: string | undefined,
+  entity: string | undefined,
+): AssetImportResult | null {
+  const existing = findAllByFigmaNode(deps.projectDir, fileKey, nodeId).find(
    (r) =>
      r.provenance.format === opts.format &&
      (r.provenance.scale ?? 1) === (opts.scale ?? 1) &&
      r.provenance.version === version &&
      existsSync(join(deps.projectDir, r.path)),
  );
-  if (existing) {
-    // Metadata supplied on a re-import still lands: upsert the row instead
-    // of silently discarding the flags.
-    let record = existing;
-    if (
-      (description !== undefined && description !== existing.description) ||
-      (entity !== undefined && entity !== existing.entity)
-    ) {
-      record = {
-        ...existing,
-        ...(description !== undefined && { description }),
-        ...(entity !== undefined && { entity }),
-      };
-      updateRecord(deps.projectDir, record);
-    }
-    safeRegenerateIndex(deps.projectDir);
-    return { record, snippet: buildAssetSnippet(record), reused: true };
+  if (!existing) return null;
+  let record = existing;
+  if (
+    (description !== undefined && description !== existing.description) ||
+    (entity !== undefined && entity !== existing.entity)
+  ) {
+    record = {
+      ...existing,
+      ...(description !== undefined && { description }),
+      ...(entity !== undefined && { entity }),
+    };
+    updateRecord(deps.projectDir, record);
  }
+  return { record, snippet: buildAssetSnippet(record), reused: true };
+}

-  const rendered = await deps.client.renderNode(ref, opts);
-  let bytes = await deps.download(rendered.url);
-  if (rendered.ext === "svg") {
+/** Freeze a rendered node's bytes and record it. Does NOT regenerate index.md
+ * — the caller does that once (batch imports would otherwise rewrite it N
+ * times). */
+async function freezeAndRecord(
+  fileKey: string,
+  nodeId: string,
+  url: string,
+  ext: FigmaAssetFormat,
+  opts: AssetImportOptions,
+  version: string,
+  deps: AssetImportDeps,
+  description: string | undefined,
+  entity: string | undefined,
+): Promise<AssetImportResult> {
+  let bytes = await deps.download(url);
+  if (ext === "svg") {
    // Sniff before decoding: an SVG starts with '<' or an XML decl/BOM. A
    // non-text payload would decode to U+FFFD soup and still write to disk.
    const b0 = bytes[0];
@@ -106,32 +119,102 @@ export async function runAssetImport(
      throw new Error("figma render returned non-SVG bytes for an svg export — retry the import");
    bytes = new TextEncoder().encode(sanitizeSvg(new TextDecoder().decode(bytes)));
  }
-
  const id = nextId(deps.projectDir, "image");
-  const destAbs = join(typeDirPath(deps.projectDir, "image"), `${id}.${rendered.ext}`);
+  const destAbs = join(typeDirPath(deps.projectDir, "image"), `${id}.${ext}`);
  freezeBytes(bytes, destAbs);
-
  const record: FigmaManifestRecord = {
    id,
    type: "image",
    path: relative(deps.projectDir, destAbs),
-    source: `figma:${ref.fileKey}/${ref.nodeId}`,
+    source: `figma:${fileKey}/${nodeId}`,
    ...(description !== undefined && { description }),
    ...(entity !== undefined && { entity }),
    provenance: {
      source: "figma",
-      fileKey: ref.fileKey,
-      nodeId: ref.nodeId,
+      fileKey,
+      nodeId,
      version,
      format: opts.format,
      scale: opts.scale,
    },
  };
  appendRecord(deps.projectDir, record);
-  safeRegenerateIndex(deps.projectDir);
  return { record, snippet: buildAssetSnippet(record), reused: false };
 }

+export async function runAssetImport(
+  refInput: string,
+  opts: AssetImportOptions,
+  deps: AssetImportDeps,
+): Promise<AssetImportResult> {
+  const [result] = await runAssetImportMany([refInput], opts, deps);
+  if (!result) throw new Error(`figma asset import produced no result for "${refInput}"`);
+  return result;
+}
+
+/**
+ * Import many nodes of ONE figma file. Cache-checks each, renders the misses
+ * in a SINGLE /v1/images batch call (figma's documented rate-limit
+ * workaround — N nodes, one REST request), freezes each, and regenerates
+ * index.md once. Results come back in input order.
+ */
+export async function runAssetImportMany(
+  refInputs: string[],
+  opts: AssetImportOptions,
+  deps: AssetImportDeps,
+): Promise<AssetImportResult[]> {
+  if (refInputs.length === 0) return [];
+  const refs = refInputs.map(requireNodeRef);
+  const fileKey = refs[0]!.fileKey;
+  const mixed = refs.find((r) => r.fileKey !== fileKey);
+  if (mixed)
+    throw new Error(
+      `all refs in one import must share a fileKey (batch is per-file) — got ${fileKey} and ${mixed.fileKey}; run separate commands per file`,
+    );
+
+  const { version } = await deps.client.fileVersion(fileKey);
+  const description = normalizeMeta(opts.description);
+  const entity = normalizeMeta(opts.entity);
+
+  // Resolve cache hits first; batch-render only the misses.
+  const slots: (AssetImportResult | null)[] = refs.map((r) =>
+    reuseExisting(fileKey, r.nodeId, opts, version, deps, description, entity),
+  );
+  const missIndexes = slots.flatMap((s, i) => (s === null ? [i] : []));
+  if (missIndexes.length > 0) {
+    const missNodeIds = missIndexes.map((i) => refs[i]!.nodeId);
+    const rendered = await deps.client.renderNodes(fileKey, missNodeIds, opts);
+    const byNode = new Map(rendered.map((r) => [r.nodeId, r] as const));
+    for (const i of missIndexes) {
+      const nodeId = refs[i]!.nodeId;
+      const r = byNode.get(nodeId);
+      // Keep the typed code: component import's rasterize fallback skips on
+      // RENDER_FAILED, so a plain Error here would abort the whole import.
+      if (!r || r.url === null)
+        throw new FigmaClientError(
+          "RENDER_FAILED",
+          `figma could not render node ${nodeId} as ${opts.format}`,
+        );
+      slots[i] = await freezeAndRecord(
+        fileKey,
+        nodeId,
+        r.url,
+        r.ext,
+        opts,
+        version,
+        deps,
+        description,
+        entity,
+      );
+    }
+  }
+  safeRegenerateIndex(deps.projectDir);
+  return slots.map((s, i) => {
+    if (!s) throw new Error(`figma asset import produced no result for "${refInputs[i]}"`);
+    return s;
+  });
+}
+
 /** index.md is a single table row per record — newlines/tabs in a
 * description would corrupt the whole table. */
 function normalizeMeta(value: string | undefined): string | undefined {
@@ -159,11 +242,12 @@ function parseFormat(raw: string): FigmaAssetFormat {
 }

 export default defineCommand({
-  meta: { name: "asset", description: "Import a figma node as a frozen local asset" },
+  meta: { name: "asset", description: "Import one or more figma nodes as frozen local assets" },
  args: {
    ref: {
      type: "positional",
-      description: "figma URL, fileKey:nodeId, or fileKey",
+      description:
+        "figma URL, fileKey:nodeId, or fileKey (pass several, or comma-separate ids, to batch)",
      required: true,
    },
    format: { type: "string", description: "png | svg | jpg | pdf", default: "svg" },
@@ -183,8 +267,18 @@ export default defineCommand({
      const t0 = Date.now();
      const token = process.env.FIGMA_TOKEN ?? "";
      const client = createFigmaClient({ token });
-      const result = await runAssetImport(
-        args.ref,
+      // citty puts ALL positionals in `args._` (including the one bound to the
+      // named `ref`), so use `_` as the source of truth — reading both would
+      // double-count the first. Split any comma-joined ids, so `asset A B`,
+      // `asset A,B`, and `asset URL1 URL2` all batch into ONE /v1/images call.
+      const positionals =
+        Array.isArray(args._) && args._.length > 0 ? (args._ as string[]) : [args.ref];
+      const refs = positionals
+        .flatMap((r) => String(r).split(","))
+        .map((r) => r.trim())
+        .filter((r) => r.length > 0);
+      const results = await runAssetImportMany(
+        refs,
        {
          format: parseFormat(args.format),
          scale: args.scale !== undefined ? Number(args.scale) : undefined,
@@ -193,11 +287,18 @@ export default defineCommand({
        },
        { projectDir: args.dir, client, download: downloadRender },
      );
-      const verb = result.reused ? "reused" : "imported";
-      console.log(`${verb} ${result.record.id} → ${result.record.path}`);
-      console.log(result.snippet.html);
+      for (const result of results) {
+        const verb = result.reused ? "reused" : "imported";
+        console.log(`${verb} ${result.record.id} → ${result.record.path}`);
+        console.log(result.snippet.html);
+      }
+      if (results.length > 1) console.log(`(${results.length} nodes in 1 figma request)`);
      const { trackFigmaImport } = await import("../../telemetry/index.js");
-      trackFigmaImport({ phase: "asset", reused: result.reused, durationMs: Date.now() - t0 });
+      trackFigmaImport({
+        phase: "asset",
+        reused: results.every((r) => r.reused),
+        durationMs: Date.now() - t0,
+      });
    });
  },
 });
@@ -41,6 +41,20 @@ const SVG = new TextEncoder().encode("<svg/>");
 function client(): FigmaClient {
  return {
    renderNode: () => Promise.resolve({ url: "https://cdn/x", ext: "svg" }),
+    // Delegates to whatever renderNode is on the final object (via `this`), so
+    // inline clients that spread `...client()` and override renderNode still
+    // drive the batch path; rejections propagate (matching production).
+    renderNodes(fileKey, nodeIds, opts) {
+      return Promise.all(
+        nodeIds.map((nodeId) =>
+          this.renderNode({ fileKey, nodeId }, opts).then((r) => ({
+            nodeId,
+            url: r.url,
+            ext: r.ext,
+          })),
+        ),
+      );
+    },
    imageFills: () => Promise.resolve(new Map()),
    variables: () => Promise.resolve({ variables: {}, variableCollections: {} }),
    styles: () => Promise.resolve([]),
@@ -46,7 +46,16 @@ export function label(name: string, value: string): string {

 export function errorBox(title: string, hint?: string, suggestion?: string): void {
  console.error(`\n${c.error("\u2717")}  ${c.bold(title)}`);
-  if (hint) console.error(`\n   ${c.dim(hint)}`);
+  if (hint) {
+    // Indent EVERY hint line, not just the first \u2014 a multi-line hint (e.g. the
+    // NO_TOKEN numbered setup list) otherwise had line 1 indented and the rest
+    // flush-left, mangling the list. Single-line hints are unchanged.
+    const indented = hint
+      .split("\n")
+      .map((line) => `   ${line}`)
+      .join("\n");
+    console.error(`\n${c.dim(indented)}`);
+  }
  if (suggestion) console.error(`   ${c.accent(suggestion)}`);
  console.error();
 }