From 1625fc3d7e055af25e21595b20767a050c014ee0 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 14 Aug 2026 16:10:37 +0700 Subject: [PATCH] fix: harden authoring contract inventory --- .../task-3-report.md | 42 +++++++ src/wf_api/capabilities.py | 41 +++++++ src/wf_api/service.py | 67 ++++++++---- tests/wf_api/test_drafts_service.py | 103 ++++++++++++++++++ 4 files changed, 234 insertions(+), 19 deletions(-) diff --git a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-3-report.md b/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-3-report.md index dbe4b52d..982a8a6a 100644 --- a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-3-report.md +++ b/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-3-report.md @@ -78,3 +78,45 @@ No Serena configuration was modified. target set passes when that isolated test is excluded. - FastAPI JSON-RPC emits deprecation warnings from the installed `fastapi-jsonrpc` dependency; no new warning class was introduced. + +## Round 1 Review Fixes + +Addressed all three Important findings from `task-3-review.md`. + +### Test-First Evidence + +Each regression was verified RED before its production fix: + +- Invalid persisted workflow schema: `test_inspect_draft_authoring_contract_tolerates_invalid_workflow_schema` initially raised `ValueError` from `schema_path_options` during inventory projection. +- Saved wrapper capability: `test_inspect_draft_authoring_contract_resolves_saved_wrapper_capability` initially returned no entry contract because the service only called `get_qualified_spec`. +- Explicit empty capability schemas: `test_inspect_draft_authoring_contract_preserves_empty_capability_schemas` was forced back to the pre-fix truthiness resolver and then advertised Pydantic model fields instead of empty projections. + +### Fixes + +- Added per-schema validation at the inventory service boundary. Invalid persisted input, state, or output schemas now produce an empty affected projection and a warning while preserving the other inventory sections. +- Added `WorkflowCapabilityApi.resolve_capability_contract` as the shared resolver for live `NodeSpec` and saved wrapper contracts. Draft inventory inspection now resolves wrapper artifacts using the same capability surface and preserves wrapper schemas/outcomes. +- Capability schema fallback now uses `is not None`, preserving explicit `{}` input and output contracts. + +### Verification + +```text +uv run pytest tests/wf_api/test_drafts_service.py tests/wf_api/test_capability_api.py tests/wf_api/test_authoring_contracts.py -q -k "inspect_draft_authoring_contract or authoring_contract or saved_wrapper" +17 passed + +uv run pytest tests/wf_api/test_drafts_service.py tests/wf_transport_rpc_http/test_app.py tests/wf_transport_rpc_http/test_client.py tests/wf_transport_rpc_http/test_openrpc_contract.py -q -k "not reads_admin_state" +394 passed, 184 warnings + +uv run ruff check +All checks passed + +uv run ruff format --check +3 files already formatted + +uv run basedpyright --level error src/wf_api/capabilities.py src/wf_api/service.py +0 errors, 0 warnings, 0 notes +``` + +Repository-wide basedpyright still reports 394 pre-existing diagnostics in +unrelated examples, CLI, MCP, and test files. The known isolated +`test_rpc_workflow_client_reads_admin_state` failure remains excluded from the +target command; no admin-event or Serena configuration files were changed. diff --git a/src/wf_api/capabilities.py b/src/wf_api/capabilities.py index 5bfed624..301b28d9 100644 --- a/src/wf_api/capabilities.py +++ b/src/wf_api/capabilities.py @@ -1,6 +1,7 @@ from __future__ import annotations from collections.abc import Mapping, Sequence +from dataclasses import dataclass from typing import Any, cast from wf_artifacts import ( @@ -56,6 +57,16 @@ _PROJECT_CREATE_DRAFT_FROM_CAPABILITY = JsonProjector( ) +@dataclass(frozen=True, slots=True) +class ResolvedCapabilityContract: + """Schema and outcome contract shared by node specs and saved wrappers.""" + + input_schema: dict[str, Any] + output_schema: dict[str, Any] + outcomes: tuple[str, ...] + description: str | None + + def _schema_field_names(schema: dict[str, Any]) -> list[str]: """Return top-level JSON object property names for compact discovery rows.""" properties = schema.get("properties") @@ -166,6 +177,36 @@ class WorkflowCapabilityApi: return wrapper_detail raise KeyError(f"unknown workflow capability {qualified_name!r}") + def resolve_capability_contract( + self, + qualified_name: str, + ) -> ResolvedCapabilityContract: + """Resolve authoring schemas for a live node or saved wrapper.""" + wrapper_artifact = self._wrapper_artifact_for_capability_name(qualified_name) + if wrapper_artifact is not None: + return ResolvedCapabilityContract( + input_schema=wrapper_artifact.input_schema, + output_schema=wrapper_artifact.output_schema, + outcomes=wrapper_artifact.outcomes, + description=wrapper_artifact.description, + ) + + spec = self.context.specs.get_qualified_spec(qualified_name) + return ResolvedCapabilityContract( + input_schema=( + spec.input_schema_contract + if spec.input_schema_contract is not None + else spec.input_model.model_json_schema() + ), + output_schema=( + spec.output_schema_contract + if spec.output_schema_contract is not None + else spec.output_model.model_json_schema() + ), + outcomes=spec.outcomes, + description=spec.description, + ) + async def call_capability( self, *, diff --git a/src/wf_api/service.py b/src/wf_api/service.py index 3ed4527b..dacefaf0 100644 --- a/src/wf_api/service.py +++ b/src/wf_api/service.py @@ -14,6 +14,7 @@ from .authoring_contracts import ( context_path_options_for_node, project_authoring_contract_inventory, project_authoring_step_contract, + schema_path_options, ) from .capabilities import WorkflowCapabilityApi from .deployments import WorkflowDeploymentApi @@ -53,9 +54,26 @@ from .operation_context import WorkflowOperationContext from .runs import TraceRangeLike, WorkflowRunApi -def _authoring_schema(value: object) -> dict[str, Any]: - """Return a safe schema object from a possibly invalid persisted draft.""" - return dict(value) if isinstance(value, Mapping) else {} +def _authoring_schema( + value: object, + *, + field_name: str, + root: str, + warnings: list[str], +) -> dict[str, Any]: + """Return one valid persisted schema, isolating invalid projections.""" + if not isinstance(value, Mapping): + warnings.append( + f"{field_name} authoring choices unavailable: expected a schema object" + ) + return {} + schema = dict(value) + try: + schema_path_options(schema, root=root, uses=[]) + except ValueError as exc: + warnings.append(f"{field_name} authoring choices unavailable: {exc}") + return {} + return schema def _authoring_outcomes(value: object) -> list[str]: @@ -393,12 +411,8 @@ class WorkflowApi: ) continue try: - spec = self.context.specs.get_qualified_spec(capability_name) - input_schema = ( - spec.input_schema_contract or spec.input_model.model_json_schema() - ) - output_schema = ( - spec.output_schema_contract or spec.output_model.model_json_schema() + resolved_contract = self.capabilities.resolve_capability_contract( + capability_name ) except (KeyError, TypeError, ValueError) as exc: if raw_step_id == selected_step_id: @@ -409,18 +423,18 @@ class WorkflowApi: description = raw_step.get("desc") if not isinstance(description, str): - description = spec.description - contract = project_authoring_step_contract( + description = resolved_contract.description + projected_contract = project_authoring_step_contract( step_id=raw_step_id, label=_step_label(raw_step_id), description=description, - input_schema=input_schema, - output_schema=output_schema, - outcomes=spec.outcomes, + input_schema=resolved_contract.input_schema, + output_schema=resolved_contract.output_schema, + outcomes=resolved_contract.outcomes, ) - entry_steps.append(contract) + entry_steps.append(projected_contract) if raw_step_id == selected_step_id: - selected_contract = contract + selected_contract = projected_contract context_entries = [] if selected_step_id is not None: @@ -449,9 +463,24 @@ class WorkflowApi: workspace_id=workspace_id, revision=checked.revision, selected_step_id=selected_step_id, - input_schema=_authoring_schema(draft.get("input_schema")), - state_schema=_authoring_schema(draft.get("state_schema")), - output_schema=_authoring_schema(draft.get("output_schema")), + input_schema=_authoring_schema( + draft.get("input_schema"), + field_name="input_schema", + root="input", + warnings=warnings, + ), + state_schema=_authoring_schema( + draft.get("state_schema"), + field_name="state_schema", + root="state", + warnings=warnings, + ), + output_schema=_authoring_schema( + draft.get("output_schema"), + field_name="output_schema", + root="output", + warnings=warnings, + ), context_entries=context_entries, step_input_targets=selected_input_targets, step_output_sources=selected_output_sources, diff --git a/tests/wf_api/test_drafts_service.py b/tests/wf_api/test_drafts_service.py index a6bb307d..72ee166e 100644 --- a/tests/wf_api/test_drafts_service.py +++ b/tests/wf_api/test_drafts_service.py @@ -8,6 +8,7 @@ import pytest from pydantic import BaseModel, Field, TypeAdapter, ValidationError from tests.wf_mcp.test_support import echo_tool +from tests.wf_mcp.workflow_surface.conftest import echo_artifact from wf_api.draft_authoring import RouteSource, WorkflowDraftAuthoringApi from wf_api.draft_updates import CapabilityStepUpdate from wf_api.drafts import WorkflowDraftApi @@ -174,6 +175,108 @@ async def test_inspect_draft_authoring_contract_projects_selected_capability( assert selected["outcomes"] == ["ok"] +@pytest.mark.asyncio +async def test_inspect_draft_authoring_contract_tolerates_invalid_workflow_schema( + tmp_path: Path, +) -> None: + draft_api, _service, authoring = _draft_api( + FileWorkflowArtifactStore(tmp_path / "authoring_contract_invalid_schema"), + register_echo=True, + ) + draft = _echo_draft() + draft["input_schema"] = {"type": 7} + draft["state_schema"] = { + "type": "object", + "properties": {"echoed": {"type": "string"}}, + } + await draft_api.create_draft_workspace(workspace_id="authoring", draft=draft) + api = WorkflowApi(authoring.context) + + inventory = await api.inspect_draft_authoring_contract( + workspace_id="authoring", + revision=1, + selected_step_id="echo", + ) + + assert inventory["readable_sources"] + assert all( + option["origin"] != "workflow_input" for option in inventory["readable_sources"] + ) + assert {option["path"] for option in inventory["state_targets"]} == {"state.echoed"} + assert {option["path"] for option in inventory["workflow_output_targets"]} == { + "output.echoed" + } + assert any("input_schema" in warning for warning in inventory["warnings"]) + + +@pytest.mark.asyncio +async def test_inspect_draft_authoring_contract_resolves_saved_wrapper_capability( + tmp_path: Path, +) -> None: + artifact_store = FileWorkflowArtifactStore(tmp_path / "authoring_contract_wrapper") + artifact_store.save_artifact( + echo_artifact().model_copy(update={"id": "echo_wrapper", "kind": "wrapper"}) + ) + draft_api, _service, authoring = _draft_api(artifact_store) + draft = _echo_draft() + draft["steps"]["echo"]["use"] = "workflow.echo_wrapper.v1" + draft["state_schema"] = { + "type": "object", + "properties": {"echoed": {"type": "string"}}, + } + await draft_api.create_draft_workspace(workspace_id="authoring", draft=draft) + api = WorkflowApi(authoring.context) + + inventory = await api.inspect_draft_authoring_contract( + workspace_id="authoring", + revision=1, + selected_step_id="echo", + ) + + assert [step["step_id"] for step in inventory["entry_steps"]] == ["echo"] + assert {option["path"] for option in inventory["step_input_targets"]} == { + "step_input.text" + } + assert {option["path"] for option in inventory["step_output_sources"]} == { + "step_output.echoed" + } + assert inventory["entry_steps"][0]["outcomes"] == ["completed"] + + +@pytest.mark.asyncio +async def test_inspect_draft_authoring_contract_preserves_empty_capability_schemas( + tmp_path: Path, +) -> None: + draft_api, service, authoring = _draft_api( + FileWorkflowArtifactStore(tmp_path / "authoring_contract_empty_schema") + ) + service.register_connection( + ConnectionConfig(id="demo.personal", server="demo", account="personal") + ) + service.register_specs( + "demo.personal", + replace(echo_tool, input_schema_contract={}, output_schema_contract={}), + ) + draft = _echo_draft() + draft["state_schema"] = { + "type": "object", + "properties": {"echoed": {"type": "string"}}, + } + await draft_api.create_draft_workspace(workspace_id="authoring", draft=draft) + api = WorkflowApi(authoring.context) + + inventory = await api.inspect_draft_authoring_contract( + workspace_id="authoring", + revision=1, + selected_step_id="echo", + ) + + assert inventory["entry_steps"][0]["input_targets"] == [] + assert inventory["entry_steps"][0]["output_sources"] == [] + assert inventory["step_input_targets"] == [] + assert inventory["step_output_sources"] == [] + + @pytest.mark.asyncio async def test_inspect_draft_authoring_contract_rejects_unknown_selected_step( tmp_path: Path,