From 50f3757e3ddc8327737def50750f11e36d4416fd Mon Sep 17 00:00:00 2001 From: Don Kackman Date: Sun, 6 Sep 2026 19:09:20 -0500 Subject: [PATCH 1/2] Add MCP server environment variable and remove obsolete scoping document --- .vscode/settings.json | 3 + docs/proposals/mcp-complete.md | 253 --------------------------------- 2 files changed, 3 insertions(+), 253 deletions(-) delete mode 100644 docs/proposals/mcp-complete.md diff --git a/.vscode/settings.json b/.vscode/settings.json index 7d57983e..73ac4371 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,8 @@ { "editor.formatOnSave": true, + "terminal.integrated.env.osx": { + "PATH": "/Users/don/.nvm/versions/node/v24.20.0/bin:${env:PATH}" + }, "cSpell.words": [ "aiohttp", "controlnet", diff --git a/docs/proposals/mcp-complete.md b/docs/proposals/mcp-complete.md deleted file mode 100644 index 17ce6abe..00000000 --- a/docs/proposals/mcp-complete.md +++ /dev/null @@ -1,253 +0,0 @@ -# MCP Server for diffusers-workflow — Scoping Doc - -> Status: scoped and verified against the code on 2026-09-01. The design -> that came out of this document is -> [2026-09-01-mcp-server-design.md](../specs/2026-09-01-mcp-server-design.md). - -## Background - -`diffusers-workflow` (dkackman/diffusers-workflow) is a declarative JSON -workflow engine for Hugging Face Diffusers, with a local HTTP server -(`dw.serve`, default port 8765) that exposes a full REST API: workflow -CRUD, a job queue with SSE progress streaming, model management, -introspection of diffusers pipeline signatures, and a gallery of past -generations with embedded metadata. - -Today, workflow authoring and debugging, for the project developer, can -happen via Claude Code in VS Code, with direct repo/filesystem access: -"create a workflow that does X," or "workflow Y produces bad output, find -out why" — Claude Code runs the workflow, reads logs, checks GPU memory, -and proposes fixes to code or workflow config. It has even gone so far as -to identify bad RAM causing intermittent generation failures. - -That flow works well for the developer (me), but it assumes a dev -environment: a repo checkout, a terminal, VS Code, comfort reading stack -traces. The next audience is **non-developer end users** of the packaged -product, who won't have any of that. They'll have the app (web UI / -`dw.serve`, or an embedded product surface), not a coding agent with shell -access. - -## Goal - -The modes a workflow can be authored and run in today: - -- a text editor plus the CLI, `python -m dw.run workflow.json` -- the command-line REPL, `python -m dw.repl` -- the HTTP API `dw.serve` and its web UI - -Add a fourth: an **MCP server** wrapping the existing `dw.serve` HTTP API, -exposing the same "author a workflow / run it / diagnose why it failed or -produced bad output" capability through a bounded, structured tool surface -— no shell, no repo checkout, no stack traces. Any MCP-compatible client -can drive it: Claude Code first, later an embedded chat UI in the product, -and eventually a local-LLM-backed client for users without a paid Claude -subscription. - -## Why MCP (not just keep using Claude Code) - -- Claude Code assumes a dev environment; end users won't have one. -- MCP tools are a defined, constrained surface (structured in/out, no - arbitrary code execution) — appropriate once non-developers are the - audience, and safer than raw shell access. -- The underlying HTTP API already exists and is already defensive (path - validation, traversal blocking, `Origin` header checks, JSON size - limits) — MCP adds a protocol layer in front of it, not new attack - surface. -- Protocol-agnostic: the same server can later be pointed at Claude Code, - a future embedded chat UI, or a local-LLM-backed client. - -## Rollout sequence (decided) - -1. **MCP server + Claude Code** as the client, for continued dev use and - to validate the tool surface. -2. **Embedded-in-app** later, once the product is packaged for - non-developer users. This phase needs its own auth story, since the - server binds to `127.0.0.1` with no authentication (see Security, - below) — fine for local/trusted-LAN use, not fine for a hosted product. -3. **Local LLM support** stays a goal throughout. MCP is - protocol-agnostic, so this is a separate, later concern, not a blocker. - -## Topology - -`dw_mcp/` is a **stdio MCP server that is an HTTP client of a running -`dw.serve`**. (It shipped as `dw/mcp/` and moved to a top-level package in -F8, below.) It owns no job state, no GPU, and no worker process; it -holds no state the REST API doesn't already hold. If `dw.serve` isn't -reachable it fails with an instruction to start it, rather than starting -one itself. - -Rejected: embedding the FastAPI app in-process. `create_app()` builds its -own `JobManager` and GPU worker, so an in-process copy running alongside a -real `dw.serve` would mean two workers contending for one GPU — which the -single-job-at-a-time design cannot tolerate. - -## Proposed MCP tool set - -Mapped to existing `dw.serve` routes. One new endpoint is required (see -"Corrections", below); everything else exists. - -### Read-only / low-cost (build first) - -| Tool | Route(s) | Notes | -| --- | --- | --- | -| `list_workflows` | `GET /api/workflows` | descriptions, output kinds, variable counts | -| `get_workflow(name)` | `GET /api/workflows/{name}` | full JSON | -| `validate_workflow(json)` | `POST /api/validate` | schema + signature-level argument checks; no GPU cost — the pre-flight tool | -| `get_schema` | `GET /api/schema` | workflow JSON schema | -| `list_pipelines` | `GET /api/pipelines` | what pipelines the installed diffusers offers | -| `get_pipeline_signature(name)` | `GET /api/pipelines/{name}` | real pipeline arguments, before proposing a config fix | -| `list_classes(kind)` / `get_class(name)` | `GET /api/classes?kind=...`, `GET /api/classes/{name}` | broader introspection beyond pipelines | -| `list_tasks` / `get_task(command)` | `GET /api/tasks`, `GET /api/tasks/{command}` | task-step argument schemas | -| `list_models` | `GET /api/models` | hub cache inventory: sizes, revisions, last-used | -| `get_memory` | `GET /api/memory` | worker VRAM/RAM stats — answers the GPU-diagnosis need directly | -| `get_health` | `GET /api/health` | liveness | -| `list_jobs` | `GET /api/jobs` | live queue plus recent history | -| `list_gallery(limit)` | `GET /api/gallery` | recent outputs, newest first | -| `get_gallery_metadata(name)` | `GET /api/gallery/{name}/metadata` | a prior run's exact workflow + seed, e.g. to reproduce a failure | -| `get_output_image(name, max_dimension)` | `GET /outputs/{name}` (static mount) | returns a downscaled MCP image block — this is how the agent actually *sees* bad output | - -### Authoring (build second) - -| Tool | Route(s) | Notes | -| --- | --- | --- | -| `save_workflow(name, json)` | `PUT /api/workflows/{name}` | overwrites; `destructiveHint: true`, `idempotentHint: true` | -| `delete_workflow(name)` | `DELETE /api/workflows/{name}` | `destructiveHint: true` | - -Without these the server is a read-only advisor: it can propose a workflow -but never complete the validate-then-save loop. - -### Diagnostic loop (build third) - -| Tool | Route(s) | Notes | -| --- | --- | --- | -| `run_workflow(workflow_path \| inline_workflow, arguments, acknowledged_cost)` | `POST /api/jobs` | **Costs GPU time; one job runs at a time.** Submits and returns immediately. See confirm-gating below. | -| `get_job(job_id)` | `GET /api/jobs/{id}` | status, arguments, warnings, manifest, error, traceback, `event_count` | -| `get_job_events(job_id, after, limit)` | `GET /api/jobs/{id}/event-log` **(new)** | live or persisted `job_status` / `phase` / `memory` / `log` events — phase transitions (loading → generating → decoding → saving) | -| `cancel_job(job_id)` | `POST /api/jobs/{id}/cancel` | cooperative cancel | -| `rerun_job(job_id, acknowledged_cost)` | `POST /api/jobs/{id}/rerun` | re-queue after a fix — cost-gated like a run | -| `move_job(job_id, direction)` | `POST /api/jobs/{id}/move` | reorder queued jobs | - -### Deferred / optional - -- `POST /api/models/download`, `DELETE /api/models`, `/api/system/diffusers*` - — model and dependency management; useful eventually, not needed for the - first author/run/diagnose loop. -- Prompt library endpoints (`/api/prompts*`, `/api/enhance`, - `/api/enhancers`, `/api/prompt-schema`) — relevant if the agent should - also help build/enhance prompts, not core to diagnosis. -- `base_dir` on `POST /api/jobs` is deliberately **not** exposed. It is a - path-authority parameter that decides where an inline workflow's - relative paths resolve; v1 lets them resolve against the server's own - workflow directory only. - -## Corrections found while verifying against the code - -Checked against `dw/server/app.py` and `dw/server/jobs.py` on 2026-09-01. - -1. **`get_job` returns no events and no spec.** `Job.detail()` - (`jobs.py:222`) returns `event_count`, not `events`, and `arguments`, - not the whole spec. The event trail exists only behind the SSE stream, - which a request/response MCP tool cannot consume sensibly. Hence the - one new endpoint, `GET /api/jobs/{job_id}/event-log`. -2. **Outputs are not on an `/api` route.** Generated media is served from - a `StaticFiles` mount at `/outputs` (`app.py:840`). Any tool that shows - the agent an image must read from there. -3. **MCP does have a confirmation primitive.** The 2025-06-18 spec - revision added **elicitation**, and tool **annotations** - (`readOnlyHint`, `destructiveHint`, `idempotentHint`, `openWorldHint`) - exist for exactly this purpose. The earlier claim that confirmation had - to live entirely in client prose was wrong. -4. **Events did not survive a restart.** They live in the in-memory `Job` - object (`jobs.py:182`); `JobHistory` persisted eleven columns and none - of them was the event log, so a job recovered from history had no - trail. Originally deferred as F1, this has been pulled into the build — - history now persists a bounded tail of each job's events. - -## Key design decisions - -1. **`run_workflow` is confirm-gated, in three layers.** A run costs real - GPU time and the engine is single-GPU/single-job-at-a-time. The tool - carries annotations so a client can surface the cost; it uses MCP - elicitation when the client advertises that capability; and it requires - an explicit `acknowledged_cost` argument as the client-agnostic floor, - since Claude Code does not implement elicitation today. - `validate_workflow` is free and runs first, without confirmation. - -2. **`run_workflow` never blocks.** It submits and returns - `{job_id, status, queue_position}` immediately. A generation takes - minutes; no MCP client will hold a tool call open that long. Progress - comes from polling `get_job_events`. - -3. **Security model carries over, doesn't change.** The server already - validates every path (`dw/security.py`), blocks traversal, rejects - non-local `Origin` headers, and confines file access to the configured - workflow/output/prompt directories. MCP wrapping this doesn't add new - risk for the local/Claude-Code phase. It does **not** solve remote - auth — the server binds to `127.0.0.1` by default with no - authentication; `--host 0.0.0.0` exists but is explicitly documented as - trusted-LAN-only. The embedded-in-app phase needs a real auth story - before any hosted/remote exposure. - -4. **Tools only, no resources or prompts, in v1.** `get_schema` and - `get_workflow` are natural MCP *resources*, and "diagnose this job" is - a natural MCP *prompt* — but client support for tools is universal and - support for the other two is not. Revisit once the tool surface has - been validated. - -5. **Build order:** read-only tools first (a workflow authoring assistant - usable almost immediately), then authoring writes, then the - run/diagnose loop once confirm-gating is settled. - -## Post-v1 follow-ups - -Deliberately out of the first build. Each is its own change. (F1, persisting -a job's event tail, was pulled into the build instead — without it the -diagnostic loop only explains jobs from the current server process, which is -the wrong trade for the non-developer audience this is aimed at.) - -- **F2 — Cost/time estimation for the confirm prompt.** Nothing in the API - can say "this will take about four minutes." Job history holds durations - per workflow id; a `GET /api/workflows/{name}/stats` could turn that - into an estimate worth putting in front of a human before a run. -- **F3 — Expose the server log.** `log_filename` is already a setting; a - `get_server_log(tail)` tool would help diagnosis, but it widens the read - surface beyond the confined directories and needs its own redaction - thinking first. -- **F4 — Model and dependency management tools.** *(Done 2026-09-01.)* The - deferred `/api/models/*` and `/api/system/diffusers*` routes — the first - wall a non-developer hits is "that model isn't downloaded," and until this - the agent could see the gap and not close it. Six tools in `dw_mcp/models.py`: - `download_model`, `list_downloads`, `cancel_download`, `delete_model`, - `get_diffusers_state`, `update_diffusers`. - - This widened the `acknowledged_cost` gate past its original GPU-only - charter. `download_model` (tens of gigabytes), `delete_model` - (unrecoverable without re-downloading) and `update_diffusers` (an untagged - development build, no undo) each carry it, with a distinct refusal message - apiece: one shared message would be wrong for each of them in a different - way, and a gate the user learns to wave through is not a gate. The two - cancels stay ungated — they end a cost rather than starting one, and - gating them would make the safe direction the harder one. -- **F5 — Prompt library tools.** `/api/prompts*` and `/api/enhance`, for - an agent that helps compose prompts rather than only workflows. -- **F6 — MCP resources and prompts.** Re-expose the schema, workflows, and - gallery as resources, and ship a "diagnose this job" prompt template, - once target clients support them. -- **F7 — Remote/auth story.** Required before the embedded-in-app phase. - Blocks any non-localhost exposure of either the REST API or the MCP - server. -- **F8 — The MCP server imported the whole engine.** *(Found during the v1 - build; not part of the original scoping. Done 2026-09-01.)* The package - shipped as `dw/mcp/`, and importing any `dw.*` submodule runs - `dw/__init__.py`, which imports torch and diffusers — so a process that is - a pure HTTP client, owning no models, paid for the model framework at - startup. Measured: `import dw` 1.05s (1.40s cumulative) against 0.35s for - the client's own dependencies (`httpx`, `PIL`, `mcp`). Paid once per - client session, since a stdio server is spawned once and lives for the - session — a real but modest cost, which is why this is recorded with its - numbers rather than asserted as severe. Fixed by moving the package to a - top-level `dw_mcp/`, taking startup to 0.36s with torch never imported. - A regression guard in `tests/test_mcp_server.py` asserts the boundary - (`torch`, `diffusers` and `dw` absent from `sys.modules` after importing - `dw_mcp.server`), because the failure mode is one convenience import away - and is otherwise invisible. From f18a6868fb59848629dcc2b0c0da29ae00201cac Mon Sep 17 00:00:00 2001 From: Don Kackman Date: Sun, 6 Sep 2026 19:14:27 -0500 Subject: [PATCH 2/2] Update workspace proposals: implement stages 1-3 and remove obsolete workspaces.md --- docs/proposals/remote-gpu-server-complete.md | 197 -------------- docs/proposals/server-workspaces.md | 7 +- docs/proposals/workspaces.md | 255 ------------------- 3 files changed, 6 insertions(+), 453 deletions(-) delete mode 100644 docs/proposals/remote-gpu-server-complete.md delete mode 100644 docs/proposals/workspaces.md diff --git a/docs/proposals/remote-gpu-server-complete.md b/docs/proposals/remote-gpu-server-complete.md deleted file mode 100644 index c5bce0b8..00000000 --- a/docs/proposals/remote-gpu-server-complete.md +++ /dev/null @@ -1,197 +0,0 @@ -# Proposal: running dw entirely on a dedicated GPU server - -Status: **accepted and implemented** - hence the `-complete` in this file's -name. See the design in -[docs/superpowers/specs/2026-09-04-remote-gpu-server-design.md](../superpowers/specs/2026-09-04-remote-gpu-server-design.md) -and the user guide in [REMOTE.md](../REMOTE.md). The notes below are the -original scoping and are kept for the reasoning; one claim in them turned -out wrong: direct-LAN browser access did **not** already work, because the -`Origin` check accepted loopback origins only. Decisions taken: MCP both as -stdio and mounted in `dw.serve --mcp`; plain HTTP + token on the LAN with a -reverse proxy or Tailscale beyond it; a shipped systemd unit; install still -via clone + install.sh. - -## What already works, unmodified - -This is closer to "already supported, thinly" than "not supported." -`dw.serve --host 0.0.0.0 --token "..."` plus an SSH tunnel or LAN reach -already gets a browser talking to a remote GPU box today - see -[SERVER.md](../SERVER.md)'s Authentication section. What's missing is doing -the same for the MCP client, and tightening the network-facing edges that -currently assume "your own GPU to your own browser" (`dw/serve.py:6`) -rather than "a GPU box you deliberately expose to one or two trusted -clients." - -- **Bind address is already a flag.** `--host`/`--port` (`dw/serve.py:23-27`) - need no change to listen on `0.0.0.0` or a specific LAN interface. -- **Token auth already exists and covers the REST/SSE/UI surface.** - `DW_API_TOKEN`/`--token` (`dw/serve.py:44-49`, `docs/SERVER.md:220-249`) - - constant-time bearer check on `/api/*`, with the three download/thumbnail/ - SSE routes accepting `?token=` for the cases a browser can't attach a - header. This is a single shared static secret, not a login system - true - today and still true after anything proposed below. -- **`Origin`/`Host` checks are already DNS-rebinding-aware for a wildcard - bind** (`docs/SERVER.md:186-200`) - nothing to add there. -- **The web UI already has a token field** (`docs/SERVER.md:246-249`) - - storing the token in `localStorage`, attached to every call. Pointing a - browser at a remote box today is: open the token field, paste it, done. - -## Gaps - -### 1. `dw-mcp` is explicitly told not to do this - -[MCP.md](../MCP.md#security) is direct about it: - -> Because it adds no authentication, `dw-mcp` must not be pointed at a -> `dw.serve` reachable beyond localhost. Treat `--url`/`DW_MCP_URL` the same -> way you would treat opening the web UI to the network: don't. - -That statement predates `--token` being usable end-to-end for this case, but -re-reading the code, it's *only partially* stale: `DwClient` (`dw_mcp/ -client.py:70-76`) already accepts a `token` and threads it as `Authorization: -Bearer` on the `httpx.Client` it builds, and `dw-mcp --token`/`DW_API_TOKEN` -(`dw_mcp/__main__.py:22-26`) already wires that through. So a token- -authenticated `dw-mcp --url http://:8765 --token ...` pointed at a -LAN/tunnel-reachable `dw.serve` is not actually unauthenticated today - the -docs warning is broader than the current code strictly requires. What's -still true and still a real gap: - -- The warning itself needs rewriting once (if) this is scoped, so it says - "don't do this without a token" rather than "don't do this." -- `dw-mcp` has no equivalent of the UI's "the page won't load without a - token" self-check - point it at a token-protected remote server with no - `--token` given, and every tool call just 401s one at a time rather than - failing fast at startup with a clear message. -- Nothing about `dw-mcp`'s stdio transport changes here. It still runs next - to Claude Code (locally, wherever Claude Code spawns it) and reaches out - to the remote `dw.serve` over HTTP - this section is about making that - HTTP leg safe to point at a non-loopback host, not about relocating - `dw-mcp` itself. (Running `dw-mcp` *on* the GPU box as a standalone HTTP- - transport service is a materially different, larger change - see - "Related, out of scope" below.) - -### 2. No TLS story - -Both the token and every job's arguments/results travel in plaintext once -the bind address is non-loopback. An SSH tunnel (the setup already in use -informally) sidesteps this by never putting plaintext on a real network -segment - the tunnel's local end is still `127.0.0.1` to `dw.serve`. Binding -`--host 0.0.0.0` directly and reaching it by LAN IP or a tunnel-free network -path has no equivalent protection: the bearer token in the `Authorization` -header (and in the three `?token=` URLs) is readable to anything that can -see the traffic. `dw.serve` has no built-in TLS termination, and adding one -(cert management, `uvicorn`'s `--ssl-keyfile`/`--ssl-certfile`, or -documenting a reverse-proxy pattern instead) is a real, non-trivial decision -- see Proposed scope. - -### 3. No "is this server remote" signal anywhere in the UI or MCP tools - -Once a browser or `dw-mcp` is pointed at a remote box, nothing in the -response surface says so. `get_health` (MCP) / `GET /api/health` (REST) -report worker liveness but not host identity; there's no equivalent of -`whoami`/`hostname` a client could use to confirm "yes, this is the 3090 -box" versus a stale tunnel pointed at nothing. Minor, but worth a line in -the health payload if this gets built out, since a remote setup is exactly -the situation where "which box am I actually talking to" stops being -obvious from context. - -### 4. Long-running server lifecycle on a headless box - -Today `dw.serve` is a foreground process a developer starts by hand in a -terminal (`docs/SERVER.md`'s examples are all bare `python -m dw.serve`). -Run "entirely on a dedicated GPU server" implies the process survives an SSH -disconnect, restarts after the box reboots, and its logs are somewhere a -person can find them without a live terminal attached - none of which is -dw's concern to build (this is systemd-unit/launchd-plist territory, not -application code), but worth naming as part of "the setup" even though it's -zero lines of code in this repo. - -## Proposed scope - -Two small code changes, everything else is documentation/ops: - -### A. Fail fast in `dw-mcp` when talking to a non-loopback host with no token - -At `DwClient` construction (`dw_mcp/client.py:70-76`) or in `dw_mcp/ -__main__.py:main()`, check: if `resolve_base_url(...)`'s host is not -loopback and no token was resolved, print a clear stderr warning (or refuse -to start, mirroring `dw.serve`'s own non-loopback-without-token startup -warning at `docs/SERVER.md:189`) instead of leaving every subsequent tool -call to fail as an unexplained 401. Small, self-contained, `dw_mcp/client.py` -+ `dw_mcp/__main__.py` only. - -### B. Rewrite the MCP.md security warning - -Replace the blanket "don't" with the actual constraint: a token- -authenticated remote `dw.serve` is supported the same way the web UI -supports it, plaintext transport risk and all - point `dw-mcp` at a remote -host only over a channel you already trust (SSH tunnel, VPN, or a LAN you -control), the same guidance `docs/SERVER.md`'s Authentication section -already gives for the browser case. Documentation-only, `docs/MCP.md`. - -### Explicitly out of scope for phase 1 - -- **TLS termination inside `dw.serve`.** The tunnel already solves transport - security for the primary case (SSH access to the box); a from-scratch TLS - story is a bigger, separate decision (self-signed vs. real certs, cert - rotation, whether to punt to a reverse proxy like Caddy/nginx in front of - uvicorn instead of adding cert flags to `dw/serve.py` itself). Worth its - own proposal if the LAN-without-tunnel case becomes a real requirement - rather than an occasional convenience. -- **Multi-client / multi-user auth.** The single shared static token is a - deliberate, already-documented limitation (`docs/SERVER.md:254-257`). - Nothing here changes that model - "connect from browser and agent - terminal" is two clients sharing one token, which the existing model - already handles. -- **A process supervisor / systemd unit shipped in this repo.** Lifecycle - management (section 4 above) is host-specific and belongs in the user's - own dotfiles/ops setup, not in `dw/` or `pyproject.toml`. -- **Relocating `dw-mcp` itself onto the GPU box as a standalone HTTP- - transport MCP service** (as opposed to running it locally and pointing it - at a remote `dw.serve`). That's a different mechanism - `dw_mcp/server.py` - only wires up `mcp.server.stdio` today (`dw_mcp/__main__.py:38`) - and a - materially larger change (new transport, its own auth story, a persistent- - service lifecycle for `dw-mcp` itself) than what this document scopes. - Worth a separate proposal if a subprocess-per-Claude-Code-session model - stops being good enough. - -## Recommended setup today, no code changes - -Since (A) and (B) above are small and this is genuinely mostly-already- -supported, the practical setup right now, before any of this is built: - -```bash -# On the GPU box -python -m dw.serve --token "$(openssl rand -hex 32)" # stays on 127.0.0.1 - -# On the client machine -ssh -L 8765:127.0.0.1:8765 user@gpu-box -N -f -export DW_API_TOKEN="" -dw-mcp # defaults to http://127.0.0.1:8765, now tunneled -# browser: http://127.0.0.1:8765, paste the token once in the UI's token field -``` - -This needs the token even though the tunnel already authenticates by SSH key -- the token is what stops anything else on the GPU box's loopback interface -(other users, other processes) from reaching the API through the tunnel's -local end. Everything in "Proposed scope" above is about making the direct -(`--host 0.0.0.0`, no tunnel) variant of this equally safe and equally -well-signposted, not about enabling something that doesn't work today. - -## Open questions - -- Is the SSH-tunnel setup (works today, zero code changes) actually - insufficient for the day-to-day flow, or does this proposal's real value - turn out to be just (A) and (B) - two small papercut fixes - rather than - anything bigger? -- If a from-scratch TLS story ever gets scoped, does it belong in - `dw/serve.py` (uvicorn's own `--ssl-keyfile`/`--ssl-certfile`) or is the - right answer "document a reverse-proxy pattern and explicitly decline to - add cert handling to the application"? -- Does the per-plugin-trust-list work in - [plugin-extensibility.md](plugin-extensibility.md) intersect with this at - all (a remote, less-trusted box being a reason to want per-plugin trust - rather than the single `--trust-workflows` switch)? Worth a cross-check - once/if both move forward, not blocking either individually. - -Citations are file:line references at the time of writing and may drift. diff --git a/docs/proposals/server-workspaces.md b/docs/proposals/server-workspaces.md index 4bbc9898..80878cb0 100644 --- a/docs/proposals/server-workspaces.md +++ b/docs/proposals/server-workspaces.md @@ -1,6 +1,11 @@ # Proposal: workspaces on the server, and client workspaces that mirror into them -Status: draft / scoping only — no implementation. +Status: stages 1–3 implemented — named server-side workspaces +(`/api/workspaces` CRUD, `dw/server/app.py`), the UI workspace switcher +(`ui/src/lib/WorkspacePicker.svelte`), and MCP workspace selection +(`dw_mcp/workspaces.py`). Stages 4–5 (client-side mirroring, `fetch_run`) are +designed here but deliberately not planned — see "Mirroring" and "Staging" +below. Follows [workspaces.md](workspaces.md), whose stages one through five and stage six level one are implemented. Replaces the first draft of level two, which made a client-side workspace the system of record and submitted runs as diff --git a/docs/proposals/workspaces.md b/docs/proposals/workspaces.md deleted file mode 100644 index a7d2b58a..00000000 --- a/docs/proposals/workspaces.md +++ /dev/null @@ -1,255 +0,0 @@ -# Proposal: workspaces — decoupling content from the repo (and from the server) - -Status: draft / design only — no implementation. - -## Problem - -The project grew CLI-first, where "outputs next to the code" was correct and -free. A workflow file, the assets it reads, the prompts it references and the -files it writes all resolved relative to the checkout, and that was the whole -story. The web UI inherited that model and mostly gets away with it — it is -single-user and usually started from the repo root. The MCP server is where it -stops working: an agent authoring a workflow writes it into the repo's example -corpus, cannot supply input bytes at all, and has no notion of "my stuff" -versus "the examples that ship with the project". - -The examples use case is still valid — `workflows/` as a runnable corpus is one -of the better things about the project. What is missing is a second place for -everything that is *not* an example. - -## Current behavior - -Four roots, four different resolution rules, three of them anchored to the -process working directory: - -| Content | Where it resolves | How | -|---|---|---| -| Workflow files | CLI: any path given on the command line. Server: `--workflow-dir`, default `./workflows`. Packaged: `builtin:name.json` → `dw/workflows/` | `workflow_from_file` / `resolve_workflow_reference` (`dw/server/app.py:192`) | -| Input assets | The **workflow file's own directory** | `resolve_relative_path(path, base_dir)` (`dw/arguments.py:769`); `base_dir` is `dirname(file_spec)` | -| Stored prompts | `DW_PROMPT_DIR` → `./prompts` → nearest `prompts/` walking up from the workflow file | `get_prompt_dir` (`dw/prompts.py:30`) | -| Generated output | `-o/--output-dir`, default `./outputs`, plus a subfolder mirroring the workflow's position under the nearest directory literally named `workflows` | `workflow_output_subfolder` (`dw/workflow.py:104`), `Workflow.effective_output_dir` | -| Settings, job history, logs | `DIFFUSERS_HELPER_ROOT` → `~/.diffusers_helper/` | `dw/settings.py` | - -Only the last row is already decoupled. Only prompts has an environment -variable and a discovery walk; there is no `DW_WORKFLOW_DIR` or -`DW_OUTPUT_DIR`, so "start it from the repo root" is load-bearing for -everything else. - -### What that costs today - -- **Output layout is coupled to repo layout.** `workflow_output_subfolder` - keys the output subfolder off a path segment literally named `workflows`. - Move a workflow out of that tree and its outputs silently flatten into the - output root. The *filesystem shape of the checkout* is the grouping key. -- **Assets must live beside the workflow.** Because `base_dir` is the workflow - file's directory, any workflow with inputs drags a sibling `assets/` folder - into the repo — hence `.gitignore`'s `workflows/*/assets/`, `local_inputs/`, - `/*.mp4`, `/*.wav`. Generated media is being gitignored *inside the source - tree* rather than kept out of it. -- **The output→input loop is a manual copy.** The GYRE workflows reference - `assets/Gyre-score_choir.10-0.0.wav` and `assets/Gyre-still_1_iris.0-0.0.jpg` - — files a previous run wrote to `outputs/` and a human copied back into the - repo. Multi-stage work (the marmot passes, `outputs/marmot_pass1..3`) is - exactly the day-to-day case, and the engine has no vocabulary for "the thing - the last stage made". -- **Inputs and outputs share a directory on the server.** `POST /api/uploads` - writes to `/uploads` (`dw/server/app.py:1233`) because the output - dir is the only writable root the server knows about. -- **History and output can disagree.** `jobs.sqlite` lives under - `~/.diffusers_helper/` and stores manifest paths *relative to* whatever - `--output-dir` the server was started with (`dw/server/jobs.py:594`). Start - it from a different cwd and old rows point at nothing. A CLI run records no - manifest at all. -- **MCP writes into git.** `save_workflow` → `PUT /api/workflows/{name}` → - the server's `workflow_dir`, which defaults to the repo's example corpus. - There is no writable/read-only distinction, and no upload tool at all - (`dw_mcp/CLAUDE.md` notes `POST /api/uploads` is deliberately uncovered), so - an agent can author a workflow it has no way to supply inputs for. -- **`workflow_dir` is doing two jobs.** It is both "where workflows are kept" - and the server's *confinement boundary* for `workflow_path`, inline - `base_dir`, and sub-workflow steps. Any change to the first has to preserve - the second. - -### What is already right - -The mechanism is mostly there; the defaults and the discovery rules are what -bind it to the repo. - -- `--workflow-dir` / `--output-dir` / `--prompt-dir` already exist as explicit - parameters. Nothing in the engine *requires* the repo. -- `builtin:` is already the precedent for "packaged, read-only, reachable by a - scheme rather than a path". -- `prompt:name` is already a library reference rooted at a library rather than - at the workflow file — the exact shape assets need, including its validator - (`validate_prompt_reference`) and its no-double-resolution rule. -- `DIFFUSERS_HELPER_ROOT` already establishes a user-data root outside the - checkout. -- Path validation is centralized in `dw/security.py`, so a new root is one - containment base, not a new class of check. - -## Proposal - -### 1. Name the container: a workspace - -One directory holding everything a user generates or curates: - -``` -/ - workflows/ # mine, writable - prompts/ # the existing prompt library - assets/ # input media, incl. uploads/ - outputs/ # runs -``` - -A single resolver (`dw/workspace.py`) replaces three ad-hoc rules: - -1. `--workspace` flag -2. `DW_WORKSPACE` -3. `workspace` in `~/.diffusers_helper/settings.json` -4. the working directory, *if it looks like a workspace* — contains any of - `workflows/`, `prompts/`, `outputs/` -5. `~/diffusers-workflow/`, created on demand - -Rule 4 is what keeps this backward compatible: a repo checkout looks like a -workspace, so running from the repo root behaves exactly as it does now. Only -a bare working directory falls through to the home workspace. The existing -per-slot flags stay and continue to override individual roots, so no current -invocation changes meaning. `get_prompt_dir`'s walk-up becomes a deprecated -fallback rather than the primary rule. - -### 2. A workflow search path, writes to the front - -Rather than one `workflow_dir`, an ordered list: - -``` -workspace/workflows/ (writable) -/workflows/ (read-only, if present) -dw/workflows/ (read-only, packaged — today's builtin:) -``` - -Reads resolve front-to-back; **writes only ever go to the first entry**. That -alone fixes the MCP-writes-into-git problem while keeping the example corpus -first-class and browsable. `GET /api/workflows` gains `origin` and `writable` -per entry; the UI and MCP grey out save/delete on non-writable ones, and -"save as" from an example into the workspace becomes the natural gesture. -`examples:Foo.json` joins `builtin:` as an explicit scheme. - -Confinement generalizes cleanly: the boundary becomes *the workspace root plus -the read-only roots* instead of a single `workflow_dir`. Server-side runs stay -confined; CLI runs of an arbitrary file stay unconfined, as today -(`workflow_from_file`'s docstring already states that a locally-run file is not -a trust boundary). - -### 3. Run directories, and output layout from workflow identity - -Replace the `workflow_output_subfolder` path-segment hack with the workflow's -own identity — its name, or its path relative to the search-path root it came -from — and give each run its own directory: - -``` -outputs/// - - manifest.json # spec, arguments, seeds, resolved model ids, saved files -``` - -This is the change that makes multi-stage work tractable: intermediates, final -outputs and provenance are colocated, prunable as a unit, and addressable. -`manifest.json` also means a CLI run leaves a record, and `jobs.sqlite` demotes -from sole record to index — history becomes rebuildable from disk. - -Costs to plan for: `dw/step_cache.py` compares `entry["output_dir"]`; the -gallery scans `output_dir` recursively and would want to group by run; the UI -builds `/outputs/` URLs from paths relative to `output_dir`. A flat mode -should stay available for CLI users who prefer today's layout. - -### 4. First-class assets, and references that close the loop - -Two new reference prefixes, built on the `prompt:` machinery rather than beside -it: - -- `asset:name` / `asset:folder/name` — rooted at `workspace/assets/`, not at - the workflow file. A workflow stops needing to live next to its inputs, which - is what currently drags media into the source tree. -- `output:/` (or a `latest` selector per workflow) — reference a - previous run's product directly. This removes the manual copy-back that the - GYRE and marmot workflows document. - -Both resolve in `realize_args` where `prompt:` does, reuse -`validate_prompt_reference`-style validation, and inherit the existing rule -that a resolved value may not itself begin with a reference prefix. - -`POST /api/uploads` moves to `workspace/assets/uploads/`. Inputs and outputs -stop sharing a directory, and — because the destination is no longer "the -output dir" — an MCP `upload_asset` tool becomes reasonable to add. - -### 5. Decoupling MCP from the server - -Two levels, worth doing in order: - -**Level 1 — workspace-aware client.** `get_server_info` reports the workspace, -its roots and the writable flags. `save_workflow` targets the workspace, never -an example root. Add `upload_asset` so an agent can supply input bytes. MCP -stays a pure HTTP client of a running `dw.serve`; the boundary -`dw_mcp/CLAUDE.md` describes (no `dw.*` imports, no torch) is untouched. - -**Level 2 — client-side workspace.** `dw-mcp --workspace ` keeps -workflows, prompts and assets *on the agent's machine*. Authoring, schema -validation and library management are local; only runs go to the remote -`dw.serve`, submitted inline (`POST /api/jobs` with `workflow` + `base_dir`, -already supported) with assets pushed content-addressed — hash-named, skipped -if the server already has them. This is the literal answer to "not everything -should be on the server": the server becomes a GPU, and the workspace is the -user's. - -Level 2 has one real snag worth deciding early: inline submission's `base_dir` -is confined to the server's workflow root, so a locally-authored workflow's -asset references have to be rewritten to server-side asset ids at submit time. -That is tractable but it is the design work, not a detail. - -Trust interacts here too: `--trust-workflows` is process-wide today. Per-root -trust ("my workspace is trusted, the shared example corpus and anything an -MCP client submits is not") is a better-shaped knob once roots are named -things, and it is the knob that lets a server usefully run untrusted while an -owner's own workflows still use `pre_load_modules`. - -### 6. Multiple workspaces - -`.gitignore` already carries `local_projects/`, so this is happening -informally. `dw workspace new|list|use`, `--workspace` on every entry point, a -switcher in the UI later. One active workspace per process — no ambiguity about -which root a bare name resolved against. - -## Staging - -Each stage is independently shippable and the first is behavior-neutral. - -1. **`dw/workspace.py` + `--workspace`/`DW_WORKSPACE`.** One resolver, all - entry points consume it, defaults unchanged in a repo checkout. Nothing - moves yet. -2. **Assets root and `asset:` references.** Move uploads out of the output - directory. Workflows can stop hoarding sibling `assets/` folders. -3. **Run directories and on-disk `manifest.json`.** Output layout derives from - workflow identity instead of file position. Touches step cache, gallery - grouping and the UI's output URLs — the biggest single stage. -4. **Workflow search path, writable-first, `examples:`.** `origin`/`writable` - through the API into the UI and MCP. This is the stage that stops agents - writing into git. -5. **`output:` references.** Closes the multi-stage loop. -6. **MCP level 1** (done), **then level 2** - scoped separately in - [server-workspaces.md](server-workspaces.md), which reframes it: named - workspaces on the server, with a client workspace mirroring into one, - rather than a client-side system of record submitting inline. Level 1 - had already removed the `base_dir` confinement problem this document - called level 2's real design work. - -## Open questions - -- Does the repo's top-level `workflows/` stay as an example root on the search - path, or migrate wholesale into the package next to `dw/workflows/` so a - pip install has the same corpus a checkout does? The second is cleaner and is - a bigger move. -- Run id: job id for server runs, but CLI runs have none. Timestamp plus a - short hash of the resolved spec would serve both and dedupes reruns. -- Should `outputs/` be inside the workspace at all, or a peer with its own - setting? Video work fills disks, and the outputs root is the one people most - plausibly want on a different volume.