refactor(producer): add remote-ready plan v2 publisher

[jrusso1020]

What

Introduces a storage-neutral, manifest-last PlanV2ArtifactPublisher contract
and a public planV2WithPublisher() entry point for S3, GCS, Temporal, and other
adapters. putBlob() receives a planner-local source path whose bytes must be
durable before its promise resolves; commitManifest() runs only after all
bounded-concurrency blob publications complete.

The included LocalPlanV2ArtifactPublisher is explicitly a planner-local
compatibility implementation. It hard-links immutable blobs when staging and
destination share a filesystem and atomically copies otherwise. Distributed
nodes never receive or depend on those paths.

Why

AWS, GCP, and the internal Temporal system exchange artifacts across nodes
through S3/GCS, not a shared filesystem. The producer therefore needs a
storage-neutral seam that cloud adapters can implement directly while
preserving manifest-last publication.

This PR establishes that seam and avoids a second set of local data blocks for
local callers. It does not remove the complete planner-local v1 staging tree
yet, so oversized-plan traffic remains disabled until the direct S3/GCS
publishers and direct-emission planner work land.

How

• separates plan catalog/manifest construction from storage publication
• publicly exports planV2WithPublisher(), publishPlanV2FromV1(), and adapter types
• publishes unique digests with bounded concurrency before committing plan.json
• validates digest paths, source sizes, and referenced blob durability
• uses typed integrity errors and collision-safe temporary directories
• preserves the synchronous local compatibility helper and byte-identical output
• aborts partial publication without exposing a manifest

Distributed-system contract

• planner-local paths exist only during one planner invocation
• adapters persist immutable blobs to S3/GCS before resolving putBlob()
• Temporal/Step Functions/Cloud Workflows pass only manifest URI, CAS prefix,
  manifest hash, chunk index, and output locators
• chunk and assemble workers independently download, verify, and materialize
  their role-scoped artifacts

Test plan

• Focused Plan V2 / size-cap / public-export tests: 39 passed
• Producer typecheck
• Changed-file oxlint and oxfmt
• Fallow, tracked-artifact, and pre-commit gates
• Remote in-memory publisher proves no shared destination filesystem
• Existing and publisher paths produce byte-identical manifests/blobs
• Hard-link fallback, digest traversal rejection, and incomplete commit tests
• Injected publication failure proves abort and no manifest exposure

[jrusso1020]

• refactor(producer): add remote-ready plan v2 publisher #2792 👈 (View in Graphite)
• 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
• main

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

[vanceingalls]

Verdict: APPROVE — the manifest-last publisher seam is well-shaped and the restack scope is clean. One P2 (path-layout duplication) and two P3s (fallback path untested; sync-in-async precedent for future S3/GCS impls) worth noting non-blocking.

Scope check (restack claim)

Diff is 3 files, all under packages/producer/src/services/distributed/: planV2.ts (+103/-51), planV2Publisher.ts (+86 new), planV2.test.ts (+49/0). Base is feat/plan-protocol-v2-gcp (b6664db), the correct parent per the 2788 → 2789 → 2790 → 2792 stack. No accidental drag-ins from lower-stack PRs — the diff surfaces only publisher extraction and dedup, exactly matching the PR body.

Nit on the framing: "direct publisher" here is not a message-broker publisher — it's an artifact publisher (manifest-last blob durability seam). The "direct emission" motivation (writing blobs directly to the eventual storage instead of copying into a second local CAS tree) is spelled out in the body but the branch-name framing could mislead a reviewer to look for Kafka/SQS semantics that aren't there.

Findings

P2 — CAS path layout duplicated across publisher and legacy helper. packages/producer/src/services/distributed/planV2Publisher.ts:57-63 hardcodes join(this.temporaryDir, "artifacts", "sha256", blob.sha256.slice(0, 2), blob.sha256). The private blobPath(planV2Dir, sha256) helper at planV2.ts:196-198 already encodes the same "artifacts"/"sha256"/prefix/full layout, and materializePlanV2Target reads via that same helper at planV2.ts:800. If the CAS layout ever changes (deeper sharding, alt hash) the two sites will drift silently — publisher writes to one path, materialization reads from another, and manifests are ostensibly valid but blobs are unfindable. This becomes load-bearing once S3/GCS publishers land in later stack PRs and each independently rebuilds the layout. Suggest either exporting blobPath from planV2.ts and reusing it (passing this.temporaryDir as the root), or extracting the layout to a small shared planV2Layout.ts module. Not a blocker for this PR since both sides are locally consistent today.

P3 — copy-fallback branch has no test coverage. planV2Publisher.ts:67-73 implements the EXDEV/EPERM/EACCES/ENOTSUP fallback to copyFileSync. The test suite (planV2.test.ts:402-448) asserts the same-inode hard-link path and the abort-on-blob-failure path — but nothing forces the fallback branch. Given the PR body specifically calls out cross-device correctness as a supported property, a mock publisher (or an fs shim that throws EXDEV on linkSync) would lock the fallback in place. ~5-10 lines.

