fix(cli): don't override exit code after artifact validated

The render command's post-artifact-validated cleanup (telemetry flush,
feedback prompt, worker/browser teardown, stray promise rejections) can
throw AFTER the producer has committed a valid MP4 to disk. Field signal
ts=1784169760, ts=1784171150, ts=1784172467 (all win32/x64, CLI 0.7.58,
ffmpeg=no, 1080x1920): ffprobe + visual QA confirmed the outputs are
valid, but the CLI exited 1 after the terminal "artifact validated" log
with no final error message.

Introduce a `renderSucceeded` sentinel that flips after `executeRenderJob`
(or the Docker child render) resolves cleanly. From that point on:

  - Post-render steps in the render command (trackRenderMetrics,
    printRenderComplete, warnIfWebmAlphaDropped, maybePromptRenderFeedback)
    run through `runPostRenderStep`/`runPostRenderStepAsync` guards that
    swallow throws, log a compact warning to stderr, and sanitize a stray
    `process.exitCode` back to 0.

  - The CLI's top-level `uncaughtException` handler logs the throw for
    diagnosis but exits 0 instead of 1 when the render already succeeded.

  - The CLI's `unhandledRejection` handler stops flipping `commandFailed`
    (which drove the success:false telemetry field) when the render
    already succeeded.

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

