fix via review findings
This commit is contained in:
@@ -520,8 +520,8 @@ def test_adapter_lowers_typed_and_untyped_interrupt_steps() -> None:
|
||||
|
||||
workflow = build_workflow_from_draft(draft)
|
||||
|
||||
review = workflow.nodes[0]
|
||||
legacy = workflow.nodes[1]
|
||||
review = next(node for node in workflow.nodes if node.id == "review")
|
||||
legacy = next(node for node in workflow.nodes if node.id == "legacy")
|
||||
assert isinstance(review, InterruptNode)
|
||||
assert review.request_schema == request_schema
|
||||
assert review.resume_schema == resume_schema
|
||||
@@ -570,6 +570,12 @@ def test_adapter_lowers_subgraph_step_without_resolving_artifact() -> None:
|
||||
assert child.workflow.version == 2
|
||||
assert child.input_schema == SchemaRef.model_validate(input_schema)
|
||||
assert child.output_schema == SchemaRef.model_validate(output_schema)
|
||||
assert child.model_dump(mode="json", by_alias=True)["input"] == [
|
||||
{"path": "state.topic", "target": "topic"}
|
||||
]
|
||||
assert child.model_dump(mode="json", by_alias=True)["output"] == [
|
||||
{"source": "report", "target": "state.report"}
|
||||
]
|
||||
assert child.outcomes == ["ok", "error"]
|
||||
|
||||
|
||||
|
||||
@@ -3,11 +3,13 @@ from __future__ import annotations
|
||||
from typing import Any
|
||||
|
||||
from wf_artifacts import (
|
||||
DraftWorkspaceStore,
|
||||
FileDraftWorkspaceStore,
|
||||
WorkflowDraftWorkspace,
|
||||
create_draft_workspace,
|
||||
get_draft_workspace,
|
||||
patch_draft_workspace,
|
||||
replace_validated_draft_document,
|
||||
summarize_draft_workspace,
|
||||
)
|
||||
|
||||
@@ -252,6 +254,44 @@ def test_patch_draft_workspace_rejects_invalid_patch_without_revision_bump(
|
||||
assert store.get_workspace("echo_draft").revision == 1
|
||||
|
||||
|
||||
def test_replace_validated_draft_document_isolates_persisted_draft() -> None:
|
||||
class ReferenceDraftWorkspaceStore(DraftWorkspaceStore):
|
||||
def __init__(self) -> None:
|
||||
self.workspace: WorkflowDraftWorkspace | None = None
|
||||
|
||||
def save_workspace(self, workspace: WorkflowDraftWorkspace) -> None:
|
||||
self.workspace = workspace
|
||||
|
||||
def get_workspace(self, workspace_id: str) -> WorkflowDraftWorkspace:
|
||||
if self.workspace is None or self.workspace.id != workspace_id:
|
||||
raise KeyError(workspace_id)
|
||||
return self.workspace
|
||||
|
||||
def list_workspaces(self) -> list[WorkflowDraftWorkspace]:
|
||||
return [] if self.workspace is None else [self.workspace]
|
||||
|
||||
def delete_workspace(self, workspace_id: str) -> bool:
|
||||
if self.workspace is None or self.workspace.id != workspace_id:
|
||||
return False
|
||||
self.workspace = None
|
||||
return True
|
||||
|
||||
store = ReferenceDraftWorkspaceStore()
|
||||
create_draft_workspace(store, workspace_id="echo_draft", draft=_draft())
|
||||
replacement = _draft()
|
||||
replacement["name"] = "replacement"
|
||||
|
||||
replace_validated_draft_document(
|
||||
store,
|
||||
workspace_id="echo_draft",
|
||||
revision=1,
|
||||
draft=replacement,
|
||||
)
|
||||
replacement["name"] = "mutated_after_save"
|
||||
|
||||
assert store.get_workspace("echo_draft").draft["name"] == "replacement"
|
||||
|
||||
|
||||
def test_get_draft_workspace_includes_full_draft_only_when_requested(tmp_path) -> None:
|
||||
store = FileDraftWorkspaceStore(tmp_path)
|
||||
create_draft_workspace(store, workspace_id="echo_draft", draft=_draft())
|
||||
|
||||
@@ -906,10 +906,12 @@ async def test_create_empty_draft_workspace_reports_duplicate_conflict(
|
||||
"contract",
|
||||
[
|
||||
{"input_schema": cast(Any, [])},
|
||||
{"input_schema": {"type": 5}},
|
||||
{"outcomes": ()},
|
||||
{"outcomes": cast(Any, (1,))},
|
||||
{"outcomes": ("ok", " ")},
|
||||
{"outcomes": ("ok", "ok")},
|
||||
{"outcomes": ("ok", " ok ")},
|
||||
],
|
||||
)
|
||||
async def test_create_empty_draft_workspace_rejects_invalid_contract_before_mutation(
|
||||
@@ -4594,6 +4596,15 @@ async def test_set_workflow_output_bindings_validates_root_literal_complete_sche
|
||||
],
|
||||
r"bindings\[0\]\.target '\.' already has an incompatible schema",
|
||||
),
|
||||
(
|
||||
[
|
||||
InputPathBinding(
|
||||
path=GraphSourcePath.context("prior_outcome"),
|
||||
target=LocalPath.root(),
|
||||
)
|
||||
],
|
||||
r"bindings\[0\]\.path 'context\.prior_outcome' cannot target '\.'",
|
||||
),
|
||||
(
|
||||
[InputValueBinding(target=LocalPath.root(), value="not-an-object")],
|
||||
r"bindings\[0\]\.value for root target must be an object",
|
||||
@@ -4605,7 +4616,9 @@ async def test_set_workflow_output_bindings_rejects_without_mutation(
|
||||
bindings: list[InputBinding],
|
||||
message: str,
|
||||
) -> None:
|
||||
workspace_id = f"invalid_output_{abs(hash(message))}"
|
||||
# Pytest gives every parameter case an isolated tmp_path, so a stable id is
|
||||
# sufficient and keeps failure artifacts reproducible across processes.
|
||||
workspace_id = "invalid_output"
|
||||
draft_api, _service, authoring = _draft_api(
|
||||
FileWorkflowArtifactStore(tmp_path / workspace_id),
|
||||
register_echo=True,
|
||||
|
||||
@@ -888,6 +888,40 @@ def test_wf_draft_add_capability_calls_composed_local_handler(monkeypatch) -> No
|
||||
assert call["bind_outputs"] == {"value": "state.value"}
|
||||
|
||||
|
||||
def test_wf_draft_add_capability_rejects_outcome_without_source_step(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
class FakeHandlers:
|
||||
async def add_step_from_capability(self, **_kwargs: Any) -> dict[str, Any]:
|
||||
raise AssertionError("handler must not be called")
|
||||
|
||||
context = SimpleNamespace(handlers=FakeHandlers(), verbose=False)
|
||||
monkeypatch.setattr(
|
||||
"wf_cli.commands.draft_add.load_cli_context", lambda _ctx: context
|
||||
)
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"draft",
|
||||
"add",
|
||||
"capability",
|
||||
"workspace",
|
||||
"--revision",
|
||||
"1",
|
||||
"--step",
|
||||
"call",
|
||||
"--capability",
|
||||
"demo.call",
|
||||
"--from-outcome",
|
||||
"error",
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 2
|
||||
assert "--from-outcome requires --from-step" in result.output
|
||||
|
||||
|
||||
def test_wf_draft_update_capability_builds_presence_aware_patch(monkeypatch) -> None:
|
||||
calls: list[dict[str, Any]] = []
|
||||
|
||||
@@ -1503,6 +1537,23 @@ def test_wf_draft_add_control_commands_reject_invalid_input_before_api_call(
|
||||
"decision=state.other_decision",
|
||||
],
|
||||
)
|
||||
malformed_request_source = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"draft",
|
||||
"add",
|
||||
"interrupt",
|
||||
"ws",
|
||||
"--revision",
|
||||
"1",
|
||||
"--step",
|
||||
"review",
|
||||
"--kind",
|
||||
"review",
|
||||
"--request",
|
||||
"not_a_graph_source=items",
|
||||
],
|
||||
)
|
||||
missing_collect_target = runner.invoke(
|
||||
app,
|
||||
[
|
||||
@@ -1522,6 +1573,27 @@ def test_wf_draft_add_control_commands_reject_invalid_input_before_api_call(
|
||||
"collect",
|
||||
],
|
||||
)
|
||||
collect_target_without_collect = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"draft",
|
||||
"add",
|
||||
"foreach",
|
||||
"ws",
|
||||
"--revision",
|
||||
"1",
|
||||
"--step",
|
||||
"each",
|
||||
"--over",
|
||||
"state.items",
|
||||
"--as",
|
||||
"item",
|
||||
"--item-error",
|
||||
"skip",
|
||||
"--collect-to",
|
||||
"state.errors",
|
||||
],
|
||||
)
|
||||
end_route = runner.invoke(
|
||||
app,
|
||||
[
|
||||
@@ -1551,11 +1623,21 @@ def test_wf_draft_add_control_commands_reject_invalid_input_before_api_call(
|
||||
assert "duplicate --resume" in duplicate_resume.output
|
||||
assert "--bind-output" not in duplicate_resume.output
|
||||
assert "Traceback" not in duplicate_resume.output
|
||||
assert malformed_request_source.exit_code == 2
|
||||
assert "--request source 'not_a_graph_source'" in malformed_request_source.output
|
||||
assert "expected" in malformed_request_source.output
|
||||
assert "GRAPH_SOURCE=LOCAL_TARGET" in malformed_request_source.output
|
||||
assert missing_collect_target.exit_code == 2
|
||||
assert (
|
||||
"collect item error policy requires collect_to" in missing_collect_target.output
|
||||
)
|
||||
assert "Traceback" not in missing_collect_target.output
|
||||
assert collect_target_without_collect.exit_code == 2
|
||||
assert (
|
||||
"--collect-to requires --item-error collect"
|
||||
in collect_target_without_collect.output
|
||||
)
|
||||
assert "Traceback" not in collect_target_without_collect.output
|
||||
assert end_route.exit_code == 2
|
||||
assert "No such option" in end_route.output
|
||||
assert "--route" in end_route.output
|
||||
@@ -2214,11 +2296,11 @@ def test_wf_draft_set_input_clear_sends_empty_binding_list(monkeypatch) -> None:
|
||||
([], "provide --map/--value, --bindings-file, or --clear"),
|
||||
(
|
||||
["--bindings-file", "bindings.json", "--map", "input.x=x"],
|
||||
"cannot be combined with --map or",
|
||||
"mutually exclusive",
|
||||
),
|
||||
(
|
||||
["--clear", "--value", "x=null"],
|
||||
"cannot be combined with --map or",
|
||||
"mutually exclusive",
|
||||
),
|
||||
(
|
||||
["--merge", "--value", "x=null"],
|
||||
|
||||
@@ -2,6 +2,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
@@ -68,7 +69,9 @@ def test_update_capability_step_request_preserves_field_presence_and_binding_typ
|
||||
{"unknown": "field"},
|
||||
],
|
||||
)
|
||||
def test_update_capability_step_request_rejects_invalid_patch(update) -> None:
|
||||
def test_update_capability_step_request_rejects_invalid_patch(
|
||||
update: dict[str, Any],
|
||||
) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
UpdateCapabilityStepRequest.model_validate(
|
||||
{
|
||||
@@ -120,6 +123,22 @@ def test_add_step_from_capability_request_rejects_both_input_forms() -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_add_step_from_capability_request_rejects_explicit_null_input_form() -> None:
|
||||
with pytest.raises(ValidationError, match="mutually exclusive"):
|
||||
AddStepFromCapabilityRequest.model_validate(
|
||||
{
|
||||
"workspace_id": "report",
|
||||
"revision": 3,
|
||||
"step_id": "publish",
|
||||
"capability_name": "local.report.publish",
|
||||
"input_map": None,
|
||||
"input_bindings": [
|
||||
{"value": "markdown", "target": "request.format"},
|
||||
],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def test_workflow_surface_rejects_unknown_draft_route_outcome_when_spec_is_known(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
|
||||
@@ -15,6 +15,7 @@ from wf_transport_rpc_http.app import create_rpc_app
|
||||
from wf_transport_rpc_http.models import (
|
||||
AddDraftStepParams,
|
||||
AddStepFromCapabilityParams,
|
||||
SetDraftContractParams,
|
||||
UpdateCapabilityStepParams,
|
||||
)
|
||||
|
||||
@@ -111,6 +112,17 @@ def test_add_step_from_capability_params_reject_both_input_forms() -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_set_draft_contract_params_reject_whitespace_duplicate_outcomes() -> None:
|
||||
with pytest.raises(ValidationError, match="unique"):
|
||||
SetDraftContractParams.model_validate(
|
||||
{
|
||||
"workspace_id": "report",
|
||||
"revision": 1,
|
||||
"outcomes": ["ok", " ok "],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
async def test_rpc_health_and_capability_methods(tmp_path) -> None:
|
||||
server = build_local_static_workflow_server(tmp_path / "store")
|
||||
app = create_rpc_app(server)
|
||||
@@ -473,6 +485,14 @@ async def test_rpc_draft_workspace_lifecycle_methods(tmp_path) -> None:
|
||||
"outcomes": ["ok", "ok"],
|
||||
},
|
||||
),
|
||||
(
|
||||
"workflow.draft_workspaces.set_contract",
|
||||
{
|
||||
"workspace_id": "rpc_control",
|
||||
"revision": 1,
|
||||
"outcomes": ["ok", " ok "],
|
||||
},
|
||||
),
|
||||
(
|
||||
"workflow.draft_workspaces.set_start",
|
||||
{"workspace_id": "rpc_control", "revision": 1, "step_id": " "},
|
||||
@@ -1077,11 +1097,20 @@ async def test_rpc_set_step_output_bindings_rejects_malformed_binding(
|
||||
app = create_rpc_app(server)
|
||||
transport = httpx.ASGITransport(app=app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.create_from_capability",
|
||||
{
|
||||
"workspace_id": "valid_ws",
|
||||
"capability_name": "wf.std.constant",
|
||||
"name": "valid",
|
||||
},
|
||||
)
|
||||
rejected = await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.set_step_output_bindings",
|
||||
{
|
||||
"workspace_id": "missing_ws",
|
||||
"workspace_id": "valid_ws",
|
||||
"revision": 1,
|
||||
"step_id": "call",
|
||||
"bindings": [binding],
|
||||
@@ -1106,11 +1135,20 @@ async def test_rpc_set_step_input_bindings_rejects_malformed_union(
|
||||
app = create_rpc_app(server)
|
||||
transport = httpx.ASGITransport(app=app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.create_from_capability",
|
||||
{
|
||||
"workspace_id": "valid_ws",
|
||||
"capability_name": "wf.std.constant",
|
||||
"name": "valid",
|
||||
},
|
||||
)
|
||||
rejected = await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.set_step_input_bindings",
|
||||
{
|
||||
"workspace_id": "missing_ws",
|
||||
"workspace_id": "valid_ws",
|
||||
"revision": 1,
|
||||
"step_id": "call",
|
||||
"bindings": [binding],
|
||||
@@ -1229,11 +1267,20 @@ async def test_rpc_set_workflow_output_bindings_rejects_malformed_binding(
|
||||
app = create_rpc_app(server)
|
||||
transport = httpx.ASGITransport(app=app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.create_from_capability",
|
||||
{
|
||||
"workspace_id": "valid_ws",
|
||||
"capability_name": "wf.std.constant",
|
||||
"name": "valid",
|
||||
},
|
||||
)
|
||||
rejected = await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.set_workflow_output_bindings",
|
||||
{
|
||||
"workspace_id": "missing_ws",
|
||||
"workspace_id": "valid_ws",
|
||||
"revision": 1,
|
||||
"bindings": [binding],
|
||||
},
|
||||
@@ -1574,7 +1621,7 @@ async def test_rpc_draft_workspace_add_typed_step_round_trip(tmp_path) -> None:
|
||||
)
|
||||
|
||||
assert added["result"]["revision"] == created["result"]["revision"] + 1
|
||||
assert added["result"]["status"] in {"valid", "invalid"}
|
||||
assert added["result"]["status"] == "valid"
|
||||
assert "error" in malformed
|
||||
assert fetched["result"]["revision"] == added["result"]["revision"]
|
||||
assert "bad" not in fetched["result"]["draft"]["steps"]
|
||||
|
||||
Reference in New Issue
Block a user