P3 — sync fs inside async methods sets a precedent. putBlob and commitManifest are declared async but exclusively call synchronous fs (linkSync, copyFileSync, renameSync, writeFileSync). Fine for the local CAS today, but every future implementer of PlanV2ArtifactPublisher (S3, GCS) will be genuinely async, and mixing paradigms across implementations tends to hide back-pressure surprises. Non-blocking; flagging for the stack.

Non-findings (verified)

• Manifest-last invariant. Blobs stage into temporaryDir with per-blob atomic rename; plan.json writes into the same temp tree; final renameSync(temporaryDir, destinationDir) in commitManifest publishes atomically at the directory level. External observers never see a manifest without its blobs. ✓
• Dedup. blobs.set(sha256, …) in buildPlanV2Publication (planV2.ts:502-504) dedups per-digest, and putBlob short-circuits on existsSync(destinationPath) (planV2Publisher.ts:64). Matches "duplicate digests publish once". ✓
• Abort semantics. abort() is guarded by #committed (planV2Publisher.ts:82-86); double-abort is safe (force: true); the outer publishPlanV2FromV1 catches, aborts, and rethrows without swallowing the original error (planV2.ts:571-580). ✓
• Compat helper. createPlanV2FromV1 retains its copyFileSync-only path via writeBlob; not called by planV2() anymore but still exported for existing consumers. ✓
• Fallback code narrowing. canFallbackToCopy requires error instanceof Error and an actual code property before matching. ENOENT deliberately not in the list — a missing source rightly throws. ✓
• Constructor TOCTOU. existsSync(destinationDir) at construction can race a later renameSync in commitManifest; the failure surfaces as ENOTEMPTY/EEXIST and cleanup runs. Acceptable per the "abort partial publication without exposing a manifest" contract.

— Via

[miguel-heygen]

Additive fresh exact-head review at d90bc43f5216df8674b31f7fc7053f780adc27a7. @vanceingalls already covered the non-blocking duplicated CAS-layout helper and copy-fallback/sync-I/O test notes.

I audited all three changed files plus the inherited AWS/GCP consumer contracts. The manifest-last ordering, SHA deduplication, abort path, hard-link lifecycle, and synchronous compatibility helper are coherent (packages/producer/src/services/distributed/planV2.ts:450-608, packages/producer/src/services/distributed/planV2Publisher.ts:41-85). The local publisher makes the complete tree visible with one final directory rename, and staging cleanup leaves the destination links valid.

Two non-blocking standards nits:

• packages/producer/src/services/distributed/planV2Publisher.ts:28-31 adds a bare as Error & { code?: unknown } assertion without the justification required by CONTRIBUTING.md; a small record/type guard would remove it.
• packages/producer/src/services/distributed/planV2.test.ts:412-414 uses an unchecked non-null assertion on .find(...), also outside the checked-path allowance in CONTRIBUTING.md. An explicit guard would make a missing fixture artifact fail diagnostically.

Verification: producer typecheck passed; plan-v2, frame-rebuild, and plan-size tests passed 30/30; git diff --check passed. The exact-head regression shards and Graphite mergeability check are still running, with no current code-test failure.

Verdict: APPROVE
Reasoning: The publisher extraction preserves manifest/hash/materialization behavior and establishes an atomic manifest-last seam; the remaining findings are standards-only test/narrowing nits.

— Magi

[james-russo-rames-d-jusso]

Reviewed at d90bc43f5216df8674b31f7fc7053f780adc27a7 via /code-review max on the 3-file diff (86 lines new publisher, 103/51 refactor on planV2.ts, +49 test coverage).

Refactor is on-disk behavior-preserving — createPlanV2FromV1 and publishPlanV2FromV1 both funnel through buildPlanV2Publication, so the manifest/artifact layout downstream #2789/#2790 adapters read is byte-for-byte identical. Hardlink-then-fallback-copy in LocalPlanV2ArtifactPublisher.putBlob is nice — staging dir + destination temp dir are same-filesystem (both under dirname(planV2Dir) per planV2.ts:596 and planV2Publisher.ts:52), so linkSync succeeds and rmSync(stagingDir) after commit doesn't unlink the destination blobs (hardlink keeps the inode alive).

Two design concerns worth landing before or alongside the future adapter-side wire-up (which is when this publisher becomes externally callable and its contract starts mattering for retry classification / path safety). Neither is a live bug at this PR, but the shape you're locking in here becomes hard to change once #2789/#2790 adapters call it.

Concerns

C1 — Publisher throws bare Error where the rest of planV2.ts throws PlanV2IntegrityError. See inline on planV2Publisher.ts:48. When an adapter eventually wires this publisher to S3/GCS uploads, the NON_RETRYABLE_* lists on #2789/#2790 classify by class name (PlanV2IntegrityError) and code (PLAN_V2_INTEGRITY_UNRECOVERABLE); a bare Error("output directory already exists") won't be classified → adapter retries a deterministic failure. Same class as the SAM template blocker I flagged on #2789 — a typed-error surface that's partially wired is worse than not wired, because reviewers assume it holds.

