add workflow run block screenshot and observer thought screenshots

[wintonzheng]

Important

Adds functionality to capture and store screenshots for workflow run blocks and observer thoughts, including new methods for artifact creation and storage.

• Behavior:
  • Adds create_workflow_run_block_artifact() in manager.py to create artifacts for workflow run blocks.
  • Implements _record_thought_screenshot() in observer_service.py to capture and store screenshots for observer thoughts.
  • Modifies execute_safe() in block.py to capture screenshots during block execution.
• Storage:
  • Adds build_workflow_run_block_uri() in base.py, local.py, and s3.py to generate URIs for workflow run block artifacts.
• Misc:
  • Imports WorkflowRunBlock in several files to support new functionality.

^{This description was created by for 799b06a. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Incremental review on 799b06a in 30 seconds

More details

• Looked at 225 lines of code in 8 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. skyvern/forge/sdk/workflow/models/block.py:215

• Draft comment:
  Add a check to ensure screenshot is not None before creating an artifact to avoid unintended behavior.

                if screenshot is not None:

• Reason this comment was not posted:
  Comment looked like it was already resolved.

Workflow ID: wflow_ISfH7oQofK5ZzzHp

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 799b06a in 1 minute and 10 seconds

More details

• Looked at 225 lines of code in 8 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. skyvern/forge/sdk/workflow/models/block.py:214

• Draft comment:
  Consider adding a check to ensure the screenshot was successfully taken before attempting to create an artifact. Log a warning if the screenshot is not available.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The code in execute_safe method in block.py is responsible for creating a screenshot and storing it as an artifact. However, the current implementation does not handle the case where the screenshot is not successfully taken, which could lead to potential issues if the screenshot is expected to be available later. Adding a check for the screenshot's success and logging a warning if it fails would improve robustness.

2. skyvern/forge/sdk/artifact/storage/local.py:48

• Draft comment:
  Consider appending a unique identifier, such as a UUID, to the URI to ensure uniqueness when datetime.utcnow().isoformat() is used multiple times within the same second. This comment also applies to s3.py.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The build_workflow_run_block_uri method in local.py and s3.py uses datetime.utcnow().isoformat() to generate a timestamp for the URI. This could lead to issues if the method is called multiple times within the same second, as the URIs would be identical. Adding a unique identifier, such as a UUID, would ensure uniqueness.

3. skyvern/forge/sdk/artifact/manager.py:162

• Draft comment:
  Ensure that either data or path is provided when calling create_workflow_run_block_artifact. Consider adding a check to raise an exception if both are None.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The create_workflow_run_block_artifact method in manager.py does not handle the case where both data and path are None. This could lead to unexpected behavior if the method is called without providing either. Adding a check to ensure at least one of them is provided would prevent this issue.

Workflow ID: wflow_NjC9xwhIgdxeAyuN

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.