refactor(producer): unify render requests

[jrusso1020]

What

Define one serializable RenderRequest and normalize environment input at the boundary.

Why

CLI, Docker, server, and distributed adapters copied option bags and silently dropped fields.

How

Add a shared request schema, explicit adapters, round-trip validation, and thread it through local/server/distributed entry points.

Test plan

• render request round-trip, server parsing, CLI Docker, and distributed validation tests
• Stack-wide lint, format, build, typecheck, and relevant integration gates

[jrusso1020]

• refactor(repo): resolve changed-code audit #2174
• test(engine): isolate peer abort regression #2173
• refactor(cli): decompose render phases #2172
• refactor(cli): centralize process lifecycle #2171
• fix(cloudrun): publish adapter contract #2170
• test(repo): execute packed subpaths #2169
• refactor(repo): centralize package subpaths #2168
• refactor(core): own edit protocol contract #2167
• test(compiler): enforce preview render parity #2166
• refactor(studio): gate SDK operation families #2165
• refactor(studio): gate Acorn writer migration #2164
• refactor(producer): make capture plans immutable #2163
• refactor(producer): unify render requests #2162 👈 (View in Graphite)
• refactor(engine): type capture failures #2161
• refactor(engine): manage child process lifecycles #2160
• refactor(engine): add fingerprinted browser leases #2159
• refactor(render): centralize execution ownership #2158
• refactor(core): unify composition contract #2157
• fix(studio): enforce optimistic file concurrency #2156
• fix(studio): make sdk cutover transactional #2155
• fix(render): publish artifacts atomically #2154
• fix(render): surface structured outcomes #2153
• fix(player): own connection and media resources #2152
• fix(core): preserve runtime transport contract #2151
• fix(player): version runtime protocol #2150
• test(repo): verify packed consumers #2149
• test(repo): enforce workspace contracts #2148
• fix(core): enforce strict runtime safety #2147
• test(producer): gate source tests by execution lane #2146
• fix(core): make timing masks safe for source loaders #2145
• chore(repo): forbid tracked generated artifacts #2144
• main

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

[miguel-heygen]

The consolidation direction is good: local CLI/server callers now normalize environment-derived engine state once, Docker preserves the two millisecond timeout flags, and the request version gives future wire changes an explicit boundary. I read the complete 10-file diff and traced the new request through local, server, distributed-plan, Lambda, and Cloud Run surfaces. The automated LGTM says the field mappings are complete; two contract gaps contradict that conclusion.

[P1] Preserve every distributed field in both adapters — packages/producer/src/renderRequest.ts:165-191,204-227

RenderRequestOptions declares strictness and outputResolutionAspectAgnostic (:33,41), and DistributedRenderConfig supports both (packages/producer/src/services/distributed/plan.ts:136,216), but distributedConfigFromRequest() emits neither and renderRequestFromDistributedConfig() reconstructs neither. On this exact head, a strict portrait request round-trip reports all four values as undefined:

distStrictness: undefined
distAspect: undefined
backStrictness: undefined
backAspect: undefined

The aspect flag is operational: the compile stage uses it to retarget aliases such as 1080p to the composition orientation. Dropping it can make distributed portrait renders resolve the wrong preset; dropping strictness silently changes warning/failure policy. The tests at renderRequest.test.ts:69-77,89-96 use subset toMatchObject assertions that omit both fields, so the advertised round-trip test cannot catch the loss. Map both directions and pin every shared field (or assert a normalized full-object equality).

[P1] Validate JSON safety before serialization instead of silently normalizing caller data — packages/producer/src/renderRequest.ts:78-92,111-134

The new public boundary calls a JSON round-trip proof of serializability, but JSON.stringify is lossy: it drops undefined and functions, converts NaN/Infinity to null, stringifies Date, and throws only for some invalid values such as BigInt/cycles. I reproduced all of these relevant shapes on the exact head. For example, variables: { keep: "yes", lost: undefined } returns a request containing only { keep: "yes" }, and serializing { bad: NaN } returns { bad: null }. parseRenderRequest() also accepts malformed typed fields such as workers: "many" because most options are never validated.

