diff --git a/CHANGELOG.md b/CHANGELOG.md index c282f202..3c98f0fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/microsoft/conductor/compare/v0.1.18...HEAD) +### Fixed + +- `human_gate` agents: the dict returned by `prompt_for` text-collection fields + is no longer spread into the gate's output root, where it could silently + overwrite the reserved `selected` key (e.g. an option declaring + `prompt_for: selected` would clobber the chosen option value with whatever + the user typed). Collected values are now nested under an explicit + `additional_input` key, matching the shape the `gate_resolved` event already + used. ([#237](https://github.com/microsoft/conductor/pull/237)) + +### Changed + +- **BREAKING (templates)** — `human_gate` output shape changed. + - Before: `{{ .output. }}` (root-level). + - After: `{{ .output.additional_input. }}` (nested). + - Gates without any `prompt_for` now produce `additional_input: {}` rather + than just `{"selected": ...}` — the key is always present. + - `.output.selected` is unchanged. + - Templates that referenced the old flat path now raise `TemplateError` + (`StrictUndefined`), so the migration fails loudly rather than rendering + to empty strings. + - In `context: explicit` mode, `input:` declarations support + `.output.additional_input` (the parent dict) but not the dotted + shorthand `.output.additional_input.`. Declare the parent + and read individual fields via Jinja2 in the consuming agent's prompt or + output template. + ## [0.1.18](https://github.com/microsoft/conductor/compare/v0.1.17...v0.1.18) - 2026-05-28 ### Added diff --git a/plugins/conductor/skills/conductor/references/authoring.md b/plugins/conductor/skills/conductor/references/authoring.md index 2c619598..517da2c6 100644 --- a/plugins/conductor/skills/conductor/references/authoring.md +++ b/plugins/conductor/skills/conductor/references/authoring.md @@ -696,8 +696,18 @@ agents: ### Gate Output Human gates automatically capture: -- `output.selected` - the `value` of the chosen option -- `output.feedback` - text input from `prompt_for` (if specified) +- `output.selected` — the `value` of the chosen option. +- `output.additional_input` — dict of values collected from `prompt_for` fields. + Always present; `{}` when no `prompt_for` was specified or the selected option + has no `prompt_for`. Access individual fields via templates as + `{{ .output.additional_input. }}` (for example + `{{ approval_gate.output.additional_input.feedback }}` when an option declares + `prompt_for: feedback`). + +> **`context: explicit` mode note.** `input:` declarations support +> `.output.additional_input` (the whole dict) but not the dotted shorthand +> `.output.additional_input.`. Declare the parent key and read +> individual fields via Jinja2 in the agent's prompt or output template. ## Context Modes diff --git a/plugins/conductor/skills/conductor/references/yaml-schema.md b/plugins/conductor/skills/conductor/references/yaml-schema.md index f107d508..5a46dda5 100644 --- a/plugins/conductor/skills/conductor/references/yaml-schema.md +++ b/plugins/conductor/skills/conductor/references/yaml-schema.md @@ -390,11 +390,19 @@ agents: route: string # Agent to route to when selected prompt_for: string # Optional: field name to collect text input from user - output: # Captured automatically - selected: # The selected option value - type: string - feedback: # Text from prompt_for (if used) + output: # Captured automatically (do not declare in YAML) + selected: # The chosen option's `value` type: string + additional_input: # Dict of values collected from `prompt_for` fields. + type: dict # Always present; `{}` when no `prompt_for` is set + # or when the selected option has no `prompt_for`. + # Access fields via templates as: + # {{ .output.additional_input. }} + # In `context: explicit` mode, `input:` declarations + # support `.output.additional_input` (the whole + # dict) but not the dotted shorthand + # `.output.additional_input.` — declare + # the parent and traverse in Jinja2. ``` ## Parallel Group Schema diff --git a/src/conductor/engine/workflow.py b/src/conductor/engine/workflow.py index f0f798c3..b78487d2 100644 --- a/src/conductor/engine/workflow.py +++ b/src/conductor/engine/workflow.py @@ -2526,7 +2526,7 @@ async def _execute_loop(self, current_agent_name: str) -> dict[str, Any]: agent.name, { "selected": gate_result.selected_option.value, - **gate_result.additional_input, + "additional_input": gate_result.additional_input, }, ) diff --git a/tests/test_engine/test_workflow.py b/tests/test_engine/test_workflow.py index 6f8b51ea..62ef35f8 100644 --- a/tests/test_engine/test_workflow.py +++ b/tests/test_engine/test_workflow.py @@ -1334,6 +1334,271 @@ def mock_handler(agent, prompt, context): assert "approval_gate" in summary["agents_executed"] assert summary["iterations"] == 3 + @pytest.mark.asyncio + async def test_human_gate_stores_additional_input_nested_in_context(self) -> None: + """Test that prompt_for values are stored nested under additional_input.""" + from unittest.mock import patch + + config = WorkflowConfig( + workflow=WorkflowDef(name="gate-prompt-for", entry_point="ask_human"), + agents=[ + AgentDef( + name="ask_human", + type="human_gate", + prompt="Provide input:", + options=[ + GateOption( + label="Provide answer", + value="provide_answer", + route="next", + prompt_for="answer", + ), + ], + ), + AgentDef( + name="next", + model="gpt-4", + prompt="Next", + output={"received": OutputField(type="string")}, + routes=[RouteDef(to="$end")], + ), + ], + output={"received": "{{ next.output.received }}"}, + ) + + captured_context: dict = {} + + def mock_handler(agent, prompt, context): + captured_context.update(context) + return {"received": "ok"} + + provider = CopilotProvider(mock_handler=mock_handler) + engine = WorkflowEngine(config, provider, skip_gates=False) + + with patch("conductor.gates.human.Prompt.ask", side_effect=["1", "README.md"]): + await engine.run({}) + + gate_output = captured_context.get("ask_human", {}).get("output", {}) + assert gate_output["selected"] == "provide_answer" + assert gate_output["additional_input"] == {"answer": "README.md"} + + @pytest.mark.asyncio + async def test_human_gate_no_prompt_for_has_empty_additional_input( + self, + human_gate_workflow_config: WorkflowConfig, + ) -> None: + """Test that gates without prompt_for produce additional_input: {} in context.""" + captured_context: dict = {} + + def mock_handler(agent, prompt, context): + captured_context.update(context) + if agent.name == "drafter": + return {"draft": "Draft content"} + return {"published": True} + + provider = CopilotProvider(mock_handler=mock_handler) + engine = WorkflowEngine(human_gate_workflow_config, provider, skip_gates=True) + + await engine.run({}) + + gate_output = captured_context.get("approval_gate", {}).get("output", {}) + assert gate_output["selected"] == "approved" + assert gate_output["additional_input"] == {} + + @pytest.mark.asyncio + async def test_human_gate_prompt_for_named_selected_does_not_corrupt_selected( + self, + ) -> None: + """Test that prompt_for field named 'selected' cannot overwrite the selected value. + + The flat spread form had a silent data-corruption bug: if prompt_for used + the field name 'selected', the user's typed text would overwrite the chosen + option value. The nested form prevents this. + """ + from unittest.mock import patch + + config = WorkflowConfig( + workflow=WorkflowDef(name="gate-collision", entry_point="gate"), + agents=[ + AgentDef( + name="gate", + type="human_gate", + prompt="Go:", + options=[ + GateOption( + label="Go", + value="go", + route="next", + prompt_for="selected", # collides with reserved key + ), + ], + ), + AgentDef( + name="next", + model="gpt-4", + prompt="Next", + output={"done": OutputField(type="boolean")}, + routes=[RouteDef(to="$end")], + ), + ], + output={"done": "{{ next.output.done }}"}, + ) + + captured_context: dict = {} + + def mock_handler(agent, prompt, context): + captured_context.update(context) + return {"done": True} + + provider = CopilotProvider(mock_handler=mock_handler) + engine = WorkflowEngine(config, provider, skip_gates=False) + + with patch("conductor.gates.human.Prompt.ask", side_effect=["1", "user typed text"]): + await engine.run({}) + + gate_output = captured_context.get("gate", {}).get("output", {}) + # The chosen option value must not be overwritten by the user's typed text + assert gate_output["selected"] == "go" + # The user's typed text is safely nested + assert gate_output["additional_input"]["selected"] == "user typed text" + + @pytest.mark.asyncio + async def test_human_gate_web_response_nests_additional_input(self) -> None: + """Web-gate responses must land under the same nested ``additional_input`` key. + + The store-site at engine/workflow.py is shared between the CLI gate + handler and ``_wait_for_web_gate``. This test stubs the web dashboard so + the web task wins the race and asserts the web-supplied + ``additional_input`` is stored under ``output.additional_input``, not + spread flat into ``output``. + """ + from unittest.mock import AsyncMock, MagicMock + + config = WorkflowConfig( + workflow=WorkflowDef(name="gate-web", entry_point="approval_gate"), + agents=[ + AgentDef( + name="approval_gate", + type="human_gate", + prompt="Approve?", + options=[ + GateOption( + label="Approve", + value="approved", + route="next", + prompt_for="comment", + ), + ], + ), + AgentDef( + name="next", + model="gpt-4", + prompt="next", + output={"received": OutputField(type="string")}, + routes=[RouteDef(to="$end")], + ), + ], + output={"received": "{{ next.output.received }}"}, + ) + + # Stub the web dashboard so the web task returns immediately with a + # realistic gate-response payload from a browser client. + mock_dashboard = MagicMock() + mock_dashboard.wait_for_gate_response = AsyncMock( + return_value={ + "selected_value": "approved", + "additional_input": {"comment": "looks good"}, + } + ) + + captured_context: dict[str, object] = {} + + def mock_handler(agent, prompt, context): + captured_context.update(context) + return {"received": "ok"} + + provider = CopilotProvider(mock_handler=mock_handler) + engine = WorkflowEngine( + config, + provider, + skip_gates=False, + web_dashboard=mock_dashboard, + ) + + # Make the CLI side of the race never return so the web task wins + # deterministically. The engine's _handle_gate_with_web cancels the + # losing task and suppresses CancelledError, so this is safe. + async def _never_returns(*_args, **_kwargs): + await asyncio.Event().wait() + + from unittest.mock import patch + + with patch.object(engine.gate_handler, "handle_gate", side_effect=_never_returns): + await engine.run({}) + + gate_output = captured_context.get("approval_gate", {}) # type: ignore[assignment] + assert isinstance(gate_output, dict) + gate_output = gate_output.get("output", {}) + assert gate_output["selected"] == "approved" + assert gate_output["additional_input"] == {"comment": "looks good"} + # Belt-and-braces: web-supplied keys must not appear flat at the root. + assert "comment" not in gate_output + + @pytest.mark.asyncio + async def test_human_gate_additional_input_readable_via_template(self) -> None: + """A downstream agent's prompt template must be able to read the nested value. + + This exercises the actual user-facing contract — templates resolving + ``{{ .output.additional_input. }}`` — rather than only the + internal context shape. Locks in the rendered-prompt path so any + regression in template rendering or dict-attr resolution would fail + loudly here. + """ + from unittest.mock import patch + + config = WorkflowConfig( + workflow=WorkflowDef(name="gate-template-readthrough", entry_point="ask_human"), + agents=[ + AgentDef( + name="ask_human", + type="human_gate", + prompt="Provide input:", + options=[ + GateOption( + label="Provide answer", + value="provide_answer", + route="echo", + prompt_for="answer", + ), + ], + ), + AgentDef( + name="echo", + model="gpt-4", + prompt="User said: {{ ask_human.output.additional_input.answer }}", + output={"echoed": OutputField(type="string")}, + routes=[RouteDef(to="$end")], + ), + ], + output={"echoed": "{{ echo.output.echoed }}"}, + ) + + rendered_prompts: list[str] = [] + + def mock_handler(agent, prompt, context): + if agent.name == "echo": + rendered_prompts.append(prompt) + return {"echoed": "ok"} + + provider = CopilotProvider(mock_handler=mock_handler) + engine = WorkflowEngine(config, provider, skip_gates=False) + + with patch("conductor.gates.human.Prompt.ask", side_effect=["1", "README.md"]): + await engine.run({}) + + assert rendered_prompts, "echo agent's mock_handler was never invoked" + assert "User said: README.md" in rendered_prompts[0] + class TestWorkflowEngineLifecycleHooks: """Tests for lifecycle hooks execution."""