fix: address run watch review feedback
This commit is contained in:
@@ -102,13 +102,19 @@ def watch_run(
|
|||||||
payload = run_cli_operation(context, context.handlers.inspect_run(run_id=run_id))
|
payload = run_cli_operation(context, context.handlers.inspect_run(run_id=run_id))
|
||||||
if payload.get("status") in _STOPPED_RUN_STATUSES:
|
if payload.get("status") in _STOPPED_RUN_STATUSES:
|
||||||
if include_trace:
|
if include_trace:
|
||||||
payload = run_cli_operation(
|
trace_payload = run_cli_operation(
|
||||||
context,
|
context,
|
||||||
context.handlers.read_run_trace(
|
context.handlers.read_run_trace(
|
||||||
run_id=run_id,
|
run_id=run_id,
|
||||||
trace_range=TraceRange(start=trace_from, limit=trace_limit),
|
trace_range=TraceRange(start=trace_from, limit=trace_limit),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
payload = {
|
||||||
|
**payload,
|
||||||
|
"trace_start": trace_payload.get("trace_start"),
|
||||||
|
"trace_limit": trace_payload.get("trace_limit"),
|
||||||
|
"trace": trace_payload.get("trace", []),
|
||||||
|
}
|
||||||
emit_json(payload)
|
emit_json(payload)
|
||||||
return
|
return
|
||||||
if timeout is not None and time.monotonic() - started_at >= timeout:
|
if timeout is not None and time.monotonic() - started_at >= timeout:
|
||||||
|
|||||||
@@ -150,6 +150,7 @@ def connection_config_to_registry_entry(
|
|||||||
identity to become the future desired-state owner after first startup.
|
identity to become the future desired-state owner after first startup.
|
||||||
"""
|
"""
|
||||||
transport = connection.metadata.get("transport")
|
transport = connection.metadata.get("transport")
|
||||||
|
legacy_transport_value: str | None = None
|
||||||
if isinstance(transport, dict):
|
if isinstance(transport, dict):
|
||||||
pass
|
pass
|
||||||
elif isinstance(transport, str):
|
elif isinstance(transport, str):
|
||||||
@@ -161,7 +162,7 @@ def connection_config_to_registry_entry(
|
|||||||
"env": dict(connection.metadata.get("env", {})),
|
"env": dict(connection.metadata.get("env", {})),
|
||||||
}
|
}
|
||||||
elif transport in _FLAT_HTTP_TRANSPORTS:
|
elif transport in _FLAT_HTTP_TRANSPORTS:
|
||||||
legacy_transport = transport
|
legacy_transport_value = transport
|
||||||
transport = {
|
transport = {
|
||||||
"kind": "http",
|
"kind": "http",
|
||||||
"url": connection.metadata.get("url", ""),
|
"url": connection.metadata.get("url", ""),
|
||||||
@@ -182,8 +183,8 @@ def connection_config_to_registry_entry(
|
|||||||
for key, value in connection.metadata.items()
|
for key, value in connection.metadata.items()
|
||||||
if key not in _TRANSPORT_METADATA_KEYS
|
if key not in _TRANSPORT_METADATA_KEYS
|
||||||
}
|
}
|
||||||
if "legacy_transport" in locals():
|
if legacy_transport_value is not None:
|
||||||
source_metadata["legacy_transport"] = legacy_transport
|
source_metadata["legacy_transport"] = legacy_transport_value
|
||||||
entry = McpSourceRegistryEntry.model_validate(
|
entry = McpSourceRegistryEntry.model_validate(
|
||||||
{
|
{
|
||||||
"id": connection.id,
|
"id": connection.id,
|
||||||
|
|||||||
@@ -44,6 +44,8 @@ def build_workflow_server_from_workflow_config(
|
|||||||
return _build_mcp_workflow_server_from_workflow_config(config)
|
return _build_mcp_workflow_server_from_workflow_config(config)
|
||||||
store = config.server.store
|
store = config.server.store
|
||||||
if not isinstance(store, FilesystemStoreConfig):
|
if not isinstance(store, FilesystemStoreConfig):
|
||||||
|
# Roadmap: SQL/transactional stores are deferred until the remote server
|
||||||
|
# storage boundary is proven with file-backed stores.
|
||||||
raise ValueError("wf-rpc-server currently requires filesystem store")
|
raise ValueError("wf-rpc-server currently requires filesystem store")
|
||||||
return build_local_static_workflow_server(store.root)
|
return build_local_static_workflow_server(store.root)
|
||||||
|
|
||||||
|
|||||||
@@ -1,13 +1,14 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
from typer.testing import CliRunner
|
from typer.testing import CliRunner
|
||||||
|
|
||||||
from wf_cli.app import app
|
from wf_cli.app import app
|
||||||
|
|
||||||
|
|
||||||
def test_wf_config_migrate_mcp_prints_neutral_config(tmp_path) -> None:
|
def test_wf_config_migrate_mcp_prints_neutral_config(tmp_path: Path) -> None:
|
||||||
legacy_path = tmp_path / "wf_mcp.config.json"
|
legacy_path = tmp_path / "wf_mcp.config.json"
|
||||||
legacy_path.write_text(
|
legacy_path.write_text(
|
||||||
"""
|
"""
|
||||||
@@ -45,7 +46,7 @@ def test_wf_config_migrate_mcp_prints_neutral_config(tmp_path) -> None:
|
|||||||
assert source["transport"]["command"] == "uvx"
|
assert source["transport"]["command"] == "uvx"
|
||||||
|
|
||||||
|
|
||||||
def test_wf_config_migrate_mcp_writes_output_file(tmp_path) -> None:
|
def test_wf_config_migrate_mcp_writes_output_file(tmp_path: Path) -> None:
|
||||||
legacy_path = tmp_path / "wf_mcp.config.json"
|
legacy_path = tmp_path / "wf_mcp.config.json"
|
||||||
output_path = tmp_path / "wf.json"
|
output_path = tmp_path / "wf.json"
|
||||||
legacy_path.write_text(
|
legacy_path.write_text(
|
||||||
|
|||||||
@@ -231,9 +231,9 @@ def test_wf_run_inspect_and_trace_existing_run() -> None:
|
|||||||
assert traced_payload["trace"][0]["node_id"] == "echo"
|
assert traced_payload["trace"][0]["node_id"] == "echo"
|
||||||
|
|
||||||
|
|
||||||
def test_wf_run_watch_outputs_terminal_run_summary() -> None:
|
def test_wf_run_watch_outputs_terminal_run_summary(tmp_path: Path) -> None:
|
||||||
root = local_temp_root() / "wf_cli_run_watch_completed"
|
root = tmp_path / "wf_cli_run_watch_completed"
|
||||||
root.mkdir(parents=True, exist_ok=True)
|
root.mkdir()
|
||||||
config_path = _seed_echo_deployment(root)
|
config_path = _seed_echo_deployment(root)
|
||||||
|
|
||||||
with patch(
|
with patch(
|
||||||
@@ -273,9 +273,9 @@ def test_wf_run_watch_outputs_terminal_run_summary() -> None:
|
|||||||
assert payload["outcome"] == "ok"
|
assert payload["outcome"] == "ok"
|
||||||
|
|
||||||
|
|
||||||
def test_wf_run_watch_stops_on_interrupted_run() -> None:
|
def test_wf_run_watch_stops_on_interrupted_run(tmp_path: Path) -> None:
|
||||||
root = local_temp_root() / "wf_cli_run_watch_interrupted"
|
root = tmp_path / "wf_cli_run_watch_interrupted"
|
||||||
root.mkdir(parents=True, exist_ok=True)
|
root.mkdir()
|
||||||
config_path = _seed_interrupt_deployment(root)
|
config_path = _seed_interrupt_deployment(root)
|
||||||
|
|
||||||
with patch(
|
with patch(
|
||||||
@@ -315,9 +315,9 @@ def test_wf_run_watch_stops_on_interrupted_run() -> None:
|
|||||||
assert payload["resume_readiness"] == "ready"
|
assert payload["resume_readiness"] == "ready"
|
||||||
|
|
||||||
|
|
||||||
def test_wf_run_watch_can_include_trace_slice() -> None:
|
def test_wf_run_watch_can_include_trace_slice(tmp_path: Path) -> None:
|
||||||
root = local_temp_root() / "wf_cli_run_watch_trace"
|
root = tmp_path / "wf_cli_run_watch_trace"
|
||||||
root.mkdir(parents=True, exist_ok=True)
|
root.mkdir()
|
||||||
config_path = _seed_echo_deployment(root)
|
config_path = _seed_echo_deployment(root)
|
||||||
|
|
||||||
with patch(
|
with patch(
|
||||||
@@ -357,6 +357,7 @@ def test_wf_run_watch_can_include_trace_slice() -> None:
|
|||||||
payload = json.loads(watched.output)
|
payload = json.loads(watched.output)
|
||||||
assert payload["run_id"] == run_id
|
assert payload["run_id"] == run_id
|
||||||
assert payload["status"] == "completed"
|
assert payload["status"] == "completed"
|
||||||
|
assert payload["outcome"] == "ok"
|
||||||
assert payload["trace_limit"] == 1
|
assert payload["trace_limit"] == 1
|
||||||
assert payload["trace"][0]["node_id"] == "echo"
|
assert payload["trace"][0]["node_id"] == "echo"
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import json
|
import json
|
||||||
|
from pathlib import Path
|
||||||
from typing import Any, cast
|
from typing import Any, cast
|
||||||
|
|
||||||
from wf_artifacts import (
|
from wf_artifacts import (
|
||||||
@@ -549,9 +550,7 @@ def test_build_service_from_config_config_shadows_registry() -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_broker_config_connection_defaults_to_locked() -> None:
|
def test_broker_config_connection_defaults_to_locked(tmp_path: Path) -> None:
|
||||||
tmp_path = local_temp_root() / "broker_config_locked_default"
|
|
||||||
tmp_path.mkdir(parents=True, exist_ok=True)
|
|
||||||
config_path = tmp_path / "wf_mcp.config.json"
|
config_path = tmp_path / "wf_mcp.config.json"
|
||||||
config_path.write_text(
|
config_path.write_text(
|
||||||
json.dumps(
|
json.dumps(
|
||||||
@@ -570,9 +569,7 @@ def test_broker_config_connection_defaults_to_locked() -> None:
|
|||||||
assert config.connections[0].source_config_ownership == "locked"
|
assert config.connections[0].source_config_ownership == "locked"
|
||||||
|
|
||||||
|
|
||||||
def test_broker_config_connection_accepts_seed_policy() -> None:
|
def test_broker_config_connection_accepts_seed_policy(tmp_path: Path) -> None:
|
||||||
tmp_path = local_temp_root() / "broker_config_seed_policy"
|
|
||||||
tmp_path.mkdir(parents=True, exist_ok=True)
|
|
||||||
config_path = tmp_path / "wf_mcp.config.json"
|
config_path = tmp_path / "wf_mcp.config.json"
|
||||||
config_path.write_text(
|
config_path.write_text(
|
||||||
json.dumps(
|
json.dumps(
|
||||||
|
|||||||
@@ -1,5 +1,9 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from wf_config import WorkflowConfigFile
|
from wf_config import WorkflowConfigFile
|
||||||
from wf_server.config import (
|
from wf_server.config import (
|
||||||
build_workflow_server_from_legacy_mcp_config,
|
build_workflow_server_from_legacy_mcp_config,
|
||||||
@@ -9,7 +13,7 @@ from wf_server.context import WorkflowServer
|
|||||||
|
|
||||||
|
|
||||||
def test_build_workflow_server_from_workflow_config_uses_local_static_for_no_mcp_sources(
|
def test_build_workflow_server_from_workflow_config_uses_local_static_for_no_mcp_sources(
|
||||||
tmp_path,
|
tmp_path: Path,
|
||||||
) -> None:
|
) -> None:
|
||||||
config = WorkflowConfigFile.model_validate(
|
config = WorkflowConfigFile.model_validate(
|
||||||
{
|
{
|
||||||
@@ -29,7 +33,7 @@ def test_build_workflow_server_from_workflow_config_uses_local_static_for_no_mcp
|
|||||||
|
|
||||||
|
|
||||||
def test_build_workflow_server_from_workflow_config_uses_mcp_builder_for_mcp_sources(
|
def test_build_workflow_server_from_workflow_config_uses_mcp_builder_for_mcp_sources(
|
||||||
monkeypatch, tmp_path
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
captured = {}
|
captured = {}
|
||||||
|
|
||||||
@@ -66,7 +70,7 @@ def test_build_workflow_server_from_workflow_config_uses_mcp_builder_for_mcp_sou
|
|||||||
|
|
||||||
|
|
||||||
def test_build_workflow_server_from_legacy_mcp_config_delegates_to_mcp_builder(
|
def test_build_workflow_server_from_legacy_mcp_config_delegates_to_mcp_builder(
|
||||||
monkeypatch, tmp_path
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
captured = {}
|
captured = {}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user