enable UrlBlock building in workflow service

[wintonzheng]

Important

Add support for UrlBlock in workflow service to handle GOTO_URL tasks, including model updates and task execution logic changes.

• Behavior:
  • Adds support for UrlBlock in execute_step() in agent.py to handle GOTO_URL tasks by marking them as completed immediately.
  • Updates run_observer_task_helper() in observer_service.py to initialize GOTO_URL tasks in the first iteration.
• Models:
  • Adds UrlBlock and UrlBlockYAML to observer_service.py and service.py.
  • Updates StepStatus in models.py to allow transition to completed from created.
• Functions:
  • Adds _generate_goto_url_task() in observer_service.py and service.py to create GOTO_URL tasks.
• Misc:
  • Removes validation for is_last in validate_update() in models.py.

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 604df13 in 1 minute and 0 seconds

More details

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

1. skyvern/forge/sdk/workflow/service.py:1752

• Draft comment:
  Missing import for UrlBlockYAML. Add from skyvern.forge.sdk.workflow.models.yaml import UrlBlockYAML to avoid runtime errors.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment points out a potential issue with missing imports. However, looking at the full file, I don't see any evidence that UrlBlockYAML is actually needed. The block_yaml_to_block function handles YAML conversion for all block types, and it's using the existing YAML types from models.yaml. There's no indication that a special YAML class is needed for URL blocks.
  I could be wrong if there's a separate YAML models file that defines block types that I can't see. The comment author may have knowledge of the codebase architecture that I don't have.
  While that's possible, the pattern shown in the code suggests that block types are handled through the existing YAML infrastructure. The comment makes an assumption without strong evidence.
  The comment should be deleted as it suggests adding an import without clear evidence that it's needed. The existing YAML handling appears sufficient.

Workflow ID: wflow_GBAFJj4J5Dwyxpkz

---

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 604df13 in 1 minute and 47 seconds

More details

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

1. skyvern/forge/sdk/workflow/service.py:1755

• Draft comment:
  Missing import for UrlBlockYAML. Add from skyvern.forge.sdk.workflow.models.yaml import UrlBlockYAML to ensure the new UrlBlock type is properly integrated.
• Reason this comment was not posted:
  Comment was not on a valid diff hunk.

Workflow ID: wflow_yA3a0pKnei1lYkAo

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on bd885b3 in 1 minute and 9 seconds

More details

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

1. skyvern/forge/agent.py:345

• Draft comment:
  The logic here assumes that if a task has no navigation_goal, data_extraction_goal, complete_criterion, or terminate_criterion, it is a GOTO_URL task. This assumption might not always hold true, leading to incorrect task completion status. Consider adding a more explicit check for GOTO_URL tasks.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment raises a valid concern about the assumption, but looking at the code more carefully:

1. The comment is speculative ("might not always hold true")
2. The code comment already acknowledges this is "most likely" a GOTO_URL task
3. The behavior seems intentional - if a task has none of these goals/criteria, what else could it be for?
4. There's no clear suggestion for what a "more explicit check" would look like
5. The code handles this case gracefully by just completing the task
   I could be missing some edge cases where a task legitimately has no goals or criteria but isn't a GOTO_URL task. The code comment's use of "most likely" suggests the developers are aware of some ambiguity.
   While edge cases might exist, the current behavior is a reasonable default - if a task has no goals or success criteria, marking it complete is sensible. The speculative nature of the comment without concrete examples or clear fixes makes it not very actionable.
   Delete the comment. While it raises a theoretical concern, it's speculative and doesn't provide actionable feedback. The code's behavior is reasonable and the inline comment already acknowledges the assumption.

2. skyvern/forge/sdk/models.py:83

• Draft comment:
  The check for is_last is False seems unnecessary and restrictive. Consider removing this check to allow more flexibility in updating steps.
• Reason this comment was not posted:
  Comment was on unchanged code.

3. skyvern/forge/sdk/services/observer_service.py:368

• Draft comment:
  The assignment task_history_record = {"type": task_type, "task": plan} is redundant as it is already assigned on line 363. Consider removing this line to clean up the code.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In observer_service.py, the run_observer_task_helper function has a redundant assignment of task_history_record in the first iteration block. This can be removed to clean up the code.

Workflow ID: wflow_ZgfKre45AGk6sqMU

---

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