fix: close Python workflow client review
This commit is contained in:
@@ -1,47 +0,0 @@
|
|||||||
# Final fix recovery report
|
|
||||||
|
|
||||||
## Scope
|
|
||||||
|
|
||||||
Audited the uncommitted patch on `b377311a` against the final review findings
|
|
||||||
and the Python workflow-client design/plan. The generated `.wf_mcp_store/` and
|
|
||||||
`test-artifacts/` directories were left untouched and unstaged.
|
|
||||||
|
|
||||||
## Fixes completed
|
|
||||||
|
|
||||||
- Opted the remaining draft-focused RPC client test into `drafts=True`; default
|
|
||||||
server/storage composition remains draft-free.
|
|
||||||
- Added the public HTTP port adapter used by `App.from_http_jsonrpc()`. HTTP,
|
|
||||||
connection, malformed JSON, and malformed JSON-RPC response failures become
|
|
||||||
`WorkflowClientError` subclasses; known workflow error codes map to stable
|
|
||||||
subclasses and unknown codes remain inspectable `ProtocolError` values with
|
|
||||||
code/message/data preserved.
|
|
||||||
- Added strict identity validation for artifact inspection/save, capability
|
|
||||||
calls, deployment lifecycle, run lifecycle, and bounded trace pages.
|
|
||||||
- Made `WorkflowArtifact`, `Deployment`, and `Run` retain deep private copies
|
|
||||||
and expose defensive copies for nested mutable values.
|
|
||||||
- Narrowed workflow-plan reconstruction handling to Pydantic `ValidationError`.
|
|
||||||
- Typed deployment drift policy with the existing `wf_artifacts.DriftPolicy`
|
|
||||||
enum and removed unused internal client exports/protocol operations.
|
|
||||||
- Hardened the underlying RPC client against valid JSON values that are not
|
|
||||||
JSON-RPC objects, and removed `frozen=True` from `RpcProtocolError` so Python
|
|
||||||
can attach exception traceback state.
|
|
||||||
|
|
||||||
## Verification
|
|
||||||
|
|
||||||
Commands were run from the feature worktree.
|
|
||||||
|
|
||||||
| Command | Result |
|
|
||||||
| --- | --- |
|
|
||||||
| `uv run pytest -q tests/wf_client tests/wf_transport_rpc_http` | **292 passed**, 257 warnings |
|
|
||||||
| `uv run pytest -q tests/wf_client tests/authoring/test_builder.py tests/authoring/test_subgraph.py tests/wf_api/test_artifact_api.py tests/wf_transport_rpc_http/test_client.py tests/wf_transport_rpc_http/test_app.py tests/wf_transport_rpc_http/test_openrpc_contract.py tests/wf_contract_manifest/test_generate.py tests/wf_contract_manifest/test_committed_manifest.py tests/wf_api/test_stores.py tests/wf_server/test_local_static_server.py tests/wf_mcp/test_mcp_workflow_server.py tests/wf_mcp/server/test_tools.py tests/wf_mcp/workflow_surface tests/wf_cli/test_context.py tests/wf_server/test_cli.py` | **429 passed**, 201 warnings |
|
|
||||||
| `uv run ruff check` | **All checks passed** |
|
|
||||||
| `uv run ruff format --check` | **677 files already formatted** |
|
|
||||||
| `uv run basedpyright --level error` | **0 errors, 0 warnings, 0 notes** |
|
|
||||||
| `uv run python -m wf_contract_manifest check` | **checked** `contracts/workflow-api.manifest.json` |
|
|
||||||
| `pnpm --dir web --filter @lda/workflow-rpc contract:check` | **passed** |
|
|
||||||
| `pnpm --dir web --filter @lda/workflow-rpc test` | **151 passed**, 3 skipped |
|
|
||||||
| `git diff --check` | **passed** |
|
|
||||||
| `uv run pytest -q` | **2644 passed**, 1 skipped, 1 xfailed; 1 known baseline failure: `tests/docs/test_big_doc_links.py::test_thesis_bundle_has_reproducible_agent_evaluation_assets` (missing generated thesis figure PDFs) |
|
|
||||||
|
|
||||||
The full-suite failure is the documented pre-existing missing-asset failure;
|
|
||||||
no thesis assets were generated or added.
|
|
||||||
@@ -1,48 +0,0 @@
|
|||||||
# Task 6 report: deployment and durable run objects
|
|
||||||
|
|
||||||
## Status
|
|
||||||
|
|
||||||
Implemented and committed as `112e9656` (`feat: add Python deployment and run objects`).
|
|
||||||
|
|
||||||
## Files
|
|
||||||
|
|
||||||
- Added `src/wf_client/deployments.py` with immutable `Deployment` and
|
|
||||||
`DeploymentValidation` snapshots plus strict artifact deployment selection.
|
|
||||||
- Added `src/wf_client/runs.py` with immutable `Run` and bounded `TracePage`
|
|
||||||
snapshots, resume/refresh lifecycle methods, and core interrupt/trace
|
|
||||||
reconstruction.
|
|
||||||
- Added `tests/wf_client/test_deployments.py` and
|
|
||||||
`tests/wf_client/test_runs.py`.
|
|
||||||
- Updated `WorkflowArtifact`, `App`, public exports, codecs, and structured
|
|
||||||
deployment exceptions.
|
|
||||||
|
|
||||||
## Verification
|
|
||||||
|
|
||||||
- `uv run pytest tests/wf_client/test_deployments.py tests/wf_client/test_runs.py tests/wf_api/test_deployment_api.py tests/wf_api/test_run_api.py -q` — **30 passed**.
|
|
||||||
- `uv run pytest tests/wf_client -q` — **28 passed**.
|
|
||||||
- `uv run ruff check src/wf_client tests/wf_client/test_deployments.py tests/wf_client/test_runs.py` — **passed**.
|
|
||||||
- `uv run basedpyright --level error src/wf_client tests/wf_client/test_deployments.py tests/wf_client/test_runs.py` — **0 errors**.
|
|
||||||
|
|
||||||
## Caveats
|
|
||||||
|
|
||||||
- `.wf_mcp_store/` and `test-artifacts/` remain untracked generated directories
|
|
||||||
and were intentionally not staged.
|
|
||||||
- Trace limits follow the existing server bound of 1–100 and are validated
|
|
||||||
before issuing a trace request.
|
|
||||||
|
|
||||||
## Review fix round 1
|
|
||||||
|
|
||||||
Identity checks now reject mismatched deployment/artifact/run ids at every
|
|
||||||
inspect, validate, start, refresh, and resume boundary. Nested interrupt route
|
|
||||||
references are converted to `InvalidResponse` with operation context, and run
|
|
||||||
decoding accepts truthful inspect/start/resume operation names.
|
|
||||||
|
|
||||||
Fixes committed in the follow-up review commit for this report.
|
|
||||||
|
|
||||||
## Review fix round 2
|
|
||||||
|
|
||||||
The sole discovered deployment path now rechecks the inspected artifact
|
|
||||||
identity, deployment creation acknowledgements and inspected ids are checked
|
|
||||||
before validation, and run-start responses enforce deployment plus artifact
|
|
||||||
identity. Missing-run errors preserve server text, including operation-aware
|
|
||||||
start interrupt decoding.
|
|
||||||
@@ -1,73 +0,0 @@
|
|||||||
# Task 7 Report
|
|
||||||
|
|
||||||
## Delivered
|
|
||||||
|
|
||||||
- Added a real `httpx.ASGITransport` proof for capability discovery, local
|
|
||||||
authoring, remote validation, immutable artifact save, deployment selection,
|
|
||||||
and durable run execution.
|
|
||||||
- Added bounded, inert `repr()` and `_repr_html_()` implementations for the
|
|
||||||
Python client's loaded capability, artifact, deployment, validation, run,
|
|
||||||
diagnostic, and trace objects. The shared private renderer HTML-escapes,
|
|
||||||
bounds nested previews, and redacts credential-shaped keys without touching
|
|
||||||
the client port.
|
|
||||||
- Added the `wf_client` package map, source/API boundary notes, and a concrete
|
|
||||||
Python walkthrough that explains the artifact -> deployment -> run model.
|
|
||||||
- Added an opt-out seam for server draft composition. `drafts=False` skips
|
|
||||||
draft service construction, does not require a draft store, and omits draft
|
|
||||||
JSON-RPC methods; the existing implementation remains available to explicit
|
|
||||||
draft-enabled callers.
|
|
||||||
|
|
||||||
## Verification
|
|
||||||
|
|
||||||
- Initial Task 7 client and cross-layer selection — 288 passed.
|
|
||||||
- Fix-round focused client/composition selection — 55 passed.
|
|
||||||
- Ruff check and basedpyright for changed client/API/transport surfaces — passed.
|
|
||||||
- `uv run python -m wf_contract_manifest check` — passed.
|
|
||||||
- `pnpm --dir web --filter @lda/workflow-rpc contract:check` — passed.
|
|
||||||
- `pnpm --dir web --filter @lda/workflow-rpc test` — 151 passed, 3 skipped.
|
|
||||||
|
|
||||||
The full repository suite was not used as the fix-round gate: legacy direct
|
|
||||||
draft-service tests still construct draft APIs without the now-required
|
|
||||||
explicit `drafts=True` opt-in, and one thesis asset test expects untracked PDF
|
|
||||||
figures absent from the base worktree. No generated `.wf_mcp_store/` or
|
|
||||||
`test-artifacts/` files are part of this change.
|
|
||||||
|
|
||||||
## Fix round 2
|
|
||||||
|
|
||||||
- `file_workflow_stores()` now skips `FileDraftWorkspaceStore` by default;
|
|
||||||
`build_local_static_workflow_server()` forwards `drafts` so default local
|
|
||||||
composition creates no draft directory, while `drafts=True` remains a
|
|
||||||
working explicit opt-in.
|
|
||||||
- Draft-bearing MCP broker/workflow-surface and CLI compositions now pass
|
|
||||||
`drafts=True` explicitly. The standalone RPC server CLI does the same for
|
|
||||||
its documented draft RPC surface. The complete OpenRPC inventory fixture and
|
|
||||||
draft-focused RPC tests now opt into both server storage and RPC method
|
|
||||||
registration.
|
|
||||||
- The API architecture walkthrough now imports `App` from `wf_client`.
|
|
||||||
|
|
||||||
Fix-round 2 verification (fresh after formatting):
|
|
||||||
|
|
||||||
- `uv run pytest tests/wf_api/test_stores.py tests/wf_server/test_local_static_server.py tests/wf_mcp/test_mcp_workflow_server.py tests/wf_mcp/server/test_tools.py tests/wf_mcp/workflow_surface tests/wf_cli/test_context.py tests/wf_server/test_cli.py tests/wf_transport_rpc_http/test_openrpc_contract.py -q` — 183 passed.
|
|
||||||
- `uv run pytest tests/wf_client tests/authoring/test_builder.py tests/authoring/test_subgraph.py tests/wf_api/test_artifact_api.py tests/wf_transport_rpc_http/test_client.py tests/wf_transport_rpc_http/test_app.py tests/wf_transport_rpc_http/test_openrpc_contract.py tests/wf_contract_manifest/test_generate.py tests/wf_contract_manifest/test_committed_manifest.py -q` — 291 passed, 201 warnings.
|
|
||||||
- Ruff check and `ruff format --check` on changed Python surfaces — passed.
|
|
||||||
- `uv run basedpyright --level error src/wf_api src/wf_cli src/wf_mcp/broker src/wf_mcp/workflow_surface src/wf_server` — 0 errors, 0 warnings, 0 notes.
|
|
||||||
- `uv run python -m wf_contract_manifest check` — passed.
|
|
||||||
- `pnpm --dir web --filter @lda/workflow-rpc contract:check` — passed.
|
|
||||||
- `pnpm --dir web --filter @lda/workflow-rpc test` — 151 passed, 3 skipped.
|
|
||||||
- `git diff --check` — passed.
|
|
||||||
|
|
||||||
## Fix round 1
|
|
||||||
|
|
||||||
- Normal `WorkflowApi`, nested capability/artifact services, durable context,
|
|
||||||
local server construction, and JSON-RPC app composition now default to
|
|
||||||
`drafts=False`. Draft APIs are stored as `None` when disabled and require
|
|
||||||
explicit `drafts=True` at composition time; RPC registration rejects an
|
|
||||||
opt-in against a disabled API.
|
|
||||||
- The two live walkthroughs now include real schemas, explicit constant input
|
|
||||||
and output bindings, an `end` step, and the terminal route.
|
|
||||||
- The repr projector now follows the console evidence policy's exact-key
|
|
||||||
matching, normalizes camelCase spellings (`apiKey`, `accessToken`, etc.),
|
|
||||||
avoids false positives (`tokenCount`, `secretary`), and consumes at most a
|
|
||||||
bounded prefix of mappings/sequences/iterables.
|
|
||||||
- Fix-round verification: focused client/composition tests `55 passed`; Ruff
|
|
||||||
and basedpyright passed with zero errors.
|
|
||||||
+11
-8
@@ -22,14 +22,17 @@ The durable product path is now `wf-rpc-server` plus neutral `wf_config` /
|
|||||||
`wf_server` composition. The old `wf-mcp` script remains a legacy/special-purpose
|
`wf_server` composition. The old `wf-mcp` script remains a legacy/special-purpose
|
||||||
MCP entrypoint and compatibility surface.
|
MCP entrypoint and compatibility surface.
|
||||||
|
|
||||||
Completed: the async `wf_client` Python slice is verified against the real
|
Delivered, with the repository-wide verification gate still open: the async
|
||||||
JSON-RPC ASGI application. It covers capability discovery, local graph
|
`wf_client` Python slice is verified against the real JSON-RPC ASGI application.
|
||||||
authoring, remote validation, immutable artifact save, deployment selection,
|
It covers capability discovery, local graph authoring, remote validation,
|
||||||
and durable run execution. Rich client objects have bounded, secret-safe
|
immutable artifact save, deployment selection, and durable run execution. Rich
|
||||||
`repr()` and `_repr_html_()` views that never perform remote I/O. Draft
|
client objects have bounded, secret-safe `repr()` and `_repr_html_()` views that
|
||||||
workspaces remain a separate server/admin surface and are not part of the
|
never perform remote I/O. Draft workspaces remain a separate server/admin
|
||||||
client; server registration is explicit so the artifact -> deployment -> run
|
surface and are not part of the client; server registration is explicit so the
|
||||||
path does not require draft storage.
|
artifact -> deployment -> run path does not require draft storage. All
|
||||||
|
slice-owned checks pass, while the full repository suite still has its
|
||||||
|
pre-existing missing-thesis-PDF failure; mark this slice complete only when that
|
||||||
|
repository asset gate also passes.
|
||||||
|
|
||||||
## Active Initiative: Workflow Console And Defense Demo
|
## Active Initiative: Workflow Console And Defense Demo
|
||||||
|
|
||||||
|
|||||||
@@ -1301,7 +1301,13 @@ artifact = await graph.save(version=1)
|
|||||||
run = await artifact.run({})
|
run = await artifact.run({})
|
||||||
```
|
```
|
||||||
|
|
||||||
State explicitly that drafts are not part of `wf_client` but remain registered in the server until the separate draft opt-out plan is executed. Update `docs/current_roadmap.md` to mark the Python-client slice complete only after every verification step below passes.
|
Drafts are not part of `wf_client`. The shipped server composition uses
|
||||||
|
`drafts=False` by default, which excludes draft storage, domain modules, and RPC
|
||||||
|
registration; real legacy draft consumers opt in explicitly with `drafts=True`.
|
||||||
|
The earlier plan assumption that draft methods would remain registered pending a
|
||||||
|
separate opt-out slice is historical. Update `docs/current_roadmap.md` to mark
|
||||||
|
the Python-client slice complete only after every verification step below
|
||||||
|
passes.
|
||||||
|
|
||||||
- [ ] **Step 6: Run focused and cross-layer verification**
|
- [ ] **Step 6: Run focused and cross-layer verification**
|
||||||
|
|
||||||
|
|||||||
@@ -633,12 +633,11 @@ capability, expose secrets, or fetch an unbounded trace.
|
|||||||
authoring compiles a complete workflow and saves it through the existing
|
authoring compiles a complete workflow and saves it through the existing
|
||||||
artifact-from-plan operation.
|
artifact-from-plan operation.
|
||||||
|
|
||||||
Making draft support uninitialized by default is a separate server-composition
|
Draft support is uninitialized by default across storage, domain modules, and
|
||||||
slice. Today `WorkflowApi` constructs draft modules unconditionally, durable
|
JSON-RPC registration. Normal composition uses `drafts=False`; callers that
|
||||||
context validation requires a draft store, and the JSON-RPC app always
|
still operate the legacy draft workspace surface must opt in explicitly with
|
||||||
registers draft methods. That follow-up must make draft storage, domain modules,
|
`drafts=True`. This keeps artifact, deployment, and run durability independent
|
||||||
and RPC registration opt-in without weakening artifact, deployment, or run
|
from draft storage.
|
||||||
durability.
|
|
||||||
|
|
||||||
## Testing Strategy
|
## Testing Strategy
|
||||||
|
|
||||||
|
|||||||
+33
-6
@@ -61,7 +61,7 @@ def _prepare_artifact_from_plan(
|
|||||||
description: str | None,
|
description: str | None,
|
||||||
plan: RawWorkflowPlan | dict[str, Any],
|
plan: RawWorkflowPlan | dict[str, Any],
|
||||||
outcomes: Sequence[str],
|
outcomes: Sequence[str],
|
||||||
required_capabilities: dict[str, dict[str, Any]] | None,
|
required_capabilities: Mapping[str, RequiredCapability | dict[str, Any]] | None,
|
||||||
source_bindings: dict[str, str] | None,
|
source_bindings: dict[str, str] | None,
|
||||||
created_from_catalog_version: str | None,
|
created_from_catalog_version: str | None,
|
||||||
) -> WorkflowArtifact:
|
) -> WorkflowArtifact:
|
||||||
@@ -80,7 +80,11 @@ def _prepare_artifact_from_plan(
|
|||||||
plan=typed_plan.model_dump(mode="json", by_alias=True),
|
plan=typed_plan.model_dump(mode="json", by_alias=True),
|
||||||
outcomes=tuple(outcomes),
|
outcomes=tuple(outcomes),
|
||||||
required_capabilities={
|
required_capabilities={
|
||||||
name: RequiredCapability.model_validate(capability)
|
name: (
|
||||||
|
capability
|
||||||
|
if isinstance(capability, RequiredCapability)
|
||||||
|
else RequiredCapability.model_validate(capability)
|
||||||
|
)
|
||||||
for name, capability in (required_capabilities or {}).items()
|
for name, capability in (required_capabilities or {}).items()
|
||||||
},
|
},
|
||||||
source_bindings=source_bindings,
|
source_bindings=source_bindings,
|
||||||
@@ -103,14 +107,16 @@ def _invalid_artifact_plan_payload(
|
|||||||
|
|
||||||
def _diagnostic_from_validation_error(
|
def _diagnostic_from_validation_error(
|
||||||
exc: ValidationError,
|
exc: ValidationError,
|
||||||
|
*,
|
||||||
|
root: str = "plan",
|
||||||
) -> ArtifactPlanDiagnosticPayload:
|
) -> ArtifactPlanDiagnosticPayload:
|
||||||
"""Project the first typed model error with a stable plan-rooted path."""
|
"""Project the first typed model error beneath its request-field root."""
|
||||||
error = exc.errors()[0]
|
error = exc.errors()[0]
|
||||||
location = ".".join(str(part) for part in error["loc"])
|
location = ".".join(str(part) for part in error["loc"])
|
||||||
return {
|
return {
|
||||||
"severity": "error",
|
"severity": "error",
|
||||||
"code": "artifact_plan_invalid",
|
"code": "artifact_plan_invalid",
|
||||||
"path": f"plan.{location}" if location else "plan",
|
"path": f"{root}.{location}" if location else root,
|
||||||
"message": str(error["msg"]),
|
"message": str(error["msg"]),
|
||||||
"repair_hint": None,
|
"repair_hint": None,
|
||||||
}
|
}
|
||||||
@@ -254,6 +260,27 @@ class WorkflowArtifactApi:
|
|||||||
source_bindings: dict[str, str] | None = None,
|
source_bindings: dict[str, str] | None = None,
|
||||||
) -> ValidateArtifactPlanResult:
|
) -> ValidateArtifactPlanResult:
|
||||||
"""Validate and inventory a plan without writing the artifact store."""
|
"""Validate and inventory a plan without writing the artifact store."""
|
||||||
|
try:
|
||||||
|
typed_plan = RawWorkflowPlan.model_validate(plan)
|
||||||
|
except ValidationError as exc:
|
||||||
|
return _PROJECT_VALIDATE_ARTIFACT(
|
||||||
|
_invalid_artifact_plan_payload(_diagnostic_from_validation_error(exc))
|
||||||
|
)
|
||||||
|
|
||||||
|
typed_requirements: dict[str, RequiredCapability] = {}
|
||||||
|
for name, capability in (required_capabilities or {}).items():
|
||||||
|
try:
|
||||||
|
typed_requirements[name] = RequiredCapability.model_validate(capability)
|
||||||
|
except ValidationError as exc:
|
||||||
|
return _PROJECT_VALIDATE_ARTIFACT(
|
||||||
|
_invalid_artifact_plan_payload(
|
||||||
|
_diagnostic_from_validation_error(
|
||||||
|
exc,
|
||||||
|
root=f"required_capabilities.{name}",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# These identity fields satisfy the shared artifact factory only;
|
# These identity fields satisfy the shared artifact factory only;
|
||||||
# validation never calls the store or emits a saved-artifact event.
|
# validation never calls the store or emits a saved-artifact event.
|
||||||
@@ -264,9 +291,9 @@ class WorkflowArtifactApi:
|
|||||||
title="Validation",
|
title="Validation",
|
||||||
kind="workflow",
|
kind="workflow",
|
||||||
description=None,
|
description=None,
|
||||||
plan=plan,
|
plan=typed_plan,
|
||||||
outcomes=outcomes,
|
outcomes=outcomes,
|
||||||
required_capabilities=required_capabilities,
|
required_capabilities=typed_requirements,
|
||||||
source_bindings=source_bindings,
|
source_bindings=source_bindings,
|
||||||
created_from_catalog_version=None,
|
created_from_catalog_version=None,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -107,6 +107,13 @@ def _workflow_dependencies_from_plan(plan: JsonObject) -> dict[str, int]:
|
|||||||
continue
|
continue
|
||||||
workflow_ref = WorkflowRef.model_validate(node.get("workflow"))
|
workflow_ref = WorkflowRef.model_validate(node.get("workflow"))
|
||||||
if workflow_ref.artifact_id is not None and workflow_ref.version is not None:
|
if workflow_ref.artifact_id is not None and workflow_ref.version is not None:
|
||||||
|
pinned = dependencies.get(workflow_ref.artifact_id)
|
||||||
|
if pinned is not None and pinned != workflow_ref.version:
|
||||||
|
raise WorkflowPlanValidationError(
|
||||||
|
"invalid workflow plan: conflicting versions "
|
||||||
|
f"{pinned} and {workflow_ref.version} pinned for child "
|
||||||
|
f"artifact {workflow_ref.artifact_id!r}"
|
||||||
|
)
|
||||||
dependencies[workflow_ref.artifact_id] = workflow_ref.version
|
dependencies[workflow_ref.artifact_id] = workflow_ref.version
|
||||||
return dependencies
|
return dependencies
|
||||||
|
|
||||||
|
|||||||
@@ -29,12 +29,8 @@ from wf_transport_rpc_http.client.base import RpcProtocolError
|
|||||||
|
|
||||||
from .errors import (
|
from .errors import (
|
||||||
ArtifactNotFound,
|
ArtifactNotFound,
|
||||||
ArtifactVersionConflict,
|
|
||||||
CapabilityNotFound,
|
CapabilityNotFound,
|
||||||
DeploymentNotRunnable,
|
|
||||||
DeploymentRequired,
|
|
||||||
ProtocolError,
|
ProtocolError,
|
||||||
RevisionConflict,
|
|
||||||
TransportError,
|
TransportError,
|
||||||
WorkflowClientError,
|
WorkflowClientError,
|
||||||
)
|
)
|
||||||
@@ -64,22 +60,8 @@ def _known_protocol_error(
|
|||||||
operation: str,
|
operation: str,
|
||||||
error: RpcProtocolError,
|
error: RpcProtocolError,
|
||||||
) -> WorkflowClientError | None:
|
) -> WorkflowClientError | None:
|
||||||
"""Translate only stable codes or exact legacy missing-resource signals."""
|
"""Translate only operation-specific errors emitted by the current server."""
|
||||||
code, detail = _server_detail(error)
|
code, detail = _server_detail(error)
|
||||||
normalized = code.casefold() if code is not None else ""
|
|
||||||
if normalized in {"capability_not_found", "capabilitynotfound"}:
|
|
||||||
return CapabilityNotFound(detail)
|
|
||||||
if normalized in {"artifact_not_found", "artifactnotfound"}:
|
|
||||||
return ArtifactNotFound(detail)
|
|
||||||
if normalized in {"artifact_version_conflict", "artifactversionconflict"}:
|
|
||||||
return ArtifactVersionConflict(detail)
|
|
||||||
if normalized in {"revision_conflict", "revisionconflict"}:
|
|
||||||
return RevisionConflict(detail)
|
|
||||||
if normalized in {"deployment_required", "deploymentrequired"}:
|
|
||||||
return DeploymentRequired()
|
|
||||||
if normalized in {"deployment_not_runnable", "deploymentnotrunnable"}:
|
|
||||||
return DeploymentNotRunnable(error=detail)
|
|
||||||
|
|
||||||
# The current RPC server reports expected application exception class names
|
# The current RPC server reports expected application exception class names
|
||||||
# in ``data.code``. A generic KeyError is safe to specialize only when both
|
# in ``data.code``. A generic KeyError is safe to specialize only when both
|
||||||
# the operation and its exact resource phrase agree.
|
# the operation and its exact resource phrase agree.
|
||||||
@@ -87,11 +69,11 @@ def _known_protocol_error(
|
|||||||
if operation.startswith("workflow.capabilities.") and (
|
if operation.startswith("workflow.capabilities.") and (
|
||||||
"unknown workflow capability" in detail
|
"unknown workflow capability" in detail
|
||||||
):
|
):
|
||||||
return CapabilityNotFound(detail)
|
return CapabilityNotFound(detail, code=error.code, data=error.data)
|
||||||
if operation == "workflow.artifacts.inspect" and (
|
if operation == "workflow.artifacts.inspect" and (
|
||||||
"unknown workflow artifact" in detail
|
"unknown workflow artifact" in detail
|
||||||
):
|
):
|
||||||
return ArtifactNotFound(detail)
|
return ArtifactNotFound(detail, code=error.code, data=error.data)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -116,7 +116,7 @@ class Deployment:
|
|||||||
deployment_id=self.deployment_id,
|
deployment_id=self.deployment_id,
|
||||||
artifact=f"{self.artifact_id}.v{self.artifact_version}",
|
artifact=f"{self.artifact_id}.v{self.artifact_version}",
|
||||||
runnable=self.runnable,
|
runnable=self.runnable,
|
||||||
diagnostics=f"{len(self.diagnostics)} diagnostics",
|
diagnostics=f"{len(self._diagnostics)} diagnostics",
|
||||||
)
|
)
|
||||||
|
|
||||||
def _repr_html_(self) -> str:
|
def _repr_html_(self) -> str:
|
||||||
@@ -126,7 +126,7 @@ class Deployment:
|
|||||||
artifact=f"{self.artifact_id}.v{self.artifact_version}",
|
artifact=f"{self.artifact_id}.v{self.artifact_version}",
|
||||||
bindings=f"{len(self.bindings)} bindings",
|
bindings=f"{len(self.bindings)} bindings",
|
||||||
runnable=self.runnable,
|
runnable=self.runnable,
|
||||||
diagnostics=f"{len(self.diagnostics)} diagnostics",
|
diagnostics=f"{len(self._diagnostics)} diagnostics",
|
||||||
)
|
)
|
||||||
|
|
||||||
@property
|
@property
|
||||||
|
|||||||
+16
-3
@@ -11,6 +11,20 @@ from wf_artifacts import DependencyDiagnostic
|
|||||||
class WorkflowClientError(Exception):
|
class WorkflowClientError(Exception):
|
||||||
"""Base class for errors that can be handled by workflow callers."""
|
"""Base class for errors that can be handled by workflow callers."""
|
||||||
|
|
||||||
|
code: int | str | None
|
||||||
|
data: object
|
||||||
|
|
||||||
|
def __init__(
|
||||||
|
self,
|
||||||
|
message: str = "",
|
||||||
|
*,
|
||||||
|
code: int | str | None = None,
|
||||||
|
data: object = None,
|
||||||
|
) -> None:
|
||||||
|
self.code = code
|
||||||
|
self.data = deepcopy(data)
|
||||||
|
super().__init__(message)
|
||||||
|
|
||||||
|
|
||||||
class TransportError(WorkflowClientError):
|
class TransportError(WorkflowClientError):
|
||||||
"""The client could not communicate with the workflow service."""
|
"""The client could not communicate with the workflow service."""
|
||||||
@@ -29,10 +43,9 @@ class ProtocolError(WorkflowClientError):
|
|||||||
message: str,
|
message: str,
|
||||||
data: object = None,
|
data: object = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
self.code = code
|
|
||||||
self.message = message
|
self.message = message
|
||||||
self.data = deepcopy(data)
|
super().__init__(message, code=code, data=data)
|
||||||
super().__init__(str(self))
|
self.args = (str(self),)
|
||||||
|
|
||||||
def __str__(self) -> str:
|
def __str__(self) -> str:
|
||||||
if isinstance(self.data, dict) and isinstance(self.data.get("message"), str):
|
if isinstance(self.data, dict) and isinstance(self.data.get("message"), str):
|
||||||
|
|||||||
@@ -51,9 +51,10 @@ class RpcClientTransport:
|
|||||||
http_client: httpx.AsyncClient | None = None
|
http_client: httpx.AsyncClient | None = None
|
||||||
|
|
||||||
async def _call(self, method: str, params: dict[str, Any]) -> dict[str, Any]:
|
async def _call(self, method: str, params: dict[str, Any]) -> dict[str, Any]:
|
||||||
|
request_id = uuid4().hex
|
||||||
request = {
|
request = {
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": uuid4().hex,
|
"id": request_id,
|
||||||
"method": method,
|
"method": method,
|
||||||
"params": params,
|
"params": params,
|
||||||
}
|
}
|
||||||
@@ -66,6 +67,10 @@ class RpcClientTransport:
|
|||||||
payload = response.json()
|
payload = response.json()
|
||||||
if not isinstance(payload, dict):
|
if not isinstance(payload, dict):
|
||||||
raise RuntimeError("JSON-RPC response must be an object")
|
raise RuntimeError("JSON-RPC response must be an object")
|
||||||
|
if payload.get("jsonrpc") != "2.0":
|
||||||
|
raise RuntimeError("JSON-RPC response must declare version '2.0'")
|
||||||
|
if payload.get("id") != request_id:
|
||||||
|
raise RuntimeError("JSON-RPC response id does not match the request")
|
||||||
if "error" in payload:
|
if "error" in payload:
|
||||||
error = payload["error"]
|
error = payload["error"]
|
||||||
if not isinstance(error, dict):
|
if not isinstance(error, dict):
|
||||||
|
|||||||
@@ -236,6 +236,29 @@ async def test_validate_artifact_plan_projects_invalid_plan_diagnostic(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_validate_artifact_plan_roots_capability_diagnostic_at_request_field(
|
||||||
|
tmp_path: Path,
|
||||||
|
) -> None:
|
||||||
|
artifact_store = FileWorkflowArtifactStore(tmp_path / "artifacts_requirement")
|
||||||
|
api, _service = _artifact_api(artifact_store)
|
||||||
|
|
||||||
|
result = await api.validate_artifact_plan(
|
||||||
|
plan=_echo_artifact().plan,
|
||||||
|
outcomes=("completed",),
|
||||||
|
required_capabilities={
|
||||||
|
"broken": {
|
||||||
|
"ref": {"source": "demo", "capability_key": "echo"},
|
||||||
|
"kind": "not-a-capability-kind",
|
||||||
|
}
|
||||||
|
},
|
||||||
|
source_bindings={},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result["status"] == "invalid"
|
||||||
|
assert result["diagnostics"][0]["path"] == "required_capabilities.broken.kind"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_validate_artifact_plan_propagates_unexpected_value_error(
|
async def test_validate_artifact_plan_propagates_unexpected_value_error(
|
||||||
tmp_path: Path,
|
tmp_path: Path,
|
||||||
@@ -290,6 +313,41 @@ async def test_validate_artifact_plan_derives_saved_workflow_dependencies(
|
|||||||
assert result["workflow_dependencies"] == {"child_workflow": 7}
|
assert result["workflow_dependencies"] == {"child_workflow": 7}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_validate_artifact_plan_rejects_conflicting_child_version_pins(
|
||||||
|
tmp_path: Path,
|
||||||
|
) -> None:
|
||||||
|
artifact_store = FileWorkflowArtifactStore(tmp_path / "artifacts_pin_conflict")
|
||||||
|
api, _service = _artifact_api(artifact_store)
|
||||||
|
plan = _echo_artifact().plan
|
||||||
|
plan["start"] = "child_v1"
|
||||||
|
plan["nodes"] = [
|
||||||
|
{
|
||||||
|
"id": node_id,
|
||||||
|
"type": "subgraph",
|
||||||
|
"workflow": {"artifact_id": "child_workflow", "version": version},
|
||||||
|
"input_schema": {"type": "object"},
|
||||||
|
"output_schema": {"type": "object"},
|
||||||
|
"outcomes": ["completed"],
|
||||||
|
}
|
||||||
|
for node_id, version in (("child_v1", 1), ("child_v2", 2))
|
||||||
|
]
|
||||||
|
plan["edges"] = [
|
||||||
|
{"from": "child_v1", "outcome": "completed", "to": "child_v2"},
|
||||||
|
{"from": "child_v2", "outcome": "completed", "to": "__end__"},
|
||||||
|
]
|
||||||
|
|
||||||
|
result = await api.validate_artifact_plan(
|
||||||
|
plan=plan,
|
||||||
|
outcomes=("completed",),
|
||||||
|
source_bindings={},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result["status"] == "invalid"
|
||||||
|
assert result["diagnostics"][0]["path"] == "plan"
|
||||||
|
assert "conflicting versions 1 and 2" in result["diagnostics"][0]["message"]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_create_artifact_from_workspace_suggests_exact_available_source_binding(
|
async def test_create_artifact_from_workspace_suggests_exact_available_source_binding(
|
||||||
tmp_path: Path,
|
tmp_path: Path,
|
||||||
|
|||||||
@@ -150,19 +150,25 @@ async def test_http_app_translates_http_and_json_failures(
|
|||||||
async def test_http_app_translates_known_workflow_protocol_error(
|
async def test_http_app_translates_known_workflow_protocol_error(
|
||||||
monkeypatch: pytest.MonkeyPatch,
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
) -> None:
|
) -> None:
|
||||||
async def error_post(*args: object, **kwargs: object) -> httpx.Response:
|
async def error_post(
|
||||||
|
_client: object,
|
||||||
|
_url: object,
|
||||||
|
*,
|
||||||
|
json: dict[str, object],
|
||||||
|
**_kwargs: object,
|
||||||
|
) -> httpx.Response:
|
||||||
return httpx.Response(
|
return httpx.Response(
|
||||||
200,
|
200,
|
||||||
request=httpx.Request("POST", "http://test/rpc"),
|
request=httpx.Request("POST", "http://test/rpc"),
|
||||||
json={
|
json={
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": "request",
|
"id": json["id"],
|
||||||
"error": {
|
"error": {
|
||||||
"code": 5000,
|
"code": 5000,
|
||||||
"message": "Workflow operation failed",
|
"message": "Workflow operation failed",
|
||||||
"data": {
|
"data": {
|
||||||
"code": "capability_not_found",
|
"code": "KeyError",
|
||||||
"message": "unknown capability app.default.search",
|
"message": "unknown workflow capability 'app.default.search'",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -175,7 +181,12 @@ async def test_http_app_translates_known_workflow_protocol_error(
|
|||||||
await app.capability("app.default.search")
|
await app.capability("app.default.search")
|
||||||
|
|
||||||
assert isinstance(raised.value, CapabilityNotFound)
|
assert isinstance(raised.value, CapabilityNotFound)
|
||||||
assert "unknown capability" in str(raised.value)
|
assert "unknown workflow capability" in str(raised.value)
|
||||||
|
assert raised.value.code == 5000
|
||||||
|
assert raised.value.data == {
|
||||||
|
"code": "KeyError",
|
||||||
|
"message": "unknown workflow capability 'app.default.search'",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -184,13 +195,19 @@ async def test_http_app_preserves_unknown_protocol_error_details(
|
|||||||
) -> None:
|
) -> None:
|
||||||
data = {"code": "future_workflow_error", "message": "future detail", "retry": 3}
|
data = {"code": "future_workflow_error", "message": "future detail", "retry": 3}
|
||||||
|
|
||||||
async def error_post(*args: object, **kwargs: object) -> httpx.Response:
|
async def error_post(
|
||||||
|
_client: object,
|
||||||
|
_url: object,
|
||||||
|
*,
|
||||||
|
json: dict[str, object],
|
||||||
|
**_kwargs: object,
|
||||||
|
) -> httpx.Response:
|
||||||
return httpx.Response(
|
return httpx.Response(
|
||||||
200,
|
200,
|
||||||
request=httpx.Request("POST", "http://test/rpc"),
|
request=httpx.Request("POST", "http://test/rpc"),
|
||||||
json={
|
json={
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": "request",
|
"id": json["id"],
|
||||||
"error": {
|
"error": {
|
||||||
"code": 5999,
|
"code": 5999,
|
||||||
"message": "Future workflow error",
|
"message": "Future workflow error",
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
@@ -33,11 +34,12 @@ from wf_transport_rpc_http.client.sources import RpcSourceAdminClientMixin
|
|||||||
|
|
||||||
async def test_rpc_client_preserves_structured_jsonrpc_error() -> None:
|
async def test_rpc_client_preserves_structured_jsonrpc_error() -> None:
|
||||||
def handler(request: httpx.Request) -> httpx.Response:
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
request_id = json.loads(request.content)["id"]
|
||||||
return httpx.Response(
|
return httpx.Response(
|
||||||
200,
|
200,
|
||||||
json={
|
json={
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": "request",
|
"id": request_id,
|
||||||
"error": {
|
"error": {
|
||||||
"code": "missing_source",
|
"code": "missing_source",
|
||||||
"message": "workflow operation failed",
|
"message": "workflow operation failed",
|
||||||
@@ -61,6 +63,31 @@ async def test_rpc_client_preserves_structured_jsonrpc_error() -> None:
|
|||||||
assert str(raised.value) == ("workflow operation failed: source is not configured")
|
assert str(raised.value) == ("workflow operation failed: source is not configured")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("jsonrpc", "response_id"),
|
||||||
|
[(None, "echo"), ("1.0", "echo"), ("2.0", "wrong")],
|
||||||
|
)
|
||||||
|
async def test_rpc_client_rejects_malformed_response_envelope(
|
||||||
|
jsonrpc: str | None,
|
||||||
|
response_id: str,
|
||||||
|
) -> None:
|
||||||
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
|
request_id = json.loads(request.content)["id"]
|
||||||
|
payload: dict[str, object] = {
|
||||||
|
"id": request_id if response_id == "echo" else response_id,
|
||||||
|
"result": {},
|
||||||
|
}
|
||||||
|
if jsonrpc is not None:
|
||||||
|
payload["jsonrpc"] = jsonrpc
|
||||||
|
return httpx.Response(200, json=payload)
|
||||||
|
|
||||||
|
async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as http_client:
|
||||||
|
client = RpcWorkflowApiClient(url="http://test/rpc", http_client=http_client)
|
||||||
|
with pytest.raises(RuntimeError, match="JSON-RPC response"):
|
||||||
|
await client.list_capabilities()
|
||||||
|
|
||||||
|
|
||||||
def _constant_plan() -> RawWorkflowPlan:
|
def _constant_plan() -> RawWorkflowPlan:
|
||||||
return RawWorkflowPlan.model_validate(
|
return RawWorkflowPlan.model_validate(
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user