From 5e23c580e7b5a82e675e82a9bc74fb7c1b8051a9 Mon Sep 17 00:00:00 2001 From: Stanislav Novosad Date: Thu, 15 Jan 2026 14:08:30 -0700 Subject: [PATCH] Validate all block parameters are defined in workflow (#4464) --- skyvern/forge/sdk/routes/agent_protocol.py | 10 +- skyvern/forge/sdk/workflow/exceptions.py | 25 ++++- .../workflow/workflow_definition_converter.py | 100 +++++++++--------- 3 files changed, 79 insertions(+), 56 deletions(-) diff --git a/skyvern/forge/sdk/routes/agent_protocol.py b/skyvern/forge/sdk/routes/agent_protocol.py index 2fd4dedd..b5f14a4c 100644 --- a/skyvern/forge/sdk/routes/agent_protocol.py +++ b/skyvern/forge/sdk/routes/agent_protocol.py @@ -90,7 +90,7 @@ from skyvern.forge.sdk.workflow.exceptions import ( FailedToCreateWorkflow, FailedToUpdateWorkflow, InvalidTemplateWorkflowPermanentId, - WorkflowParameterMissingRequiredValue, + WorkflowDefinitionValidationException, ) from skyvern.forge.sdk.workflow.models.workflow import ( RunWorkflowResponse, @@ -522,7 +522,7 @@ async def create_workflow_legacy( return await app.WORKFLOW_SERVICE.create_workflow_from_request( organization=current_org, request=workflow_create_request ) - except WorkflowParameterMissingRequiredValue as e: + except WorkflowDefinitionValidationException as e: raise e except Exception as e: LOG.error("Failed to create workflow", exc_info=True, organization_id=current_org.organization_id) @@ -583,7 +583,7 @@ async def create_workflow( ) except yaml.YAMLError: raise HTTPException(status_code=422, detail="Invalid YAML") - except WorkflowParameterMissingRequiredValue as e: + except WorkflowDefinitionValidationException as e: raise e except Exception as e: LOG.error("Failed to create workflow", exc_info=True, organization_id=current_org.organization_id) @@ -842,7 +842,7 @@ async def update_workflow_legacy( status_code=422, detail=str(e), ) from e - except WorkflowParameterMissingRequiredValue as e: + except WorkflowDefinitionValidationException as e: raise e except (SkyvernHTTPException, ValidationError) as e: # Bubble up well-formed client errors so they are not converted to 500s @@ -916,7 +916,7 @@ async def update_workflow( ) except yaml.YAMLError: raise HTTPException(status_code=422, detail="Invalid YAML") - except WorkflowParameterMissingRequiredValue as e: + except WorkflowDefinitionValidationException as e: raise e except (SkyvernHTTPException, ValidationError) as e: # Bubble up well-formed client errors so they are not converted to 500s diff --git a/skyvern/forge/sdk/workflow/exceptions.py b/skyvern/forge/sdk/workflow/exceptions.py index 2593b4a4..f3d2728b 100644 --- a/skyvern/forge/sdk/workflow/exceptions.py +++ b/skyvern/forge/sdk/workflow/exceptions.py @@ -119,7 +119,11 @@ class InvalidFileType(BaseWorkflowHTTPException): ) -class WorkflowParameterMissingRequiredValue(BaseWorkflowHTTPException): +class WorkflowDefinitionValidationException(BaseWorkflowHTTPException): + """Base exception for workflow definition validation errors.""" + + +class WorkflowParameterMissingRequiredValue(WorkflowDefinitionValidationException): def __init__(self, workflow_parameter_type: str, workflow_parameter_key: str, required_value: str) -> None: super().__init__( f"Missing required value for workflow parameter. Workflow parameter type: {workflow_parameter_type}. workflow_parameter_key: {workflow_parameter_key}. Required value: {required_value}", @@ -127,6 +131,25 @@ class WorkflowParameterMissingRequiredValue(BaseWorkflowHTTPException): ) +class WorkflowDefinitionHasUndefinedParameters(WorkflowDefinitionValidationException): + def __init__(self, undefined_parameters: dict[str, list[str]]) -> None: + # Format: {"block_label": ["param1", "param2"]} + error_details = [] + for block_label, params in undefined_parameters.items(): + params_str = ", ".join(f"'{p}'" for p in params) + error_details.append(f" - Block '{block_label}' references undefined parameter(s): {params_str}") + + error_message = ( + f"Workflow definition has blocks that reference undefined parameters:\n" + f"{chr(10).join(error_details)}\n\n" + f"Make sure to define all parameters in the workflow parameters list before using them in blocks." + ) + super().__init__( + error_message, + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + ) + + class InvalidWaitBlockTime(SkyvernException): def __init__(self, max_sec: int) -> None: super().__init__(f"Invalid wait time for wait block, it should be a number between 0 and {max_sec}.") diff --git a/skyvern/forge/sdk/workflow/workflow_definition_converter.py b/skyvern/forge/sdk/workflow/workflow_definition_converter.py index 6a387f8c..c9728dfb 100644 --- a/skyvern/forge/sdk/workflow/workflow_definition_converter.py +++ b/skyvern/forge/sdk/workflow/workflow_definition_converter.py @@ -23,6 +23,7 @@ from skyvern.forge.sdk.workflow.exceptions import ( InvalidWorkflowDefinition, WorkflowDefinitionHasDuplicateParameterKeys, WorkflowDefinitionHasReservedParameterKeys, + WorkflowDefinitionHasUndefinedParameters, WorkflowParameterMissingRequiredValue, ) from skyvern.forge.sdk.workflow.models.block import ( @@ -288,6 +289,11 @@ def convert_workflow_definition( if duplicate_parameter_keys: raise WorkflowDefinitionHasDuplicateParameterKeys(duplicate_keys=duplicate_parameter_keys) + # Validate that all blocks reference defined parameters + undefined_parameters = _collect_undefined_parameters(workflow_definition_yaml.blocks, parameters) + if undefined_parameters: + raise WorkflowDefinitionHasUndefinedParameters(undefined_parameters=undefined_parameters) + # Create blocks from the request block_label_mapping = {} blocks: list[BlockTypeVar] = [] @@ -362,11 +368,7 @@ def block_yaml_to_block( output_parameter = cast(OutputParameter, parameters[f"{block_yaml.label}_output"]) base_kwargs = _build_block_kwargs(block_yaml, output_parameter) if block_yaml.block_type == BlockType.TASK: - task_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + task_block_parameters = _resolve_block_parameters(block_yaml, parameters) return TaskBlock( **base_kwargs, url=block_yaml.url, @@ -452,22 +454,14 @@ def block_yaml_to_block( return CodeBlock( **base_kwargs, code=block_yaml.code, - parameters=( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ), + parameters=_resolve_block_parameters(block_yaml, parameters), ) elif block_yaml.block_type == BlockType.TEXT_PROMPT: return TextPromptBlock( **base_kwargs, llm_key=block_yaml.llm_key, prompt=block_yaml.prompt, - parameters=( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ), + parameters=_resolve_block_parameters(block_yaml, parameters), json_schema=block_yaml.json_schema, ) elif block_yaml.block_type == BlockType.DOWNLOAD_TO_S3: @@ -520,11 +514,7 @@ def block_yaml_to_block( json_schema=block_yaml.json_schema, ) elif block_yaml.block_type == BlockType.VALIDATION: - validation_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + validation_block_parameters = _resolve_block_parameters(block_yaml, parameters) if not block_yaml.complete_criterion and not block_yaml.terminate_criterion: raise InvalidWorkflowDefinition( @@ -543,11 +533,7 @@ def block_yaml_to_block( ) elif block_yaml.block_type == BlockType.ACTION: - action_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + action_block_parameters = _resolve_block_parameters(block_yaml, parameters) if not block_yaml.navigation_goal: raise InvalidWorkflowDefinition(f"Action block '{block_yaml.label}' requires navigation_goal") @@ -573,11 +559,7 @@ def block_yaml_to_block( ) elif block_yaml.block_type == BlockType.NAVIGATION: - navigation_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + navigation_block_parameters = _resolve_block_parameters(block_yaml, parameters) return NavigationBlock( **base_kwargs, url=block_yaml.url, @@ -614,11 +596,7 @@ def block_yaml_to_block( ) elif block_yaml.block_type == BlockType.EXTRACTION: - extraction_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + extraction_block_parameters = _resolve_block_parameters(block_yaml, parameters) return ExtractionBlock( **base_kwargs, url=block_yaml.url, @@ -634,11 +612,7 @@ def block_yaml_to_block( ) elif block_yaml.block_type == BlockType.LOGIN: - login_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + login_block_parameters = _resolve_block_parameters(block_yaml, parameters) return LoginBlock( **base_kwargs, url=block_yaml.url, @@ -667,11 +641,7 @@ def block_yaml_to_block( ) elif block_yaml.block_type == BlockType.FILE_DOWNLOAD: - file_download_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + file_download_block_parameters = _resolve_block_parameters(block_yaml, parameters) return FileDownloadBlock( **base_kwargs, url=block_yaml.url, @@ -702,11 +672,7 @@ def block_yaml_to_block( max_steps=block_yaml.max_steps, ) elif block_yaml.block_type == BlockType.HTTP_REQUEST: - http_request_block_parameters = ( - [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] - if block_yaml.parameter_keys - else [] - ) + http_request_block_parameters = _resolve_block_parameters(block_yaml, parameters) return HttpRequestBlock( **base_kwargs, method=block_yaml.method, @@ -739,6 +705,40 @@ def block_yaml_to_block( raise ValueError(f"Invalid block type {block_yaml.block_type}") +def _collect_undefined_parameters( + block_yamls: list[BLOCK_YAML_TYPES], + parameters: dict[str, PARAMETER_TYPE], +) -> dict[str, list[str]]: + """ + Collect all undefined parameters referenced by blocks (including nested blocks in for_loop). + Returns a dict mapping block labels to lists of undefined parameter keys. + """ + undefined_params: dict[str, list[str]] = {} + + for block_yaml in block_yamls: + # Check parameters for this block + if block_yaml.parameter_keys: + undefined_for_block = [param_key for param_key in block_yaml.parameter_keys if param_key not in parameters] + if undefined_for_block: + undefined_params[block_yaml.label] = undefined_for_block + + # Recursively check nested blocks in for_loop + if isinstance(block_yaml, ForLoopBlockYAML) and block_yaml.loop_blocks: + nested_undefined = _collect_undefined_parameters(block_yaml.loop_blocks, parameters) + undefined_params.update(nested_undefined) + + return undefined_params + + +def _resolve_block_parameters( + block_yaml: BLOCK_YAML_TYPES, + parameters: dict[str, PARAMETER_TYPE], +) -> list[PARAMETER_TYPE]: + return ( + [parameters[parameter_key] for parameter_key in block_yaml.parameter_keys] if block_yaml.parameter_keys else [] + ) + + def _has_dag_metadata(block_yamls: list[BLOCK_YAML_TYPES]) -> bool: for block_yaml in block_yamls: if block_yaml.next_block_label: