feat(aws-lambda): support plan protocol v2

[jrusso1020]

Summary

• add explicit PlanProtocol: "v1" | "v2" support to the AWS Lambda SDK, handler, SAM template, and CDK state machine; absence still defaults to v1
• transport v2 as a small manifest plus immutable content-addressed S3 artifacts
• materialize only chunk-targeted artifacts on render workers and assembler-targeted artifacts on assembly workers
• add local parity/pressure harnesses and a live smoke mode that runs v1 and v2 against the same fixture
• make smoke resources collision-resistant, fail closed on AWS verification errors, purge versioned buckets, discover retained resources after failed deploys, and verify cleanup

Live AWS validation

Profile engineering-767398024897, region us-east-2.

Visual fixture

• identical 4 chunk hashes
• identical 60 decoded frames
• identical metadata and duration
• byte-identical final MP4: 094e92736f986e9c4c5cad0803b982136c11d013a63179bcf3b0dc81728dd20a
• both protocols exceeded the fixture PSNR gate

Visual + audio fixture

• identical 4 chunk hashes
• identical 300 decoded frames
• identical metadata and 10.005333s duration
• identical decoded PCM: 1,921,024 bytes, 99335b1a89d630b071ed5cc9cd5ff8a85e0774878ab5d4e9208c869999e2a577
• byte-identical final MP4: b5c8e7350d23cf77e78642f0a2a8122426696748ce678416c2177e780dca920b

All test stacks, Lambda functions, state machines, log groups, render buckets, deploy buckets, object versions, and delete markers were removed and independently verified absent.

A final live rerun also exercised the atomic stack-name reservation and
ownership-gated teardown path: v1/v2 remained byte-identical, cleanup passed,
and independent API queries again confirmed every scoped resource absent.

Other validation

• 121 AWS tests
• 42 focused producer tests
• shell isolation, S3 purge, and semantic-comparison tests
• producer/AWS typechecks, lint, format, tracked-artifact, Fallow, and SAM validation gates
• handler ZIP size gate: 122.0 MiB compressed / 236.4 MiB uncompressed
• pressure case: v1 returns typed PLAN_TOO_LARGE at 189,170 bytes against a 32,768-byte test cap; v2 completes

Rollout

