Skip to content

Commit 7da28ab

Browse files
committed
Enhance metadata handling and logging: add safety checker warnings, embed real prompts in metadata, and ensure unique gallery URLs for rewritten files
1 parent cb6caf2 commit 7da28ab

9 files changed

Lines changed: 368 additions & 9 deletions

File tree

‎docs/superpowers/scope/mcp.md‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# MCP Server for diffusers-workflow — Scoping Doc
2+
3+
## Background
4+
5+
`diffusers-workflow` (dkackman/diffusers-workflow) is a declarative JSON
6+
workflow engine for Hugging Face Diffusers, with a local HTTP server
7+
(`dw.serve`, default port 8765) that exposes a full REST API: workflow
8+
CRUD, a job queue with SSE progress streaming, model management,
9+
introspection of diffusers pipeline signatures, and a gallery of past
10+
generations with embedded metadata.
11+
12+
Today, workflow authoring and debugging, for the project developer,
13+
can happen via Claude Code in VS Code,
14+
with direct repo/filesystem access: "create a workflow that does X," or
15+
"workflow Y produces bad output, find out why" — Claude Code runs the
16+
workflow, reads logs, checks GPU memory, and proposes fixes to code or
17+
workflow config. It has even gone so far as to identify bad RAM causing
18+
intermittent generation failures.
19+
20+
That flow works well for the developer (me), but it assumes a dev
21+
environment: a repo checkout, a terminal, VS Code, comfort reading stack
22+
traces. The next audience is **non-developer end users** of the packaged
23+
product, who won't have any of that. They'll have the app (web UI /
24+
`dw.serve`, or an embedded product surface), not a coding agent with shell
25+
access.
26+
27+
## Goal
28+
29+
Turn this from a dev-only workflow authoring/debugging tool into a **non-dev-friendly MCP server** that exposes the same "author a workflow / run it / diagnose why it
30+
failed or produced bad output" capability, but in a structured, bounded way that
31+
doesn't require shell access or a dev environment. The MCP server will wrap the existing `dw.serve` HTTP API, exposing a constrained tool surface for workflow authoring and debugging, suitable for non-developer users.
32+
33+
The modes the underlying workflow can be authored and run today are::
34+
35+
- a text editor and CLI `python -m dw.run workflow.json`
36+
- with command line REPL `python -m dw.repl`
37+
- via the HTTP API `dw.serve` and existing web UI
38+
39+
So this adds a fourth mode, which would be cool if we turned this into an AI first product: an MCP server that can be driven by a Claude Code agent, or eventually a local LLM-backed agent, to author and run workflows without deep
40+
knowledge of diffusers, my workflow engine, or any of that complexity. The MCP server would expose a structured tool surface for workflow authoring and debugging, suitable for non-developer users.
41+
42+
- via the MCP server (new) — structured tool surface
43+
44+
Expose the same "author a workflow / run it / diagnose why it failed or
45+
produced bad output" capability as an **MCP server**, wrapping the existing
46+
`dw.serve` HTTP API. This lets any MCP-compatible client — Claude Code
47+
first, later Claude Desktop/claude.ai, and eventually a local-LLM-backed
48+
client — drive the engine through a bounded, structured tool surface
49+
instead of raw shell/API access.
50+
51+
## Why MCP (not just keep using Claude Code)
52+
53+
- Claude Code assumes a dev environment; end users won't have one.
54+
- MCP tools are a defined, constrained surface (structured in/out, no
55+
arbitrary code execution) — appropriate once non-developers are the
56+
audience, and safer than raw shell access.
57+
- The underlying HTTP API already exists and is already defensive
58+
(path validation, traversal blocking, `Origin` header checks, JSON size
59+
limits) — MCP adds a protocol layer in front of it, not new attack
60+
surface.
61+
- Protocol-agnostic: same server can later be pointed at Claude Code, a
62+
future embedded chat UI in the product, or a local-LLM-backed client.
63+
64+
## Rollout sequence (decided)
65+
66+
1. **Start with MCP server + Claude Code** as the client, for continued dev
67+
use and to validate the tool surface.
68+
2. **Move to embedded-in-app** later, once the product is packaged for
69+
non-developer users. This later phase will need its own auth story,
70+
since the server currently binds to `127.0.0.1` with no authentication
71+
(see Security, below) — fine for local/trusted-LAN use, not fine for a
72+
hosted/remote product.
73+
3. **Local LLM support** stays a goal throughout — keep the option open
74+
for users who won't have/want a paid Claude subscription. MCP is
75+
protocol-agnostic, so this is a separate, later concern from the MCP
76+
server itself, not a blocker to starting.
77+
78+
## Proposed MCP tool set
79+
80+
Mapped to existing `dw.serve` routes — no new server-side endpoints needed
81+
for the initial build.
82+
83+
### Read-only / low-cost (build first)
84+
85+
| Tool | Route(s) | Notes |
86+
| --- | --- | --- |
87+
| `list_workflows` | `GET /api/workflows` | descriptions, output kinds, variable counts |
88+
| `get_workflow(name)` | `GET /api/workflows/{name}` | full JSON |
89+
| `validate_workflow(json)` | `POST /api/validate` | schema + signature-level argument checks; no GPU cost — good pre-flight tool |
90+
| `get_schema` | `GET /api/schema` | workflow JSON schema |
91+
| `get_pipeline_signature(name)` | `GET /api/pipelines/{name}` | lets the agent check real pipeline arguments before proposing a config fix |
92+
| `list_classes(kind)` / `get_class(name)` | `GET /api/classes?kind=...`, `GET /api/classes/{name}` | broader introspection beyond pipelines |
93+
| `list_tasks` / `get_task(command)` | `GET /api/tasks`, `GET /api/tasks/{command}` | task-step argument schemas |
94+
| `list_models` | `GET /api/models` | hub cache inventory: sizes, revisions, last-used |
95+
| `get_memory` | `GET /api/memory` | worker VRAM/RAM stats — answers the GPU-diagnosis need directly, no new endpoint required |
96+
| `get_health` | `GET /api/health` | liveness |
97+
| `get_gallery_item(name)` | `GET /api/gallery/{name}/metadata` | pull back a prior run's exact workflow + seed, e.g. to reproduce a failure |
98+
99+
### Diagnostic loop (build second)
100+
101+
| Tool | Route(s) | Notes |
102+
| --- | --- | --- |
103+
| `run_workflow(workflow_path \| inline_workflow, arguments)` | `POST /api/jobs` | **Costs GPU time; one job runs at a time.** See confirm-gating below. |
104+
| `get_job(job_id)` | `GET /api/jobs/{id}` | full detail: spec, events, manifest, error — primary diagnosis payload |
105+
| `stream_job_events(job_id, after?)` | `GET /api/jobs/{id}/events` | SSE; `job_status`/`phase`/`memory`/`log` events cover exactly the "instrument the process" need (phase transitions: loading → generating → decoding → saving) |
106+
| `cancel_job(job_id)` | `POST /api/jobs/{id}/cancel` | cooperative cancel |
107+
| `rerun_job(job_id)` | `POST /api/jobs/{id}/rerun` | re-queue after a fix |
108+
| `move_job(job_id, direction)` | `POST /api/jobs/{id}/move` | reorder queued jobs |
109+
110+
### Deferred / optional
111+
112+
- `POST /api/models/download`, `DELETE /api/models`, `/api/system/diffusers*`
113+
— model and dependency management; useful eventually, not needed for the
114+
first author/run/diagnose loop.
115+
- Prompt library endpoints (`/api/prompts*`, `/api/enhance`) — relevant if
116+
the agent should also help build/enhance prompts, not core to diagnosis.
117+
118+
## Key design decisions
119+
120+
1. **`run_workflow` is confirm-gated.** A run costs real GPU time (and
121+
possibly money) and the engine is single-GPU/single-job-at-a-time. MCP
122+
has no built-in elicitation/confirmation primitive, so the confirm step
123+
should live in the client/agent behavior (state what's about to run and
124+
get explicit go-ahead) rather than the tool silently firing.
125+
`validate_workflow` is free and can run without confirmation first.
126+
127+
2. **Security model carries over, doesn't change.** The server already
128+
validates every path (`dw/security.py`), blocks traversal, rejects
129+
non-local `Origin` headers, and confines file access to the configured
130+
workflow/output/prompt directories. MCP wrapping this doesn't add new
131+
risk for the local/Claude-Code phase. It does **not** solve remote
132+
auth — the server binds to `127.0.0.1` by default with no
133+
authentication; `--host 0.0.0.0` exists but is explicitly documented as
134+
trusted-LAN-only. The embedded-in-app phase needs a real auth story
135+
before any hosted/remote exposure.
136+
137+
3. **Build order:** read-only tools first (workflow authoring assistant
138+
usable almost immediately), then the run/diagnose loop once the
139+
confirm-gating pattern for `run_workflow` is settled.
140+
141+
## Open questions for implementation
142+
143+
- Which MCP transport/SDK — Python `mcp` SDK (project is already Python)
144+
talking to `http://127.0.0.1:8765/api/...` via `httpx`, run as a stdio
145+
server for Claude Code.
146+
- Exact confirmation UX for `run_workflow` — client-side prompt text,
147+
and whether to surface estimated cost/time if the API can supply it.
148+
- Whether GPU/worker diagnostics need anything beyond `/api/memory`, or
149+
whether that's sufficient for the diagnostic loop as scoped.