This is silent user-data mutation at the schema that the PR says prevents fields from being dropped. The repository already has the correct recursive JSON-safety contract in renderConfigValidation.ts:225-287 (validateVariablesPayload/walkVariables). Validate the raw value before stringification, validate the full parsed request shape, and add rejection tests for undefined/function/BigInt/non-finite numbers/non-plain objects/cycles plus malformed optional scalar and distributed fields.

Important follow-up in the same wire contract: distributedConfigFromRequest() returns a runtime DistributedRenderConfig containing both producerConfig and engineConfig (renderRequest.ts:184-186). SerializableDistributedRenderConfig excludes producerConfig only statically, while the Lambda and Cloud Run SDKs serialize opts.config verbatim. A variable returned by this function can therefore cross the wire with the runtime-only duplicate field despite satisfying structural typing. A distinct serializable adapter (or omitting producerConfig from the cloud-facing conversion) would make the ownership explicit.

Verification: exact head 85820d284d3383bd7213b96780b681595f27f201; complete main...head diff read (10 files, +469/-47); focused RenderRequest suite 5/5; full workspace build passed; direct Bun repros confirmed both omitted adapter fields, undefined deletion, NaN -> null, and malformed workers acceptance. GitHub reports all exact-head required checks green and the PR is mergeable; this is a runtime/wire-contract blocker, not a CI issue.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The shared request currently drops two operational fields across the distributed adapters and silently mutates JSON-unsafe caller data at the boundary intended to prevent data loss. Both need explicit mapping and validation before this can serve as the canonical render request.

[jrusso1020]

Addressed Magi’s P1s on exact head 82eff92. The distributed adapters now preserve strictness and outputResolutionAspectAgnostic in both directions, validate the reverse wire input with the shared distributed validator, and omit the runtime-only producerConfig duplicate while retaining engineConfig for serialization. RenderRequest now rejects JSON-unsafe values and malformed optional/distributed fields before serialization. Regressions cover the full adapter contract, unsafe values, and malformed fields. Validation passed: focused RenderRequest 8/8, Lambda/Cloud Run config 52/52, Producer unit 356/356, Producer typecheck, lint, format, diff check, and commit hooks. Fresh stack CI is running.

[jrusso1020]

Follow-up CI compatibility fix published on exact head 3d0c90b. Windows render and CLI smoke exposed that the CLI legitimately passes absent optional fields (for example gifLoop) as own properties with undefined. The request boundary now strips only undefined top-level typed options before JSON validation; nested user payloads such as variables still reject undefined and every other JSON-unsafe value. Added regression coverage; focused RenderRequest is 9/9, Producer typecheck/lint/format/diff check and commit hooks pass. Fresh stack CI is running.

[miguel-heygen]

Additive re-review on exact head ab91290a2f918fa9ddbf9da55c3ac1e7e6c58970: the prior adapter and JSON-boundary blockers are substantially fixed. strictness and outputResolutionAspectAgnostic now survive both directions (packages/producer/src/renderRequest.ts:276-303,333-347), nested JSON-unsafe values are rejected before serialization (:211-245), absent top-level options are intentionally omitted (:207-245), and the distributed conversion no longer emits runtime-only producerConfig (:276-303). The new tests pin all four behaviors. I also verified the Studio reliability work seen in the old-head delta is now base commit 2b65b4efc; no Studio/Studio Server path is in this PR's current base...head diff.

[P1] Validate the resolved engine snapshot, not just that it is a JSON object — packages/producer/src/renderRequest.ts:178-185,211-230

The remaining parse boundary still treats any plain JSON object as a complete EngineConfig. On this exact head, all of these are accepted and forwarded verbatim by renderConfigFromRequest() (:248-257):

engineConfig: {}
engineConfig: { ...DEFAULT_CONFIG, protocolTimeout: "forever" }
engineConfig: { ...DEFAULT_CONFIG, browserGpuMode: "turbo" }

