Commit Graph
7 Commits
Author SHA1 Message Date
Miguel Ángel fdc5932897 fix(cli): honor check navigation timeout (#2860)
* fix(cli): honor check navigation timeout

* test(cli): clarify diagnostic timeout precedence
2026-07-29 20:50:20 +02:00
James 3aa2404747 refactor(cli): centralize process lifecycle 2026-07-20 09:04:03 -07:00
Miguel Ángel 6933e8acda fix(cli): consolidate snapshot and frame diagnostics (#2402)
* fix(snapshot): preserve exact requested times

* fix(cli): fail video snapshots without FFmpeg

* fix(cli): honor navigation timeout in diagnostics

* fix(cli): preserve snapshot alpha and create shot dirs
2026-07-14 01:46:44 -04:00
Miguel Ángel 796d5df156 fix(render): allow explicit entry without index (#2307)
* fix(render): allow explicit entry without index

* fix(render): unify explicit composition handling

* test(cli): allow explicit render integration under CI load
2026-07-12 23:01:27 -04:00
Miguel Ángel de27b46680 fix(cli): default render fps to the composition's data-fps
* fix(cli): default render fps to the composition's data-fps

hyperframes render hard-coded fps to 30 when --fps was omitted, ignoring a
data-fps declared on the composition root — so a composition authored at
data-fps="24" silently rendered at 30fps unless the user knew to pass --fps 24.
The runtime already honors data-fps; the CLI now matches it.

Precedence: explicit --fps > composition root data-fps > 30. New pure
readCompositionFps() extracts the root data-fps via linkedom (mirrors the
runtime's root resolution: [data-composition-id][data-root=true], else the
outermost [data-composition-id]); render validates it through parseFps and
falls back to 30 on an absent/invalid value. Unit-tested.

* fix(cli): honor composition data-fps on cloud renders and --composition targets

The local render command read data-fps from project.dir/index.html even when
--composition rendered a different file, and the lambda/cloudrun render paths
ignored data-fps entirely (hardcoded ?? 30). Both are the same silently-wrong-
fps bug on other render entry points:
- render.ts resolves the entry file first, then reads data-fps from the file
  actually being rendered (falling back to index.html).
- lambda render/render-batch and cloudrun render/render-batch default fps from
  the composition's data-fps, accepted only when it is one of the cloud-allowed
  values {24,30,60}, else the existing 30 default. Explicit --fps still wins.

* fix(cli): drop citty fps default so data-fps resolution actually runs

The fps arg had default: "30", so citty set args.fps="30" on omission and
resolveDefaultFpsArg short-circuited (explicitFps never null) — reverting the
command to always-30 and making the whole data-fps feature a no-op (caught in
review). Remove the arg default; the "30" fallback already lives at
parseFps(fpsArg ?? "30"). Adds a regression guard asserting the arg has no
default.

* test(cli): read citty args through a plain record in the fps-default guard

The regression guard accessed cmd.args.fps directly, but citty types args as
Resolvable<ArgsDef> so .fps failed typecheck in CI. Read it through a plain
record cast.
2026-07-07 17:10:10 -04:00
Matt Van HornandMatt Van Horn e6b8d66c2d feat(cli,producer): add gif output format with two-pass palette encode (#1333)
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
2026-06-10 21:17:55 -04:00
James RussoandClaude Opus 4.7 6affe2d212 fix(cli): reject directory --composition and add --browser-timeout (#1199) (#1200)
* fix(cli): reject directory --composition and add --browser-timeout (#1199)

Two unrelated symptoms from issue #1199, fixed together:

1. `--composition .` (or any directory path) used to slip past the
   existsSync check in render.ts and explode downstream as
   `EISDIR: illegal operation on a directory, read` when the producer
   readFileSync'd the entry. The CLI now treats `.` / `""` as "omit
   the flag" (falls back to index.html) and rejects other directory
   paths with an actionable error pointing at the .html shape.

2. The 60s Puppeteer page.goto timeout in frameCapture.ts was hard-
   coded, so heavy compositions (many videos / fonts / asset requests)
   could not complete `domcontentloaded` in time. Add a configurable
   `pageNavigationTimeout` to EngineConfig (default 60_000, env
   fallback PRODUCER_PAGE_NAVIGATION_TIMEOUT_MS) and expose it as
   `--browser-timeout <seconds>` on `hyperframes render`. The flag
   threads through both renderLocal (via resolveConfig) and the
   docker bridge (via buildDockerRunArgs).

Tests:
- render.test.ts: forwards/omits pageNavigationTimeout into resolveConfig
- dockerRunArgs.test.ts: forwards/omits --browser-timeout (seconds)

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

* fix(cli): address PR #1200 review — extract validators, tighten bounds

Addresses Vai's blockers and Miguel's nits on PR #1200:

- Vai blocker 1 (fallow CRAP) + blocker 3 (no argv tests):
  Extract --browser-timeout and --composition validators into pure
  helpers in utils/renderArgs.ts with a structured-result discriminant.
  Drops ~45 lines of inline validation from run(), reducing its CRAP
  score 1290→978 and cyclomatic 75→65. 19 new unit tests cover the
  parse branches (sub-ms, overflow, NaN, Infinity, empty, negative,
  ".", "./", whitespace, directory, missing, ../escape, sibling-prefix).

- Vai blocker 2 (sub-ms → timeout:0 = "no timeout"): reject inputs
  that round to <1 ms. Puppeteer treats page.goto({timeout:0}) as
  wait-forever, so --browser-timeout 0.0004 silently flipped the
  semantics. Now rejected with an explicit "rounds to 0 ms" error.

- Vai important 5 (1e10 accepted → setTimeout overflow): cap at
  86_400s (24h). Above Node's TIMEOUT_MAX ≈ 2^31-1 ms setTimeout
  fires immediately, the opposite of "long timeout."

- Vai important 4 (related timeouts unmentioned): CLI help and docs
  now flag PRODUCER_PUPPETEER_PROTOCOL_TIMEOUT_MS and the 45s
  playerReadyTimeout as the other knobs heavy compositions may need.

- Vai nit 7 (s/ms unit mismatch): help text and docs row both call
  out the SECONDS-vs-MILLISECONDS difference between flag and env.

- Vai nit 8 / Miguel nit (composition flag discoverability): the
  --composition description now says "Pass `.` (or omit the flag)
  to render the project's index.html."

- Miguel nit (dead branch): the entryFile === "" unreachable branch
  is gone. New helper uses `if (!trimmed || trimmed === ".")`.

Also adds a trailing-separator guard on the project-containment check
(sibling-prefix bypass: /proj-evil/x.html no longer slips past
startsWith('/proj')) — flagged by the code review.

The three remaining fallow complexity findings on render.ts (run,
renderDocker, trackRenderMetrics) are inherited from main; this PR
reduces run() but does not refactor it. Suppressed with
fallow-ignore-next-line markers and inline rationale.

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

* fix(cli): diverge --browser-timeout error messages per Vai nit 5

The `not-a-number` and `not-positive` branches in browserTimeoutErrorMessage
shared the generic "Must be a positive number of seconds" message even
though the discriminant carried distinct kinds. Diverge them so users see
the specific failure mode:

  --browser-timeout abc   →  "Got \"abc\", which is not a number."
  --browser-timeout -5    →  "Got \"-5\" seconds, which is not positive."

The shared hint ("pass a positive number of seconds, e.g. 180") is
preserved on both branches.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-04 16:47:10 -04:00