fix(producer): close orphaned probe session before verify-triggered retries

On a parallel-capture disk-verify or streaming-drain breach, the outer
catch cleared probeSession without first closing the still-owned session,
orphaning the probe Chrome process precisely when the retry was recovering
from GPU/memory pressure. Introduce closeOrphanedProbeForRetry so both
retry catches close the session (with defensive .catch that logs on close
error) before releasing the reference, and cover it with a focused unit
test asserting closure-before-clear and the swallow-and-warn behaviour.

Addresses Magi's REQUEST_CHANGES on heygen-com#2749; also closes Rames' sibling
concern at the streaming-retry path (renderOrchestrator.ts:3093).

— Via

Author: vanceingalls

packages/producer/src/services/renderOrchestrator.test.ts

@@ -17,6 +17,7 @@ vi.mock("@hyperframes/engine", async (importOriginal) => {
 import {
   buildMissingFrameRetryBatches,
   captureAttemptMadeProgress,
+  closeOrphanedProbeForRetry,
   describeMemoryExhaustion,
   executeDiskCaptureWithAdaptiveRetry,
   collectVideoMetadataHints,
@@ -2130,3 +2131,63 @@ describe("shouldStreamParallelCapture (non-DE parallel streaming router)", () =>
     expect(shouldStreamParallelCapture({ ...eligible, layeredOrEffectRoute: true })).toBe(false);
   });
 });
+
+describe("closeOrphanedProbeForRetry (probe cleanup before verify-triggered retry)", () => {
+  // Enough of a CaptureSession stand-in to exercise the closer path — the
+  // helper never inspects the object; it just hands it to the injected closer.
+  const stubSession = { browserConsoleBuffer: [] } as unknown as Parameters<
+    typeof closeOrphanedProbeForRetry
+  >[0];
+
+  it("hands the still-owned probe to the closer before the caller clears it", async () => {
+    const closer = vi.fn(async () => {});
+    const log = { warn: vi.fn() };
+
+    await closeOrphanedProbeForRetry(stubSession, closer, log, "streaming");
+
+    expect(closer).toHaveBeenCalledTimes(1);
+    expect(closer).toHaveBeenCalledWith(stubSession);
+    expect(log.warn).not.toHaveBeenCalled();
+  });
+
+  it("swallows a close failure with a warn so the caller's retry can proceed", async () => {
+    const closer = vi.fn(async () => {
+      throw new Error("chrome zombie");
+    });
+    const log = { warn: vi.fn() };
+
+    await expect(
+      closeOrphanedProbeForRetry(stubSession, closer, log, "disk verify"),
+    ).resolves.toBeUndefined();
+
+    expect(closer).toHaveBeenCalledTimes(1);
+    expect(log.warn).toHaveBeenCalledTimes(1);
+    const [message, meta] = log.warn.mock.calls[0];
+    expect(message).toContain("disk verify");
+    expect((meta as { error: string }).error).toBe("chrome zombie");
+  });
+
+  it("preserves the retry context in the warn message so the audit trail names which retry path leaked", async () => {
+    const closer = vi.fn(async () => {
+      throw new Error("session already closed");
+    });
+    const log = { warn: vi.fn() };
+
+    await closeOrphanedProbeForRetry(stubSession, closer, log, "streaming");
+
+    expect(log.warn.mock.calls[0][0]).toContain("streaming");
+    expect(log.warn.mock.calls[0][0]).not.toContain("disk verify");
+  });
+
+  it("stringifies non-Error rejections so the log entry still names the cause", async () => {
+    const closer = vi.fn(async () => Promise.reject("string-only rejection"));
+    const log = { warn: vi.fn() };
+
+    await closeOrphanedProbeForRetry(stubSession, closer, log, "streaming");
+
+    expect(log.warn).toHaveBeenCalledTimes(1);
+    expect((log.warn.mock.calls[0][1] as { error: string }).error).toBe(
+      "string-only rejection",
+    );
+  });
+});

packages/producer/src/services/renderOrchestrator.ts

@@ -1468,6 +1468,32 @@ export function shouldRetryViaPinnedFallback(args: {
   return args.deWorkerInversion === "inverted" || args.deParallelRouter === "routed";
 }
 
+/**
+ * When a self-verify (or pinned-fallback) retry is triggered mid-capture, the
+ * caller may still hold a live probe session that the failed stage was passed
+ * but did not (or could not) close in its own `finally` before it threw. Left
+ * behind, that session's Chrome process orphans until the containing render
+ * exits — precisely when we are recovering from GPU/memory pressure and can
+ * least afford an unaccounted Chrome. Close it before the caller clears its
+ * reference; swallow any close error with a warn so the retry itself is never
+ * derailed by a shutdown hiccup.
+ */
+export async function closeOrphanedProbeForRetry(
+  probe: CaptureSession,
+  closer: (session: CaptureSession) => Promise<void>,
+  log: Pick<ProducerLogger, "warn">,
+  retryContext: string,
+): Promise<void> {
+  try {
+    await closer(probe);
+  } catch (closeErr) {
+    log.warn(
+      `[Render] probe close before ${retryContext} retry failed; continuing with retry`,
+      { error: closeErr instanceof Error ? closeErr.message : String(closeErr) },
+    );
+  }
+}
+
 /**
  * Parallel-streaming router for NON-drawElement capture (screenshot on
  * macOS/Windows/forced-screenshot, BeginFrame on Linux): should this
@@ -3090,7 +3116,16 @@ async function executeRenderPipeline(input: {
             deWorkerInversion,
             deParallelRouter,
           });
-          probeSession = null;
+          // Streaming stage aims to close the probe in its own finally; if it
+          // threw before doing so, the Chrome process would orphan through the
+          // pinned-fallback retry. Close defensively before we release the
+          // reference — see closeOrphanedProbeForRetry.
+          if (probeSession) {
+            lastBrowserConsole = probeSession.browserConsoleBuffer;
+            const orphaned = probeSession;
+            probeSession = null;
+            await closeOrphanedProbeForRetry(orphaned, closeCaptureSession, log, "streaming");
+          }
           if (failedRouting === "worker_inversion") {
             // The inversion bet on drawElement and lost — re-render on the
             // pre-inversion parallel screenshot path instead of single-worker
@@ -3243,7 +3278,17 @@ async function executeRenderPipeline(input: {
           resetCaptureAttemptProgress(job);
           dedupPerfs.length = 0;
           cfg.useDrawElement = false;
-          probeSession = null;
+          // Same shape as the streaming retry above: `runCaptureStage` was
+          // passed the probe and threw before it could close it, so we must
+          // release the Chrome process ourselves before starting the
+          // screenshot-baseline retry — otherwise it orphans until render
+          // exit. See closeOrphanedProbeForRetry.
+          if (probeSession) {
+            lastBrowserConsole = probeSession.browserConsoleBuffer;
+            const orphaned = probeSession;
+            probeSession = null;
+            await closeOrphanedProbeForRetry(orphaned, closeCaptureSession, log, "disk verify");
+          }
           capturePlan = replanAfterFailure(capturePlan, { kind: "draw_element_verification" });
           syncCapturePlan();
           updateCaptureObservability({