This bypasses fallback resolution because the orchestrator selects job.config.producerConfig ?? resolveConfig() (packages/producer/src/services/renderOrchestrator.ts:1635): an accepted {} is truthy, so required defaults remain missing, while malformed timeout/GPU values reach execution. The distributed reverse path has the same hole: validateDistributedRenderConfig() only walks engineConfig for JSON safety (packages/producer/src/services/distributed/renderConfigValidation.ts:203-208), then renderRequestFromDistributedConfig() installs it as the resolved snapshot (renderRequest.ts:348-352). This also weakens version compatibility: a v1 request produced before a new required EngineConfig field is added will still parse as v1 but run without the new default.

Please validate the full engine snapshot (required fields, scalar/enum shapes and ranges) or normalize a validated partial snapshot through an explicit versioned wire schema before treating it as resolved. Add regressions for {}, a wrong scalar type, and an invalid enum in both direct parse and reverse-distributed input.

Verification: current PR diff is 10 files, +670/-48; git diff --check passes; focused RenderRequest tests pass 9/9; AWS/GCP distributed validation tests pass 52/52; exact-head repro confirms both adapter fields round-trip, nested undefined rejects, top-level absent options omit, producerConfig stays off the wire, and malformed engineConfig still accepts. All exact-head GitHub checks are green; the PR is mergeable but blocked by review state.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The original data-loss issues are fixed, but the canonical versioned parser still accepts and forwards an incomplete or malformed engine snapshot, bypassing defaults and allowing invalid execution configuration across local and distributed boundaries.

[jrusso1020]

Addressed Magi’s engine-snapshot P1 on exact head 44ee5f6423f57e81f43defb9dbad7fb9f58995d7 (rebased on main@7acabbcde). A shared Engine validator now requires every resolved snapshot field, rejects unknown fields, checks scalar ranges/enums and HDR/path shapes, and is applied to both direct RenderRequest parsing and reverse distributed input. Regressions cover {}, protocolTimeout: "forever", and browserGpuMode: "turbo" on both paths. Validation passed: Engine config + RenderRequest tests 108/108, Engine and Producer typechecks, lint, format, changed-code fallow gate, tracked-artifact check, and commit hooks. Fresh exact-head CI is running.

[miguel-heygen]

The snapshot validation is a good direction, and I verified the earlier strictness / aspect-field loss, JSON-safety, runtime-only producerConfig, and plain-object-only validation gaps remain fixed on exact head fc4e9a6c0. I also traced the full direct and reverse-distributed boundaries. One valid canonical snapshot still cannot cross them, and the distributed error contract regresses on the new validator path:

• [blocker] packages/engine/src/config.ts:346-360,429-461 — accept the full output domain of resolveConfig(). coresPerWorker is first accepted as a finite number >= 0, but it is also in POSITIVE_NUMBER_ENGINE_CONFIG_FIELDS, where the second pass requires >= 1. The canonical producer at config.ts:744 parses PRODUCER_CORES_PER_WORKER as a number without imposing that minimum, and the runtime consumes fractional values as a divisor in parallelCoordinator.ts:223-258. Exact-head falsifier:

  PRODUCER_CORES_PER_WORKER=0.5
  resolveConfig().coresPerWorker === 0.5
  validateEngineConfigSnapshot(resolveConfig())
  => Error: Engine config coresPerWorker must be a finite number >= 1
  

  This makes a previously valid CLI/environment configuration fail synchronously in createRenderRequest() at renderRequest.ts:232-242, before rendering. Please remove coresPerWorker from the generic >= 1 list, validate it once as finite > 0, and pin a fractional canonical snapshot such as 0.5 through the request boundary. The governing invariant is: every complete snapshot produced by canonical resolveConfig must validate and round-trip unchanged.

