Commit Graph
4 Commits
Author SHA1 Message Date
Vance IngallsandClaude Fable 5 dfe63af3ae fix(core,cli): parent-relative figma child geometry; groups stop rejecting subcommand flags
Both found running the brand-loop guide end-to-end against the Simple
Design System:

- nodeToHtml subtracted the ROOT origin from every node's absolute bounds,
  but CSS absolute positioning resolves against the nearest positioned
  ancestor — every nesting level re-added its ancestors' offsets, drifting
  nested content down-right and pushing deep children off-frame (hero
  buttons invisible, pricing grid collapsed to one card). Children now
  subtract their PARENT's box; regression test with a two-level tree.

- trackCommandFailures asserted unknown flags against the command group's
  own (flagless) arg table even when the group was delegating to a
  subcommand, so `figma component <ref> --name x` imported and THEN threw
  "Unknown flag: --name". The assertion is now skipped when the first
  positional names a subcommand; leaf and non-delegating behavior is
  unchanged and covered by tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 00:47:03 -07:00
Miguel Ángel 38b27c4d82 fix(cli): report unknown-flag errors + cover nested subcommands (HF#2033) [P2] (#2072)
* fix(cli): report unknown-flag errors + cover nested subcommands (HF#2033)

Two flag-hygiene gaps behind the assertKnownFlags arc:

1. Telemetry loss: assertKnownFlags ran BEFORE the try/catch in the command
   wrapper, so an unknown-flag throw skipped reportCommandFailure entirely —
   zero signal on how often users hit bad flags. Moved the assertion inside
   the try so it reports like any other failure.

2. Nested-subcommand scope: cli.ts wraps only the top-level command loaders,
   so command groups' leaves (cloud/*, auth/*, figma/*, lambda/*, capture/*,
   skills) were never wrapped — citty dispatches to the leaf, whose run had no
   assertion and no failure reporting. So `hyperframes cloud render --badflag`
   silently ignored the flag. trackCommandFailures now recurses through
   cmd.subCommands (normalizing citty's Resolvable entries to loaders) and
   wraps every leaf. Identity is preserved for bare no-run/no-subcommand defs.

Verified: `auth status --badflag` now errors "Unknown flag: --badflag"
(previously silent); `auth --help` still dispatches; top-level `lint
--badflag` still rejected. Tests: unknown-flag rejection is reported, and a
nested subcommand's failure reaches onFailure.

* test(cli): guard indexed subCommands access for noUncheckedIndexedAccess

CI Typecheck (tsc, unlike the local tsup build) flagged the nested-subcommand
test: indexing `subCommands["render"]` yields `T | undefined` under
noUncheckedIndexedAccess, so invoking it tripped TS2722/TS18048. Guard the
loader before calling it.
2026-07-08 16:24:36 -04:00
Miguel Ángel 401dd1d27f fix: media-use bug-bash fixes (codex gate, id race, provider/reuse/adopt guards) + CLI unknown-flag rejection (#2033)
* fix(media-use): codex gate misfires as 'not logged in' when piped

codexUnavailableReason() gated generation on parsing `codex login status`
stdout, but that command prints 'Logged in using ChatGPT' to stderr and
exits 0 — so the piped stdout media-use captures (execFileSync returns
stdout only on success) was empty, and the gate falsely reported 'not
logged in'. Every headless / CI / agent run was blocked from codex image
gen even when fully authed.

Gate on the durable credentials file ($CODEX_HOME/auth.json) instead of
the TTY/stderr-only human text. Token validity is still proven by the
exec, which fails cleanly on a stale login. The stdout `features list`
capability check is unchanged.

Verified: reproduced the false 'not logged in' block, then after the fix
generated end-to-end via `resolve -t image --provider codex` (valid
1254x1254 PNG, source=generated, provider=codex.image_gen).

* fix(media-use): bug-bash fixes — id race, provider/reuse/adopt guards

From the bug-bash against main:

- MU-23 (HIGH): concurrent resolves raced on nextId (read-max-then-append,
  non-atomic), so parallel agents got duplicate ids and clobbered each
  other's files. Add allocateId(): a coarse per-project lock (.media/.lock,
  15s stale-steal) around id allocation that scans the manifest AND the
  type dir for reserved ids, then O_EXCL-creates a placeholder file so the
  slow download between allocate and append can't collide. 5 parallel
  resolves now yield 5 distinct ids + files.
- X4: --reuse imported across a type mismatch (bgm asset under images/).
  Apply typesMatch on the --reuse path; reject mismatches (icon<->image
  still interchangeable).
- X5: --provider silently overrode --local-only and made a network call.
  --local-only is now a hard guard: network providers are skipped even
  under a forced provider; the miss message explains the conflict.
- BUG-2: --provider ignored the exact-cache floor and could hand back an
  asset from a different provider. A forced --provider now bypasses all
  reuse rungs (regenerate with THIS provider); the unforced floor is intact.
- MU-26/X6: 0-byte assets accepted. --adopt skips 0-byte files (loud); ingest
  refuses a 0-byte local file (freezeUrl already rejects empty responses).
- BUG-4: unknown/unavailable --provider now errors with the available list
  instead of a generic 'no provider could resolve' (typo != catalog miss).
- BUG-5: --reuse "" gave the wrong 'type and intent required' error; it now
  routes to a clear empty-sha message.
- BUG-3: voice duration leaked an unrounded float into index.md; round all
  durations to 0.1s centrally at record build (matches probe).
- Nits: whitespace-only --intent is rejected; nudge grammar (exists/exist).

Tests: allocateId reservation + registry local-only-wins added; full
media-use suite green. All fixes verified e2e.

* fix(cli): reject unknown flags instead of silently ignoring them

citty is permissive: an unrecognized flag was dropped, not rejected — so
`render . --out x` (the flag is --output/-o) silently ignored --out and
rendered to the default renders/<name>.mp4 path. A mistyped flag read as a
render/catalog miss.

Add assertKnownFlags(): validate every dash-prefixed token against the
command's declared args + aliases + the global set (help/version/json)
before the command runs, in the shared trackCommandFailures run-wrapper so
every leaf command is covered. Handles --flag=value, --no-<bool> negation,
camelCase<->kebab arg names, and combined shorts; stops at --; positionals
and flag values pass through.

Verified: `render . --out x` -> 'Error: Unknown flag: --out'; --output/-o/
--json/--help still accepted. Unit tests added.

* docs(skills): install with --full-depth so agents get current main

The documented `npx skills add heygen-com/hyperframes` fetched the
skills.sh registry blob, which lags GitHub main by hours — so users
following the docs got a stale skill (e.g. media-use v1: no --candidates,
voice stubbed). The CLI's own `hyperframes skills` command already forces
a full clone via --full-depth to bypass this; the docs didn't pass it.

Add --full-depth to every documented install command (README, CLAUDE.md,
docs/guides/skills.mdx) with a one-line note on the lag. Addresses the
user-facing half of the publish/registry lag (#2034).

* chore(media-use): collapse resolve.mjs import to satisfy oxfmt --check

* fix(cli): extract longFlagName to keep flag validator under complexity gate

Also regenerate skills-manifest.json (resolve.mjs formatting change re-hashed
the media-use skill). Fixes the Fallow audit + skills-manifest-in-sync CI gates.
2026-07-07 19:19:28 -04:00
Miguel Ángel 5f6ced116d feat(cli): report command failure reasons to telemetry (de-blind browser/info) (#1484)
Observability showed `browser` (~75% fail, ~1.3k users/day) and `info` (~60%
fail) failing at high rates with no captured reason — only
`cli_command_result success=false`. citty's `runMain` catches a command's
thrown error and `process.exit(1)`s without re-throwing, so a thrown failure
never reached the existing `cli_error` telemetry (which only fired from the
uncaughtException / unhandledRejection handlers).

Wrap every command's `run()` at the dispatch boundary (cli.ts) so a thrown
failure reports its reason via `cli_error` (kind=command_error) before being
re-thrown unchanged — citty's print + exit-1 behavior is preserved. This
de-blinds every throw-style command at once: `browser ensure` (Chrome
download), `tts`, `inspect`, `render`, etc.

Paths that bypass the wrapper are handled inline:
- `browser` self-exits (`path` download failure, unknown subcommand) — report
  inline; the ARM64 `ensure` branch previously swallowed a failed install and
  returned success, now reports and exits 1.
- `resolveProject()` self-exits on InvalidProjectError (the dominant `info`
  failure — run outside a project) — report inline before exit.

Hardening:
- PII: `trackCliError` now redacts error_message + stack_trace via
  redactTelemetryString (matching render_* events) — CLI errors and stacks
  carry absolute install paths / cache dirs / user args.
- Race: the wrapper awaits an on-demand telemetry import before re-throwing, so
  a command that fails before the lazy telemetry import settles still reports
  (a telemetry failure is swallowed and never masks the real error).

Pure helpers in utils/command-failure-tracking.ts with unit tests for the
throw / success / no-run / onFailure-rejection cases, the reporter wiring, and
trackCliError redaction. CommandDef<any> mirrors citty's SubCommandsDef.

Known scope: commands that print + `process.exit(1)` on their own validation
paths (tts/validate/lint argument errors) remain wrapper-blind — follow-up.
2026-06-16 01:39:34 -04:00