feat(producer): version distributed plan protocol

[jrusso1020]

Summary

• write an explicit v1 distributed-plan protocol descriptor without changing plan hashes or rendered pixels
• accept descriptor-less historical v1 plans, while failing closed on partial, malformed, or unknown descriptors
• validate the descriptor before chunk and assembler workers inspect v1-specific artifact paths
• expose per-role planner/chunk/assembler capabilities that can represent a v2 planner with dual v1/v2 readers

Why

The current monolithic plan layout needs to evolve before PLAN_TOO_LARGE can be removed safely. Rolling out a new layout without an explicit protocol risks an old assembler silently consuming only the v1 artifacts it understands (for example, omitting future externalized audio). This PR is the pixel-neutral compatibility prerequisite; it does not introduce Plan v2 or enable any new render path.

Compatibility and rollout

• descriptor absent: treated as legacy plan-dir-v1
• known complete v1 descriptor: accepted; unknown optional metadata is ignored
• partial/malformed/unknown descriptor: typed PLAN_PROTOCOL_UNSUPPORTED before layout-specific reads
• capabilities advertise planner.produces, chunk.accepts, and assembler.accepts independently, plus descriptor-less legacy support only for reader roles
• a future v2 rollout must still use an explicit history-recorded protocol choice, a distinct v2 locator/prefix, and build-ID pinning or a dedicated canary queue; no mixed-protocol render is permitted after the first chunk

Validation

• 20 focused protocol/public-reader tests
• 41 distributed plan tests
• producer typecheck
• changed-file oxlint and oxfmt
• test classification and tracked-artifact checks
• pre-commit checks
• independent review completed after fixing validation-order and dual-reader capability blockers

[jrusso1020]

• refactor(producer): add remote-ready plan v2 publisher #2792
• feat(gcp-cloud-run): support plan protocol v2 #2790
• feat(aws-lambda): support plan protocol v2 #2789
• feat(producer): add content-addressed plan v2 #2788
• feat(producer): version distributed plan protocol #2777 👈 (View in Graphite)
• main

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

[james-russo-rames-d-jusso]

Reviewed at 156cefe414c571a4f1d425ba8871add89c931d3c.

Tight, careful compat prerequisite. All the load-bearing invariants hold end-to-end:

• Plan hash stability verified — computePlanHash inputs at freezePlan.ts:348 don't include protocol, and the delete-and-re-add invariance test at plan.test.ts:466-476 locks that in.
• Wire response shape unchanged — PlanResult.planProtocol doesn't propagate to handleRenderChunk/handlePlan responses (both packages/aws-lambda/src/handler.ts:279-293 and packages/gcp-cloud-run/src/server.ts:256-270 construct field-by-field with no ...result spread), so EF-side Pydantic contract for the sidecar plan response is unaffected — no risk of tripping the schema-mismatch fail-fast landed in heygen-ef#43345.
• EF regex parser (_PLAN_SIZE_PATTERN in _distributed_render_branch.py) unaffected — plan.ts:348 [plan] planDir size ... exceeds the configured ceiling ... message is byte-identical.
• Compat matrix — old readers on new plans ignore the extra protocol field (JSON tolerance), new readers on old plans return CURRENT_PLAN_PROTOCOL for the missing-descriptor case per readPlanProtocol lines 116-118. Both directions safe for a mixed fleet.
• Error message bounded/non-reflective — assertProtocolField at planProtocol.ts:99-103 stringifies only the expected constant, never the untrusted actual value; the test at planProtocol.test.ts:135-145 explicitly locks that.
• Fail-closed ordering — readPlanProtocol runs before any layout-specific reads in both readers (assemble.ts:124, renderChunk.ts:345), matching the PR body claim.

Three minor observations left inline. Nothing blocking.

Stack context — this is the bottom of #2777 → #2788 (v2 introduction, 1163/41) → #2789 (aws-lambda v2 support, 4561/105) → #2790 (gcp-cloud-run v2 support, 2031/243). I'll review the follow-ups as they come.

Questions for the follow-up stack

1. Does #2788 wire readPlanProtocol into a capability-aware form (readPlanProtocol(plan, acceptedProtocols)) or add a separate readPlanProtocolV2 reader? Asking because right now the DISTRIBUTED_RENDER_CAPABILITIES object is public-but-unenforced — the acceptsLegacyV1WithoutDescriptor: true flag on chunk/assembler doesn't affect reader behavior (readers unconditionally accept absent descriptor). Fine now, but the moment a role wants false, the reader needs to consult capabilities.
2. Once v2's content-addressed layout lands, does the 2 GiB PLAN_DIR_SIZE_LIMIT_BYTES cap still fire for v2 plans? If v2 splits the plan root from content-addressed artifacts, sizeBytes semantics may need adjusting so the [plan] planDir size ... message stays honest and the EF-side regex parser doesn't get fed misleading numbers.