🤖 Generated with [Claude Code](https://claude.com/claude-code)

— Via

packages/cli/src/cli.ts

@@ -103,6 +103,7 @@ import { defineCommand, runMain } from "citty";
 import type { ArgsDef, CommandDef } from "citty";
 import { getRunId } from "./telemetry/runId.js";
 import { reportCommandFailure, trackCommandFailures } from "./utils/command-failure-tracking.js";
+import { isRenderSucceeded } from "./utils/render-success-state.js";
 
 const isHelp = process.argv.includes("--help") || process.argv.includes("-h");
 
@@ -299,6 +300,30 @@ process.on("uncaughtException", (error) => {
     commandFailed = true;
     process.exit(0);
   }
+  // Post-artifact-validated shutdown throws (worker teardown, browser
+  // process cleanup, stray subprocess stream error) must not turn a valid
+  // render into an exit-1 "no final error message" failure. The render
+  // command sets `renderSucceeded` right after the producer resolves and
+  // the artifact is committed — from that point on, every throw here is
+  // a teardown artifact, not a render failure. Field signals:
+  //   ts=1784169760, ts=1784171150, ts=1784172467 (all win32/x64, CLI
+  //   0.7.58, ffmpeg=no, 1080x1920, valid MP4s on disk).
+  if (isRenderSucceeded()) {
+    // Log to stderr for diagnosis but do NOT flip commandFailed and do NOT
+    // exit non-zero. The render is valid.
+    process.stderr.write(
+      `  [hyperframes] Post-render uncaughtException (render already succeeded): ${error.message}\n`,
+    );
+    _trackCliError?.({
+      error_name: error.name,
+      error_message: error.message,
+      stack_trace: error.stack,
+      command,
+      kind: "uncaught_exception",
+    });
+    _flushSync?.();
+    process.exit(0);
+  }
   commandFailed = true;
   _trackCliError?.({
     error_name: error.name,
@@ -315,8 +340,25 @@ process.on("uncaughtException", (error) => {
 // running if the rejection is non-fatal (e.g. a fire-and-forget promise).
 // The exit handler above will still fire with the real exit code.
 process.on("unhandledRejection", (reason) => {
-  commandFailed = true;
   const error = reason instanceof Error ? reason : new Error(String(reason));
+  // Same rationale as the uncaughtException branch above: a stray promise
+  // rejection during post-artifact-validated cleanup must not mark a valid
+  // render as failed. `commandFailed` gates the success:true telemetry
+  // field — keep it false when the render actually succeeded.
+  if (isRenderSucceeded()) {
+    process.stderr.write(
+      `  [hyperframes] Post-render unhandledRejection (render already succeeded): ${error.message}\n`,
+    );
+    _trackCliError?.({
+      error_name: error.name,
+      error_message: error.message,
+      stack_trace: error.stack,
+      command,
+      kind: "unhandled_rejection",
+    });
+    return;
+  }
+  commandFailed = true;
   _trackCliError?.({
     error_name: error.name,
     error_message: error.message,

packages/cli/src/commands/render.ts

@@ -81,6 +81,11 @@ import { runEnvironmentChecks } from "../browser/preflight.js";
 import { detectH264EncoderMode } from "../browser/ffmpeg.js";
 import { chromeLaunchRemediation } from "../browser/linuxDeps.js";
 import { killOrphanedProcesses } from "../utils/orphanCleanup.js";
+import {
+  markRenderSucceeded,
+  runPostRenderStep,
+  runPostRenderStepAsync,
+} from "../utils/render-success-state.js";
 import type { ProducerLogger, RenderJob } from "@hyperframes/producer";
 import {
   MAX_VP9_CPU_USED,
@@ -1413,7 +1418,14 @@ async function renderDocker(
 
   const elapsed = Date.now() - startTime;
 
+  // Docker child exited 0 → the containerized producer already validated
+  // AND committed the artifact. Mirror renderLocal's post-success guarantee
+  // so any late throw here (telemetry flush, feedback prompt) cannot flip
+  // the exit code.
+  markRenderSucceeded();
+
   // Track metrics (no job object available from Docker — use a minimal stub)
+  runPostRenderStep("trackRenderComplete", () =>
     trackRenderComplete({
       durationMs: elapsed,
       fps: fpsToNumber(options.fps),
@@ -1423,13 +1435,18 @@ async function renderDocker(
       gpu: options.gpu,
       authoringSkill: options.authoringSkill,
       ...getMemorySnapshot(),
-  });
+    }),
+  );
 
   // ponytail: Docker runs the producer in a child process, so no perfSummary is
   // threaded back here; the summary shows render time only (never a wrong video
   // length). Probe the output with ffprobe if a duration figure is wanted here.
-  printRenderComplete(outputPath, elapsed, options.quiet);
+  runPostRenderStep("printRenderComplete", () =>
-  warnIfWebmAlphaDropped(outputPath, options.format, options.quiet);
+    printRenderComplete(outputPath, elapsed, options.quiet),
+  );
+  runPostRenderStep("warnIfWebmAlphaDropped", () =>
+    warnIfWebmAlphaDropped(outputPath, options.format, options.quiet),
+  );
   if (options.exitAfterComplete) scheduleRenderProcessExit();
   return { renderTimeMs: elapsed };
 }
@@ -1559,6 +1576,13 @@ export async function renderLocal(
     );
   }
 
+  // Render resolved without throwing → producer's `artifact validated`
+  // checkpoint fired AND the artifact was committed to disk. From this
+  // point on, ANY thrown teardown error must not be allowed to override
+  // the exit code. Field signal ts=1784169760 / ts=1784171150 / ts=1784172467
+  // (win32/x64, CLI 0.7.58): valid MP4 on disk, exited 1 with no error print.
+  markRenderSucceeded();
+
   maybeConsumeDeParallelRouterTrial(deParallelRouterTrialArmed, job, options.quiet);
   const elapsed = Date.now() - startTime;
   if (job.outcome === "completed_with_warnings") {
@@ -1566,20 +1590,26 @@ export async function renderLocal(
       console.warn(c.warn(`  [${warning.code}] ${warning.message}`));
     }
   }
-  trackRenderMetrics(job, elapsed, options, false);
+  runPostRenderStep("trackRenderMetrics", () => trackRenderMetrics(job, elapsed, options, false));
+  runPostRenderStep("printRenderComplete", () =>
     printRenderComplete(
       outputPath,
       elapsed,
       options.quiet,
       job.perfSummary?.compositionDurationSeconds,
       job.perfSummary?.totalFrames,
+    ),
+  );
+  runPostRenderStep("warnIfWebmAlphaDropped", () =>
+    warnIfWebmAlphaDropped(outputPath, options.format, options.quiet),
   );
-  warnIfWebmAlphaDropped(outputPath, options.format, options.quiet);
   if (!options.skipFeedback) {
-    await maybePromptRenderFeedback({
+    await runPostRenderStepAsync("maybePromptRenderFeedback", () =>
+      maybePromptRenderFeedback({
         renderDurationMs: elapsed,
         quiet: options.quiet,
-    });
+      }),
+    );
   }
   if (options.exitAfterComplete) scheduleRenderProcessExit();
   const durationMs = job.perfSummary

packages/cli/src/utils/render-success-state.test.ts

@@ -0,0 +1,153 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import {
+  _resetRenderSuccessForTests,
+  isRenderSucceeded,
+  markRenderSucceeded,
+  runPostRenderStep,
+  runPostRenderStepAsync,
+} from "./render-success-state.js";
+
+describe("render-success-state flag", () => {
+  afterEach(() => {
+    _resetRenderSuccessForTests();
+  });
+
+  it("starts unset", () => {
+    expect(isRenderSucceeded()).toBe(false);
+  });
+
+  it("flips true after markRenderSucceeded()", () => {
+    markRenderSucceeded();
+    expect(isRenderSucceeded()).toBe(true);
+  });
+
+  it("stays true across repeated calls (idempotent)", () => {
+    markRenderSucceeded();
+    markRenderSucceeded();
+    markRenderSucceeded();
+    expect(isRenderSucceeded()).toBe(true);
+  });
+
+  it("reset restores the initial state", () => {
+    markRenderSucceeded();
+    _resetRenderSuccessForTests();
+    expect(isRenderSucceeded()).toBe(false);
+  });
+});
+
+describe("runPostRenderStep", () => {
+  const originalExitCode = process.exitCode;
+
+  afterEach(() => {
+    process.exitCode = originalExitCode;
+    _resetRenderSuccessForTests();
+  });
+
+  it("runs the step and returns normally on success", () => {
+    const sink = vi.fn();
+    const step = vi.fn();
+    runPostRenderStep("noop", step, sink);
+    expect(step).toHaveBeenCalledTimes(1);
+    expect(sink).not.toHaveBeenCalled();
+  });
+
+  it("swallows a thrown error and reports it to the sink", () => {
+    const sink = vi.fn();
+    const err = new Error("teardown blew up");
+    runPostRenderStep(
+      "trackRenderMetrics",
+      () => {
+        throw err;
+      },
+      sink,
+    );
+    expect(sink).toHaveBeenCalledTimes(1);
+    const msg = sink.mock.calls[0]?.[0] as string;
+    expect(msg).toContain("trackRenderMetrics");
+    expect(msg).toContain("teardown blew up");
+    expect(msg).toContain("render already succeeded");
+  });
+
+  it("sanitizes a stray non-zero process.exitCode back to 0", () => {
+    // Regression: a cleanup step that sets process.exitCode=1 (or a helper it
+    // calls that does) must not leave the CLI exiting 1 after a successful
+    // render. Field signal ts=1784169760 / ts=1784171150 / ts=1784172467.
+    process.exitCode = 1;
+    runPostRenderStep(
+      "printRenderComplete",
+      () => {
+        throw new Error("stat threw");
+      },
+      vi.fn(),
+    );
+    expect(process.exitCode).toBe(0);
+  });
+
+  it("does not touch process.exitCode when the step succeeds", () => {
+    process.exitCode = 1;
+    runPostRenderStep("ok", () => undefined, vi.fn());
+    expect(process.exitCode).toBe(1);
+  });
+
+  it("does not touch process.exitCode when it was already 0", () => {
+    process.exitCode = 0;
+    runPostRenderStep(
+      "warnIfWebmAlphaDropped",
+      () => {
+        throw new Error("stat missing");
+      },
+      vi.fn(),
+    );
+    expect(process.exitCode).toBe(0);
+  });
+
+  it("stringifies a non-Error throw value", () => {
+    const sink = vi.fn();
+    runPostRenderStep(
+      "misc",
+      () => {
+        throw "just a string";
+      },
+      sink,
+    );
+    const msg = sink.mock.calls[0]?.[0] as string;
+    expect(msg).toContain("just a string");
+  });
+});
+
+describe("runPostRenderStepAsync", () => {
+  const originalExitCode = process.exitCode;
+
+  afterEach(() => {
+    process.exitCode = originalExitCode;
+    _resetRenderSuccessForTests();
+  });
+
+  it("awaits the async step and returns normally on success", async () => {
+    const sink = vi.fn();
+    const step = vi.fn(() => Promise.resolve());
+    await runPostRenderStepAsync("feedback", step, sink);
+    expect(step).toHaveBeenCalledTimes(1);
+    expect(sink).not.toHaveBeenCalled();
+  });
+
+  it("swallows a rejected promise and reports it to the sink", async () => {
+    const sink = vi.fn();
+    const err = new Error("feedback prompt crashed");
+    await runPostRenderStepAsync("maybePromptRenderFeedback", () => Promise.reject(err), sink);
+    expect(sink).toHaveBeenCalledTimes(1);
+    const msg = sink.mock.calls[0]?.[0] as string;
+    expect(msg).toContain("maybePromptRenderFeedback");
+    expect(msg).toContain("feedback prompt crashed");
+  });
+
+  it("sanitizes process.exitCode back to 0 on async failure", async () => {
+    process.exitCode = 1;
+    await runPostRenderStepAsync(
+      "maybePromptRenderFeedback",
+      () => Promise.reject(new Error("stdin closed")),
+      vi.fn(),
+    );
+    expect(process.exitCode).toBe(0);
+  });
+});

packages/cli/src/utils/render-success-state.ts

@@ -0,0 +1,95 @@
+/**
+ * Shared render-success sentinel + post-artifact-validated cleanup guards.
+ *
+ * Read by the top-level CLI process handlers (uncaughtException /
+ * unhandledRejection) to sanitize the exit code when a post-artifact-validated
+ * cleanup step throws.
+ *
+ * Set by the render command AFTER `executeRenderJob` (or the Docker child
+ * render) resolves cleanly — the point at which the artifact has been
+ * validated AND committed to disk. Any throw after this point (worker
+ * teardown, browser shutdown, telemetry flush, feedback prompt, stray
+ * promise rejection) must not turn a valid render into an exit-1
+ * "no final error message" failure.
+ *
+ * Field signal (all win32/x64, CLI 0.7.58, ffmpeg=no, 1080x1920 renders):
+ *   - ts=1784169760 — 6-worker capture retried down after Runtime.evaluate
+ *     timeout, completed all 1260 frames, printed 'artifact validated',
+ *     exited 1 with no final error message. Output MP4 valid on disk.
+ *   - ts=1784171150 — full REPRO command provided; identical shape.
+ *   - ts=1784172467 — `--workers 2`, identical shape.
+ * All three: ffprobe + visual QA confirmed the output was valid; the CLI
+ * still exited 1 after the terminal "artifact validated" checkpoint.
+ */
+
+let renderSucceeded = false;
+
+/**
+ * Called by the render command after the producer's `executeRenderJob` (or
+ * the Docker child) resolves cleanly. From this point on, any thrown
+ * teardown error must not be allowed to override the exit code.
+ */
+export function markRenderSucceeded(): void {
+  renderSucceeded = true;
+}
+
+/**
+ * Read by cli.ts process handlers to decide whether a late throw is fatal.
+ * When true, the handlers log the throw at warn level (so it's still visible
+ * for diagnosis) but do not surface it as an exit-1 failure.
+ */
+export function isRenderSucceeded(): boolean {
+  return renderSucceeded;
+}
+
+/** Test-only reset. Not exported from the package. */
+export function _resetRenderSuccessForTests(): void {
+  renderSucceeded = false;
+}
+
+type PostRenderErrorSink = (message: string) => void;
+
+const defaultErrorSink: PostRenderErrorSink = (message) => {
+  process.stderr.write(`${message}\n`);
+};
+
+/**
+ * Run a post-artifact-validated cleanup step so a throw cannot flip the CLI
+ * exit code. `markRenderSucceeded()` MUST have been called first — this
+ * helper is only safe on the success path where the artifact is already
+ * committed to disk. Logs a compact warning to stderr, resets a stray
+ * `process.exitCode` back to 0, and swallows the error.
+ */
+export function runPostRenderStep(
+  label: string,
+  fn: () => void,
+  sink: PostRenderErrorSink = defaultErrorSink,
+): void {
+  try {
+    fn();
+  } catch (err) {
+    reportPostRenderStepFailure(label, err, sink);
+  }
+}
+
+export async function runPostRenderStepAsync(
+  label: string,
+  fn: () => Promise<void>,
+  sink: PostRenderErrorSink = defaultErrorSink,
+): Promise<void> {
+  try {
+    await fn();
+  } catch (err) {
+    reportPostRenderStepFailure(label, err, sink);
+  }
+}
+
+function reportPostRenderStepFailure(label: string, err: unknown, sink: PostRenderErrorSink): void {
+  const message = err instanceof Error ? err.message : String(err);
+  sink(`  [hyperframes] Post-render step '${label}' failed (render already succeeded): ${message}`);
+  // Guard against the failing step (or something it triggered) setting a
+  // non-zero exitCode. The render succeeded → the CLI must exit 0.
+  if (process.exitCode !== undefined && process.exitCode !== 0) {
+    process.exitCode = 0;
+  }
+}