fix(core,cli): attribute figma REST failures to the endpoint that failed

[vanceingalls]

Summary

cli_error events for figma failures had no way to attribute a failure to
the specific REST call that produced it — the dashboard could see
RATE_LIMITED/FORBIDDEN/etc spike, but not which endpoint (images,
files_nodes, variables_local, styles, files_images, file_meta)
was actually hitting the limit.

Retry-with-backoff for 429s is already shipped (#1873 area / 1bb768834,
4fc699fee, 87e2a70f9) — this PR does not touch that. It adds an
endpoint label alongside it:

• FigmaClientError gains an optional endpoint field — a low-cardinality
  call label, never the raw fileKey/nodeId (so no PII risk).
• Every throw site in client.ts (401/403/429/HTTP_ERROR via the shared
  get(), plus the direct RENDER_FAILED/NODE_NOT_FOUND throws) now
  carries its endpoint label.
• withFigmaErrors (cliError.ts) forwards err.endpoint into
  trackCliError, which now emits it as endpoint on the cli_error
  PostHog event.

Net effect: once this ships, the "Figma errors by typed code" dashboard
tile can be broken down by endpoint to see exactly which figma REST call
is the rate-limit hotspot, instead of just an aggregate error count.

Test plan

• packages/core/src/figma/client.test.ts — new endpoint attribution
  describe block asserts each client method's error carries the right
  low-cardinality label and never leaks the fileKey/nodeId (26 tests
  total, all passing)
• packages/cli/src/telemetry/events.test.ts — new case asserting
  trackCliError forwards endpoint onto the cli_error event (35
  tests total, all passing)
• bunx tsc --noEmit clean on both packages/core and packages/cli
• bunx oxfmt / bunx oxlint clean on all touched files

🤖 Generated with Claude Code

[vanceingalls]

• fix(core,cli): attribute figma REST failures to the endpoint that failed #2358 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

Review — fix(core,cli): attribute figma REST failures to the endpoint that failed

SSOT check: clean. The endpoint label flows through one path: GetOptions.endpoint (required on every get() call) → FigmaClientError.endpoint → withFigmaErrors reads it → trackCliError forwards it to the PostHog event. Single chain, no duplication.

get() signature tightening: the default opts = {} was removed, making opts required. Since GetOptions.endpoint is non-optional (endpoint: string), every get() call site MUST provide the label — no silent omission possible. All 6 call sites updated: images, files_images, variables_local, styles, files_nodes, file_meta. Good enforcement.

Direct throws outside get(): RENDER_FAILED and NODE_NOT_FOUND are thrown directly (not through throwOnStatus), so they manually pass "images" and "files_nodes" respectively. Test explicitly covers this: "still labels RENDER_FAILED and NODE_NOT_FOUND (thrown outside the shared get())".

Low-cardinality labels: 6 fixed strings, never the raw fileKey/nodeId. Test asserts err.endpoint.not.toContain("SECRET"). No PII risk.

Constructor change: FigmaClientError(code, message, status?, endpoint?) — endpoint is the 4th positional arg. All existing throw sites updated. The forbiddenError helper (4 return paths) correctly forwards opts.endpoint to every variant.

Telemetry forwarding: cliError.ts reads err.endpoint only for FigmaClientError instances (err instanceof FigmaClientError ? err.endpoint : undefined). events.ts adds endpoint as an optional property on the PostHog event. Clean.

LGTM — no issues found.

— Miga

[james-russo-rames-d-jusso]

Clean telemetry attribution — making endpoint required on GetOptions gets TypeScript to enforce labeling at every REST call site inside client.ts, and the SECRET-in-fileKey assertion in the client tests is a nice PII belt-and-suspenders. One attribution gap outside the diff plus a couple of nits.

Concerns

• packages/cli/src/commands/figma/asset.ts:209 — the batch asset-import RENDER_FAILED misses the endpoint label. This throw fires when client.renderNodes succeeds (200) but figma returns no URL for a specific node in the batch response — the exact same semantic condition as renderNode's single-node throw in client.ts:315, which this PR does label "images". Because asset.ts:209 throws from the CLI package rather than from client.ts, it slipped past the "every throw site in client.ts" audit. Net effect: the "Figma errors by typed code" tile's endpoint breakdown will systematically miss per-node RENDER_FAILEDs from the asset-import flow (cli_error.endpoint === undefined on that path), for the flow that most heavily exercises /v1/images. Fix is one line — pass undefined, "images" as the two trailing args to match client.ts:315. Worth catching in this PR since the whole point is endpoint parity.

