refactor(app, api): Extend Error Recovery Policies to "fixit" commands

api/src/opentrons/protocol_engine/state/commands.py

@@ -340,7 +340,9 @@ def _handle_succeed_command_action(self, action: SucceedCommandAction) -> None:
         succeeded_command = action.command
         self._state.command_history.set_command_succeeded(succeeded_command)
 
-    def _handle_fail_command_action(self, action: FailCommandAction) -> None:
+    def _handle_fail_command_action(  # noqa: C901
+        self, action: FailCommandAction
+    ) -> None:
         prev_entry = self.state.command_history.get(action.command_id)
 
         if isinstance(action.error, EnumeratedError):  # The error was undefined.
@@ -383,9 +385,15 @@ def _handle_fail_command_action(self, action: FailCommandAction) -> None:
                 self._state.command_history.get_setup_queue_ids()
             )
         elif prev_entry.command.intent == CommandIntent.FIXIT:
-            other_command_ids_to_fail = list(
-                self._state.command_history.get_fixit_queue_ids()
-            )
+            if (
+                action.type == ErrorRecoveryType.CONTINUE_WITH_ERROR
+                or action.type == ErrorRecoveryType.ASSUME_FALSE_POSITIVE_AND_CONTINUE
+            ):
+                other_command_ids_to_fail = []
+            else:
+                other_command_ids_to_fail = list(
+                    self._state.command_history.get_fixit_queue_ids()
+                )
         elif (
             prev_entry.command.intent == CommandIntent.PROTOCOL
             or prev_entry.command.intent is None

api/tests/opentrons/protocol_engine/state/test_command_state.py

@@ -389,6 +389,112 @@ def test_nonfatal_command_failure() -> None:
     assert subject_view.get_running_command_id() is None
 
 
+def test_fixit_command_failure_handling_by_recovery_type() -> None:
+    """Test that fixit command failures are handled differently based on error recovery type.
+
+    When a fixit command fails with permissible error recovery types, other queued fixit
+    commands should remain in the queue as is.
+
+    When a fixit command fails with other error recovery types, all queued fixit commands
+    should be marked as failed.
+    """
+
+    def test_with_recovery_type(
+        recovery_type: ErrorRecoveryType, should_fail_queued_cmds: bool
+    ) -> None:
+        subject = CommandStore(
+            is_door_open=False,
+            config=_make_config(),
+            error_recovery_policy=_placeholder_error_recovery_policy,
+        )
+        subject_view = CommandView(subject.state)
+
+        protocol_cmd = actions.QueueCommandAction(
+            request=commands.CommentCreate(
+                params=commands.CommentParams(message=""),
+                key="protocol-cmd",
+            ),
+            request_hash=None,
+            created_at=datetime(year=2021, month=1, day=1),
+            command_id="protocol-cmd-id",
+        )
+        subject.handle_action(protocol_cmd)
+        subject.handle_action(
+            actions.RunCommandAction(
+                command_id="protocol-cmd-id",
+                started_at=datetime(year=2021, month=1, day=2),
+            )
+        )
+        subject.handle_action(
+            actions.FailCommandAction(
+                command_id="protocol-cmd-id",
+                running_command=subject_view.get("protocol-cmd-id"),
+                error_id="error-1",
+                failed_at=datetime(year=2021, month=1, day=3),
+                error=errors.ProtocolEngineError(message="recovery needed"),
+                notes=[],
+                type=ErrorRecoveryType.WAIT_FOR_RECOVERY,
+            )
+        )
+
+        for i in range(1, 4):
+            subject.handle_action(
+                actions.QueueCommandAction(
+                    request=commands.CommentCreate(
+                        params=commands.CommentParams(message=f"fixit {i}"),
+                        key=f"fixit-{i}",
+                        intent=commands.CommandIntent.FIXIT,
+                    ),
+                    request_hash=None,
+                    created_at=datetime(year=2021, month=2, day=i),
+                    command_id=f"fixit-id-{i}",
+                )
+            )
+
+        subject.handle_action(
+            actions.RunCommandAction(
+                command_id="fixit-id-1",
+                started_at=datetime(year=2021, month=2, day=10),
+            )
+        )
+        subject.handle_action(
+            actions.FailCommandAction(
+                command_id="fixit-id-1",
+                running_command=subject_view.get("fixit-id-1"),
+                error_id="error-2",
+                failed_at=datetime(year=2021, month=2, day=11),
+                error=errors.ProtocolEngineError(
+                    message=f"fixit failed with {recovery_type}"
+                ),
+                notes=[],
+                type=recovery_type,
+            )
+        )
+
+        if should_fail_queued_cmds:
+            assert all(
+                c.status == commands.CommandStatus.FAILED
+                for c in subject_view.get_all()
+            )
+        else:
+            assert [(c.id, c.status) for c in subject_view.get_all()] == [
+                ("protocol-cmd-id", commands.CommandStatus.FAILED),
+                ("fixit-id-1", commands.CommandStatus.FAILED),
+                ("fixit-id-2", commands.CommandStatus.QUEUED),
+                ("fixit-id-3", commands.CommandStatus.QUEUED),
+            ]
+            assert subject_view.get_next_to_execute() == "fixit-id-2"
+
+    test_with_recovery_type(
+        ErrorRecoveryType.CONTINUE_WITH_ERROR, should_fail_queued_cmds=False
+    )
+    test_with_recovery_type(
+        ErrorRecoveryType.ASSUME_FALSE_POSITIVE_AND_CONTINUE,
+        should_fail_queued_cmds=False,
+    )
+    test_with_recovery_type(ErrorRecoveryType.FAIL_RUN, should_fail_queued_cmds=True)
+
+
 def test_door_during_setup_phase() -> None:
     """Test behavior when the door is opened during the setup phase."""
     subject = CommandStore(

app/src/local-resources/commands/utils/__tests__/shouldCommandSucceedGivenRecoveryPolicy.test.ts

@@ -0,0 +1,202 @@
+import { describe, expect, it } from 'vitest'
+
+import { shouldCommandSucceedGivenRecoveryPolicy } from '..'
+
+import type { ErrorRecoveryPolicy } from '@opentrons/api-client'
+
+describe('shouldCommandSucceedGivenRecoveryPolicy', () => {
+  const COMMAND_TYPE = 'pickUpTip'
+  const ERROR_TYPE = 'assumeFalsePositiveAndContinue'
+  const DIFFERENT_COMMAND_TYPE = 'dropTip'
+  const DIFFERENT_ERROR_TYPE = 'ignoreAndContinue'
+
+  const createCommand = (commandType: any, errorType: string | null): any => ({
+    commandType,
+    params: {},
+    ...(errorType
+      ? { error: { errorType, detail: {}, message: 'Test error' } }
+      : {}),
+  })
+
+  const createPolicy = (
+    commandType: any,
+    errorType: string,
+    ifMatch: string
+  ): ErrorRecoveryPolicy => ({
+    policyRules: [
+      {
+        matchCriteria: {
+          command: {
+            commandType,
+            error: {
+              errorType,
+            },
+          },
+        },
+        ifMatch: ifMatch as any,
+      },
+    ],
+  })
+
+  it('should return true for commands without errors, regardless of policy', () => {
+    const cmd = createCommand(COMMAND_TYPE, null)
+    const policy = createPolicy(COMMAND_TYPE, ERROR_TYPE, 'waitForRecovery')
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(true)
+  })
+
+  it('should return true for commands with errors but no policy', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, undefined)
+
+    expect(result).toBe(true)
+  })
+
+  it('should return true for commands with errors and matching policy with ifMatch = "ignoreAndContinue"', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy = createPolicy(COMMAND_TYPE, ERROR_TYPE, 'ignoreAndContinue')
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(true)
+  })
+
+  it('should return true for commands with errors and matching policy with ifMatch = "assumeFalsePositiveAndContinue"', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy = createPolicy(
+      COMMAND_TYPE,
+      ERROR_TYPE,
+      'assumeFalsePositiveAndContinue'
+    )
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(true)
+  })
+
+  it('should return false for commands with errors and matching policy with ifMatch = "failRun"', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy = createPolicy(COMMAND_TYPE, ERROR_TYPE, 'failRun')
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(false)
+  })
+
+  it('should return false for commands with errors and matching policy with ifMatch = "waitForRecovery"', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy = createPolicy(COMMAND_TYPE, ERROR_TYPE, 'waitForRecovery')
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(false)
+  })
+
+  it('should return false for commands with errors and policy with non-matching commandType', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy = createPolicy(
+      DIFFERENT_COMMAND_TYPE,
+      ERROR_TYPE,
+      'ignoreAndContinue'
+    )
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(false)
+  })
+
+  it('should return false for commands with errors and policy with non-matching errorType', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy = createPolicy(
+      COMMAND_TYPE,
+      DIFFERENT_ERROR_TYPE,
+      'ignoreAndContinue'
+    )
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(false)
+  })
+
+  it('should return true if at least one policy rule matches and allows continuation', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy: ErrorRecoveryPolicy = {
+      policyRules: [
+        {
+          matchCriteria: {
+            command: {
+              commandType: DIFFERENT_COMMAND_TYPE,
+              error: {
+                errorType: ERROR_TYPE,
+              },
+            },
+          },
+          ifMatch: 'ignoreAndContinue',
+        },
+        {
+          matchCriteria: {
+            command: {
+              commandType: COMMAND_TYPE,
+              error: {
+                errorType: ERROR_TYPE,
+              },
+            },
+          },
+          ifMatch: 'ignoreAndContinue',
+        },
+      ],
+    }
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(true)
+  })
+
+  it('should return false if no policy rule matches', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy: ErrorRecoveryPolicy = {
+      policyRules: [
+        {
+          matchCriteria: {
+            command: {
+              commandType: DIFFERENT_COMMAND_TYPE,
+              error: {
+                errorType: ERROR_TYPE,
+              },
+            },
+          },
+          ifMatch: 'ignoreAndContinue',
+        },
+        {
+          matchCriteria: {
+            command: {
+              commandType: COMMAND_TYPE,
+              error: {
+                errorType: DIFFERENT_ERROR_TYPE,
+              },
+            },
+          },
+          ifMatch: 'ignoreAndContinue',
+        },
+      ],
+    }
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(false)
+  })
+
+  it('should work with an empty policy rules array', () => {
+    const cmd = createCommand(COMMAND_TYPE, ERROR_TYPE)
+    const policy: ErrorRecoveryPolicy = {
+      policyRules: [],
+    }
+
+    const result = shouldCommandSucceedGivenRecoveryPolicy(cmd, policy)
+
+    expect(result).toBe(false)
+  })
+})

