fix(auth): keep the OAuth listTools request alive across every phase

src/cli.ts

@@ -325,7 +325,7 @@ export async function runCli(argv: string[]): Promise<void> {
         return;
       }
       const { handleAuth: importedHandleAuth } = await import('./cli/auth-command.js');
-      await importedHandleAuth(runtime, resolvedArgs);
+      await importedHandleAuth(runtime, resolvedArgs, { oauthTimeoutMs: runtimeOptionsWithPath.oauthTimeoutMs });
       return;
     }
 
@@ -441,7 +441,7 @@ async function invokeAuthCommand(runtimeOptions: RuntimeOptions, args: string[])
   ]);
   const runtime = await createRuntime(runtimeOptions);
   try {
-    await importedHandleAuth(runtime, args);
+    await importedHandleAuth(runtime, args, { oauthTimeoutMs: runtimeOptions.oauthTimeoutMs });
   } finally {
     await runtime.close().catch(() => {});
   }
@@ -630,6 +630,7 @@ function createDaemonOnlyRuntime(daemonClient: import('./daemon/client.js').Daem
         autoAuthorize: options?.autoAuthorize,
         allowCachedAuth: options?.allowCachedAuth,
         disableOAuth: options?.disableOAuth,
+        timeoutMs: options?.timeoutMs,
       })) as Awaited<ReturnType<Runtime['listTools']>>,
     callTool: (server, toolName, options) =>
       daemonClient.callTool({

src/cli/auth-command.ts

@@ -4,7 +4,7 @@ import type { OAuthAuthorizationRequest, OAuthSessionOptions } from '../oauth.js
 import { analyzeConnectionError } from '../error-classifier.js';
 import { clearOAuthCaches } from '../oauth-persistence.js';
 import type { createRuntime } from '../runtime.js';
-import { isOAuthFlowError } from '../runtime/oauth.js';
+import { isOAuthFlowError, resolveOAuthTimeoutFromEnv } from '../runtime/oauth.js';
 import type { EphemeralServerSpec } from './adhoc-server.js';
 import { extractEphemeralServerFlags } from './ephemeral-flags.js';
 import { persistPreparedEphemeralServer, prepareEphemeralServerTarget } from './ephemeral-target.js';
@@ -17,12 +17,17 @@ type Runtime = Awaited<ReturnType<typeof createRuntime>>;
 
 type BrowserSuppression = 'default' | 'no-browser';
 
+export interface AuthCommandOptions {
+  readonly oauthTimeoutMs?: number;
+}
+
 const TRUE_VALUES = new Set(['1', 'true', 'yes']);
 const FALSE_VALUES = new Set(['0', 'false', 'no']);
 
-export async function handleAuth(runtime: Runtime, args: string[]): Promise<void> {
+export async function handleAuth(runtime: Runtime, args: string[], options: AuthCommandOptions = {}): Promise<void> {
   const browserSuppression = consumeBrowserSuppression(args, process.env);
   const noBrowser = browserSuppression === 'no-browser';
+  const oauthTimeoutMs = options.oauthTimeoutMs ?? resolveOAuthTimeoutFromEnv();
   let authorizationOutputEmitted = false;
   const markAuthorizationOutputEmitted = () => {
     authorizationOutputEmitted = true;
@@ -85,6 +90,11 @@ export async function handleAuth(runtime: Runtime, args: string[]): Promise<void
       const tools = await withInfoLogsSuppressed(noBrowser, () =>
         runtime.listTools(target, {
           autoAuthorize: true,
+          // The interactive OAuth browser flow runs inside this listTools
+          // request. Let it ride for as long as we're willing to wait for the
+          // authorization code (MCPORTER_OAUTH_TIMEOUT_MS, default 300s) instead
+          // of being killed by the SDK's 60s request cap.
+          timeoutMs: oauthTimeoutMs,
           ...(noBrowser
             ? {
                 oauthSessionOptions: buildNoBrowserOAuthOptions(format, markAuthorizationOutputEmitted),

src/daemon/client.ts

@@ -5,6 +5,12 @@ import path from 'node:path';
 import { listConfigLayerPaths } from '../config/path-discovery.js';
 import { withFileLock } from '../fs-json.js';
 import { getDaemonMetadataPath, getDaemonSocketPath } from './paths.js';
+import {
+  DAEMON_PROTOCOL_VERSION,
+  DaemonFrameDecoder,
+  isDaemonProgressFrame,
+  resolveProgressInterval,
+} from './protocol.js';
 import type {
   CallToolParams,
   CloseServerParams,
@@ -25,6 +31,27 @@ export interface DaemonClientOptions {
 
 const DEFAULT_DAEMON_TIMEOUT_MS = 30_000;
 const MIN_DAEMON_STATUS_TIMEOUT_MS = 1_000;
+// How long a client is willing to wait for a shared daemon it has decided to
+// replace to finish its in-flight work before the replacement is abandoned.
+// Capped so an upgraded client never blocks the calling command indefinitely
+// waiting on another process; the cap is also the only knob for v1 daemons,
+// which do not advertise `activeRequests` and so look busy by definition.
+// Overridable through `MCPORTER_DAEMON_DRAIN_TIMEOUT_MS` for tests that need
+// to exercise the refusal branch without waiting the full default.
+const DEFAULT_DAEMON_DRAIN_TIMEOUT_MS = 60_000;
+const DAEMON_DRAIN_POLL_MS = 100;
+
+function resolveDrainTimeoutMs(): number {
+  const raw = process.env.MCPORTER_DAEMON_DRAIN_TIMEOUT_MS;
+  if (!raw) {
+    return DEFAULT_DAEMON_DRAIN_TIMEOUT_MS;
+  }
+  const parsed = Number.parseInt(raw, 10);
+  if (!Number.isFinite(parsed) || parsed <= 0) {
+    return DEFAULT_DAEMON_DRAIN_TIMEOUT_MS;
+  }
+  return parsed;
+}
 
 export interface DaemonPaths {
   readonly key: string;
@@ -34,6 +61,7 @@ export interface DaemonPaths {
 
 interface DaemonMetadata {
   readonly pid: number;
+  readonly protocolVersion?: number;
   readonly socketPath: string;
   readonly configPath: string;
   readonly configMtimeMs?: number | null;
@@ -69,7 +97,7 @@ export class DaemonClient {
   }
 
   async listTools(params: ListToolsParams): Promise<unknown> {
-    return this.invoke('listTools', params);
+    return this.invoke('listTools', params, params.timeoutMs);
   }
 
   async listResources(params: ListResourcesParams): Promise<unknown> {
@@ -130,24 +158,120 @@ export class DaemonClient {
 
   private async restartDaemon(options: { reason?: 'stale-config'; expectedPid?: number } = {}): Promise<void> {
     await this.startingWithLock(async () => {
-      const currentStatus = await this.readVerifiedStatus();
-      if (
-        currentStatus &&
-        options.expectedPid !== undefined &&
-        currentStatus.pid !== options.expectedPid &&
-        (await this.checkConfigState()) === 'fresh'
-      ) {
+      // Probe the daemon without the pid-match filter so the drain check
+      // below can see whether the daemon is actually busy. `readVerifiedStatus`
+      // would silently return null on a pid mismatch and the busy-daemon
+      // regression would slip past us.
+      const liveStatus = await this.probeLiveStatus();
+      if (options.reason === 'stale-config' && liveStatus && (await this.checkConfigState()) === 'fresh') {
         return;
       }
-      if (options.reason === 'stale-config' && currentStatus && (await this.checkConfigState()) === 'fresh') {
+      // If the daemon has already been replaced by another client (PID
+      // mismatch before the drain even starts), the replacement is theirs to
+      // own -- stopping it would kill their in-flight request. The config
+      // check above is not enough: the peer may not have rewritten the
+      // metadata yet, so `checkConfigState` can still report `stale` and the
+      // condition would fall through to `stop()`.
+      if (liveStatus && options.expectedPid !== undefined && liveStatus.pid !== options.expectedPid) {
         return;
       }
+      // Codex / ClawSweeper P1: an upgraded client must not stop a daemon that
+      // another client is currently waiting on. Wait for the in-flight count to
+      // reach zero before issuing `stop`; bail out (do not replace) if the
+      // daemon is still busy past the drain timeout, so the busy client keeps
+      // its daemon and the caller fails loud rather than silently replaying.
+      if (liveStatus && options.expectedPid !== undefined && liveStatus.pid === options.expectedPid) {
+        const drainTimeoutMs = resolveDrainTimeoutMs();
+        const drainResult = await this.waitForDaemonIdle(liveStatus, drainTimeoutMs);
+        if (!drainResult.drained) {
+          throw new Error(
+            `Shared daemon pid=${liveStatus.pid} is still busy after ${drainTimeoutMs}ms; refusing to replace it.`
+          );
+        }
+        // If another client replaced the daemon during the drain, do not stop
+        // their fresh daemon -- that is exactly the regression the drain was
+        // added to prevent.
+        if (drainResult.replacedByPeer) {
+          return;
+        }
+      }
       await this.stop().catch(() => {});
       await this.waitForStopped();
       await this.launchDaemonAndWait();
     });
   }
 
+  /**
+   * Sends a raw `status` probe without the pid-match filter that
+   * `readVerifiedStatus` applies. Needed by callers that want to decide
+   * between acting on the current daemon, a peer's fresh daemon, and a
+   * missing daemon -- the verified variant collapses all three into null and
+   * loses the signal.
+   */
+  private async probeLiveStatus(timeoutMs?: number): Promise<StatusResult | null> {
+    const metadata = await readDaemonMetadata(this.metadataPath);
+    if (!metadata || metadata.socketPath !== this.socketPath || !isProcessRunning(metadata.pid)) {
+      return null;
+    }
+    try {
+      return (await this.sendRequest<StatusResult>('status', {}, timeoutMs)) as StatusResult;
+    } catch (error) {
+      if (isTransportError(error)) {
+        return null;
+      }
+      throw error;
+    }
+  }
+
+  /**
+   * Polls the live daemon's status until its `activeRequests` counter reads
+   * zero. Returns both whether the drain condition was met AND whether the
+   * daemon got swapped out by another client mid-wait -- those are different
+   * outcomes that the caller needs to distinguish (a replaced daemon is
+   * "drained" but must not be stopped).
+   *
+   * `drained` is `true` once the daemon is idle, OR the daemon is gone (status
+   * probe fails), OR the daemon was replaced by another client (different
+   * pid). `drained` is `false` only if the timeout expires with the original
+   * daemon still advertising in-flight work. `replacedByPeer` is `true` only
+   * when a fresh daemon answered a status poll while we were waiting.
+   *
+   * A daemon that does not advertise `activeRequests` (pre-v2) is treated as
+   * permanently busy -- the only safe read of "is this daemon still serving
+   * a v1 request?" is to wait long enough for that request to finish, which
+   * is exactly the drain we are trying to guarantee.
+   */
+  private async waitForDaemonIdle(
+    seedStatus: StatusResult,
+    timeoutMs: number
+  ): Promise<{ drained: boolean; replacedByPeer: boolean }> {
+    const deadline = Date.now() + timeoutMs;
+    let status: StatusResult | null = seedStatus;
+    let lastSeenPid: number | null = seedStatus.pid;
+    while (Date.now() < deadline) {
+      if (!status) {
+        return { drained: true, replacedByPeer: false };
+      }
+      const active = status.activeRequests;
+      if (active === undefined) {
+        await delay(DAEMON_DRAIN_POLL_MS);
+      } else if (active <= 0) {
+        return { drained: true, replacedByPeer: lastSeenPid !== seedStatus.pid };
+      } else {
+        await delay(DAEMON_DRAIN_POLL_MS);
+      }
+      try {
+        status = await this.sendRequest<StatusResult>('status', {});
+      } catch {
+        return { drained: true, replacedByPeer: false };
+      }
+      if (status) {
+        lastSeenPid = status.pid;
+      }
+    }
+    return { drained: false, replacedByPeer: lastSeenPid !== seedStatus.pid };
+  }
+
   private async startDaemon(options: { preflightTimeoutMs?: number } = {}): Promise<void> {
     await this.startingWithLock(async () => {
       if (await this.isResponsive(options.preflightTimeoutMs)) {
@@ -232,6 +356,9 @@ export class DaemonClient {
     if (!metadata) {
       return 'missing';
     }
+    if (metadata.protocolVersion !== DAEMON_PROTOCOL_VERSION) {
+      return 'stale';
+    }
     const currentLayers = normalizeLayers(await collectConfigLayers(this.options));
     const metadataLayers = normalizeLayers(
       metadata.configLayers ?? [{ path: metadata.configPath, mtimeMs: metadata.configMtimeMs ?? null }]
@@ -250,15 +377,26 @@ export class DaemonClient {
   }
 
   private async sendRequest<T>(method: DaemonRequestMethod, params: unknown, timeoutOverrideMs?: number): Promise<T> {
+    // This is an *idle* deadline, not an operation deadline: the daemon keeps
+    // resetting it with progress frames for as long as it is working, so a
+    // request survives any number of sequential phases (an OAuth code wait plus
+    // however many `tools/list` pages a server returns). The daemon still owns
+    // the operation deadline and answers with `operation_timeout` when a phase
+    // exceeds it; the socket only gives up when the daemon goes silent.
+    const idleTimeoutMs = resolveDaemonTimeout(timeoutOverrideMs);
     const request: DaemonRequest = {
       id: randomUUID(),
       method,
       params,
+      // Tell the daemon how often this deadline needs proof of life, so a short
+      // deadline cannot expire before the first frame arrives.
+      progressIntervalMs: resolveProgressInterval(idleTimeoutMs),
     };
     const payload = JSON.stringify(request);
-    const timeoutMs = resolveDaemonTimeout(timeoutOverrideMs);
-    const response = await new Promise<string>((resolve, reject) => {
+    const parsed = await new Promise<DaemonResponse<T>>((resolve, reject) => {
       const socket = net.createConnection(this.socketPath);
+      const decoder = new DaemonFrameDecoder();
+      let response: DaemonResponse<T> | undefined;
       let settled = false;
       const finishReject = (error: Error): void => {
         if (settled) {
@@ -267,23 +405,31 @@ export class DaemonClient {
         settled = true;
         reject(error);
       };
-      const finishResolve = (value: string): void => {
+      const finishResolve = (value: DaemonResponse<T>): void => {
         if (settled) {
           return;
         }
         settled = true;
         resolve(value);
       };
-      socket.setTimeout(timeoutMs, () => {
-        // If the daemon doesn't answer in time we treat it as a transport error, destroy the socket,
-        // and let invoke() restart the daemon so hung keep-alive servers get a fresh start.
-        socket.destroy(
-          Object.assign(new Error('Daemon request timed out.'), {
-            code: 'ETIMEDOUT',
-          })
-        );
+      socket.setTimeout(idleTimeoutMs);
+      socket.on('timeout', () => {
+        // The daemon stopped proving it is alive. Treat that as a transport error, destroy the
+        // socket, and let invoke() restart the daemon so hung keep-alive servers get a fresh start.
+        socket.destroy(transportError('Daemon request timed out.', 'ETIMEDOUT'));
       });
-      let buffer = '';
+      const consume = (frames: ReturnType<DaemonFrameDecoder['push']>): void => {
+        for (const frame of frames) {
+          if (isDaemonProgressFrame(frame)) {
+            // Proof of life from the daemon: restart the idle deadline.
+            socket.setTimeout(idleTimeoutMs);
+            continue;
+          }
+          response = frame as DaemonResponse<T>;
+          // The answer is in hand; stop policing the socket while it closes.
+          socket.setTimeout(0);
+        }
+      };
       socket.on('connect', () => {
         socket.write(payload, (error) => {
           if (error) {
@@ -293,27 +439,24 @@ export class DaemonClient {
         });
       });
       socket.on('data', (chunk) => {
-        buffer += chunk.toString();
+        consume(decoder.push(chunk.toString()));
+      });
+      socket.on('end', () => {
+        consume(decoder.flush());
+        if (response) {
+          finishResolve(response);
+          return;
+        }
+        finishReject(
+          decoder.malformed
+            ? transportError('Failed to parse daemon response.', 'ECONNRESET')
+            : transportError('Empty daemon response.', 'ECONNRESET')
+        );
       });
-      socket.on('end', () => finishResolve(buffer));
       socket.on('error', (error) => {
         finishReject(error as Error);
       });
     });
-    const trimmed = response.trim();
-    if (!trimmed) {
-      const error = new Error('Empty daemon response.');
-      (error as NodeJS.ErrnoException).code = 'ECONNRESET';
-      throw error;
-    }
-    let parsed: DaemonResponse<T>;
-    try {
-      parsed = JSON.parse(trimmed) as DaemonResponse<T>;
-    } catch {
-      const parseError = new Error('Failed to parse daemon response.');
-      (parseError as NodeJS.ErrnoException).code = 'ECONNRESET';
-      throw parseError;
-    }
     if (!parsed.ok) {
       const error = new Error(parsed.error?.message ?? 'Daemon error');
       (error as NodeJS.ErrnoException).code = parsed.error?.code;
@@ -328,6 +471,12 @@ function deriveConfigKey(configPath: string): string {
   return crypto.createHash('sha1').update(absolute).digest('hex').slice(0, 12);
 }
 
+function transportError(message: string, code: string): Error {
+  const error = new Error(message);
+  (error as NodeJS.ErrnoException).code = code;
+  return error;
+}
+
 function isTransportError(error: unknown): boolean {
   if (!error || typeof error !== 'object') {
     return false;