diff --git a/src/platforms/ios/__tests__/runner-command-retry.test.ts b/src/platforms/ios/__tests__/runner-command-retry.test.ts index eff55b04e..baec49766 100644 --- a/src/platforms/ios/__tests__/runner-command-retry.test.ts +++ b/src/platforms/ios/__tests__/runner-command-retry.test.ts @@ -7,15 +7,27 @@ import type { RunnerSession } from '../runner-session-types.ts'; const { mockEnsureRunnerSession, mockExecuteRunnerCommandWithSession, + mockEmitDiagnostic, mockInvalidateRunnerSession, mockStopRunnerSession, } = vi.hoisted(() => ({ mockEnsureRunnerSession: vi.fn(), mockExecuteRunnerCommandWithSession: vi.fn(), + mockEmitDiagnostic: vi.fn(), mockInvalidateRunnerSession: vi.fn(), mockStopRunnerSession: vi.fn(), })); +vi.mock('../../../utils/diagnostics.ts', async () => { + const actual = await vi.importActual( + '../../../utils/diagnostics.ts', + ); + return { + ...actual, + emitDiagnostic: mockEmitDiagnostic, + }; +}); + vi.mock('../runner-session.ts', async () => { const actual = await vi.importActual('../runner-session.ts'); @@ -151,6 +163,11 @@ test('mutating commands do not restart or replay after command send failure', as ]); assert.equal(mockStopRunnerSession.mock.calls.length, 0); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assertDiagnosticDecision({ + decision: 'retained', + reason: 'unknown_lifecycle_state', + lifecycleState: 'notAccepted', + }); }); test('mutating commands recover cached responses before invalidating after command send failure', async () => { @@ -168,6 +185,11 @@ test('mutating commands recover cached responses before invalidating after comma assert.deepEqual(result, { message: 'tapped' }); assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0); + assertDiagnosticDecision({ + decision: 'skipped', + reason: 'completed_with_retained_response', + lifecycleState: 'completed', + }); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); const sentCommand = mockExecuteRunnerCommandWithSession.mock.calls[0]?.[2]; const statusCommand = mockExecuteRunnerCommandWithSession.mock.calls[1]?.[2]; @@ -193,6 +215,65 @@ test('mutating commands keep invalidating when status cannot find the command', [session, 'transport_error_after_command_send'], ]); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assertDiagnosticDecision({ + decision: 'retained', + reason: 'unknown_lifecycle_state', + lifecycleState: 'notAccepted', + }); +}); + +test('mutating commands keep invalidating when status recovery probe fails', async () => { + const session = makeRunnerSession({ port: 8100, ready: true }); + + mockEnsureRunnerSession.mockResolvedValueOnce(session); + mockExecuteRunnerCommandWithSession + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed')) + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'status probe failed')); + + await assert.rejects(() => + runIosRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), + ); + + assert.deepEqual(mockInvalidateRunnerSession.mock.calls, [ + [session, 'transport_error_after_command_send'], + ]); + assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assertDiagnosticDecision({ + decision: 'retained', + reason: 'status_probe_failed', + }); +}); + +test('mutating commands keep invalidating when status reports an unknown lifecycle state', async () => { + const session = makeRunnerSession({ port: 8100, ready: true }); + + mockEnsureRunnerSession.mockResolvedValueOnce(session); + mockExecuteRunnerCommandWithSession + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed')) + .mockResolvedValueOnce({ + lifecycleState: 'paused', + }); + + await assert.rejects( + () => runIosRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.match(error.message, /lifecycle status was "paused"/); + assert.equal(error.details?.recovery, 'lifecycle_state_not_recoverable'); + assert.match(String(error.details?.hint), /conservative invalidation path/); + return true; + }, + ); + + assert.deepEqual(mockInvalidateRunnerSession.mock.calls, [ + [session, 'transport_error_after_command_send'], + ]); + assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assertDiagnosticDecision({ + decision: 'retained', + reason: 'unknown_lifecycle_state', + lifecycleState: 'paused', + }); }); test('read-only commands retry when completed status has no retained response', async () => { @@ -211,6 +292,11 @@ test('read-only commands retry when completed status has no retained response', assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 3); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[2].command, 'status'); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[2]?.[2].command, 'snapshot'); + assertDiagnosticDecision({ + decision: 'skipped', + reason: 'read_only_completed_without_retained_response', + lifecycleState: 'completed', + }); }); test('read-only commands retry when status shows in-flight work', async () => { @@ -245,6 +331,7 @@ test('mutating commands report recovery guidance when completed status has no re assert.ok(error instanceof AppError); assert.match(error.message, /"tap" completed after the transport response was lost/); assert.equal(error.details?.recovery, 'completed_without_retained_response'); + assert.match(String(error.details?.hint), /kept the session open/); assert.match(String(error.details?.hint), /will not replay/); assert.match(String(error.details?.hint), /snapshot -i/); assert.equal(error.details?.transportError, 'fetch failed'); @@ -254,6 +341,11 @@ test('mutating commands report recovery guidance when completed status has no re assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assertDiagnosticDecision({ + decision: 'skipped', + reason: 'completed_without_retained_response', + lifecycleState: 'completed', + }); }); test('mutating commands preserve runner failure details from status recovery', async () => { @@ -284,6 +376,41 @@ test('mutating commands preserve runner failure details from status recovery', a assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assertDiagnosticDecision({ + decision: 'skipped', + reason: 'runner_reported_failure', + lifecycleState: 'failed', + }); +}); + +test('mutating commands use recovery guidance when failed status has no runner hint', async () => { + const session = makeRunnerSession({ port: 8100, ready: true }); + + mockEnsureRunnerSession.mockResolvedValueOnce(session); + mockExecuteRunnerCommandWithSession + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed')) + .mockResolvedValueOnce({ + lifecycleState: 'failed', + lifecycleErrorMessage: 'Runner command failed after dispatch', + }); + + await assert.rejects( + () => runIosRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.message, 'Runner command failed after dispatch'); + assert.match(String(error.details?.hint), /kept the session open/); + assert.match(String(error.details?.hint), /did not replay/); + return true; + }, + ); + + assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0); + assertDiagnosticDecision({ + decision: 'skipped', + reason: 'runner_reported_failure', + lifecycleState: 'failed', + }); }); test('mutating commands report wait-and-inspect guidance when status shows in-flight work', async () => { @@ -300,7 +427,7 @@ test('mutating commands report wait-and-inspect guidance when status shows in-fl assert.ok(error instanceof AppError); assert.match(error.message, /"tap" is still started/); assert.equal(error.details?.recovery, 'command_still_in_flight'); - assert.match(String(error.details?.hint), /may still finish/); + assert.match(String(error.details?.hint), /kept the session open/); assert.match(String(error.details?.hint), /snapshot -i/); assert.equal(error.details?.transportError, 'fetch failed'); return true; @@ -309,6 +436,11 @@ test('mutating commands report wait-and-inspect guidance when status shows in-fl assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2); + assertDiagnosticDecision({ + decision: 'skipped', + reason: 'command_still_in_flight', + lifecycleState: 'started', + }); }); test('mutating commands invalidate the retry session without replaying again', async () => { @@ -331,8 +463,31 @@ test('mutating commands invalidate the retry session without replaying again', a [freshSession, 'transport_error_after_retry_command_send'], ]); assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 3); + assertDiagnosticDecision({ + decision: 'retained', + reason: 'unknown_lifecycle_state', + lifecycleState: 'notAccepted', + }); }); +function assertDiagnosticDecision(expected: { + decision: 'skipped' | 'retained'; + reason: string; + lifecycleState?: string; +}): void { + assert.ok( + mockEmitDiagnostic.mock.calls.some(([event]) => { + return ( + event.phase === 'ios_runner_command_invalidation_decision' && + event.data?.decision === expected.decision && + event.data?.reason === expected.reason && + event.data?.lifecycleState === expected.lifecycleState + ); + }), + `missing invalidation decision diagnostic ${JSON.stringify(expected)}`, + ); +} + function makeRunnerSession(overrides: Partial = {}): RunnerSession { return { sessionId: `session-${overrides.port ?? 8100}`, diff --git a/src/platforms/ios/runner-client.ts b/src/platforms/ios/runner-client.ts index 22253b04f..8c8bbf25b 100644 --- a/src/platforms/ios/runner-client.ts +++ b/src/platforms/ios/runner-client.ts @@ -40,6 +40,18 @@ type LifecycleResponsePayload = { data?: unknown; }; +type RunnerTransportRecovery = + | { type: 'recovered'; data: Record; reason: string; lifecycleState?: string } + | { type: 'skipInvalidation'; error: AppError; reason: string; lifecycleState?: string } + | { type: 'retainInvalidation'; error?: AppError; reason: string; lifecycleState?: string }; + +type RunnerTransportRecoveryContext = { + command: RunnerCommand; + session: RunnerSession; + transportError: AppError; + invalidationReason: string; +}; + const RUNNER_STATUS_RECOVERY_TIMEOUT_MS = 3_000; // --- Runner command execution --- @@ -153,16 +165,15 @@ async function executeRunnerCommand( ? retryErr : new AppError('COMMAND_FAILED', String(retryErr)); if (isRetryableRunnerError(retryAppErr)) { - const recovered = await tryRecoverRunnerCommandAfterTransportError( + return await handleRunnerTransportErrorAfterCommandSend( device, session, command, retryAppErr, options, signal, + 'transport_error_after_retry_command_send', ); - if (recovered) return recovered; - await invalidateRunnerSession(session, 'transport_error_after_retry_command_send'); } throw retryErr; } @@ -189,16 +200,15 @@ async function executeRunnerCommand( ? retryErr : new AppError('COMMAND_FAILED', String(retryErr)); if (isRetryableRunnerError(retryAppErr)) { - const recovered = await tryRecoverRunnerCommandAfterTransportError( + return await handleRunnerTransportErrorAfterCommandSend( device, session, command, retryAppErr, options, signal, + 'transport_error_after_retry_command_send', ); - if (recovered) return recovered; - await invalidateRunnerSession(session, 'transport_error_after_retry_command_send'); } throw retryErr; } @@ -207,21 +217,108 @@ async function executeRunnerCommand( await stopIosRunnerSession(device.id); } if (session && isRetryableRunnerError(appErr)) { - const recovered = await tryRecoverRunnerCommandAfterTransportError( + return await handleRunnerTransportErrorAfterCommandSend( device, session, command, appErr, options, signal, + 'transport_error_after_command_send', ); - if (recovered) return recovered; - await invalidateRunnerSession(session, 'transport_error_after_command_send'); } throw err; } } +async function handleRunnerTransportErrorAfterCommandSend( + device: DeviceInfo, + session: RunnerSession, + command: RunnerCommand, + transportError: AppError, + options: AppleRunnerCommandOptions, + signal: AbortSignal | undefined, + invalidationReason: string, +): Promise> { + const recovery = await tryRecoverRunnerCommandAfterTransportError( + device, + session, + command, + transportError, + options, + signal, + ); + return await applyRunnerTransportRecovery(recovery, { + command, + session, + transportError, + invalidationReason, + }); +} + +async function applyRunnerTransportRecovery( + recovery: RunnerTransportRecovery | undefined, + context: RunnerTransportRecoveryContext, +): Promise> { + if (!recovery) return await retainRunnerInvalidation(context, 'status_recovery_unavailable'); + if (recovery.type === 'recovered') return recoverRunnerResponse(recovery, context); + if (recovery.type === 'skipInvalidation') throw skipRunnerInvalidation(recovery, context); + return await retainRunnerInvalidation( + context, + recovery.reason, + recovery.lifecycleState, + recovery.error, + ); +} + +function recoverRunnerResponse( + recovery: Extract, + context: RunnerTransportRecoveryContext, +): Record { + emitRunnerInvalidationDecision({ + command: context.command, + session: context.session, + transportError: context.transportError, + decision: 'skipped', + reason: recovery.reason, + lifecycleState: recovery.lifecycleState, + }); + return recovery.data; +} + +function skipRunnerInvalidation( + recovery: Extract, + context: RunnerTransportRecoveryContext, +): AppError { + emitRunnerInvalidationDecision({ + command: context.command, + session: context.session, + transportError: context.transportError, + decision: 'skipped', + reason: recovery.reason, + lifecycleState: recovery.lifecycleState, + }); + return recovery.error; +} + +async function retainRunnerInvalidation( + context: RunnerTransportRecoveryContext, + reason: string, + lifecycleState?: string, + error?: AppError, +): Promise { + emitRunnerInvalidationDecision({ + command: context.command, + session: context.session, + transportError: context.transportError, + decision: 'retained', + reason, + lifecycleState, + }); + await invalidateRunnerSession(context.session, context.invalidationReason); + throw error ?? context.transportError; +} + async function tryRecoverRunnerCommandAfterTransportError( device: DeviceInfo, session: RunnerSession, @@ -229,7 +326,7 @@ async function tryRecoverRunnerCommandAfterTransportError( transportError: AppError, options: AppleRunnerCommandOptions, signal?: AbortSignal, -): Promise | undefined> { +): Promise { if (command.command === 'status' || !command.commandId?.trim()) return undefined; let status: Record; try { @@ -251,7 +348,7 @@ async function tryRecoverRunnerCommandAfterTransportError( error: error instanceof Error ? error.message : String(error), }, }); - return undefined; + return { type: 'retainInvalidation', reason: 'status_probe_failed' }; } const lifecycleState = typeof status.lifecycleState === 'string' ? status.lifecycleState : ''; @@ -279,20 +376,48 @@ function handleRunnerCommandStatusRecovery( command: RunnerCommand, transportError: AppError, options: AppleRunnerCommandOptions, -): Record | undefined { +): RunnerTransportRecovery | undefined { if (lifecycleState === 'completed') { return handleCompletedRunnerStatus(status, command, transportError, options); } if (lifecycleState === 'failed') { - throw runnerStatusFailureError(status, command, transportError, options); + return { + type: 'skipInvalidation', + reason: 'runner_reported_failure', + lifecycleState, + error: runnerStatusFailureError(status, command, transportError, options), + }; } if (lifecycleState === 'accepted' || lifecycleState === 'started') { - throw runnerStatusInFlightError(lifecycleState, command, transportError, options); + return { + type: 'skipInvalidation', + reason: 'command_still_in_flight', + lifecycleState, + error: runnerStatusInFlightError(lifecycleState, command, transportError, options), + }; } - return undefined; + return { + type: 'retainInvalidation', + reason: lifecycleState ? 'unknown_lifecycle_state' : 'missing_lifecycle_state', + lifecycleState, + error: new AppError( + 'COMMAND_FAILED', + `Runner command "${command.command}" lost its transport response and lifecycle status was ${lifecycleState ? `"${lifecycleState}"` : 'missing'}, so agent-device invalidated the runner session instead of replaying the command.`, + { + command: command.command, + commandId: command.commandId, + lifecycleState, + recovery: 'lifecycle_state_not_recoverable', + hint: unknownLifecycleStateHint(command.command), + logPath: options.logPath, + transportError: transportError.message, + }, + transportError, + ), + }; } function handleCompletedRunnerStatus( @@ -300,26 +425,43 @@ function handleCompletedRunnerStatus( command: RunnerCommand, transportError: AppError, options: AppleRunnerCommandOptions, -): Record | undefined { +): RunnerTransportRecovery { const recovered = parseLifecycleResponseJson(status.lifecycleResponseJson); - if (recovered) return recovered; - if (isReadOnlyRunnerCommand(command.command)) { - throw transportError; + if (recovered) { + return { + type: 'recovered', + data: recovered, + reason: 'completed_with_retained_response', + lifecycleState: 'completed', + }; } - throw new AppError( - 'COMMAND_FAILED', - `Runner command "${command.command}" completed after the transport response was lost, but no recoverable response was retained.`, - { - command: command.command, - commandId: command.commandId, + if (isReadOnlyRunnerCommand(command.command)) { + return { + type: 'skipInvalidation', + error: transportError, + reason: 'read_only_completed_without_retained_response', lifecycleState: 'completed', - recovery: 'completed_without_retained_response', - hint: completedWithoutRetainedResponseHint(command.command), - logPath: options.logPath, - transportError: transportError.message, - }, - transportError, - ); + }; + } + return { + type: 'skipInvalidation', + reason: 'completed_without_retained_response', + lifecycleState: 'completed', + error: new AppError( + 'COMMAND_FAILED', + `Runner command "${command.command}" completed after the transport response was lost, but no recoverable response was retained.`, + { + command: command.command, + commandId: command.commandId, + lifecycleState: 'completed', + recovery: 'completed_without_retained_response', + hint: completedWithoutRetainedResponseHint(command.command), + logPath: options.logPath, + transportError: transportError.message, + }, + transportError, + ), + }; } function runnerStatusFailureError( @@ -369,7 +511,7 @@ function runnerStatusInFlightError( commandId: command.commandId, lifecycleState, recovery: 'command_still_in_flight', - hint: inFlightAfterLostResponseHint(command.command), + hint: inFlightAfterLostResponseHint(command.command, lifecycleState), logPath: options.logPath, transportError: transportError.message, }, @@ -396,15 +538,44 @@ function parseLifecycleResponsePayload(value: string): LifecycleResponsePayload } function completedWithoutRetainedResponseHint(command: string): string { - return `The runner reports "${command}" already completed, so agent-device will not replay it. Run snapshot -i to inspect the current UI, then continue from that observed state. If the session is stale, close and reopen the session before retrying.`; + return `The runner is still reachable and reports "${command}" already completed, so agent-device kept the session open and will not replay it. Run snapshot -i to inspect the current UI, then continue from that observed state.`; } function runnerReportedFailureHint(command: string): string { - return `The runner observed "${command}" fail after the transport response was lost, so agent-device did not replay it. Run snapshot -i to inspect the current UI and retry with a selector visible in that snapshot. If the session is stale, close and reopen the session before retrying.`; + return `The runner is still reachable and reports "${command}" failed after the transport response was lost, so agent-device kept the session open and did not replay it. Run snapshot -i to inspect the current UI and retry with a selector visible in that snapshot.`; +} + +function inFlightAfterLostResponseHint(command: string, lifecycleState: string): string { + return `The runner is still reachable and reports "${command}" is ${lifecycleState}, so agent-device kept the session open and will not replay it. Wait briefly, run snapshot -i to inspect the current UI, then continue from that observed state.`; +} + +function unknownLifecycleStateHint(command: string): string { + return `The runner did not confirm that "${command}" reached a safe terminal state, so agent-device kept the conservative invalidation path. Run snapshot -i before retrying if the UI may have changed.`; } -function inFlightAfterLostResponseHint(command: string): string { - return `The runner has accepted "${command}" and it may still finish, so agent-device will not replay it. Wait briefly, run snapshot -i to inspect the current UI, then continue from that observed state. If the session stops responding, close and reopen the session before retrying.`; +function emitRunnerInvalidationDecision(params: { + command: RunnerCommand; + session: RunnerSession; + transportError: AppError; + decision: 'skipped' | 'retained'; + reason: string; + lifecycleState?: string; +}): void { + const { command, session, transportError, decision, reason, lifecycleState } = params; + emitDiagnostic({ + level: decision === 'retained' ? 'warn' : 'debug', + phase: 'ios_runner_command_invalidation_decision', + data: { + command: command.command, + commandId: command.commandId, + decision, + reason, + lifecycleState, + runnerReachable: lifecycleState !== undefined, + sessionId: session.sessionId, + transportError: transportError.message, + }, + }); } function isRunnerReadinessPreflightError(error: AppError): boolean {