fix(cli): fix concurrency race, none-vs-undefined bug, and 3 more DE trial gaps

Six findings from a third max-effort code review, focused on the previous
commit's fixes:

1. --batch-concurrency N>=2 runs genuinely concurrent renderLocal() calls
   (Promise.all workers in batchRender.ts), which can't safely share the
   trial's one process-wide env var + module flag — a row finishing first
   could tear down the env var/flag mid-render for a sibling row still in
   flight. Rather than attempt to make shared process-global state safe
   under real concurrency, added RenderOptions.disableDeParallelRouterTrial
   and set it whenever batchConcurrency > 1 — the trial simply isn't
   offered when it can't be evaluated safely.

2. maybeConsumeDeParallelRouterTrial's "outcome === undefined" no-op guard
   almost never fired: aggregateDrawElement (perfSummary.ts) defaults
   parallelRouter to the string "none" for every render, whether or not
   drawElement/the router ever engaged — never undefined. Every ordinary
   render below the router's own frame threshold (the common case) was
   ticking the render-count backstop, tripping
   DE_PARALLEL_ROUTER_TRIAL_MAX_RENDERS after 25 completely unrelated
   renders that never touched the router. Now treats "none" the same as
   undefined.

3. isDeParallelRouterTrialBlocked relied solely on shouldTrack(), which
   memoizes its verdict once per process — during a long --batch run, a
   `hyperframes telemetry off` issued from another terminal mid-batch would
   never be observed. Restored a direct config.telemetryEnabled check
   (read fresh every call, unlike shouldTrack()'s cache) alongside it.

4. maybeConsumeDeParallelRouterTrial's config write had no way to detect a
   losing race against a concurrent process — added a verify-and-retry
   loop (write, re-read fresh, retry up to 3x if a concurrent writer
   landed in between) that narrows the window further without a full
   file-locking rewrite.

5. The trial could arm before the first-run telemetry disclosure
   (showTelemetryNotice) was guaranteed to have printed — that notice runs
   via a fire-and-forget, unawaited dynamic import in cli.ts with no
   ordering guarantee relative to the render command. Rather than touch
   that pre-existing async bootstrap chain, gated the trial on
   config.telemetryNoticeShown: it simply never offers itself on a fresh
   install's very first invocation.

6. Added a dedicated config.test.ts exercising readConfig/readConfigFresh/
   writeConfig through the REAL module (node:fs mocked with an in-memory
   fake, not a HOME-env hack) — readConfigFresh's cache-bypass and the
   type-guarded boolean/number parsing had zero coverage through the real
   implementation before this.

Also fixed the test fixture that was supposed to cover finding #2 but used
an unrealistic `drawElement: {}` shape instead of the real
`{ parallelRouter: "none" }` aggregateDrawElement actually produces.

Extracted applyDeParallelRouterOutcome to keep maybeConsumeDeParallelRouterTrial
under the repo's complexity gate after adding the retry loop.

11 new/updated tests in render.test.ts (56 total) + 7 new tests in
config.test.ts. Verified against fallow's audit gate clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

packages/cli/src/commands/render.test.ts

@@ -509,33 +509,79 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
   };
 
   it("enables the trial (sets the env var) on a fresh install with telemetry on", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
     expect(process.env.HF_DE_PARALLEL_ROUTER).toBe("true");
   });
 
   it("does not override an env var the user already set themselves", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     process.env.HF_DE_PARALLEL_ROUTER = "false";
     await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
     expect(process.env.HF_DE_PARALLEL_ROUTER).toBe("false");
   });
 
   it("does not enable the trial once it has already fired for this install", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: true };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: true,
+      telemetryNoticeShown: true,
+    };
     await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
     expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
   });
 
