fix: harden authoring schema and contract projections
This commit is contained in:
@@ -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
|
|
||||||
```
|
|
||||||
@@ -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.<key>` 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
|
|
||||||
```
|
|
||||||
@@ -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 <touched source and test files>
|
|
||||||
All checks passed!
|
|
||||||
|
|
||||||
uv run ruff format --check <touched source and test files>
|
|
||||||
10 files already formatted
|
|
||||||
|
|
||||||
uv run basedpyright --level error <touched source files>
|
|
||||||
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 <touched files>
|
|
||||||
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.
|
|
||||||
@@ -275,7 +275,9 @@ def _append_schema_options(
|
|||||||
"path": path,
|
"path": path,
|
||||||
"label": _label_for(name, child_fragment, resolved_child),
|
"label": _label_for(name, child_fragment, resolved_child),
|
||||||
"origin": origin,
|
"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,
|
"required": name in required,
|
||||||
"availability": "available",
|
"availability": "available",
|
||||||
"uses": list(uses),
|
"uses": list(uses),
|
||||||
|
|||||||
@@ -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:
|
def test_project_authoring_contract_inventory_composes_pure_inputs() -> None:
|
||||||
context_entry: AuthoringPathOptionPayload = {
|
context_entry: AuthoringPathOptionPayload = {
|
||||||
"path": "context.loop_item",
|
"path": "context.loop_item",
|
||||||
|
|||||||
@@ -95,4 +95,32 @@ describe("WorkflowContractInspector", () => {
|
|||||||
await user.click(screen.getByRole("button", { name: "Save outcomes" }));
|
await user.click(screen.getByRole("button", { name: "Save outcomes" }));
|
||||||
expect(controller.setContract).toHaveBeenCalledWith({ outcomes: ["ok"] });
|
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(<WorkflowContractInspector contract="output" controller={controller} draft={outputDraft} inventory={inventory} />);
|
||||||
|
|
||||||
|
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(<WorkflowContractInspector contract="outcomes" controller={controller} draft={draft} inventory={inventory} />);
|
||||||
|
|
||||||
|
await user.click(screen.getAllByRole("button", { name: "Remove" })[0]!);
|
||||||
|
expect(controller.markDirty).toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -127,7 +127,20 @@ const WorkflowOutputBindingsForm = ({ controller, draft, inventory }: WorkflowCo
|
|||||||
) : <label>Literal JSON<textarea value={row.value} onChange={(event) => update(row.id, { value: event.target.value })} /></label>}
|
) : <label>Literal JSON<textarea value={row.value} onChange={(event) => update(row.id, { value: event.target.value })} /></label>}
|
||||||
<label>Output target<select value={row.target} onChange={(event) => update(row.id, { target: event.target.value })}><option value="">Choose output field</option>{targets.map((target) => <option key={target.path} value={target.path.replace(/^output\./, "")}>{target.label}</option>)}</select></label>
|
<label>Output target<select value={row.target} onChange={(event) => update(row.id, { target: event.target.value })}><option value="">Choose output field</option>{targets.map((target) => <option key={target.path} value={target.path.replace(/^output\./, "")}>{target.label}</option>)}</select></label>
|
||||||
<div className="workflow-output-bindings__actions">
|
<div className="workflow-output-bindings__actions">
|
||||||
<button disabled={index === 0} onClick={() => setRows((current) => { const copy = [...current]; [copy[index - 1], copy[index]] = [copy[index]!, copy[index - 1]!]; return copy; })} type="button">Move up</button>
|
<button
|
||||||
|
disabled={index === 0}
|
||||||
|
onClick={() => {
|
||||||
|
setRows((current) => {
|
||||||
|
const copy = [...current];
|
||||||
|
[copy[index - 1], copy[index]] = [copy[index]!, copy[index - 1]!];
|
||||||
|
return copy;
|
||||||
|
});
|
||||||
|
controller.markDirty();
|
||||||
|
}}
|
||||||
|
type="button"
|
||||||
|
>
|
||||||
|
Move up
|
||||||
|
</button>
|
||||||
<button onClick={() => { setRows((current) => current.filter((item) => item.id !== row.id)); controller.markDirty(); }} type="button">Remove binding</button>
|
<button onClick={() => { setRows((current) => current.filter((item) => item.id !== row.id)); controller.markDirty(); }} type="button">Remove binding</button>
|
||||||
</div>
|
</div>
|
||||||
</fieldset>
|
</fieldset>
|
||||||
@@ -143,7 +156,7 @@ const OutcomesForm = ({ controller, draft }: WorkflowContractInspectorProps) =>
|
|||||||
const [rows, setRows] = useState<ReadonlyArray<string>>(() => Array.isArray(raw) ? raw.map(String) : []);
|
const [rows, setRows] = useState<ReadonlyArray<string>>(() => Array.isArray(raw) ? raw.map(String) : []);
|
||||||
return (
|
return (
|
||||||
<form className="workflow-outcomes-form" onSubmit={(event) => { event.preventDefault(); void controller.setContract({ outcomes: normalizeOutcomes(rows) }); }}>
|
<form className="workflow-outcomes-form" onSubmit={(event) => { event.preventDefault(); void controller.setContract({ outcomes: normalizeOutcomes(rows) }); }}>
|
||||||
{rows.map((outcome, index) => <div key={index}><label>Outcome {index + 1}<input value={outcome} onChange={(event) => { setRows((current) => current.map((item, itemIndex) => itemIndex === index ? event.target.value : item)); controller.markDirty(); }} /></label><button onClick={() => setRows((current) => current.filter((_, itemIndex) => itemIndex !== index))} type="button">Remove</button></div>)}
|
{rows.map((outcome, index) => <div key={index}><label>Outcome {index + 1}<input value={outcome} onChange={(event) => { setRows((current) => current.map((item, itemIndex) => itemIndex === index ? event.target.value : item)); controller.markDirty(); }} /></label><button onClick={() => { setRows((current) => current.filter((_, itemIndex) => itemIndex !== index)); controller.markDirty(); }} type="button">Remove</button></div>)}
|
||||||
<button onClick={() => { setRows((current) => [...current, ""]); controller.markDirty(); }} type="button">Add outcome</button>
|
<button onClick={() => { setRows((current) => [...current, ""]); controller.markDirty(); }} type="button">Add outcome</button>
|
||||||
<button type="submit">Save outcomes</button>
|
<button type="submit">Save outcomes</button>
|
||||||
</form>
|
</form>
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { cleanup, render, screen } from "@testing-library/react";
|
import { cleanup, render, screen, within } from "@testing-library/react";
|
||||||
import userEvent from "@testing-library/user-event";
|
import userEvent from "@testing-library/user-event";
|
||||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
import { WorkflowSchemaFieldsForm } from "./WorkflowSchemaFieldsForm.js";
|
import { WorkflowSchemaFieldsForm } from "./WorkflowSchemaFieldsForm.js";
|
||||||
@@ -79,4 +79,68 @@ describe("WorkflowSchemaFieldsForm", () => {
|
|||||||
await user.click(screen.getByRole("button", { name: "Save output schema" }));
|
await user.click(screen.getByRole("button", { name: "Save output schema" }));
|
||||||
expect(onSubmit).toHaveBeenLastCalledWith(expect.objectContaining({ properties: {} }));
|
expect(onSubmit).toHaveBeenLastCalledWith(expect.objectContaining({ properties: {} }));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("updates dotted property names independently from nested properties", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const onSubmit = vi.fn();
|
||||||
|
render(
|
||||||
|
<WorkflowSchemaFieldsForm
|
||||||
|
contract="input"
|
||||||
|
onSubmit={onSubmit}
|
||||||
|
schema={{
|
||||||
|
type: "object",
|
||||||
|
properties: {
|
||||||
|
"a.b": { type: "string", description: "Flat property" },
|
||||||
|
a: {
|
||||||
|
type: "object",
|
||||||
|
properties: { b: { type: "string", description: "Nested property" } },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
const nested = screen.getByRole("group", { name: "Schema field b" });
|
||||||
|
const description = within(nested).getByRole("textbox", { name: "Description" });
|
||||||
|
await user.clear(description);
|
||||||
|
await user.type(description, "Updated nested property");
|
||||||
|
await user.click(screen.getByRole("button", { name: "Save input schema" }));
|
||||||
|
|
||||||
|
expect(onSubmit).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
properties: expect.objectContaining({
|
||||||
|
"a.b": expect.objectContaining({ description: "Flat property" }),
|
||||||
|
a: expect.objectContaining({
|
||||||
|
properties: { b: expect.objectContaining({ description: "Updated nested property" }) },
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects blank and duplicate field names before serializing", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const onSubmit = vi.fn();
|
||||||
|
render(
|
||||||
|
<WorkflowSchemaFieldsForm
|
||||||
|
contract="output"
|
||||||
|
onSubmit={onSubmit}
|
||||||
|
schema={{
|
||||||
|
type: "object",
|
||||||
|
properties: { first: { type: "string" }, second: { type: "string" } },
|
||||||
|
}}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
const names = screen.getAllByRole("textbox", { name: "Field name" });
|
||||||
|
await user.clear(names[1]!);
|
||||||
|
await user.click(screen.getByRole("button", { name: "Save output schema" }));
|
||||||
|
expect(onSubmit).not.toHaveBeenCalled();
|
||||||
|
expect(screen.getByRole("alert")).toHaveTextContent("must not be blank");
|
||||||
|
|
||||||
|
await user.type(names[1]!, "first");
|
||||||
|
await user.click(screen.getByRole("button", { name: "Save output schema" }));
|
||||||
|
expect(onSubmit).not.toHaveBeenCalled();
|
||||||
|
expect(screen.getByRole("alert")).toHaveTextContent("must be unique");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import type { JsonObject } from "../domain/draft-workspace-models.js";
|
|||||||
import {
|
import {
|
||||||
projectWorkflowSchema,
|
projectWorkflowSchema,
|
||||||
serializeWorkflowSchema,
|
serializeWorkflowSchema,
|
||||||
|
validateWorkflowSchemaRows,
|
||||||
type WorkflowContractKind,
|
type WorkflowContractKind,
|
||||||
type WorkflowSchemaFieldRow,
|
type WorkflowSchemaFieldRow,
|
||||||
type WorkflowSchemaFieldType,
|
type WorkflowSchemaFieldType,
|
||||||
@@ -210,8 +211,12 @@ export const WorkflowSchemaFieldsForm = ({
|
|||||||
}: WorkflowSchemaFieldsFormProps) => {
|
}: WorkflowSchemaFieldsFormProps) => {
|
||||||
const [projection] = useState(() => projectWorkflowSchema(schema));
|
const [projection] = useState(() => projectWorkflowSchema(schema));
|
||||||
const [rows, setRows] = useState(projection.rows);
|
const [rows, setRows] = useState(projection.rows);
|
||||||
|
const [validationError, setValidationError] = useState<string | null>(null);
|
||||||
const nextId = useRef(0);
|
const nextId = useRef(0);
|
||||||
const markDirty = (): void => onDirtyChange?.(true);
|
const markDirty = (): void => {
|
||||||
|
setValidationError(null);
|
||||||
|
onDirtyChange?.(true);
|
||||||
|
};
|
||||||
const createId = (): string => `new-field-${nextId.current++}`;
|
const createId = (): string => `new-field-${nextId.current++}`;
|
||||||
const update = (
|
const update = (
|
||||||
id: string,
|
id: string,
|
||||||
@@ -226,6 +231,11 @@ export const WorkflowSchemaFieldsForm = ({
|
|||||||
};
|
};
|
||||||
const submit = (event: FormEvent<HTMLFormElement>): void => {
|
const submit = (event: FormEvent<HTMLFormElement>): void => {
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
|
const issues = validateWorkflowSchemaRows(rows);
|
||||||
|
if (issues.length > 0) {
|
||||||
|
setValidationError(issues.join(" "));
|
||||||
|
return;
|
||||||
|
}
|
||||||
void Promise.resolve(
|
void Promise.resolve(
|
||||||
onSubmit(serializeWorkflowSchema(projection, rows, { state: contract === "state" })),
|
onSubmit(serializeWorkflowSchema(projection, rows, { state: contract === "state" })),
|
||||||
).catch(() => undefined);
|
).catch(() => undefined);
|
||||||
@@ -233,6 +243,7 @@ export const WorkflowSchemaFieldsForm = ({
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<form className="workflow-schema-fields-form" noValidate onSubmit={submit}>
|
<form className="workflow-schema-fields-form" noValidate onSubmit={submit}>
|
||||||
|
{validationError !== null && <p role="alert">{validationError}</p>}
|
||||||
{projection.rootUnsupportedReason !== null ? (
|
{projection.rootUnsupportedReason !== null ? (
|
||||||
<section aria-label="Unsupported root schema" className="workflow-schema-field--unsupported">
|
<section aria-label="Unsupported root schema" className="workflow-schema-field--unsupported">
|
||||||
<p>{projection.rootUnsupportedReason}</p>
|
<p>{projection.rootUnsupportedReason}</p>
|
||||||
|
|||||||
@@ -45,12 +45,21 @@ export type WorkflowContractPatch = {
|
|||||||
readonly outcomes?: ReadonlyArray<string>;
|
readonly outcomes?: ReadonlyArray<string>;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type WorkflowSchemaPathSegment =
|
||||||
|
| { readonly kind: "property"; readonly name: string }
|
||||||
|
| { readonly kind: "array-item" };
|
||||||
|
|
||||||
const MAX_DEPTH = 64;
|
const MAX_DEPTH = 64;
|
||||||
const COMPOSITION_KEYS = ["oneOf", "anyOf", "allOf", "not", "if", "then", "else"] as const;
|
const COMPOSITION_KEYS = ["oneOf", "anyOf", "allOf", "not", "if", "then", "else"] as const;
|
||||||
|
|
||||||
const isRecord = (value: unknown): value is JsonObject =>
|
const isRecord = (value: unknown): value is JsonObject =>
|
||||||
typeof value === "object" && value !== null && !Array.isArray(value);
|
typeof value === "object" && value !== null && !Array.isArray(value);
|
||||||
|
|
||||||
|
const schemaFieldId = (path: ReadonlyArray<WorkflowSchemaPathSegment>): string =>
|
||||||
|
// JSON-encoded tagged segments keep a property named "a.b" distinct from
|
||||||
|
// a nested property path ["a", "b"].
|
||||||
|
JSON.stringify(path);
|
||||||
|
|
||||||
export const copyJson = (value: unknown): unknown => {
|
export const copyJson = (value: unknown): unknown => {
|
||||||
if (Array.isArray(value)) return value.map(copyJson);
|
if (Array.isArray(value)) return value.map(copyJson);
|
||||||
if (!isRecord(value)) return value;
|
if (!isRecord(value)) return value;
|
||||||
@@ -97,7 +106,7 @@ const projectField = (
|
|||||||
name: string,
|
name: string,
|
||||||
schema: unknown,
|
schema: unknown,
|
||||||
required: boolean,
|
required: boolean,
|
||||||
path: string,
|
path: ReadonlyArray<WorkflowSchemaPathSegment>,
|
||||||
depth: number,
|
depth: number,
|
||||||
): WorkflowSchemaFieldRow => {
|
): WorkflowSchemaFieldRow => {
|
||||||
const raw = isRecord(schema) ? copyObject(schema) : {};
|
const raw = isRecord(schema) ? copyObject(schema) : {};
|
||||||
@@ -117,16 +126,16 @@ const projectField = (
|
|||||||
childName,
|
childName,
|
||||||
childSchema,
|
childSchema,
|
||||||
requiredNames.has(childName),
|
requiredNames.has(childName),
|
||||||
`${path}.${childName}`,
|
[...path, { kind: "property", name: childName }],
|
||||||
depth + 1,
|
depth + 1,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
: [];
|
: [];
|
||||||
const item = reason === null && type === "array" && raw.items !== undefined
|
const item = reason === null && type === "array" && raw.items !== undefined
|
||||||
? projectField("item", raw.items, true, `${path}.items`, depth + 1)
|
? projectField("item", raw.items, true, [...path, { kind: "array-item" }], depth + 1)
|
||||||
: null;
|
: null;
|
||||||
return {
|
return {
|
||||||
id: path,
|
id: schemaFieldId(path),
|
||||||
name,
|
name,
|
||||||
type,
|
type,
|
||||||
required,
|
required,
|
||||||
@@ -154,13 +163,37 @@ export const projectWorkflowSchema = (schema: unknown): WorkflowSchemaProjection
|
|||||||
schema: root,
|
schema: root,
|
||||||
rows: rootReason === null
|
rows: rootReason === null
|
||||||
? Object.entries(properties).map(([name, field]) =>
|
? Object.entries(properties).map(([name, field]) =>
|
||||||
projectField(name, field, requiredNames.has(name), name, 1),
|
projectField(
|
||||||
|
name,
|
||||||
|
field,
|
||||||
|
requiredNames.has(name),
|
||||||
|
[{ kind: "property", name }],
|
||||||
|
1,
|
||||||
|
),
|
||||||
)
|
)
|
||||||
: [],
|
: [],
|
||||||
rootUnsupportedReason: rootReason,
|
rootUnsupportedReason: rootReason,
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
|
export const validateWorkflowSchemaRows = (
|
||||||
|
rows: ReadonlyArray<WorkflowSchemaFieldRow>,
|
||||||
|
): ReadonlyArray<string> => {
|
||||||
|
const issues = new Set<string>();
|
||||||
|
const visit = (scopeRows: ReadonlyArray<WorkflowSchemaFieldRow>): void => {
|
||||||
|
const names = new Set<string>();
|
||||||
|
for (const row of scopeRows) {
|
||||||
|
if (row.name.trim() === "") issues.add("Field names must not be blank.");
|
||||||
|
else if (names.has(row.name)) issues.add("Field names must be unique within each object.");
|
||||||
|
names.add(row.name);
|
||||||
|
visit(row.children);
|
||||||
|
if (row.item !== null) visit([row.item]);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
visit(rows);
|
||||||
|
return [...issues];
|
||||||
|
};
|
||||||
|
|
||||||
const serializeField = (row: WorkflowSchemaFieldRow, state: boolean): JsonObject => {
|
const serializeField = (row: WorkflowSchemaFieldRow, state: boolean): JsonObject => {
|
||||||
if (row.unsupportedReason !== null) return copyObject(row.raw);
|
if (row.unsupportedReason !== null) return copyObject(row.raw);
|
||||||
const next = copyObject(row.raw);
|
const next = copyObject(row.raw);
|
||||||
@@ -200,6 +233,8 @@ export const serializeWorkflowSchema = (
|
|||||||
options: { readonly state?: boolean } = {},
|
options: { readonly state?: boolean } = {},
|
||||||
): JsonObject => {
|
): JsonObject => {
|
||||||
if (projection.rootUnsupportedReason !== null) return copyObject(projection.schema);
|
if (projection.rootUnsupportedReason !== null) return copyObject(projection.schema);
|
||||||
|
const issues = validateWorkflowSchemaRows(rows);
|
||||||
|
if (issues.length > 0) throw new Error(issues.join(" "));
|
||||||
const next = copyObject(projection.schema);
|
const next = copyObject(projection.schema);
|
||||||
next.type = "object";
|
next.type = "object";
|
||||||
next.properties = Object.fromEntries(
|
next.properties = Object.fromEntries(
|
||||||
|
|||||||
@@ -531,7 +531,9 @@ export const authoredRpcSchemas = {
|
|||||||
payload: Schema.Struct({
|
payload: Schema.Struct({
|
||||||
workspace_id: Schema.String.pipe(Schema.minLength(1)),
|
workspace_id: Schema.String.pipe(Schema.minLength(1)),
|
||||||
revision: PositiveIntegerSchema,
|
revision: PositiveIntegerSchema,
|
||||||
selected_step_id: Schema.optional(Schema.NullOr(Schema.String)),
|
selected_step_id: Schema.optional(
|
||||||
|
Schema.NullOr(Schema.String.pipe(Schema.minLength(1))),
|
||||||
|
),
|
||||||
}),
|
}),
|
||||||
success: Schema.Union(AuthoringContractInventorySchema, DraftWorkspaceSchema),
|
success: Schema.Union(AuthoringContractInventorySchema, DraftWorkspaceSchema),
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -548,7 +548,11 @@ const parityCases: ReadonlyArray<ParityCase> = [
|
|||||||
revision: 7,
|
revision: 7,
|
||||||
selected_step_id: "render",
|
selected_step_id: "render",
|
||||||
},
|
},
|
||||||
invalidPayload: { workspace_id: "console.demo", revision: 0 },
|
invalidPayload: {
|
||||||
|
workspace_id: "console.demo",
|
||||||
|
revision: 7,
|
||||||
|
selected_step_id: "",
|
||||||
|
},
|
||||||
validSuccess: authoringContractInventory,
|
validSuccess: authoringContractInventory,
|
||||||
invalidSuccess: {
|
invalidSuccess: {
|
||||||
...authoringContractInventory,
|
...authoringContractInventory,
|
||||||
|
|||||||
@@ -61,6 +61,35 @@ describe("run operation registry", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("preserves explicit null and empty contract replacement semantics", async () => {
|
||||||
|
const { getOperationMeta } = await import("./method-registry.js");
|
||||||
|
const operation = getOperationMeta("workflow.draft_workspaces.set_contract");
|
||||||
|
if (operation === undefined) throw new Error("missing set contract operation");
|
||||||
|
|
||||||
|
const cli = operation.equivalentCli({
|
||||||
|
workspace_id: "console.demo",
|
||||||
|
revision: 7,
|
||||||
|
input_schema: null,
|
||||||
|
outcomes: [],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(cli).toContain("input_schema=null (no equivalent CLI clear flag)");
|
||||||
|
expect(cli).toContain("outcomes=[] (no equivalent CLI clear flag)");
|
||||||
|
expect(cli).not.toContain("--outcome");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders non-empty contract outcomes as repeated CLI flags", async () => {
|
||||||
|
const { getOperationMeta } = await import("./method-registry.js");
|
||||||
|
const operation = getOperationMeta("workflow.draft_workspaces.set_contract");
|
||||||
|
if (operation === undefined) throw new Error("missing set contract operation");
|
||||||
|
|
||||||
|
expect(operation.equivalentCli({
|
||||||
|
workspace_id: "console.demo",
|
||||||
|
revision: 7,
|
||||||
|
outcomes: ["ok", "cancelled"],
|
||||||
|
})).toContain("--outcome ok --outcome cancelled");
|
||||||
|
});
|
||||||
|
|
||||||
it("decodes start results with the start success schema", async () => {
|
it("decodes start results with the start success schema", async () => {
|
||||||
vi.doMock("./rpcs.js", async () => {
|
vi.doMock("./rpcs.js", async () => {
|
||||||
const actual = await vi.importActual<typeof import("./rpcs.js")>(
|
const actual = await vi.importActual<typeof import("./rpcs.js")>(
|
||||||
|
|||||||
@@ -839,12 +839,32 @@ const operationEntries = defineOperationEntries([
|
|||||||
String(p.revision),
|
String(p.revision),
|
||||||
];
|
];
|
||||||
const unavailable: string[] = [];
|
const unavailable: string[] = [];
|
||||||
if (p.input_schema != null) unavailable.push("input_schema (use --input-schema-file)");
|
if (p.input_schema !== undefined) {
|
||||||
if (p.state_schema != null) unavailable.push("state_schema (use --state-schema-file)");
|
unavailable.push(p.input_schema === null
|
||||||
if (p.output_schema != null) unavailable.push("output_schema (use --output-schema-file)");
|
? "input_schema=null (no equivalent CLI clear flag)"
|
||||||
for (const outcome of p.outcomes ?? []) {
|
: "input_schema (use --input-schema-file)");
|
||||||
|
}
|
||||||
|
if (p.state_schema !== undefined) {
|
||||||
|
unavailable.push(p.state_schema === null
|
||||||
|
? "state_schema=null (no equivalent CLI clear flag)"
|
||||||
|
: "state_schema (use --state-schema-file)");
|
||||||
|
}
|
||||||
|
if (p.output_schema !== undefined) {
|
||||||
|
unavailable.push(p.output_schema === null
|
||||||
|
? "output_schema=null (no equivalent CLI clear flag)"
|
||||||
|
: "output_schema (use --output-schema-file)");
|
||||||
|
}
|
||||||
|
if (p.outcomes !== undefined) {
|
||||||
|
if (p.outcomes === null) {
|
||||||
|
unavailable.push("outcomes=null (no equivalent CLI clear flag)");
|
||||||
|
} else if (p.outcomes.length === 0) {
|
||||||
|
unavailable.push("outcomes=[] (no equivalent CLI clear flag)");
|
||||||
|
} else {
|
||||||
|
for (const outcome of p.outcomes) {
|
||||||
parts.push("--outcome", shellArg(outcome));
|
parts.push("--outcome", shellArg(outcome));
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
return nonEquivalentCli(parts.join(" "), unavailable);
|
return nonEquivalentCli(parts.join(" "), unavailable);
|
||||||
},
|
},
|
||||||
interpret: (result) => {
|
interpret: (result) => {
|
||||||
|
|||||||
Reference in New Issue
Block a user