From 0830431971ce726e09a3a419c00c71a7c416144c Mon Sep 17 00:00:00 2001 From: mika <211269698+mikamikasuki@users.noreply.github.com> Date: Fri, 2 Oct 2026 16:07:51 -0700 Subject: [PATCH 1/2] fix(workflow): preserve unresolved positional placeholders --- workflows/tests/test_workflow_execution.py | 38 +++++++++++++++++++++- workflows/workflow_use/workflow/service.py | 4 +-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/workflows/tests/test_workflow_execution.py b/workflows/tests/test_workflow_execution.py index 472c6456..8bab299a 100644 --- a/workflows/tests/test_workflow_execution.py +++ b/workflows/tests/test_workflow_execution.py @@ -4,7 +4,10 @@ Tests the fixes for go_back/go_forward (empty action models) and deterministic execution. """ -from workflow_use.schema.views import NavigationStep +import pytest + +from workflow_use.schema.views import InputStep, NavigationStep +from workflow_use.workflow.service import Workflow class TestWorkflowExecution: @@ -180,6 +183,39 @@ def test_actions_requiring_wait(self): assert 'extract' not in actions_requiring_wait +@pytest.mark.parametrize('value', ['Query param: {0}', 'literal {}']) +def test_resolve_placeholders_preserves_positional_input(value): + """Positional placeholders in input steps remain literal without positional context.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {'name': 'Ada'} + step = InputStep(type='input', target_text='Query', value=value) + + resolved = workflow._resolve_placeholders(step) + + assert resolved.value == value + + +def test_resolve_placeholders_keeps_named_context_behavior(): + """Known named placeholders resolve and unknown names remain literal.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {'name': 'Ada'} + data = {'known': 'Hello {name}', 'unknown': 'Hello {missing}'} + + assert workflow._resolve_placeholders(data) == {'known': 'Hello Ada', 'unknown': 'Hello {missing}'} + + +def test_resolve_placeholders_positional_input_uses_default(): + """Unresolved positional input still follows the existing default-value path.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {} + step = InputStep(type='input', target_text='Query', value='{0}', default_value='fallback') + + resolved = workflow._resolve_placeholders(step) + + assert resolved.value == 'fallback' + assert step.value == '{0}' + + # Helper to run async tests import asyncio diff --git a/workflows/workflow_use/workflow/service.py b/workflows/workflow_use/workflow/service.py index 1a6c325b..365fcb99 100644 --- a/workflows/workflow_use/workflow/service.py +++ b/workflows/workflow_use/workflow/service.py @@ -517,8 +517,8 @@ def _resolve_placeholders(self, data: Any) -> Any: formatted_data = data.format(**self.context) return formatted_data return data # No placeholders, return as is - except KeyError: - # A key in the placeholder was not found in the context. + except (KeyError, IndexError): + # A placeholder could not be resolved from the context. # Return the original string as per previous behavior. return data From 83d8711ac88ea5ca2ab699f6250f406272224d1b Mon Sep 17 00:00:00 2001 From: mika <211269698+mikamikasuki@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:34:16 -0700 Subject: [PATCH 2/2] fix(workflow): limit positional fallback and cover script harness --- workflows/tests/test_workflow_execution.py | 66 ++++++++++++++-------- workflows/workflow_use/workflow/service.py | 15 ++++- 2 files changed, 56 insertions(+), 25 deletions(-) diff --git a/workflows/tests/test_workflow_execution.py b/workflows/tests/test_workflow_execution.py index 8bab299a..efd3d07c 100644 --- a/workflows/tests/test_workflow_execution.py +++ b/workflows/tests/test_workflow_execution.py @@ -182,38 +182,60 @@ def test_actions_requiring_wait(self): assert 'input' not in actions_requiring_wait assert 'extract' not in actions_requiring_wait + def test_resolve_placeholders_preserves_positional_input(self): + """Positional placeholders in input steps remain literal without positional context.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {'name': 'Ada'} + value = 'Query param: {0}' + step = InputStep(type='input', target_text='Query', value=value) -@pytest.mark.parametrize('value', ['Query param: {0}', 'literal {}']) -def test_resolve_placeholders_preserves_positional_input(value): - """Positional placeholders in input steps remain literal without positional context.""" - workflow = Workflow.__new__(Workflow) - workflow.context = {'name': 'Ada'} - step = InputStep(type='input', target_text='Query', value=value) + resolved = workflow._resolve_placeholders(step) - resolved = workflow._resolve_placeholders(step) + assert resolved.value == value - assert resolved.value == value + def test_resolve_placeholders_keeps_named_context_behavior(self): + """Known named placeholders resolve and unknown names remain literal.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {'name': 'Ada', 'items': ['first']} + data = {'known': 'Hello {name}', 'unknown': 'Hello {missing}', 'nested': '{items[0]}'} + assert workflow._resolve_placeholders(data) == {'known': 'Hello Ada', 'unknown': 'Hello {missing}', 'nested': 'first'} -def test_resolve_placeholders_keeps_named_context_behavior(): - """Known named placeholders resolve and unknown names remain literal.""" - workflow = Workflow.__new__(Workflow) - workflow.context = {'name': 'Ada'} - data = {'known': 'Hello {name}', 'unknown': 'Hello {missing}'} + def test_resolve_placeholders_positional_input_uses_default(self): + """Unresolved positional input still follows the existing default-value path.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {} + step = InputStep(type='input', target_text='Query', value='{0}', default_value='fallback') - assert workflow._resolve_placeholders(data) == {'known': 'Hello Ada', 'unknown': 'Hello {missing}'} + resolved = workflow._resolve_placeholders(step) + assert resolved.value == 'fallback' + assert step.value == '{0}' -def test_resolve_placeholders_positional_input_uses_default(): - """Unresolved positional input still follows the existing default-value path.""" - workflow = Workflow.__new__(Workflow) - workflow.context = {} - step = InputStep(type='input', target_text='Query', value='{0}', default_value='fallback') + def test_resolve_placeholders_preserves_automatic_input(self): + """Automatic positional placeholders remain literal without positional context.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {} + step = InputStep(type='input', target_text='Query', value='literal {}') - resolved = workflow._resolve_placeholders(step) + assert workflow._resolve_placeholders(step).value == 'literal {}' - assert resolved.value == 'fallback' - assert step.value == '{0}' + def test_resolve_placeholders_named_index_error_propagates(self): + """An invalid index on a known context value is not a missing positional field.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {'items': []} + + with pytest.raises(IndexError): + workflow._resolve_placeholders('{items[0]}') + + def test_resolve_placeholders_named_index_error_does_not_use_default(self): + """Input defaults must not hide indexing errors on a known context value.""" + workflow = Workflow.__new__(Workflow) + workflow.context = {'items': []} + step = InputStep(type='input', target_text='Query', value='{items[0]}', default_value='fallback') + + with pytest.raises(IndexError): + workflow._resolve_placeholders(step) # Helper to run async tests diff --git a/workflows/workflow_use/workflow/service.py b/workflows/workflow_use/workflow/service.py index 365fcb99..d9539e97 100644 --- a/workflows/workflow_use/workflow/service.py +++ b/workflows/workflow_use/workflow/service.py @@ -4,6 +4,7 @@ import json import logging from pathlib import Path +from string import Formatter from typing import Any, Dict, List, TypeVar from typing import cast as _cast @@ -517,10 +518,18 @@ def _resolve_placeholders(self, data: Any) -> Any: formatted_data = data.format(**self.context) return formatted_data return data # No placeholders, return as is - except (KeyError, IndexError): - # A placeholder could not be resolved from the context. - # Return the original string as per previous behavior. + except KeyError: + # A named placeholder could not be resolved from the context. return data + except IndexError: + # Only bare positional fields have no context to resolve. Preserve + # indexing/formatting errors from named context values. + if all( + field_name is None or field_name == '' or field_name.isdecimal() + for _, field_name, _, _ in Formatter().parse(data) + ): + return data + raise # TODO: This next things are not really supported atm, we'll need to to do it in the future. elif isinstance(data, list):