— Review by Rames D Jusso

[miguel-heygen]

Audited: all 10 changed files end-to-end, plus the sibling AWS Lambda and GCP plan readers and retry policies.

The descriptor validation itself is carefully fail-closed: packages/producer/src/services/distributed/planProtocol.ts:111 preserves descriptor-less v1, accepts complete known v1 plus optional metadata, and rejects partial or unknown descriptors before chunk or assembler layout reads. The role-specific capability model at packages/producer/src/services/distributed/planProtocol.ts:35 also cleanly represents dual-version readers, and the existing plan-hash/pixel contract remains unchanged.

P1 blocker: packages/producer/src/services/distributed/planProtocol.ts:73 defines this deterministic incompatibility as unable to heal on retry, but the new error is not wired into either shipped retry classifier. AWS omits PlanProtocolUnsupportedError from the chunk/assemble non-retryable lists at packages/aws-lambda/src/cdk/HyperframesRenderStack.ts:206; GCP omits it from NON_RETRYABLE_ERROR_NAMES at packages/gcp-cloud-run/src/server.ts:577, which maps it to a retryable HTTP 500. A partial or future-v2 plan will therefore exhaust retries on workers that can never consume it. Please classify the new typed error as terminal in both adapters and pin the behavior in the CDK snapshot and GCP HTTP tests.

Fresh verification on exact head 156cefe414c571a4f1d425ba8871add89c931d3c: focused protocol/public-export tests (20/20), distributed plan tests (41/41), producer typecheck, and git diff --check all pass; required CI is green.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The core reader contract is sound, but the unsupported-protocol failure is retryable at both production adapter boundaries, so the central version-skew path does not yet fail terminally as designed.

[vanceingalls]

Verdict: REQUEST CHANGES

Well-scoped, pixel-neutral compat prerequisite: descriptor lands on plan.json, renderChunk/assemble fail-closed on unknown or partial descriptors before touching v1 artifacts, capabilities model already supports dual-version readers. Independent adversarial pass confirms hash stability, mixed-fleet compat, and error bounding. Matching Miguel's P1 — the terminal PlanProtocolUnsupportedError is not classified as non-retryable in either shipped adapter, which negates the "retrying on the same worker cannot heal it" guarantee at the boundary that actually decides retryability. One additional P2 in the same adapter-drift family, plus a few P3 nits.

Reviewer inventory

• Miguel (CHANGES_REQUESTED) — one P1: PlanProtocolUnsupportedError missing from AWS NON_RETRYABLE_* lists in HyperframesRenderStack.ts and from GCP NON_RETRYABLE_ERROR_NAMES in server.ts. Verified independently below; matched.
• Rames D (COMMENTED) — audit of pixel/wire/EF-regex/mixed-fleet/error-bounding/ordering. Nothing blocking. Forward-looking Q1 (capability flag is public-but-unenforced) overlaps with a P3 I found; noted.

Plan-size root-cause verification

