refactor: isolate rpc server config composition
This commit is contained in:
@@ -1,18 +1,29 @@
|
||||
# pitfalls / guide
|
||||
|
||||
## extra fields
|
||||
|
||||
prefer asserts actual['field'] == expected['field'] over assert actual == expected unless we know better (eg. no extra fields allowed)
|
||||
|
||||
## tests
|
||||
|
||||
Prefer pytest `tmp_path` for test-local filesystem state. Avoid fixed paths under `local_temp_root()` for tests that create durable files unless the test explicitly cleans or needs cross-process persistence; stale files there can change later test runs.
|
||||
|
||||
Now that pytest-asyncio is installed, prefer `async def test_x()`
|
||||
instead of `def test_x(): async def scenario(): ...; asyncio.run(scenario())`
|
||||
|
||||
more later
|
||||
## mgmt
|
||||
|
||||
More packages please. we spent a while cleaning flatten packages/modules; putting files of similar interests in folders and sub-folders.
|
||||
example: some of tests/ and some packages. (simple example: src/pack/foo_bar.py -> src/pack/foo/bar.py)
|
||||
|
||||
Lets just do that from the start this time, ok?
|
||||
|
||||
## Docs mgmt
|
||||
|
||||
read docs/AGENTS.md
|
||||
|
||||
more later
|
||||
|
||||
# Test suite
|
||||
|
||||
```bash
|
||||
|
||||
@@ -191,6 +191,8 @@ implementation state.
|
||||
import-direction guard. The durable fix is not a permanent split launcher;
|
||||
it is making `wf_config` wide enough that the RPC server can compose from
|
||||
neutral config while MCP-specific adapters stay selected by source kind.
|
||||
Completed: `wf_transport_rpc_http` no longer imports `wf_mcp`; server
|
||||
composition from neutral or legacy MCP config lives behind `wf_server.config`.
|
||||
- Legacy config migration: add a converter from old `wf_mcp.config.json`
|
||||
(`store_root`, `connections[]`) into the wider `wf_config` shape
|
||||
(`server.store`, `server.sources[]`). `server.store` is already a
|
||||
@@ -217,6 +219,13 @@ implementation state.
|
||||
upstream credentials, and surface missing auth as validation diagnostics.
|
||||
- Run watch/progress: start with polling over existing inspect/trace APIs;
|
||||
defer SSE/WebSocket/MCP progress until the polling UX is proven insufficient.
|
||||
- MCP package split direction: keep separating "MCP as a client transport"
|
||||
from "MCP as an upstream workflow source provider." The future shape is
|
||||
likely `wf_transport_mcp` for exposing workflow/admin surfaces to MCP
|
||||
clients, and `wf_sources_mcp` for discovering/invoking upstream MCP servers
|
||||
as workflow capabilities. The current `wf_mcp` package still contains both
|
||||
roles plus compatibility entrypoints; new server/transport work should avoid
|
||||
depending on that combined facade.
|
||||
- Cleanup candidate: consolidate store/source registry id validation patterns
|
||||
(`SOURCE_REGISTRY_ID_PATTERN`, `STORE_ID_PATTERN`) only after another package
|
||||
needs the same rule. Today they intentionally stay close to their stores.
|
||||
|
||||
@@ -0,0 +1,543 @@
|
||||
# RPC Transport Config Boundary Cleanup 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:** Remove direct `wf_mcp` imports from `wf_transport_rpc_http` now that neutral `wf_config.server.sources[]` can describe MCP sources.
|
||||
|
||||
**Architecture:** Keep JSON-RPC transport modules focused on serving a `WorkflowServer`. Move config-to-server composition into a server-layer module that can select local/static or MCP-backed composition by config source kind. `wf_server.context` remains MCP-free; the new config composition module is the explicit boundary for source-provider selection.
|
||||
|
||||
**Tech Stack:** Typer CLI, `wf_config.WorkflowConfigFile`, `wf_server.WorkflowServer`, existing MCP-backed builder in `wf_mcp.broker.server`, AST import-direction tests.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- Create `src/wf_server/config.py`: server composition from neutral workflow config and legacy MCP config path.
|
||||
- Modify `src/wf_transport_rpc_http/cli.py`: import server composition helpers from `wf_server.config`, not `wf_mcp.broker`.
|
||||
- Modify `tests/wf_transport_rpc_http/test_cli.py`: monkeypatch new helper paths.
|
||||
- Modify `tests/wf_transport_rpc_http/test_import_direction.py`: no change expected; it should pass once direct imports are removed.
|
||||
- Create `tests/wf_server/test_config_composition.py`: cover local/static and MCP-source selection.
|
||||
- Modify docs after implementation.
|
||||
|
||||
## Current Context
|
||||
|
||||
`src/wf_transport_rpc_http/cli.py` currently imports:
|
||||
|
||||
```python
|
||||
from wf_mcp.broker import (
|
||||
build_workflow_server_from_config,
|
||||
build_workflow_server_from_workflow_config,
|
||||
load_broker_config,
|
||||
)
|
||||
```
|
||||
|
||||
That makes `tests/wf_transport_rpc_http/test_import_direction.py` fail:
|
||||
|
||||
```python
|
||||
def test_wf_transport_rpc_http_imports_no_wfmcp_modules() -> None:
|
||||
...
|
||||
assert violations == []
|
||||
```
|
||||
|
||||
Do not remove the test. The transport package should not import MCP modules directly.
|
||||
|
||||
`src/wf_server/context.py` has a narrower guard in `tests/wf_server/test_local_static_server.py` that only checks `context.py` for `WfMcpService`. Keep `context.py` untouched.
|
||||
|
||||
## Task 1: Add Server Composition Tests
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/wf_server/test_config_composition.py`
|
||||
|
||||
- [ ] **Step 1: Create test file**
|
||||
|
||||
Create `tests/wf_server/test_config_composition.py`:
|
||||
|
||||
```python
|
||||
from __future__ import annotations
|
||||
|
||||
from wf_config import WorkflowConfigFile
|
||||
from wf_server.config import (
|
||||
build_workflow_server_from_legacy_mcp_config,
|
||||
build_workflow_server_from_workflow_config,
|
||||
)
|
||||
from wf_server.context import WorkflowServer
|
||||
|
||||
|
||||
def test_build_workflow_server_from_workflow_config_uses_local_static_for_no_mcp_sources(
|
||||
tmp_path,
|
||||
) -> None:
|
||||
config = WorkflowConfigFile.model_validate(
|
||||
{
|
||||
"version": 1,
|
||||
"server": {
|
||||
"store": {"kind": "filesystem", "root": str(tmp_path / "store")},
|
||||
"sources": [{"kind": "stdlib", "id": "wf.std"}],
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
server = build_workflow_server_from_workflow_config(config)
|
||||
|
||||
assert isinstance(server, WorkflowServer)
|
||||
assert server.config.store_root == tmp_path / "store"
|
||||
assert server.source_registry_admin is None
|
||||
|
||||
|
||||
def test_build_workflow_server_from_workflow_config_uses_mcp_builder_for_mcp_sources(
|
||||
monkeypatch, tmp_path
|
||||
) -> None:
|
||||
captured = {}
|
||||
|
||||
def fake_builder(config):
|
||||
captured["source_kinds"] = [source.kind for source in config.server.sources]
|
||||
return "mcp-server"
|
||||
|
||||
monkeypatch.setattr(
|
||||
"wf_server.config._build_mcp_workflow_server_from_workflow_config",
|
||||
fake_builder,
|
||||
)
|
||||
config = WorkflowConfigFile.model_validate(
|
||||
{
|
||||
"version": 1,
|
||||
"server": {
|
||||
"store": {"kind": "filesystem", "root": str(tmp_path / "store")},
|
||||
"sources": [
|
||||
{
|
||||
"kind": "mcp",
|
||||
"id": "everything.default",
|
||||
"provider": "everything",
|
||||
"account": "default",
|
||||
"transport": {"kind": "stdio", "command": "uvx"},
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
server = build_workflow_server_from_workflow_config(config)
|
||||
|
||||
assert server == "mcp-server"
|
||||
assert captured["source_kinds"] == ["mcp"]
|
||||
|
||||
|
||||
def test_build_workflow_server_from_legacy_mcp_config_delegates_to_mcp_builder(
|
||||
monkeypatch, tmp_path
|
||||
) -> None:
|
||||
captured = {}
|
||||
|
||||
def fake_builder(path):
|
||||
captured["path"] = path
|
||||
return "legacy-mcp-server"
|
||||
|
||||
monkeypatch.setattr(
|
||||
"wf_server.config._build_mcp_workflow_server_from_legacy_config",
|
||||
fake_builder,
|
||||
)
|
||||
legacy_path = tmp_path / "wf_mcp.config.json"
|
||||
legacy_path.write_text('{"store_root": "store", "connections": []}', encoding="utf-8")
|
||||
|
||||
server = build_workflow_server_from_legacy_mcp_config(legacy_path)
|
||||
|
||||
assert server == "legacy-mcp-server"
|
||||
assert captured["path"] == legacy_path
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests and verify failure**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run pytest tests/wf_server/test_config_composition.py -q
|
||||
```
|
||||
|
||||
Expected: fail because `wf_server.config` does not exist.
|
||||
|
||||
## Task 2: Implement Server Config Composition Module
|
||||
|
||||
**Files:**
|
||||
- Create: `src/wf_server/config.py`
|
||||
|
||||
- [ ] **Step 1: Create module**
|
||||
|
||||
Create `src/wf_server/config.py`:
|
||||
|
||||
```python
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from wf_config import FilesystemStoreConfig, WorkflowConfigFile
|
||||
|
||||
from .context import WorkflowServer, build_local_static_workflow_server
|
||||
|
||||
|
||||
def _has_mcp_sources(config: WorkflowConfigFile) -> bool:
|
||||
return any(getattr(source, "kind", None) == "mcp" for source in config.server.sources)
|
||||
|
||||
|
||||
def _build_mcp_workflow_server_from_workflow_config(
|
||||
config: WorkflowConfigFile,
|
||||
) -> WorkflowServer:
|
||||
"""Build an MCP-backed server from neutral config.
|
||||
|
||||
This import is intentionally isolated here: transport packages should not
|
||||
import MCP modules, while this server composition boundary is allowed to
|
||||
select source-provider implementations by source kind.
|
||||
"""
|
||||
from wf_mcp.broker import build_workflow_server_from_workflow_config
|
||||
|
||||
return build_workflow_server_from_workflow_config(config)
|
||||
|
||||
|
||||
def _build_mcp_workflow_server_from_legacy_config(path: Path) -> WorkflowServer:
|
||||
"""Build an MCP-backed server from legacy broker config."""
|
||||
from wf_mcp.broker import build_workflow_server_from_config, load_broker_config
|
||||
|
||||
return build_workflow_server_from_config(load_broker_config(path))
|
||||
|
||||
|
||||
def build_workflow_server_from_workflow_config(
|
||||
config: WorkflowConfigFile,
|
||||
) -> WorkflowServer:
|
||||
"""Build a WorkflowServer from neutral workflow config.
|
||||
|
||||
Local/static configs use built-in sources. Configs with `kind: "mcp"`
|
||||
sources delegate to the MCP provider adapter.
|
||||
"""
|
||||
if _has_mcp_sources(config):
|
||||
return _build_mcp_workflow_server_from_workflow_config(config)
|
||||
store = config.server.store
|
||||
if not isinstance(store, FilesystemStoreConfig):
|
||||
raise ValueError("wf-rpc-server currently requires filesystem store")
|
||||
return build_local_static_workflow_server(store.root)
|
||||
|
||||
|
||||
def build_workflow_server_from_legacy_mcp_config(path: str | Path) -> WorkflowServer:
|
||||
"""Build a WorkflowServer from legacy wf_mcp.config.json.
|
||||
|
||||
Prefer neutral `wf_config` for new setups. This compatibility hook keeps the
|
||||
transport CLI free of direct MCP imports while existing users migrate.
|
||||
"""
|
||||
return _build_mcp_workflow_server_from_legacy_config(Path(path))
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run pytest tests/wf_server/test_config_composition.py -q
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
- [ ] **Step 3: Run import-boundary check**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run pytest tests/wf_server/test_local_static_server.py::test_wf_server_context_imports_no_wfmcp_service -q
|
||||
```
|
||||
|
||||
Expected: pass because `context.py` was not changed.
|
||||
|
||||
## Task 3: Update RPC CLI to Use Server Composition Helpers
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/wf_transport_rpc_http/cli.py`
|
||||
- Modify: `tests/wf_transport_rpc_http/test_cli.py`
|
||||
|
||||
- [ ] **Step 1: Update imports**
|
||||
|
||||
Replace:
|
||||
|
||||
```python
|
||||
from wf_mcp.broker import (
|
||||
build_workflow_server_from_config,
|
||||
build_workflow_server_from_workflow_config,
|
||||
load_broker_config,
|
||||
)
|
||||
|
||||
from wf_server import build_local_static_workflow_server
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
from wf_server.config import (
|
||||
build_workflow_server_from_legacy_mcp_config,
|
||||
build_workflow_server_from_workflow_config,
|
||||
)
|
||||
from wf_server.context import build_local_static_workflow_server
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update legacy mcp config branch**
|
||||
|
||||
Replace:
|
||||
|
||||
```python
|
||||
if mcp_config is not None:
|
||||
broker_config = load_broker_config(mcp_config)
|
||||
server = build_workflow_server_from_config(broker_config)
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
if mcp_config is not None:
|
||||
server = build_workflow_server_from_legacy_mcp_config(mcp_config)
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Remove duplicated mcp source detection from CLI**
|
||||
|
||||
Inside the `if config is not None:` block, remove:
|
||||
|
||||
```python
|
||||
has_mcp_sources = any(
|
||||
getattr(source, "kind", None) == "mcp"
|
||||
for source in workflow_config.server.sources
|
||||
)
|
||||
if server is None and has_mcp_sources:
|
||||
server = build_workflow_server_from_workflow_config(workflow_config)
|
||||
```
|
||||
|
||||
Then replace the local/static store handling block:
|
||||
|
||||
```python
|
||||
store = workflow_config.server.store
|
||||
if server is None and not isinstance(store, FilesystemStoreConfig):
|
||||
raise typer.BadParameter(
|
||||
"wf-rpc-server currently requires filesystem store"
|
||||
)
|
||||
if server is None:
|
||||
resolved_store_root = resolved_store_root or store.root
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
store = workflow_config.server.store
|
||||
if server is None and store_root is None:
|
||||
server = build_workflow_server_from_workflow_config(workflow_config)
|
||||
elif server is None:
|
||||
if not isinstance(store, FilesystemStoreConfig):
|
||||
raise typer.BadParameter(
|
||||
"wf-rpc-server currently requires filesystem store"
|
||||
)
|
||||
resolved_store_root = resolved_store_root or store.root
|
||||
```
|
||||
|
||||
Keep `--store-root` as an override for local/static configs. `--store-root` is still forbidden with `--mcp-config`; do not add new behavior there.
|
||||
|
||||
- [ ] **Step 4: Update tests monkeypatch paths**
|
||||
|
||||
In `tests/wf_transport_rpc_http/test_cli.py`, update existing monkeypatches to
|
||||
match the new boundary.
|
||||
|
||||
For `test_rpc_server_cli_uses_configured_store_and_transport`, stop monkeypatching
|
||||
`build_local_static_workflow_server`; the CLI now delegates config-based server
|
||||
composition to `build_workflow_server_from_workflow_config`.
|
||||
|
||||
Replace:
|
||||
|
||||
```python
|
||||
def fake_build_server(root):
|
||||
captured["store_root"] = root
|
||||
return object()
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
def fake_build_server(config):
|
||||
captured["store_root"] = config.server.store.root
|
||||
return object()
|
||||
```
|
||||
|
||||
Replace this monkeypatch:
|
||||
|
||||
```python
|
||||
monkeypatch.setattr(
|
||||
"wf_transport_rpc_http.cli.build_local_static_workflow_server",
|
||||
fake_build_server,
|
||||
)
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
monkeypatch.setattr(
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_workflow_config",
|
||||
fake_build_server,
|
||||
)
|
||||
```
|
||||
|
||||
Keep the assertion:
|
||||
|
||||
```python
|
||||
assert captured["store_root"] == (tmp_path / ".wf_store").resolve()
|
||||
```
|
||||
|
||||
For legacy MCP config tests, replace monkeypatch paths:
|
||||
|
||||
```python
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_config"
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_workflow_config"
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_legacy_mcp_config"
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_workflow_config"
|
||||
```
|
||||
|
||||
If a test currently monkeypatches `load_broker_config`, remove that monkeypatch and have the fake legacy builder capture the config path directly.
|
||||
|
||||
For `test_rpc_server_cli_uses_mcp_config_server`, replace the two fake helpers:
|
||||
|
||||
```python
|
||||
def fake_load_broker_config(path):
|
||||
captured["mcp_config_path"] = path
|
||||
return "broker-config"
|
||||
|
||||
def fake_build_mcp_server(config):
|
||||
captured["mcp_config"] = config
|
||||
return object()
|
||||
```
|
||||
|
||||
with one helper:
|
||||
|
||||
```python
|
||||
def fake_build_mcp_server(path):
|
||||
captured["mcp_config_path"] = path
|
||||
return object()
|
||||
```
|
||||
|
||||
Replace the two monkeypatches:
|
||||
|
||||
```python
|
||||
monkeypatch.setattr("wf_transport_rpc_http.cli.load_broker_config", fake_load_broker_config)
|
||||
monkeypatch.setattr(
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_config",
|
||||
fake_build_mcp_server,
|
||||
)
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```python
|
||||
monkeypatch.setattr(
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_legacy_mcp_config",
|
||||
fake_build_mcp_server,
|
||||
)
|
||||
```
|
||||
|
||||
Remove the assertion:
|
||||
|
||||
```python
|
||||
assert captured["mcp_config"] == "broker-config"
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run CLI tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run pytest tests/wf_transport_rpc_http/test_cli.py -q
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
## Task 4: Restore Transport Import-Direction Guard
|
||||
|
||||
**Files:**
|
||||
- Test: `tests/wf_transport_rpc_http/test_import_direction.py`
|
||||
|
||||
- [ ] **Step 1: Run guard**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run pytest tests/wf_transport_rpc_http/test_import_direction.py -q
|
||||
```
|
||||
|
||||
Expected: pass. If it fails, inspect `src/wf_transport_rpc_http` for any remaining `wf_mcp` imports and remove them.
|
||||
|
||||
- [ ] **Step 2: Run wider RPC subset**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run pytest tests/wf_transport_rpc_http tests/wf_cli/test_remote_target.py -q
|
||||
```
|
||||
|
||||
Expected: pass. The prior known failure should be gone.
|
||||
|
||||
## Task 5: Update Docs and Mark Cleanup Complete
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/current_roadmap.md`
|
||||
- Modify: `docs/superpowers/specs/2026-06-03-long-lived-workflow-api-boundary.md`
|
||||
|
||||
- [ ] **Step 1: Update roadmap**
|
||||
|
||||
In `docs/current_roadmap.md`, under "Transport package boundary cleanup", append:
|
||||
|
||||
```markdown
|
||||
Completed: `wf_transport_rpc_http` no longer imports `wf_mcp`; server
|
||||
composition from neutral or legacy MCP config lives behind `wf_server.config`.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update long-lived API spec**
|
||||
|
||||
In `docs/superpowers/specs/2026-06-03-long-lived-workflow-api-boundary.md`, under "Transport package boundary cleanup", append:
|
||||
|
||||
```markdown
|
||||
Completed when `tests/wf_transport_rpc_http/test_import_direction.py` passes
|
||||
and the RPC transport CLI imports only `wf_config`, `wf_server`, and transport
|
||||
modules for server construction.
|
||||
```
|
||||
|
||||
## Task 6: Final Verification and Commit
|
||||
|
||||
**Files:**
|
||||
- All touched files.
|
||||
|
||||
- [ ] **Step 1: Run focused tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run pytest tests/wf_server/test_config_composition.py tests/wf_transport_rpc_http tests/wf_cli/test_remote_target.py -q
|
||||
```
|
||||
|
||||
Expected: pass.
|
||||
|
||||
- [ ] **Step 2: Run lint/type checks**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
uv run ruff check src/wf_server src/wf_transport_rpc_http tests/wf_server tests/wf_transport_rpc_http
|
||||
uv run basedpyright --level error src/wf_server src/wf_transport_rpc_http tests/wf_server tests/wf_transport_rpc_http
|
||||
```
|
||||
|
||||
Expected: pass with 0 errors.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git add src/wf_server src/wf_transport_rpc_http tests/wf_server tests/wf_transport_rpc_http docs/current_roadmap.md docs/superpowers/specs/2026-06-03-long-lived-workflow-api-boundary.md
|
||||
git commit -m "refactor: isolate rpc server config composition"
|
||||
```
|
||||
|
||||
## Self-Review Checklist
|
||||
|
||||
- `src/wf_transport_rpc_http` contains no direct `wf_mcp` imports.
|
||||
- `src/wf_server/context.py` remains MCP-free.
|
||||
- The MCP-specific import is isolated in `src/wf_server/config.py` with a docstring explaining the boundary.
|
||||
- `wf-rpc-server --config` still works for local/static and MCP-source neutral configs.
|
||||
- `wf-rpc-server --mcp-config` still works as a legacy compatibility path.
|
||||
@@ -54,6 +54,19 @@ wf_mcp
|
||||
MCP transport and upstream MCP integration
|
||||
```
|
||||
|
||||
`wf_mcp` is intentionally treated as a combined compatibility package in this
|
||||
diagram, not as the desired final boundary. It currently contains two different
|
||||
roles:
|
||||
|
||||
- MCP as a client transport: an MCP client connects to workflow/admin surfaces.
|
||||
- MCP as an upstream source provider: workflows discover and invoke external MCP
|
||||
servers as capability sources.
|
||||
|
||||
Those roles should separate over time. A future `wf_transport_mcp` can expose
|
||||
the same `WorkflowServer` / `wf_api` surfaces to MCP clients, while a future
|
||||
`wf_sources_mcp` can own upstream MCP discovery, sessions, tool invocation,
|
||||
resource/prompt access, and FastMCP-specific provider behavior.
|
||||
|
||||
`wf_transport_rpc_http` should call `WorkflowApi` through the server
|
||||
composition. It should not call `WfMcpService`.
|
||||
|
||||
@@ -252,6 +265,10 @@ Hard rules:
|
||||
MCP-specific context adapter.
|
||||
- A future MCP transport mounted through `wf_server` must still keep upstream
|
||||
MCP source execution separate from transport request handling.
|
||||
- Do not add new generic server or transport code that depends on the combined
|
||||
`wf_mcp` facade. If it needs MCP-specific upstream behavior, isolate that as a
|
||||
source-provider adapter; if it needs to expose workflow operations to MCP
|
||||
clients, isolate that as a transport adapter.
|
||||
|
||||
If a reusable service currently lives under `wf_mcp.broker.service` but has no
|
||||
MCP dependency, later slices may move or duplicate a protocol-neutral version.
|
||||
@@ -368,6 +385,19 @@ Possible providers:
|
||||
This slice should avoid making "source" mean "MCP connection." MCP is one
|
||||
source provider, not the source model.
|
||||
|
||||
Future package direction:
|
||||
|
||||
```text
|
||||
wf_transport_mcp
|
||||
exposes WorkflowServer / wf_api operations to MCP clients
|
||||
|
||||
wf_sources_mcp
|
||||
consumes upstream MCP servers as workflow capability sources
|
||||
|
||||
wf_mcp
|
||||
compatibility package until old MCP entrypoints can shrink or retire
|
||||
```
|
||||
|
||||
Current MCP-backed server status:
|
||||
|
||||
- MCP-backed `WorkflowServer` construction is implemented.
|
||||
@@ -398,6 +428,9 @@ Next implementation slices should be:
|
||||
transport-only. After `wf_config` can describe MCP sources, `wf-rpc-server
|
||||
--config ...` should compose MCP-backed sources from neutral config and the
|
||||
`--mcp-config` path can become deprecated/legacy.
|
||||
Completed when `tests/wf_transport_rpc_http/test_import_direction.py` passes
|
||||
and the RPC transport CLI imports only `wf_config`, `wf_server`, and transport
|
||||
modules for server construction.
|
||||
3. Legacy MCP config migration. Provide an explicit converter from old
|
||||
`wf_mcp.config.json` into `WorkflowConfigFile`: `store_root` maps to
|
||||
`server.store` (`StoreConfig` is already a discriminated union; currently
|
||||
|
||||
@@ -158,9 +158,20 @@ gone.
|
||||
|
||||
If this becomes multiple distributions, likely split points are:
|
||||
|
||||
- `wf-mcp-proxy`: `proxy`, `control`, `shared`
|
||||
- `wf-mcp-broker`: `broker`, `storage`, `workflow`, `shared`
|
||||
- `wf-mcp-sdk`: `sdk`, `capabilities`, `models`, `shared`
|
||||
- `wf-transport-mcp`: MCP-facing transport that exposes neutral
|
||||
`WorkflowServer` / `wf_api` workflow, source-admin, and platform-admin
|
||||
surfaces to MCP clients.
|
||||
- `wf-sources-mcp`: upstream MCP source provider that owns external MCP server
|
||||
discovery, sessions, tool invocation, resources/prompts, FastMCP integration,
|
||||
and conversion of discovered MCP tools into workflow node specs.
|
||||
- `wf-mcp-proxy`: compatibility/proxy runtime for mounting upstream MCP servers
|
||||
through FastMCP, if that remains useful after `wf-sources-mcp` exists.
|
||||
|
||||
For now, keep one distribution and use import discipline to preserve those
|
||||
boundaries.
|
||||
The key boundary is direction, not naming: "MCP as a client transport" and
|
||||
"MCP as an upstream workflow source" are separate roles. The current `wf_mcp`
|
||||
package still contains both roles plus compatibility entrypoints. New
|
||||
server/transport code should depend on `wf_server` / `wf_api` surfaces and keep
|
||||
MCP-specific upstream behavior behind a source-provider adapter.
|
||||
|
||||
For now, keep one distribution and use import discipline to preserve these
|
||||
future split points.
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from wf_config import FilesystemStoreConfig, WorkflowConfigFile
|
||||
|
||||
from .context import WorkflowServer, build_local_static_workflow_server
|
||||
|
||||
|
||||
def _has_mcp_sources(config: WorkflowConfigFile) -> bool:
|
||||
return any(getattr(source, "kind", None) == "mcp" for source in config.server.sources)
|
||||
|
||||
|
||||
def _build_mcp_workflow_server_from_workflow_config(
|
||||
config: WorkflowConfigFile,
|
||||
) -> WorkflowServer:
|
||||
"""Build an MCP-backed server from neutral config.
|
||||
|
||||
This import is intentionally isolated here: transport packages should not
|
||||
import MCP modules, while this server composition boundary is allowed to
|
||||
select source-provider implementations by source kind.
|
||||
"""
|
||||
from wf_mcp.broker import build_workflow_server_from_workflow_config
|
||||
|
||||
return build_workflow_server_from_workflow_config(config)
|
||||
|
||||
|
||||
def _build_mcp_workflow_server_from_legacy_config(path: Path) -> WorkflowServer:
|
||||
"""Build an MCP-backed server from legacy broker config."""
|
||||
from wf_mcp.broker import build_workflow_server_from_config, load_broker_config
|
||||
|
||||
return build_workflow_server_from_config(load_broker_config(path))
|
||||
|
||||
|
||||
def build_workflow_server_from_workflow_config(
|
||||
config: WorkflowConfigFile,
|
||||
) -> WorkflowServer:
|
||||
"""Build a WorkflowServer from neutral workflow config.
|
||||
|
||||
Local/static configs use built-in sources. Configs with ``kind: "mcp"``
|
||||
sources delegate to the MCP provider adapter.
|
||||
"""
|
||||
if _has_mcp_sources(config):
|
||||
return _build_mcp_workflow_server_from_workflow_config(config)
|
||||
store = config.server.store
|
||||
if not isinstance(store, FilesystemStoreConfig):
|
||||
raise ValueError("wf-rpc-server currently requires filesystem store")
|
||||
return build_local_static_workflow_server(store.root)
|
||||
|
||||
|
||||
def build_workflow_server_from_legacy_mcp_config(path: str | Path) -> WorkflowServer:
|
||||
"""Build a WorkflowServer from legacy wf_mcp.config.json.
|
||||
|
||||
Prefer neutral ``wf_config`` for new setups. This compatibility hook keeps the
|
||||
transport CLI free of direct MCP imports while existing users migrate.
|
||||
"""
|
||||
return _build_mcp_workflow_server_from_legacy_config(Path(path))
|
||||
@@ -11,13 +11,11 @@ from wf_config import (
|
||||
load_workflow_config,
|
||||
)
|
||||
|
||||
from wf_mcp.broker import (
|
||||
build_workflow_server_from_config,
|
||||
from wf_server.config import (
|
||||
build_workflow_server_from_legacy_mcp_config,
|
||||
build_workflow_server_from_workflow_config,
|
||||
load_broker_config,
|
||||
)
|
||||
|
||||
from wf_server import build_local_static_workflow_server
|
||||
from wf_server.context import build_local_static_workflow_server
|
||||
|
||||
from .app import create_rpc_app
|
||||
|
||||
@@ -65,23 +63,26 @@ def serve(
|
||||
|
||||
server = None
|
||||
if mcp_config is not None:
|
||||
broker_config = load_broker_config(mcp_config)
|
||||
server = build_workflow_server_from_config(broker_config)
|
||||
server = build_workflow_server_from_legacy_mcp_config(mcp_config)
|
||||
|
||||
if config is not None:
|
||||
workflow_config = load_workflow_config(config)
|
||||
has_mcp_sources = any(
|
||||
getattr(source, "kind", None) == "mcp"
|
||||
for source in workflow_config.server.sources
|
||||
)
|
||||
if server is None and has_mcp_sources:
|
||||
server = build_workflow_server_from_workflow_config(workflow_config)
|
||||
store = workflow_config.server.store
|
||||
if server is None and not isinstance(store, FilesystemStoreConfig):
|
||||
raise typer.BadParameter(
|
||||
"wf-rpc-server currently requires filesystem store"
|
||||
if server is None and store_root is None:
|
||||
server = build_workflow_server_from_workflow_config(workflow_config)
|
||||
elif server is None:
|
||||
has_mcp_sources = any(
|
||||
getattr(source, "kind", None) == "mcp"
|
||||
for source in workflow_config.server.sources
|
||||
)
|
||||
if server is None:
|
||||
if has_mcp_sources:
|
||||
raise typer.BadParameter(
|
||||
"--store-root cannot override MCP-source config"
|
||||
)
|
||||
if not isinstance(store, FilesystemStoreConfig):
|
||||
raise typer.BadParameter(
|
||||
"wf-rpc-server currently requires filesystem store"
|
||||
)
|
||||
resolved_store_root = resolved_store_root or store.root
|
||||
rpc_transport = next(
|
||||
(
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from wf_config import WorkflowConfigFile
|
||||
from wf_server.config import (
|
||||
build_workflow_server_from_legacy_mcp_config,
|
||||
build_workflow_server_from_workflow_config,
|
||||
)
|
||||
from wf_server.context import WorkflowServer
|
||||
|
||||
|
||||
def test_build_workflow_server_from_workflow_config_uses_local_static_for_no_mcp_sources(
|
||||
tmp_path,
|
||||
) -> None:
|
||||
config = WorkflowConfigFile.model_validate(
|
||||
{
|
||||
"version": 1,
|
||||
"server": {
|
||||
"store": {"kind": "filesystem", "root": str(tmp_path / "store")},
|
||||
"sources": [{"kind": "stdlib", "id": "wf.std"}],
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
server = build_workflow_server_from_workflow_config(config)
|
||||
|
||||
assert isinstance(server, WorkflowServer)
|
||||
assert server.config.store_root == tmp_path / "store"
|
||||
assert server.source_registry_admin is None
|
||||
|
||||
|
||||
def test_build_workflow_server_from_workflow_config_uses_mcp_builder_for_mcp_sources(
|
||||
monkeypatch, tmp_path
|
||||
) -> None:
|
||||
captured = {}
|
||||
|
||||
def fake_builder(config):
|
||||
captured["source_kinds"] = [source.kind for source in config.server.sources]
|
||||
return "mcp-server"
|
||||
|
||||
monkeypatch.setattr(
|
||||
"wf_server.config._build_mcp_workflow_server_from_workflow_config",
|
||||
fake_builder,
|
||||
)
|
||||
config = WorkflowConfigFile.model_validate(
|
||||
{
|
||||
"version": 1,
|
||||
"server": {
|
||||
"store": {"kind": "filesystem", "root": str(tmp_path / "store")},
|
||||
"sources": [
|
||||
{
|
||||
"kind": "mcp",
|
||||
"id": "everything.default",
|
||||
"provider": "everything",
|
||||
"account": "default",
|
||||
"transport": {"kind": "stdio", "command": "uvx"},
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
server = build_workflow_server_from_workflow_config(config)
|
||||
|
||||
assert server == "mcp-server"
|
||||
assert captured["source_kinds"] == ["mcp"]
|
||||
|
||||
|
||||
def test_build_workflow_server_from_legacy_mcp_config_delegates_to_mcp_builder(
|
||||
monkeypatch, tmp_path
|
||||
) -> None:
|
||||
captured = {}
|
||||
|
||||
def fake_builder(path):
|
||||
captured["path"] = path
|
||||
return "legacy-mcp-server"
|
||||
|
||||
monkeypatch.setattr(
|
||||
"wf_server.config._build_mcp_workflow_server_from_legacy_config",
|
||||
fake_builder,
|
||||
)
|
||||
legacy_path = tmp_path / "wf_mcp.config.json"
|
||||
legacy_path.write_text('{"store_root": "store", "connections": []}', encoding="utf-8")
|
||||
|
||||
server = build_workflow_server_from_legacy_mcp_config(legacy_path)
|
||||
|
||||
assert server == "legacy-mcp-server"
|
||||
assert captured["path"] == legacy_path
|
||||
@@ -70,8 +70,8 @@ def test_rpc_server_cli_uses_configured_store_and_transport(
|
||||
)
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_build_server(root):
|
||||
captured["store_root"] = root
|
||||
def fake_build_server(config):
|
||||
captured["store_root"] = config.server.store.root
|
||||
return object()
|
||||
|
||||
def fake_create_rpc_app(server, *, rpc_path="/rpc"):
|
||||
@@ -86,7 +86,7 @@ def test_rpc_server_cli_uses_configured_store_and_transport(
|
||||
captured["access_log"] = access_log
|
||||
|
||||
monkeypatch.setattr(
|
||||
"wf_transport_rpc_http.cli.build_local_static_workflow_server",
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_workflow_config",
|
||||
fake_build_server,
|
||||
)
|
||||
monkeypatch.setattr("wf_transport_rpc_http.cli.create_rpc_app", fake_create_rpc_app)
|
||||
@@ -115,12 +115,8 @@ def test_rpc_server_cli_uses_mcp_config_server(monkeypatch, tmp_path) -> None:
|
||||
)
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_load_broker_config(path):
|
||||
def fake_build_mcp_server(path):
|
||||
captured["mcp_config_path"] = path
|
||||
return "broker-config"
|
||||
|
||||
def fake_build_mcp_server(config):
|
||||
captured["mcp_config"] = config
|
||||
return object()
|
||||
|
||||
def fake_create_rpc_app(server, *, rpc_path="/rpc"):
|
||||
@@ -134,9 +130,8 @@ def test_rpc_server_cli_uses_mcp_config_server(monkeypatch, tmp_path) -> None:
|
||||
captured["port"] = port
|
||||
captured["access_log"] = access_log
|
||||
|
||||
monkeypatch.setattr("wf_transport_rpc_http.cli.load_broker_config", fake_load_broker_config)
|
||||
monkeypatch.setattr(
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_config",
|
||||
"wf_transport_rpc_http.cli.build_workflow_server_from_legacy_mcp_config",
|
||||
fake_build_mcp_server,
|
||||
)
|
||||
monkeypatch.setattr("wf_transport_rpc_http.cli.create_rpc_app", fake_create_rpc_app)
|
||||
@@ -156,7 +151,6 @@ def test_rpc_server_cli_uses_mcp_config_server(monkeypatch, tmp_path) -> None:
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert captured["mcp_config_path"] == config_path
|
||||
assert captured["mcp_config"] == "broker-config"
|
||||
assert captured["server"] is not None
|
||||
assert captured["rpc_path"] == "/rpc"
|
||||
assert captured["host"] == "127.0.0.9"
|
||||
@@ -343,3 +337,35 @@ def test_rpc_server_cli_config_with_mcp_source_uses_mcp_builder(
|
||||
assert result.exit_code == 0, result.output
|
||||
assert captured["source_kinds"] == ["mcp"]
|
||||
assert captured["run"]["app"] == "app"
|
||||
|
||||
|
||||
def test_rpc_server_cli_rejects_store_root_with_mcp_source_config(tmp_path) -> None:
|
||||
config_path = tmp_path / "wf.json"
|
||||
config_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"version": 1,
|
||||
"server": {
|
||||
"store": {"kind": "filesystem", "root": ".wf_store"},
|
||||
"sources": [
|
||||
{
|
||||
"kind": "mcp",
|
||||
"id": "everything.default",
|
||||
"provider": "everything",
|
||||
"account": "default",
|
||||
"transport": {"kind": "stdio", "command": "uvx"},
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
result = CliRunner().invoke(
|
||||
app,
|
||||
["--config", str(config_path), "--store-root", str(tmp_path / "override")],
|
||||
)
|
||||
|
||||
assert result.exit_code != 0
|
||||
assert "--store-root cannot override MCP-source config" in result.output
|
||||
|
||||
Reference in New Issue
Block a user