fix: harden canonical output binding projection
This commit is contained in:
+8
-5
@@ -371,11 +371,14 @@ For single-field `input.*` and `state.*` sources, the command projects missing
|
||||
top-level `output_schema` fields from the source schema. More complex or
|
||||
undeclared paths still rely on `wf draft validate` diagnostics.
|
||||
|
||||
By default, `set-input` replaces the complete ordered input-binding list, while
|
||||
`set-output` and `set-workflow-output` replace their complete maps. Use repeated
|
||||
flags in one command when you know the complete replacement. Use `--merge` only
|
||||
for compatibility map edits; they cannot add literals or preserve canonical
|
||||
ordering and repeated-source fan-out.
|
||||
By default, `set-input` and `set-output` replace complete ordered canonical
|
||||
binding lists, while `set-workflow-output` replaces its complete map. Repeated
|
||||
`set-output --map LOCAL_SOURCE=STATE_TARGET` flags preserve their order and may
|
||||
repeat a source to fan it out to several state targets. Use
|
||||
`set-output --bindings-file` for an exported canonical list and `--clear` for an
|
||||
explicit empty replacement. Use `--merge --map` only for compatibility output
|
||||
map edits; that path cannot preserve canonical ordering or repeated-source
|
||||
fan-out.
|
||||
|
||||
### Bind A Step Path
|
||||
|
||||
|
||||
@@ -708,6 +708,9 @@ wf draft set-name <workspace_id> --revision <n> --name <name>
|
||||
wf draft set-route <workspace_id> --revision <n> --step <step_id> --outcome ok --to <target_step_or___end__>
|
||||
wf draft set-input <workspace_id> --revision <n> --step <step_id> --map input.text=text
|
||||
wf draft set-output <workspace_id> --revision <n> --step <step_id> --map text=state.text
|
||||
wf draft set-output <workspace_id> --revision <n> --step <step_id> --bindings-file output-bindings.json
|
||||
wf draft set-output <workspace_id> --revision <n> --step <step_id> --clear
|
||||
wf draft set-output <workspace_id> --revision <n> --step <step_id> --merge --map other=state.other
|
||||
wf draft set-workflow-output <workspace_id> --revision <n> --map state.value=result
|
||||
wf draft set-workflow-output <workspace_id> --revision <n> --merge --map state.other=other
|
||||
wf draft branch <workspace_id> --revision <n> --step <step_id> --route ok=__end__ --route error=tool_error
|
||||
@@ -715,6 +718,14 @@ wf draft handle <workspace_id> --revision <n> --to fail --branch lookup:error --
|
||||
wf draft compile <workspace_id>
|
||||
```
|
||||
|
||||
`set-output` replaces the step's complete ordered canonical binding list.
|
||||
Repeat `--map LOCAL_SOURCE=STATE_TARGET` to preserve order and fan one local
|
||||
source out to multiple state targets, or use `--bindings-file` with the exact
|
||||
JSON list exported by `draft inspect --include-draft`. `--clear` is the explicit
|
||||
empty replacement. The legacy `--merge --map` path is compatibility-only and
|
||||
potentially lossy because a map cannot preserve ordering or repeated-source
|
||||
fan-out.
|
||||
|
||||
Use `draft patch` when these focused commands do not cover the structural edit.
|
||||
|
||||
Drafts are not raw workflow plans. Drafts use `steps`, `routes`, and step field
|
||||
|
||||
@@ -525,17 +525,11 @@ class WorkflowDraftAuthoringApi:
|
||||
spec.output_schema_contract or spec.output_model.model_json_schema()
|
||||
)
|
||||
|
||||
targets = [str(binding.target) for binding in bindings]
|
||||
if has_overlapping_paths(targets):
|
||||
raise _overlapping_output_targets_error(bindings)
|
||||
|
||||
projected_state = _draft_schema(workspace.draft, "state_schema")
|
||||
for index, binding in enumerate(bindings):
|
||||
source_parts = binding.source.parts
|
||||
try:
|
||||
schema_fragment_at_path(
|
||||
capability_schema,
|
||||
source_parts,
|
||||
binding.source.parts,
|
||||
label="capability output schema",
|
||||
)
|
||||
except ValueError as exc:
|
||||
@@ -544,6 +538,13 @@ class WorkflowDraftAuthoringApi:
|
||||
f"is not declared by capability {capability_name!r}: {exc}"
|
||||
) from exc
|
||||
|
||||
targets = [str(binding.target) for binding in bindings]
|
||||
if has_overlapping_paths(targets):
|
||||
raise _overlapping_output_targets_error(bindings)
|
||||
|
||||
projected_state = _draft_schema(workspace.draft, "state_schema")
|
||||
for index, binding in enumerate(bindings):
|
||||
source_parts = binding.source.parts
|
||||
target_parts = binding.target.parts
|
||||
try:
|
||||
projected_state = project_schema_path_to_schema_path(
|
||||
|
||||
@@ -110,8 +110,18 @@ def project_schema_path_to_schema_path(
|
||||
raise ValueError(
|
||||
f"schema path {'.'.join(target_parts[: index + 1])!r} is not an object"
|
||||
)
|
||||
_ensure_object_schema(child, ".".join(target_parts[: index + 1]))
|
||||
parent = child
|
||||
target_label = ".".join(target_parts[: index + 1])
|
||||
# Mutate the referenced definition rather than adding sibling schema
|
||||
# keywords beside $ref, which path lookup intentionally does not merge.
|
||||
resolved_child = _resolve_local_reference(
|
||||
projected,
|
||||
child,
|
||||
label=target_label,
|
||||
)
|
||||
if not isinstance(resolved_child, dict):
|
||||
raise ValueError(f"schema path {target_label!r} is not mutable")
|
||||
_ensure_object_schema(resolved_child, target_label)
|
||||
parent = resolved_child
|
||||
|
||||
properties = _properties_for_object(
|
||||
parent, ".".join(target_parts[:-1]) or "target_schema"
|
||||
|
||||
@@ -1722,6 +1722,36 @@ async def test_set_step_output_bindings_rejects_semantic_errors_without_mutation
|
||||
assert after == before
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_set_step_output_bindings_reports_invalid_source_before_overlap(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
_draft_api_instance, _service, api = await _create_nested_output_binding_api(
|
||||
tmp_path,
|
||||
"invalid_source_before_overlap",
|
||||
)
|
||||
|
||||
with pytest.raises(
|
||||
ValueError,
|
||||
match=r"bindings\[0\]\.source 'report\.missing' is not declared",
|
||||
):
|
||||
await api.set_step_output_bindings(
|
||||
workspace_id="invalid_source_before_overlap",
|
||||
revision=1,
|
||||
step_id="render",
|
||||
bindings=[
|
||||
OutputBinding(
|
||||
source=LocalPath.parse("report.missing"),
|
||||
target=StatePath.parse("state.report.title"),
|
||||
),
|
||||
OutputBinding(
|
||||
source=LocalPath.parse("report.title"),
|
||||
target=StatePath.parse("state.report.title"),
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_set_step_output_bindings_rejects_incompatible_existing_target(
|
||||
tmp_path: Path,
|
||||
@@ -2451,6 +2481,63 @@ async def test_set_step_input_bindings_compiles_and_assembles_nested_payload(
|
||||
assert run.trace[0].output == {"rendered": "Thesis|Evidence|markdown"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_set_step_output_bindings_compile_and_execute_source_fan_out(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
draft_api, service, api = await _create_structured_binding_api(
|
||||
tmp_path,
|
||||
"execute_output_fan_out",
|
||||
)
|
||||
input_result = await api.set_step_input_bindings(
|
||||
workspace_id="execute_output_fan_out",
|
||||
revision=1,
|
||||
step_id="report",
|
||||
bindings=[
|
||||
InputPathBinding(
|
||||
path=GraphSourcePath.input("title"),
|
||||
target=LocalPath.of("request", "title"),
|
||||
),
|
||||
InputPathBinding(
|
||||
path=GraphSourcePath.input("body"),
|
||||
target=LocalPath.of("request", "body"),
|
||||
),
|
||||
InputValueBinding(
|
||||
target=LocalPath.of("request", "format"),
|
||||
value="markdown",
|
||||
),
|
||||
],
|
||||
)
|
||||
await api.set_step_output_bindings(
|
||||
workspace_id="execute_output_fan_out",
|
||||
revision=input_result["revision"],
|
||||
step_id="report",
|
||||
bindings=[
|
||||
OutputBinding(
|
||||
source=LocalPath.parse("rendered"),
|
||||
target=StatePath.parse("state.report"),
|
||||
),
|
||||
OutputBinding(
|
||||
source=LocalPath.parse("rendered"),
|
||||
target=StatePath.parse("state.audit"),
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
compiled = await draft_api.compile_draft_workspace(
|
||||
workspace_id="execute_output_fan_out"
|
||||
)
|
||||
plan = RawWorkflowPlan.model_validate(compiled["compiled_plan"])
|
||||
run = await service.run_workflow_from_plan(
|
||||
plan,
|
||||
{"title": "Thesis", "body": "Evidence"},
|
||||
)
|
||||
|
||||
assert run.error is None
|
||||
assert run.state["report"] == "Thesis|Evidence|markdown"
|
||||
assert run.state["audit"] == "Thesis|Evidence|markdown"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bind_draft_rejects_unsupported_direction(tmp_path: Path) -> None:
|
||||
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_bind_bad_direction")
|
||||
|
||||
@@ -427,6 +427,30 @@ def test_project_schema_path_traverses_legacy_definitions_reference() -> None:
|
||||
assert "Report" in projected["definitions"]
|
||||
|
||||
|
||||
def test_project_schema_path_traverses_target_defs_reference() -> None:
|
||||
projected = project_schema_path_to_schema_path(
|
||||
target_schema={
|
||||
"type": "object",
|
||||
"properties": {"report": {"$ref": "#/$defs/Report"}},
|
||||
"$defs": {
|
||||
"Report": {
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
}
|
||||
},
|
||||
},
|
||||
source_schema={
|
||||
"type": "object",
|
||||
"properties": {"title": {"type": "string"}},
|
||||
},
|
||||
source_parts=("title",),
|
||||
target_parts=("report", "title"),
|
||||
)
|
||||
|
||||
assert projected["properties"]["report"] == {"$ref": "#/$defs/Report"}
|
||||
assert projected["$defs"]["Report"]["properties"]["title"] == {"type": "string"}
|
||||
|
||||
|
||||
def test_schema_path_exists_follows_local_defs() -> None:
|
||||
schema = {
|
||||
"type": "object",
|
||||
|
||||
Reference in New Issue
Block a user