fix: reject scalar context foreach sources
This commit is contained in:
+8
-3
@@ -45,11 +45,16 @@ docstring at the code seam. Future agents usually encounter code first.
|
|||||||
Run Markdown lint on the exact files you changed:
|
Run Markdown lint on the exact files you changed:
|
||||||
|
|
||||||
```powershell
|
```powershell
|
||||||
pnpx markdownlint-cli2 'docs/AGENTS.md' 'docs/current_roadmap.md'
|
$changedMarkdown = @(
|
||||||
|
'docs/path/to/changed-file.md'
|
||||||
|
'skills/path/to/another-changed-file.md'
|
||||||
|
)
|
||||||
|
pnpx markdownlint-cli2 $changedMarkdown
|
||||||
```
|
```
|
||||||
|
|
||||||
Use narrow lint or fix targets. Broad autofixes can rewrite historical files or
|
Replace the example paths with every Markdown file changed in the current
|
||||||
unrelated user changes.
|
worktree. Use narrow lint or fix targets. Broad autofixes can rewrite
|
||||||
|
historical files or unrelated user changes.
|
||||||
|
|
||||||
Follow CommonMark list indentation. When a Markdown example contains fenced
|
Follow CommonMark list indentation. When a Markdown example contains fenced
|
||||||
code, wrap the outer example in a fence of four or more backticks.
|
code, wrap the outer example in a fence of four or more backticks.
|
||||||
|
|||||||
@@ -115,11 +115,7 @@ class SchemaView:
|
|||||||
"""Return the first declared homogeneous item schema for an array."""
|
"""Return the first declared homogeneous item schema for an array."""
|
||||||
for cursor in self._cursors:
|
for cursor in self._cursors:
|
||||||
for branch, _ in _structural_branches(cursor, frozenset()):
|
for branch, _ in _structural_branches(cursor, frozenset()):
|
||||||
schema_type = branch.contents.get("type")
|
if not _declares_array(branch.contents):
|
||||||
is_array = schema_type == "array" or (
|
|
||||||
isinstance(schema_type, list) and "array" in schema_type
|
|
||||||
)
|
|
||||||
if not is_array:
|
|
||||||
continue
|
continue
|
||||||
child = branch.child(branch.contents.get("items"))
|
child = branch.child(branch.contents.get("items"))
|
||||||
if child is not None:
|
if child is not None:
|
||||||
@@ -127,8 +123,16 @@ class SchemaView:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
def is_array(self) -> bool:
|
def is_array(self) -> bool:
|
||||||
"""Return whether any structural branch declares an array."""
|
"""Return whether any structural branch declares an array type.
|
||||||
return self.array_items() is not None
|
|
||||||
|
JSON Schema permits arrays without an ``items`` keyword; those are
|
||||||
|
still arrays whose element schema is unconstrained.
|
||||||
|
"""
|
||||||
|
return any(
|
||||||
|
_declares_array(branch.contents)
|
||||||
|
for cursor in self._cursors
|
||||||
|
for branch, _ in _structural_branches(cursor, frozenset())
|
||||||
|
)
|
||||||
|
|
||||||
def standalone_schema(self) -> dict[str, Any]:
|
def standalone_schema(self) -> dict[str, Any]:
|
||||||
"""Detach this view as a valid Draft 2020-12 schema resource.
|
"""Detach this view as a valid Draft 2020-12 schema resource.
|
||||||
@@ -208,6 +212,13 @@ class SchemaNavigator:
|
|||||||
return None, ""
|
return None, ""
|
||||||
|
|
||||||
|
|
||||||
|
def _declares_array(schema: JsonSchema) -> bool:
|
||||||
|
schema_type = schema.get("type")
|
||||||
|
return schema_type == "array" or (
|
||||||
|
isinstance(schema_type, list) and "array" in schema_type
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _structural_branches(
|
def _structural_branches(
|
||||||
cursor: _Cursor,
|
cursor: _Cursor,
|
||||||
active_references: frozenset[ReferenceIdentity],
|
active_references: frozenset[ReferenceIdentity],
|
||||||
|
|||||||
@@ -79,7 +79,7 @@ def validate_context_paths(
|
|||||||
# Context-rooted `over` paths reach this pass; the old
|
# Context-rooted `over` paths reach this pass; the old
|
||||||
# input/state-only check stays permissive for them.
|
# input/state-only check stays permissive for them.
|
||||||
if node.over.root == "context":
|
if node.over.root == "context":
|
||||||
_validate_one_context_path(
|
_validate_context_foreach_source(
|
||||||
node.over, f"nodes[{idx}].over", node.id, schema, report
|
node.over, f"nodes[{idx}].over", node.id, schema, report
|
||||||
)
|
)
|
||||||
elif isinstance(node, InterruptNode):
|
elif isinstance(node, InterruptNode):
|
||||||
@@ -197,6 +197,30 @@ def _validate_one_context_path(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_context_foreach_source(
|
||||||
|
path: GraphSourcePath,
|
||||||
|
location: str,
|
||||||
|
node_id: str,
|
||||||
|
schema: ContextSchema | None,
|
||||||
|
report: ValidationReport,
|
||||||
|
) -> None:
|
||||||
|
"""Require an existing context path to declare an array source.
|
||||||
|
|
||||||
|
Missing paths keep the more precise ``INVALID_CONTEXT_PATH`` diagnostic;
|
||||||
|
this adds the foreach-specific error only after the path resolves.
|
||||||
|
"""
|
||||||
|
_validate_one_context_path(path, location, node_id, schema, report)
|
||||||
|
if schema is None:
|
||||||
|
return
|
||||||
|
source = SchemaNavigator(schema).at_path(path.parts)
|
||||||
|
if source is not None and not source.is_array():
|
||||||
|
report.add(
|
||||||
|
ValidationIssueCode.INVALID_FOREACH_SOURCE,
|
||||||
|
location,
|
||||||
|
f"foreach source {str(path)!r} must resolve to an array",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _failing_segment(
|
def _failing_segment(
|
||||||
schema: Mapping[str, Any], parts: tuple[str, ...]
|
schema: Mapping[str, Any], parts: tuple[str, ...]
|
||||||
) -> tuple[str | None, str]:
|
) -> tuple[str | None, str]:
|
||||||
|
|||||||
@@ -82,6 +82,33 @@ def _run_with(*frames: ExecutionFrame) -> RunState:
|
|||||||
return run
|
return run
|
||||||
|
|
||||||
|
|
||||||
|
def _nested_foreach_workflow() -> Workflow:
|
||||||
|
return _workflow(
|
||||||
|
start="outer",
|
||||||
|
nodes=[
|
||||||
|
_foreach("outer", alias="outer_item"),
|
||||||
|
_foreach("inner", alias="inner_item", over="state.inner_items"),
|
||||||
|
_node("inner_body"),
|
||||||
|
_node("after_inner"),
|
||||||
|
],
|
||||||
|
edges=[
|
||||||
|
{"from": "outer", "outcome": "loop", "to": "inner"},
|
||||||
|
{"from": "inner", "outcome": "loop", "to": "inner_body"},
|
||||||
|
{"from": "inner", "outcome": "done", "to": "after_inner"},
|
||||||
|
{"from": "inner_body", "outcome": "ok", "to": "inner"},
|
||||||
|
{"from": "after_inner", "outcome": "ok", "to": "outer"},
|
||||||
|
{"from": "outer", "outcome": "done", "to": END},
|
||||||
|
],
|
||||||
|
state_schema={
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"items": {"type": "array", "items": {"type": "string"}},
|
||||||
|
"inner_items": {"type": "array", "items": {"type": "integer"}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_frame_context_view_uses_standard_and_foreach_contract_keys() -> None:
|
def test_frame_context_view_uses_standard_and_foreach_contract_keys() -> None:
|
||||||
root = ExecutionFrame(
|
root = ExecutionFrame(
|
||||||
id="root",
|
id="root",
|
||||||
@@ -280,30 +307,7 @@ def test_region_conflicted_node_receives_no_guaranteed_foreach_fields() -> None:
|
|||||||
|
|
||||||
|
|
||||||
def test_nested_foreach_replaces_inner_scope_and_restores_outer_scope() -> None:
|
def test_nested_foreach_replaces_inner_scope_and_restores_outer_scope() -> None:
|
||||||
workflow = _workflow(
|
workflow = _nested_foreach_workflow()
|
||||||
start="outer",
|
|
||||||
nodes=[
|
|
||||||
_foreach("outer", alias="outer_item"),
|
|
||||||
_foreach("inner", alias="inner_item", over="state.inner_items"),
|
|
||||||
_node("inner_body"),
|
|
||||||
_node("after_inner"),
|
|
||||||
],
|
|
||||||
edges=[
|
|
||||||
{"from": "outer", "outcome": "loop", "to": "inner"},
|
|
||||||
{"from": "inner", "outcome": "loop", "to": "inner_body"},
|
|
||||||
{"from": "inner", "outcome": "done", "to": "after_inner"},
|
|
||||||
{"from": "inner_body", "outcome": "ok", "to": "inner"},
|
|
||||||
{"from": "after_inner", "outcome": "ok", "to": "outer"},
|
|
||||||
{"from": "outer", "outcome": "done", "to": END},
|
|
||||||
],
|
|
||||||
state_schema={
|
|
||||||
"type": "object",
|
|
||||||
"properties": {
|
|
||||||
"items": {"type": "array", "items": {"type": "string"}},
|
|
||||||
"inner_items": {"type": "array", "items": {"type": "integer"}},
|
|
||||||
},
|
|
||||||
},
|
|
||||||
)
|
|
||||||
|
|
||||||
inner = _field_map(workflow, "inner_body")
|
inner = _field_map(workflow, "inner_body")
|
||||||
after_inner = _field_map(workflow, "after_inner")
|
after_inner = _field_map(workflow, "after_inner")
|
||||||
@@ -326,30 +330,7 @@ def test_nested_foreach_replaces_inner_scope_and_restores_outer_scope() -> None:
|
|||||||
|
|
||||||
|
|
||||||
def test_inner_completion_schema_restores_outer_structured_entry() -> None:
|
def test_inner_completion_schema_restores_outer_structured_entry() -> None:
|
||||||
workflow = _workflow(
|
workflow = _nested_foreach_workflow()
|
||||||
start="outer",
|
|
||||||
nodes=[
|
|
||||||
_foreach("outer", alias="outer_item"),
|
|
||||||
_foreach("inner", alias="inner_item", over="state.inner_items"),
|
|
||||||
_node("inner_body"),
|
|
||||||
_node("after_inner"),
|
|
||||||
],
|
|
||||||
edges=[
|
|
||||||
{"from": "outer", "outcome": "loop", "to": "inner"},
|
|
||||||
{"from": "inner", "outcome": "loop", "to": "inner_body"},
|
|
||||||
{"from": "inner", "outcome": "done", "to": "after_inner"},
|
|
||||||
{"from": "inner_body", "outcome": "ok", "to": "inner"},
|
|
||||||
{"from": "after_inner", "outcome": "ok", "to": "outer"},
|
|
||||||
{"from": "outer", "outcome": "done", "to": END},
|
|
||||||
],
|
|
||||||
state_schema={
|
|
||||||
"type": "object",
|
|
||||||
"properties": {
|
|
||||||
"items": {"type": "array", "items": {"type": "string"}},
|
|
||||||
"inner_items": {"type": "array", "items": {"type": "integer"}},
|
|
||||||
},
|
|
||||||
},
|
|
||||||
)
|
|
||||||
after_inner = _field_map(workflow, "after_inner")
|
after_inner = _field_map(workflow, "after_inner")
|
||||||
foreach_schema = after_inner["foreach"].schema
|
foreach_schema = after_inner["foreach"].schema
|
||||||
assert set(foreach_schema["properties"]) == {"outer"}
|
assert set(foreach_schema["properties"]) == {"outer"}
|
||||||
@@ -646,16 +627,17 @@ def test_ref_sibling_constraints_remain_conjunctive() -> None:
|
|||||||
"type": "array",
|
"type": "array",
|
||||||
"items": {
|
"items": {
|
||||||
"$ref": "#/$defs/ShortName",
|
"$ref": "#/$defs/ShortName",
|
||||||
"maxLength": 10,
|
"maxLength": 3,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
"$defs": {"ShortName": {"type": "string", "maxLength": 5}},
|
"$defs": {"ShortName": {"type": "string", "pattern": "^[A-Z]+$"}},
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
schema = context_schema_for_node(workflow, "body")
|
schema = context_schema_for_node(workflow, "body")
|
||||||
item = schema["properties"]["foreach"]["properties"]["names"]["properties"]["item"]
|
item = schema["properties"]["foreach"]["properties"]["names"]["properties"]["item"]
|
||||||
validator = Draft202012Validator(item)
|
validator = Draft202012Validator(item)
|
||||||
|
|
||||||
assert validator.is_valid("12345")
|
assert validator.is_valid("ABC")
|
||||||
assert not validator.is_valid("123456")
|
assert not validator.is_valid("ABCD") # Sibling maxLength still applies.
|
||||||
|
assert not validator.is_valid("abc") # Referenced pattern still applies.
|
||||||
|
|||||||
@@ -0,0 +1,10 @@
|
|||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from wf_core.schema_navigation import SchemaNavigator
|
||||||
|
|
||||||
|
|
||||||
|
def test_array_type_does_not_require_an_items_schema() -> None:
|
||||||
|
"""Unconstrained array elements do not make the collection non-array."""
|
||||||
|
view = SchemaNavigator({"type": "array"}).root
|
||||||
|
|
||||||
|
assert view.is_array()
|
||||||
@@ -96,6 +96,61 @@ def test_active_structured_foreach_item_path_is_valid() -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_context_foreach_source_rejects_declared_scalar() -> None:
|
||||||
|
"""A readable context path is not iterable merely because it exists."""
|
||||||
|
from wf_core.validation import validate_workflow
|
||||||
|
|
||||||
|
workflow = _base_workflow()
|
||||||
|
workflow.nodes[1] = _foreach(
|
||||||
|
"orders",
|
||||||
|
over="context.foreach.customers.index",
|
||||||
|
alias="order",
|
||||||
|
)
|
||||||
|
|
||||||
|
report = validate_workflow(workflow)
|
||||||
|
|
||||||
|
assert (
|
||||||
|
_issue(
|
||||||
|
report,
|
||||||
|
ValidationIssueCode.INVALID_FOREACH_SOURCE,
|
||||||
|
"nodes[1].over",
|
||||||
|
)
|
||||||
|
is not None
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_context_foreach_source_accepts_array_without_item_schema() -> None:
|
||||||
|
"""An array remains iterable when its element type is unconstrained."""
|
||||||
|
from wf_core.validation import validate_workflow
|
||||||
|
|
||||||
|
workflow = _base_workflow()
|
||||||
|
workflow.state_schema = StateSchema.model_validate(
|
||||||
|
{
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"items": {"type": "array", "items": {"type": "array"}},
|
||||||
|
"orders_list": {"type": "array"},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
workflow.nodes[1] = _foreach(
|
||||||
|
"orders",
|
||||||
|
over="context.foreach.customers.item",
|
||||||
|
alias="order",
|
||||||
|
)
|
||||||
|
|
||||||
|
report = validate_workflow(workflow)
|
||||||
|
|
||||||
|
assert (
|
||||||
|
_issue(
|
||||||
|
report,
|
||||||
|
ValidationIssueCode.INVALID_FOREACH_SOURCE,
|
||||||
|
"nodes[1].over",
|
||||||
|
)
|
||||||
|
is None
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_nested_body_can_read_outer_and_inner_entries() -> None:
|
def test_nested_body_can_read_outer_and_inner_entries() -> None:
|
||||||
from wf_core.validation import validate_workflow
|
from wf_core.validation import validate_workflow
|
||||||
|
|
||||||
@@ -267,8 +322,6 @@ def test_all_model_surfaces_reject_missing_foreach_id() -> None:
|
|||||||
|
|
||||||
# Foreach over
|
# Foreach over
|
||||||
workflow = _base_workflow()
|
workflow = _base_workflow()
|
||||||
foreach = workflow.nodes[1]
|
|
||||||
assert isinstance(foreach, ForeachNode)
|
|
||||||
workflow.nodes[1] = ForeachNode.model_validate(
|
workflow.nodes[1] = ForeachNode.model_validate(
|
||||||
{"id": "orders", "type": "foreach", "over": bad, "as": "order"}
|
{"id": "orders", "type": "foreach", "over": bad, "as": "order"}
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user