app/src/local-resources/commands/utils/index.ts

@@ -1 +1,2 @@
 export * from './lastRunCommandPromptedErrorRecovery'
+export * from './shouldCommandSucceedGivenRecoveryPolicy'

app/src/local-resources/commands/utils/shouldCommandSucceedGivenRecoveryPolicy.ts

@@ -0,0 +1,28 @@
+import some from 'lodash/some'
+
+import type { ErrorRecoveryPolicy } from '@opentrons/api-client'
+import type { RunTimeCommand } from '@opentrons/shared-data'
+
+// Whether the command should succeed given the error recovery policy.
+export function shouldCommandSucceedGivenRecoveryPolicy(
+  cmd: RunTimeCommand,
+  policy: ErrorRecoveryPolicy | undefined
+): boolean {
+  if (!policy || !cmd.error) {
+    if (cmd.error) {
+      console.warn(
+        'Check for command errors before passing invoking isCommandErrorRecoveryPolicyException.'
+      )
+    }
+    return true
+  }
+
+  return some(
+    policy.policyRules,
+    rule =>
+      rule.matchCriteria.command.commandType === cmd.commandType &&
+      rule.matchCriteria.command.error.errorType === cmd.error?.errorType &&
+      (rule.ifMatch === 'assumeFalsePositiveAndContinue' ||
+        rule.ifMatch === 'ignoreAndContinue')
+  )
+}

app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts

@@ -2,6 +2,7 @@ import { act, renderHook } from '@testing-library/react'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 import {
+  useErrorRecoveryPolicy,
   useResumeRunFromRecoveryAssumingFalsePositiveMutation,
   useResumeRunFromRecoveryMutation,
   useStopRunMutation,
@@ -91,6 +92,7 @@ describe('useRecoveryCommands', () => {
     ).mockReturnValue({
       mutateAsync: mockResumeRunFromRecoveryAssumingFalsePositive,
     } as any)
+    vi.mocked(useErrorRecoveryPolicy).mockReturnValue({} as any)
   })
 
   it('should call chainRunRecoveryCommands with continuePastCommandFailure set to false', async () => {