From 0570ef8a781d02034d3f60999ec9a79c80c1fcd5 Mon Sep 17 00:00:00 2001 From: lda Date: Sat, 29 Aug 2026 21:08:05 +0700 Subject: [PATCH] fix: harden authoring schema and contract projections --- .../task-1-report.md | 93 ------------- .../task-2-report.md | 127 ------------------ .../task-3-report.md | 122 ----------------- src/wf_api/authoring_contracts.py | 4 +- tests/wf_api/test_authoring_contracts.py | 18 +++ .../WorkflowContractInspector.test.tsx | 28 ++++ .../authoring/WorkflowContractInspector.tsx | 17 ++- .../WorkflowSchemaFieldsForm.test.tsx | 66 ++++++++- .../authoring/WorkflowSchemaFieldsForm.tsx | 13 +- .../authoring/workflow-contract-editor.ts | 45 ++++++- .../src/json-schema/authored-rpc-fixtures.ts | 4 +- .../rpc/src/json-schema/rpc-parity.test.ts | 6 +- web/packages/rpc/src/method-registry.test.ts | 29 ++++ web/packages/rpc/src/method-registry.ts | 30 ++++- 14 files changed, 243 insertions(+), 359 deletions(-) delete mode 100644 .superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md delete mode 100644 .superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-2-report.md delete mode 100644 .superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-3-report.md diff --git a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md b/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md deleted file mode 100644 index 2a883401..00000000 --- a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-1-report.md +++ /dev/null @@ -1,93 +0,0 @@ -# Task 1 Report: Model Schema-Derived Authoring Choices - -## Status - -Implemented Task 1 of the workflow contract graph backend slice. - -## Changes - -- Added explicit transport payload types for path options, step contracts, and - revision-scoped authoring inventories. -- Added `schema_path_options`, which derives deterministic parent-before-child - choices from JSON Schema object properties. -- Preserved whole arrays as selectable paths without synthetic wildcard paths. -- Omitted invented child names for unconstrained or open-ended - `additionalProperties`. -- Reused `schema_fragment_at_location` and its bounded local-reference depth for - nested schema fragments and `$defs`/`definitions` references. -- Derived labels from schema titles or humanized path segments and copied only - string descriptions. -- Added pure inventory composition for input/state/context sources, selected - step targets and sources, state/output targets, entry steps, outcomes, and - warnings. -- Re-exported the new payload types through `wf_api.models`. - -## Test-First Evidence - -The required RED command was run before production implementation: - -```text -uv run pytest tests/wf_api/test_authoring_contracts.py -q -``` - -It failed during collection with: - -```text -ModuleNotFoundError: No module named 'wf_api.authoring_contracts' -``` - -After implementation, the focused authoring-contract tests passed. - -## Verification - -```text -uv run pytest tests/wf_api/test_authoring_contracts.py tests/wf_api/test_schema_projection.py -q -48 passed - -uv run basedpyright --level error src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py -0 errors, 0 warnings, 0 notes - -uv run ruff check src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py src/wf_api/models/__init__.py tests/wf_api/test_authoring_contracts.py -All checks passed! - -uv run ruff format --check src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py src/wf_api/models/__init__.py tests/wf_api/test_authoring_contracts.py -4 files already formatted -``` - -## Concerns - -None for the Task 1 scope. Runtime-context analysis and persisted workspace or -capability loading remain intentionally deferred to Tasks 2 and 3. - -## Round 1 Fix - -The review identified that recursive schema walking was not bounded even -though individual `$ref` chains were bounded. A self-referential local -definition repeatedly expanded `_append_schema_options` until -`RecursionError`. - -Added the regression test -`test_schema_path_options_stops_expanding_recursive_local_definition` before -changing production code. The RED run failed with `RecursionError` in -`_append_schema_options` after repeatedly traversing the self-reference. - -The fix tracks active local `$ref` definitions per traversal branch. A repeated -definition is emitted as a selectable path but is not expanded again. Distinct -active references are also capped using the existing -`_MAX_LOCAL_SCHEMA_REFERENCE_DEPTH` limit from `schema_projection`. - -Round 1 verification: - -```text -uv run pytest tests/wf_api/test_authoring_contracts.py tests/wf_api/test_schema_projection.py -q -49 passed - -uv run basedpyright --level error src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py -0 errors, 0 warnings, 0 notes - -uv run ruff check src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py src/wf_api/models/__init__.py tests/wf_api/test_authoring_contracts.py -All checks passed! - -uv run ruff format --check src/wf_api/authoring_contracts.py src/wf_api/models/authoring_contracts.py src/wf_api/models/__init__.py tests/wf_api/test_authoring_contracts.py -4 files already formatted -``` diff --git a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-2-report.md b/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-2-report.md deleted file mode 100644 index d61a9eff..00000000 --- a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-2-report.md +++ /dev/null @@ -1,127 +0,0 @@ -# Task 2 Report: Analyze Node-Scoped Runtime Context - -## Status - -Implemented Task 2 of the workflow contract graph backend slice. - -## Changes - -- Added `wf_core.context_contracts` with the exact standard runtime context - field schemas and shared key constants. -- Added `foreach_context_fields`, including `loop_item`, `loop_index`, and a - configured alias with duplicate loop-key aliases removed. -- Updated `frame_context_values` to use the shared context key registry while - preserving its existing runtime values. -- Added `context_fields_by_node`, an abstract traversal that memoizes - `(node_id, active_foreach_id)` and distinguishes available from conditional - fields across reachable frame scopes. -- Added bounded graph warnings for missing route targets, missing loop routes, - invalid workflow starts, and invalid edge sources. -- Derived foreach item schemas from declared input/state array sources, falling - back to `{}` when the source is not declared as an array with an item schema. -- Added Task 1 authoring projection helpers for canonical `context.` paths. - Runtime context is offered only for `step_input`; workflow output projections - do not advertise `context.*`. -- Added coverage for ordinary frames, serial/concurrent foreach, conditional - reachability, nested scope replacement/restoration, malformed routes, cyclic - graphs, alias deduplication, and authoring projection behavior. - -## Test-First Evidence - -The required RED command was run after adding tests and before production -changes: - -```text -uv run pytest tests/core/test_context_scopes.py tests/core/test_scheduler.py tests/wf_api/test_authoring_contracts.py -q -``` - -It failed during collection for the expected missing production seams: - -```text -ModuleNotFoundError: No module named 'wf_core.analysis' -ImportError: cannot import name 'context_path_options' from 'wf_api.authoring_contracts' -14 passed, 2 errors -``` - -## Verification - -```text -uv run pytest tests/core/test_context_scopes.py tests/core/test_scheduler.py tests/wf_api/test_authoring_contracts.py -q -29 passed - -uv run pytest tests/core -q -296 passed - -uv run pytest tests/wf_api/test_authoring_contracts.py -q -7 passed - -uv run ruff check src/wf_core/context_contracts.py src/wf_core/analysis src/wf_core/runtime/ops/frames.py src/wf_api/authoring_contracts.py tests/core/test_context_scopes.py tests/core/test_scheduler.py tests/wf_api/test_authoring_contracts.py -All checks passed! - -uv run basedpyright --level error src/wf_core/context_contracts.py src/wf_core/analysis src/wf_core/runtime/ops/frames.py -0 errors, 0 warnings, 0 notes -``` - -## Concerns - -- Foreach item schema traversal intentionally handles declared direct object - properties and array `items`; complex external or deeply composed schema - references fall back conservatively to `{}` rather than inventing a type. -- The authoring projector accepts an optional workflow for automatic context - projection, while existing callers can continue supplying Task 1 payloads - explicitly. - -## Round 1 Fix - -The review identified three Important findings and the fixes were tested at -their affected seams. - -### 1. Bounded local `$ref` item schemas - -Added `test_foreach_item_schema_resolves_bounded_local_array_reference` before -the production change. Its targeted RED run failed because `loop_item` had no -`type` after a declared `state.items` array was selected through -`#/$defs/Items`. - -The analyzer now resolves bounded local `$defs`/`definitions` references for -both the selected array source and its `items` schema. Cyclic, unsupported, or -unresolved references remain conservative and produce `{}`. - -### 2. Reserved context aliases - -Added `test_all_standard_context_names_are_reserved_from_foreach_aliases` -before the production change. Its targeted RED run failed because -`prior_outcome` was accepted as a foreach alias. - -The shared `RESERVED_CONTEXT_KEYS` registry now covers every standard context -name plus `loop_item` and `loop_index`. Both contract generation and -`frame_context_values` use it, so runtime values and authoring inventory cannot -disagree on a colliding alias. - -### 3. Scoped-cycle regression - -Added `test_scoped_cycle_terminates_and_preserves_scoped_field_availability`, -which enters a foreach body, routes back to the foreach node under the child -scope, and asserts the body/owner availability results. The test passed before -the round-1 production changes because Task 2 already memoized -`(node_id, active_foreach_id)` correctly; this finding required regression -coverage but did not require a production change. - -Round 1 verification: - -```text -uv run pytest tests/core/test_context_scopes.py tests/core/test_scheduler.py tests/wf_api/test_authoring_contracts.py -q -32 passed - -uv run pytest tests/core -q -299 passed - -uv run ruff check src/wf_core/context_contracts.py src/wf_core/analysis src/wf_core/runtime/ops/frames.py src/wf_api/authoring_contracts.py tests/core/test_context_scopes.py tests/core/test_scheduler.py tests/wf_api/test_authoring_contracts.py -All checks passed! - -uv run ruff format --check src/wf_core/context_contracts.py src/wf_core/analysis src/wf_core/runtime/ops/frames.py src/wf_api/authoring_contracts.py tests/core/test_context_scopes.py tests/core/test_scheduler.py tests/wf_api/test_authoring_contracts.py -8 files already formatted - -uv run basedpyright --level error src/wf_core/context_contracts.py src/wf_core/analysis src/wf_core/runtime/ops/frames.py src/wf_api/authoring_contracts.py -0 errors, 0 warnings, 0 notes -``` 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 deleted file mode 100644 index 982a8a6a..00000000 --- a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-3-report.md +++ /dev/null @@ -1,122 +0,0 @@ -# Task 3 Report: Expose Authoring Contract Inspection - -## Status - -Implemented Task 3 of the workflow contract graph backend slice. - -## Changes - -- Added `WorkflowDraftSurface.inspect_draft_authoring_contract` and the - matching `WorkflowApi` implementation. -- Added read-only persisted workspace loading with canonical revision-conflict - precedence and no validation-save or revision mutation. -- Projected tolerant workflow input/state/output schemas through the Task 1 - inventory projector. -- Projected resolved capability input/output schemas, outcomes, descriptions, - and executable entry candidates for keyed `use` steps. Projection ids and - `__end__` are not advertised as entry candidates. -- Integrated Task 2 runtime context analysis for the selected step. Compile or - interpretation failures leave scoped context empty and become warnings. -- Added the JSON-RPC params model, method dispatch, typed remote client method, - nullable `selected_step_id`, and named OpenRPC payload references. -- Preserved existing domain error mapping for unknown steps and missing - workspaces, `-32602` for malformed RPC params, and the existing - `revision_conflict` result for stale revisions. - -## Test-First Evidence - -The required RED command was run after adding the service tests and before -production implementation: - -```text -uv run pytest tests/wf_api/test_drafts_service.py -q -k authoring_contract -``` - -It failed for the expected missing seam: - -```text -4 failed -AttributeError: 'WorkflowApi' object has no attribute -'inspect_draft_authoring_contract' -``` - -After implementation, the new authoring-contract service and transport tests -passed: - -```text -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 authoring_contract -12 passed -``` - -## Verification - -```text -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" -391 passed, 184 warnings - -uv run ruff check -All checks passed! - -uv run ruff format --check -10 files already formatted - -uv run basedpyright --level error -0 errors, 0 warnings, 0 notes -``` - -The exact broad target command also contains the existing -`test_rpc_workflow_client_reads_admin_state` failure: its recorded event lacks -the required `timestamp_epoch_ms` field when serialized as `AdminEventPayload`. -That failure reproduces in isolation and is unrelated to the Task 3 files. - -No Serena configuration was modified. - -## Concerns - -- The repository's existing admin-event timestamp validation failure prevents - the unfiltered four-file target command from being fully green; the full - 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/authoring_contracts.py b/src/wf_api/authoring_contracts.py index 1c04327e..ae0af01b 100644 --- a/src/wf_api/authoring_contracts.py +++ b/src/wf_api/authoring_contracts.py @@ -275,7 +275,9 @@ def _append_schema_options( "path": path, "label": _label_for(name, child_fragment, resolved_child), "origin": origin, - "schema": child_fragment, + # The UI may annotate an option schema; keep that mutation away from + # the canonical draft schema used to build the rest of the inventory. + "schema": deepcopy(dict(child_fragment)), "required": name in required, "availability": "available", "uses": list(uses), diff --git a/tests/wf_api/test_authoring_contracts.py b/tests/wf_api/test_authoring_contracts.py index f53bdc99..e26487cd 100644 --- a/tests/wf_api/test_authoring_contracts.py +++ b/tests/wf_api/test_authoring_contracts.py @@ -255,6 +255,24 @@ def test_schema_path_options_returns_empty_schema_for_unconstrained_property() - ] +def test_schema_path_options_copies_schema_fragments() -> None: + schema = { + "type": "object", + "properties": { + "request": { + "type": "object", + "properties": {"id": {"type": "string"}}, + } + }, + } + + options = schema_path_options(schema, root="input", uses=["step_input"]) + # Options are safe for UI consumers to annotate without mutating the source schema. + options[0]["schema"]["title"] = "Edited through option" + + assert schema["properties"]["request"].get("title") is None + + def test_project_authoring_contract_inventory_composes_pure_inputs() -> None: context_entry: AuthoringPathOptionPayload = { "path": "context.loop_item", diff --git a/web/apps/console/src/workspace/authoring/WorkflowContractInspector.test.tsx b/web/apps/console/src/workspace/authoring/WorkflowContractInspector.test.tsx index 2b45d571..27ec60b1 100644 --- a/web/apps/console/src/workspace/authoring/WorkflowContractInspector.test.tsx +++ b/web/apps/console/src/workspace/authoring/WorkflowContractInspector.test.tsx @@ -95,4 +95,32 @@ describe("WorkflowContractInspector", () => { await user.click(screen.getByRole("button", { name: "Save outcomes" })); expect(controller.setContract).toHaveBeenCalledWith({ outcomes: ["ok"] }); }); + + it("marks output reordering as dirty", async () => { + const user = userEvent.setup(); + controller.markDirty.mockClear(); + const outputDraft = { + ...draft, + draft: { + ...draft.draft, + output: [ + { path: "state.report", target: "report" }, + { path: "state.report", target: "report_copy" }, + ], + }, + } satisfies DraftWorkspace; + render(); + + await user.click(screen.getAllByRole("button", { name: "Move up" })[1]!); + expect(controller.markDirty).toHaveBeenCalled(); + }); + + it("marks outcome removal as dirty", async () => { + const user = userEvent.setup(); + controller.markDirty.mockClear(); + render(); + + await user.click(screen.getAllByRole("button", { name: "Remove" })[0]!); + expect(controller.markDirty).toHaveBeenCalled(); + }); }); diff --git a/web/apps/console/src/workspace/authoring/WorkflowContractInspector.tsx b/web/apps/console/src/workspace/authoring/WorkflowContractInspector.tsx index 821ccac7..b6787f42 100644 --- a/web/apps/console/src/workspace/authoring/WorkflowContractInspector.tsx +++ b/web/apps/console/src/workspace/authoring/WorkflowContractInspector.tsx @@ -127,7 +127,20 @@ const WorkflowOutputBindingsForm = ({ controller, draft, inventory }: WorkflowCo ) :