cli: make ocx restart complete transient failures instead of failing fast

docs-site/src/content/docs/reference/cli/lifecycle.md

@@ -115,6 +115,23 @@ If a live listener cannot be attested to a runtime PID (including a pre-update p
 closed without an `ensure` or stop/start fallback. After confirming ownership, use `ocx stop` then
 `ocx start` for a standalone proxy. For a service-managed proxy, use `ocx stop` followed by
 `ocx service start` so supervision is restored.
+One invocation is the whole transaction: transient races are retried inside it, so a failed
+start is attempted again and an uncertain discovery round is re-observed instead of failing
+the command. When an accepted restart never publishes a replacement, the command re-observes
+once before giving up — a proxy that crashed mid-restart reads absent and is started fresh,
+while a replacement that landed just past the deadline still proves success. A live target is
+never stopped to make room, so a stale-but-listening process can not be replaced by a second
+proxy racing it for the port.
+Every failed start attempt is followed by a beat and a strong re-observation before the
+next attempt: a child that was just launched may still be binding, and post-health steps
+may have thrown on an already-serving proxy. A live reading means
+no second start is spawned: a clean refusal then attests success, while a throw
+propagates as a start failure because post-health work failed on a serving process.
+Only confirmed absence earns a retry. While the previous PID is still live, the
+confirmation window keeps polling for a replacement inside the reserve instead of
+failing at once. — a second proxy is never spawned next to a live one.
+The replacement wait ends early enough that the confirmation window still fits inside the
+overall observation deadline.
 
 Port recovery after stop or update respects a failed OCX process check even when the PID was
 recorded before shutdown. A rejected live holder is left running and prevents TCP-row cleanup.

src/cli/index.ts

@@ -75,6 +75,7 @@ import { parseStartOptions, StartArgsError } from "./start-args";
 import {
   discoverStableProxyForRestart,
   isProxyReplacement,
+  pollReplacementDeparture,
   runProxyRestart,
   runTrayProxyStart,
   type ProxyRestartLive,
@@ -861,6 +862,16 @@ async function handleTrayProxyStart(existingIsSuccess = true): Promise<boolean>
 
 const PROXY_RESTART_OBSERVE_MS = MEMORY_DRAIN_RESTART_MS + REPLACEMENT_READY_TIMEOUT_MS + 15_000;
 
+/**
+ * Confirmation budget carved INSIDE the observe deadline above, never added to it:
+ * the replacement wait ends this much early so the post-failure re-observation still
+ * fits. A late replacement inside the reserve still proves success through the
+ * re-observation below instead of being cut off by the wait.
+ */
+const RESTART_REOBSERVE_RESERVE_MS = 10_000;
+/** Upper bound of the post-failure confirmation window. */
+const RESTART_REOBSERVE_WINDOW_MS = 5_000;
+
 async function waitForProxyReplacement(
   previous: ProxyRestartLive,
   deadlineAt: number,
@@ -913,7 +924,29 @@ async function handleProxyRestart(
     }),
     startWhenStopped,
     requestInPlaceRestart: previous => requestBoundSystemRestart(previous, deadlineAt),
-    waitForReplacement: previous => waitForProxyReplacement(previous, deadlineAt),
+    // The replacement wait ends RESTART_REOBSERVE_RESERVE_MS early so the confirmation
+    // below still fits inside the overall deadline; the total never grows past it.
+    waitForReplacement: previous => waitForProxyReplacement(previous, deadlineAt - RESTART_REOBSERVE_RESERVE_MS),
+    // Leftover budget only: with nothing left the window reports uncertain and the
+    // transaction fails closed instead of overrunning the deadline. While budget
+    // remains, a still-live old PID keeps being polled — a replacement that lands
+    // late in the reserve is still attested instead of being cut off by the wait.
+    reobserveAfterReplacement: previous => {
+      const windowMs = Math.min(RESTART_REOBSERVE_WINDOW_MS, deadlineAt - Date.now());
+      if (windowMs < 1_500) {
+        return Promise.resolve({
+          status: "uncertain",
+          error: new Error("restart_reobserve_window_exhausted"),
+        } as const);
+      }
+      const end = Date.now() + windowMs;
+      const observe = () => discoverStableProxyForRestart({
+        findLive: () => findLiveProxy({ deadlineAt: end, attempts: 2, acceptPackageTreeFenced: true }),
+        waitBetweenChecks: () => Bun.sleep(250),
+        expired: () => Date.now() >= end,
+      });
+      return pollReplacementDeparture(observe, previous, () => Date.now() + 750 < end, () => Bun.sleep(250));
+    },
   });
   if (!result.ok) reportRestartFailure(result);
   process.exitCode = result.ok ? 0 : 1;

