fix(daemon): stop branch-named daemon before replacement

scripts/__tests__/test-file-size-ratchet.test.ts

@@ -38,7 +38,7 @@ const PINNED_TEST_FILE_LINES: Readonly<Record<string, number>> = Object.freeze({
   'src/commands/interaction/runtime/settle.test.ts': 2359,
   'src/daemon/handlers/__tests__/session-replay-runtime-maestro.test.ts': 2024,
   'src/platforms/apple/core/__tests__/runner-session.test.ts': 2001,
-  'src/utils/__tests__/daemon-client.test.ts': 1910,
+  'src/utils/__tests__/daemon-client.test.ts': 1873,
   'src/utils/__tests__/output.test.ts': 1861,
   'src/platforms/android/__tests__/snapshot.test.ts': 1445,
   'src/platforms/apple/core/__tests__/runner-client.test.ts': 1615,

src/daemon/__tests__/daemon-process-takeover.test.ts

@@ -0,0 +1,85 @@
+import assert from 'node:assert/strict';
+import { spawn } from 'node:child_process';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { afterEach, test } from 'vitest';
+import { isProcessAlive, readProcessStartTime } from '../../utils/host-process.ts';
+import { isAgentDeviceDaemonProcess, stopProcessForTakeover } from '../daemon-process.ts';
+
+const TAKEOVER_TIMEOUTS = { termTimeoutMs: 5_000, killTimeoutMs: 2_000 };
+const spawnedPids: number[] = [];
+const spawnedRoots: string[] = [];
+
+afterEach(() => {
+  for (const pid of spawnedPids.splice(0)) {
+    if (!isProcessAlive(pid)) continue;
+    try {
+      process.kill(pid, 'SIGKILL');
+    } catch {
+      // The test's assertion already observed that the child exited.
+    }
+  }
+  for (const root of spawnedRoots.splice(0)) {
+    fs.rmSync(root, { recursive: true, force: true });
+  }
+});
+
+/** A daemon entry from a branch-named checkout, whose path has no project-name marker. */
+function spawnFakeDaemonFromBranchNamedCheckout(): { pid: number; entryPath: string } {
+  // Vitest redirects `os.tmpdir()` under a worktree-named run directory. Use
+  // the host temp root so this path itself cannot accidentally contain the
+  // project name and mask the worktree identity regression.
+  const tempRoot = process.platform === 'win32' ? os.tmpdir() : '/tmp';
+  const root = fs.realpathSync.native(fs.mkdtempSync(path.join(tempRoot, 'repair-evidence-')));
+  spawnedRoots.push(root);
+  const entryPath = path.join(root, 'dist', 'src', 'internal', 'daemon.js');
+  fs.mkdirSync(path.dirname(entryPath), { recursive: true });
+  fs.writeFileSync(entryPath, 'setInterval(() => {}, 1000);\n', 'utf8');
+  const child = spawn(process.execPath, [entryPath], { stdio: 'ignore' });
+  const pid = child.pid ?? 0;
+  assert.ok(pid > 0, 'expected the fake daemon to have a pid');
+  spawnedPids.push(pid);
+  return { pid, entryPath };
+}
+
+// #1545: a reachable daemon with a code-signature mismatch is replaced. If a
+// branch-named worktree is rejected as "not ours", replacement leaves the old
+// daemon alive and the next request reaches a fresh empty session store.
+test('stops a branch-named daemon before replacement can strand its session', async () => {
+  const { pid, entryPath } = spawnFakeDaemonFromBranchNamedCheckout();
+  assert.equal(entryPath.toLowerCase().includes('agent-device'), false);
+
+  const startTime = readProcessStartTime(pid);
+  assert.ok(startTime, 'expected the spawned daemon to report a start time');
+  assert.equal(isAgentDeviceDaemonProcess(pid, startTime), true);
+
+  await stopProcessForTakeover(pid, { ...TAKEOVER_TIMEOUTS, expectedStartTime: startTime });
+  assert.equal(isProcessAlive(pid), false);
+});
+
+test('does not stop a branch-named daemon when process identity is missing', async () => {
+  const { pid, entryPath } = spawnFakeDaemonFromBranchNamedCheckout();
+  assert.equal(entryPath.toLowerCase().includes('agent-device'), false);
+
+  assert.equal(isAgentDeviceDaemonProcess(pid, undefined), false);
+
+  await stopProcessForTakeover(pid, { ...TAKEOVER_TIMEOUTS, expectedStartTime: undefined });
+  assert.equal(isProcessAlive(pid), true);
+});
+
+test('does not stop a branch-named daemon when the pid belongs to a different process lifetime', async () => {
+  const { pid, entryPath } = spawnFakeDaemonFromBranchNamedCheckout();
+  assert.equal(entryPath.toLowerCase().includes('agent-device'), false);
+
+  const actualStartTime = readProcessStartTime(pid);
+  assert.ok(actualStartTime, 'expected the spawned daemon to report a start time');
+  const staleStartTime = `${actualStartTime}-previous-lifetime`;
+  assert.equal(isAgentDeviceDaemonProcess(pid, staleStartTime), false);
+
+  await stopProcessForTakeover(pid, {
+    ...TAKEOVER_TIMEOUTS,
+    expectedStartTime: staleStartTime,
+  });
+  assert.equal(isProcessAlive(pid), true);
+});

src/daemon/__tests__/daemon-process.test.ts

@@ -16,3 +16,44 @@ test('isAgentDeviceDaemonCommand matches expected daemon command', () => {
   );
   assert.equal(isAgentDeviceDaemonCommand('node -e "setInterval(() => {}, 1000)"'), false);
 });
+
+test('isAgentDeviceDaemonCommand matches daemons from branch-named checkouts', () => {
+  assert.equal(
+    isAgentDeviceDaemonCommand(
+      '/usr/bin/node /Users/dev/worktrees/repair-evidence/dist/src/internal/daemon.js',
+    ),
+    true,
+  );
+  assert.equal(
+    isAgentDeviceDaemonCommand('/usr/bin/node /Users/dev/wt/fix-1545/dist/src/daemon.js'),
+    true,
+  );
+  assert.equal(
+    isAgentDeviceDaemonCommand(
+      '/usr/bin/node --experimental-strip-types /Users/dev/wt/fix-1545/src/daemon.ts',
+    ),
+    true,
+  );
+  assert.equal(
+    isAgentDeviceDaemonCommand(
+      '"C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\dev\\wt\\fix-1545\\dist\\src\\daemon.js"',
+    ),
+    true,
+  );
+});
+
+test('isAgentDeviceDaemonCommand rejects commands that only resemble a daemon entry', () => {
+  assert.equal(
+    isAgentDeviceDaemonCommand('/usr/bin/node /Users/dev/wt/fix-1545/dist/src/daemon-worker.js'),
+    false,
+  );
+  assert.equal(
+    isAgentDeviceDaemonCommand('/usr/bin/node /Users/dev/wt/fix-1545/dist/src/internal/server.js'),
+    false,
+  );
+  assert.equal(
+    isAgentDeviceDaemonCommand('/usr/bin/node /Users/dev/wt/fix-1545/dist/src/bin.js'),
+    false,
+  );
+  assert.equal(isAgentDeviceDaemonCommand('vim src/daemon.ts'), false);
+});

src/daemon/daemon-process.ts

@@ -6,23 +6,32 @@ import {
 } from '../utils/host-process.ts';
 
 const DAEMON_COMMAND_PATTERNS = [
-  /(^|[/\s"'=])dist\/src\/daemon\.js($|[\s"'])/,
-  /(^|[/\s"'=])dist\/src\/internal\/daemon\.js($|[\s"'])/,
-  /(^|[/\s"'=])src\/daemon\.ts($|[\s"'])/,
+  /\/dist\/src\/daemon\.js($|[\s"'])/,
+  /\/dist\/src\/internal\/daemon\.js($|[\s"'])/,
+  /\/src\/daemon\.ts($|[\s"'])/,
 ];
 
+/**
+ * Identity is the daemon entry path, never the checkout's directory name: a
+ * git worktree is named after its branch, so a `agent-device` substring gate
+ * classified every worktree daemon as "not ours" (#1545). The pid always
+ * comes from our own daemon.json/daemon.lock alongside the processStartTime
+ * recorded with it. Missing identity must fail closed so a stale PID cannot
+ * be authorized by the path match alone.
+ */
 export function isAgentDeviceDaemonCommand(command: string): boolean {
   const normalized = command.toLowerCase().replaceAll('\\', '/');
-  if (!normalized.includes('agent-device')) return false;
   return DAEMON_COMMAND_PATTERNS.some((pattern) => pattern.test(normalized));
 }
 
-export function isAgentDeviceDaemonProcess(pid: number, expectedStartTime?: string): boolean {
+export function isAgentDeviceDaemonProcess(
+  pid: number,
+  expectedStartTime: string | undefined,
+): boolean {
+  if (!expectedStartTime) return false;
   if (!isProcessAlive(pid)) return false;
-  if (expectedStartTime) {
-    const actualStartTime = readProcessStartTime(pid);
-    if (!actualStartTime || actualStartTime !== expectedStartTime) return false;
-  }
+  const actualStartTime = readProcessStartTime(pid);
+  if (!actualStartTime || actualStartTime !== expectedStartTime) return false;
   const command = readProcessCommand(pid);
   if (!command) return false;
   return isAgentDeviceDaemonCommand(command);
@@ -44,7 +53,7 @@ export async function stopProcessForTakeover(
   options: {
     termTimeoutMs: number;
     killTimeoutMs: number;
-    expectedStartTime?: string;
+    expectedStartTime: string | undefined;
   },
 ): Promise<void> {
   if (!isAgentDeviceDaemonProcess(pid, options.expectedStartTime)) return;

src/utils/__tests__/daemon-client.test.ts

@@ -1844,44 +1844,6 @@ test('computeDaemonCodeSignature ignores a relative-path-shaped string that is n
   }
 });
 
-test('stopDaemonProcessForTakeover terminates a matching daemon process', async (t) => {
-  const root = mkdtempForTestSync('agent-device-daemon-test-');
-  const daemonDir = path.join(root, 'agent-device', 'dist', 'src', 'internal');
-  const daemonScriptPath = path.join(daemonDir, 'daemon.js');
-  fs.mkdirSync(daemonDir, { recursive: true });
-  fs.writeFileSync(daemonScriptPath, 'setInterval(() => {}, 1000);\n', 'utf8');
-  const daemonProcess = runCmdBackground(process.execPath, [daemonScriptPath], {
-    stdio: 'ignore',
-    allowFailure: true,
-    captureOutput: false,
-  });
-  void daemonProcess.wait.catch(() => {});
-  const child = daemonProcess.child;
-  const pid = child.pid;
-  assert.ok(pid, 'spawned child should have a pid');
-
-  try {
-    await new Promise((resolve) => setTimeout(resolve, 50));
-    if (readProcessCommand(pid) === null) {
-      t.skip('process command inspection is unavailable in this environment');
-      return;
-    }
-    assert.equal(isProcessAlive(pid), true);
-    await stopProcessForTakeover(pid, {
-      termTimeoutMs: 1_500,
-      killTimeoutMs: 1_500,
-    });
-    const exited = await waitForProcessExit(pid, 1500);
-    assert.equal(exited, true);
-  } finally {
-    if (isProcessAlive(pid)) {
-      process.kill(pid, 'SIGKILL');
-      await waitForProcessExit(pid, 1_500);
-    }
-    fs.rmSync(root, { recursive: true, force: true });
-  }
-});
-
 test('stopDaemonProcessForTakeover does not terminate non-daemon process', async () => {
   const daemonProcess = runCmdBackground(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], {
     stdio: 'ignore',
@@ -1899,6 +1861,7 @@ test('stopDaemonProcessForTakeover does not terminate non-daemon process', async
     await stopProcessForTakeover(pid, {
       termTimeoutMs: 100,
       killTimeoutMs: 100,
+      expectedStartTime: undefined,
     });
     assert.equal(isProcessAlive(pid), true);
   } finally {

test/integration/smoke-web-platform.test.ts

@@ -1,5 +1,6 @@
 import assert from 'node:assert/strict';
-import { mkdirSync, readFileSync, writeFileSync } from 'node:fs';
+import { spawn } from 'node:child_process';
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
 import { createServer, type Server } from 'node:http';
 import path from 'node:path';
 import test from 'node:test';
@@ -21,6 +22,7 @@ import {
   expandProcessTree,
   isProcessAlive,
   listHostProcesses,
+  readProcessStartTime,
   stopPidsWithEscalation,
 } from '../../src/utils/host-process.ts';
 
@@ -38,6 +40,73 @@ const WEB_SHUTDOWN_CLEANUP_TIMEOUT_MS = 5_000;
 // actively closed the browser, not that agent-browser's own idle timer beat this test's own wait.
 const WEB_SHUTDOWN_IDLE_TIMEOUT_MS = WEB_SHUTDOWN_SETTLE_TIMEOUT_MS + 60_000;
 
+type WebShutdownDaemonIdentity = {
+  pid: number;
+  startTime: string;
+};
+
+test('web shutdown cleanup reaps the exact daemon that survived graceful shutdown', async (t) => {
+  const root = mkdtempSync('/tmp/agent-device-web-shutdown-cleanup-');
+  const entryPath = path.join(root, 'src', 'daemon.ts');
+  mkdirSync(path.dirname(entryPath), { recursive: true });
+  writeFileSync(
+    entryPath,
+    [
+      "process.on('SIGTERM', () => process.send?.('sigterm-ignored'));",
+      "process.send?.('ready');",
+      'setInterval(() => {}, 1000);',
+      '',
+    ].join('\n'),
+    'utf8',
+  );
+  const child = spawn(process.execPath, [entryPath], {
+    stdio: ['ignore', 'ignore', 'ignore', 'ipc'],
+  });
+  const daemonPid = child.pid ?? 0;
+  assert.ok(daemonPid > 0, 'expected the fake daemon to have a pid');
+  t.after(() => {
+    if (isProcessAlive(daemonPid)) process.kill(daemonPid, 'SIGKILL');
+    rmSync(root, { recursive: true, force: true });
+  });
+
+  await new Promise<void>((resolve, reject) => {
+    child.once('error', reject);
+    child.once('message', (message) => {
+      child.off('error', reject);
+      assert.equal(message, 'ready');
+      resolve();
+    });
+  });
+  let ignoredSigterm = false;
+  child.on('message', (message) => {
+    if (message === 'sigterm-ignored') ignoredSigterm = true;
+  });
+
+  const daemonStartTime = readProcessStartTime(daemonPid);
+  assert.ok(daemonStartTime, 'expected the fake daemon to report a start time');
+  await cleanupWebShutdownSmoke(
+    {
+      artifactDir: root,
+      common: [],
+      env: {},
+      screenshotPath: path.join(root, 'unused.png'),
+      server: createServer(),
+      stepHistory: [],
+      url: 'http://127.0.0.1',
+    },
+    { pid: daemonPid, startTime: daemonStartTime },
+    undefined,
+    { termTimeoutMs: 50, killTimeoutMs: 1_000 },
+  );
+
+  assert.equal(
+    ignoredSigterm,
+    true,
+    'expected cleanup to escalate after the child ignored SIGTERM',
+  );
+  assert.equal(isProcessAlive(daemonPid), false);
+});
+
 type StepRecord = {
   step: string;
   command: string;
@@ -99,7 +168,7 @@ async function runWebShutdownSmoke(context: WebSmokeContext): Promise<void> {
   // Cleanup authority lives entirely in `finally`, driven by these two, so a failed assertion
   // above (including the very failure this test exists to catch: processes still alive when the
   // fix regresses) can never leave a daemon or a Chrome fleet running on the host afterward.
-  let daemonPid: number | undefined;
+  let daemonIdentity: WebShutdownDaemonIdentity | undefined;
   let status: AgentBrowserToolStatus | undefined;
   try {
     await runStep(context, 'set up managed web backend', ['web', 'setup', '--json']);
@@ -115,7 +184,10 @@ async function runWebShutdownSmoke(context: WebSmokeContext): Promise<void> {
       `expected the managed browser fleet to be running after open, found none: ${formatProcessSummary(before)}`,
     );
 
-    daemonPid = readDaemonPid(stateDir);
+    const daemonPid = readDaemonPid(stateDir);
+    const daemonStartTime = readProcessStartTime(daemonPid);
+    assert.ok(daemonStartTime, 'expected the daemon process to report a start time');
+    daemonIdentity = { pid: daemonPid, startTime: daemonStartTime };
     assert.equal(isProcessAlive(daemonPid), true, 'expected a live daemon before SIGTERM');
 
     // The scenario #1868 is about: SIGTERM the daemon directly, the way an operator or an
@@ -138,7 +210,7 @@ async function runWebShutdownSmoke(context: WebSmokeContext): Promise<void> {
     // state-dir residue either, on top of the process-level proof above.
     await assertNoDaemonLeaks({ stateDir, daemonPids: [daemonPid], phase: 'after-shutdown' });
   } finally {
-    await cleanupWebShutdownSmoke(context, daemonPid, status);
+    await cleanupWebShutdownSmoke(context, daemonIdentity, status);
   }
 }
 
@@ -150,15 +222,20 @@ async function runWebShutdownSmoke(context: WebSmokeContext): Promise<void> {
 // alongside.
 async function cleanupWebShutdownSmoke(
   context: WebSmokeContext,
-  daemonPid: number | undefined,
+  daemonIdentity: WebShutdownDaemonIdentity | undefined,
   status: AgentBrowserToolStatus | undefined,
+  timeouts = {
+    termTimeoutMs: WEB_SHUTDOWN_CLEANUP_TIMEOUT_MS,
+    killTimeoutMs: WEB_SHUTDOWN_CLEANUP_TIMEOUT_MS,
+  },
 ): Promise<void> {
   const errors: unknown[] = [];
-  if (daemonPid !== undefined) {
+  if (daemonIdentity !== undefined) {
     try {
-      await stopProcessForTakeover(daemonPid, {
-        termTimeoutMs: WEB_SHUTDOWN_CLEANUP_TIMEOUT_MS,
-        killTimeoutMs: WEB_SHUTDOWN_CLEANUP_TIMEOUT_MS,
+      await stopProcessForTakeover(daemonIdentity.pid, {
+        termTimeoutMs: timeouts.termTimeoutMs,
+        killTimeoutMs: timeouts.killTimeoutMs,
+        expectedStartTime: daemonIdentity.startTime,
       });
     } catch (error) {
       errors.push(error);

vitest.config.ts

@@ -16,6 +16,9 @@ const SUBPROCESS_STUB_TESTS: readonly string[] = [
   'src/__tests__/client-metro.test.ts',
   // The SUT is the subprocess watchdog: a node subprocess per case, one hangs on purpose (#1414).
   'scripts/fuzz/harness.test.ts',
+  // The daemon takeover test launches a branch-named daemon per case to prove
+  // that worktree identity does not strand the predecessor (#1545).
+  'src/daemon/__tests__/daemon-process-takeover.test.ts',
 ];
 
 // The fuzz corpus replay, which must not run under V8 coverage instrumentation.