From fb0c11a013561f53c45b1b78e9d54e7006e08f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 12:44:59 +0200 Subject: [PATCH] fix(doctor): warm the runner cache only for simulators this host discovered Doctor's fresh-machine warmup picked its simulator from provider-first inventory, which drops the discovery source. A simulator a provider reported is not on this host, yet doctor started a local xcodebuild runner build for it. Doctor now keeps the inventory source and names a warmup device only from devices this host discovered. --- .../__tests__/session-doctor-device.test.ts | 38 +++++++- .../__tests__/session-doctor-warmup.test.ts | 90 ++++++++++++++----- src/daemon/handlers/session-doctor-device.ts | 42 ++++++--- src/daemon/handlers/session-doctor.ts | 13 ++- 4 files changed, 148 insertions(+), 35 deletions(-) diff --git a/src/daemon/handlers/__tests__/session-doctor-device.test.ts b/src/daemon/handlers/__tests__/session-doctor-device.test.ts index 53bd87a454..86860aacba 100644 --- a/src/daemon/handlers/__tests__/session-doctor-device.test.ts +++ b/src/daemon/handlers/__tests__/session-doctor-device.test.ts @@ -1,6 +1,9 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; -import { withTestDeviceInventoryProvider as withDeviceInventoryProvider } from '../../../__tests__/test-utils/device-inventory-gateways.ts'; +import { + withTestDeviceInventory, + withTestDeviceInventoryProvider as withDeviceInventoryProvider, +} from '../../../__tests__/test-utils/device-inventory-gateways.ts'; import { AppError } from '@agent-device/kernel/errors'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { attachAdbFailureHint } from '@agent-device/platform-android/mechanics'; @@ -58,3 +61,36 @@ test('doctor android inventory timeout surfaces the wedged-adb-server hint', asy assert.match(androidCheck?.summary ?? '', /timed out after 10000ms/); assert.match(androidCheck?.hint ?? '', /adb kill-server && adb start-server/); }); + +test('doctor inventory names as host devices only the platforms this host discovered', async () => { + const androidEmulator: DeviceInfo = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', + booted: true, + }; + const req = { token: 't', session: 'default', command: 'doctor' } as DaemonRequest; + + const inventory = await withTestDeviceInventory( + { + provider: { + discover: async (request) => + request.platform === 'apple' + ? { kind: 'inventory', devices: [BOOTED_IOS_SIMULATOR] } + : { kind: 'declined' }, + }, + local: async () => [androidEmulator], + }, + async () => await appendDeviceInventoryCheck([], req, undefined), + ); + + assert.deepEqual( + inventory?.devices.map((device) => device.id), + [androidEmulator.id, BOOTED_IOS_SIMULATOR.id], + ); + assert.deepEqual( + inventory?.hostDevices.map((device) => device.id), + [androidEmulator.id], + ); +}); diff --git a/src/daemon/handlers/__tests__/session-doctor-warmup.test.ts b/src/daemon/handlers/__tests__/session-doctor-warmup.test.ts index dad880c522..23654f5456 100644 --- a/src/daemon/handlers/__tests__/session-doctor-warmup.test.ts +++ b/src/daemon/handlers/__tests__/session-doctor-warmup.test.ts @@ -4,6 +4,7 @@ import { isActiveProviderDevice } from '../../provider-device-admission.ts'; import { handleDoctorCommand } from '../session-doctor.ts'; import { createHostDiagnostics } from '../../../platform-runtime-host-diagnostics.ts'; import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; +import { withTestDeviceInventory } from '../../../__tests__/test-utils/device-inventory-gateways.ts'; import type { DaemonResponse } from '../../daemon-request.ts'; const { mockAppleRunnerWarmupCheck } = vi.hoisted(() => ({ @@ -14,14 +15,6 @@ vi.mock('@agent-device/platform-apple/doctor', async (importOriginal) => ({ ...(await importOriginal()), appleRunnerWarmupCheck: mockAppleRunnerWarmupCheck, })); -vi.mock('../session-doctor-device.ts', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - appendDeviceInventoryCheck: vi.fn(async () => undefined), - resolveDoctorDeviceForAppCheck: vi.fn(() => undefined), - }; -}); vi.mock('../session-doctor-app.ts', () => ({ appendAppChecks: vi.fn(async () => {}), })); @@ -68,18 +61,46 @@ async function runDoctorWithSessionDevice(device: DeviceInfo): Promise + await handleDoctorCommand({ + req: { + token: 't', + session: 'doctor-session', + command: 'doctor', + positionals: [], + flags: { session: 'doctor-session' }, + }, + sessionName: 'doctor-session', + sessionStore, + hostDiagnostics: createHostDiagnostics(), + }), + ); +} + +async function runSessionlessDoctor( + source: 'host' | 'provider', + flags: { targetApp?: string } = {}, +): Promise { + const inventory = + source === 'host' + ? { local: async () => [IOS_SIMULATOR] } + : { + provider: { + discover: async () => ({ kind: 'inventory' as const, devices: [IOS_SIMULATOR] }), + }, + }; + return await withTestDeviceInventory( + inventory, + async () => + await handleDoctorCommand({ + req: { token: 't', session: 'default', command: 'doctor', positionals: [], flags }, + sessionName: 'default', + sessionStore: makeSessionStore('agent-device-doctor-warmup-'), + hostDiagnostics: createHostDiagnostics(), + }), + ); } function readCheck(response: DaemonResponse | null, id: string): Record | null { @@ -163,3 +184,32 @@ test('doctor skips the runner warmup for provider-backed devices', async () => { expect(mockAppleRunnerWarmupCheck).toHaveBeenCalledWith(IOS_SIMULATOR, expect.any(Object)); expect(readCheck(response, 'ios-runner-cache')).toBeNull(); }); + +const SESSIONLESS_WARMUP_CANDIDATES = [ + { name: 'an inventory simulator', flags: {} }, + { name: 'the --app device', flags: { targetApp: 'com.example.demo' } }, +]; + +test.each(SESSIONLESS_WARMUP_CANDIDATES)( + 'doctor warms the runner cache for $name this host discovered', + async ({ flags }) => { + const response = await runSessionlessDoctor('host', flags); + + expect(response?.ok).toBe(true); + expect(mockAppleRunnerWarmupCheck).toHaveBeenCalledTimes(1); + expect(mockAppleRunnerWarmupCheck).toHaveBeenCalledWith( + expect.objectContaining({ id: IOS_SIMULATOR.id }), + expect.any(Object), + ); + }, +); + +test.each(SESSIONLESS_WARMUP_CANDIDATES)( + 'doctor starts no runner warmup for $name a provider reported', + async ({ flags }) => { + const response = await runSessionlessDoctor('provider', flags); + + expect(response?.ok).toBe(true); + expect(mockAppleRunnerWarmupCheck).not.toHaveBeenCalled(); + }, +); diff --git a/src/daemon/handlers/session-doctor-device.ts b/src/daemon/handlers/session-doctor-device.ts index bc201d515b..9b9e0ca4fb 100644 --- a/src/daemon/handlers/session-doctor-device.ts +++ b/src/daemon/handlers/session-doctor-device.ts @@ -1,5 +1,5 @@ import { buildDeviceInventoryRequestFromFlags } from '@agent-device/device-selection/dispatch-resolve'; -import { listDeviceInventory } from '@agent-device/device-selection/device-inventory-context'; +import { readDeviceInventory } from '@agent-device/device-selection/device-inventory-context'; import { countDeviceInventoryByGroup, LOCAL_DEVICE_INVENTORY_PLATFORM_SELECTORS, @@ -18,10 +18,13 @@ import { normalizeError } from '@agent-device/kernel/errors'; import type { DaemonRequest } from '../daemon-request.ts'; import type { SessionState } from '../session-state.ts'; import type { DoctorCheck } from '@agent-device/contracts/observability'; +import type { DeviceInventoryDiscovery } from '@agent-device/contracts/platform-module'; import { appendDoctorCheck } from './session-doctor-output.ts'; export type DoctorDeviceInventory = { devices: DeviceInfo[]; + /** The devices this host discovered itself; a provider-reported device is not on this host. */ + hostDevices: DeviceInfo[]; platform?: PlatformSelector; target?: DeviceTarget; }; @@ -53,7 +56,12 @@ export async function appendDeviceInventoryCheck( if (devices.length > 0) { appendInventoryFailureChecks(checks, inventory.failures); } - return { devices, platform: selector.platform, target: selector.target }; + return { + devices, + hostDevices: devices.filter((device) => inventory.hostDevices.includes(device)), + platform: selector.platform, + target: selector.target, + }; } catch (error) { const normalized = normalizeError(error); appendDoctorCheck(checks, { @@ -64,7 +72,7 @@ export async function appendDeviceInventoryCheck( command: 'agent-device devices', evidence: { code: normalized.code, details: normalized.details }, }); - return { devices: [], platform: selector.platform, target: selector.target }; + return { devices: [], hostDevices: [], platform: selector.platform, target: selector.target }; } } @@ -126,23 +134,37 @@ function filterInventoryForSelector( ); } -async function readDoctorDeviceInventory( - selector: DeviceInventoryRequest, -): Promise<{ devices: DeviceInfo[]; failures: DoctorInventoryFailure[] }> { +async function readDoctorDeviceInventory(selector: DeviceInventoryRequest): Promise<{ + devices: DeviceInfo[]; + hostDevices: DeviceInfo[]; + failures: DoctorInventoryFailure[]; +}> { if (selector.platform) { - return { devices: await listDeviceInventory(selector), failures: [] }; + return { ...combineDiscoveries([await readDeviceInventory(selector)]), failures: [] }; } - const devices: DeviceInfo[] = []; + const discoveries: DeviceInventoryDiscovery[] = []; const failures: DoctorInventoryFailure[] = []; for (const platform of LOCAL_DEVICE_INVENTORY_PLATFORM_SELECTORS) { try { - devices.push(...(await listDeviceInventory({ ...selector, platform }))); + discoveries.push(await readDeviceInventory({ ...selector, platform })); } catch (error) { failures.push(inventoryFailure(platform, error)); } } - return { devices, failures }; + return { ...combineDiscoveries(discoveries), failures }; +} + +function combineDiscoveries(discoveries: readonly DeviceInventoryDiscovery[]): { + devices: DeviceInfo[]; + hostDevices: DeviceInfo[]; +} { + return { + devices: discoveries.flatMap((discovery) => discovery.devices), + hostDevices: discoveries.flatMap((discovery) => + discovery.source === 'local' ? discovery.devices : [], + ), + }; } function appendInventoryFailureChecks( diff --git a/src/daemon/handlers/session-doctor.ts b/src/daemon/handlers/session-doctor.ts index 23371a69a8..85a8667cca 100644 --- a/src/daemon/handlers/session-doctor.ts +++ b/src/daemon/handlers/session-doctor.ts @@ -96,7 +96,7 @@ export async function handleDoctorCommand(params: { bindDevice, req, }); - const warmupDevice = appCheckDevice ?? resolveWarmupSimulator(inventory); + const warmupDevice = session?.device ?? resolveHostWarmupDevice(inventory, appCheckDevice); if (warmupDevice) { const warmup = await hostDiagnostics.warmupCheck(warmupDevice, context); if (warmup) appendDoctorCheck(checks, warmup); @@ -137,11 +137,16 @@ function hostDiagnosticsContext( // background so the first `open` skips the ~10s xcodebuild build. The check // line makes the warmup visible either way. Any simulator record works as // the build device — the artifact builds against a generic simulator -// destination and is shared across simulators and runtimes. -function resolveWarmupSimulator( +// destination and is shared across simulators and runtimes. Inventory names +// the device only from what this host discovered: a provider-reported +// simulator is not on this host, so the host runner cache is not its to warm. +function resolveHostWarmupDevice( inventory: DoctorDeviceInventory | undefined, + appCheckDevice: DeviceInfo | undefined, ): DeviceInfo | undefined { - const simulators = (inventory?.devices ?? []).filter( + const hostDevices = inventory?.hostDevices ?? []; + if (appCheckDevice) return hostDevices.includes(appCheckDevice) ? appCheckDevice : undefined; + const simulators = hostDevices.filter( (device) => isIosFamily(device) && device.kind === 'simulator', ); return simulators.find((device) => device.booted === true) ?? simulators[0];