fix: reduce ios runner invalidation after status recovery (#666)

Author: thymikee

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<typeof import('../../../utils/diagnostics.ts')>(
+    '../../../utils/diagnostics.ts',
+  );
+  return {
+    ...actual,
+    emitDiagnostic: mockEmitDiagnostic,
+  };
+});
+
 vi.mock('../runner-session.ts', async () => {
   const actual =
     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
   ]);
   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> = {}): RunnerSession {
   return {
     sessionId: `session-${overrides.port ?? 8100}`,