Skip to content

Conversation

Linell
Copy link

@Linell Linell commented Sep 25, 2025

@Linell Linell requested review from amh4r and Copilot September 25, 2025 18:37
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for the StepFailed opcode to properly handle steps that have exhausted their retry attempts. The implementation distinguishes between retryable step errors and final failures.

  • Adds StepFailed opcode and error code to handle final step failures
  • Implements logic to check if maximum attempts are reached and raise StepFailed instead of StepError
  • Extends server request context to include max_attempts field

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
step_sync.py Adds logic to detect final attempt and raise StepFailed error with new opcode
step_async.py Mirror implementation of step failure logic for async operations
execution_request.py Adds max_attempts field to server request context
consts.py Defines new STEP_FAILED error code and opcode constants
errors.py Implements StepFailed error class as non-retryable StepError

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Linell Linell force-pushed the linell/step-failed-opcode-v2 branch 3 times, most recently from ff15101 to 9983290 Compare September 25, 2025 21:09
@Linell Linell force-pushed the linell/step-failed-opcode-v2 branch from 9983290 to 4b8238c Compare September 26, 2025 17:54
@Linell Linell requested a review from amh4r September 26, 2025 17:55
self._stack = stack


class StepFailedError(StepError):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can delete this now!

SIGNING_KEY_UNSPECIFIED = "signing_key_unspecified"
SIG_VERIFICATION_FAILED = "sig_verification_failed"
STEP_ERRORED = "step_errored"
STEP_FAILED = "step_failed"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can delete this too

Comment on lines +221 to +223
raise base.ResponseInterrupt(
base.StepResponse(original_error=err, step=step_info)
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise base.ResponseInterrupt(
base.StepResponse(original_error=err, step=step_info)
)

Not needed since we're already raising below

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants