Skip to content

feat(producer): freezePlan snapshots PRODUCER_RUNTIME_* env vars#770

Open
jrusso1020 wants to merge 1 commit into
feat/producer-seeded-random-shimfrom
feat/producer-freeze-plan-runtime-env
Open

feat(producer): freezePlan snapshots PRODUCER_RUNTIME_* env vars#770
jrusso1020 wants to merge 1 commit into
feat/producer-seeded-random-shimfrom
feat/producer-freeze-plan-runtime-env

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 13, 2026

What

Adds snapshotRuntimeEnv(env = process.env) and RUNTIME_ENV_SNAPSHOT_PREFIXES to packages/producer/src/services/render/stages/freezePlan.ts. The helper captures process.env keys matching PRODUCER_RUNTIME_ or PRODUCER_RENDER_ prefixes into a fresh Record<string, string>, ready to be embedded in LockedRenderConfig.runtimeEnv.

Why

Part of Phase 2, §4.3 (LockedRenderConfig.runtimeEnv) and §5.2 (RENDER_SEEK_MODE row).

fileServer.ts reads several PRODUCER_RUNTIME_* / PRODUCER_RENDER_* env vars at module-load time (RENDER_SEEK_MODE, RENDER_SEEK_STEP, RENDER_SEEK_OFFSET_FRACTION, …) and bakes them into the served HTML's RENDER_MODE_SCRIPT. Distributed chunk workers are separate processes that may inherit a different environment; without a snapshot, two chunks of the same plan could serve different HTML and produce divergent output. The plan freezes the controller's env so Phase 3's renderChunk() can materialize it back into the worker's process.env before launching its file server.

