fix(performance): retry failed CPU and FPS closes instead of blocking new sessions

packages/shared/test/display-fps.test.ts

@@ -10,6 +10,7 @@ import { DisplayFpsPanel } from "../../ui/src/display-fps-panel.ts";
 import type { DisplayFpsState } from "../../ui/src/display-fps-panel.ts";
 import type { App } from "@modelcontextprotocol/ext-apps";
 import type { DisplayFpsBatch } from "../src/display-fps.ts";
+import type { SimulatorDevice } from "../src/protocol.ts";
 import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
 import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
@@ -404,6 +405,110 @@ test("the device FPS panel records on physical iOS without an app, backfills his
   assert.equal(state.fpsPhase, "stopped");
 });
 
+function closeFailureHost() {
+  const calls: string[] = [];
+  let closeFailure: string | undefined;
+  let opening: Promise<void> | undefined;
+  let sequence = 0;
+  let emit: ((batch: DisplayFpsBatch) => void) | undefined;
+  const app = {
+    async callServerTool(input: { name: string; arguments: { sessionId?: string; target?: { deviceId: string } } }) {
+      if (input.name === "mobile_display_fps_close") {
+        calls.push(`close:${input.arguments.sessionId}`);
+        if (closeFailure) throw new Error(closeFailure);
+        return { content: [] };
+      }
+      await opening;
+      const id = `fps${++sequence}`;
+      calls.push(`open:${id}:${input.arguments.target?.deviceId}`);
+      return { content: [], structuredContent: { sessionId: id, fpsUri: `display-fps://mobile-dev/${id}/batch?after=0`, timeOrigin: 99 } };
+    },
+    readServerResource(input: { uri: string }, options: { signal: AbortSignal }) {
+      return new Promise((resolve, reject) => {
+        const cancelled = () => reject(new Error("cancelled"));
+        options.signal.addEventListener("abort", cancelled, { once: true });
+        emit = batch => {
+          options.signal.removeEventListener("abort", cancelled);
+          const text = JSON.stringify(batch);
+          resolve({ contents: [{ uri: input.uri, mimeType: "application/json", text }] });
+        };
+      });
+    },
+  };
+  let state: Partial<DisplayFpsState> = {};
+  const panel = new DisplayFpsPanel(app as unknown as App, fields => { state = { ...state, ...fields }; });
+  return { panel, calls, state: () => state, emit(batch: DisplayFpsBatch) { emit?.(batch); },
+    failClose(message?: string) { closeFailure = message; }, delayOpen(promise: Promise<void>) { opening = promise; } };
+}
+const pixel: SimulatorDevice = { platform: "android", udid: "emulator-5554", name: "Pixel", state: "Booted", runtime: "Android" };
+async function settle() { for (let i = 0; i < 8; i++) await setImmediate(); }
+
+test("a transient Display FPS close failure stays visible and is retried before the next session opens", async () => {
+  const fake = closeFailureHost();
+  const { panel, calls } = fake;
+  panel.select(pixel);
+  panel.setAvailable(true);
+  panel.show();
+  await settle();
+  assert.deepEqual(calls, ["open:fps1:emulator-5554"]);
+
+  fake.failClose("transient close failure");
+  await assert.rejects(panel.stop(), /transient close failure/);
+  panel.retry();
+  await settle();
+  assert.equal(fake.state().fpsPhase, "failed");
+  assert.equal(fake.state().fpsError, "transient close failure");
+  assert.equal(fake.state().fpsMonitoring, false);
+  assert.deepEqual(calls.slice(1), ["close:fps1", "close:fps1"], "No session opens beside one that may still be collecting.");
+
+  fake.failClose();
+  panel.retry();
+  await settle();
+  assert.deepEqual(calls.slice(3), ["close:fps1", "open:fps2:emulator-5554"]);
+  fake.emit({ cursor: 1, phase: "recording", samples: [{ time: 100, interval: 1, fps: 60 }] });
+  await settle();
+  assert.equal(fake.state().fpsPhase, "recording");
+  assert.equal(fake.state().fpsError, "");
+  assert.deepEqual(fake.state().fpsSamples, [{ time: 100, interval: 1, fps: 60 }]);
+
+  fake.failClose("transient close failure");
+  await assert.rejects(panel.stop(), /transient close failure/);
+  fake.failClose();
+  panel.show();
+  await settle();
+  assert.deepEqual(calls.slice(5), ["close:fps2", "close:fps2", "open:fps3:emulator-5554"], "Showing the panel again after recovery closes the stale session first.");
+  await panel.disconnect();
+  assert.deepEqual(calls.slice(8), ["close:fps3"]);
+});
+
+test("a Display FPS session that opens after a device switch keeps its failed close for retry", async () => {
+  const fake = closeFailureHost();
+  const { panel, calls } = fake;
+  let resolveOpen!: () => void;
+  fake.delayOpen(new Promise(resolve => { resolveOpen = resolve; }));
+  panel.select(pixel);
+  panel.setAvailable(true);
+  panel.show();
+  await settle();
+  panel.select({ ...pixel, udid: "emulator-5556", name: "Tablet" });
+  fake.failClose("transient close failure");
+  resolveOpen();
+  await settle();
+  assert.equal(fake.state().fpsPhase, "failed");
+  assert.equal(fake.state().fpsError, "transient close failure");
+  assert.deepEqual(calls, ["open:fps1:emulator-5554", "close:fps1", "close:fps1"], "The new device waits for the late session to close.");
+
+  fake.failClose();
+  panel.retry();
+  await settle();
+  assert.deepEqual(calls.slice(3), ["close:fps1", "open:fps2:emulator-5556"]);
+  fake.emit({ cursor: 1, phase: "recording", samples: [{ time: 100, interval: 1, fps: 60 }] });
+  await settle();
+  assert.equal(fake.state().fpsPhase, "recording");
+  await panel.disconnect();
+  assert.deepEqual(calls.slice(5), ["close:fps2"]);
+});
+
 test("CPU and display samples share a clock and selecting another app preserves the device recording", async t => {
   const calls: string[] = [];
   const reads = new Map<string, (batch: CpuBatch | DisplayFpsBatch) => void>();

packages/ui/src/display-fps-panel.ts

@@ -29,6 +29,7 @@ export class DisplayFpsPanel {
       const message = errorMessage(error);
       this.update({ fpsPhase: "failed", fpsError: message });
     });
+    return stopped;
   }
   setAvailable(available: boolean) {
     this.available = available;
@@ -43,12 +44,12 @@ export class DisplayFpsPanel {
     this.samples = [];
     const supported = device?.platform === "android" || device?.kind === "physical";
     this.update({ ...initialFpsState, fpsSupported: supported });
-    this.stopSafely();
-    if (this.enabled) this.start();
+    const stopped = this.stopSafely();
+    if (this.enabled) this.start(stopped);
   }
   show() { this.enabled = true; this.start(); }
-  retry() { this.stopSafely(); this.start(); }
-  private start() {
+  retry() { this.start(this.stopSafely()); }
+  private start(cleanup = this.cleanup) {
     const device = this.device;
     if (this.running || this.enabled === false || this.available === false || device === undefined) return;
     const platform = device.platform ?? "ios";
@@ -57,7 +58,7 @@ export class DisplayFpsPanel {
     const epoch = this.epoch;
     const target: DisplayFpsTarget = { platform, deviceId: device.udid };
     this.update({ fpsPhase: "connecting", fpsError: "", fpsMonitoring: true });
-    const receiving = this.receive(target, epoch, this.cleanup);
+    const receiving = this.receive(target, epoch, cleanup);
     this.running = receiving.catch(error => {
       if (epoch !== this.epoch) return;
       const message = errorMessage(error);
@@ -76,19 +77,20 @@ export class DisplayFpsPanel {
   }
   private async receive(target: DisplayFpsTarget, epoch: number, cleanup: Promise<void>) {
     await cleanup;
+    if (epoch === this.epoch && this.session) await this.closeSession(this.session);
     if (epoch !== this.epoch) return;
     const opened = await this.call("mobile_display_fps_session", { target });
     const id = opened.structuredContent?.sessionId;
     const fpsUri = opened.structuredContent?.fpsUri;
     const origin = opened.structuredContent?.timeOrigin;
     if (typeof id !== "string") throw new Error("The plugin did not return a Display FPS session.");
     const session: Session = { id, abort: new AbortController() };
+    this.session = session;
     if (typeof fpsUri !== "string" || typeof origin !== "number") {
       await this.closeSession(session);
       throw new Error("The plugin did not return a Display FPS resource and clock.");
     }
     if (epoch !== this.epoch) { await this.closeSession(session); return; }
-    this.session = session;
     this.update({}, origin);
     const uri = new URL(fpsUri);
     let after = 0;
@@ -118,13 +120,18 @@ export class DisplayFpsPanel {
       }
     } finally { await this.closeSession(session); }
   }
+  // A session stays current until its close succeeds, so a failed close is retried before another session opens.
   private closeSession(session: Session) {
     session.abort.abort();
     if (session.closing === undefined) {
       const closing = this.call("mobile_display_fps_close", { sessionId: session.id });
-      session.closing = closing.then(() => {});
+      session.closing = closing.then(() => {
+        if (this.session === session) this.session = undefined;
+      }, error => {
+        session.closing = undefined;
+        throw error;
+      });
     }
-    if (this.session === session) this.session = undefined;
     return session.closing;
   }
   stop() {
@@ -133,9 +140,9 @@ export class DisplayFpsPanel {
     const running = this.running;
     this.running = undefined;
     const closing = session ? this.closeSession(session) : Promise.resolve();
-    const finishing = Promise.all([this.cleanup, closing, running]);
+    const finishing = Promise.allSettled([this.cleanup, closing, running]);
     this.cleanup = finishing.then(() => {});
-    return this.cleanup;
+    return finishing.then(() => closing);
   }
   async disconnect() {
     this.enabled = false;

packages/ui/src/performance-panel.ts

@@ -189,14 +189,15 @@ export class PerformancePanel {
 
   private async receive(target: CpuTarget, epoch: number, stopping: Promise<void>) {
     await stopping;
+    if (epoch === this.epoch && this.session) await this.closeSession(this.session);
     if (epoch !== this.epoch || this.disposed) return;
     const result = await this.call("mobile_cpu_session", { target });
     const id = result.structuredContent?.sessionId;
     const cpuUri = result.structuredContent?.cpuUri;
     if (typeof id !== "string" || typeof cpuUri !== "string") throw new Error("The plugin did not return a CPU session.");
     const session: Session = { id, abort: new AbortController() };
-    if (epoch !== this.epoch || this.disposed) { await this.closeSession(session); return; }
     this.session = session;
+    if (epoch !== this.epoch || this.disposed) { await this.closeSession(session); return; }
     const uri = new URL(cpuUri);
     let after = 0;
     try {
@@ -242,10 +243,15 @@ export class PerformancePanel {
     } finally { await this.closeSession(session); }
   }
 
+  // A session stays current until its close succeeds, so a failed close is retried before another session opens.
   private closeSession(session: Session): Promise<void> {
     session.abort.abort();
-    if (session.closing === undefined) session.closing = this.call("mobile_cpu_close", { sessionId: session.id }).then(() => {});
-    if (this.session === session) this.session = undefined;
+    session.closing ??= this.call("mobile_cpu_close", { sessionId: session.id }).then(() => {
+      if (this.session === session) this.session = undefined;
+    }, error => {
+      session.closing = undefined;
+      throw error;
+    });
     return session.closing;
   }
 
@@ -254,10 +260,11 @@ export class PerformancePanel {
     const session = this.session;
     const running = this.running;
     this.running = undefined;
-    const previous = this.cleanup;
     const closing = session ? this.closeSession(session) : Promise.resolve();
-    this.cleanup = Promise.all([previous, closing, running]).then(() => {});
-    return this.cleanup;
+    const settled = Promise.allSettled([this.cleanup, closing, running]);
+    this.cleanup = settled.then(() => {});
+    await settled;
+    return closing;
   }
 
   async dispose() {

packages/ui/test/performance-panel.test.ts

@@ -18,6 +18,7 @@ function host() {
   const targets: CpuTarget[] = [];
   let apps: DeviceApp[] = [{ bundleId: "com.example.app", pid: 123, foreground: true }];
   let sequence = 0;
+  let closeFailure: string | undefined;
   let opening: Promise<void> | undefined;
   let emit: ((batch: CpuBatch) => void) | undefined;
   const app = {
@@ -29,7 +30,11 @@ function host() {
         return { content: [], structuredContent: { apps: marked, foregroundApp: foreground ?? null } };
       }
       if (input.name === "mobile_display_fps_session") return { isError: true, content: [{ type: "text", text: "FPS not provided by this CPU fixture" }] };
-      if (input.name === "mobile_cpu_close") { events.push(`close:${input.arguments.sessionId}`); return { content: [] }; }
+      if (input.name === "mobile_cpu_close") {
+        events.push(`close:${input.arguments.sessionId}`);
+        if (closeFailure) throw new Error(closeFailure);
+        return { content: [] };
+      }
       await opening;
       if (input.arguments.target) targets.push(input.arguments.target);
       const id = String(++sequence).padStart(64, "0");
@@ -49,7 +54,8 @@ function host() {
     },
   };
   return { app: app as unknown as App, events, sources, targets, emit(batch: CpuBatch) { emit?.(batch); },
-    setApps(next: DeviceApp[]) { apps = next; }, delayOpen(promise: Promise<void>) { opening = promise; } };
+    setApps(next: DeviceApp[]) { apps = next; }, delayOpen(promise: Promise<void>) { opening = promise; },
+    failClose(message?: string) { closeFailure = message; } };
 }
 
 test("CPU collection defaults to the foreground app, persists across tabs, and resets on PID changes", async t => {
@@ -169,6 +175,73 @@ test("closing during session creation closes the late session before disposal co
   assert.equal(panel.getSnapshot().open, false);
 });
 
+test("a failed CPU close is reported and retried before the next session opens", async t => {
+  const fake = host();
+  const deviceApps = createDeviceApps(fake.app);
+  const panel = new PerformancePanel(fake.app, deviceApps);
+  t.after(() => { deviceApps.dispose(); return panel.dispose(); });
+  deviceApps.selectDevice(device);
+  panel.setAvailable(true);
+  deviceApps.setAvailable(true);
+  panel.show();
+  await tick();
+  const first = String(1).padStart(64, "0");
+  assert.deepEqual(fake.events, [`open:${first}`]);
+
+  fake.failClose("transient close failure");
+  panel.retry();
+  await tick();
+  const failed = panel.getSnapshot();
+  assert.equal(failed.phase, "failed");
+  assert.equal(failed.error, "transient close failure");
+  assert.equal(failed.monitoring, false);
+  assert.deepEqual(fake.events, [`open:${first}`, `close:${first}`], "No session opens beside one that may still be collecting.");
+
+  fake.failClose();
+  panel.retry();
+  await tick();
+  const second = String(2).padStart(64, "0");
+  assert.deepEqual(fake.events, [`open:${first}`, `close:${first}`, `close:${first}`, `open:${second}`]);
+  fake.emit({ cursor: 1, phase: "recording", memoryMetric: "physical-footprint", samples: [{ time: 1, interval: 1, cpuPercent: 40, memoryBytes: 104857600, threads: [] }] });
+  await tick();
+  const recovered = panel.getSnapshot();
+  assert.equal(recovered.phase, "recording");
+  assert.equal(recovered.error, "");
+  assert.equal(recovered.samples[0].cpuPercent, 40);
+});
+
+test("a CPU session that opens after a stop keeps its failed close for retry", async t => {
+  const fake = host();
+  let resolveOpen!: () => void;
+  fake.delayOpen(new Promise(resolve => { resolveOpen = resolve; }));
+  const deviceApps = createDeviceApps(fake.app);
+  const panel = new PerformancePanel(fake.app, deviceApps);
+  t.after(() => { deviceApps.dispose(); return panel.dispose(); });
+  deviceApps.selectDevice(device);
+  panel.setAvailable(true);
+  deviceApps.setAvailable(true);
+  panel.show();
+  await tick();
+  panel.retry();
+  fake.failClose("transient close failure");
+  resolveOpen();
+  await tick();
+  const first = String(1).padStart(64, "0");
+  const failed = panel.getSnapshot();
+  assert.equal(failed.phase, "failed");
+  assert.equal(failed.error, "transient close failure");
+  assert.deepEqual(fake.events, [`open:${first}`, `close:${first}`, `close:${first}`], "The restart waits for the late session to close.");
+
+  fake.failClose();
+  panel.retry();
+  await tick();
+  const second = String(2).padStart(64, "0");
+  assert.deepEqual(fake.events.slice(3), [`close:${first}`, `open:${second}`]);
+  fake.emit({ cursor: 1, phase: "recording", memoryMetric: "physical-footprint", samples: [{ time: 1, interval: 1, cpuPercent: 40, memoryBytes: 104857600, threads: [] }] });
+  await tick();
+  assert.equal(panel.getSnapshot().phase, "recording");
+});
+
 test("switching between iOS and Android closes the previous monitor and selects the active device", async t => {
   const fake = host();
   const deviceApps = createDeviceApps(fake.app);