fix(sidecar): harden recovery convergence

Author: PerishCode

apps/packaged/src/index.ts

@@ -34,7 +34,7 @@ import {
   waitForLauncherAfterQuit,
 } from "./launcher-after-quit.js";
 import {
-  clearPackagedLauncherDelegatedAttempt,
+  clearPackagedLauncherDelegatedExitAttempt,
   confirmPackagedLauncherRuntime,
   resolvePackagedLauncherRuntime,
 } from "./launcher-runtime.js";
@@ -125,9 +125,12 @@ async function main(): Promise<void> {
     logger: console,
     paths: initialPaths,
   });
-  if (existingDesktop.action === "exit" && existingDesktop.reason === "existing-focused") {
-    await clearPackagedLauncherDelegatedAttempt(namespaceConfig, initialPaths, delegated);
-  }
+  await clearPackagedLauncherDelegatedExitAttempt(
+    namespaceConfig,
+    initialPaths,
+    delegated,
+    existingDesktop,
+  );
   if (exitPackagedLauncherForExistingDesktop(existingDesktop, (code) => app.exit(code))) {
     return;
   }

apps/packaged/src/launcher-runtime.ts

@@ -25,6 +25,7 @@ import {
 import { releaseChannelFromNamespace, releaseChannelFromVersion } from "@open-design/release";
 
 import type { PackagedConfig, PackagedWebOutputMode, RawPackagedConfig } from "./config.js";
+import type { LauncherExistingDesktopGateResult } from "./launcher-after-quit.js";
 import type { PackagedNamespacePaths } from "./paths.js";
 
 type LauncherPayloadManifest = {
@@ -620,16 +621,17 @@ export async function recordPackagedLauncherRuntimeFailedAttempt(
 
 /**
  * Clear only the attempt pre-armed by the launcher that delegated this exact
- * payload pointer. A payload candidate can focus an already-running owner and
- * exit before resolving its own runtime; without this boundary the fresh
- * attempt would be misread as a failed generation on the next cold start.
+ * payload pointer whenever the delegated candidate exits without ownership.
+ * Whether focus succeeds or fails, leaving the fresh attempt behind would be
+ * misread as a failed generation on the next cold start.
  */
-export async function clearPackagedLauncherDelegatedAttempt(
+export async function clearPackagedLauncherDelegatedExitAttempt(
   config: PackagedConfig,
   paths: PackagedNamespacePaths,
   delegated: LauncherVersionPointer | null,
+  result: LauncherExistingDesktopGateResult,
 ): Promise<boolean> {
-  if (delegated == null) return false;
+  if (result.action !== "exit" || delegated == null) return false;
   const channel = inferLauncherChannel(config);
   const launcherPaths = resolveLauncherPaths({
     channel,

apps/packaged/tests/launcher-runtime.test.ts

@@ -11,7 +11,7 @@ import { describe, expect, it } from "vitest";
 
 import type { PackagedConfig } from "../src/config.js";
 import {
-  clearPackagedLauncherDelegatedAttempt,
+  clearPackagedLauncherDelegatedExitAttempt,
   confirmPackagedLauncherRuntime,
   type PackagedLauncherRuntime,
   resolvePackagedLauncherRuntime,
@@ -56,18 +56,26 @@ describe("resolvePackagedLauncherRuntime", () => {
       await mkdir(dirname(runtime.launcherPaths.attemptsPath), { recursive: true });
       await writeFile(runtime.launcherPaths.attemptsPath, `${JSON.stringify(attempt)}\n`);
 
-      await expect(clearPackagedLauncherDelegatedAttempt(config, paths, {
+      await expect(clearPackagedLauncherDelegatedExitAttempt(config, paths, {
+        generation: 2,
+        version: "1.2.3-beta.6",
+      }, { action: "continue", reason: "not-running" })).resolves.toBe(false);
+      await expect(readFile(runtime.launcherPaths.attemptsPath, "utf8")).resolves.toContain(
+        "1.2.3-beta.6",
+      );
+
+      await expect(clearPackagedLauncherDelegatedExitAttempt(config, paths, {
         generation: 1,
         version: "1.2.3-beta.5",
-      })).resolves.toBe(false);
+      }, { action: "exit", reason: "existing-focused" })).resolves.toBe(false);
       await expect(readFile(runtime.launcherPaths.attemptsPath, "utf8")).resolves.toContain(
         "1.2.3-beta.6",
       );
 
-      await expect(clearPackagedLauncherDelegatedAttempt(config, paths, {
+      await expect(clearPackagedLauncherDelegatedExitAttempt(config, paths, {
         generation: 2,
         version: "1.2.3-beta.6",
-      })).resolves.toBe(true);
+      }, { action: "exit", reason: "existing-focus-failed" })).resolves.toBe(true);
       await expect(readFile(runtime.launcherPaths.attemptsPath, "utf8")).rejects.toThrow();
     } finally {
       await rm(root, { force: true, recursive: true });

packages/sidecar/src/control/controller.ts

@@ -529,17 +529,20 @@ export function bootstrapControlPlane({
       throw new SidecarControlError("invalid-input", "sidecar existing launch mode must be adopt or replace");
     }
     if (existing === "adopt") {
+      let current: PrivateReadyDescriptor | null = null;
       try {
-        const current = await readCurrentDescriptor(
+        current = await readCurrentDescriptor(
           descriptor.identity,
           descriptor.roots,
           descriptor.projection,
         );
+      } catch (error) {
+        if (!(error instanceof SidecarControlError) || error.code !== "peer-unavailable") throw error;
+      }
+      if (current != null && processAlive(current.pid)) {
         const client = createClient<TMethods>(current);
         await client.probe();
         return adoptedLaunch({ peer: { client, descriptor: current }, stopTimeoutMs });
-      } catch (error) {
-        if (!(error instanceof SidecarControlError) || error.code !== "peer-unavailable") throw error;
       }
     } else {
       const converged = await stop(options.service, { graceMs: stopTimeoutMs });

packages/sidecar/tests/control-plane.test.ts

@@ -1,5 +1,5 @@
 import { spawn } from "node:child_process";
-import { mkdtemp, rm } from "node:fs/promises";
+import { access, mkdtemp, rm } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
@@ -346,6 +346,77 @@ describe("independent sidecar controller and body", () => {
     await expect(right.client.call("echo", { value: "right" })).resolves.toEqual({ value: "right" });
   });
 
+  it("does not spawn when an adopted identity exists but its peer is unprobeable", async () => {
+    const { roots, scope } = await createFixture();
+    const controller = createDemoController(scope, roots);
+    const holder = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], {
+      stdio: "ignore",
+      windowsHide: true,
+    });
+    await new Promise<void>((resolveSpawn, rejectSpawn) => {
+      holder.once("error", rejectSpawn);
+      holder.once("spawn", resolveSpawn);
+    });
+    const holderPid = holder.pid;
+    if (holderPid == null) throw new Error("unprobeable peer fixture did not report a pid");
+    cleanups.push(async () => {
+      if (holder.exitCode == null && holder.signalCode == null) holder.kill("SIGKILL");
+      await new Promise<void>((resolveExit) => {
+        if (holder.exitCode != null || holder.signalCode != null) resolveExit();
+        else holder.once("exit", () => resolveExit());
+      });
+    });
+
+    const metadata = createPrivateLaunchForTest({
+      projection: demoProjection,
+      roots,
+      scope,
+      service: "daemon",
+    });
+    await writePrivateReadyDescriptorForTest(metadata, holderPid);
+    const spawnMarker = join(roots.runtimeRoot, "unexpected-spawn");
+
+    await expect(controller.launch<DemoMethods>({
+      args: [
+        "-e",
+        "require('node:fs').writeFileSync(process.argv[1], 'spawned')",
+        spawnMarker,
+      ],
+      executable: process.execPath,
+      existing: "adopt",
+      readyTimeoutMs: 200,
+      service: "daemon",
+    })).rejects.toMatchObject({ code: "peer-unavailable" });
+    await expect(access(spawnMarker)).rejects.toThrow();
+  });
+
+  it("reclaims an adopted identity whose recorded process has exited", async () => {
+    const { roots, scope } = await createFixture();
+    const controller = createDemoController(scope, roots);
+    const metadata = createPrivateLaunchForTest({
+      projection: demoProjection,
+      roots,
+      scope,
+      service: "daemon",
+    });
+    await writePrivateReadyDescriptorForTest(metadata, 2_147_483_647);
+
+    const launch = await controller.launch<DemoMethods>({
+      args: ["--import", "tsx", join(import.meta.dirname, "fixtures", "control-child.ts")],
+      executable: process.execPath,
+      existing: "adopt",
+      readyTimeoutMs: 5_000,
+      service: "daemon",
+    });
+    cleanups.push(async () => {
+      await launch.stop();
+    });
+
+    await expect(launch.client.call("echo", { value: "reclaimed" })).resolves.toEqual({
+      value: "reclaimed",
+    });
+  });
+
   it("agree on normalized identity, roots and caller-owned methods", async () => {
     const { roots, scope } = await createFixture();
     const launch = createPrivateLaunchForTest({

tools/pack/src/control.ts

@@ -1,4 +1,4 @@
-import { readFileSync, statSync } from "node:fs";
+import { readFileSync } from "node:fs";
 import { join, resolve } from "node:path";
 
 import {
@@ -18,39 +18,42 @@ import {
 
 import type { ToolPackConfig } from "./config.js";
 
+export type ToolPackControlMode = "desktop" | "headless";
+
+const TOOL_PACK_SERVICE_STOPS = [
+  { service: APP_KEYS.DESKTOP, options: { graceMs: 15_000 } },
+  { service: APP_KEYS.WEB },
+  { service: APP_KEYS.DAEMON },
+] as const;
+
 export function createToolPackControl(
   config: ToolPackConfig,
+  mode: ToolPackControlMode,
 ): SidecarControlAccess {
   const fallbackChannel = releaseChannelFromVersion(config.appVersion)
     ?? releaseChannelFromNamespace(config.namespace, "default")
     ?? "local";
   const namespaceRoot = resolve(config.roots.runtime.namespaceRoot);
   let scope: SidecarControlScope = { channel: fallbackChannel, generation: 0, namespace: config.namespace };
-  let newestIdentityMtime = Number.NEGATIVE_INFINITY;
-  for (const name of ["desktop-root.json", "headless-root.json"]) {
-    const identityPath = join(namespaceRoot, "runtime", name);
-    try {
-      const identity = JSON.parse(readFileSync(identityPath, "utf8")) as {
-        runtime?: { channel?: unknown; generation?: unknown; namespace?: unknown };
+  const identityPath = join(namespaceRoot, "runtime", `${mode}-root.json`);
+  try {
+    const identity = JSON.parse(readFileSync(identityPath, "utf8")) as {
+      runtime?: { channel?: unknown; generation?: unknown; namespace?: unknown };
+    };
+    if (
+      typeof identity.runtime?.channel === "string"
+      && Number.isSafeInteger(identity.runtime.generation)
+      && (identity.runtime.generation as number) >= 0
+      && identity.runtime.namespace === config.namespace
+    ) {
+      scope = {
+        channel: identity.runtime.channel,
+        generation: identity.runtime.generation as number,
+        namespace: config.namespace,
       };
-      const mtime = statSync(identityPath).mtimeMs;
-      if (
-        mtime > newestIdentityMtime
-        && typeof identity.runtime?.channel === "string"
-        && Number.isSafeInteger(identity.runtime.generation)
-        && (identity.runtime.generation as number) >= 0
-        && identity.runtime.namespace === config.namespace
-      ) {
-        newestIdentityMtime = mtime;
-        scope = {
-          channel: identity.runtime.channel,
-          generation: identity.runtime.generation as number,
-          namespace: config.namespace,
-        };
-      }
-    } catch {
-      // Missing or invalid identities do not define a live control scope.
     }
+  } catch {
+    // A missing or invalid mode-owned identity does not define a live scope.
   }
   return accessControlPlane({
     runtimeRoot: join(namespaceRoot, "runtime"),
@@ -61,11 +64,7 @@ export function createToolPackControl(
 export async function stopToolPackServices(
   control: SidecarControlAccess,
 ): Promise<SidecarConvergeResult[]> {
-  const attempts = await stopSidecarServices(control, [
-    { service: APP_KEYS.DESKTOP, options: { graceMs: 15_000 } },
-    { service: APP_KEYS.WEB },
-    { service: APP_KEYS.DAEMON },
-  ]);
+  const attempts = await stopSidecarServices(control, TOOL_PACK_SERVICE_STOPS);
   const failures = attempts.flatMap((attempt) =>
     attempt.status === "rejected"
       ? [new Error(`failed to stop ${attempt.service}`, { cause: attempt.error })]
@@ -79,3 +78,21 @@ export async function stopToolPackServices(
     return attempt.result;
   });
 }
+
+export async function convergeToolPackServices(
+  control: SidecarControlAccess,
+): Promise<SidecarConvergeResult[]> {
+  const results = await stopToolPackServices(control);
+  const failures = results.flatMap((result, index) =>
+    result.stopped
+      ? []
+      : [new Error(
+          `could not prove ${TOOL_PACK_SERVICE_STOPS[index]!.service} stopped`
+          + (result.pid == null ? "" : ` (pid ${result.pid})`),
+        )],
+  );
+  if (failures.length > 0) {
+    throw new AggregateError(failures, "failed to converge one or more packaged services");
+  }
+  return results;
+}

tools/pack/src/linux.ts

@@ -23,7 +23,7 @@ import {
 } from "@open-design/platform";
 
 import type { ToolPackConfig } from "./config.js";
-import { createToolPackControl, stopToolPackServices } from "./control.js";
+import { convergeToolPackServices, createToolPackControl, stopToolPackServices } from "./control.js";
 import { domToPptxBundleResource } from "./dom-to-pptx-resource.js";
 import { copyBundledResourceTrees, linuxResources, packBundledDshRuntime } from "./resources.js";
 import { copyOptionalVelaCliBinary } from "./vela-cli.js";
@@ -932,7 +932,7 @@ async function waitForMarker(markerPath: string, timeoutMs: number): Promise<boo
 
 async function fetchDesktopStatus(config: ToolPackConfig): Promise<DesktopStatusSnapshot | null> {
   try {
-    const desktop = await createToolPackControl(config).connect<DesktopSidecarMethods>(APP_KEYS.DESKTOP);
+    const desktop = await createToolPackControl(config, "desktop").connect<DesktopSidecarMethods>(APP_KEYS.DESKTOP);
     return await desktop.call("status", {}, { timeoutMs: 2_000 });
   } catch {
     return null;
@@ -956,7 +956,7 @@ export async function startPackedLinuxApp(config: ToolPackConfig): Promise<Linux
 
   // Resolve and converge the exact live generation before clearing its product
   // identity. Clearing first would make control fall back to cold generation 0.
-  await stopToolPackServices(createToolPackControl(config));
+  await convergeToolPackServices(createToolPackControl(config, "desktop"));
   await rm(desktopIdentityPath(config), { force: true }).catch(() => undefined);
 
   // --appimage-extract-and-run bypasses FUSE-mounted SquashFS, which is too slow
@@ -1012,7 +1012,7 @@ async function teardownOrphanedStart(rootPid: number): Promise<void> {
 }
 
 export async function stopPackedLinuxApp(config: ToolPackConfig): Promise<LinuxStopResult> {
-  const results = await stopToolPackServices(createToolPackControl(config));
+  const results = await stopToolPackServices(createToolPackControl(config, "desktop"));
   const pids = [...new Set(results.flatMap((result) => result.pid == null ? [] : [result.pid]))];
   const remainingPids = [...new Set(results.flatMap((result) => !result.stopped && result.pid != null ? [result.pid] : []))];
   const stoppedPids = [...new Set(results.flatMap((result) => result.stopped && result.pid != null ? [result.pid] : []))];
@@ -1050,7 +1050,8 @@ export async function inspectPackedLinuxApp(
     throw new Error("linux inspect --headless supports status only; omit --expr and --path");
   }
 
-  const desktop = await createToolPackControl(config).connect<DesktopSidecarMethods>(APP_KEYS.DESKTOP).catch(() => null);
+  const controlMode = options.headless === true ? "headless" : "desktop";
+  const desktop = await createToolPackControl(config, controlMode).connect<DesktopSidecarMethods>(APP_KEYS.DESKTOP).catch(() => null);
   const status = await desktop?.call("status", {}, { timeoutMs: 2000 }).catch(() => null) ?? null;
 
   if (options.headless === true) {
@@ -1313,7 +1314,7 @@ export async function startPackedLinuxHeadless(config: ToolPackConfig): Promise<
   }
 
   const nodeCommand = (await pathExists(nodePath)) ? nodePath : process.execPath;
-  await stopToolPackServices(createToolPackControl(config));
+  await convergeToolPackServices(createToolPackControl(config, "headless"));
   const logPath = headlessLogPath(config);
   await mkdir(dirname(logPath), { recursive: true });
   await writeFile(logPath, "", "utf8");
@@ -1375,7 +1376,7 @@ export async function startPackedLinuxHeadless(config: ToolPackConfig): Promise<
 }
 
 export async function stopPackedLinuxHeadless(config: ToolPackConfig): Promise<LinuxStopResult> {
-  const results = await stopToolPackServices(createToolPackControl(config));
+  const results = await stopToolPackServices(createToolPackControl(config, "headless"));
   const pids = [...new Set(results.flatMap((result) => result.pid == null ? [] : [result.pid]))];
   const remainingPids = [...new Set(results.flatMap((result) => !result.stopped && result.pid != null ? [result.pid] : []))];
   const stoppedPids = [...new Set(results.flatMap((result) => result.stopped && result.pid != null ? [result.pid] : []))];