The driving problem — distributed renders failing because the plan payload is too large — is not addressed by this PR, and the PR body says so plainly ("this PR is the pixel-neutral compatibility prerequisite; it does not introduce Plan v2"). Root-cause verified: foundation PR, root-cause fix deferred to the v2 introduction later in the stack (#2788 per Rames' context).

Pixel-neutrality is empirically supported: computePlanHash at packages/producer/src/services/render/stages/freezePlan.ts:348 feeds only compositionHtml + on-disk asset shas + raw meta/encoder.json bytes + fontSnapshotSha + producerVersion + ffmpegVersion + dimensions — never plan.json — and plan.test.ts:466-478 now round-trips add/delete of the protocol field and asserts the hash is unchanged.

Findings

P1 — PlanProtocolUnsupportedError is retryable at both adapter boundaries (matched with Miguel). packages/producer/src/services/distributed/planProtocol.ts:112-115 documents the failure as "typed, deterministic … retrying on the same worker cannot heal it," but that intent stops at the producer boundary:

• packages/aws-lambda/src/cdk/HyperframesRenderStack.ts:196-220 — NON_RETRYABLE_PLAN / NON_RETRYABLE_CHUNK / NON_RETRYABLE_ASSEMBLE list PLAN_HASH_MISMATCH, FFMPEG_VERSION_MISMATCH, PLAN_TOO_LARGE, etc. but omit PLAN_PROTOCOL_UNSUPPORTED and PlanProtocolUnsupportedError.
• packages/gcp-cloud-run/src/server.ts:577-593 — NON_RETRYABLE_ERROR_NAMES lists the analogous producer error names/codes but omits both PlanProtocolUnsupportedError and PLAN_PROTOCOL_UNSUPPORTED, so the HTTP shell maps it to 500 and the workflow retries.

Consequence: once a partial or unknown-version descriptor reaches a worker (the exact scenario Plan v2's rollout depends on catching cleanly), the Step Functions state / Cloud Run workflow burns its full retry budget on a case that cannot heal. Add the class name AND the string code alias to both classifiers, pin with a CDK snapshot assertion and a GCP HTTP .name === 'PlanProtocolUnsupportedError' test.

P2 — Downstream test mocks lie about the new required PlanResult.planProtocol field. plan.ts widens PlanResult with a required planProtocol: Readonly<PlanProtocolV1Descriptor>, but two adapter mocks still construct PlanResult literals without it: packages/aws-lambda/src/handler.test.ts:158-169, 225-235 and packages/gcp-cloud-run/src/server.test.ts:57-69. CI passes only because both adapter tsconfig.jsons exclude src/**/*.test.ts from typecheck, so the assignability drift is silent. Same theme as the P1: the producer widened its contract and neither adapter was updated. Add planProtocol: CURRENT_PLAN_PROTOCOL to each mock — or, if the plan is to keep adapters strictly consumer-side, drop the .test.ts exclude in tsconfig and let the compiler enforce it.

P3 — DISTRIBUTED_RENDER_CAPABILITIES.acceptsLegacyV1WithoutDescriptor is decorative today. planProtocol.ts:41-60 exposes the flag on chunk and assembler, but readPlanProtocol (lines 116-119) accepts descriptor-less plans unconditionally without consulting the capability payload. Fine while every reader still accepts legacy — the flag is truthful — but the moment a role advertises false, the reader must consult it. Consider readPlanProtocol(planJson, role?) taking the caller role, or renaming to make the "wire-only" nature explicit until enforcement lands. (Same observation Rames flagged as follow-up Q1.)

P3 — recomputePlanHashFromPlanDir implicitly assumes v1. freezePlan.ts:263-297 reads plan.json, ignores protocol, and computes a v1-shaped hash. All three current callers (freezePlan, renderChunk at renderChunk.ts:425, plan.test.ts) happen to have readPlanProtocol earlier in their flow, so today it is safe. But once v2 lands, a v2 plan reaching this helper through a new caller silently computes the wrong hash. Rename to recomputeV1PlanHashFromPlanDir or gate on readPlanProtocol(planJson) at the top so the v1 assumption is enforced, not implicit.

P3 — readPlanProtocol return type is too narrow for the eventual v2 branch. Signature is Readonly<PlanProtocolV1Descriptor> and the function always returns the singleton. Once v2 exists, callers may want to discriminate on p.schemaVersion, but TypeScript reports the === 2 branch as unreachable because the literal type is 1. Widen to Readonly<PlanProtocolDescriptor> and return the descriptor that was read, or document that this is validation-only forever.

P3 — assertProtocolField doesn't correlate field with expected. planProtocol.ts:97-114 types field: keyof PlanProtocolV1Descriptor and expected: PlanProtocolV1Descriptor[keyof PlanProtocolV1Descriptor] independently, so assertProtocolField(desc, "schemaVersion", "plan-dir-v1") currently typechecks. Tighten with <K extends keyof PlanProtocolV1Descriptor>(descriptor, field: K, expected: PlanProtocolV1Descriptor[K]).

P3 — Sentinel-based "protocol accepted" test assertion. planProtocol.test.ts:216-228 relies on renderChunk(planDir, 999, ...) throwing CHUNK_INDEX_OUT_OF_RANGE to show that legacy protocol handling passed. A future reordering of renderChunk validation could break the sentinel silently. Prefer asserting readPlanProtocol(legacyPlanJson).toBe(CURRENT_PLAN_PROTOCOL) directly for the descriptor-less case.

Adversarial lenses

• A — Plan-size root cause: not addressed; PR is the honestly-scoped prerequisite. Deferred to v2 in #2788.
• B — Schema evolution / mixed fleet: sound. Descriptor absent → legacy v1; unknown or partial → typed PLAN_PROTOCOL_UNSUPPORTED before v1 artifact reads; DISTRIBUTED_RENDER_CAPABILITIES expresses planner.produces/chunk.accepts/assembler.accepts with acceptsLegacyV1WithoutDescriptor: true. Rollout stays safe because v1 semantics are unchanged — but see P1 above for where the rollout guarantee currently breaks at the adapter retry boundary.
• C — Serialization boundary: clean. CURRENT_PLAN_PROTOCOL is a frozen literal of primitives; plan.json is JSON.stringify(_, null, 2); not hashed; delete-restore round-trip verified. No Date / RegExp / Map / Buffer / circular refs introduced.
• D — Chunking / streaming semantics: N/A; deferred to Plan v2. Refire this lens then.
• E — Observability / failure attribution: PlanProtocolUnsupportedError carries code: PLAN_PROTOCOL_UNSUPPORTED, name self-identifies subsystem ([planProtocol] …), message is bounded (<200 chars) and non-reflective per planProtocol.test.ts:132-146. Retryability is called out in the doc-comment — but the adapters currently override that intent (P1).

— Via

[jrusso1020]

Addressed the full review on the new Graphite head: capability declarations now drive protocol-reader acceptance; descriptor matching no longer permits field/constant type drift; PLAN_PROTOCOL_UNSUPPORTED and PlanProtocolUnsupportedError are terminal in AWS and GCP; adapter PlanResult mocks include the required protocol; and the complete stack was restacked. Focused result: producer protocol/v2 25/25, AWS handler/CDK 18/18, GCP server 20/20, all three package typechecks, formatting, lint, and pre-commit checks passed.

[miguel-heygen]

Re-review on exact head 607a74568c98bda16c8fc4ee8d73b577f2dffec3.

The prior blocker is fully addressed:

• AWS now classifies both PLAN_PROTOCOL_UNSUPPORTED and PlanProtocolUnsupportedError as terminal for plan, chunk, and assemble at packages/aws-lambda/src/cdk/HyperframesRenderStack.ts:196, with the union pinned by HyperframesRenderStack.snapshot.test.ts:61.
• GCP now maps the typed incompatibility to non-retryable HTTP 400 at packages/gcp-cloud-run/src/server.ts:577, pinned through the real HTTP surface at server.test.ts:298.
• The stale adapter fixtures now include CURRENT_PLAN_PROTOCOL in both AWS and GCP PlanResult literals.

The follow-up capability change is also coherent: readPlanProtocol now enforces the caller's accepted set and descriptor-less legacy policy at packages/producer/src/services/distributed/planProtocol.ts:108, while chunk and assembler pass their role-specific capabilities before reading v1 artifacts.

Fresh local verification: producer protocol/public-export/plan suites 63/63, AWS handler/CDK 15/15, GCP server 17/17, all three package typechecks, and git diff --check pass. Required CI is green with no failed check runs; optional regression shards were still in progress at review time.

— Magi

Verdict: APPROVE
Reasoning: The terminal retry semantics and adapter contract drift are fixed with direct regression coverage, and the exact-head delta introduces no new blocking correctness issues.

[vanceingalls]

Re-reviewed at 607a74568c98bda16c8fc4ee8d73b577f2dffec3 (single-commit rewrite; branch force-pushed since R1's 156cefe4).

Verdict: APPROVE

Both P1 (retry-classifier adapter drift, convergent with Miguel) and P2 (adapter test-mock lie) are cleanly resolved, and the addressable P3s were folded in as well. Independent light adversarial pass on the new-code delta surfaces no new blockers. Miguel already re-approved at this SHA.

R2 verification

R1 finding	New head status	Evidence
P1 — PlanProtocolUnsupportedError not in AWS NON_RETRYABLE_* lists nor GCP NON_RETRYABLE_ERROR_NAMES	Addressed	packages/aws-lambda/src/cdk/HyperframesRenderStack.ts:203-224 adds both "PLAN_PROTOCOL_UNSUPPORTED" and "PlanProtocolUnsupportedError" to all three lists (Plan, Chunk, Assemble). packages/gcp-cloud-run/src/server.ts:587-591 adds both to NON_RETRYABLE_ERROR_NAMES. Snapshot test at HyperframesRenderStack.snapshot.test.ts:67-70 asserts both names appear in the union of the three tasks' Retry blocks with MaxAttempts: 0. GCP HTTP layer pins the mapping at server.test.ts:305-330 — new test case asserts PlanProtocolUnsupportedError returns HTTP 400 with error === "PlanProtocolUnsupportedError".
P2 — adapter mocks construct PlanResult without required planProtocol	Addressed	packages/aws-lambda/src/handler.test.ts:22, 165, 233 and packages/gcp-cloud-run/src/server.test.ts:22-27, 64 now import CURRENT_PLAN_PROTOCOL and set planProtocol: CURRENT_PLAN_PROTOCOL on every PlanResult literal. Adapter tsconfigs still exclude src/**/*.test.ts from typecheck — a broader structural concern that isn't in scope here — but the drift is fixed at the mock level.
P3 — capability flag decorative (acceptsLegacyV1WithoutDescriptor unread)	Addressed	planProtocol.ts:118-138 — readPlanProtocol now takes a capabilities argument (defaulted to chunk) and enforces both acceptsLegacyV1WithoutDescriptor and capabilitiesAccept() on the descriptor-less path. renderChunk.ts:341 passes roles.chunk; assemble.ts:124 passes roles.assembler. Tests at planProtocol.test.ts:157-176 cover a worker rejecting descriptor-less legacy and one with an empty accepts set.
P3 — assertProtocolField typing lets wrong constant type-check per field	Addressed via refactor	The helper was replaced with a straight hasOwnProperty loop (planProtocol.ts:144-148) followed by protocolMatches (:82-91), which does field-by-field equality against the frozen CURRENT_PLAN_PROTOCOL. No mis-correlatable API remains.
P3 — sentinel-based "protocol accepted" test	Addressed	planProtocol.test.ts:107-109 now asserts readPlanProtocol({ planHash: "legacy" }) directly returns CURRENT_PLAN_PROTOCOL; the renderChunk-sentinel case at :212-222 is now a supplementary integration check rather than the primary signal.
P3 — recomputePlanHashFromPlanDir implicit v1 assumption	Acceptable, unchanged	Not renamed / not gated on readPlanProtocol. All three current callers still run readPlanProtocol earlier in-flow; today's callers are safe. Forward-looking risk when v2 lands is unchanged from R1.
P3 — readPlanProtocol return type narrow to v1	Acceptable, unchanged	Still Readonly<PlanProtocolV1Descriptor> returning the singleton. Both call sites at renderChunk.ts:341 / assemble.ts:124 drop the return, which James self-noted and marked as intentional ("v2 is discriminated and materialized at the adapter boundary"). Non-blocking.

Fresh adversarial pass (new-code delta only)

• Validation ordering: renderChunk.ts:335-354 now checks plan.json existence, parses, calls readPlanProtocol, then checks the v1-specific encoder.json / chunks.json. assemble.ts:120-125 mirrors this — protocol read happens before the chunks.json existence check. Fail-closed order matches the PR's stated invariant.
• PlanResult widening: required field is safe for the in-process pipeline (result flows producer → adapter in memory, not persisted); both adapter mocks are updated.
• Serialization boundary: freezePlan.ts:359 writes protocol: CURRENT_PLAN_PROTOCOL as the first key of plan.json via the standard JSON.stringify(_, null, 2) path; the frozen literal contains only primitives. plan.test.ts:466-478 round-trips add/delete of the protocol field against recomputePlanHashFromPlanDir and asserts the hash is unchanged — pixel-neutrality empirically preserved.
• Public surface parity: distributed.ts:85-100 and index.ts:135-152 export the same protocol contract identifiers; publicExports.test.ts:67-108 asserts both entry points expose the identical singleton and helpers. No divergence.
• Adversarial lenses (new):
  • Project code-lint: no new as T at JSON boundaries beyond the pre-existing as PlanJson / as PlanJsonForAssemble patterns, which are immediately re-validated by readPlanProtocol's isRecord. No ! non-null assertions.
  • Spec bullet forward + reverse: every claim in the PR body's Summary / Compatibility section maps to code (freezePlan writes descriptor; readPlanProtocol handles absent + partial/malformed/unknown; renderChunk/assemble validate before v1-artifact reads; DISTRIBUTED_RENDER_CAPABILITIES exposes per-role capabilities). Reverse-scan finds nothing in the diff that isn't declared in the body.
  • Sibling-precision divergence: PLAN_PROTOCOL_UNSUPPORTED and PlanProtocolUnsupportedError appear as string literals across HyperframesRenderStack.ts (three lists), server.ts, HyperframesRenderStack.snapshot.test.ts, and server.test.ts — every occurrence matches byte-exact. The EXPECTED_NON_RETRYABLE_ERRORS set is asserted against the union across the three AWS tasks (not per-task), so a future partial regression on a single list could pass the snapshot — pre-existing behavior, not introduced here.
  • Middle-man wrap/unwrap: readPlanProtocol return value is dropped at both call sites; James confirmed intentional in the inline discussion. No sibling fields dropped elsewhere.

Non-blocking observation: the adapter tsconfig.json .test.ts excludes remain in place, so the mock-drift class of bug will still be caught at runtime rather than compile time. Fine for this PR; worth a standalone follow-up if the team wants compile-time enforcement of adapter-side PlanResult compat.

— Via