• [important] packages/producer/src/services/distributed/renderConfigValidation.ts:38-49,207-212 — preserve the public InvalidConfigError contract for nested engine failures. This API documents that every client-side violation throws InvalidConfigError with a dotted .field, and every other branch does so. validateEngineConfigSnapshot() throws plain Error, but this call is not wrapped, so malformed config.engineConfig now escapes without instanceof InvalidConfigError or field. Please translate it to InvalidConfigError("config.engineConfig", ...) (or inject the error factory) and add an SDK-facing assertion for the type and field.

Adversarial coverage: I enumerated canonical/environment producers, direct CLI/server consumers, reverse distributed/AWS/GCP consumers, wire encoding, and terminal effects; read the complete 12-file diff and all prior reviews/comments; verified exact head/base freshness and all green checks. The first finding is reproduced directly against the exact source head. I did not re-run the full workspace matrix locally; GitHub’s exact-head matrix is green.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The new schema validator rejects a semantically supported snapshot emitted by its own canonical resolver, causing a real pre-render compatibility break, and its distributed integration bypasses the SDK's typed validation-error contract.

[jrusso1020]

Addressed the fresh review on exact head 1a66c881b8f83932ebbf2f9c09b42f2f63de163e.

• validateEngineConfigSnapshot() now accepts the canonical positive fractional coresPerWorker domain (for example 0.5) while continuing to reject zero and invalid values; coresPerWorker is no longer subjected to the unrelated >= 1 runtime-field pass.
• Distributed nested engine-snapshot validation now translates failures to InvalidConfigError with field === "config.engineConfig", preserving the documented SDK error contract.
• Regressions cover a resolveConfig({ coresPerWorker: 0.5 }) snapshot through direct serialization and reverse distributed input, plus the typed distributed error/field assertion.

Validation passed: Engine config + RenderRequest tests 110/110; Engine and Producer typechecks; lint; format; diff check; tracked-artifact; and commit hooks. The stack has been restacked and republished; fresh exact-head CI is running.

[miguel-heygen]

Re-review on exact head 1a66c881b: both outstanding contracts are fixed.

• packages/engine/src/config.ts:396-401,435-446 now validates coresPerWorker exactly once as finite and strictly positive, so canonical fractional values survive while zero, negatives, NaN, and infinity reject. The boundary regression at packages/producer/src/renderRequest.test.ts:207-233 pins 0.5 through direct serialization and the reverse distributed adapter.
• packages/producer/src/services/distributed/renderConfigValidation.ts:210-218 translates nested snapshot failures back into the public InvalidConfigError with field === "config.engineConfig"; packages/producer/src/renderRequest.test.ts:235-250 pins both the type and field.

Invariant: every complete snapshot produced by canonical resolveConfig() crosses direct and distributed request boundaries unchanged; malformed distributed snapshots terminate with the documented typed field error. Contract ledger: producers are environment/programmatic resolveConfig and distributed wire input; consumers are createRenderRequest/parse and reverse distributed validation; siblings are CLI/server plus AWS/GCP adapters; encoding owner is the shared Engine snapshot validator, with distributed translation owned by validateDistributedRenderConfig; terminal failure would be a valid render rejected before execution or SDK error handling losing its field identity. Falsifiers: on this exact source head, 0.5 resolves and validates, while 0, negative, NaN, and infinity all reject; source tracing confirms every snapshot-validator call is either the direct boundary or the typed distributed wrapper.

Audited: the complete 12-file base...head diff, the 3-file repair delta, both request boundaries, validator producers/consumers, new tests, all prior reviews/comments, and exact head/base state. Trusting: the author-reported 110 focused tests and currently running GitHub matrix; I independently ran the exact Engine-domain probe but did not install dependencies into the clean review worktree. Not exercised: live AWS/GCP launch, because the repaired failure is synchronous before either provider call. Terminal effect: none found after the fixes.

— Magi

Verdict: APPROVE
Reasoning: The canonical snapshot domain now round-trips without narrowing, malformed snapshots retain the distributed SDK's typed error contract, and focused regressions cover both repaired boundaries. No code blocker remains on this exact head; exact-head CI is still in progress and remains the merge gate.