This remains explicit opt-in. Before broad production enablement we should remove full-v1 local staging, budget the v2 materialized working set, narrow compiled/** to reachable assets or packs, and benchmark S3 object count/cold-start behavior.

GCP Cloud Run remains on Plan v1 and needs its own manifest/CAS transport and materializer migration; it will not automatically consume a v2 root.

Stack

Base: feat/plan-protocol-v2 / #2788

[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 👈 (View in Graphite)
• feat(producer): add content-addressed plan v2 #2788
• feat(producer): version distributed plan protocol #2777
• main

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

[miguel-heygen]

Fresh exact-head review at 5e12d686ebee47863f642b14fc0b971979241878.

The core v2 AWS path is carefully constructed: manifest-last publication and verified target materialization are coherent in packages/aws-lambda/src/handler.ts:342-412,487-706, and the CDK state machine correctly classifies both the new producer class/code and S3 digest failures as terminal (packages/aws-lambda/src/cdk/HyperframesRenderStack.ts:198-238). The per-v2-task CDK snapshot pins that contract.

Audited: packages/aws-lambda/src/{events,handler,s3Transport}.ts, the CDK state machine and snapshot, SDK protocol propagation, SAM v1/v2 state-machine branches, and their focused tests.

Trusting: the large parity harness and live-smoke shell implementation beyond its protocol/resource-lifecycle integration points, plus documentation/fixtures.

Blocker

• examples/aws-lambda/template.yaml:296-310,487-499,528-540 — the SAM state machine omits both PLAN_V2_INTEGRITY_UNRECOVERABLE and PlanV2IntegrityError from the terminal retry lists for PlanV2, RenderChunkV2, and AssembleV2. The sibling CDK state machine includes both at HyperframesRenderStack.ts:206-207,221-222,232-233, but the separately deployed SAM path does not. Therefore a malformed v2 manifest or deterministic post-materialization integrity failure is terminal under CDK yet consumes all four States.ALL retries under SAM. This is the same non-healing retry-budget bug the stack is intended to close. Add both aliases to all three SAM v2 tasks and pin semantic SAM/CDK classifier parity in a test; sam validate checks syntax, not this contract.

Verification: AWS typecheck passed; handler, S3 transport, CDK snapshot, progress, and SDK tests passed 55/55; git diff --check passed. The current regression-shard failure is infrastructure-only: every shard failed/cancelled while BuildKit timed out pulling moby/buildkit from Docker Hub before tests ran.

Verdict: REQUEST CHANGES
Reasoning: The CDK rollout handles the new terminal producer error correctly, but the equally supported SAM state machine leaves every v2 task retrying deterministic integrity failures.

— Magi

[vanceingalls]

Verdict: REQUEST_CHANGES

Adversarial lenses

(A) Terminal classifier correctness — P1

examples/aws-lambda/template.yaml does not list PLAN_V2_INTEGRITY_UNRECOVERABLE or PlanV2IntegrityError in any of the three v2 Retry blocks:

• PlanV2 retry: examples/aws-lambda/template.yaml:297-306
• RenderChunkV2 retry: examples/aws-lambda/template.yaml:492-503
• AssembleV2 retry: examples/aws-lambda/template.yaml:533-543

Verified empirically: grep -n "PLAN_V2_INTEGRITY\|PlanV2IntegrityError" examples/aws-lambda/template.yaml returns nothing.

The CDK synth adds both entries (packages/aws-lambda/src/cdk/HyperframesRenderStack.ts:205-206,222-223,232-233) and the snapshot test now asserts terminality across every v2 task (HyperframesRenderStack.snapshot.test.ts:183-205). But PlanV2IntegrityError — thrown by readPlanV2Manifest and by all v2 manifest-integrity failures (packages/producer/src/services/distributed/planV2.ts:56-65) — will surface to Step Functions with errorType: "PlanV2IntegrityError". In the SAM-deployed stack that class name matches only the States.ALL catch: MaxAttempts=4, IntervalSeconds=2, BackoffRate=2, MaxDelaySeconds=60. Each attempt runs the full 15-minute Lambda budget on unrecoverable corruption. This is exactly the retry-storm the classifier is meant to prevent.

Both deployment surfaces are documented as first-class (packages/aws-lambda/README.md:13-16,135; examples/aws-lambda/README.md:22-33). The CDK/SAM drift was partly pre-existing, but this PR is the vehicle introducing PlanV2IntegrityError and closing the sibling gaps (it added PlanProtocolUnsupportedError + PLAN_ARTIFACT_DIGEST_MISMATCH to SAM), so the omission is scoped-fixable here. Ask: add both entries to all three v2 retry blocks (and to the v1 chunk/assemble blocks that co-list PLAN_ARTIFACT_DIGEST_MISMATCH, since a v1 → v2 manifest read is impossible today but the parity keeps the terminal contract symmetric).

(B) CDK snapshot coverage — pass

HyperframesRenderStack.snapshot.test.ts:53-63,140-179 locks state names, non-retryable errors across all four top-level tasks (Plan/PlanV2/Assemble/AssembleV2) plus both nested RenderChunk / RenderChunkV2, and the new "v1/v2 locators disjoint" invariant at lines 190-205. No timestamps or random IDs in expected names. IAM permission surface unchanged versus v1 (same S3CrudPolicy, template.yaml:161-163).

(C) Adapter fixture correctness — pass

makeMinimalV1PlanDir at handler.test.ts:754-776 writes a plausible v1 planDir, then recomputes the real hash via recomputePlanHashFromPlanDir and rewrites plan.json. The v2 fixture is built by createPlanV2FromV1 (handler.test.ts:526-531), not a fabricated string. Plan → chunk → assemble paths are all exercised; the chunk mock asserts audio is absent in the chunk-scoped materialization and the assemble mock asserts audio is present, verifying PlanV2MaterializationTarget scoping (handler.test.ts:534-560). Blob-URI derivation for the audio artifact recomputes sha256("AAC") and asserts it was downloaded during assemble but not during chunk (handler.test.ts:614-632).

(D) v1/v2 branching — pass

Discrimination is on the explicit PlanProtocol === "v2" string (handler.ts:265,461,559), not on field presence. The SAM SelectPlanProtocol Choice sends any unrecognized non-null PlanProtocol to a Fail state (template.yaml:210-229); the CDK mirrors that at HyperframesRenderStack.ts:469-475. The renderChunk V2 event interface deliberately cannot carry PlanS3Uri (events.ts:99-103), so a mistyped hybrid event fails at compile time.

(E) Observability — P3

handler_start includes planProtocol in the summarized event (handler.ts:216,222,231), but handler_error (handler.ts:129-134) does not — it emits {event, action, message, name}. On triage, v1-vs-v2 failure attribution requires a second CloudWatch join instead of a single log line.

Standards + Spec + Precision + Round-trip

(F) Standards — pass

No new as T narrowings or ! outside the checked values[index]! in mapConcurrent (handler.ts:673-687), which is safe under the while (cursor < values.length) guard.

(G) Spec forward + reverse — one finding (P2)

Forward: all three PR-body bullets land — SAM/CDK classifier updates, per-v2-task CDK snapshot (HyperframesRenderStack.snapshot.test.ts:183-205), and the hash-valid fixture (handler.test.ts:754-776). Reverse: PlanEvent / RenderChunkEvent / AssembleEvent are refactored from single interfaces to discriminated unions extending a private *EventBase (events.ts:47-145). The refactor is disclosed by the PR title but the shared PlanHash docstring at events.ts:72-78 is now v1-only ("verifies this against the untarred planDir's plan.json") while it lives on the shared base. For v2, PlanHash is verified against manifest.planHash in downloadAndMaterializePlanV2 (handler.ts:670-672), not against a planDir. Reword the base docstring.

(H) Precision — one finding (P2)

normalizeTerminalErrorName (handler.ts:143-150) maps only PLAN_PROTOCOL_UNSUPPORTED and PLAN_TOO_LARGE codes back to Error.name. PLAN_V2_INTEGRITY_UNRECOVERABLE is the same class-vs-code shape (planV2.ts:56-65) and is left out. Given the class name is on both retry lists, this is defense-in-depth today; but if any future producer throw path attaches the code without instantiating the class, v2 integrity errors regress silently to retryable while the sibling codes stay covered. Either normalize all three or drop the function and rely on class names alone.

Also handler.ts:640 hardcodes "audio.aac" while packages/producer/src/services/distributed/shared.ts:39 exports PLAN_AUDIO_RELATIVE_PATH — same magic string, minor drift risk.

(I) Middle-man wrap/unwrap — pass

downloadS3ObjectToFileVerified and uploadContentAddressedFileToS3 (s3Transport.ts:84-100,130-193) throw typed PLAN_ARTIFACT_DIGEST_MISMATCH errors with .name set for Step Functions. HeadObject → 404-caught → Put branch is control-flow-correct; digest-mismatch throws re-propagate through the catch (the guard is !isS3NotFound).

Test hygiene — P2

handler.test.ts:30 reaches into ../../producer/src/services/render/stages/freezePlan.js — the only cross-package deep import into producer internals in this package. Consider exporting recomputePlanHashFromPlanDir from @hyperframes/producer/distributed or adding a test-only surface, so the fixture can't silently break under an internal producer refactor.

Editor-UI (12 axes)

N/A — no editor UI, no React, no ARIA / keyboard / commit-semantics surface.

— Via

[james-russo-rames-d-jusso]

Reviewed at 5e12d686ebee47863f642b14fc0b971979241878 via /code-review max on the delta + full read of the 44 changed files.

One blocker in the SAM template, three drift-detection concerns, several nits. The CDK path is thoroughly wired and pinned — my R2 carry-forward (PlanV2IntegrityError + PLAN_V2_INTEGRITY_UNRECOVERABLE in NON_RETRYABLE_{PLAN,CHUNK,ASSEMBLE} on Step Functions with MaxAttempts: 0) lands cleanly at HyperframesRenderStack.ts:196-239 and is pinned by snapshot assertions at HyperframesRenderStack.snapshot.test.ts:76-77 + :202-204 per-task. The v2 integration test path is exercised at handler.test.ts:519-630. That side of the change is exactly what we asked for on #2788 R2.

The blocker is that the SAM template — which the packages/aws-lambda/README.md line 3 describes as the reference deployment path and which examples/aws-lambda/scripts/smoke.sh deploys — never got the same fix. See B1 inline.

Blockers

B1 — SAM template.yaml v2 tasks omit PLAN_V2_INTEGRITY_UNRECOVERABLE and PlanV2IntegrityError from their NON_RETRYABLE_* lists. See inline on the PlanV2 Retry.ErrorEquals block; the same drift repeats on RenderChunkV2 (template.yaml:488-498) and AssembleV2 (template.yaml:528-540). The CDK stack has both entries in all three lists; the SAM template has neither in any of the three. An adopter following the README's SAM path will see the exact retry-storm the PR title claims to fix on any deterministic v2 integrity failure (chunk digest mismatch, malformed manifest, tampered blob, etc.) — Step Functions will fall through to the States.ALL retry rule at 2s/4s/8s/16s backoff × 4 attempts with a 15-minute per-attempt Lambda timeout. Per-chunk retry budget burn on non-recoverable errors is ~60 minutes worst case. Same production impact as the pre-#2777 R2 gap on PlanProtocolUnsupportedError. Grep confirms: grep -c 'PLAN_V2_INTEGRITY_UNRECOVERABLE\|PlanV2IntegrityError' examples/aws-lambda/template.yaml returns 0.

Concerns

C1 — Docblock claim about SAM/CDK drift protection is false. HyperframesRenderStack.ts:15-19 says "Drift from the SAM template is guarded by the snapshot test (HyperframesRenderStack.snapshot.test.ts), which diffs the synthed CloudFormation against the SAM-rendered CloudFormation modulo normalisation." But HyperframesRenderStack.snapshot.test.ts never loads template.yaml, never runs cfn-lint, and only reads back what Template.fromStack synthesised. This is exactly how B1 slipped — the docblock convinced everyone (author, prior reviewers, me at R2 when I recommended this shape) that CDK + SAM parity was pinned. It isn't. Either add the actual SAM-diff test the docblock promises, or amend the docblock so it stops misleading reviewers.

C2 — NON_RETRYABLE_ASSEMBLE doesn't include BROWSER_GPU_NOT_SOFTWARE or FONT_FETCH_FAILED. Inspect HyperframesRenderStack.ts:226-239. Those errors are in NON_RETRYABLE_PLAN and NON_RETRYABLE_CHUNK because those states hit Chromium. Assemble does not, so the omission is probably correct. Worth a one-line comment at the constant declaration naming the reasoning — otherwise the next reviewer applying "add the new terminal code to all three lists" will have to re-derive the invariant.

C3 — normalizeTerminalErrorName at handler.ts:145-151 handles PLAN_PROTOCOL_UNSUPPORTED and PLAN_TOO_LARGE but not PLAN_V2_INTEGRITY_UNRECOVERABLE. Not a live bug because the producer sets error.name = "PlanV2IntegrityError" in the class constructor (packages/producer/src/services/distributed/planV2.ts:63) and that name is in the Step Functions non-retryable list — Step Functions matches on the class name. But a future refactor that throws new Error(...) with only .code = "PLAN_V2_INTEGRITY_UNRECOVERABLE" and default .name = "Error" would return Error to Step Functions, miss the classifier, retry-storm. Cheap defensive fix: add || candidate.code === "PLAN_V2_INTEGRITY_UNRECOVERABLE" to the conditional. Same class as C1 — the design implicit-dependency is exactly the kind of thing that decays over time without a test guarding it.

C4 — HyperframesRenderStack.snapshot.test.ts snapshot pinning is one-directional and misses S3_URI_NOT_ALLOWED. EXPECTED_NON_RETRYABLE_ERRORS at lines 67-81 is a Set iterated for present: true checks only; it never asserts the collected set has no extras. Two consequences: (a) the CDK lists include "S3_URI_NOT_ALLOWED" (HyperframesRenderStack.ts:199, 215, 229) but the test set doesn't, so a regression that removes it from the CDK lists would slip through. (b) A future PR removing an entire MaxAttempts: 0 retry block would pass this test. The v2 additions themselves are pinned tightly at :76-77, 202-204, so the PR-specific change is guarded — but the general "changing the list breaks the snapshot" property the CDK docblock claims doesn't quite hold.

C5 — No unit test covers the PR's headline invariant. packages/aws-lambda/src/handler.test.ts has zero references to PlanV2IntegrityError or PLAN_V2_INTEGRITY_UNRECOVERABLE. The normalizes producer terminal codes test at handler.test.ts:244-278 covers PLAN_TOO_LARGE only. There's a happy-path v2 e2e test at :519-630 but no failure-path test that a thrown PlanV2IntegrityError from the wrapped primitive surfaces the right .name/.code to Step Functions. The CDK snapshot pins the list; a producer-side name-collision test would pin the throw path.

C6 — Smoke examples/aws-lambda/scripts/smoke.sh:522-660 covers the happy v1↔v2 semantic-parity path only. No negative-path test that intentionally corrupts a plan-v2 manifest or blob to verify the retry classifier terminates instead of retry-storming. The failure branch at line 577-586 just dumps history and exits with code 4 — so if the SAM classifier drift (B1) is ever fixed via the smoke path, it'll get discovered by an adopter, not by a smoke run. Also: --plan-protocol both (line 632) is the only branch that runs v1↔v2 comparison; the default PLAN_PROTOCOL=v1 (line 85) skips both v2 execution AND the comparison.

Nits

• N1 — AssembleV2Event.AudioS3Uri: string | null at events.ts:138-143 is dead. handleAssembleV2 (handler.ts:627) reads audio from the materialised planDir, ignoring event.AudioS3Uri. HyperframesRenderStack.ts:435 hardwires AudioS3Uri: null. If a hand-built event passes a real URI it's silently dropped. Either remove the field or add a runtime throw when non-null.
• N2 — handler.ts:723 values[index]! non-null assertion. while (cursor < values.length) narrows it in practice; the assertion is safe but reads like an accident. const value = values[index]; if (value === undefined) continue; is a two-line rewrite that satisfies CONTRIBUTING.md:55.
• N3 — s3Transport.ts:162-176: HeadObjectCommand with ChecksumMode: "ENABLED" returns existing.ChecksumSHA256 but the reuse path never compares it against the expected sha256. If bucket policy weren't BlockPublic*+scoped IAM, a supply-chain attacker could rewrite the object plus its x-meta-sha256 metadata and the reuse would trust it. Not exploitable today; adding the base64→hex compare would be defense-in-depth.

Verified clean

• Retry classifier CDK wire-up + snapshot pin (see above)
• v2 dispatch on event.PlanProtocol === "v2" at handler.ts:265-267, 420-422, 560-562 with Extract<..., {PlanProtocol: "v2"}> narrowing
• v2 sample events match discriminated types — plan-v2.json/render-chunk-v2.json/assemble-v2.json
• SHA256 verify-before-use on both download (s3Transport.ts:85-102) and upload (s3Transport.ts:140-190), typed PLAN_ARTIFACT_DIGEST_MISMATCH on mismatch
• Digest format enforced — assertSha256 at :200-206 (64 hex chars); blob keys never traverse user input
• getEventS3Uris allowlist covers v2 locators (handler.ts:768-786) → validateEventS3Uris throws typed S3_URI_NOT_ALLOWED before any S3 call
• v2 chunk skips audio — handler.test.ts:611 verifies chunk did NOT download audio.aac; assemble did
• IAM narrowness — RenderStateMachineRole (template.yaml:549-590) is single-Lambda-invoke-scoped, no wildcards
• S3 bucket protections — BlockPublic* all true, lifecycle 7-day expiry, DeletionPolicy: Retain
• any grep — zero in the added source (only match is inside a JSDoc string)
• No bare as T at JSON.parse boundaries in the added handler code (handler.ts:844 uses as { planHash?: unknown } immediately narrowed via typeof, which is compliant)

CI: required checks all green; regression shards failed as an infra flake — shard-2 hit Error response from daemon: Get "https://registry-1.docker.io/v2/": ... Client.Timeout on Set up Docker Buildx, fail-fast cascaded to shards 1/3/4/5/6/7/8 (cancelled) then the regression aggregate. Not a code regression; author should retry the shard job.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🔴 BLOCKER — SAM template PlanV2 task's non-retryable list omits the v2 integrity errors.

The list here (lines 297-309) has FFMPEG_VERSION_MISMATCH, PLAN_HASH_MISMATCH, S3_URI_NOT_ALLOWED, BROWSER_GPU_NOT_SOFTWARE, FONT_FETCH_FAILED, PLAN_TOO_LARGE, PlanTooLargeError, PLAN_PROTOCOL_UNSUPPORTED, PlanProtocolUnsupportedError, PLAN_ARTIFACT_DIGEST_MISMATCH, FORMAT_NOT_SUPPORTED_IN_DISTRIBUTED, ChromeBinaryUnavailableError — but not PLAN_V2_INTEGRITY_UNRECOVERABLE and not PlanV2IntegrityError.

The CDK path (HyperframesRenderStack.ts:196-211) has both. This SAM path silently omits both, which means a deterministic v2 integrity failure (chunk digest mismatch, malformed manifest, tampered blob) will fall through to the States.ALL retry at 2s/4s/8s/16s × 4 attempts with 15-minute Lambda timeouts each — the exact retry-storm this PR is titled to fix. Adopters on the SAM path (which packages/aws-lambda/README.md line 3 documents as the reference and which examples/aws-lambda/scripts/smoke.sh deploys) ship the unfixed classifier.

Same drift repeats on RenderChunkV2.Retry.ErrorEquals (template.yaml:488-498) and AssembleV2.Retry.ErrorEquals (template.yaml:528-540). Add both entries in all three v2 task lists.

Fix suggestion for this block:

- ErrorEquals:
    - FFMPEG_VERSION_MISMATCH
    - PLAN_HASH_MISMATCH
    - S3_URI_NOT_ALLOWED
    - BROWSER_GPU_NOT_SOFTWARE
    - FONT_FETCH_FAILED
    - PLAN_TOO_LARGE
    - PlanTooLargeError
    - PLAN_PROTOCOL_UNSUPPORTED
    - PlanProtocolUnsupportedError
    - PLAN_V2_INTEGRITY_UNRECOVERABLE
    - PlanV2IntegrityError
    - PLAN_ARTIFACT_DIGEST_MISMATCH
    - FORMAT_NOT_SUPPORTED_IN_DISTRIBUTED
    - ChromeBinaryUnavailableError
  MaxAttempts: 0

To prevent this class of drift from recurring, wire the actual SAM/CDK diff test the HyperframesRenderStack.ts:15-19 docblock claims exists — see C1 in the review body.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 DEFENSIVE — normalizeTerminalErrorName should include PLAN_V2_INTEGRITY_UNRECOVERABLE.

Line 148 currently handles PLAN_PROTOCOL_UNSUPPORTED and PLAN_TOO_LARGE — both codes that the producer either sometimes throws with only a .code (no matching .name), or that need name-normalization for adapter compatibility.

PlanV2IntegrityError (producer/src/services/distributed/planV2.ts:56-64) currently sets error.name = "PlanV2IntegrityError" in its constructor AND carries .code = "PLAN_V2_INTEGRITY_UNRECOVERABLE", so it's redundant at HEAD — Step Functions matches on .name. But the pattern established by this function is to catch code-only throws.

Failure scenario: a future producer refactor throws new Error("...") with only .code = "PLAN_V2_INTEGRITY_UNRECOVERABLE" (default .name = "Error"). Step Functions receives Error as the error name → not in NON_RETRYABLE_* lists → 4-attempt retry-storm on a deterministic failure. Same implicit-dependency shape as the PLAN_TOO_LARGE case this function already normalizes.

Cheap fix — one-line change:

if (
  candidate.code === "PLAN_PROTOCOL_UNSUPPORTED" ||
  candidate.code === "PLAN_TOO_LARGE" ||
  candidate.code === "PLAN_V2_INTEGRITY_UNRECOVERABLE"
) {
  candidate.name = candidate.code;
}

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

LGTM at fd233f839. All R1 items I flagged are addressed cleanly.

R1 → R2 verification

• ✅ B1 (SAM v2 error drift) — examples/aws-lambda/template.yaml adds PLAN_V2_INTEGRITY_UNRECOVERABLE and PlanV2IntegrityError to PlanV2 (L306-307), RenderChunkV2 (L498-499), and AssembleV2 (L541-542). Exact parity with the CDK NON_RETRYABLE_{PLAN,CHUNK,ASSEMBLE} lists at HyperframesRenderStack.ts:196-239.
• ✅ C1 (docblock-claim vs. reality) — the new it("keeps SAM and CDK terminal classifiers identical for every v2 Lambda task") test at HyperframesRenderStack.snapshot.test.ts:196-208 parses the actual template.yaml via readFileSync(new URL("../../../../examples/aws-lambda/template.yaml", ...)) + parseYaml({ logLevel: "silent" }) (rationale on the !Ref/!GetAtt handling is spelled out in the surrounding comment), extracts the three v2 tasks via a shared getV2TaskStates helper, and asserts sorted-set equality on the errors surfaced by collectNonRetryableErrors (which filters on MaxAttempts === 0). The docblock at HyperframesRenderStack.ts:15-19 is now retroactively true — this is the real guard I was asking for.
• ✅ C3 (mirror bug on normalizeTerminalErrorName) — handler.ts:148-152 now covers PLAN_V2_INTEGRITY_UNRECOVERABLE alongside the two prior codes, and handler.test.ts:242-286 loops all three terminal codes asserting rejects.toMatchObject({ name: code }). The GCP mirror (server.ts:184) landing together in #2790 keeps the two adapter classifiers symmetric.
• ✅ Cleanup wins — events.ts:72-79 PlanHash docs correctly distinguish v1 (untarred plan.json) vs. v2 (content-addressed manifest) verification paths; input: summarizeEvent(unwrapped) addition to the handler_error log emits the action-specific S3 URIs + protocol + structural fields per action variant without leaking secrets — solid triage aid.

Delta scope check

Real changes since my R1 at 5e12d686e: ~140 lines across 8 files (the planV2.ts +2 is the merged #2788 CodeQL suppression). The yaml: ^2.9.0 devDep addition is the minimum surface needed for SAM parsing.

Owning an R1 miss

My R1 concern about AudioS3Uri being "dead" was wrong — it's plumbed end-to-end (HyperframesRenderStack.ts:256,361,435 in the state machine, handler.ts:583-585 in the assemble path, and now included in summarizeEvent at handler.ts:243). Sorry for the noise on that one.

Nothing left blocking on my side. The remaining R1 smoke-happy-path-only observation is worth revisiting in a follow-up but doesn't gate this PR — the new parity test + handler-normalizer coverage is a stronger guard than smoke would have provided.

[miguel-heygen]

Exact-head re-review at fd233f83980e2501a390a5b0f7b0d93983960dba.

The prior blocker is resolved. The SAM PlanV2, RenderChunkV2, and AssembleV2 tasks now classify both PLAN_V2_INTEGRITY_UNRECOVERABLE and PlanV2IntegrityError as terminal (examples/aws-lambda/template.yaml:296-312,489-503,532-546). More importantly, HyperframesRenderStack.snapshot.test.ts:196-209 parses the real SAM template and asserts exact per-task classifier parity with the CDK definition, closing the drift seam rather than pinning only one deployment surface.

The related follow-ups are also addressed: code-only v2 integrity failures normalize before Step Functions observes them (packages/aws-lambda/src/handler.ts:139-155) with all three terminal codes exercised, handler_error now carries the summarized protocol/input (packages/aws-lambda/src/handler.ts:122-135), and the shared PlanHash contract correctly documents both v1 and v2 verification (packages/aws-lambda/src/events.ts:69-79).

Audited: the full fix commit delta, including SAM/CDK classifier parity, handler normalization and observability, event docs, and the smoke-run reservation/ownership cleanup changes.

Trusting: the previously reviewed large parity harness and unchanged AWS v2 transport implementation beyond the integration points rechecked in this round.

Verification: AWS package tests passed 124/124; AWS typecheck, focused formatting, AWS isolation shell test, and git diff --check passed. Required exact-head CI is still running with no required failure, so merge should continue to wait for the required checks to finish green.

Verdict: APPROVE
Reasoning: The SAM retry-storm blocker is fixed on all three v2 tasks and is now guarded by an exact SAM/CDK parity test; the associated normalization, observability, and contract-documentation gaps are also closed.

— Magi

[vanceingalls]

APPROVE — prior R1 P1 fully resolved; parity test is strong; residual items are P2/P3 non-blockers.

Verification of prior R1 findings

• (A) SAM v2 terminal classifier gap — RESOLVED. All six citations land at head:
  • PlanV2 retry: examples/aws-lambda/template.yaml:307 (PLAN_V2_INTEGRITY_UNRECOVERABLE), :308 (PlanV2IntegrityError).
  • RenderChunkV2 retry: examples/aws-lambda/template.yaml:499, :500.
  • AssembleV2 retry: examples/aws-lambda/template.yaml:542, :543.
    Each sits inside a MaxAttempts: 0 block ahead of the States.ALL catch, so Step Functions will terminally fail on either name in every v2 task.
• (B) CDK snapshot coverage — still pass. packages/aws-lambda/src/cdk/HyperframesRenderStack.snapshot.test.ts:68-83,150-194 pins every terminal name and asserts terminality for each v2 task.
• (C) Adapter fixture correctness — still pass.
• (D) v1/v2 branching — still pass. SelectPlanProtocol at examples/aws-lambda/template.yaml:216-234 + CDK mirror; RenderChunkV2Event still cannot carry PlanS3Uri (packages/aws-lambda/src/events.ts:99-103).
• (E) handler_error missing planProtocol — RESOLVED. packages/aws-lambda/src/handler.ts:128-134 now emits input: summarizeEvent(unwrapped), and summarizeEvent inlines planProtocol on every branch at handler.ts:223,229,238.
• (G) Shared PlanHash docstring v1-only — RESOLVED. Base docstring at packages/aws-lambda/src/events.ts:72-78 now names both verification paths ("For v1, the handler verifies it against the untarred planDir's plan.json; for v2, it verifies it against the content-addressed manifest").
• (H1) normalizeTerminalErrorName missing PLAN_V2_INTEGRITY_UNRECOVERABLE — RESOLVED. packages/aws-lambda/src/handler.ts:146-156 now maps all three codes (PLAN_PROTOCOL_UNSUPPORTED, PLAN_TOO_LARGE, PLAN_V2_INTEGRITY_UNRECOVERABLE) back to .name. Test at packages/aws-lambda/src/handler.test.ts:244-284 iterates the same three-tuple and asserts .rejects.toMatchObject({ name: code }).
• (H2) Hardcoded "audio.aac" — STILL PRESENT at packages/aws-lambda/src/handler.ts:313,315,320,408,584,632. Not blocking; producer's PLAN_AUDIO_RELATIVE_PATH remains the canonical constant. P3 nit carry-forward.
• Test hygiene P2 — STILL PRESENT. packages/aws-lambda/src/handler.test.ts:30 still deep-imports ../../producer/src/services/render/stages/freezePlan.js. Non-blocking; would benefit from a producer public re-export.

Claim-by-claim verification

Claim 1: SAM parity

All six citations verified at head (see (A) above). v2 lists across the three states are identical modulo intentional per-task differences (FONT_FETCH_FAILED only on PlanV2, FORMAT_NOT_SUPPORTED_IN_DISTRIBUTED on PlanV2 + AssembleV2), matching CDK's NON_RETRYABLE_{PLAN,CHUNK,ASSEMBLE} at packages/aws-lambda/src/cdk/HyperframesRenderStack.ts:196-239.

Claim 2: parity test

packages/aws-lambda/src/cdk/HyperframesRenderStack.snapshot.test.ts:196-210 — strong shape, not subset-only.

• Reads the real SAM template from disk via readSamDefinition() at :274-303 (uses readFileSync on ../../../../examples/aws-lambda/template.yaml, parses YAML with logLevel: "silent" to tolerate CFN intrinsic tags).
• Extracts CDK v2 states from the synthed definition and SAM v2 states from the parsed YAML via getV2TaskStates at :259-272.
• Assertion at :205-208: expect({ taskName, errors: [...samErrors].sort() }).toEqual({ taskName, errors: [...cdkErrors].sort() }) — set-equal via sorted-array toEqual, so an extra code on either side fails the test. Not "SAM includes CDK codes" subset semantics.
• Iterates all three v2 tasks (PlanV2, RenderChunkV2, AssembleV2).
• collectNonRetryableErrors at :232-240 only harvests Retry entries with MaxAttempts === 0, so terminal semantics (not just "in some retry block") are what's compared.

Coverage caveat: the test does not cover v1 tasks — see the follow-up under "Spec reverse-check" below.

Claim 3: normalization ordering

• Catch happens inside the Lambda handler at packages/aws-lambda/src/handler.ts:103-136: normalizeTerminalErrorName(err) runs at :123 before the throw err at :135, so Step Functions only ever sees the normalized .name.
• All three terminal codes covered at :149-155: PLAN_PROTOCOL_UNSUPPORTED, PLAN_TOO_LARGE, PLAN_V2_INTEGRITY_UNRECOVERABLE. If any producer code arrives as code without a matching class instance, the .name is stamped to match Step Functions' ErrorEquals.
• Test coverage at packages/aws-lambda/src/handler.test.ts:244-284 iterates the same three codes and asserts the normalized .name propagates.
• The SAM/CDK terminal lists already contain both the code form and the class-name form for PlanProtocolUnsupportedError, PlanTooLargeError, PlanV2IntegrityError, so normalization is defense-in-depth rather than the primary contract.

Claim 4: handler_error payload

• Emitted at packages/aws-lambda/src/handler.ts:128-134. Fields: event, action, input: summarizeEvent(unwrapped), message, name.
• summarizeEvent at :215-248 emits only routable fields — S3 URIs, chunk index, format, fps, planProtocol, chunk count, hasAudio, outputS3Uri. Deliberately omits Config payload (which includes the composition JSON). No raw project content, no credentials.
• planProtocol is present on every branch at :223,229,238.
• One PII adjacency worth calling out below (bucket/key in S3 URIs); it's the same disclosure surface as handler_start and was pre-existing.

Claim 5: PlanHash contract docs

• Shared docstring at packages/aws-lambda/src/events.ts:72-78 distinguishes v1 (plan.json recomputation) from v2 (manifest.planHash cross-check). Matches the runtime paths at handler.ts:458 (v1: verifyPlanHash(planDir, ...)) and handler.ts:367-369,673-676 (v2: cross-check against readPlanV2Manifest).
• AssembleV2Event at events.ts:138-143 also carries PlanHash, matching the v2 assemble path's throwPlanHashMismatch at handler.ts:711-717.
• The v1↔v2 hash construction contract (frame count + video refs vs. dependency graph + video refs, bidirectional canonicalization) lives in the producer package and is not in this PR's scope; the diff correctly documents only the AWS adapter's verification contract, which is what the PR touches.

Fresh 4-lens pass at fd233f8

Standards

• No new as T narrowings at JSON boundaries. values[index]! in mapConcurrent (handler.ts:719-732) is safe under the while (cursor < values.length) guard; unchanged from R1.
• fallow-ignore-next-line complexity at handler.ts:145,213,346,491,619 is used on functions that are legitimately branchy transactional lifecycles (v2 plan/chunk/assemble handlers, summarizeEvent, normalizeTerminalErrorName) — each has a one-line justification comment above. Not gratuitous.

Spec forward-check

Every PR-body bullet lands:

• "explicit PlanProtocol: 'v1' | 'v2' support" → events.ts:29,58-67,89-105,131-145; SAM SelectPlanProtocol Choice at template.yaml:216-234; CDK mirror at HyperframesRenderStack.ts:196-239,292,402,446.
• "manifest + immutable content-addressed S3 artifacts" → handler.ts:372-394,703-708; s3Transport.ts:85-107 verified download.
• "materialize only chunk-targeted / assembler-targeted artifacts" → handler.ts:504-509,629,660-687 via listPlanV2ArtifactsForTarget.
• "smoke resources collision-resistant, fail closed, purge versioned buckets, discover retained resources" → examples/aws-lambda/scripts/_aws-isolation.sh, _s3-purge.sh, _semantic-compare.sh, smoke.sh; each has a paired .test.sh at scripts/*.test.sh.

Spec reverse-check

• In-scope but undisclosed by the PR summary: the parity test also asserts only v2 SAM↔CDK equality (HyperframesRenderStack.snapshot.test.ts:196-210). SAM's v1 Plan/RenderChunk/Assemble retry lists at template.yaml:252-273,395-410,431-451 were widened by this PR (added PlanTooLargeError, PLAN_PROTOCOL_UNSUPPORTED, PlanProtocolUnsupportedError, PLAN_ARTIFACT_DIGEST_MISMATCH, FORMAT_NOT_SUPPORTED_IN_DISTRIBUTED) but still diverge from CDK's shared NON_RETRYABLE_PLAN in reachable ways: SAM v1 Plan omits S3_URI_NOT_ALLOWED (reachable via validateEventS3Uris at handler.ts:97, which runs before any dispatch) and ChromeBinaryUnavailableError (reachable via resolveChromeExecutablePath at handler.ts:283, unconditional in v1 Plan). A v1 render on SAM that throws either name will retry-storm 4× while CDK terminally fails on the first attempt. Not a peer-block since Miguel didn't converge on it and my prior R1 flagged the v1 chunk/assemble parity only parenthetically — filing as P2 follow-up rather than a re-block, because the fix is a small list extension and belongs in the same file already being edited. Suggested: also extend keeps SAM and CDK terminal classifiers identical to iterate Plan, RenderChunk (nested), Assemble in addition to the v2 trio.
• Refactor of PlanEvent/RenderChunkEvent/AssembleEvent into discriminated unions (events.ts:47-145) is disclosed by the PR title and R1 already covered it.

Sibling-precision divergence

• Concurrency = 16 in two places: handler.ts:376 (v2 plan artifact upload) and handler.ts:681 (v2 chunk/assemble artifact download). Different physical operations (PUT vs. GET) but same magic number. Not a bug, but a small drift risk if one is retuned. P3 nit.
• Retry constants { IntervalSeconds: 2, MaxAttempts: 4, BackoffRate: 2, MaxDelaySeconds: 60 } repeat in every SAM States.ALL catch and in CDK plan.addRetry. The parity test only compares MaxAttempts: 0 lists — a divergence in the transient-retry constants would not be caught. Very low priority; existing pattern.

Middle-man wrap/unwrap

• downloadAndMaterializePlanV2 (handler.ts:660-687) takes a { PlanV2ManifestS3Uri, PlanV2ArtifactS3Prefix, PlanHash } structural type. Callers pass the raw event object (handler.ts:504-509,629). The parameter shape is a structural narrowing of both RenderChunkV2Event and AssembleV2Event, not an object literal built at the call site — correct middle-man discipline.
• throwPlanHashMismatch(expected, actual) (handler.ts:711-717) — pure primitive params; no wrap/unwrap concern.

Behavior + editor-UI lens complement

Silent-catch + error-invariant

• Handler catch (handler.ts:122-136) re-throws after normalization + structured logging. No swallow.
• readPlanV2Manifest is called at handler.ts:366 (planV2 publish) and :673 (chunk/assemble materialize). Its PlanV2IntegrityError propagates directly through the handler's try/catch to Step Functions with the class name intact; the code-only path is covered by normalizeTerminalErrorName.
• Retryable-vs-terminal classification stays consistent: Cloud Run remains on v1 per PR body; only Lambda's SAM + CDK carry v2 semantics. No cross-runtime drift in this PR.

Concurrency/lifecycle

• Plan-v2 publish ordering (handler.ts:373-394) is transactionally correct: all unique artifacts CAS-upload first, then manifest publishes last. Matches the "manifest-last" invariant the CDK snapshot's terminal classifier depends on.
• mapConcurrent (handler.ts:719-732) uses a shared cursor counter; simple work-stealing loop. Bounded by values.length.
• IAM surface: SAM keeps S3CrudPolicy on the bucket only (template.yaml:161-163). CDK's snapshot pins AWS::IAM::Role: 2, AWS::IAM::Policy: 2 (HyperframesRenderStack.snapshot.test.ts:46-47). No new privilege in this PR.

PR-body-vs-diff parity

• PR body: "SAM template" gets the same v2 classifier fix as CDK. Verified above (six citations).
• PR body: "121 AWS tests" — file diff shows the test count grew via handler.test.ts (+212), s3Transport.test.ts (+116), HyperframesRenderStack.snapshot.test.ts (+133), and SDK tests. Consistent.
• PR body: "handler ZIP size gate: 122.0 MiB compressed / 236.4 MiB uncompressed" — untestable at this layer, trusting the CI gate.

Perf audit

• No O(n²) template synth. Parity test dedupes to sets before comparing.
• MAX_ENVELOPE_DEPTH = 4 guards against infinite unwrap loops (handler.ts:165-189).
• Uniqueness-map for CAS uploads (handler.ts:373-375,678-680) is O(n) and avoids re-uploading the same digest.
• No cold-start regression: module-scoped cachedS3Client (handler.ts:62-67) preserves keep-alive across warm invocations.

Verdict

APPROVE — the R1 P1 (SAM v2 terminal classifier omission) is fully resolved at six exact citations, and the follow-on parity test at HyperframesRenderStack.snapshot.test.ts:196-210 is genuinely set-equal (both directions, all three v2 states, real disk-parsed SAM YAML) so the drift is now guarded rather than one-shot patched. Normalization, handler_error observability, and the shared PlanHash docstring are also closed. Non-blocking follow-ups: (1) extend the SAM/CDK parity test to v1 tasks — SAM v1 Plan still omits S3_URI_NOT_ALLOWED and ChromeBinaryUnavailableError that the CDK v1 path terminally fails on; (2) "audio.aac" remains hardcoded; (3) cross-package deep import at handler.test.ts:30 remains.

— Via