C2 — Two divergent local-publish code paths for the same on-disk contract. createPlanV2FromV1 still uses writeBlob (planV2.ts:437-452, copyFileSync via per-blob mkdtempSync). planV2() uses LocalPlanV2ArtifactPublisher (linkSync w/ copy fallback). Both must produce byte-identical output for the manifest to hash the same — currently they do (both funnel through buildPlanV2Publication), but any future hardening (re-verify sha256 after copy, add fsync, adjust atomicity guarantees) has to be applied twice. No shared unit test asserts createPlanV2FromV1(v1, X) and publishPlanV2FromV1(v1, new LocalPlanV2ArtifactPublisher(Y)) produce byte-identical directories. Cleaner endpoint would be createPlanV2FromV1 delegating to the publisher too.

C3 — Publisher doesn't validate blob.sha256 shape before joining it into the temp path. planV2Publisher.ts:56-62 does join(this.temporaryDir, "artifacts", "sha256", blob.sha256.slice(0, 2), blob.sha256) with no isSha256(blob.sha256) gate. Internal callers (via publishPlanV2FromV1) always pass a real digest, so no live bug. But this class is a public API surface for future adapter callers — an adapter that read a manifest field into blob.sha256 without validating (or a fuzzer testing the publisher directly) could escape the temp dir. assertSafeRelativePath is never invoked inside the publisher. One-line fix at the top of putBlob: if (!/^[a-f0-9]{64}$/.test(blob.sha256)) throw new PlanV2IntegrityError(...).

C4 — commitManifest doesn't enforce the "every referenced blob is durable" contract in its docstring. planV2Publisher.ts:22-23 says "Commit the manifest only after every referenced blob is durable." The impl at :75-79 just writes plan.json and renames — never inspects the manifest bytes to check every referenced sha256 exists under temporaryDir/artifacts/sha256/. Convention is enforced by publishPlanV2FromV1's loop, not defensively. A future adapter that calls the publisher directly and skips one blob would commit a manifest referencing missing artifacts; downstream verifyBlob at planV2.ts:799-812 catches it on materialize (typed error, safe), but the commit lets a poison manifest reach storage. Adding a loop-over-parsed-manifest assertion is cheap defense-in-depth.

C5 — putBlob uses fixed .tmp suffix, not mkdtempSync. planV2Publisher.ts:65 const temporaryPath = \${destinationPath}.tmp`;. writeBlob (planV2.ts:437-451) uses mkdtempSyncper blob — this PR's new publisher path is a slight concurrency-safety downgrade.publishPlanV2FromV1serializes uploads withawait, so no live race — but any adapter that parallelizes putBlobfor throughput hitsEEXIST(not incanFallbackToCopycodes → aborts). If parallel puts are intended for the S3/GCS adapters, this needs to move tomkdtempSync`.

Nits (CONTRIBUTING.md compliance)

• planV2Publisher.ts:30 — const code = (error as Error & { code?: unknown }).code;. Bare as T. The !(error instanceof Error) || !("code" in error) guard on line 29 already narrows to a type with .code; the cast is unnecessary. Just do const code = error.code; after the guard.
• planV2.test.ts:44, 256-258, 376 — bare as T at JSON.parse boundaries. Should be as unknown as ... per CONTRIBUTING.md:47/54. Test-only, low priority.
• planV2.test.ts:339, 386, 412-414 — [0]! and .find(...)! non-null assertions. CONTRIBUTING.md:55 flags these. Test-only.

Verified clean

• Manifest determinism preserved — both paths funnel buildPlanV2Publication, so planHash + artifact list are byte-identical. Determinism test at planV2.test.ts:145-158 locks in.
• On-disk layout unchanged — plan.json at root, blobs at artifacts/sha256/xx/xxxxx... (matches blobPath at planV2.ts:196-198). Downstream #2789/#2790 adapters see the same tree.
• Symlink rejection preserved — listFiles (planV2.ts:172-194) still throws typed error; test at planV2.test.ts:182-192.
• v1 planHash re-verification preserved — recomputePlanHashFromPlanDir gate still runs at planV2.ts:475-481.
• Atomic rename atomicity preserved in both paths — mkdtempSync(prefix) → populate → renameSync(tempDir, finalDir).
• Partial-failure cleanup wired — publishPlanV2FromV1 calls publisher.abort() on any error (planV2.ts:572-579); LocalPlanV2ArtifactPublisher.abort() removes temporaryDir only if !#committed.
• Hardlink safety — staging dir and destination temp dir on the same filesystem; linkSync succeeds; rmSync(stagingDir) doesn't unlink destination blobs (hardlink → inode kept alive).
• Blob dedup — blobs.set(sha256, ...) Map at planV2.ts:502-504 plus defense-in-depth existsSync check at planV2Publisher.ts:63.
• Test coverage exists for happy paths — hardlink verification via dev+ino compare at planV2.test.ts:421-424, abort-on-blob-failure ordering test with a mock publisher at :427-448. Gaps noted in C4/C5 above.

