From 0ef416b70c8ef49baf76aad970020e544fed15d1 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Wed, 6 May 2026 09:12:09 -0400 Subject: [PATCH 1/2] fix(schema): forbid extra fields on AgentDef/ParallelGroup/ForEachDef/WorkflowConfig Closes #140 A user nesting parallel: or for_each: inside an agents: item used to have the misnested field silently dropped, leaving a no-prompt/no-model wrapper agent that failed obscurely at the provider with "Model 'gpt-4o' is not available". Add model_config = ConfigDict(extra="forbid") to AgentDef, ParallelGroup, ForEachDef, and WorkflowConfig so misnesting and field typos are rejected at parse time with a Pydantic extra_forbidden error pointing at the offending location (e.g. agents.0.parallel). Also: - Show Parallel Groups and For-each Groups counts in the conductor validate summary so the absence of an expected group is visible (issue #140 suggestion c). - Fix examples/for-each-simple.yaml: its top-level input: block was silently being dropped (it should be nested under workflow:). With extra=forbid this now hard-fails, so move it under workflow: where every other example has it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- examples/for-each-simple.yaml | 12 ++--- src/conductor/cli/validate.py | 6 +++ src/conductor/config/schema.py | 10 +++- tests/test_config/test_schema.py | 88 ++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) diff --git a/examples/for-each-simple.yaml b/examples/for-each-simple.yaml index da93ce7..1dd9c2f 100644 --- a/examples/for-each-simple.yaml +++ b/examples/for-each-simple.yaml @@ -29,12 +29,12 @@ workflow: limits: max_iterations: 20 -# Optional input: if provided, items are used directly; otherwise generated -input: - items: - type: string - required: false - description: JSON array of items to process (e.g. '["apple", "banana"]') + # Optional input: if provided, items are used directly; otherwise generated + input: + items: + type: string + required: false + description: JSON array of items to process (e.g. '["apple", "banana"]') # For-each group definition for_each: diff --git a/src/conductor/cli/validate.py b/src/conductor/cli/validate.py index 64a6a49..418e84f 100644 --- a/src/conductor/cli/validate.py +++ b/src/conductor/cli/validate.py @@ -120,6 +120,8 @@ def display_validation_success( # Build summary info agent_count = len(config.agents) human_gate_count = sum(1 for a in config.agents if a.type == "human_gate") + parallel_group_count = len(config.parallel) + for_each_group_count = len(config.for_each) # Count conditional routes conditional_route_count = sum(1 for a in config.agents for r in a.routes if r.when) @@ -163,6 +165,10 @@ def display_validation_success( table.add_row("Agents", str(agent_count)) if human_gate_count > 0: table.add_row("Human Gates", str(human_gate_count)) + if parallel_group_count > 0: + table.add_row("Parallel Groups", str(parallel_group_count)) + if for_each_group_count > 0: + table.add_row("For-each Groups", str(for_each_group_count)) table.add_row("Max Iterations", str(config.workflow.limits.max_iterations)) timeout_val = config.workflow.limits.timeout_seconds table.add_row("Timeout", f"{timeout_val}s" if timeout_val else "unlimited") diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index def5bf5..0e0f41f 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -8,7 +8,7 @@ from typing import Any, Literal -from pydantic import BaseModel, Field, field_validator, model_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator from conductor.providers.reasoning import ReasoningEffort @@ -109,6 +109,8 @@ def validate_target(cls, v: str) -> str: class ParallelGroup(BaseModel): """Definition for a parallel agent execution group.""" + model_config = ConfigDict(extra="forbid") + name: str """Unique identifier for this parallel group.""" @@ -160,6 +162,8 @@ class ForEachDef(BaseModel): ``` """ + model_config = ConfigDict(extra="forbid") + name: str """Unique identifier for this for-each group.""" @@ -444,6 +448,8 @@ class ReasoningConfig(BaseModel): class AgentDef(BaseModel): """Definition for a single agent in the workflow.""" + model_config = ConfigDict(extra="forbid") + name: str """Unique identifier for this agent.""" @@ -939,6 +945,8 @@ class WorkflowDef(BaseModel): class WorkflowConfig(BaseModel): """Complete workflow configuration file.""" + model_config = ConfigDict(extra="forbid") + workflow: WorkflowDef """Workflow-level settings.""" diff --git a/tests/test_config/test_schema.py b/tests/test_config/test_schema.py index 6c33119..f32678d 100644 --- a/tests/test_config/test_schema.py +++ b/tests/test_config/test_schema.py @@ -1396,3 +1396,91 @@ def test_rejects_invalid_effort(self, effort: object) -> None: """Test that invalid effort values raise ValidationError.""" with pytest.raises(ValidationError): RuntimeConfig(default_reasoning_effort=effort) # type: ignore[arg-type] + + +class TestExtraFieldsForbidden: + """Tests that workflow models reject unknown fields. + + Regression tests for https://github.com/microsoft/conductor/issues/140 — + misnesting `parallel:` or `for_each:` inside an `agents:` item used to + silently drop the field, leaving a wrapper agent with no model/prompt + that failed obscurely at the provider. + """ + + def test_agentdef_misnested_parallel_rejected(self) -> None: + """An `agents:` item with a nested `parallel:` field is rejected.""" + with pytest.raises(ValidationError) as exc_info: + AgentDef.model_validate( + { + "name": "review_group", + "parallel": ["technical_reviewer", "readability_reviewer"], + "failure_mode": "fail_fast", + "routes": [{"to": "$end"}], + } + ) + errors = exc_info.value.errors() + # The unknown field must be reported by Pydantic's extra_forbidden + assert any( + err["type"] == "extra_forbidden" and "parallel" in err["loc"] for err in errors + ), f"Expected extra_forbidden error for 'parallel', got: {errors}" + + def test_agentdef_misnested_for_each_rejected(self) -> None: + """An `agents:` item with a nested `for_each:` field is rejected.""" + with pytest.raises(ValidationError) as exc_info: + AgentDef.model_validate( + { + "name": "fanout", + "for_each": [{"name": "x", "type": "for_each"}], + } + ) + errors = exc_info.value.errors() + assert any( + err["type"] == "extra_forbidden" and "for_each" in err["loc"] for err in errors + ), f"Expected extra_forbidden error for 'for_each', got: {errors}" + + def test_agentdef_typo_field_rejected(self) -> None: + """A typo'd field on an agent is rejected (e.g., `prmpt` instead of `prompt`).""" + with pytest.raises(ValidationError) as exc_info: + AgentDef.model_validate( + { + "name": "answerer", + "model": "claude-haiku-4.5", + "prmpt": "Answer the question.", + } + ) + errors = exc_info.value.errors() + assert any(err["type"] == "extra_forbidden" and "prmpt" in err["loc"] for err in errors), ( + f"Expected extra_forbidden error for 'prmpt', got: {errors}" + ) + + def test_parallel_group_extra_field_rejected(self) -> None: + """An unknown field on a ParallelGroup is rejected.""" + with pytest.raises(ValidationError) as exc_info: + from conductor.config.schema import ParallelGroup + + ParallelGroup.model_validate( + { + "name": "g", + "agents": ["a", "b"], + "fail_fast": True, # typo: actually `failure_mode` + } + ) + errors = exc_info.value.errors() + assert any(err["type"] == "extra_forbidden" for err in errors) + + def test_workflow_config_top_level_extra_field_rejected(self) -> None: + """An unknown top-level workflow field is rejected (catches `agent:` typo etc.).""" + from conductor.config.schema import ParallelGroup # noqa: F401 + + with pytest.raises(ValidationError) as exc_info: + WorkflowConfig.model_validate( + { + "workflow": {"name": "x", "version": "1", "entry_point": "a"}, + "agents": [{"name": "a", "model": "m", "prompt": "p"}], + "agnts": [], # typo + } + ) + errors = exc_info.value.errors() + assert any(err["type"] == "extra_forbidden" and "agnts" in err["loc"] for err in errors), ( + f"Expected extra_forbidden error for 'agnts', got: {errors}" + ) From 768f8f5e1770eb4702452d433ba671fd9f705475 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Wed, 6 May 2026 12:15:56 -0400 Subject: [PATCH 2/2] fix(schema): also forbid extra fields on RouteDef and WorkflowDef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the extra="forbid" coverage from PR review feedback. These models are equally user-authored in YAML and exhibit the same silent-typo footgun that #140 reports. - RouteDef: a typo like `whn:` instead of `when:` was silently dropped, leaving `route.when = None` and turning a conditional route into an unconditional one — a workflow-semantics bug that is nearly impossible to debug. - WorkflowDef: typos like `entery_point:` or `limts:` in the `workflow:` block were silently ignored, falling back to defaults instead of the user's intent. Adds two regression tests covering both cases. `make validate-examples` still passes — no in-repo example was relying on these silent drops. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/conductor/config/schema.py | 4 ++++ tests/test_config/test_schema.py | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index 0e0f41f..eaa6b8f 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -88,6 +88,8 @@ def validate_type_specific_fields(self) -> OutputField: class RouteDef(BaseModel): """Definition for a routing rule.""" + model_config = ConfigDict(extra="forbid") + to: str """Target agent name, '$end', or human gate name.""" @@ -882,6 +884,8 @@ class RuntimeConfig(BaseModel): class WorkflowDef(BaseModel): """Top-level workflow configuration.""" + model_config = ConfigDict(extra="forbid") + name: str """Unique workflow identifier.""" diff --git a/tests/test_config/test_schema.py b/tests/test_config/test_schema.py index f32678d..bba4994 100644 --- a/tests/test_config/test_schema.py +++ b/tests/test_config/test_schema.py @@ -1484,3 +1484,41 @@ def test_workflow_config_top_level_extra_field_rejected(self) -> None: assert any(err["type"] == "extra_forbidden" and "agnts" in err["loc"] for err in errors), ( f"Expected extra_forbidden error for 'agnts', got: {errors}" ) + + def test_workflowdef_typo_field_rejected(self) -> None: + """A typo'd field on the `workflow:` block is rejected. + + Without `extra="forbid"` on WorkflowDef, typos like `entery_point:` or + `limts:` are silently dropped, leaving the user's intent ignored. + """ + with pytest.raises(ValidationError) as exc_info: + WorkflowDef.model_validate( + { + "name": "demo", + "entry_point": "a", + "entery_point": "b", # typo + } + ) + errors = exc_info.value.errors() + assert any( + err["type"] == "extra_forbidden" and "entery_point" in err["loc"] for err in errors + ), f"Expected extra_forbidden error for 'entery_point', got: {errors}" + + def test_routedef_typo_when_rejected(self) -> None: + """A typo'd `when:` on a route is rejected. + + Without `extra="forbid"` on RouteDef, `whn:` was silently dropped and + `route.when` defaulted to `None`, turning a conditional route into an + unconditional one — a workflow-semantics bug nearly impossible to debug. + """ + with pytest.raises(ValidationError) as exc_info: + RouteDef.model_validate( + { + "to": "next_agent", + "whn": "output.score > 5", # typo for `when` + } + ) + errors = exc_info.value.errors() + assert any(err["type"] == "extra_forbidden" and "whn" in err["loc"] for err in errors), ( + f"Expected extra_forbidden error for 'whn', got: {errors}" + )