fix: harden workflow contract tooling
This commit is contained in:
@@ -51,6 +51,9 @@ _PROJECT_NODE_SPEC_SUMMARY = JsonProjector(NodeSpecCapabilitySummary)
|
||||
_PROJECT_WRAPPER_SUMMARY = JsonProjector(WrapperArtifactCapabilitySummary)
|
||||
_PROJECT_NODE_SPEC_DETAIL = JsonProjector(NodeSpecCapabilityDetail)
|
||||
_PROJECT_WRAPPER_DETAIL = JsonProjector(WrapperArtifactCapabilityDetail)
|
||||
_PROJECT_CREATE_DRAFT_FROM_CAPABILITY = JsonProjector(
|
||||
CreateDraftWorkspaceFromCapabilityResult
|
||||
)
|
||||
|
||||
|
||||
def _schema_field_names(schema: dict[str, Any]) -> list[str]:
|
||||
@@ -421,13 +424,10 @@ class WorkflowCapabilityApi:
|
||||
hints=dict(hints),
|
||||
).model_dump(mode="json"),
|
||||
)
|
||||
# Both merged extensions are independently validated or model-dumped,
|
||||
# while the workspace payload was projected by the mutating API.
|
||||
return cast(
|
||||
CreateDraftWorkspaceFromCapabilityResult,
|
||||
return _PROJECT_CREATE_DRAFT_FROM_CAPABILITY(
|
||||
{
|
||||
**result,
|
||||
"wrapper_hints": hints,
|
||||
"next_actions": next_actions,
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
@@ -40,6 +40,8 @@ type CapabilitySummary = NodeSpecCapabilitySummary | WrapperArtifactCapabilitySu
|
||||
class ListCapabilitiesResult(PageMetadataPayload):
|
||||
"""Cursor-paged planner-visible capability discovery result."""
|
||||
|
||||
# Keep this union inline: Pydantic otherwise emits a new named component
|
||||
# and changes the established OpenRPC wire contract.
|
||||
capabilities: list[NodeSpecCapabilitySummary | WrapperArtifactCapabilitySummary]
|
||||
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
from .generate import generate_manifest
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from .io import (
|
||||
DEFAULT_MANIFEST_PATH,
|
||||
ManifestDriftError,
|
||||
@@ -9,6 +10,18 @@ from .io import (
|
||||
from .model import ContractManifest, JsonSchema, JsonValue, ManifestError
|
||||
from .normalize import manifest_from_openrpc
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .generate import generate_manifest
|
||||
|
||||
|
||||
def __getattr__(name: str) -> object:
|
||||
"""Load the server-backed generator only when a caller requests it."""
|
||||
if name == "generate_manifest":
|
||||
from .generate import generate_manifest
|
||||
|
||||
return generate_manifest
|
||||
raise AttributeError(name)
|
||||
|
||||
__all__ = [
|
||||
"ContractManifest",
|
||||
"JsonSchema",
|
||||
|
||||
@@ -6,6 +6,7 @@ from pathlib import Path
|
||||
from .model import ContractManifest
|
||||
|
||||
REPOSITORY_ROOT = Path(__file__).resolve().parents[2]
|
||||
REPOSITORY_MARKER = REPOSITORY_ROOT / "pyproject.toml"
|
||||
DEFAULT_MANIFEST_PATH = REPOSITORY_ROOT / "contracts" / "workflow-api.manifest.json"
|
||||
|
||||
|
||||
@@ -34,6 +35,11 @@ def canonical_manifest_json(manifest: ContractManifest) -> str:
|
||||
def write_manifest(
|
||||
manifest: ContractManifest, path: Path = DEFAULT_MANIFEST_PATH
|
||||
) -> Path:
|
||||
if path == DEFAULT_MANIFEST_PATH and not REPOSITORY_MARKER.is_file():
|
||||
raise RuntimeError(
|
||||
"cannot write the default manifest outside the repository checkout: "
|
||||
f"missing {REPOSITORY_MARKER}"
|
||||
)
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(canonical_manifest_json(manifest), encoding="utf-8", newline="\n")
|
||||
return path
|
||||
|
||||
@@ -251,11 +251,12 @@ def manifest_from_openrpc(document: Mapping[str, object]) -> ContractManifest:
|
||||
):
|
||||
parameter_path = f"{method_path}.params[{parameter_index}]"
|
||||
parameter = _mapping(parameter_value, parameter_path)
|
||||
raw_required = parameter.get("required", False)
|
||||
params.append(
|
||||
{
|
||||
"name": _string(parameter.get("name"), f"{parameter_path}.name"),
|
||||
"required": _boolean(
|
||||
parameter.get("required"), f"{parameter_path}.required"
|
||||
raw_required, f"{parameter_path}.required"
|
||||
),
|
||||
"schema": _schema(
|
||||
parameter.get("schema"), f"{parameter_path}.schema"
|
||||
@@ -266,9 +267,11 @@ def manifest_from_openrpc(document: Mapping[str, object]) -> ContractManifest:
|
||||
result_path = f"{method_path}.result"
|
||||
result = _mapping(method.get("result"), result_path)
|
||||
if "schema" not in result:
|
||||
raise ManifestError(f"{result_path}.schema", "expected an object")
|
||||
raise ManifestError(
|
||||
f"{result_path}.schema", "missing success result schema"
|
||||
)
|
||||
raw_result_schema = result["schema"]
|
||||
if isinstance(raw_result_schema, Mapping) and (
|
||||
if not isinstance(raw_result_schema, Mapping) or (
|
||||
set(raw_result_schema) != {"$ref"}
|
||||
or not (
|
||||
isinstance(raw_result_schema.get("$ref"), str)
|
||||
|
||||
@@ -4,6 +4,7 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
import wf_api.capabilities as capabilities_module
|
||||
from tests.wf_mcp.test_support import echo_tool
|
||||
from tests.wf_mcp.workflow_surface.conftest import echo_artifact, failing_tool
|
||||
from wf_api.capabilities import WorkflowCapabilityApi
|
||||
@@ -232,6 +233,34 @@ async def test_create_draft_workspace_from_capability(tmp_path: Path) -> None:
|
||||
assert fetched["draft"]["steps"]["call"]["use"] == "demo.personal.echo_tool"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_draft_workspace_validates_the_merged_result(
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
artifact_store = FileWorkflowArtifactStore(tmp_path / "cap_api_projection")
|
||||
api, _service = _capability_api(artifact_store, register_echo=True)
|
||||
projected: list[object] = []
|
||||
projector = capabilities_module._PROJECT_CREATE_DRAFT_FROM_CAPABILITY
|
||||
|
||||
def capture_projection(value: object):
|
||||
projected.append(value)
|
||||
return projector(value)
|
||||
|
||||
monkeypatch.setattr(
|
||||
capabilities_module,
|
||||
"_PROJECT_CREATE_DRAFT_FROM_CAPABILITY",
|
||||
capture_projection,
|
||||
)
|
||||
|
||||
result = await api.create_draft_workspace_from_capability(
|
||||
workspace_id="echo_projected",
|
||||
capability_name="demo.personal.echo_tool",
|
||||
)
|
||||
|
||||
assert projected == [result]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handler_delegates_to_capability_api(tmp_path: Path) -> None:
|
||||
"""WorkflowSurfaceHandlers methods produce the same result as direct API."""
|
||||
|
||||
@@ -21,7 +21,16 @@ def _manifest() -> ContractManifest:
|
||||
def test_write_generates_once_and_writes_requested_contract(monkeypatch, tmp_path: Path) -> None:
|
||||
manifest = _manifest()
|
||||
calls: list[tuple[object, Path]] = []
|
||||
monkeypatch.setattr("wf_contract_manifest.__main__.generate_manifest", lambda: manifest)
|
||||
generate_calls = 0
|
||||
|
||||
def fake_generate_manifest() -> ContractManifest:
|
||||
nonlocal generate_calls
|
||||
generate_calls += 1
|
||||
return manifest
|
||||
|
||||
monkeypatch.setattr(
|
||||
"wf_contract_manifest.__main__.generate_manifest", fake_generate_manifest
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"wf_contract_manifest.__main__.write_manifest",
|
||||
lambda value, path: calls.append((value, path)) or path,
|
||||
@@ -30,6 +39,7 @@ def test_write_generates_once_and_writes_requested_contract(monkeypatch, tmp_pat
|
||||
|
||||
assert main(["write"]) == 0
|
||||
assert calls == [(manifest, tmp_path / "manifest.json")]
|
||||
assert generate_calls == 1
|
||||
|
||||
|
||||
def test_check_returns_nonzero_and_prints_drift_guidance(monkeypatch, capsys) -> None:
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
|
||||
def test_package_import_keeps_server_stack_lazy() -> None:
|
||||
script = """
|
||||
import sys
|
||||
import wf_contract_manifest
|
||||
|
||||
assert "wf_server" not in sys.modules
|
||||
assert "wf_transport_rpc_http" not in sys.modules
|
||||
assert callable(wf_contract_manifest.generate_manifest)
|
||||
assert "wf_server" in sys.modules
|
||||
assert "wf_transport_rpc_http" in sys.modules
|
||||
"""
|
||||
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", script],
|
||||
capture_output=True,
|
||||
check=False,
|
||||
text=True,
|
||||
)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
@@ -7,6 +7,7 @@ from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
import wf_contract_manifest.io as manifest_io
|
||||
from wf_contract_manifest import (
|
||||
ManifestDriftError,
|
||||
canonical_manifest_json,
|
||||
@@ -70,6 +71,16 @@ def test_write_and_check_round_trip(tmp_path: Path) -> None:
|
||||
assert path.read_bytes() == canonical_manifest_json(_manifest()).encode("utf-8")
|
||||
|
||||
|
||||
def test_default_write_refuses_to_target_a_non_checkout_parent(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
missing_marker = manifest_io.REPOSITORY_ROOT / "missing-pyproject.toml"
|
||||
monkeypatch.setattr(manifest_io, "REPOSITORY_MARKER", missing_marker)
|
||||
|
||||
with pytest.raises(RuntimeError, match="repository checkout"):
|
||||
write_manifest(_manifest())
|
||||
|
||||
|
||||
def test_check_reports_drift_without_mutating_the_file(tmp_path: Path) -> None:
|
||||
path = tmp_path / "workflow-api.manifest.json"
|
||||
path.write_text("stale\n", encoding="utf-8")
|
||||
|
||||
@@ -73,6 +73,15 @@ def test_preserves_parameter_order_optionality_and_nullability() -> None:
|
||||
assert operation["params"][1]["schema"]["additionalProperties"] is False
|
||||
|
||||
|
||||
def test_defaults_an_absent_parameter_required_flag_to_false() -> None:
|
||||
document = synthetic_openrpc_document()
|
||||
del document["methods"][1]["params"][0]["required"]
|
||||
|
||||
operation = manifest_from_openrpc(document)["operations"][0]
|
||||
|
||||
assert operation["params"][0]["required"] is False
|
||||
|
||||
|
||||
def test_removes_only_titles_and_preserves_unknown_schema_keywords() -> None:
|
||||
manifest = manifest_from_openrpc(synthetic_openrpc_document())
|
||||
optional_schema = manifest["operations"][0]["params"][0]["schema"]
|
||||
@@ -199,7 +208,6 @@ def test_rejects_empty_or_malformed_dotted_method_names(
|
||||
("field", "value", "missing", "message"),
|
||||
[
|
||||
("required", "yes", False, "expected a boolean"),
|
||||
("required", None, True, "expected a boolean"),
|
||||
("schema", [], False, "expected a schema object"),
|
||||
("schema", None, True, "expected a schema object"),
|
||||
],
|
||||
@@ -228,7 +236,18 @@ def test_rejects_invalid_result_schema_shape() -> None:
|
||||
assert_manifest_error(
|
||||
document,
|
||||
"$.methods[1].result.schema",
|
||||
"expected a schema object",
|
||||
"success result must reference a named schema component",
|
||||
)
|
||||
|
||||
|
||||
def test_reports_a_missing_success_result_schema() -> None:
|
||||
document = synthetic_openrpc_document()
|
||||
del document["methods"][1]["result"]["schema"]
|
||||
|
||||
assert_manifest_error(
|
||||
document,
|
||||
"$.methods[1].result.schema",
|
||||
"missing success result schema",
|
||||
)
|
||||
|
||||
|
||||
@@ -324,7 +343,7 @@ def test_rejects_non_string_component_keys_before_sorting(
|
||||
(
|
||||
lambda document: document["methods"][0].update({"result": {}}),
|
||||
"$.methods[0].result.schema",
|
||||
"expected an object",
|
||||
"missing success result schema",
|
||||
),
|
||||
(
|
||||
lambda document: document["methods"][0]["result"].update(
|
||||
|
||||
Reference in New Issue
Block a user