CI: 7 required checks green, 9 pending (regression shards + parity — likely same infra flake as #2789's Docker registry timeout).

— Review by Rames D Jusso

The base branch was changed.

[james-russo-rames-d-jusso]

LGTM at 712f0f19f. All five R1 design concerns are addressed and locked in by tests. This is a clean refactor for a genuinely storage-neutral publisher; my prior R1 pushback stands as motivation but the shape here is the right one.

R1 → R2 verification

• ✅ C1 (typed errors) — LocalPlanV2ArtifactPublisher now throws PlanV2IntegrityError in every place it previously threw bare Error: constructor (planV2Publisher.ts:83-85), putBlob sha256 / size validation (:96-101), commitManifest durability check (:118-122), and the new manifestDigests JSON parser (:47-63). The class + code moved to a dedicated planV2Errors.ts and are re-exported from planV2.ts, so producer and publisher share the same throw contract.
• ✅ C2 (divergent local-publish paths) — planV2() now delegates to planV2WithPublisher(projectDir, config, publisher, { stagingParentDir: dirname(planV2Dir) }) (planV2.ts:625-631), and createPlanV2FromV1 + LocalPlanV2ArtifactPublisher.putBlob both call the shared planV2BlobPath from planV2Layout.ts. The "produces byte-identical local CAS output through both publication paths" test at planV2.test.ts:461-479 asserts readFileSync(plan.json) and every artifact blob are byte-identical between the two paths. Real single-source-of-truth now.
• ✅ C3 (no sha256 shape validation in putBlob) — assertPlanV2Sha256 at planV2Layout.ts:5-10 enforces /^[0-9a-f]{64}$/ and throws PlanV2IntegrityError with named field context. Called in putBlob on entry (:95), in commitManifest for every manifest artifact (manifestDigests:60), and implicitly by every planV2BlobPath call. The "rejects malformed digests before constructing a local CAS path" test at :502-513 pins the ../escape path-traversal case.
• ✅ C4 (commitManifest durability contract) — commitManifest at planV2Publisher.ts:114-125 iterates every unique digest referenced in the manifest and existsSync-checks the actual blob file before writing plan.json. The "refuses to commit a manifest until every referenced blob is durable" test at :515-523 pins the failure case with a manifest referencing a never-uploaded digest.
• ✅ C5 (mkdtempSync concurrency safety) — mkdtempSync(join(dirname(destinationDir), ".plan-v2-publish-")) at :89 for the staging root, and mkdtempSync(join(dirname(destinationPath), ".plan-v2-blob-")) at :103 for per-blob staging. No fixed .tmp suffix anywhere.

New public API surface — reviewed

The PlanV2ArtifactPublisher interface (putBlob / commitManifest / abort) plus PlanV2PublishBlob shape are now the contract adapters will implement. Design reviewed:

• The "supports a remote publisher contract with no shared destination filesystem" test at :481-500 uses a Map<string, Buffer> publisher to prove the interface is genuinely storage-neutral. Confirms manifest bytes match canonicalJsonStringify(manifest), every artifact is put with correct sizeBytes, and no shared-filesystem assumption leaks.
• Bounded concurrency at planV2.ts:562-568 (batch of 16 via Promise.allSettled + throw-first-rejected) is the right shape for the first-cut storage-neutral API.
• sourcePath docstring explicitly warns "planner-local and valid only for the lifetime of this call" — remote adapters must complete the upload before resolving. Good contract.
• stagingParentDir option cleanly parameterizes the "local wants hardlink locality, remote uses tmpdir()" distinction without leaking implementation into the interface.
• Copy fallback (EXDEV / EPERM / EACCES / ENOTSUP) has direct test coverage via the injectable linkFile seam at :74-88 and the "falls back to an atomic copy when hard-linking is unavailable" test at :428-458.
• Abort semantics are correctly plumbed at BOTH levels: publishPlanV2FromV1 at planV2.ts:571-579 wraps the full build/put/commit sequence in try/catch, awaits publisher.abort() in a nested try/catch that swallows cleanup errors to preserve the original failure, then rethrows; planV2WithPublisher at planV2.ts:602-615 adds an idempotent outer abort so failures during the preceding plan() stage are covered too. No caller-owned try/finally gap.

Minor forward-looking considerations (non-blocking)

None of these are blockers for this PR — they're worth thinking about when the first non-local adapter lands. Owning them here so we don't forget:

1. concurrency = 16 is a hardcoded local const at planV2.ts:562. Fine for local hardlinks; for future remote adapters over slow links, a per-publisher tunable would probably be worth adding. Non-breaking to add later via an options bag.
2. Promise.allSettled + throw-first-rejected means on failure of one blob in a batch, we still wait for the other 15 to settle before rejecting. Interface has no AbortSignal — worth revisiting once the first remote adapter's error paths are visible, since a hung upload could delay error propagation by minutes on a slow S3 backend. Same non-breaking-to-add-later story.

Scope statement acknowledged

James's framing that this still holds the complete v1 staging tree in one planner pod and does NOT solve large-plan / ENOSPC is exactly right — this PR is the foundation for storage-neutral publishing (interface + shared layout + tests proving no shared-fs assumption), not the final streaming/ENOSPC fix. Oversized-plan rollout staying disabled is the right posture.

Correction (edited): an earlier version of this review flagged publishPlanV2FromV1 as not calling publisher.abort() on rejection. That was a misread — I stopped one screen too early. The full function at planV2.ts:571-579 does wrap the entire flow in try/catch and awaits abort() in a nested cleanup block. Nothing to add there.

Nothing left blocking on my side.

[miguel-heygen]

Fresh exact-head review at 712f0f19fc62464531e4c55b44c3ad5ac767a488.

The storage-neutral seam is coherent:

• packages/producer/src/services/distributed/planV2.ts:556-579 starts at most 16 blob publications per batch, waits for every started promise with Promise.allSettled, skips manifest commit after any rejection, and calls the publisher's idempotent abort() while preserving the original error.
• packages/producer/src/services/distributed/planV2.ts:588-611 keeps the planner-private v1 tree alive until publication finishes, then removes it in finally; the contract at packages/producer/src/services/distributed/planV2Publisher.ts:22-32 explicitly requires remote implementations to consume/upload each planner-local source before resolving.
• packages/producer/src/services/distributed/planV2Publisher.ts:83-135 now uses typed deterministic errors, validated shared CAS layout, source-size checks, collision-safe per-blob staging, manifest completeness checks, manifest-last directory publication, and idempotent cleanup.

All prior publisher concerns are addressed: shared layout/error leaves landed in planV2Layout.ts and planV2Errors.ts; the cast and unchecked .find(...)! were removed; copy fallback, malformed digests, incomplete manifests, remote publication without a destination filesystem, and byte-identical local output are pinned at packages/producer/src/services/distributed/planV2.test.ts:428-545. The new entry point and publisher types are exposed through both supported package surfaces at packages/producer/src/distributed.ts:57-83 and packages/producer/src/index.ts:134-183.

I also traced the existing transports rather than inferring from the interface: AWS workers exchange S3 manifest/CAS locators, GCP workers exchange GCS locators, and the current Experiment Framework activities exchange S3 plan/chunk locators while local paths are confined to the Python/Node sidecar pod. Nothing in this PR requires a filesystem shared across distributed nodes.

One existing forward-looking note is not applicable to this head: direct publishPlanV2FromV1() callers do get automatic abort-on-rejection at planV2.ts:572-578.

The scope statement is accurate: this establishes the publication boundary but still materializes the complete v1 staging tree in one planner pod, so it is not yet the large-plan/ENOSPC rollout fix.

Local verification: focused producer tests 39/39; producer typecheck; changed-file oxlint and oxfmt; git diff --check. All completed required checks are green; the two Windows required jobs are still running with no current failure.

Verdict: APPROVE
Reasoning: The exact head preserves local compatibility while adding a correctly ordered, bounded, storage-neutral publisher contract; prior correctness, safety, and coverage concerns are resolved with no new blocker found.

— Magi

[vanceingalls]

Verdict: APPROVE — All prior findings from R1 (my P2 layout drift, Magi's as T + non-null nits, James's C1-C5) are resolved at head. The publisher extraction is genuinely storage-neutral for cross-pod artifact exchange; sourcePath coupling on putBlob is the intended planner-local upload seam, not abstraction leakage. Scope carve-out honored — the complete v1 staging tree still lives on the planner pod, and no adapter yet consumes planV2WithPublisher().

Verification of prior findings

R1 P2 (mine) — CAS layout duplicated across publisher and legacy helper. RESOLVED. Extracted to shared module packages/producer/src/services/distributed/planV2Layout.ts:13-16 (planV2BlobPath()) + :6-11 (assertPlanV2Sha256()). Consumed by both:

• packages/producer/src/services/distributed/planV2Publisher.ts:101 (publisher write path)
• packages/producer/src/services/distributed/planV2.ts:47, 541, 823 (legacy write, verify, materialize)

Producer-internal drift risk is gone. AWS/GCP handlers still carry their own local planV2BlobPath copies at handler.ts:703-705 and server.ts:711-713, but those are out-of-scope pre-existing consumer duplicates — noted as follow-up, not a new finding.

R1 P3 (mine) — copy-fallback branch untested. RESOLVED. planV2Publisher.ts:63-66 introduces the linkFile test seam; planV2.test.ts:428-456 exercises the EXDEV fallback by throwing an EXDEV-coded error from the injected linkFile. Asserts destination has different inode from source (proving copy, not link).

R1 P3 (mine) — sync fs inside async methods. Not addressed by design — still linkSync/copyFileSync/renameSync/writeFileSync inside async methods. Explicitly non-blocking in R1; remains a stylistic note for future S3/GCS impls to genuinely be async. No action.

Magi P3 — bare as Error & { code?: unknown } cast at prior publisher line 28-31. RESOLVED. planV2Publisher.ts:35-39 now narrows via !(error instanceof Error) || !("code" in error) and reads error.code off the narrowed type — no cast.

Magi P3 — unchecked non-null assertion on .find(...) at prior test line 412-414. RESOLVED. planV2.test.ts:412-415 now uses if (artifact === undefined) throw new Error(...) explicit guard.

James C1 — Publisher throws bare Error, breaking adapter classification. RESOLVED. planV2Errors.ts:8-17 promotes PlanV2IntegrityError (with code = PLAN_V2_INTEGRITY_UNRECOVERABLE) to a shared module. Every publisher throw at planV2Publisher.ts:50, 53, 57, 59, 85, 97-99, 122-124 now constructs the typed error. Both AWS (handler.ts:150-156 name normalization) and GCP (server.ts:905-928 NON_RETRYABLE_ERROR_NAMES) already contain both the class name PlanV2IntegrityError and the code PLAN_V2_INTEGRITY_UNRECOVERABLE from earlier stack work, so the wire discriminator catches this.

James C2 — Two divergent local-publish paths, no parity test. RESOLVED. planV2.test.ts:458-476 "produces byte-identical local CAS output through both publication paths" — reads plan.json and every artifact blob from both createPlanV2FromV1 and publishPlanV2FromV1(...LocalPlanV2ArtifactPublisher) outputs, asserts readFileSync equality.

James C3 — Publisher doesn't validate blob.sha256 shape before path-joining. RESOLVED. planV2Publisher.ts:94 calls assertPlanV2Sha256(blob.sha256, "published blob sha256") at the top of putBlob. Test at planV2.test.ts:501-511 exercises the "../escape" rejection.

James C4 — commitManifest doesn't enforce "every referenced blob is durable." RESOLVED. planV2Publisher.ts:119-126 iterates new Set(manifestDigests(manifestBytes)) and existsSync-checks each blob's CAS path before writing plan.json. Test at planV2.test.ts:513-522 proves missing-blob rejection.

James C5 — Fixed .tmp suffix in putBlob, not mkdtempSync. RESOLVED. planV2Publisher.ts:104 now mkdtempSync(join(dirname(destinationPath), ".plan-v2-blob-")) per blob — collision-safe under parallel putBlob.

James nits (test-only bare as T at test lines 44/256/376; bare [0]! at 339/386). Not addressed — pre-existing test-only, explicitly flagged low-priority in R1. Note only.

Claim-by-claim verification

Claim 1: filesystem-not-shared (AWS + GCP + EF Temporal)

AWS Lambda: packages/aws-lambda/src/handler.ts:672 (downloadS3ObjectToFile(s3, event.PlanV2ManifestS3Uri, ...)) and :695-700 (downloadS3ObjectToFileVerified per artifact) prove chunk/assemble pods each pull manifest + artifacts fresh from S3 into their own /tmp via downloadAndMaterializePlanV2 (:660-687). No shared FS assumption between planner and worker pods — the transport is S3 exclusively.

GCP Cloud Run: packages/gcp-cloud-run/src/server.ts:676 (downloadGcsObjectToFile(storage, ...)) and :697-706 (downloadPlanV2Artifact → downloadGcsObjectToFileVerified) — identical shape via GCS. Chunk/assemble pods each download to their own /tmp before materializePlanV2Target.

EF Temporal: No Temporal-side code in this repo (private HeyGen integration). The verifiable proxy is (a) the interface signatures allow a Temporal adapter and (b) the remote in-memory publisher test at planV2.test.ts:478-499 demonstrates the full flow with a putBlob/commitManifest/abort implementation that only writes to an in-memory Map<string, Buffer> — zero filesystem side-effects on the destination. That is the "no shared destination filesystem" claim in test form.

Verdict: FS-not-shared is upheld everywhere in-scope; adapter-side S3/GCS transport is pre-existing infrastructure that this PR does not disturb.

Claim 2: planner-local publisher docs

planV2Publisher.ts:68-76 docstring: "Planner-local manifest-last publisher. Same-filesystem blobs are hard-linked from the frozen source plan… This implementation is a compatibility adapter for local callers; distributed nodes exchange remote manifest/CAS locators through their cloud adapters." Explicit constraint call-out. Name is LocalPlanV2ArtifactPublisher (not DefaultPublisher / bare Publisher), making the local-only intent visible at every use site. The public planning entry planV2() (planV2.ts:619-632) is the only in-tree constructor. Interface-level docs on putBlob (:23-27) also spell out that sourcePath is "planner-local and valid only for the lifetime of this call."

Claim 3: storage-neutral planV2WithPublisher()

planV2.ts:588-612. Signature: (projectDir, config, publisher, options?). PlanV2WithPublisherOptions (:112-120) exposes only stagingParentDir — the planner's private v1 staging directory, not a publish-side concept. The publisher interface (planV2Publisher.ts:22-33) is:

• putBlob({sourcePath, sha256, sizeBytes}) → Promise<void>
• commitManifest(manifestBytes: string) → Promise<void>
• abort() → Promise<void>

commitManifest and abort are cleanly path-free. putBlob takes a planner-local sourcePath — this is the intentional upload seam (publisher reads from planner-local disk, uploads to durable storage). It does not leak filesystem to workers; the manifest carries only sha256 + size, and workers dereference via adapter-owned locators. An S3/GCS/Temporal publisher can implement putBlob with s3.upload(sourcePath, ...) / gcs.upload(sourcePath, ...) / activity.uploadFromLocal(sourcePath, ...) and resolve after upload — the "porous" concern is about consumer-side, and that side is clean.

Public entry-point test at publicExports.test.ts:101-104 locks the exported symbol set: planV2WithPublisher, publishPlanV2FromV1, LocalPlanV2ArtifactPublisher.

Claim 4: bounded-concurrent + manifest-last

planV2.ts:556-580 (publishPlanV2FromV1):

const concurrency = 16;
for (let offset = 0; offset < publication.blobs.length; offset += concurrency) {
  const batch = publication.blobs.slice(offset, offset + concurrency);
  const results = await Promise.allSettled(batch.map((blob) => publisher.putBlob(blob)));
  for (const result of results) {
    if (result.status === "rejected") throw result.reason;
  }
}
await publisher.commitManifest(canonicalJsonStringify(publication.manifest));

• Bounded: batches of ≤16 via slice(offset, offset + concurrency).
• Manifest-last: commitManifest awaits after the entire blob loop, on a separate line — not inside a Promise.all([...blobs, commitManifest]). No race window.
• On any blob rejection, throw result.reason skips the commit and enters the outer catch (:572-579) which calls publisher.abort() before re-throwing.
• Test at planV2.test.ts:524-545 verifies the ordering: injected putBlob failure produces ["blob:…", "abort"] — "manifest" never present.

Claim 5: prior publisher feedback resolutions

Enumerated with grep evidence:

• Typed errors: planV2Errors.ts:8-17 — PlanV2IntegrityError with readonly code = PLAN_V2_INTEGRITY_UNRECOVERABLE. All publisher throws (planV2Publisher.ts:50, 53, 57, 59, 85, 97, 122) construct this. No bare throw new Error(...) in publisher.
• Shared validated CAS layout: planV2Layout.ts:13-16. Consumed by publisher + legacy write + verify + materialize (paths listed in prior finding above).
• Source-size checks: planV2Publisher.ts:95-100 — statSync(sourcePath).size compared against declared sizeBytes, mismatch throws typed error. Defends against post-hash file swap.
• Defensive manifest completeness: planV2Publisher.ts:119-126 — parses manifest, extracts every artifact's sha256, existsSync-checks each in temporaryDir. Blocks a poison manifest from reaching the destination via a direct-adapter caller that skipped a putBlob.
• Collision-safe temp dirs: planV2Publisher.ts:90 (constructor: mkdtempSync(join(dirname(destinationDir), ".plan-v2-publish-"))) + :104 (per-blob: mkdtempSync(join(dirname(destinationPath), ".plan-v2-blob-"))). Unique per call, no .tmp collision under parallel put.
• Copy-fallback coverage: planV2.test.ts:428-456 (EXDEV via injected linkFile) — asserts different inode.
• No cast/non-null shortcut: planV2Publisher.ts / planV2Layout.ts / planV2Errors.ts all grep-clean for as non-unknown casts and !. non-null.

Claim 6: remote in-memory publisher coverage + byte-identical CAS

• Remote in-memory: planV2.test.ts:478-499. Publisher literally is {blobs: Map<string,Buffer>, committedManifest: string|undefined} — no mkdir/writeFile. Asserts committedManifest === canonicalJsonStringify(manifest) and every artifact's bytes land in the Map with correct length. Proves the interface can hold on an in-memory destination.
• Byte-identical local CAS: planV2.test.ts:458-476. Runs both createPlanV2FromV1(v1, directDir) and publishPlanV2FromV1(v1, new LocalPlanV2ArtifactPublisher(publishedDir)), then readFileSync(...)-compares plan.json and every artifact blob byte-for-byte between the two output trees. Pins the compat guarantee against future publisher refactors.

Multi-shape throw normalization (new lens)

New error type: PlanV2IntegrityError in planV2Errors.ts. Class construction is the ONLY code path — grep of planV2Publisher.ts and planV2.ts finds new PlanV2IntegrityError(...) at every throw site, no bare .code = "..." or .name = "..." assignments outside the class. Every throw guarantees both:

• error.name = "PlanV2IntegrityError" (constructor :15)
• error.code = "PLAN_V2_INTEGRITY_UNRECOVERABLE" (readonly field :11)

Wire discriminators reach both:

• AWS handler.ts:150-156 normalizeTerminalErrorName maps code→name for Step Functions.
• GCP server.ts:905-928 NON_RETRYABLE_ERROR_NAMES contains both "PlanV2IntegrityError" (class) and "PLAN_V2_INTEGRITY_UNRECOVERABLE" (code).

No multi-shape variant this PR introduces bypasses either discriminator. Clean.

Fresh 4-lens pass at 712f0f1

Standards

• No new bare as T in publisher/errors/layout modules.
• No new non-null !. in publisher/errors/layout.
• Test file still carries pre-existing bare as Record<string, unknown> (planV2.test.ts:44, 256, 376) and [0]! (planV2.test.ts:339, 386). Explicitly test-only and flagged non-blocking in R1. No change.

Spec forward-check

Every PR-body bullet has diff evidence (traced in Claims 1-6 above). No aspirational claim uncovered.

Spec reverse-check

• All 8 changed files are packages/producer/**. No accidental adapter/scaffold edits.
• Scope carve-out (large-plan/ENOSPC not fixed) explicitly disclosed and honored: planV2WithPublisher at planV2.ts:594-601 still stages the complete v1 tree at mkdtempSync(...) then calls the existing plan(...) with planDirSizeLimitBytes: Number.MAX_SAFE_INTEGER. Publisher then reads back from the staging tree. No direct-emission planner introduced.
• No adapter (packages/aws-lambda, packages/gcp-cloud-run) calls planV2WithPublisher yet — grep-clean. Oversized-plan rollout gate is preserved by simple non-adoption.

Sibling-precision divergence

• publishPlanV2FromV1 uses hardcoded const concurrency = 16; (planV2.ts:562).
• DEFAULT_MAX_PARALLEL_CHUNKS = 16 in packages/producer/src/services/distributed/plan.ts:334.
  Same magic number, two logically-distinct knobs (blob upload fan-out vs chunk worker fleet size). Currently coincidentally aligned to infra scale. P3 note — worth naming as DEFAULT_PUBLISH_CONCURRENCY when the S3/GCS publishers land so the tuning story is legible; not blocking.

Middle-man wrap/unwrap

• commitManifest receives canonicalJsonStringify(publication.manifest) (planV2.ts:570), then re-parses it in manifestDigests (planV2Publisher.ts:45-61) to enumerate declared blob digests. Round-trip is intentional (the publisher is the boundary that must validate the serialized manifest, since a direct adapter caller could construct the string differently). assertPlanV2Sha256 runs on every re-parsed digest, so nothing bypasses validation.

Behavior + editor-UI lens complement

Silent-catch + error-invariant

• planV2.ts:573-577 and :602-606 swallow publisher.abort() errors inside outer catches, with comment "Preserve the publication failure; cleanup is best-effort." Original error re-thrown. Correct behavior.
• planV2Publisher.ts:35-39 canFallbackToCopy narrows only to EXDEV|EPERM|EACCES|ENOTSUP; other errors re-throw (including ENOENT deliberately, so a missing source surfaces). Clean.
• manifestDigests at :45-61 try/catch on JSON.parse re-throws as typed PlanV2IntegrityError — no silent swallow.

Concurrency/lifecycle

• Constructor eagerly creates temporaryDir at :90. If caller never calls abort or commit, temp dir leaks — standard "must call abort()" idiom; publishPlanV2FromV1 always does. Acceptable.
• Double-commitManifest: second call would fail on renameSync (already-committed temp path gone); error not specifically typed, but it can't produce a valid manifest → not exploitable. Minor.
• Double-abort: safe via !#committed guard + rmSync({force: true}) (:132-135).
• Test at planV2.test.ts:524-545 explicitly asserts abort is the last call on failure, and commitManifest never runs.
• No AbortSignal/cancellation support. Not a design flaw for this PR; can be added when the actual remote publishers land.

PR-body-vs-diff parity

Full match. No claim in body without corresponding diff evidence; no diff line without a corresponding body statement.

Perf audit

• Blob loop is bounded per-batch (16). No unbounded fan-out.
• manifestDigests iterates artifacts once; Set dedup on the caller side. O(n).
• Bytes stream through linkSync/copyFileSync at the FS layer — bounded per blob, not buffered in JS heap.
• Manifest bytes held as a single string argument to commitManifest. For extreme plans (~millions of artifacts) the manifest string could dominate memory; scope carve-out explicitly defers oversized-plan support, so P3 note only.

Scope carve-out verification

James's disclosure: "still holds the complete v1 staging tree in one planner pod… oversized-plan rollout remains disabled."

Verified in code:

• planV2WithPublisher (planV2.ts:594-601) creates stagingRoot via mkdtempSync on the planner pod's disk, then calls plan(projectDir, ..., stagingRoot) to produce the full v1 tree locally, then publishPlanV2FromV1(stagingRoot, publisher) reads it back. The complete v1 tree exists on planner-local disk during publication. Carve-out honored.
• planDirSizeLimitBytes: Number.MAX_SAFE_INTEGER (planV2.ts:598) disables the historical 2 GiB cap only inside planV2WithPublisher; existing plan() call sites are untouched. Consistent with body claim "its historical 2 GiB transport cap is disabled because no monolithic archive is emitted."
• No adapter file consumes planV2WithPublisher at head (grep-clean across packages/aws-lambda/src/** and packages/gcp-cloud-run/src/**). Oversized-plan traffic on adapters is off by simple non-adoption.
• No feature flag needed at the producer layer — the rollout gate is the adapter migration itself, which this PR does not perform.

Positive verification: James's scope carve-out is honored end-to-end.

Verdict

APPROVE. All R1 findings resolved with test evidence; publisher extraction is genuinely storage-neutral for the cross-pod contract that matters; scope carve-out is honored in code; wire-discriminator reachability for the new typed error is intact on both AWS and GCP. Non-blocking notes: concurrency = 16 sibling-alignment with DEFAULT_MAX_PARALLEL_CHUNKS (name the constant when the S3/GCS publishers land), and pre-existing consumer-side planV2BlobPath duplicates in aws-lambda/handler.ts:703-705 + gcp-cloud-run/server.ts:711-713 are worth pulling into the shared planV2Layout module in a follow-up.

— Via