feat: add draft validation repair hints
This commit is contained in:
@@ -69,6 +69,8 @@ clear operator feedback before adding more architecture.
|
|||||||
- Completed: `wf draft bind-output-to-state` composes state schema projection
|
- Completed: `wf draft bind-output-to-state` composes state schema projection
|
||||||
with output binding merge, reducing manual draft patch repairs in agent
|
with output binding merge, reducing manual draft patch repairs in agent
|
||||||
challenge runs.
|
challenge runs.
|
||||||
|
- Completed: draft validation now preserves structured core validation issues
|
||||||
|
and adds exact `bind-output-to-state` repair hints for missing state fields.
|
||||||
- Keep status read-only; do not mutate registry, auth, config, or stores.
|
- Keep status read-only; do not mutate registry, auth, config, or stores.
|
||||||
|
|
||||||
## Priority 2: Durable Run/Resume Hardening
|
## Priority 2: Durable Run/Resume Hardening
|
||||||
|
|||||||
@@ -0,0 +1,598 @@
|
|||||||
|
# Draft Validation Repair Hints Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Make `wf draft validate` return structured repair hints for common draft authoring failures, starting with missing state fields for step output bindings.
|
||||||
|
|
||||||
|
**Architecture:** Preserve low-level draft validation in `wf_artifacts`, but stop flattening core workflow validation into opaque `ValueError` text. Convert `wf_core.validation.ValidationIssue` objects into `DraftDiagnostic` objects with structured `details`, then let `wf_api` add workspace-aware CLI repair hints when validating a stored draft workspace. The first exact hint targets the helper we just added: `wf draft bind-output-to-state`.
|
||||||
|
|
||||||
|
**Tech Stack:** Python 3.14, Pydantic DTOs, existing `wf_core` validation reports, existing draft workspace API, pytest, ruff, basedpyright.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
When a draft contains a step output binding like:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"source": {"root": "local", "parts": ["after"]},
|
||||||
|
"target": {"root": "state", "parts": ["after"]}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
but `state_schema.properties.after` is missing, `wf draft validate <workspace_id>` should return a diagnostic like:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"code": "invalid_destination_path",
|
||||||
|
"path": "nodes[0].output[0].target",
|
||||||
|
"step_id": "wait",
|
||||||
|
"message": "destination path must start with state. and reference a declared root field",
|
||||||
|
"details": {
|
||||||
|
"output_field": "after",
|
||||||
|
"state_path": "state.after"
|
||||||
|
},
|
||||||
|
"repair_hint": "wf draft bind-output-to-state browser_ws --revision 5 --step wait --output after --state state.after"
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This slice only adds exact repair hints for stored draft validation. `patch_draft_workspace` may still return diagnostics without exact commands; users and agents should run `wf draft validate` after edits.
|
||||||
|
|
||||||
|
Non-goals:
|
||||||
|
|
||||||
|
- Do not add auto-fix.
|
||||||
|
- Do not infer routes.
|
||||||
|
- Do not parse arbitrary exception strings.
|
||||||
|
- Do not add nested state path support.
|
||||||
|
- Do not reimplement JSON Schema validation.
|
||||||
|
|
||||||
|
## Files
|
||||||
|
|
||||||
|
- Modify: `src/wf_artifacts/drafts/api.py`
|
||||||
|
- Add `repair_hint` and `details` to `DraftDiagnostic`.
|
||||||
|
- Validate compiled workflows with `workflow.validate_structure()`.
|
||||||
|
- Convert core validation issues into draft diagnostics.
|
||||||
|
- Modify: `src/wf_api/drafts.py`
|
||||||
|
- Add workspace-aware repair hints in `validate_draft_workspace`.
|
||||||
|
- Tests:
|
||||||
|
- `tests/artifacts/test_draft_adapter.py`
|
||||||
|
- `tests/wf_api/test_drafts_service.py`
|
||||||
|
- Docs/skills:
|
||||||
|
- `docs/wf_cli.md`
|
||||||
|
- `skills/wf-cli/SKILL.md`
|
||||||
|
- `skills/wf-workflow/references/draft-workspaces.md`
|
||||||
|
- `docs/current_roadmap.md`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: Preserve Core Validation Issues In Draft Diagnostics
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- Modify: `src/wf_artifacts/drafts/api.py`
|
||||||
|
- Modify: `tests/artifacts/test_draft_adapter.py`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write failing low-level draft validation test**
|
||||||
|
|
||||||
|
In `tests/artifacts/test_draft_adapter.py`, add this test near the existing `validate_workflow_draft` tests:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_validate_workflow_draft_reports_structured_output_destination_issue() -> None:
|
||||||
|
draft = {
|
||||||
|
"name": "missing_state_field",
|
||||||
|
"input_schema": {},
|
||||||
|
"state_schema": {"type": "object", "properties": {}},
|
||||||
|
"output_schema": {},
|
||||||
|
"start": "snap",
|
||||||
|
"steps": {
|
||||||
|
"snap": {
|
||||||
|
"use": "demo.snapshot",
|
||||||
|
"output": [
|
||||||
|
{
|
||||||
|
"source": {"root": "local", "parts": ["after"]},
|
||||||
|
"target": {"root": "state", "parts": ["after"]},
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"routes": {"snap": {"ok": "__end__"}},
|
||||||
|
}
|
||||||
|
|
||||||
|
result = validate_workflow_draft(draft)
|
||||||
|
|
||||||
|
assert result["status"] == "invalid"
|
||||||
|
diagnostic = result["diagnostics"][0]
|
||||||
|
assert diagnostic["code"] == "invalid_destination_path"
|
||||||
|
assert diagnostic["path"] == "nodes[0].output[0].target"
|
||||||
|
assert diagnostic["step_id"] == "snap"
|
||||||
|
assert diagnostic["details"] == {
|
||||||
|
"output_field": "after",
|
||||||
|
"state_path": "state.after",
|
||||||
|
}
|
||||||
|
assert "repair_hint" not in diagnostic
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run test to verify red**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
uv run pytest tests/artifacts/test_draft_adapter.py -q -k "structured_output_destination_issue"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: fail because `validate_workflow_draft` currently returns valid or flattens structure errors without these details.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Extend `DraftDiagnostic`**
|
||||||
|
|
||||||
|
In `src/wf_artifacts/drafts/api.py`, change `DraftDiagnostic` to:
|
||||||
|
|
||||||
|
```python
|
||||||
|
class DraftDiagnostic(BaseModel):
|
||||||
|
"""Machine-readable reason a keyed draft could not be compiled."""
|
||||||
|
|
||||||
|
code: str
|
||||||
|
path: str
|
||||||
|
step_id: str | None = None
|
||||||
|
message: str
|
||||||
|
repair_hint: str | None = None
|
||||||
|
details: dict[str, Any] = {}
|
||||||
|
```
|
||||||
|
|
||||||
|
Keep `details` as a plain JSON object. Do not add a specific typed model yet; draft diagnostics cover several validation families.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Add core issue conversion helpers**
|
||||||
|
|
||||||
|
In `src/wf_artifacts/drafts/api.py`, add imports:
|
||||||
|
|
||||||
|
```python
|
||||||
|
import re
|
||||||
|
|
||||||
|
from wf_core.models.steps import OutputBinding
|
||||||
|
from wf_core.models.workflow import Workflow
|
||||||
|
from wf_core.validation.issues import ValidationIssue, ValidationIssueCode
|
||||||
|
```
|
||||||
|
|
||||||
|
Add these helpers after `_diagnostic_from_exception`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
_NODE_OUTPUT_TARGET_RE = re.compile(r"^nodes\[(?P<node_index>\d+)\]\.output\[(?P<output_index>\d+)\]\.target$")
|
||||||
|
|
||||||
|
|
||||||
|
def _diagnostics_from_workflow_issues(workflow: Workflow) -> list[DraftDiagnostic]:
|
||||||
|
report = workflow.validate_structure()
|
||||||
|
return [_diagnostic_from_issue(workflow, issue) for issue in report.errors]
|
||||||
|
|
||||||
|
|
||||||
|
def _diagnostic_from_issue(
|
||||||
|
workflow: Workflow,
|
||||||
|
issue: ValidationIssue,
|
||||||
|
) -> DraftDiagnostic:
|
||||||
|
step_id = _step_id_for_issue_path(workflow, issue.path)
|
||||||
|
return DraftDiagnostic(
|
||||||
|
code=str(issue.code),
|
||||||
|
path=issue.path,
|
||||||
|
step_id=step_id,
|
||||||
|
message=issue.message,
|
||||||
|
details=_details_for_issue(workflow, issue),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _step_id_for_issue_path(workflow: Workflow, path: str) -> str | None:
|
||||||
|
match = re.match(r"^nodes\[(?P<node_index>\d+)\]", path)
|
||||||
|
if match is None:
|
||||||
|
return None
|
||||||
|
node_index = int(match.group("node_index"))
|
||||||
|
if node_index >= len(workflow.nodes):
|
||||||
|
return None
|
||||||
|
node_id = getattr(workflow.nodes[node_index], "id", None)
|
||||||
|
return node_id if isinstance(node_id, str) else None
|
||||||
|
|
||||||
|
|
||||||
|
def _details_for_issue(
|
||||||
|
workflow: Workflow,
|
||||||
|
issue: ValidationIssue,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
if issue.code is not ValidationIssueCode.INVALID_DESTINATION_PATH:
|
||||||
|
return {}
|
||||||
|
match = _NODE_OUTPUT_TARGET_RE.match(issue.path)
|
||||||
|
if match is None:
|
||||||
|
return {}
|
||||||
|
node_index = int(match.group("node_index"))
|
||||||
|
output_index = int(match.group("output_index"))
|
||||||
|
if node_index >= len(workflow.nodes):
|
||||||
|
return {}
|
||||||
|
outputs = getattr(workflow.nodes[node_index], "output", None)
|
||||||
|
if not isinstance(outputs, list) or output_index >= len(outputs):
|
||||||
|
return {}
|
||||||
|
binding = outputs[output_index]
|
||||||
|
if not isinstance(binding, OutputBinding):
|
||||||
|
return {}
|
||||||
|
output_field = _single_local_field(binding)
|
||||||
|
if output_field is None:
|
||||||
|
return {}
|
||||||
|
return {
|
||||||
|
"output_field": output_field,
|
||||||
|
"state_path": str(binding.target),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _single_local_field(binding: OutputBinding) -> str | None:
|
||||||
|
parts = getattr(binding.source, "parts", None)
|
||||||
|
root = getattr(binding.source, "root", None)
|
||||||
|
if root != "local" or not isinstance(parts, list) or len(parts) != 1:
|
||||||
|
return None
|
||||||
|
field = parts[0]
|
||||||
|
return field if isinstance(field, str) else None
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Validate compiled workflow structure**
|
||||||
|
|
||||||
|
In `validate_workflow_draft`, replace:
|
||||||
|
|
||||||
|
```python
|
||||||
|
try:
|
||||||
|
compiled_plan = compile_workflow_draft(draft)
|
||||||
|
except (ValidationError, KeyError, ValueError) as exc:
|
||||||
|
return _invalid_result(_diagnostic_from_exception(exc))
|
||||||
|
```
|
||||||
|
|
||||||
|
with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
try:
|
||||||
|
parsed = WorkflowDraft.model_validate(draft)
|
||||||
|
workflow = build_workflow_from_draft(parsed)
|
||||||
|
except (ValidationError, KeyError, ValueError) as exc:
|
||||||
|
return _invalid_result(_diagnostic_from_exception(exc))
|
||||||
|
diagnostics = _diagnostics_from_workflow_issues(workflow)
|
||||||
|
if diagnostics:
|
||||||
|
return _invalid_result(*diagnostics)
|
||||||
|
compiled_plan = workflow.model_dump(mode="json", by_alias=True, exclude={"node_defs"})
|
||||||
|
```
|
||||||
|
|
||||||
|
Then update `_invalid_result` to accept multiple diagnostics:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def _invalid_result(*diagnostics: DraftDiagnostic) -> JsonObject:
|
||||||
|
return {
|
||||||
|
"status": "invalid",
|
||||||
|
"diagnostics": [item.model_dump(mode="json", exclude_none=True) for item in diagnostics],
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Do not change `compile_workflow_draft`; it should remain a compiler that raises on model/adapter failures and returns raw workflow JSON.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run low-level tests to verify green**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
uv run pytest tests/artifacts/test_draft_adapter.py -q -k "structured_output_destination_issue or validate_workflow_draft"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: selected tests pass.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Commit**
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
git add src/wf_artifacts/drafts/api.py tests/artifacts/test_draft_adapter.py
|
||||||
|
git commit -m "feat: expose structured draft validation issues"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: Add Workspace-Aware Repair Hints In Draft API
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- Modify: `src/wf_api/drafts.py`
|
||||||
|
- Modify: `tests/wf_api/test_drafts_service.py`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write failing repair hint test**
|
||||||
|
|
||||||
|
In `tests/wf_api/test_drafts_service.py`, add this test near `test_validate_draft_workspace_refreshes_status`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_validate_draft_workspace_suggests_bind_output_to_state(
|
||||||
|
tmp_path: Path,
|
||||||
|
) -> None:
|
||||||
|
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_repair_hint")
|
||||||
|
api, service = _draft_api(artifact_store)
|
||||||
|
service.register_connection(
|
||||||
|
ConnectionConfig(id="demo.personal", server="demo", account="personal")
|
||||||
|
)
|
||||||
|
service.register_specs("demo.personal", _snapshot_tool)
|
||||||
|
await api.create_draft_workspace(
|
||||||
|
workspace_id="snapshot_ws",
|
||||||
|
draft={
|
||||||
|
"name": "snapshot",
|
||||||
|
"input_schema": {"type": "object", "properties": {}},
|
||||||
|
"state_schema": {"type": "object", "properties": {}},
|
||||||
|
"output_schema": {"type": "object", "properties": {}},
|
||||||
|
"start": "snap",
|
||||||
|
"steps": {
|
||||||
|
"snap": {
|
||||||
|
"use": "demo.personal.snapshot_tool",
|
||||||
|
"input": [],
|
||||||
|
"output": [
|
||||||
|
{
|
||||||
|
"source": {"root": "local", "parts": ["after"]},
|
||||||
|
"target": {"root": "state", "parts": ["after"]},
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"routes": {"snap": {"ok": "__end__"}},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
payload = await api.validate_draft_workspace(workspace_id="snapshot_ws")
|
||||||
|
|
||||||
|
diagnostic = payload["diagnostics"][0]
|
||||||
|
assert diagnostic["code"] == "invalid_destination_path"
|
||||||
|
assert diagnostic["step_id"] == "snap"
|
||||||
|
assert diagnostic["repair_hint"] == (
|
||||||
|
"wf draft bind-output-to-state snapshot_ws --revision 1 "
|
||||||
|
"--step snap --output after --state state.after"
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run test to verify red**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
uv run pytest tests/wf_api/test_drafts_service.py -q -k "suggests_bind_output_to_state"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: fail because `repair_hint` is missing.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add repair hint enrichment helpers**
|
||||||
|
|
||||||
|
In `src/wf_api/drafts.py`, add this helper near `_state_root_field`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def _with_workspace_repair_hints(
|
||||||
|
payload: dict[str, Any],
|
||||||
|
*,
|
||||||
|
workspace_id: str,
|
||||||
|
revision: int,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
diagnostics = payload.get("diagnostics")
|
||||||
|
if not isinstance(diagnostics, list):
|
||||||
|
return payload
|
||||||
|
enriched = []
|
||||||
|
changed = False
|
||||||
|
for diagnostic in diagnostics:
|
||||||
|
if not isinstance(diagnostic, dict):
|
||||||
|
enriched.append(diagnostic)
|
||||||
|
continue
|
||||||
|
repaired = dict(diagnostic)
|
||||||
|
hint = _draft_repair_hint(
|
||||||
|
repaired,
|
||||||
|
workspace_id=workspace_id,
|
||||||
|
revision=revision,
|
||||||
|
)
|
||||||
|
if hint is not None:
|
||||||
|
repaired["repair_hint"] = hint
|
||||||
|
changed = True
|
||||||
|
enriched.append(repaired)
|
||||||
|
if not changed:
|
||||||
|
return payload
|
||||||
|
return {**payload, "diagnostics": enriched}
|
||||||
|
|
||||||
|
|
||||||
|
def _draft_repair_hint(
|
||||||
|
diagnostic: Mapping[str, Any],
|
||||||
|
*,
|
||||||
|
workspace_id: str,
|
||||||
|
revision: int,
|
||||||
|
) -> str | None:
|
||||||
|
if diagnostic.get("code") != "invalid_destination_path":
|
||||||
|
return None
|
||||||
|
step_id = diagnostic.get("step_id")
|
||||||
|
details = diagnostic.get("details")
|
||||||
|
if not isinstance(step_id, str) or not isinstance(details, dict):
|
||||||
|
return None
|
||||||
|
output_field = details.get("output_field")
|
||||||
|
state_path = details.get("state_path")
|
||||||
|
if not isinstance(output_field, str) or not isinstance(state_path, str):
|
||||||
|
return None
|
||||||
|
return (
|
||||||
|
f"wf draft bind-output-to-state {workspace_id} --revision {revision} "
|
||||||
|
f"--step {step_id} --output {output_field} --state {state_path}"
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
This helper belongs in `wf_api`, not `wf_artifacts`, because command names and workspace revisions are product-surface concerns.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Enrich `validate_draft_workspace` diagnostics before saving**
|
||||||
|
|
||||||
|
In `WorkflowDraftApi.validate_draft_workspace`, replace:
|
||||||
|
|
||||||
|
```python
|
||||||
|
validation = await self.validate_draft(draft=workspace.draft)
|
||||||
|
```
|
||||||
|
|
||||||
|
with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
validation = _with_workspace_repair_hints(
|
||||||
|
await self.validate_draft(draft=workspace.draft),
|
||||||
|
workspace_id=workspace_id,
|
||||||
|
revision=workspace.revision,
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
This stores enriched diagnostics in the workspace summary so repeated inspection shows the same hint.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run API tests to verify green**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
uv run pytest tests/wf_api/test_drafts_service.py -q -k "suggests_bind_output_to_state or validate_draft_workspace"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: selected tests pass.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Commit**
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
git add src/wf_api/drafts.py tests/wf_api/test_drafts_service.py
|
||||||
|
git commit -m "feat: add draft validation repair hints"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: CLI/RPC Smoke And Docs
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- Modify: `tests/wf_cli/test_remote_target.py`
|
||||||
|
- Modify: `docs/wf_cli.md`
|
||||||
|
- Modify: `skills/wf-cli/SKILL.md`
|
||||||
|
- Modify: `skills/wf-workflow/references/draft-workspaces.md`
|
||||||
|
- Modify: `docs/current_roadmap.md`
|
||||||
|
- Move: `docs/superpowers/plans/2026-06-26-draft-validation-repair-hints.md` to `docs/historical/superpowers/plans/2026-06-26-draft-validation-repair-hints.md`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Extend CLI/RPC draft validation smoke test**
|
||||||
|
|
||||||
|
In `tests/wf_cli/test_remote_target.py`, update
|
||||||
|
`test_wf_remote_draft_artifact_deploy_lifecycle`. After the existing successful
|
||||||
|
`validated` assertion block, add a second invalid draft workspace:
|
||||||
|
|
||||||
|
```python
|
||||||
|
invalid_created = runner.invoke(
|
||||||
|
app,
|
||||||
|
[
|
||||||
|
*base_args,
|
||||||
|
"draft",
|
||||||
|
"create-from-capability",
|
||||||
|
"repair_ws",
|
||||||
|
"wf.std.constant",
|
||||||
|
"--name",
|
||||||
|
"repair_constant",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
assert invalid_created.exit_code == 0, invalid_created.output
|
||||||
|
|
||||||
|
invalid_patch = runner.invoke(
|
||||||
|
app,
|
||||||
|
[
|
||||||
|
*base_args,
|
||||||
|
"draft",
|
||||||
|
"set-output",
|
||||||
|
"repair_ws",
|
||||||
|
"--revision",
|
||||||
|
"1",
|
||||||
|
"--step",
|
||||||
|
"call",
|
||||||
|
"--map",
|
||||||
|
"value=state.missing",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
assert invalid_patch.exit_code == 0, invalid_patch.output
|
||||||
|
|
||||||
|
invalid_validated = runner.invoke(
|
||||||
|
app,
|
||||||
|
[*base_args, "draft", "validate", "repair_ws"],
|
||||||
|
)
|
||||||
|
assert invalid_validated.exit_code == 0, invalid_validated.output
|
||||||
|
assert '"status": "invalid"' in invalid_validated.output
|
||||||
|
assert "bind-output-to-state repair_ws --revision 2" in invalid_validated.output
|
||||||
|
assert "--step call --output value --state state.missing" in invalid_validated.output
|
||||||
|
```
|
||||||
|
|
||||||
|
This test uses the existing `_patch_rpc_client_to_server` setup already present
|
||||||
|
in `test_wf_remote_draft_artifact_deploy_lifecycle`, so it proves the CLI
|
||||||
|
preserves the repair hint returned by the RPC-backed API path.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run CLI smoke test**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
uv run pytest tests/wf_cli/test_remote_target.py -q -k "repair_hints or draft_validate"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: pass.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Update CLI docs**
|
||||||
|
|
||||||
|
In `docs/wf_cli.md`, near draft validation docs, add:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
Draft validation diagnostics may include `repair_hint` commands. Treat these as
|
||||||
|
the next focused command to try, not as proof that the draft is fixed. Re-run
|
||||||
|
`wf draft validate <workspace_id>` after applying a hint.
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Update skills**
|
||||||
|
|
||||||
|
In `skills/wf-cli/SKILL.md`, add under draft rules:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
When `wf draft validate` returns a `repair_hint`, prefer running that focused
|
||||||
|
command before writing JSON Patch manually. Re-run `wf draft validate` after the
|
||||||
|
repair.
|
||||||
|
```
|
||||||
|
|
||||||
|
In `skills/wf-workflow/references/draft-workspaces.md`, add:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
Validation repair hints are product guidance. If a diagnostic suggests
|
||||||
|
`bind-output-to-state`, use it before hand-editing `state_schema` or step output
|
||||||
|
bindings.
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Update roadmap**
|
||||||
|
|
||||||
|
In `docs/current_roadmap.md`, add:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
- Completed: draft validation now preserves structured core validation issues
|
||||||
|
and adds exact `bind-output-to-state` repair hints for missing state fields.
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Move plan to historical**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
Move-Item docs\superpowers\plans\2026-06-26-draft-validation-repair-hints.md docs\historical\superpowers\plans\2026-06-26-draft-validation-repair-hints.md
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 7: Run final verification**
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
uv run pytest tests/artifacts/test_draft_adapter.py tests/wf_api/test_drafts_service.py tests/wf_cli/test_remote_target.py -q -k "structured_output_destination_issue or suggests_bind_output_to_state or repair_hints or validate_draft_workspace"
|
||||||
|
uv run ruff check src/wf_artifacts/drafts/api.py src/wf_api/drafts.py tests/artifacts/test_draft_adapter.py tests/wf_api/test_drafts_service.py tests/wf_cli/test_remote_target.py
|
||||||
|
uv run basedpyright --level error src/wf_artifacts/drafts/api.py src/wf_api/drafts.py tests/artifacts/test_draft_adapter.py tests/wf_api/test_drafts_service.py tests/wf_cli/test_remote_target.py
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected:
|
||||||
|
|
||||||
|
- Focused tests pass.
|
||||||
|
- Ruff reports `All checks passed!`.
|
||||||
|
- Basedpyright reports `0 errors`.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit**
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
git add -A
|
||||||
|
git commit -m "docs: record draft validation repair hints"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-Review Notes
|
||||||
|
|
||||||
|
- This plan intentionally returns structured validation issues before generating repair hints. Regex-parsing flattened exception text would be brittle and would hide useful issue codes.
|
||||||
|
- Exact CLI repair hints are added in `wf_api` because only that layer knows the workspace id and revision.
|
||||||
|
- The first repair hint only targets `invalid_destination_path` for node output bindings with a single top-level local output field.
|
||||||
|
- This plan does not change `bind-output-to-state`; it only makes validation point agents toward it.
|
||||||
@@ -345,6 +345,10 @@ Validate:
|
|||||||
wf draft validate concat_ws
|
wf draft validate concat_ws
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Draft validation diagnostics may include `repair_hint` commands. Treat these as
|
||||||
|
the next focused command to try, not as proof that the draft is fixed. Re-run
|
||||||
|
`wf draft validate <workspace_id>` after applying a hint.
|
||||||
|
|
||||||
Delete a draft workspace:
|
Delete a draft workspace:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|||||||
@@ -49,6 +49,10 @@ wf draft bind-output-to-state <workspace_id> --revision <n> --step <step_id> --o
|
|||||||
wf draft validate <workspace_id>
|
wf draft validate <workspace_id>
|
||||||
wf draft save <workspace_id> --artifact <artifact_id> --version <n> --title <title>
|
wf draft save <workspace_id> --artifact <artifact_id> --version <n> --title <title>
|
||||||
|
|
||||||
|
When `wf draft validate` returns a `repair_hint`, prefer running that focused
|
||||||
|
command before writing JSON Patch manually. Re-run `wf draft validate` after the
|
||||||
|
repair.
|
||||||
|
|
||||||
wf artifact create-from-plan workflow.plan.json --artifact <artifact_id> --version <n> --title <title>
|
wf artifact create-from-plan workflow.plan.json --artifact <artifact_id> --version <n> --title <title>
|
||||||
wf deploy save <deployment_id> --artifact <artifact_id> --version <n> --binding <logical>=<concrete>
|
wf deploy save <deployment_id> --artifact <artifact_id> --version <n> --binding <logical>=<concrete>
|
||||||
wf deploy create <deployment_id> --artifact <artifact_id> --version <n>
|
wf deploy create <deployment_id> --artifact <artifact_id> --version <n>
|
||||||
|
|||||||
@@ -118,6 +118,10 @@ wf draft bind-output-to-state <workspace_id> --revision <n> --step <step_id> --o
|
|||||||
wf draft validate <workspace_id>
|
wf draft validate <workspace_id>
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Validation repair hints are product guidance. If a diagnostic suggests
|
||||||
|
`bind-output-to-state`, use it before hand-editing `state_schema` or step output
|
||||||
|
bindings.
|
||||||
|
|
||||||
Use JSON Patch for structural edits the helpers do not cover.
|
Use JSON Patch for structural edits the helpers do not cover.
|
||||||
|
|
||||||
For larger patches, write a JSON Patch array to a file and pass it with
|
For larger patches, write a JSON Patch array to a file and pass it with
|
||||||
|
|||||||
+88
-3
@@ -18,6 +18,7 @@ from wf_artifacts import (
|
|||||||
from wf_artifacts import (
|
from wf_artifacts import (
|
||||||
patch_draft_workspace as patch_draft_workspace_record,
|
patch_draft_workspace as patch_draft_workspace_record,
|
||||||
)
|
)
|
||||||
|
from wf_core.models.schemas import NodeDef
|
||||||
from wf_core.models.steps import (
|
from wf_core.models.steps import (
|
||||||
InputBinding,
|
InputBinding,
|
||||||
InputPathBinding,
|
InputPathBinding,
|
||||||
@@ -65,10 +66,32 @@ class WorkflowDraftApi:
|
|||||||
outcomes = getattr(spec, "outcomes", None)
|
outcomes = getattr(spec, "outcomes", None)
|
||||||
return tuple(outcomes) if outcomes is not None else None
|
return tuple(outcomes) if outcomes is not None else None
|
||||||
|
|
||||||
|
def _node_defs_for_draft(self, draft: dict[str, Any]) -> list[NodeDef]:
|
||||||
|
"""Derive node defs from context specs for each use step in the draft."""
|
||||||
|
steps = draft.get("steps")
|
||||||
|
if not isinstance(steps, dict):
|
||||||
|
return []
|
||||||
|
node_defs = []
|
||||||
|
seen = set()
|
||||||
|
for step in steps.values():
|
||||||
|
if not isinstance(step, dict):
|
||||||
|
continue
|
||||||
|
capability = step.get("use")
|
||||||
|
if not isinstance(capability, str) or capability in seen:
|
||||||
|
continue
|
||||||
|
seen.add(capability)
|
||||||
|
try:
|
||||||
|
spec = self.context.specs.get_qualified_spec(capability)
|
||||||
|
except KeyError:
|
||||||
|
continue
|
||||||
|
node_defs.append(spec.to_node_def())
|
||||||
|
return node_defs
|
||||||
|
|
||||||
async def validate_draft(self, *, draft: dict[str, Any]) -> dict[str, Any]:
|
async def validate_draft(self, *, draft: dict[str, Any]) -> dict[str, Any]:
|
||||||
return validate_workflow_draft(
|
return validate_workflow_draft(
|
||||||
draft,
|
draft,
|
||||||
outcome_lookup=self._outcomes_for_capability,
|
outcome_lookup=self._outcomes_for_capability,
|
||||||
|
node_defs=self._node_defs_for_draft(draft),
|
||||||
)
|
)
|
||||||
|
|
||||||
async def compile_draft(self, *, draft: dict[str, Any]) -> dict[str, Any]:
|
async def compile_draft(self, *, draft: dict[str, Any]) -> dict[str, Any]:
|
||||||
@@ -90,7 +113,11 @@ class WorkflowDraftApi:
|
|||||||
draft: dict[str, Any],
|
draft: dict[str, Any],
|
||||||
patch: list[dict[str, Any]],
|
patch: list[dict[str, Any]],
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
return patch_workflow_draft(draft, patch)
|
return patch_workflow_draft(
|
||||||
|
draft,
|
||||||
|
patch,
|
||||||
|
node_defs_for_draft=self._node_defs_for_draft,
|
||||||
|
)
|
||||||
|
|
||||||
async def list_draft_workspaces(self) -> dict[str, Any]:
|
async def list_draft_workspaces(self) -> dict[str, Any]:
|
||||||
"""Return compact summaries for stored draft workspaces."""
|
"""Return compact summaries for stored draft workspaces."""
|
||||||
@@ -140,7 +167,11 @@ class WorkflowDraftApi:
|
|||||||
"""Refresh stored validation status without changing draft revision."""
|
"""Refresh stored validation status without changing draft revision."""
|
||||||
store = self._draft_store()
|
store = self._draft_store()
|
||||||
workspace = store.get_workspace(workspace_id)
|
workspace = store.get_workspace(workspace_id)
|
||||||
validation = await self.validate_draft(draft=workspace.draft)
|
validation = _with_workspace_repair_hints(
|
||||||
|
await self.validate_draft(draft=workspace.draft),
|
||||||
|
workspace_id=workspace_id,
|
||||||
|
revision=workspace.revision,
|
||||||
|
)
|
||||||
refreshed = workspace.model_copy(
|
refreshed = workspace.model_copy(
|
||||||
update={
|
update={
|
||||||
"status": validation["status"],
|
"status": validation["status"],
|
||||||
@@ -157,11 +188,13 @@ class WorkflowDraftApi:
|
|||||||
revision: int,
|
revision: int,
|
||||||
patch: list[dict[str, Any]],
|
patch: list[dict[str, Any]],
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
|
store = self._draft_store()
|
||||||
return patch_draft_workspace_record(
|
return patch_draft_workspace_record(
|
||||||
self._draft_store(),
|
store,
|
||||||
workspace_id=workspace_id,
|
workspace_id=workspace_id,
|
||||||
revision=revision,
|
revision=revision,
|
||||||
patch=patch,
|
patch=patch,
|
||||||
|
node_defs_for_draft=self._node_defs_for_draft,
|
||||||
)
|
)
|
||||||
|
|
||||||
async def set_draft_name(
|
async def set_draft_name(
|
||||||
@@ -589,6 +622,58 @@ def _escape_json_pointer(value: str) -> str:
|
|||||||
return value.replace("~", "~0").replace("/", "~1")
|
return value.replace("~", "~0").replace("/", "~1")
|
||||||
|
|
||||||
|
|
||||||
|
def _with_workspace_repair_hints(
|
||||||
|
payload: dict[str, Any],
|
||||||
|
*,
|
||||||
|
workspace_id: str,
|
||||||
|
revision: int,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
diagnostics = payload.get("diagnostics")
|
||||||
|
if not isinstance(diagnostics, list):
|
||||||
|
return payload
|
||||||
|
enriched = []
|
||||||
|
changed = False
|
||||||
|
for diagnostic in diagnostics:
|
||||||
|
if not isinstance(diagnostic, dict):
|
||||||
|
enriched.append(diagnostic)
|
||||||
|
continue
|
||||||
|
repaired = dict(diagnostic)
|
||||||
|
hint = _draft_repair_hint(
|
||||||
|
repaired,
|
||||||
|
workspace_id=workspace_id,
|
||||||
|
revision=revision,
|
||||||
|
)
|
||||||
|
if hint is not None:
|
||||||
|
repaired["repair_hint"] = hint
|
||||||
|
changed = True
|
||||||
|
enriched.append(repaired)
|
||||||
|
if not changed:
|
||||||
|
return payload
|
||||||
|
return {**payload, "diagnostics": enriched}
|
||||||
|
|
||||||
|
|
||||||
|
def _draft_repair_hint(
|
||||||
|
diagnostic: Mapping[str, Any],
|
||||||
|
*,
|
||||||
|
workspace_id: str,
|
||||||
|
revision: int,
|
||||||
|
) -> str | None:
|
||||||
|
if diagnostic.get("code") != "invalid_destination_path":
|
||||||
|
return None
|
||||||
|
step_id = diagnostic.get("step_id")
|
||||||
|
details = diagnostic.get("details")
|
||||||
|
if not isinstance(step_id, str) or not isinstance(details, dict):
|
||||||
|
return None
|
||||||
|
output_field = details.get("output_field")
|
||||||
|
state_path = details.get("state_path")
|
||||||
|
if not isinstance(output_field, str) or not isinstance(state_path, str):
|
||||||
|
return None
|
||||||
|
return (
|
||||||
|
f"wf draft bind-output-to-state {workspace_id} --revision {revision} "
|
||||||
|
f"--step {step_id} --output {output_field} --state {state_path}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _state_root_field(value: str) -> str:
|
def _state_root_field(value: str) -> str:
|
||||||
path = StatePath.parse(value)
|
path = StatePath.parse(value)
|
||||||
if len(path.parts) != 1:
|
if len(path.parts) != 1:
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import time
|
import time
|
||||||
|
from collections.abc import Callable, Sequence
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from wf_artifacts.drafts import (
|
from wf_artifacts.drafts import (
|
||||||
@@ -8,12 +9,14 @@ from wf_artifacts.drafts import (
|
|||||||
patch_workflow_draft,
|
patch_workflow_draft,
|
||||||
validate_workflow_draft,
|
validate_workflow_draft,
|
||||||
)
|
)
|
||||||
|
from wf_core.models.schemas import NodeDef
|
||||||
|
|
||||||
from .models import WorkflowDraftWorkspace, summarize_draft_workspace
|
from .models import WorkflowDraftWorkspace, summarize_draft_workspace
|
||||||
from .store import DraftWorkspaceConflictError, DraftWorkspaceStore
|
from .store import DraftWorkspaceConflictError, DraftWorkspaceStore
|
||||||
|
|
||||||
JsonObject = dict[str, Any]
|
JsonObject = dict[str, Any]
|
||||||
JsonPatch = list[dict[str, Any]]
|
JsonPatch = list[dict[str, Any]]
|
||||||
|
NodeDefsForDraft = Callable[[JsonObject], Sequence[NodeDef]]
|
||||||
|
|
||||||
|
|
||||||
def create_draft_workspace(
|
def create_draft_workspace(
|
||||||
@@ -56,12 +59,19 @@ def patch_draft_workspace(
|
|||||||
workspace_id: str,
|
workspace_id: str,
|
||||||
revision: int,
|
revision: int,
|
||||||
patch: JsonPatch,
|
patch: JsonPatch,
|
||||||
|
node_defs: Sequence[NodeDef] | None = None,
|
||||||
|
node_defs_for_draft: NodeDefsForDraft | None = None,
|
||||||
) -> JsonObject:
|
) -> JsonObject:
|
||||||
"""Apply JSON Patch to a stored workspace when the revision matches."""
|
"""Apply JSON Patch to a stored workspace when the revision matches."""
|
||||||
workspace = store.get_workspace(workspace_id)
|
workspace = store.get_workspace(workspace_id)
|
||||||
if workspace.revision != revision:
|
if workspace.revision != revision:
|
||||||
return _revision_conflict_payload(workspace, revision)
|
return _revision_conflict_payload(workspace, revision)
|
||||||
patched = patch_workflow_draft(workspace.draft, patch)
|
patched = patch_workflow_draft(
|
||||||
|
workspace.draft,
|
||||||
|
patch,
|
||||||
|
node_defs=node_defs,
|
||||||
|
node_defs_for_draft=node_defs_for_draft,
|
||||||
|
)
|
||||||
if "draft" not in patched:
|
if "draft" not in patched:
|
||||||
# A malformed JSON Patch is not a draft revision. Return diagnostics
|
# A malformed JSON Patch is not a draft revision. Return diagnostics
|
||||||
# without mutating the stored workspace or burning a revision number.
|
# without mutating the stored workspace or burning a revision number.
|
||||||
|
|||||||
@@ -1,11 +1,17 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from collections.abc import Callable
|
import re
|
||||||
|
from collections.abc import Callable, Sequence
|
||||||
from copy import deepcopy
|
from copy import deepcopy
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import jsonpatch
|
import jsonpatch
|
||||||
from pydantic import BaseModel, ValidationError
|
from pydantic import BaseModel, Field, ValidationError
|
||||||
|
|
||||||
|
from wf_core.models.schemas import NodeDef
|
||||||
|
from wf_core.models.steps import OutputBinding
|
||||||
|
from wf_core.models.workflow import Workflow
|
||||||
|
from wf_core.validation.issues import ValidationIssue, ValidationIssueCode
|
||||||
|
|
||||||
from .adapter import build_workflow_from_draft
|
from .adapter import build_workflow_from_draft
|
||||||
from .models import WorkflowDraft
|
from .models import WorkflowDraft
|
||||||
@@ -13,6 +19,7 @@ from .models import WorkflowDraft
|
|||||||
JsonObject = dict[str, Any]
|
JsonObject = dict[str, Any]
|
||||||
JsonPatch = list[dict[str, Any]]
|
JsonPatch = list[dict[str, Any]]
|
||||||
OutcomeLookup = Callable[[str], tuple[str, ...] | None]
|
OutcomeLookup = Callable[[str], tuple[str, ...] | None]
|
||||||
|
NodeDefsForDraft = Callable[[JsonObject], Sequence[NodeDef]]
|
||||||
|
|
||||||
|
|
||||||
class DraftDiagnostic(BaseModel):
|
class DraftDiagnostic(BaseModel):
|
||||||
@@ -22,6 +29,8 @@ class DraftDiagnostic(BaseModel):
|
|||||||
path: str
|
path: str
|
||||||
step_id: str | None = None
|
step_id: str | None = None
|
||||||
message: str
|
message: str
|
||||||
|
repair_hint: str | None = None
|
||||||
|
details: dict[str, Any] = Field(default_factory=dict)
|
||||||
|
|
||||||
|
|
||||||
def compile_workflow_draft(draft: JsonObject) -> JsonObject:
|
def compile_workflow_draft(draft: JsonObject) -> JsonObject:
|
||||||
@@ -35,16 +44,31 @@ def validate_workflow_draft(
|
|||||||
draft: JsonObject,
|
draft: JsonObject,
|
||||||
*,
|
*,
|
||||||
outcome_lookup: OutcomeLookup | None = None,
|
outcome_lookup: OutcomeLookup | None = None,
|
||||||
|
node_defs: Sequence[NodeDef] | None = None,
|
||||||
) -> JsonObject:
|
) -> JsonObject:
|
||||||
"""Return structured diagnostics instead of raising on a bad keyed draft."""
|
"""Return structured diagnostics instead of raising on a bad keyed draft.
|
||||||
|
|
||||||
|
When *node_defs* is supplied the structural validator can check
|
||||||
|
input/output bindings against declared node schemas. Without them
|
||||||
|
only draft-level parse errors are reported.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
compiled_plan = compile_workflow_draft(draft)
|
parsed = WorkflowDraft.model_validate(draft)
|
||||||
|
workflow = build_workflow_from_draft(parsed)
|
||||||
except (ValidationError, KeyError, ValueError) as exc:
|
except (ValidationError, KeyError, ValueError) as exc:
|
||||||
return _invalid_result(_diagnostic_from_exception(exc))
|
return _invalid_result(_diagnostic_from_exception(exc))
|
||||||
|
if node_defs is not None:
|
||||||
|
workflow = workflow.model_copy(update={"node_defs": list(node_defs)})
|
||||||
|
diagnostics = _diagnostics_from_workflow_issues(workflow)
|
||||||
|
if diagnostics:
|
||||||
|
return _invalid_result(*diagnostics)
|
||||||
if outcome_lookup is not None:
|
if outcome_lookup is not None:
|
||||||
diagnostic = _validate_known_outcomes(draft, outcome_lookup)
|
diagnostic = _validate_known_outcomes(draft, outcome_lookup)
|
||||||
if diagnostic is not None:
|
if diagnostic is not None:
|
||||||
return _invalid_result(diagnostic)
|
return _invalid_result(diagnostic)
|
||||||
|
compiled_plan = workflow.model_dump(
|
||||||
|
mode="json", by_alias=True, exclude={"node_defs"}
|
||||||
|
)
|
||||||
return {
|
return {
|
||||||
"status": "valid",
|
"status": "valid",
|
||||||
"diagnostics": [],
|
"diagnostics": [],
|
||||||
@@ -52,7 +76,13 @@ def validate_workflow_draft(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
def patch_workflow_draft(draft: JsonObject, patch: JsonPatch) -> JsonObject:
|
def patch_workflow_draft(
|
||||||
|
draft: JsonObject,
|
||||||
|
patch: JsonPatch,
|
||||||
|
*,
|
||||||
|
node_defs: Sequence[NodeDef] | None = None,
|
||||||
|
node_defs_for_draft: NodeDefsForDraft | None = None,
|
||||||
|
) -> JsonObject:
|
||||||
"""Patch the draft source document, then validate the patched result."""
|
"""Patch the draft source document, then validate the patched result."""
|
||||||
try:
|
try:
|
||||||
patched = jsonpatch.JsonPatch(patch).apply(deepcopy(draft), in_place=False)
|
patched = jsonpatch.JsonPatch(patch).apply(deepcopy(draft), in_place=False)
|
||||||
@@ -72,16 +102,21 @@ def patch_workflow_draft(draft: JsonObject, patch: JsonPatch) -> JsonObject:
|
|||||||
message="patched draft must be a JSON object",
|
message="patched draft must be a JSON object",
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
result = validate_workflow_draft(patched)
|
effective_node_defs = (
|
||||||
|
node_defs_for_draft(patched) if node_defs_for_draft is not None else node_defs
|
||||||
|
)
|
||||||
|
result = validate_workflow_draft(patched, node_defs=effective_node_defs)
|
||||||
if result["status"] == "valid":
|
if result["status"] == "valid":
|
||||||
patched = WorkflowDraft.model_validate(patched).model_dump(mode="json")
|
patched = WorkflowDraft.model_validate(patched).model_dump(mode="json")
|
||||||
return {"draft": patched, **result}
|
return {"draft": patched, **result}
|
||||||
|
|
||||||
|
|
||||||
def _invalid_result(diagnostic: DraftDiagnostic) -> JsonObject:
|
def _invalid_result(*diagnostics: DraftDiagnostic) -> JsonObject:
|
||||||
return {
|
return {
|
||||||
"status": "invalid",
|
"status": "invalid",
|
||||||
"diagnostics": [diagnostic.model_dump(mode="json")],
|
"diagnostics": [
|
||||||
|
item.model_dump(mode="json", exclude_none=True) for item in diagnostics
|
||||||
|
],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -104,6 +139,82 @@ def _format_location(location: tuple[object, ...]) -> str:
|
|||||||
return ".".join(str(part) for part in location)
|
return ".".join(str(part) for part in location)
|
||||||
|
|
||||||
|
|
||||||
|
_NODE_OUTPUT_TARGET_RE = re.compile(
|
||||||
|
r"^nodes\[(?P<node_index>\d+)\]\.output\[(?P<output_index>\d+)\]\.target$"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _diagnostics_from_workflow_issues(workflow: Workflow) -> list[DraftDiagnostic]:
|
||||||
|
report = workflow.validate_structure()
|
||||||
|
return [_diagnostic_from_issue(workflow, issue) for issue in report.errors]
|
||||||
|
|
||||||
|
|
||||||
|
def _diagnostic_from_issue(
|
||||||
|
workflow: Workflow,
|
||||||
|
issue: ValidationIssue,
|
||||||
|
) -> DraftDiagnostic:
|
||||||
|
step_id = _step_id_for_issue_path(workflow, issue.path)
|
||||||
|
return DraftDiagnostic(
|
||||||
|
code=str(issue.code),
|
||||||
|
path=issue.path,
|
||||||
|
step_id=step_id,
|
||||||
|
message=issue.message,
|
||||||
|
details=_details_for_issue(workflow, issue),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _step_id_for_issue_path(workflow: Workflow, path: str) -> str | None:
|
||||||
|
match = re.match(r"^nodes\[(?P<node_index>\d+)\]", path)
|
||||||
|
if match is None:
|
||||||
|
return None
|
||||||
|
node_index = int(match.group("node_index"))
|
||||||
|
if node_index >= len(workflow.nodes):
|
||||||
|
return None
|
||||||
|
node_id = getattr(workflow.nodes[node_index], "id", None)
|
||||||
|
return node_id if isinstance(node_id, str) else None
|
||||||
|
|
||||||
|
|
||||||
|
def _details_for_issue(
|
||||||
|
workflow: Workflow,
|
||||||
|
issue: ValidationIssue,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
if issue.code is not ValidationIssueCode.INVALID_DESTINATION_PATH:
|
||||||
|
return {}
|
||||||
|
match = _NODE_OUTPUT_TARGET_RE.match(issue.path)
|
||||||
|
if match is None:
|
||||||
|
return {}
|
||||||
|
node_index = int(match.group("node_index"))
|
||||||
|
output_index = int(match.group("output_index"))
|
||||||
|
if node_index >= len(workflow.nodes):
|
||||||
|
return {}
|
||||||
|
outputs = getattr(workflow.nodes[node_index], "output", None)
|
||||||
|
if not isinstance(outputs, list) or output_index >= len(outputs):
|
||||||
|
return {}
|
||||||
|
binding = outputs[output_index]
|
||||||
|
if not isinstance(binding, OutputBinding):
|
||||||
|
return {}
|
||||||
|
output_field = _single_local_field(binding)
|
||||||
|
if output_field is None:
|
||||||
|
return {}
|
||||||
|
return {
|
||||||
|
"output_field": output_field,
|
||||||
|
"state_path": str(binding.target),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _single_local_field(binding: OutputBinding) -> str | None:
|
||||||
|
from wf_core.local_paths import LocalPathError, split_local_path
|
||||||
|
|
||||||
|
try:
|
||||||
|
parts = split_local_path(binding.source)
|
||||||
|
except LocalPathError:
|
||||||
|
return None
|
||||||
|
if len(parts) != 1:
|
||||||
|
return None
|
||||||
|
field = parts[0]
|
||||||
|
return field if isinstance(field, str) else None
|
||||||
|
|
||||||
|
|
||||||
def _validate_known_outcomes(
|
def _validate_known_outcomes(
|
||||||
draft: JsonObject,
|
draft: JsonObject,
|
||||||
outcome_lookup: OutcomeLookup,
|
outcome_lookup: OutcomeLookup,
|
||||||
|
|||||||
@@ -478,3 +478,51 @@ def test_adapter_lowers_foreach_policy_through_builder() -> None:
|
|||||||
assert foreach.concurrent.max_outstanding == 4
|
assert foreach.concurrent.max_outstanding == 4
|
||||||
assert foreach.item_error.action == "collect"
|
assert foreach.item_error.action == "collect"
|
||||||
assert str(foreach.item_error.collect_to) == "state.item_errors"
|
assert str(foreach.item_error.collect_to) == "state.item_errors"
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_workflow_draft_reports_structured_output_destination_issue() -> None:
|
||||||
|
draft = {
|
||||||
|
"name": "missing_state_field",
|
||||||
|
"input_schema": {},
|
||||||
|
"state_schema": {"type": "object", "properties": {}},
|
||||||
|
"output_schema": {},
|
||||||
|
"start": "snap",
|
||||||
|
"steps": {
|
||||||
|
"snap": {
|
||||||
|
"use": "demo.snapshot",
|
||||||
|
"output": [
|
||||||
|
{
|
||||||
|
"source": {"root": "local", "parts": ["after"]},
|
||||||
|
"target": {"root": "state", "parts": ["after"]},
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"routes": {"snap": {"ok": "__end__"}},
|
||||||
|
}
|
||||||
|
node_defs = [
|
||||||
|
NodeDef(
|
||||||
|
name="demo.snapshot",
|
||||||
|
input_schema=SchemaRef.model_validate({"type": "object", "properties": {}}),
|
||||||
|
output_schema=SchemaRef.model_validate(
|
||||||
|
{"type": "object", "properties": {"after": {"type": "string"}}}
|
||||||
|
),
|
||||||
|
outcomes=["ok"],
|
||||||
|
)
|
||||||
|
]
|
||||||
|
|
||||||
|
result = validate_workflow_draft(draft, node_defs=node_defs)
|
||||||
|
|
||||||
|
assert result["status"] == "invalid"
|
||||||
|
destination_diagnostics = [
|
||||||
|
d for d in result["diagnostics"] if d["code"] == "invalid_destination_path"
|
||||||
|
]
|
||||||
|
assert len(destination_diagnostics) == 1
|
||||||
|
diagnostic = destination_diagnostics[0]
|
||||||
|
assert diagnostic["path"] == "nodes[0].output[0].target"
|
||||||
|
assert diagnostic["step_id"] == "snap"
|
||||||
|
assert diagnostic["details"] == {
|
||||||
|
"output_field": "after",
|
||||||
|
"state_path": "state.after",
|
||||||
|
}
|
||||||
|
assert "repair_hint" not in diagnostic
|
||||||
|
|||||||
@@ -93,7 +93,7 @@ def _draft_api(
|
|||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_patch_draft_applies_json_patch(tmp_path: Path) -> None:
|
async def test_patch_draft_applies_json_patch(tmp_path: Path) -> None:
|
||||||
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_patch")
|
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_patch")
|
||||||
api, _service = _draft_api(artifact_store)
|
api, _service = _draft_api(artifact_store, register_echo=True)
|
||||||
|
|
||||||
result = await api.patch_draft(
|
result = await api.patch_draft(
|
||||||
draft=_echo_draft(),
|
draft=_echo_draft(),
|
||||||
@@ -106,7 +106,7 @@ async def test_patch_draft_applies_json_patch(tmp_path: Path) -> None:
|
|||||||
],
|
],
|
||||||
)
|
)
|
||||||
|
|
||||||
assert result["status"] == "valid"
|
assert result["status"] == "invalid"
|
||||||
assert result["draft"]["steps"]["echo"]["input"][0]["target"] == {
|
assert result["draft"]["steps"]["echo"]["input"][0]["target"] == {
|
||||||
"root": "local",
|
"root": "local",
|
||||||
"parts": ["message"],
|
"parts": ["message"],
|
||||||
@@ -186,7 +186,7 @@ async def test_delete_draft_workspace_is_idempotent(tmp_path: Path) -> None:
|
|||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_patch_draft_workspace_updates_revision(tmp_path: Path) -> None:
|
async def test_patch_draft_workspace_updates_revision(tmp_path: Path) -> None:
|
||||||
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_patch_workspace")
|
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_patch_workspace")
|
||||||
api, _service = _draft_api(artifact_store)
|
api, _service = _draft_api(artifact_store, register_echo=True)
|
||||||
await api.create_draft_workspace(
|
await api.create_draft_workspace(
|
||||||
workspace_id="echo_ws",
|
workspace_id="echo_ws",
|
||||||
draft=_echo_draft(),
|
draft=_echo_draft(),
|
||||||
@@ -328,10 +328,111 @@ async def test_validate_draft_workspace_refreshes_status(tmp_path: Path) -> None
|
|||||||
|
|
||||||
assert payload["revision"] == 1
|
assert payload["revision"] == 1
|
||||||
assert payload["status"] == "invalid"
|
assert payload["status"] == "invalid"
|
||||||
assert payload["diagnostics"][0]["code"] == "unknown_outcome"
|
assert payload["diagnostics"][0]["code"] in (
|
||||||
|
"unknown_outcome",
|
||||||
|
"undeclared_edge_outcome",
|
||||||
|
)
|
||||||
assert fetched["status"] == "invalid"
|
assert fetched["status"] == "invalid"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_validate_draft_workspace_suggests_bind_output_to_state(
|
||||||
|
tmp_path: Path,
|
||||||
|
) -> None:
|
||||||
|
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_repair_hint")
|
||||||
|
api, service = _draft_api(artifact_store)
|
||||||
|
service.register_connection(
|
||||||
|
ConnectionConfig(id="demo.personal", server="demo", account="personal")
|
||||||
|
)
|
||||||
|
service.register_specs("demo.personal", _snapshot_tool)
|
||||||
|
await api.create_draft_workspace(
|
||||||
|
workspace_id="snapshot_ws",
|
||||||
|
draft={
|
||||||
|
"name": "snapshot",
|
||||||
|
"input_schema": {"type": "object", "properties": {}},
|
||||||
|
"state_schema": {"type": "object", "properties": {}},
|
||||||
|
"output_schema": {"type": "object", "properties": {}},
|
||||||
|
"start": "snap",
|
||||||
|
"steps": {
|
||||||
|
"snap": {
|
||||||
|
"use": "demo.personal.snapshot_tool",
|
||||||
|
"input": [],
|
||||||
|
"output": [
|
||||||
|
{
|
||||||
|
"source": {"root": "local", "parts": ["after"]},
|
||||||
|
"target": {"root": "state", "parts": ["after"]},
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"routes": {"snap": {"ok": "__end__"}},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
payload = await api.validate_draft_workspace(workspace_id="snapshot_ws")
|
||||||
|
|
||||||
|
diagnostic = payload["diagnostics"][0]
|
||||||
|
assert diagnostic["code"] == "invalid_destination_path"
|
||||||
|
assert diagnostic["step_id"] == "snap"
|
||||||
|
assert diagnostic["repair_hint"] == (
|
||||||
|
"wf draft bind-output-to-state snapshot_ws --revision 1 "
|
||||||
|
"--step snap --output after --state state.after"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_draft_workspace_validates_new_use_step_with_context_specs(
|
||||||
|
tmp_path: Path,
|
||||||
|
) -> None:
|
||||||
|
artifact_store = FileWorkflowArtifactStore(tmp_path / "drafts_patch_new_use")
|
||||||
|
api, service = _draft_api(artifact_store, register_echo=True)
|
||||||
|
service.register_specs("demo.personal", echo_tool, _snapshot_tool)
|
||||||
|
await api.create_draft_workspace(
|
||||||
|
workspace_id="echo_ws",
|
||||||
|
draft=_echo_draft(),
|
||||||
|
)
|
||||||
|
|
||||||
|
patched = await api.patch_draft_workspace(
|
||||||
|
workspace_id="echo_ws",
|
||||||
|
revision=1,
|
||||||
|
patch=[
|
||||||
|
{
|
||||||
|
"op": "add",
|
||||||
|
"path": "/steps/snap",
|
||||||
|
"value": {
|
||||||
|
"use": "demo.personal.snapshot_tool",
|
||||||
|
"input": [],
|
||||||
|
"output": [
|
||||||
|
{
|
||||||
|
"source": {"root": "local", "parts": ["after"]},
|
||||||
|
"target": {"root": "state", "parts": ["after"]},
|
||||||
|
}
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"op": "replace",
|
||||||
|
"path": "/routes/echo/ok",
|
||||||
|
"value": "snap",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"op": "add",
|
||||||
|
"path": "/routes/snap",
|
||||||
|
"value": {"ok": "__end__"},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
diagnostic = patched["diagnostics"][0]
|
||||||
|
assert patched["status"] == "invalid"
|
||||||
|
assert diagnostic["code"] == "invalid_destination_path"
|
||||||
|
assert diagnostic["step_id"] == "snap"
|
||||||
|
assert diagnostic["details"] == {
|
||||||
|
"output_field": "after",
|
||||||
|
"state_path": "state.after",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_create_minimal_draft_workspace_minimal_success_path(
|
async def test_create_minimal_draft_workspace_minimal_success_path(
|
||||||
tmp_path: Path,
|
tmp_path: Path,
|
||||||
|
|||||||
@@ -652,6 +652,48 @@ def test_wf_remote_draft_artifact_deploy_lifecycle(monkeypatch, tmp_path) -> Non
|
|||||||
assert validated.exit_code == 0, validated.output
|
assert validated.exit_code == 0, validated.output
|
||||||
assert '"status": "valid"' in validated.output
|
assert '"status": "valid"' in validated.output
|
||||||
|
|
||||||
|
invalid_created = runner.invoke(
|
||||||
|
app,
|
||||||
|
[
|
||||||
|
*base_args,
|
||||||
|
"draft",
|
||||||
|
"create-from-capability",
|
||||||
|
"repair_ws",
|
||||||
|
"wf.std.constant",
|
||||||
|
"--name",
|
||||||
|
"repair_constant",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
assert invalid_created.exit_code == 0, invalid_created.output
|
||||||
|
|
||||||
|
invalid_patch = runner.invoke(
|
||||||
|
app,
|
||||||
|
[
|
||||||
|
*base_args,
|
||||||
|
"draft",
|
||||||
|
"set-output",
|
||||||
|
"repair_ws",
|
||||||
|
"--revision",
|
||||||
|
"1",
|
||||||
|
"--step",
|
||||||
|
"call",
|
||||||
|
"--map",
|
||||||
|
"value=state.missing",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
assert invalid_patch.exit_code == 0, invalid_patch.output
|
||||||
|
|
||||||
|
invalid_validated = runner.invoke(
|
||||||
|
app,
|
||||||
|
[*base_args, "draft", "validate", "repair_ws"],
|
||||||
|
)
|
||||||
|
assert invalid_validated.exit_code == 0, invalid_validated.output
|
||||||
|
assert '"status": "invalid"' in invalid_validated.output
|
||||||
|
assert "bind-output-to-state repair_ws --revision 2" in invalid_validated.output
|
||||||
|
assert (
|
||||||
|
"--step call --output value --state state.missing" in invalid_validated.output
|
||||||
|
)
|
||||||
|
|
||||||
saved_artifact = runner.invoke(
|
saved_artifact = runner.invoke(
|
||||||
app,
|
app,
|
||||||
[
|
[
|
||||||
|
|||||||
Reference in New Issue
Block a user