src/cli/tray-proxy.ts

@@ -53,6 +53,39 @@ export interface ProxyRestartIo {
     previous: ProxyRestartLive,
   ) => ProxyRestartRequestOutcome | Promise<ProxyRestartRequestOutcome>;
   waitForReplacement: (previous: ProxyRestartLive) => Promise<ProxyRestartLive | null>;
+  /** Pause between start attempts; defaults to a short sleep. Tests pass a recorder. */
+  waitBetweenAttempts?: () => Promise<void>;
+  /**
+   * Strong re-observation after a missed replacement. Defaults to `findLive`, but the
+   * production wiring passes a fresh bounded window: by the time the replacement wait
+   * expires, the shared observe deadline has expired too, so reusing it would answer
+   * `uncertain` forever and the crash-recovery path could never run.
+   */
+  reobserveAfterReplacement?: (previous: ProxyRestartLive) => Promise<ProxyRestartDiscovery>;
+}
+
+/**
+ * Poll for a departed predecessor inside a bounded confirmation window. Each round is
+ * already a strong observation (the caller supplies it); this only decides whether
+ * another round still fits: absent and valid-replacement verdicts return immediately,
+ * while the still-live old PID (or another uncertain round) keeps polling until the
+ * budget check refuses. A replacement that lands late in the window is still attested
+ * instead of being cut off by the wait that reserved the window.
+ */
+export async function pollReplacementDeparture(
+  observe: () => Promise<ProxyRestartDiscovery>,
+  previous: ProxyRestartLive,
+  shouldContinue: () => boolean,
+  wait: () => Promise<void>,
+): Promise<ProxyRestartDiscovery> {
+  for (;;) {
+    const round = await observe();
+    if (round.status === "absent") return round;
+    if (round.status === "live" && isProxyReplacement(previous, round.live)) return round;
+    if (round.status === "uncertain") return round;
+    if (!shouldContinue()) return round;
+    await wait();
+  }
 }
 
 /**
@@ -145,27 +178,100 @@ export async function runTrayProxyStart(io: TrayProxyStartIo): Promise<boolean>
  * child and lets ordinary /api/stop restore native routing between the two halves.
  * When no proxy is live there is nothing to recycle, so restart degrades to the
  * caller's normal start path.
+ *
+ * One invocation is the whole transaction: transient races are retried inside it so the
+ * operator never has to re-run the command by hand. An uncertain discovery round is
+ * re-observed, a failed start is retried, and a replacement that never arrives triggers
+ * one strong re-observation (a crashed proxy reads absent and is started fresh; a late
+ * replacement still proves success). A live target is never stopped to make room, so the
+ * no-stop/start-fallback invariant holds: absence is confirmed twice before anything starts.
+ */
+/**
+ * Start attempts per restart transaction. A service child that is still exiting, a port
+ * that has not been released yet, or a supervisor that has not re-armed all fail the
+ * first attempt and succeed on the next; an install that is actually broken still fails
+ * fast enough to read the error. A deliberate operator refusal (`"skipped"`) never retries.
  */
