diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/AgentDeviceRunnerUITests-Bridging-Header.h b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/AgentDeviceRunnerUITests-Bridging-Header.h index 090e1b31e8..f448d0ae80 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/AgentDeviceRunnerUITests-Bridging-Header.h +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/AgentDeviceRunnerUITests-Bridging-Header.h @@ -15,3 +15,7 @@ #if __has_include("UnitTests/RunnerEdgeContactTesting.h") #import "UnitTests/RunnerEdgeContactTesting.h" #endif + +#if __has_include("UnitTests/RunnerSnapshotBridgeTesting.h") +#import "UnitTests/RunnerSnapshotBridgeTesting.h" +#endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSnapshotBridge.m b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSnapshotBridge.m index 0bd9ccf50e..b224ab7290 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSnapshotBridge.m +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSnapshotBridge.m @@ -179,10 +179,13 @@ static NSUInteger RunnerSnapshotBridgeBudgetMs(NSDictionary *request) return MIN(budget.unsignedIntegerValue, kMaximumDurationMs); } -/// 1 while a read is admitted and has not returned, including a read abandoned past its budget: the +/// A read stays admitted until it returns, including a read abandoned past its budget: the /// accessibility call it waits on cannot be cancelled, so later reads are refused until it returns /// rather than queued behind it. -static atomic_int RunnerSnapshotBridgeReadsInFlight = 0; +static BOOL RunnerSnapshotBridgeReadsInFlight = NO; +static NSUInteger RunnerSnapshotBridgeReadSequence = 0; +static NSTimeInterval RunnerSnapshotBridgeAbandonedAt = 0; +static const NSTimeInterval RunnerSnapshotBridgeWedgeSeconds = 5; static dispatch_queue_t RunnerSnapshotBridgeReaderQueue(void) { @@ -206,13 +209,21 @@ @implementation RunnerSnapshotBridgeAnswer /// is abandoned, not stopped: its answer is dropped when it returns. static NSDictionary *RunnerSnapshotBridgeRead(NSDictionary *request, NSUInteger budgetMs, - NSDictionary * (^read)(void)) + NSDictionary * (^read)(void), + NSTimeInterval (^clock)(void)) { - // Admission is one atomic step, because requests arrive on a concurrent queue. - int idle = 0; - if (!atomic_compare_exchange_strong(&RunnerSnapshotBridgeReadsInFlight, &idle, 1)) { - return RunnerSnapshotBridgeFailure(request, @"application_not_responding", @"bridge-read-in-flight", - @"another accessibility read has not returned yet"); + NSUInteger sequence; + @synchronized (RunnerSnapshotBridgeAnswer.class) { + if (RunnerSnapshotBridgeReadsInFlight) { + BOOL wedged = RunnerSnapshotBridgeAbandonedAt > 0 && + clock() - RunnerSnapshotBridgeAbandonedAt >= RunnerSnapshotBridgeWedgeSeconds; + return RunnerSnapshotBridgeFailure(request, @"application_not_responding", + wedged ? @"bridge-reader-wedged" : @"bridge-read-in-flight", + @"another accessibility read has not returned yet"); + } + RunnerSnapshotBridgeReadsInFlight = YES; + RunnerSnapshotBridgeAbandonedAt = 0; + sequence = ++RunnerSnapshotBridgeReadSequence; } RunnerSnapshotBridgeAnswer *answer = [RunnerSnapshotBridgeAnswer new]; dispatch_semaphore_t done = dispatch_semaphore_create(0); @@ -223,16 +234,36 @@ @implementation RunnerSnapshotBridgeAnswer answer.value = RunnerSnapshotBridgeFailure(request, @"reader_unavailable", @"reader-exception", exception.reason ?: @"the snapshot reader raised an exception"); } - atomic_store(&RunnerSnapshotBridgeReadsInFlight, 0); + @synchronized (RunnerSnapshotBridgeAnswer.class) { + RunnerSnapshotBridgeReadsInFlight = NO; + RunnerSnapshotBridgeAbandonedAt = 0; + } dispatch_semaphore_signal(done); }); if (dispatch_semaphore_wait(done, dispatch_time(DISPATCH_TIME_NOW, (int64_t)budgetMs * NSEC_PER_MSEC)) != 0) { + @synchronized (RunnerSnapshotBridgeAnswer.class) { + if (RunnerSnapshotBridgeReadsInFlight && RunnerSnapshotBridgeReadSequence == sequence) { + RunnerSnapshotBridgeAbandonedAt = clock(); + } + } return RunnerSnapshotBridgeFailure(request, @"application_not_responding", @"bridge-read-abandoned", @"the accessibility read outlived its budget and was abandoned"); } return answer.value; } +#if defined(AGENT_DEVICE_RUNNER_UNIT_TESTS) +NSDictionary *RunnerSnapshotBridgeReadForTest(NSDictionary *request, NSUInteger budgetMs, + NSDictionary *(^read)(void), NSTimeInterval now) +{ + return RunnerSnapshotBridgeRead(request, budgetMs, read, ^{ return now; }); +} +void RunnerSnapshotBridgeDrainForTest(void) +{ + dispatch_sync(RunnerSnapshotBridgeReaderQueue(), ^{}); +} +#endif + uint32_t RunnerSnapshotBridgeMaximumFrameBytes(void) { return kMaximumFrameBytes; @@ -248,12 +279,12 @@ uint32_t RunnerSnapshotBridgeMaximumFrameBytes(void) if ([verb isEqualToString:@"target"]) { return RunnerSnapshotBridgeRead(request, RunnerSnapshotBridgeBudgetMs(request), ^{ return RunnerSnapshotBridgeTarget(bundleId, request); - }); + }, ^{ return NSProcessInfo.processInfo.systemUptime; }); } if ([verb isEqualToString:@"describe"]) { return RunnerSnapshotBridgeRead(request, RunnerSnapshotBridgeBudgetMs(request), ^{ return RunnerSnapshotBridgeDescribe(bundleId, request); - }); + }, ^{ return NSProcessInfo.processInfo.systemUptime; }); } return RunnerSnapshotBridgeFailure(request, @"bad_request", @"verb-not-supported", @"the runner snapshot bridge accepts target and describe requests only"); diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerSnapshotBridgeTesting.h b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerSnapshotBridgeTesting.h new file mode 100644 index 0000000000..07f2b5b93c --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerSnapshotBridgeTesting.h @@ -0,0 +1,9 @@ +#import +#import +NS_ASSUME_NONNULL_BEGIN +#if TARGET_OS_IOS && TARGET_OS_SIMULATOR +NSDictionary *RunnerSnapshotBridgeReadForTest(NSDictionary *request, NSUInteger budgetMs, + NSDictionary *(^read)(void), NSTimeInterval now); +void RunnerSnapshotBridgeDrainForTest(void); +#endif +NS_ASSUME_NONNULL_END diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotBridgeTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotBridgeTests.swift index fe1e50257b..deb1bf6b20 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotBridgeTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotBridgeTests.swift @@ -66,6 +66,24 @@ extension RunnerTests { let named = Self.snapshotBridgeReaderRequest(payload, simulatorUdid: "SIM") XCTAssertEqual(named["simulatorUdid"] as? String, "SIM") } + func testAbandonedReadBecomesWedgedUntilItReturns() { + let release = DispatchSemaphore(value: 0) + let request = ["requestId": "wedge"] + let first = RunnerSnapshotBridgeReadForTest(request, 100, { + release.wait() + return ["ok": true] + }, 10) + XCTAssertEqual(first["error_code"] as? String, "bridge-read-abandoned") + let busy = RunnerSnapshotBridgeReadForTest(request, 100, { ["ok": true] }, 14.999) + XCTAssertEqual(busy["error_code"] as? String, "bridge-read-in-flight") + let wedged = RunnerSnapshotBridgeReadForTest(request, 100, { ["ok": true] }, 15) + XCTAssertEqual(wedged["error_code"] as? String, "bridge-reader-wedged") + release.signal() + RunnerSnapshotBridgeDrainForTest() + let admitted = RunnerSnapshotBridgeReadForTest(request, 1000, { ["ok": true] }, 16) + XCTAssertEqual(admitted["ok"] as? Bool, true) + } + func testReaderDoesNotRefuseTheBoundsOfAFirstRead() throws { var request = Self.snapshotBridgeReaderRequest(try firstReadPayload()) request["pid"] = 1 diff --git a/contracts/fixtures/ios-snapshot-bridge-refusals.json b/contracts/fixtures/ios-snapshot-bridge-refusals.json index 75bddb823a..cf349b7137 100644 --- a/contracts/fixtures/ios-snapshot-bridge-refusals.json +++ b/contracts/fixtures/ios-snapshot-bridge-refusals.json @@ -1,5 +1,5 @@ { - "description": "Every refusal code the snapshot bridge emits, with the scope the route gives it. A capture-scoped refusal speaks for one capture and keeps the app generation on the bridge; every other code retires the generation. Emitters: the Objective-C reader (apple/snapshot-bridge), the runner-served bridge (RunnerSnapshotBridge.m, RunnerTests+SnapshotBridge.swift), and the TypeScript snapshot source (packages/platform-apple/src/snapshot-source, runner-snapshot-bridge.ts). The TypeScript policy packages/platform-apple/src/snapshot-route-failure-policy.ts mirrors this table. test/repo-guards/snapshot-bridge-refusals.test.ts fails when an emitted code is missing here, when a code here is emitted nowhere, or when the policy disagrees. Classify a new code here by an explicit decision; an unlisted code fails the guard.", + "description": "Every refusal code the snapshot bridge emits, with the scope the route gives it. A capture-scoped refusal speaks for one capture and keeps the app generation on the bridge; generation-scoped codes retire the generation, and runner-session-scoped codes retire the runner bridge. Emitters: the Objective-C reader (apple/snapshot-bridge), the runner-served bridge (RunnerSnapshotBridge.m, RunnerTests+SnapshotBridge.swift), and the TypeScript snapshot source (packages/platform-apple/src/snapshot-source, runner-snapshot-bridge.ts). The TypeScript policy packages/platform-apple/src/snapshot-route-failure-policy.ts mirrors this table. test/repo-guards/snapshot-bridge-refusals.test.ts fails when an emitted code is missing here, when a code here is emitted nowhere, or when the policy disagrees. Classify a new code here by an explicit decision; an unlisted code fails the guard.", "captureScoped": [ { "code": "window-coordinate-space-unresolved", @@ -112,5 +112,6 @@ "unknown", "user-interaction-invalid", "verb-not-supported" - ] + ], + "runnerSessionScoped": ["bridge-reader-wedged"] } diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index f91394d5c1..21027f27a5 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -811,7 +811,11 @@ The runner must not end itself, because it also runs every XCTest command. The runner abandons such a read (`bridge-read-abandoned`). The runner admits one read at a time. Until the admitted read returns, the runner refuses each new read (`bridge-read-in-flight`). -A read that a cancelled capture left running meets that refusal too. +After an abandoned read stays in flight for five seconds, the runner refuses with `bridge-reader-wedged`. +That refusal retires the bridge for the runner session, across app generations. +The route clears its circuit and cached targets when the runner session changes. +The runner does not replace the reader queue, because a second concurrent accessibility read is not proven safe. +A read that a cancelled capture left running meets the in-flight refusal too. The refusal says only that another read is running, so it has capture scope. The daemon gives the runner its read budget minus a transport margin. Thus, the runner abandons a slow read first, and the daemon receives `bridge-read-abandoned`, not a lost reply. diff --git a/packages/platform-apple/src/runner-runtime.ts b/packages/platform-apple/src/runner-runtime.ts index 252dce109f..7c8f2f3600 100644 --- a/packages/platform-apple/src/runner-runtime.ts +++ b/packages/platform-apple/src/runner-runtime.ts @@ -59,6 +59,7 @@ export function createAppleRunnerRuntime(options: { // Every capture of a remote Simulator reaches the runner, through the bridge or through XCTest. So // the foreground read starts a runner that is not up yet, under the capture's own budget, instead // of giving up the proof that keeps the capture on the bridge. + let snapshotRoute: SnapshotBridgeRoute | undefined; const readForeground = createSystemSurfaceForegroundProbe(async (device, input) => { if (!options.provider.hasLiveSession(device)) { try { @@ -73,9 +74,9 @@ export function createAppleRunnerRuntime(options: { } } const session = readRunnerSessionLiveness(device.id); - return session?.liveness === 'ready' ? { sessionId: session.sessionId } : undefined; + if (session?.liveness !== 'ready') return undefined; + return { sessionId: session.sessionId }; }); - let snapshotRoute: SnapshotBridgeRoute | undefined; /** * The route outlives every request binding: its circuit, depth hints, and cached target describe * the app generations this runner has read. The presenter comes with the first binding and is the @@ -95,6 +96,7 @@ export function createAppleRunnerRuntime(options: { // The runner answers the target read itself; there is no detached discovery to wait on. isDiscoveryPending: () => false, targetFreshness: 'cached', + sessionChanged: bridge.resetTargets, }, async (device, input) => await withAppleRunnerProvider( diff --git a/packages/platform-apple/src/runner-snapshot-bridge.test.ts b/packages/platform-apple/src/runner-snapshot-bridge.test.ts index c59b92c187..d4ef0b16b8 100644 --- a/packages/platform-apple/src/runner-snapshot-bridge.test.ts +++ b/packages/platform-apple/src/runner-snapshot-bridge.test.ts @@ -143,7 +143,7 @@ function verbs(sent: readonly RunnerCommand[]): string[] { } /** The bridge route of a runner whose foreground read proves no system surface covers the app. */ -function bridgeRoute(provider: AppleRunnerProvider) { +function bridgeRoute(provider: AppleRunnerProvider, runnerSessionId?: string) { const bridge = createRunnerSnapshotBridge({ device: IOS_SIMULATOR, provider }); const presented: SnapshotRuntimeAcquiredResult[] = []; const fallbacks: CaptureSnapshotInput[] = []; @@ -162,8 +162,10 @@ function bridgeRoute(provider: AppleRunnerProvider) { resolveTarget: bridge.resolveTarget, isDiscoveryPending: () => false, targetFreshness: 'cached', + ...(runnerSessionId ? { sessionChanged: bridge.resetTargets } : {}), }, - async () => 'background', + async () => + runnerSessionId ? { foreground: 'background', sessionId: runnerSessionId } : 'background', ); const snapshot = (signal = new AbortController().signal) => route.capture( @@ -485,3 +487,183 @@ test('a runner whose session id cannot be read answers the foreground proof as u expect(fallbacks).toHaveLength(1); expect(sent.map((command) => command.command)).not.toContain('appState'); }); + +test('a wedged runner retires the bridge for the session across captures and app generations', async () => { + const state: { process: RunnerProcess; refuseTarget?: string; refuseDescribe?: string } = { + process: { pid: 41, processStartTime: '1700000000.000001' }, + refuseDescribe: 'bridge-read-abandoned', + }; + const { provider, sent } = bridgeRunner(state); + const { snapshot, fallbacks } = runtimeSnapshots(withHostStates(provider, 'notRunning')); + await snapshot(); + state.refuseTarget = 'bridge-reader-wedged'; + const wedged = await snapshot(); + state.process = { pid: 77, processStartTime: '1700000100.000002' }; + const later = await snapshot(); + const last = await snapshot(); + for (const result of [wedged, later, last]) { + expect(result.warnings).toEqual([ + 'Simulator AX snapshot unavailable (bridge-reader-wedged); used XCTest for this runner session.', + ]); + } + expect(verbs(sent)).toEqual(['target', 'describe', 'target']); + expect(fallbacks).toHaveLength(4); +}); + +test('a wedged refusal retires the session after a held read', async () => { + const state: { + process: RunnerProcess; + holdNextDescribe?: Promise; + refuseDescribe?: string; + } = { + process: { pid: 41, processStartTime: '1700000000.000001' }, + }; + const { provider, sent } = bridgeRunner(state); + const { snapshot } = bridgeRoute(provider, 'runner-session-1'); + await snapshot(); + state.holdNextDescribe = new Promise(() => {}); + const controller = new AbortController(); + const pending = snapshot(controller.signal); + await new Promise((resolve) => setTimeout(resolve, 0)); + controller.abort(); + await expect(pending).rejects.toThrow(); + await snapshot(); + // The fake runner reaches its abandonment bound with the same read still held. + state.refuseDescribe = 'bridge-reader-wedged'; + await snapshot(); + await snapshot(); + await snapshot(); + expect(verbs(sent)).toEqual(['target', 'describe', 'describe', 'describe', 'describe']); +}); + +test('a replacement runner retries a generation retired by the previous runner once', async () => { + const state: { process: RunnerProcess; refuseDescribe?: string } = { + process: { pid: 41, processStartTime: '1700000000.000001' }, + refuseDescribe: 'bridge-read-abandoned', + }; + const { provider, sent } = bridgeRunner(state); + const { snapshot, presented } = runtimeSnapshots(withHostStates(provider, 'notRunning')); + await snapshot(); + registry.session = { sessionId: 'runner-session-2', liveness: 'ready' }; + state.refuseDescribe = undefined; + sent.length = 0; + await snapshot(); + await snapshot(); + expect(verbs(sent)).toEqual(['target', 'describe', 'describe']); + expect(presented).toHaveLength(2); +}); + +test('a late presentation failure cannot retire the replacement session generation', async () => { + const state = { process: { pid: 41, processStartTime: '1700000000.000001' } }; + const { provider, sent } = bridgeRunner(state); + const bridge = createRunnerSnapshotBridge({ device: IOS_SIMULATOR, provider }); + let sessionId = 'first'; + let release!: () => void; + let entered!: () => void; + const blocked = new Promise((resolve) => { + release = resolve; + }); + const presenting = new Promise((resolve) => { + entered = resolve; + }); + let presentationCount = 0; + const route = createSnapshotBridgeRoute( + { + snapshot: { + presentIosAcquisition: async (result) => { + if (++presentationCount === 1) { + entered(); + await blocked; + throw new Error('old presentation failed'); + } + return { ...runnerResult(), producer: result.acquisition.producer }; + }, + }, + appleApplications: { hasLiveRunnerSession: async () => true }, + }, + { + source: bridge.source, + resolveTarget: bridge.resolveTarget, + isDiscoveryPending: () => false, + targetFreshness: 'cached', + sessionChanged: bridge.resetTargets, + }, + async () => ({ foreground: 'background', sessionId }), + ); + const snapshot = () => + route.capture( + IOS_SIMULATOR, + { options: { appBundleId: APP, interactiveOnly: true } }, + new AbortController().signal, + async () => runnerResult(), + ); + const old = snapshot(); + await presenting; + sessionId = 'second'; + expect((await snapshot()).producer).toBe('simulator-ax-bridge'); + release(); + expect((await old).warnings).toContain( + 'Simulator AX snapshot unavailable (presentation-invariant); used XCTest for this app generation.', + ); + expect((await snapshot()).producer).toBe('simulator-ax-bridge'); + expect(verbs(sent)).toEqual(['target', 'describe', 'target', 'describe', 'describe']); +}); + +test('a wedge on a route that never names a runner session speaks for that capture only', async () => { + const state: { process: RunnerProcess; refuseTarget?: string } = { + process: { pid: 41, processStartTime: '1700000000.000001' }, + refuseTarget: 'bridge-reader-wedged', + }; + const { provider, sent } = bridgeRunner(state); + const { snapshot, presented } = bridgeRoute(provider); + expect((await snapshot()).warnings).toEqual([ + 'Simulator AX snapshot unavailable (bridge-reader-wedged); used XCTest for this capture while an earlier bridge read was still running.', + ]); + state.refuseTarget = undefined; + await snapshot(); + expect(presented).toHaveLength(1); + expect(verbs(sent)).toEqual(['target', 'target', 'describe']); +}); + +test('a wedge before the runner id is known does not disable a newly identified session', async () => { + const state: { process: RunnerProcess; refuseTarget?: string } = { + process: { pid: 41, processStartTime: '1700000000.000001' }, + refuseTarget: 'bridge-reader-wedged', + }; + const { provider, sent } = bridgeRunner(state); + const bridge = createRunnerSnapshotBridge({ device: IOS_SIMULATOR, provider }); + let sessionId: string | undefined = undefined; + const route = createSnapshotBridgeRoute( + { + snapshot: { + presentIosAcquisition: (result) => ({ + ...runnerResult(), + producer: result.acquisition.producer, + }), + }, + appleApplications: { hasLiveRunnerSession: async () => true }, + }, + { + source: bridge.source, + resolveTarget: bridge.resolveTarget, + isDiscoveryPending: () => false, + targetFreshness: 'cached', + sessionChanged: bridge.resetTargets, + }, + async () => (sessionId ? { foreground: 'background', sessionId } : 'background'), + ); + const snapshot = () => + route.capture( + IOS_SIMULATOR, + { options: { appBundleId: APP, interactiveOnly: true } }, + new AbortController().signal, + async () => runnerResult(), + ); + expect((await snapshot()).warnings).toEqual([ + 'Simulator AX snapshot unavailable (bridge-reader-wedged); used XCTest for this capture while an earlier bridge read was still running.', + ]); + sessionId = 'new'; + state.refuseTarget = undefined; + expect((await snapshot()).producer).toBe('simulator-ax-bridge'); + expect(verbs(sent)).toEqual(['target', 'target', 'describe']); +}); diff --git a/packages/platform-apple/src/runner-snapshot-bridge.ts b/packages/platform-apple/src/runner-snapshot-bridge.ts index 3e0b519cd7..38242d7077 100644 --- a/packages/platform-apple/src/runner-snapshot-bridge.ts +++ b/packages/platform-apple/src/runner-snapshot-bridge.ts @@ -43,6 +43,7 @@ const RUNNER_TRANSPORT_MARGIN_MS = 1_000; export type RunnerSnapshotBridge = Readonly<{ source: SnapshotBridgeSource; + resetTargets(): void; resolveTarget( device: DeviceInfo, appBundleId: string, @@ -59,7 +60,7 @@ export type RunnerSnapshotBridge = Readonly<{ export function createRunnerSnapshotBridge( options: Readonly<{ device: DeviceInfo; provider: AppleRunnerProvider }>, ): RunnerSnapshotBridge { - const targets = new Map(); + let targets = new Map(); // The runner admits one bridge read at a time and refuses an overlapping one at once, so it alone // owns that order: a read a cancelled capture left running costs the next capture its bridge read, // and the route answers that refusal for that capture only. @@ -93,6 +94,7 @@ export function createRunnerSnapshotBridge( if (cached && refresh !== 'refresh') return cached; // Only a resolved target replaces the cached one: a refresh that fails leaves the generation the // route planned against. + const captureTargets = targets; const requestId = randomUUID(); const envelope = await send( appBundleId, @@ -104,7 +106,7 @@ export function createRunnerSnapshotBridge( AbortSignal.any([signal, AbortSignal.timeout(TARGET_READ_TIMEOUT_MS)]), ); const target = readTarget(envelope, { requestId, appBundleId, targetId }); - targets.set(targetId, target); + captureTargets.set(targetId, target); return target; }; @@ -158,7 +160,13 @@ export function createRunnerSnapshotBridge( close: async () => targets.clear(), }, }); - return { source, resolveTarget }; + return { + source, + resolveTarget, + resetTargets: () => { + targets = new Map(); + }, + }; } function parseRunnerEnvelope(data: Record): SnapshotBridgeEnvelope { @@ -192,6 +200,8 @@ function readTarget( !processStartTime ) { const code = typeof envelope.error_code === 'string' ? envelope.error_code : 'target-invalid'; + if (code === 'bridge-reader-wedged') + throw snapshotSourceError('timeout', 'bridge-reader-wedged'); throw new AppError( 'COMMAND_FAILED', `Unable to resolve the foreground app in the runner (${code}).`, diff --git a/packages/platform-apple/src/snapshot-route-failure-policy.test.ts b/packages/platform-apple/src/snapshot-route-failure-policy.test.ts index 2cd34f996b..a50cc12272 100644 --- a/packages/platform-apple/src/snapshot-route-failure-policy.test.ts +++ b/packages/platform-apple/src/snapshot-route-failure-policy.test.ts @@ -135,9 +135,19 @@ const fixture = JSON.parse( ), 'utf8', ), -) as { captureScoped: { code: string; consequence: string }[]; generationScoped: string[] }; +) as { + captureScoped: { code: string; consequence: string }[]; + generationScoped: string[]; + runnerSessionScoped: string[]; +}; test('the policy gives each code of the shared refusal table its scope and consequence', () => { + for (const code of fixture.runnerSessionScoped) { + expect(bridgeFailureDisposition({ kind: 'timeout', code })).toEqual({ + scope: 'runner-session', + code, + }); + } for (const { code, consequence } of fixture.captureScoped) { // A preparation still running is classified by its failure kind, not by its code. const kind = consequence === 'bridge-preparation' ? 'preparing' : 'unsupported'; diff --git a/packages/platform-apple/src/snapshot-route-failure-policy.ts b/packages/platform-apple/src/snapshot-route-failure-policy.ts index 3152393531..e29674b972 100644 --- a/packages/platform-apple/src/snapshot-route-failure-policy.ts +++ b/packages/platform-apple/src/snapshot-route-failure-policy.ts @@ -27,7 +27,7 @@ export type SurfaceHostRefusal = IosSystemSurfaceHost | 'unverified'; /** * The one classification a bridge failure receives before the route acts on it. `code` travels - * unchanged for diagnostics; the scope says whether the generation is implicated, and a + * unchanged for diagnostics; the scope identifies the capture, generation, or runner session, and a * capture-scoped verdict must name what it saw instead. A surface host refusal names the host, and * the route serves it from the runner before any fallback warning applies. */ @@ -39,7 +39,7 @@ export type BridgeFailureDisposition = consequence: 'system-surface-host'; surfaceHost: SurfaceHostRefusal; }> - | Readonly<{ scope: 'generation'; code: string }>; + | Readonly<{ scope: 'generation' | 'runner-session'; code: string }>; /** Every disposition that reaches the generic fallback: all but a surface host refusal. */ export type BridgeFallbackDisposition = Exclude< @@ -68,14 +68,19 @@ const CAPTURE_SCOPED_REFUSALS: ReadonlyMap = n ['snapshot-tree-changed', 'tree-changed'], ]); +const RUNNER_SESSION_SCOPED: ReadonlySet = new Set(['bridge-reader-wedged']); + /** * Whether a bridge failure is evidence that this app generation cannot be served by the bridge. - * The preparation and the table's screen refusals speak for this capture only; every other failure - * retires the generation. That default is conservative on purpose: `code` is an open string from + * Preparation and screen refusals speak for this capture; a wedged reader retires its runner + * session. Other failures retire the generation. That default is conservative on purpose: `code` is an open string from * the guest, and an unknown code must not quietly keep a generation on a path that just refused * it. Do not widen the default; classify a new known refusal in the table. */ export function bridgeFailureDisposition(failure: SnapshotSourceFailure): BridgeFailureDisposition { + if (RUNNER_SESSION_SCOPED.has(failure.code)) { + return { scope: 'runner-session', code: failure.code }; + } if (failure.kind === 'preparing') { return { scope: 'capture', code: failure.code, consequence: 'bridge-preparation' }; } diff --git a/packages/platform-apple/src/snapshot-route.ts b/packages/platform-apple/src/snapshot-route.ts index 0e1a679ed1..edc14c24be 100644 --- a/packages/platform-apple/src/snapshot-route.ts +++ b/packages/platform-apple/src/snapshot-route.ts @@ -37,13 +37,14 @@ import { type SimulatorSnapshotTarget, type SimulatorSnapshotTargetResolver, } from './snapshot-target.ts'; +import { SnapshotSourceError } from './snapshot-source/errors.ts'; import { createSystemSurfaceForegroundProbe, - type SystemSurfaceForeground, type SystemSurfaceForegroundProbe, } from './system-surface-foreground.ts'; import { bridgeFailureDisposition, + type BridgeFailureDisposition, type BridgeFallbackDisposition, type CaptureScopedConsequence, } from './snapshot-route-failure-policy.ts'; @@ -122,6 +123,7 @@ export type SnapshotBridgeChannel = Re * trusts one. */ targetFreshness: 'verified' | 'cached'; + sessionChanged?(): void; }>; /** What the route reads from the platform host: presentation and runner liveness only. */ @@ -175,7 +177,10 @@ export function createSnapshotBridgeRoute( @@ -184,18 +189,58 @@ function createSnapshotBridgeRouteCore surfaceForeground: SystemSurfaceForegroundProbe, ) { const { source, resolveTarget } = channel; - const disabledGenerations = new Set(); - const latestGeneration = new Map(); + const freshCircuit = (sessionId?: string) => ({ + sessionId, + disabledGenerations: new Set(), + latestGeneration: new Map(), + sessionDisabled: false, + }); + let currentCircuit = freshCircuit(); + const isDisabledIn = (circuit: typeof currentCircuit, target: SnapshotBridgeRouteTarget) => { + rebaselineGeneration(target, circuit.latestGeneration, circuit.disabledGenerations); + return circuit.disabledGenerations.has(generationKey(target)); + }; + /** + * The circuit a capture writes its refusals to. A capture writes through the returned object, so + * it must be the circuit object itself, never a copy: when it is `currentCircuit`, the next capture + * reads what this one wrote. A capture that read through a session another capture already + * replaced keeps its starting circuit, so its late writes land in the discarded object. + */ + const circuitForProbe = ( + startingCircuit: typeof currentCircuit, + probeResult: Awaited>, + device: DeviceInfo, + ): typeof currentCircuit => { + const probedSessionId = + channel.sessionChanged && typeof probeResult === 'object' && 'sessionId' in probeResult + ? probeResult.sessionId + : undefined; + if (probedSessionId === undefined) return startingCircuit; + if (currentCircuit !== startingCircuit) { + return probedSessionId === currentCircuit.sessionId ? currentCircuit : startingCircuit; + } + if (probedSessionId === startingCircuit.sessionId) return startingCircuit; + emitDiagnostic({ + level: 'debug', + phase: 'ios_snapshot_route_runner_session_changed', + data: { + deviceId: device.id, + previousSessionId: startingCircuit.sessionId, + sessionId: probedSessionId, + }, + }); + currentCircuit = freshCircuit(probedSessionId); + channel.sessionChanged?.(); + return currentCircuit; + }; /** * Records `target` as the newest generation of its app — which clears the circuit an earlier * generation opened — and reports whether the bridge is disabled for it. Both a capture and the * launch-observation probe ask before they spend a bridge round trip, so one generation's * failure is paid once rather than once per route (#2198, #2199). */ - const isBridgeDisabled = (target: SnapshotBridgeRouteTarget): boolean => { - rebaselineGeneration(target, latestGeneration, disabledGenerations); - return disabledGenerations.has(generationKey(target)); - }; + const isBridgeDisabled = (target: SnapshotBridgeRouteTarget): boolean => + isDisabledIn(currentCircuit, target); return { isBridgeDisabled, @@ -216,16 +261,43 @@ function createSnapshotBridgeRouteCore // A ready runner answers first, with the foreground state of every registered host: the // predicate it serves a surface by, read in a few milliseconds. When it cannot answer, the // bridge itself refuses the capture while a host process runs for this Simulator. - const foreground = await readSurfaceForeground(device, input, signal, surfaceForeground); + const startingCircuit = currentCircuit; + const probeResult = await readSurfaceForeground(device, input, signal, surfaceForeground); + const circuit = circuitForProbe(startingCircuit, probeResult, device); + const foreground = + typeof probeResult === 'object' && 'sessionId' in probeResult + ? probeResult.foreground + : probeResult; + const captureBridgeDisabled = (target: SnapshotBridgeRouteTarget) => + isDisabledIn(circuit, target); if (foreground !== 'background' && foreground !== 'unknown') { return await runSurfaceFallback(device, input, fallback, foreground.host.bundleId); } + if (circuit.sessionDisabled) { + emitRouteDiagnostic('bridge-reader-wedged', device); + return await runFallback( + device.id, + input, + fallback, + appLineage(device.id, input), + requestFor(input), + { scope: 'runner-session', code: 'bridge-reader-wedged' }, + [unknownGenerationResidue()], + ); + } let target: Target; try { - target = await resolveCaptureTarget(host, channel, device, input, signal, isBridgeDisabled); + target = await resolveCaptureTarget( + host, + channel, + device, + input, + signal, + captureBridgeDisabled, + ); } catch (error) { rethrowIfResolutionCancelled(signal, error); - emitRouteDiagnostic('target-resolution-failed', device, undefined, error); + const reason = targetFailureReason(error, device, circuit); // The app's current generation is unknown, so the capture names none. return await runFallback( device.id, @@ -233,11 +305,11 @@ function createSnapshotBridgeRouteCore fallback, appLineage(device.id, input), requestFor(input), - 'target-resolution-failed', + reason, [unknownGenerationResidue()], ); } - if (isBridgeDisabled(target)) { + if (captureBridgeDisabled(target)) { emitRouteDiagnostic('circuit-disabled', device, target.generation); return await runFallback( device.id, @@ -258,33 +330,19 @@ function createSnapshotBridgeRouteCore ...(foreground === 'unknown' ? { surfaceHostExecutables: SURFACE_HOST_EXECUTABLES } : {}), }); if (outcome.stage === 'failed') { - if (outcome.failure.kind === 'cancelled') { - signal.throwIfAborted(); - throw new AppError('COMMAND_FAILED', 'Simulator AX snapshot acquisition was cancelled.', { - reason: outcome.failure.code, - ...outcome.failure.details, - }); - } - const disposition = bridgeFailureDisposition(outcome.failure); + rethrowCancelledFailure(outcome.failure, signal); + const disposition = settleRunnerSessionRefusal( + circuit, + bridgeFailureDisposition(outcome.failure), + ); if ('surfaceHost' in disposition) { - if (disposition.surfaceHost !== 'unverified') { - return await runSurfaceFallback( - device, - input, - fallback, - disposition.surfaceHost.bundleId, - ); - } - // The bridge could not place a host process. Take the runner rather than a bridge - // capture that may hold the occluded app tree, and say so: silently losing the bridge - // fast path would be its own defect. - return await runFallback( - device.id, + return await fallbackForSurfaceHost( + device, input, fallback, target, request, - 'system-surface-host-unverified', + disposition, ); } const fallbackIdentity = await resolveFailureFallbackIdentity( @@ -304,7 +362,7 @@ function createSnapshotBridgeRouteCore request, outcome.failure, disposition, - disabledGenerations, + circuit.disabledGenerations, ); } try { @@ -329,7 +387,7 @@ function createSnapshotBridgeRouteCore failure, // A tree the presenter cannot publish is app evidence, so it retires the generation. { scope: 'generation', code: failure.code }, - disabledGenerations, + circuit.disabledGenerations, error, ); } @@ -337,6 +395,55 @@ function createSnapshotBridgeRouteCore }; } +function rethrowCancelledFailure(failure: SnapshotSourceFailure, signal: AbortSignal): void { + if (failure.kind !== 'cancelled') return; + signal.throwIfAborted(); + throw new AppError('COMMAND_FAILED', 'Simulator AX snapshot acquisition was cancelled.', { + reason: failure.code, + ...failure.details, + }); +} + +function targetFailureReason( + error: unknown, + device: DeviceInfo, + circuit: RunnerSessionCircuit, +): FallbackReason { + const disposition = + error instanceof SnapshotSourceError + ? bridgeFailureDisposition({ kind: error.failureKind, code: error.failureCode }) + : undefined; + emitRouteDiagnostic( + disposition?.scope === 'runner-session' ? disposition.code : 'target-resolution-failed', + device, + undefined, + error, + ); + if (disposition?.scope === 'runner-session') + return settleRunnerSessionRefusal(circuit, disposition); + return 'target-resolution-failed'; +} + +type RunnerSessionCircuit = { readonly sessionId?: string; sessionDisabled: boolean }; + +/** + * Retires the bridge for the circuit's runner session when the circuit names that session; a later + * probe that names another session replaces the circuit and clears it. A circuit with no session id + * has nothing to tie the retirement to, so there the refusal speaks for this capture only, and its + * warning says so. + */ +function settleRunnerSessionRefusal( + circuit: RunnerSessionCircuit, + disposition: Disposition, +): Disposition | Readonly<{ scope: 'capture'; code: string; consequence: 'bridge-read-busy' }> { + if (disposition.scope !== 'runner-session') return disposition; + if (circuit.sessionId === undefined) { + return { scope: 'capture', code: disposition.code, consequence: 'bridge-read-busy' }; + } + circuit.sessionDisabled = true; + return disposition; +} + const SURFACE_HOST_EXECUTABLES = IOS_SYSTEM_SURFACE_HOSTS.map((host) => host.processExecutable); /** @@ -350,10 +457,15 @@ async function readSurfaceForeground( input: CaptureSnapshotInput, signal: AbortSignal, probe: SystemSurfaceForegroundProbe, -): Promise { +): Promise>> { const requestId = input.execution?.requestId; const foreground = await probe(device, { signal, ...(requestId ? { requestId } : {}) }); - if (foreground === 'background') { + if ( + foreground === 'background' || + (typeof foreground === 'object' && + 'sessionId' in foreground && + foreground.foreground === 'background') + ) { emitDiagnostic({ level: 'debug', phase: 'ios_snapshot_route_surface_host_background', @@ -428,6 +540,30 @@ function isEligible(device: DeviceInfo, input: CaptureSnapshotInput): boolean { ); } +async function fallbackForSurfaceHost( + device: DeviceInfo, + input: CaptureSnapshotInput, + fallback: SnapshotFallback, + target: SnapshotBridgeRouteTarget, + request: ReturnType, + disposition: Extract< + ReturnType, + { consequence: 'system-surface-host' } + >, +): Promise { + if (disposition.surfaceHost !== 'unverified') { + return await runSurfaceFallback(device, input, fallback, disposition.surfaceHost.bundleId); + } + return await runFallback( + device.id, + input, + fallback, + target, + request, + 'system-surface-host-unverified', + ); +} + async function fallbackAfterFailure( device: DeviceInfo, input: CaptureSnapshotInput, @@ -604,6 +740,8 @@ function fallbackWarning( if (typeof reason !== 'string' && reason.scope === 'capture') { return captureScopedFallbackWarning(code, reason.consequence); } + if (typeof reason !== 'string' && reason.scope === 'runner-session') + return `Simulator AX snapshot unavailable (${code}); used XCTest for this runner session.`; const generation = lineage.generation ? 'this app generation' : 'an unverified app generation'; return `Simulator AX snapshot unavailable (${code}); used XCTest for ${generation}.`; } diff --git a/packages/platform-apple/src/system-surface-foreground.test.ts b/packages/platform-apple/src/system-surface-foreground.test.ts index fec0c175d7..c1868e0a9f 100644 --- a/packages/platform-apple/src/system-surface-foreground.test.ts +++ b/packages/platform-apple/src/system-surface-foreground.test.ts @@ -44,7 +44,7 @@ function probe() { test('every registered host off screen is background, read only from the ready runner session', async () => { statesByHost({ [safari!]: 'runningBackgroundSuspended', [passbook!]: 'notRunning' }); - await expect(probe()).resolves.toBe('background'); + await expect(probe()).resolves.toEqual({ foreground: 'background', sessionId: 'runner-1' }); expect( runner.runAppleRunnerCommand.mock.calls.map(([, command, options]) => [command, options]), ).toEqual( @@ -67,8 +67,11 @@ test('a decisive first host answers without waiting for the hosts after it', asy }); await expect(probe()).resolves.toEqual({ - kind: 'foreground', - host: IOS_SYSTEM_SURFACE_HOSTS.find((host) => host.bundleId === safari), + foreground: { + kind: 'foreground', + host: IOS_SYSTEM_SURFACE_HOSTS.find((host) => host.bundleId === safari), + }, + sessionId: 'runner-1', }); // Every read still went out together. expect(runner.runAppleRunnerCommand).toHaveBeenCalledTimes(IOS_SYSTEM_SURFACE_HOSTS.length); @@ -79,8 +82,11 @@ test('one background host does not hide another one presenting', async () => { statesByHost({ [safari!]: 'runningBackground', [passbook!]: 'runningForeground' }); await expect(probe()).resolves.toEqual({ - kind: 'foreground', - host: IOS_SYSTEM_SURFACE_HOSTS.find((host) => host.bundleId === passbook), + foreground: { + kind: 'foreground', + host: IOS_SYSTEM_SURFACE_HOSTS.find((host) => host.bundleId === passbook), + }, + sessionId: 'runner-1', }); }); @@ -101,12 +107,12 @@ test.for([ ] as const)('%s is unknown', async ([, state]) => { statesByHost({ [safari!]: state, [passbook!]: 'notRunning' }); - await expect(probe()).resolves.toBe('unknown'); + await expect(probe()).resolves.toEqual({ foreground: 'unknown', sessionId: 'runner-1' }); }); test('a failed read is unknown, and a cancelled request still cancels', async () => { runner.runAppleRunnerCommand.mockRejectedValue(new Error('runner unreachable')); - await expect(probe()).resolves.toBe('unknown'); + await expect(probe()).resolves.toEqual({ foreground: 'unknown', sessionId: 'runner-1' }); const controller = new AbortController(); controller.abort(new Error('request cancelled')); diff --git a/packages/platform-apple/src/system-surface-foreground.ts b/packages/platform-apple/src/system-surface-foreground.ts index 3c6f45802d..7b7bd85a66 100644 --- a/packages/platform-apple/src/system-surface-foreground.ts +++ b/packages/platform-apple/src/system-surface-foreground.ts @@ -26,7 +26,9 @@ export type SystemSurfaceForeground = export type SystemSurfaceForegroundProbe = ( device: DeviceInfo, input: Readonly<{ signal: AbortSignal; requestId?: string }>, -) => Promise; +) => Promise< + SystemSurfaceForeground | Readonly<{ foreground: SystemSurfaceForeground; sessionId: string }> +>; /** * Bounds each read below the read-only resend window, so a runner busy draining abandoned XCTest @@ -74,10 +76,12 @@ export function createSystemSurfaceForegroundProbe( for (const read of reads) read.catch(() => {}); for (const [index, host] of IOS_SYSTEM_SURFACE_HOSTS.entries()) { const state = await reads[index]; - if (state === 'runningForeground') return { kind: 'foreground', host }; - if (state === undefined || !BACKGROUND_STATES.has(state)) return 'unknown'; + if (state === 'runningForeground') + return { foreground: { kind: 'foreground', host }, sessionId: session.sessionId }; + if (state === undefined || !BACKGROUND_STATES.has(state)) + return { foreground: 'unknown', sessionId: session.sessionId }; } - return 'background'; + return { foreground: 'background', sessionId: session.sessionId }; }; } diff --git a/test/repo-guards/snapshot-bridge-refusals.test.ts b/test/repo-guards/snapshot-bridge-refusals.test.ts index 6db47e00c1..312d800503 100644 --- a/test/repo-guards/snapshot-bridge-refusals.test.ts +++ b/test/repo-guards/snapshot-bridge-refusals.test.ts @@ -14,6 +14,7 @@ import { REPO_ROOT } from '@agent-device/test-kit'; type Fixture = { captureScoped: { code: string; consequence: string }[]; generationScoped: string[]; + runnerSessionScoped: string[]; }; const fixture = JSON.parse( @@ -26,6 +27,7 @@ const fixture = JSON.parse( const fixtureCodes = [ ...fixture.captureScoped.map(({ code }) => code), ...fixture.generationScoped, + ...fixture.runnerSessionScoped, ]; function read(relative: string): string {