Skip to content

Commit 6d0e46c

Browse files
committed
fix: reduce ios runner invalidation after status recovery
1 parent f2ef688 commit 6d0e46c

2 files changed

Lines changed: 300 additions & 38 deletions

File tree

src/platforms/ios/__tests__/runner-command-retry.test.ts

Lines changed: 134 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,27 @@ import type { RunnerSession } from '../runner-session-types.ts';
77
const {
88
mockEnsureRunnerSession,
99
mockExecuteRunnerCommandWithSession,
10+
mockEmitDiagnostic,
1011
mockInvalidateRunnerSession,
1112
mockStopRunnerSession,
1213
} = vi.hoisted(() => ({
1314
mockEnsureRunnerSession: vi.fn(),
1415
mockExecuteRunnerCommandWithSession: vi.fn(),
16+
mockEmitDiagnostic: vi.fn(),
1517
mockInvalidateRunnerSession: vi.fn(),
1618
mockStopRunnerSession: vi.fn(),
1719
}));
1820

21+
vi.mock('../../../utils/diagnostics.ts', async () => {
22+
const actual = await vi.importActual<typeof import('../../../utils/diagnostics.ts')>(
23+
'../../../utils/diagnostics.ts',
24+
);
25+
return {
26+
...actual,
27+
emitDiagnostic: mockEmitDiagnostic,
28+
};
29+
});
30+
1931
vi.mock('../runner-session.ts', async () => {
2032
const actual =
2133
await vi.importActual<typeof import('../runner-session.ts')>('../runner-session.ts');
@@ -151,6 +163,11 @@ test('mutating commands do not restart or replay after command send failure', as
151163
]);
152164
assert.equal(mockStopRunnerSession.mock.calls.length, 0);
153165
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
166+
assertDiagnosticDecision({
167+
decision: 'retained',
168+
reason: 'unknown_lifecycle_state',
169+
lifecycleState: 'notAccepted',
170+
});
154171
});
155172

156173
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
168185

169186
assert.deepEqual(result, { message: 'tapped' });
170187
assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0);
188+
assertDiagnosticDecision({
189+
decision: 'skipped',
190+
reason: 'completed_with_retained_response',
191+
lifecycleState: 'completed',
192+
});
171193
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
172194
const sentCommand = mockExecuteRunnerCommandWithSession.mock.calls[0]?.[2];
173195
const statusCommand = mockExecuteRunnerCommandWithSession.mock.calls[1]?.[2];
@@ -193,6 +215,43 @@ test('mutating commands keep invalidating when status cannot find the command',
193215
[session, 'transport_error_after_command_send'],
194216
]);
195217
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
218+
assertDiagnosticDecision({
219+
decision: 'retained',
220+
reason: 'unknown_lifecycle_state',
221+
lifecycleState: 'notAccepted',
222+
});
223+
});
224+
225+
test('mutating commands keep invalidating when status reports an unknown lifecycle state', async () => {
226+
const session = makeRunnerSession({ port: 8100, ready: true });
227+
228+
mockEnsureRunnerSession.mockResolvedValueOnce(session);
229+
mockExecuteRunnerCommandWithSession
230+
.mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed'))
231+
.mockResolvedValueOnce({
232+
lifecycleState: 'paused',
233+
});
234+
235+
await assert.rejects(
236+
() => runIosRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }),
237+
(error: unknown) => {
238+
assert.ok(error instanceof AppError);
239+
assert.match(error.message, /lifecycle status was "paused"/);
240+
assert.equal(error.details?.recovery, 'lifecycle_state_not_recoverable');
241+
assert.match(String(error.details?.hint), /conservative invalidation path/);
242+
return true;
243+
},
244+
);
245+
246+
assert.deepEqual(mockInvalidateRunnerSession.mock.calls, [
247+
[session, 'transport_error_after_command_send'],
248+
]);
249+
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
250+
assertDiagnosticDecision({
251+
decision: 'retained',
252+
reason: 'unknown_lifecycle_state',
253+
lifecycleState: 'paused',
254+
});
196255
});
197256

198257
test('read-only commands retry when completed status has no retained response', async () => {
@@ -211,6 +270,11 @@ test('read-only commands retry when completed status has no retained response',
211270
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 3);
212271
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[1]?.[2].command, 'status');
213272
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[2]?.[2].command, 'snapshot');
273+
assertDiagnosticDecision({
274+
decision: 'skipped',
275+
reason: 'read_only_completed_without_retained_response',
276+
lifecycleState: 'completed',
277+
});
214278
});
215279