‎dw/pipeline_processors/pipeline.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,7 @@ def _run_once(self, arguments):
456456
output = output.to("cpu")
457457

458458
attach_audio_sample_rate(self.pipeline, output)
459+
warn_if_safety_checker_blanked(output)
459460

460461
return output
461462

@@ -1056,6 +1057,31 @@ def get_component(pipeline, component_name):
10561057
return component
10571058

10581059

1060+
def warn_if_safety_checker_blanked(output):
1061+
"""Say so when a safety checker replaced a generated image with a black one.
1062+
1063+
Stable Diffusion 1.5's checker false-positives readily, and it returns a
1064+
solid black image rather than an error. Run to run that reads as the seed
1065+
having no effect - the same result every time - so the reason belongs in
1066+
the log where the identical images do.
1067+
1068+
Args:
1069+
output: The pipeline output, which may carry nsfw_content_detected
1070+
"""
1071+
flags = getattr(output, "nsfw_content_detected", None)
1072+
if not flags:
1073+
return
1074+
1075+
blanked = sum(1 for flag in flags if flag)
1076+
if blanked:
1077+
logger.warning(
1078+
f"The safety checker blanked {blanked} of {len(flags)} generated "
1079+
"images - they are solid black, and no seed will change that. "
1080+
"Pass 'safety_checker': null in from_pretrained_arguments to "
1081+
"load the pipeline without it."
1082+
)
1083+
1084+
10591085
def attach_audio_sample_rate(pipeline, output):
10601086
"""Record the vocoder's sample rate on an output that carries generated audio.
10611087

‎dw/server/app.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,8 +678,11 @@ def gallery(limit: int = 200):
678678
{
679679
"name": name,
680680
# Quoted: a name carrying '#', '?' or '%' would otherwise
681-
# break the src the gallery renders it into
682-
"url": f"/outputs/{quote(name)}",
681+
# break the src the gallery renders it into. The mtime
682+
# rides along because a rerun overwrites the same name -
683+
# on an unchanging URL the browser would keep showing the
684+
# image it cached from the previous run
685+
"url": f"/outputs/{quote(name)}?v={int(stat.st_mtime)}",
683686
"kind": kind,
684687
"size": stat.st_size,
685688
"mtime": stat.st_mtime,

‎dw/step.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ def run(self, previous_results, previous_pipelines, step_action):
6767
iterations = get_iterations(step_action.argument_template, previous_results)
6868
logger.debug(f"Generated {len(iterations)} argument combinations")
6969

70+
# The metadata above was read from the definition, where an
71+
# argument may still be a 'previous_result:' reference - now that
72+
# the iterations exist, record what the step actually ran with
73+
if result.metadata is not None:
74+
_realize_metadata_arguments(result.metadata, iterations)
75+
7076
# Execute the action for each set of arguments
7177
if not iterations:
7278
logger.warning(f"Step {step_name} has no iterations to execute")
@@ -130,3 +136,38 @@ def _collect_metadata(self):
130136
metadata["arguments"] = dict(task_def.get("arguments", {}))
131137

132138
return metadata
139+
140+
141+
# What json.dumps can embed without falling back to str() - anything else
142+
# (a PIL image, a tensor, a video's frames) keeps its reference in metadata
143+
JSON_SCALARS = (str, int, float, bool)
144+
145+
146+
def _realize_metadata_arguments(metadata, iterations):
147+
"""Replace the embedded arguments with the values the step really ran with.
148+
149+
Step metadata is collected from the definition, so an argument written as
150+
"previous_result:expand" is still a reference there - a gallery showing it
151+
would display the reference instead of the prompt the pipeline saw. The
152+
iterations hold the resolved values, so lift the JSON-safe ones back into
153+
the metadata. When iterations disagree (a step fanned out over several
154+
prior results) every distinct value is kept, in order.
155+
"""
156+
arguments = metadata.get("arguments")
157+
if not arguments or not iterations:
158+
return
159+
160+
for key in list(arguments):
161+
values = []
162+
for iteration in iterations:
163+
if not isinstance(iteration, dict) or key not in iteration:
164+
values = []
165+
break
166+
value = iteration[key]
167+
if not isinstance(value, JSON_SCALARS):
168+
values = []
169+
break
170+
if value not in values:
171+
values.append(value)
172+
if values:
173+
arguments[key] = values[0] if len(values) == 1 else values

‎tests/test_pipeline_components.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from dw.pipeline_processors.pipeline import (
1010
Pipeline,
11+
warn_if_safety_checker_blanked,
1112
configure_components,
1213
declared_component_names,
1314
enable_cache_on_transformer,
@@ -449,3 +450,34 @@ def test_remote_text_encoder_does_not_edit_the_definition(self, monkeypatch):
449450

450451
assert arguments["text_encoder"] is None
451452
assert "text_encoder" not in definition["from_pretrained_arguments"]
453+
454+
455+
class TestSafetyCheckerWarning:
456+
"""A blanked image looks like a repeated result - it has to say so."""
457+
458+
def test_warns_when_the_safety_checker_blanked_an_image(self, caplog):
459+
output = MagicMock()
460+
output.nsfw_content_detected = [False, True]
461+
462+
with caplog.at_level("WARNING", logger="dw"):
463+
warn_if_safety_checker_blanked(output)
464+
465+
assert "safety checker" in caplog.text.lower()
466+
assert "1 of 2" in caplog.text
467+
468+
def test_silent_when_nothing_was_flagged(self, caplog):
469+
output = MagicMock()
470+
output.nsfw_content_detected = [False, False]
471+
472+
with caplog.at_level("WARNING", logger="dw"):
473+
warn_if_safety_checker_blanked(output)
474+
475+
assert caplog.text == ""
476+
477+
def test_silent_for_a_pipeline_without_a_safety_checker(self, caplog):
478+
output = MagicMock(spec=[])
479+
480+
with caplog.at_level("WARNING", logger="dw"):
481+
warn_if_safety_checker_blanked(output)
482+
483+
assert caplog.text == ""

‎tests/test_server.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,30 @@ def test_gallery_lists_media_and_reads_metadata(server, tmp_path):
490490
assert read_embedded_metadata(str(outputs / "meta.jpg"))["step_name"] == "gen"
491491

492492

493+
def test_gallery_urls_change_when_a_file_is_rewritten(server, tmp_path):
494+
"""A rerun overwrites the same name - the URL must move or the browser
495+
keeps showing the image it already cached."""
496+
import os
497+
from PIL import Image
498+
499+
with server(success_script) as client:
500+
outputs = tmp_path / "outputs"
501+
path = outputs / "test_image-gen.0-0.0.png"
502+
Image.new("RGB", (4, 4), "red").save(path)
503+
504+
first = client.get("/api/gallery").json()["files"][0]["url"]
505+
assert first.startswith("/outputs/test_image-gen.0-0.0.png?")
506+
assert client.get(first).status_code == 200
507+
508+
# the same file, rewritten - as a second run of the workflow does
509+
Image.new("RGB", (4, 4), "blue").save(path)
510+
os.utime(path, (0, os.stat(path).st_mtime + 10))
511+
512+
second = client.get("/api/gallery").json()["files"][0]["url"]
513+
assert second != first
514+
assert client.get(second).status_code == 200
515+
516+
493517
def test_embed_metadata_carries_the_workflow_definition(tmp_path):
494518
"""A run with embed_metadata writes an image the gallery can reopen."""
495519
from unittest.mock import patch

0 commit comments

Comments
 (0)