Nits

• renderNode (single-node) and renderNodes (batch) both label "images". Same REST endpoint / same rate-limit bucket, so aggregation is probably intended — but the dashboard won't distinguish the two SDK entry points if you ever wanted to know which one is hotter.
• Label style is slightly inconsistent — variables_local is two-segment while styles / file_meta are one-segment. Project-internal labels so trivially migratable, non-blocking.

Questions

• Is the "Figma errors by typed code" PostHog tile already set up to consume endpoint, or is that queued as a follow-up? Older cli_error events won't have the field, so whatever tile groups on it will need to handle the undefined bucket gracefully.

What I didn't verify

• HogQL / existing PostHog dashboard queries — I don't know if any current filter would misbehave when endpoint is undefined vs. absent.

— Review by Rames D Jusso

[miguel-heygen]

Additive review on current head 4a2a0f685ca16599a479baf5818ec321ffee7462. Miga covered the six shared get() call sites and the two direct throws inside client.ts; I independently confirmed the required GetOptions.endpoint contract at packages/core/src/figma/client.ts:219-225 and the low-cardinality labels at all six calls.

blocker — one production direct throw still silently drops attribution: packages/cli/src/commands/figma/asset.ts:209-212 synthesizes FigmaClientError("RENDER_FAILED", ...) when a batch renderNodes() result is absent/null, but does not pass "images". That error reaches withFigmaErrors, whose new telemetry forwarding consequently emits endpoint: undefined. This is the same direct-throw class the PR says is fully covered, but the new tests only exercise the two throws in client.ts, so the batch asset path remains invisible in the endpoint breakdown. Please label this throw "images" and add a regression assertion for the batch-miss path.

CI was green across completed checks at review time; four required jobs were still in progress. This code-path blocker is independent of those pending results.

Verdict: REQUEST CHANGES
Reasoning: Shared REST failures are attributed correctly, but one real batch asset RENDER_FAILED path still omits the endpoint and defeats the PR's completeness guarantee.
— Deepwork

[vanceingalls]

Fixed — good catch. `asset.ts:209`'s batch-miss `RENDER_FAILED` now labels `"images"` (matching `client.ts`'s single-node throw), plus a regression test asserting the batch-miss path carries the endpoint. Re-pushed.

Re the nits: keeping `renderNode`/`renderNodes` both labeled `"images"` is intentional — same REST endpoint, same rate-limit bucket, so aggregation is what the dashboard cares about. Label-format consistency (`variables_local` vs `styles`) is cosmetic and left as-is since it's internal-only and trivially migratable.

On the PostHog tile question: no follow-up queued yet — the dashboard's "Figma errors by typed code" tile doesn't group by `endpoint` today. Older/pre-this-PR `cli_error` events simply won't have the field (undefined, not a broken join), so it's safe to add a breakdown once there's a release with data to look at.

[miguel-heygen]

Re-review at c774ece6: the prior attribution blocker is fixed correctly. packages/cli/src/commands/figma/asset.ts:209-214 now labels the batch-miss RENDER_FAILED as images, matching the underlying /v1/images call and the single-node path. The new regression at asset.test.ts:182-197 positively pins both the typed code and endpoint.

I re-enumerated the production surface: all six shared get() call sites carry required, fixed low-cardinality labels; all direct production FigmaClientError throws are covered (client.ts plus this asset.ts path); and withFigmaErrors forwards only the stored label. No remaining omission or high-cardinality value found.

I cannot stamp this revision yet because required CI is red: Test failed on the unrelated, already-diagnosed withInstallLock reports progress while waiting instead of staying silent main-branch flake (the fix is #2361), while this PR's touched suites passed in that same job (client.test.ts 26/26, asset.test.ts 12/12, telemetry events.test.ts 35/35). Please rebase/cherry-pick the test stabilization or otherwise get the required lane green, then this is ready to approve.

Verdict: COMMENT
Reasoning: The endpoint-attribution blocker is fully resolved, but a required CI check is currently failing, so I cannot approve this head yet.

— Deepwork