+const RESTART_START_ATTEMPTS = 3;
+
+/** Discovery rounds per restart transaction; an uncertain round is a transient race. */
+const RESTART_DISCOVERY_ATTEMPTS = 3;
+
+/**
+ * Start leg with phase evidence. A bare boolean cannot tell a pre-launch refusal from
+ * a child that was launched but is not healthy yet, or from post-health steps that
+ * threw on an already-live proxy — and blindly respawning on any of those risks a
+ * second proxy racing the first for the port. So every failed attempt is followed by a
+ * beat and a strong re-observation: a live proxy attests success (no second start),
+ * and only confirmed absence earns another attempt.
+ */
+async function startRestartedProxy(
+  io: ProxyRestartIo,
+  waitBetweenAttempts: () => Promise<void>,
+): Promise<ProxyRestartResult> {
+  let lastError: unknown;
+  let sawError = false;
+  for (let attempt = 0; attempt < RESTART_START_ATTEMPTS; attempt++) {
+    let threw: unknown;
+    let didThrow = false;
+    try {
+      const started = await io.startWhenStopped();
+      if (started === "skipped") return { ok: true, mode: "skipped" };
+      if (started) return { ok: true, mode: "started" };
+      sawError = false;
+    } catch (error) {
+      didThrow = true;
+      threw = error;
+      sawError = true;
+      lastError = error;
+    }
+    // Beat first: a child that was just launched may still be binding, and post-health
+    // steps may have thrown on an already-live proxy. Re-observe before deciding: live
+    // attests success (never a second start), only confirmed absence earns a retry.
+    await waitBetweenAttempts();
+    let recheck: ProxyRestartDiscovery;
+    try {
+      recheck = await io.findLive();
+    } catch {
+      continue;
+    }
+    // Runtime-attested only: a config-sourced observation is not proof a proxy serves,
+    // so it keeps the start failure instead of reporting a success nobody earned.
+    // A throw on an attested-live proxy is different from a clean refusal: post-health
+    // work failed on a serving process, so the error propagates instead of converting
+    // to success — still without spawning again.
+    const attested = recheck.status === "live"
+      && recheck.live.pid !== null
+      && recheck.live.source === "runtime";
+    if (attested && didThrow) return { ok: false, phase: "start", error: threw };
+    if (attested) return { ok: true, mode: "started" };
+  }
+  return sawError
+    ? { ok: false, phase: "start", error: lastError }
+    : { ok: false, phase: "start" };
+}
+
 export async function runProxyRestart(io: ProxyRestartIo): Promise<ProxyRestartResult> {
-  let discovery: ProxyRestartDiscovery;
-  try {
-    discovery = await io.findLive();
-  } catch (error) {
-    return { ok: false, phase: "request", error };
+  const waitBetweenAttempts = io.waitBetweenAttempts ?? (() => Bun.sleep(500));
+  // An uncertain round is a transient appear/vanish race, not a verdict: re-observe a
+  // few times before failing, so one invocation survives a supervisor mid-handoff.
+  let discovery: ProxyRestartDiscovery = { status: "uncertain", error: new Error("restart_discovery_no_attempt") };
+  for (let attempt = 0; attempt < RESTART_DISCOVERY_ATTEMPTS; attempt++) {
+    if (attempt > 0) await waitBetweenAttempts();
+    try {
+      discovery = await io.findLive();
+    } catch (error) {
+      discovery = { status: "uncertain", error };
+    }
+    if (discovery.status !== "uncertain") break;
   }
 
   if (discovery.status === "uncertain") {
     return { ok: false, phase: "request", error: discovery.error };
   }
 
   if (discovery.status === "absent") {
-    try {
-      const started = await io.startWhenStopped();
-      if (started === "skipped") return { ok: true, mode: "skipped" };
-      return started ? { ok: true, mode: "started" } : { ok: false, phase: "start" };
-    } catch (error) {
-      return { ok: false, phase: "start", error };
-    }
+    return startRestartedProxy(io, waitBetweenAttempts);
   }
 
   const previous = discovery.live;
@@ -187,13 +293,31 @@ export async function runProxyRestart(io: ProxyRestartIo): Promise<ProxyRestartR
     return { ok: false, phase: "request", error: request.error };
   }
 
+  let replacement: ProxyRestartLive | null;
   try {
-    const replacement = await io.waitForReplacement(previous);
-    if (replacement) return { ok: true, mode: "restarted", live: replacement };
-    return request.accepted
-      ? { ok: false, phase: "replacement" }
-      : { ok: false, phase: "request", error: request.error };
+    replacement = await io.waitForReplacement(previous);
   } catch (error) {
     return { ok: false, phase: "replacement", error };
   }
+  if (replacement) return { ok: true, mode: "restarted", live: replacement };
+  // The replacement never arrived: re-observe once instead of failing blind. A proxy
+  // that crashed mid-restart reads absent (safe to start fresh — nothing live can race
+  // the bind); a replacement that landed just past the deadline still proves success;
+  // the same PID or another uncertain round fails closed exactly as before. A live
+  // target is never stopped to make room: the no-stop/start-fallback invariant holds.
+  const reobserveHook = io.reobserveAfterReplacement;
+  const reobserve = reobserveHook ? () => reobserveHook(previous) : io.findLive;
+  let again: ProxyRestartDiscovery;
+  try {
+    again = await reobserve();
+  } catch (error) {
+    return { ok: false, phase: "replacement", error };
+  }
+  if (again.status === "absent") return startRestartedProxy(io, waitBetweenAttempts);
+  if (again.status === "live" && isProxyReplacement(previous, again.live)) {
+    return { ok: true, mode: "restarted", live: again.live };
+  }
+  return request.accepted
+    ? { ok: false, phase: "replacement" }
+    : { ok: false, phase: "request", error: request.error };
 }