216280
test('read-only commands retry when status shows in-flight work', async () => {
@@ -245,6 +309,7 @@ test('mutating commands report recovery guidance when completed status has no re
245309
assert.ok(error instanceof AppError);
246310
assert.match(error.message, /"tap" completed after the transport response was lost/);
247311
assert.equal(error.details?.recovery, 'completed_without_retained_response');
312+
assert.match(String(error.details?.hint), /kept the session open/);
248313
assert.match(String(error.details?.hint), /will not replay/);
249314
assert.match(String(error.details?.hint), /snapshot -i/);
250315
assert.equal(error.details?.transportError, 'fetch failed');
@@ -254,6 +319,11 @@ test('mutating commands report recovery guidance when completed status has no re
254319

255320
assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0);
256321
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
322+
assertDiagnosticDecision({
323+
decision: 'skipped',
324+
reason: 'completed_without_retained_response',
325+
lifecycleState: 'completed',
326+
});
257327
});
258328

259329
test('mutating commands preserve runner failure details from status recovery', async () => {
@@ -284,6 +354,41 @@ test('mutating commands preserve runner failure details from status recovery', a
284354

285355
assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0);
286356
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
357+
assertDiagnosticDecision({
358+
decision: 'skipped',
359+
reason: 'runner_reported_failure',
360+
lifecycleState: 'failed',
361+
});
362+
});
363+
364+
test('mutating commands use recovery guidance when failed status has no runner hint', async () => {
365+
const session = makeRunnerSession({ port: 8100, ready: true });
366+
367+
mockEnsureRunnerSession.mockResolvedValueOnce(session);
368+
mockExecuteRunnerCommandWithSession
369+
.mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed'))
370+
.mockResolvedValueOnce({
371+
lifecycleState: 'failed',
372+
lifecycleErrorMessage: 'Runner command failed after dispatch',
373+
});
374+
375+
await assert.rejects(
376+
() => runIosRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }),
377+
(error: unknown) => {
378+
assert.ok(error instanceof AppError);
379+
assert.equal(error.message, 'Runner command failed after dispatch');
380+
assert.match(String(error.details?.hint), /kept the session open/);
381+
assert.match(String(error.details?.hint), /did not replay/);
382+
return true;
383+
},
384+
);
385+
386+
assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0);
387+
assertDiagnosticDecision({
388+
decision: 'skipped',
389+
reason: 'runner_reported_failure',
390+
lifecycleState: 'failed',
391+
});
287392
});
288393

289394
test('mutating commands report wait-and-inspect guidance when status shows in-flight work', async () => {
@@ -300,7 +405,7 @@ test('mutating commands report wait-and-inspect guidance when status shows in-fl
300405
assert.ok(error instanceof AppError);
301406
assert.match(error.message, /"tap" is still started/);
302407
assert.equal(error.details?.recovery, 'command_still_in_flight');
303-
assert.match(String(error.details?.hint), /may still finish/);
408+
assert.match(String(error.details?.hint), /kept the session open/);
304409
assert.match(String(error.details?.hint), /snapshot -i/);
305410
assert.equal(error.details?.transportError, 'fetch failed');
306411
return true;
@@ -309,6 +414,11 @@ test('mutating commands report wait-and-inspect guidance when status shows in-fl
309414

310415
assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0);
311416
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 2);
417+
assertDiagnosticDecision({
418+
decision: 'skipped',
419+
reason: 'command_still_in_flight',
420+
lifecycleState: 'started',
421+
});
312422
});
313423

314424
test('mutating commands invalidate the retry session without replaying again', async () => {
@@ -331,8 +441,31 @@ test('mutating commands invalidate the retry session without replaying again', a
331441
[freshSession, 'transport_error_after_retry_command_send'],
332442
]);
333443
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 3);
444+
assertDiagnosticDecision({
445+
decision: 'retained',
446+
reason: 'unknown_lifecycle_state',
447+
lifecycleState: 'notAccepted',
448+
});
334449
});
335450

451+
function assertDiagnosticDecision(expected: {
452+
decision: 'skipped' | 'retained';
453+
reason: string;
454+
lifecycleState?: string;
455+
}): void {
456+
assert.ok(
457+
mockEmitDiagnostic.mock.calls.some(([event]) => {
458+
return (
459+
event.phase === 'ios_runner_command_invalidation_decision' &&
460+
event.data?.decision === expected.decision &&
461+
event.data?.reason === expected.reason &&
462+
event.data?.lifecycleState === expected.lifecycleState
463+
);
464+
}),
465+
`missing invalidation decision diagnostic ${JSON.stringify(expected)}`,
466+
);
467+
}
468+
336469
function makeRunnerSession(overrides: Partial<RunnerSession> = {}): RunnerSession {
337470
return {
338471
sessionId: `session-${overrides.port ?? 8100}`,

0 commit comments

Comments
 (0)