-  it("does not enable the trial when telemetry isn't actually trackable (shouldTrack() false — dev mode / DO_NOT_TRACK / disabled)", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+  it("does not enable the trial when shouldTrack() is false (dev mode / DO_NOT_TRACK)", async () => {
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     trackingState.shouldTrack = false;
     await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
     expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
   });
 
+  it("does not enable the trial when config.telemetryEnabled is false, even if shouldTrack() is stale-true (e.g. `hyperframes telemetry off` mid-batch)", async () => {
+    configState.config = {
+      telemetryEnabled: false,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
+    trackingState.shouldTrack = true;
+    await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
+    expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
+  });
+
+  it("does not enable the trial before the first-run telemetry disclosure has been shown at least once", async () => {
+    // cli.ts shows this notice via a fire-and-forget, unawaited dynamic
+    // import — there's no guarantee it printed before renderLocal runs on a
+    // brand-new install's very first invocation. Requiring
+    // telemetryNoticeShown means the trial never races an opt-in message
+    // against the disclosure it depends on.
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: false,
+    };
+    await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
+    expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
+  });
+
   it("does NOT persist the trial as fired on a clean 'routed' success — keeps trying on future renders", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     producerState.executeImpl = async (job) => {
       job.perfSummary = {
         resolution: { width: 100, height: 100 },
@@ -554,7 +600,11 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
   });
 
   it("persists the trial as fired when the router's own safety net actually reverted", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     producerState.executeImpl = async (job) => {
       job.perfSummary = {
         resolution: { width: 100, height: 100 },
@@ -567,17 +617,33 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
     );
   });
 
-  it("does not persist the trial as fired when the router never became eligible for this render", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+  it("does not persist the trial as fired or increment the render count when the router never became eligible for this render", async () => {
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     producerState.executeImpl = async (job) => {
-      job.perfSummary = { resolution: { width: 100, height: 100 }, drawElement: {} };
+      // aggregateDrawElement (perfSummary.ts) ALWAYS defaults parallelRouter
+      // to the string "none" for every render, whether or not drawElement
+      // ever ran — never undefined. This fixture must match that shape, not
+      // an unrealistic empty object, or the test doesn't actually exercise
+      // the "none"-vs-undefined distinction (review finding).
+      job.perfSummary = {
+        resolution: { width: 100, height: 100 },
+        drawElement: { parallelRouter: "none" },
+      };
     };
     await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
     expect(configState.writeConfigCalls).toHaveLength(0);
   });
 
   it("does NOT persist the trial as fired when a render merely 'routed' crashes for an unrelated reason (e.g. cancellation) — not a router failure", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     producerState.executeImpl = async (job) => {
       job.errorDetails = { observability: { capture: { deParallelRouter: "routed" } } };
       throw new Error("render cancelled");
@@ -597,7 +663,11 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
   });
 
   it("persists the trial as fired from the failure path when the router's safety net reverted but the retry still failed", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     producerState.executeImpl = async (job) => {
       job.errorDetails = { observability: { capture: { deParallelRouter: "reverted" } } };
       throw new Error("worker crashed even after fallback");
@@ -616,7 +686,11 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
     // maybeEnableDeParallelRouterTrial saw process.env.HF_DE_PARALLEL_ROUTER
     // already "true" (set by row 1) and mistook that for "the user set it",
     // returning trialArmed=false — silently dropping row 2's revert.
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
 
     producerState.executeImpl = async (job) => {
       job.perfSummary = {
@@ -642,8 +716,31 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
     expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
   });
 
+  it("does not arm the trial when disableDeParallelRouterTrial is set (real batch concurrency, --batch-concurrency N>=2)", async () => {
+    // Concurrent renderLocal calls share one process-wide env var and one
+    // module-level flag — safe for sequential --batch rows (every other
+    // test in this block), not for genuinely concurrent ones (review
+    // finding). render.ts sets this option to true whenever batchConcurrency
+    // > 1; verify that gate actually prevents arming.
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
+    await renderLocal("/tmp/project", "/tmp/out.mp4", {
+      ...baseOptions,
+      disableDeParallelRouterTrial: true,
+    });
+    expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
+    expect(configState.writeConfigCalls).toHaveLength(0);
+  });
+
   it("does not override an env var the user set between two renders in the same process", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     producerState.executeImpl = async (job) => {
       job.perfSummary = {
         resolution: { width: 100, height: 100 },
@@ -662,7 +759,11 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
   });
 
   it("caps exposure at DE_PARALLEL_ROUTER_TRIAL_MAX_RENDERS even when the router never reverts", async () => {
-    configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
+    configState.config = {
+      telemetryEnabled: true,
+      deParallelRouterTrialFired: false,
+      telemetryNoticeShown: true,
+    };
     producerState.executeImpl = async (job) => {
       job.perfSummary = {
         resolution: { width: 100, height: 100 },