mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-12 07:09:59 +00:00
feat(cli): validate cloud render aspect/composition/format before upload (#1156)
* feat(cli): validate cloud render aspect/composition/format before upload `hyperframes cloud render` accepted inputs the render pipeline can't satisfy and only failed server-side with a generic message. Add three client-side, pre-upload checks: - Missing `--composition` entry → clean "Composition not found" error instead of uploading a zip the render rejects opaquely. - Explicit `--aspect-ratio` that conflicts with the composition's authored data-width/data-height → "Aspect ratio mismatch" error. Aspect ratio is derived from the composition (auto-detected for local dirs), so the flag is rarely needed and can't reshape — only match. - `--resolution 4k` with `--format webm|mov` → rejected, since the alpha capture path can't supersample. Replaces maybeAutoDetectAspectRatio with resolveAspectRatioForSubmit, which folds detection + explicit-flag validation into one pass. Both new validators are exported and unit-tested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): reject explicit --aspect-ratio on unsupported-ratio compositions Addresses review on #1153. The mismatch guard only fired for `matched` compositions. For a composition whose dims resolve to an unsupported ratio (e.g. 4:5 → detection `no-match`), a conflicting explicit `--aspect-ratio` silently passed through and was forwarded to the server, which rejected it later — the opposite experience from a `matched` composition with the same wrong flag. Extend the guard to the `no-match` case: dims are known and the ratio can never equal a supported (16:9/9:16/1:1) explicit value, so it's a definite conflict. Kinds with unknown dims (no-dims/no-root-div/invalid-dims/read-error) still forward the explicit value since a conflict can't be proven. +1 test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
8b6d35e226
commit
42ad305073
@@ -0,0 +1,115 @@
|
|||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import { mkdtempSync, writeFileSync } from "node:fs";
|
||||||
|
import { join } from "node:path";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
|
||||||
|
import {
|
||||||
|
resolveAspectRatioForSubmit,
|
||||||
|
validateResolutionFormatCombo,
|
||||||
|
type ProjectInputSource,
|
||||||
|
} from "./render.js";
|
||||||
|
|
||||||
|
// errorBox writes to console; silence it so test output stays clean.
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.spyOn(console, "log").mockImplementation(() => undefined);
|
||||||
|
vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||||
|
});
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
/** Make process.exit throw so we can assert on the failure path. */
|
||||||
|
function trapExit() {
|
||||||
|
return vi.spyOn(process, "exit").mockImplementation((code?: string | number | null): never => {
|
||||||
|
throw new Error(`process.exit:${code ?? ""}`);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function writeComposition(width: number, height: number): string {
|
||||||
|
const dir = mkdtempSync(join(tmpdir(), "hf-cloud-render-test-"));
|
||||||
|
writeFileSync(
|
||||||
|
join(dir, "index.html"),
|
||||||
|
`<!doctype html><html><body><div data-composition-id="main" data-width="${width}" data-height="${height}"></div></body></html>`,
|
||||||
|
"utf-8",
|
||||||
|
);
|
||||||
|
return dir;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("validateResolutionFormatCombo", () => {
|
||||||
|
it("rejects 4k + webm and 4k + mov", () => {
|
||||||
|
const exit = trapExit();
|
||||||
|
expect(() => validateResolutionFormatCombo("4k", "webm")).toThrow("process.exit:1");
|
||||||
|
expect(() => validateResolutionFormatCombo("4k", "mov")).toThrow("process.exit:1");
|
||||||
|
expect(exit).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows 4k + mp4 and 1080p + any format", () => {
|
||||||
|
trapExit();
|
||||||
|
expect(() => validateResolutionFormatCombo("4k", "mp4")).not.toThrow();
|
||||||
|
expect(() => validateResolutionFormatCombo("1080p", "webm")).not.toThrow();
|
||||||
|
expect(() => validateResolutionFormatCombo(undefined, undefined)).not.toThrow();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("resolveAspectRatioForSubmit — non-local sources", () => {
|
||||||
|
it("trusts an explicit flag for asset_id / url", () => {
|
||||||
|
trapExit();
|
||||||
|
const asset: ProjectInputSource = { kind: "asset_id", assetId: "a" };
|
||||||
|
expect(resolveAspectRatioForSubmit(asset, undefined, "9:16", true)).toBe("9:16");
|
||||||
|
const url: ProjectInputSource = { kind: "url", url: "https://x/z.zip" };
|
||||||
|
expect(resolveAspectRatioForSubmit(url, undefined, undefined, true)).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("resolveAspectRatioForSubmit — local dir", () => {
|
||||||
|
it("auto-detects from composition dims when no explicit flag", () => {
|
||||||
|
trapExit();
|
||||||
|
const dir = writeComposition(1920, 1080);
|
||||||
|
expect(resolveAspectRatioForSubmit({ kind: "dir", dir }, undefined, undefined, true)).toBe(
|
||||||
|
"16:9",
|
||||||
|
);
|
||||||
|
const tall = writeComposition(1080, 1920);
|
||||||
|
expect(
|
||||||
|
resolveAspectRatioForSubmit({ kind: "dir", dir: tall }, undefined, undefined, true),
|
||||||
|
).toBe("9:16");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("accepts an explicit flag that matches the composition", () => {
|
||||||
|
trapExit();
|
||||||
|
const dir = writeComposition(1920, 1080);
|
||||||
|
expect(resolveAspectRatioForSubmit({ kind: "dir", dir }, undefined, "16:9", true)).toBe("16:9");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects an explicit flag that conflicts with the composition", () => {
|
||||||
|
const exit = trapExit();
|
||||||
|
const dir = writeComposition(1920, 1080);
|
||||||
|
expect(() => resolveAspectRatioForSubmit({ kind: "dir", dir }, undefined, "1:1", true)).toThrow(
|
||||||
|
"process.exit:1",
|
||||||
|
);
|
||||||
|
expect(exit).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects an explicit flag when the composition ratio is unsupported (no-match)", () => {
|
||||||
|
const exit = trapExit();
|
||||||
|
// 1080×1350 is 4:5 — not one of 16:9 / 9:16 / 1:1, so detection is `no-match`.
|
||||||
|
const dir = writeComposition(1080, 1350);
|
||||||
|
expect(() =>
|
||||||
|
resolveAspectRatioForSubmit({ kind: "dir", dir }, undefined, "9:16", true),
|
||||||
|
).toThrow("process.exit:1");
|
||||||
|
expect(exit).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("fails fast when the --composition entry is missing", () => {
|
||||||
|
const exit = trapExit();
|
||||||
|
const dir = writeComposition(1920, 1080);
|
||||||
|
expect(() =>
|
||||||
|
resolveAspectRatioForSubmit(
|
||||||
|
{ kind: "dir", dir },
|
||||||
|
"compositions/missing.html",
|
||||||
|
undefined,
|
||||||
|
true,
|
||||||
|
),
|
||||||
|
).toThrow("process.exit:1");
|
||||||
|
expect(exit).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -59,6 +59,7 @@ import type {
|
|||||||
HyperframesRenderDetail,
|
HyperframesRenderDetail,
|
||||||
} from "../../cloud/index.js";
|
} from "../../cloud/index.js";
|
||||||
import { isAbsolute, resolve as resolvePath } from "node:path";
|
import { isAbsolute, resolve as resolvePath } from "node:path";
|
||||||
|
import { existsSync } from "node:fs";
|
||||||
|
|
||||||
const VALID_QUALITY = ["draft", "standard", "high"] as const;
|
const VALID_QUALITY = ["draft", "standard", "high"] as const;
|
||||||
const VALID_FORMAT = ["mp4", "webm", "mov"] as const;
|
const VALID_FORMAT = ["mp4", "webm", "mov"] as const;
|
||||||
@@ -202,13 +203,23 @@ export default defineCommand({
|
|||||||
url: args.url,
|
url: args.url,
|
||||||
});
|
});
|
||||||
|
|
||||||
// When the user didn't pass --aspect-ratio explicitly AND the project is
|
// 4k supersampling runs through the alpha-incompatible screenshot path;
|
||||||
// a local dir, parse the entry HTML's root composition div for
|
// reject the combination client-side instead of failing mid-render.
|
||||||
// data-width/data-height and pick the matching supported ratio. Saves
|
validateResolutionFormatCombo(resolution, format);
|
||||||
// the user from having to specify a value that's already implicit in
|
|
||||||
// the composition they authored. Explicit flag always wins.
|
// Aspect ratio is derived from the composition's authored dimensions: for
|
||||||
const aspectRatio =
|
// a local dir we parse the entry HTML and auto-detect, so the user rarely
|
||||||
explicitAspectRatio ?? maybeAutoDetectAspectRatio(project, args.composition, asJson);
|
// needs --aspect-ratio at all. When they DO pass it, we validate it
|
||||||
|
// matches the composition (the renderer can't reshape, only supersample to
|
||||||
|
// a matching ratio) and fail fast on a mismatch. This also fails fast when
|
||||||
|
// the --composition entry file is missing, rather than uploading a zip the
|
||||||
|
// render rejects with a generic server-side error.
|
||||||
|
const aspectRatio = resolveAspectRatioForSubmit(
|
||||||
|
project,
|
||||||
|
args.composition,
|
||||||
|
explicitAspectRatio,
|
||||||
|
asJson,
|
||||||
|
);
|
||||||
|
|
||||||
const variables = resolveVariablesAndValidateIfLocal(
|
const variables = resolveVariablesAndValidateIfLocal(
|
||||||
args.variables,
|
args.variables,
|
||||||
@@ -322,7 +333,7 @@ function validateIdempotencyKey(key: string | undefined): void {
|
|||||||
// Project resolution (dir | asset-id | url) — exactly one source
|
// Project resolution (dir | asset-id | url) — exactly one source
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
interface ProjectInputSource {
|
export interface ProjectInputSource {
|
||||||
kind: "dir" | "asset_id" | "url";
|
kind: "dir" | "asset_id" | "url";
|
||||||
dir?: string;
|
dir?: string;
|
||||||
assetId?: string;
|
assetId?: string;
|
||||||
@@ -354,40 +365,100 @@ function resolveProjectInput(opts: {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Best-effort aspect-ratio detection for the cloud-render submit body when
|
* Resolve the aspect ratio for the submit body, validating local inputs.
|
||||||
* the user hasn't passed `--aspect-ratio`. Returns the detected value (one
|
|
||||||
* of `"16:9" | "9:16" | "1:1"`) or `undefined` to let the server's default
|
|
||||||
* (16:9) apply.
|
|
||||||
*
|
*
|
||||||
* Detection only fires when the project source is a local directory — for
|
* Aspect ratio is a property of the composition (its `data-width`/
|
||||||
* `--asset-id` and `--url` the composition zip isn't on disk and parsing
|
* `data-height`), not an independent render knob — the pipeline supersamples
|
||||||
* it client-side isn't worth the extra fetch. The user gets a one-line
|
* to a *matching* ratio and can't reshape. So for a local dir we auto-detect
|
||||||
* note explaining the fallback.
|
* from the entry HTML and the user rarely needs `--aspect-ratio`. Behaviour:
|
||||||
*
|
*
|
||||||
* Logs to stdout in human-readable mode; suppressed in `--json` mode so the
|
* - Local dir, no explicit flag → auto-detect and log the result.
|
||||||
* machine-readable output stays clean.
|
* - Local dir, explicit flag that conflicts with the detected dims → hard
|
||||||
|
* error (the render would otherwise fail or silently ignore the request).
|
||||||
|
* - Local dir with a missing `--composition` entry → hard error before
|
||||||
|
* upload, instead of a generic server-side render failure.
|
||||||
|
* - `--asset-id` / `--url` → the zip isn't on disk; trust an explicit flag,
|
||||||
|
* otherwise let the server default (16:9) apply.
|
||||||
|
*
|
||||||
|
* Logs are suppressed in `--json` mode so machine output stays clean.
|
||||||
*/
|
*/
|
||||||
// fallow-ignore-next-line complexity
|
// fallow-ignore-next-line complexity
|
||||||
function maybeAutoDetectAspectRatio(
|
export function resolveAspectRatioForSubmit(
|
||||||
project: ProjectInputSource,
|
project: ProjectInputSource,
|
||||||
compositionArg: string | undefined,
|
compositionArg: string | undefined,
|
||||||
|
explicit: "16:9" | "9:16" | "1:1" | undefined,
|
||||||
asJson: boolean,
|
asJson: boolean,
|
||||||
): "16:9" | "9:16" | "1:1" | undefined {
|
): "16:9" | "9:16" | "1:1" | undefined {
|
||||||
if (project.kind !== "dir") {
|
if (project.kind !== "dir") {
|
||||||
const reason = project.kind === "asset_id" ? "--asset-id" : "--url";
|
if (!explicit) {
|
||||||
logDetection(asJson, `Auto-detect skipped (project is ${reason})`);
|
const reason = project.kind === "asset_id" ? "--asset-id" : "--url";
|
||||||
return undefined;
|
logDetection(asJson, `Auto-detect skipped (project is ${reason})`);
|
||||||
|
}
|
||||||
|
return explicit;
|
||||||
}
|
}
|
||||||
|
|
||||||
const dir = project.dir ?? ".";
|
const dir = project.dir ?? ".";
|
||||||
const entryRelative = compositionArg ?? "index.html";
|
const entryRelative = compositionArg ?? "index.html";
|
||||||
const entryPath = resolvePath(dir, entryRelative);
|
const entryPath = resolvePath(dir, entryRelative);
|
||||||
|
|
||||||
|
if (!existsSync(entryPath)) {
|
||||||
|
errorBox(
|
||||||
|
"Composition not found",
|
||||||
|
`Entry file "${entryRelative}" does not exist in ${dir}.`,
|
||||||
|
"Pass --composition with a path that exists inside the project, or omit it to use index.html.",
|
||||||
|
);
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
const detection = detectAspectRatioFromHtml(entryPath);
|
const detection = detectAspectRatioFromHtml(entryPath);
|
||||||
|
|
||||||
|
if (explicit) {
|
||||||
|
// The renderer matches the composition's authored aspect ratio — it can't
|
||||||
|
// reshape. Both a `matched` ratio that differs from `explicit` AND a
|
||||||
|
// `no-match` (dims are known but the ratio isn't 16:9/9:16/1:1, so it can
|
||||||
|
// never equal the requested supported ratio) are definite conflicts.
|
||||||
|
// Other kinds (no-dims / no-root-div / invalid-dims / read-error) leave the
|
||||||
|
// ratio unknown, so we can't prove a conflict and forward the explicit value.
|
||||||
|
const conflictDetail =
|
||||||
|
detection.kind === "matched" && detection.aspectRatio !== explicit
|
||||||
|
? `${detection.width}×${detection.height} → ${detection.aspectRatio}`
|
||||||
|
: detection.kind === "no-match"
|
||||||
|
? `${detection.width}×${detection.height}, ratio ${detection.ratio.toFixed(2)} — not a supported ratio`
|
||||||
|
: undefined;
|
||||||
|
if (conflictDetail) {
|
||||||
|
errorBox(
|
||||||
|
"Aspect ratio mismatch",
|
||||||
|
`--aspect-ratio ${explicit} doesn't match the composition (${conflictDetail}).`,
|
||||||
|
"The renderer matches the composition's authored aspect ratio — it can't reshape it. Drop --aspect-ratio (it's auto-detected) or re-author the composition at the target ratio.",
|
||||||
|
);
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
return explicit;
|
||||||
|
}
|
||||||
|
|
||||||
logDetection(asJson, summarizeDetection(detection, entryRelative));
|
logDetection(asJson, summarizeDetection(detection, entryRelative));
|
||||||
return detection.kind === "matched" ? detection.aspectRatio : undefined;
|
return detection.kind === "matched" ? detection.aspectRatio : undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 4k output is produced by supersampling through the screenshot capture path,
|
||||||
|
* which doesn't support an alpha channel. webm/mov carry alpha, so the
|
||||||
|
* combination can't be satisfied — reject it before upload.
|
||||||
|
*/
|
||||||
|
export function validateResolutionFormatCombo(
|
||||||
|
resolution: "1080p" | "4k" | undefined,
|
||||||
|
format: "mp4" | "webm" | "mov" | undefined,
|
||||||
|
): void {
|
||||||
|
if (resolution === "4k" && (format === "webm" || format === "mov")) {
|
||||||
|
errorBox(
|
||||||
|
"Unsupported combination",
|
||||||
|
`--resolution 4k cannot be combined with --format ${format}.`,
|
||||||
|
"The alpha (webm/mov) capture path doesn't support 4k supersampling. Render 4k as mp4, or render alpha at composition resolution.",
|
||||||
|
);
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const ASPECT_FALLBACK_HINT =
|
const ASPECT_FALLBACK_HINT =
|
||||||
"server will default aspect_ratio to 16:9. Pass --aspect-ratio to override.";
|
"server will default aspect_ratio to 16:9. Pass --aspect-ratio to override.";
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user