How

  • Helper iterates Object.keys(env), retains entries whose key starts with one of the prefixes, skips entries whose value is undefined, and returns a NEW object each call (so subsequent env mutations don't retroactively change a frozen plan).
  • Exported RUNTIME_ENV_SNAPSHOT_PREFIXES so the Phase 3 worker side can apply the same filter (asymmetric handling would leak stale controller env into worker behavior).
  • The freezePlan() function body remains a skeleton — Phase 3 owns the full implementation. The snapshot helper is exported on its own so this gate is testable without that body.

Test plan

  • Unit tests added: freezePlan.test.ts (9 cases) covers PRODUCER_RUNTIME_* capture, PRODUCER_RENDER_* capture, both-prefix mix, ignored unrelated keys, off-by-one prefix variants (PRODUCER_RUNTIM_, PRODUCER_RENDR_, PRODUCER_DEBUG_), undefined-value skip, fresh-object contract, default-to-process.env, and the exported prefix list assertion.
  • No caller invokes snapshotRuntimeEnv yet — Phase 3's freezePlan body will.
  • bun run --cwd packages/producer typecheck clean.
  • bunx oxlint + bunx oxfmt --check clean.

This is part of a stack of 10 PRs; this is PR 5 of 10. Stacked on top of #769.

🤖 Generated with Claude Code

Part of Phase 2 of the distributed rendering plan (determinism hardening).
See DISTRIBUTED-RENDERING-PLAN.md §4.3 (LockedRenderConfig.runtimeEnv) and
§5.2 (RENDER_SEEK_MODE row).

`fileServer.ts` reads several `PRODUCER_RUNTIME_*` and `PRODUCER_RENDER_*`
env vars at module-load time (RENDER_SEEK_MODE, RENDER_SEEK_STEP,
RENDER_SEEK_OFFSET_FRACTION, …) and bakes them into the served HTML's
RENDER_MODE_SCRIPT. Distributed chunk workers are separate processes that
may inherit a different environment, so the plan needs to freeze a
snapshot.

Adds `snapshotRuntimeEnv(env = process.env)` in
packages/producer/src/services/render/stages/freezePlan.ts. Captures keys
matching `PRODUCER_RUNTIME_` or `PRODUCER_RENDER_` prefixes into a fresh
plain object, ignoring everything else. Phase 3's `renderChunk` will
materialize the snapshot back into `process.env` before launching its
file server.

Also exports `RUNTIME_ENV_SNAPSHOT_PREFIXES` so the chunk-worker side can
apply the same prefix filter (asymmetric handling would leak stale
controller env into worker behavior).

The freezePlan function body remains a skeleton — Phase 3 owns the full
implementation. The snapshot helper is exported on its own so this gate's
unit test can pin the behavior without depending on the not-yet-written
freezePlan body.

In-process behavior is unchanged: no in-process caller invokes
freezePlan or snapshotRuntimeEnv yet.

9 unit tests at packages/producer/src/services/render/stages/
freezePlan.test.ts cover: prefix matches (both families), non-matching
keys ignored, undefined values skipped, fresh-object contract, and
default-to-process.env behavior.

This is part of a stack of 10 PRs; this is PR 5 of 10.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jrusso1020 jrusso1020 force-pushed the feat/producer-seeded-random-shim branch from 693bd29 to f485c81 Compare May 13, 2026 01:33
@jrusso1020 jrusso1020 force-pushed the feat/producer-freeze-plan-runtime-env branch from f1935ef to 1800f49 Compare May 13, 2026 01:33
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, focused utility. The separation into runtimeEnvSnapshot.ts (standalone, no freeze-pipeline deps) with a re-export from freezePlan.ts is the right call — chunk workers need the snapshot helper at boot without importing the entire freeze pipeline.

Key correctness checks:

  • Fresh object each call (Object.keys + new Record) — prevents live-reference mutation from leaking across plan snapshots.
  • typeof value !== "string" guard handles the undefined slots in Record<string, string | undefined> cleanly.
  • Exported RUNTIME_ENV_PREFIXES keeps capture and re-apply sides in sync — the test asserts the exact prefix list, which is a good regression anchor.
  • Off-by-one prefix variants in tests (PRODUCER_RUNTIM_, PRODUCER_RENDR_, PRODUCER_DEBUG_) are exactly the edge cases that catch startsWith bugs.

The freezePlan() body staying a skeleton is fine — it's Phase 3 scope and the error message update (dropping the plan-doc reference) is a minor cleanup.

LGTM.

— Magi

Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strengths:

  • Clean separation: runtimeEnvSnapshot.ts is a standalone utility with no dependency on the freeze pipeline (runtimeEnvSnapshot.ts:14-16) — chunk workers can re-apply the snapshot without dragging in the planner. Right call.
  • RUNTIME_ENV_PREFIXES exported for the worker side (runtimeEnvSnapshot.ts:24) is the single source of truth that prevents capture/replay asymmetry — pinned by the "exports the prefix list for chunk-worker materialization" test (runtimeEnvSnapshot.test.ts:96).
  • Fresh-object contract pinned via the mutate-after-snapshot test (runtimeEnvSnapshot.test.ts:77-85) — guards against accidental return env pass-through.
  • Off-by-one prefix variants explicitly negative-tested (runtimeEnvSnapshot.test.ts:48-50PRODUCER_RUNTIM_, PRODUCER_RENDR_, PRODUCER_DEBUG_) — catches typo-prefix mistakes.

Findings:

important: PR description claims snapshotRuntimeEnv and RUNTIME_ENV_SNAPSHOT_PREFIXES live in freezePlan.ts, but the actual diff puts them in a new file runtimeEnvSnapshot.ts and names the const RUNTIME_ENV_PREFIXES (without _SNAPSHOT_). The re-export at freezePlan.ts:97 preserves backward-compat for any caller that already imports from freezePlan — but the description should match the diff so reviewers and future spelunkers find the right file. Not a code issue, just a description-vs-diff drift. (Rule 3 cross-check: no other reviewers caught this yet — surfacing as the first reviewer.)

important: runtimeEnvSnapshot.ts:42 filters typeof value !== "string" — but process.env only ever returns string | undefined in Node, never numbers/booleans. The defensive guard is fine, but the test at runtimeEnvSnapshot.test.ts:56 only covers undefined. If a worker materializes the snapshot back via Object.assign(process.env, snapshot), all process.env reads will return strings — no breakage. But: when freezing the plan, if a caller passes a non-string-only env object (e.g. a test fixture using Record<string, unknown>), the silent skip means the snapshot is incomplete and the failure surfaces only at chunk-worker time. Consider: either widen the input type to Record<string, unknown> and explicitly throw on non-string for matched-prefix keys, or narrow the input type to Record<string, string | undefined> (what's there now) and rely on the type system. Currently it's a soft-skip — easy to miss in CI.

nit: runtimeEnvSnapshot.ts:24 as const on the array is fine but readonly string[] is a wider type than readonly ["PRODUCER_RUNTIME_", "PRODUCER_RENDER_"]. If you want callers (e.g. chunk workers) to pattern-match exhaustively, the tuple form is more useful. Minor.

Verdict: APPROVE
Reasoning: Helper is correctly shaped (fresh-object, prefix-shared, no live process.env ref). Description drift is doc-only and the input-type concern is a follow-up.

— Vai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants