diff --git a/docs/superpowers/plans/2026-09-19-agent-discoverability.md b/docs/superpowers/plans/2026-09-19-agent-discoverability.md index 27349190..219f970b 100644 --- a/docs/superpowers/plans/2026-09-19-agent-discoverability.md +++ b/docs/superpowers/plans/2026-09-19-agent-discoverability.md @@ -87,63 +87,69 @@ MCP (which cannot afford it). Add narrowing, keep the default byte-identical. Add to `tests/test_server.py`, inside `class TestPromptLibrary`: ```python - def _store(self, client, name, prompt): - assert client.put(f"/api/prompts/{name}", json={"prompt": prompt}).status_code == 200 - - def test_the_listing_narrows_by_tag_and_model_and_can_omit_the_text( - self, server, tmp_path - ): - with server(success_script) as client: - self._store( - client, - "minimax/Song", - { - "text": "Global Metadata\nbpm is 58.", - "description": "a song", - "intended_model": "minimax-music3", - "tags": ["music", "score"], - }, - ) - self._store( - client, - "minimax/Fox", - { - "text": "a red fox at dawn", - "description": "a fox", - "intended_model": "minimax-h3", - "tags": ["wildlife"], - }, - ) - - # A caller that passes nothing gets what it always got - plain = client.get("/api/prompts").json() - assert "minimax/Song" in plain["prompts"] - assert plain["details"]["minimax/Fox"]["text"] == "a red fox at dawn" - assert "text_chars" not in plain["details"]["minimax/Fox"] - - # include_text=false swaps the payload for its size - slim = client.get("/api/prompts?include_text=false").json() - entry = slim["details"]["minimax/Fox"] - assert "text" not in entry - assert entry["text_chars"] == len("a red fox at dawn") - assert entry["description"] == "a fox" +def _store(self, client, name, prompt): + assert ( + client.put(f"/api/prompts/{name}", json={"prompt": prompt}).status_code == 200 + ) - # A filter narrows the names, the origins and the details together - by_model = client.get("/api/prompts?intended_model=MINIMAX-MUSIC3").json() - assert by_model["prompts"] == ["minimax/Song"] - assert list(by_model["details"]) == ["minimax/Song"] - assert list(by_model["origins"]) == ["minimax/Song"] - by_tag = client.get("/api/prompts?tag=Wildlife").json() - assert by_tag["prompts"] == ["minimax/Fox"] +def test_the_listing_narrows_by_tag_and_model_and_can_omit_the_text( + self, server, tmp_path +): + with server(success_script) as client: + self._store( + client, + "minimax/Song", + { + "text": "Global Metadata\nbpm is 58.", + "description": "a song", + "intended_model": "minimax-music3", + "tags": ["music", "score"], + }, + ) + self._store( + client, + "minimax/Fox", + { + "text": "a red fox at dawn", + "description": "a fox", + "intended_model": "minimax-h3", + "tags": ["wildlife"], + }, + ) - # Both at once, and a miss is an empty listing rather than a 404 - assert client.get( - "/api/prompts?tag=music&intended_model=minimax-h3" - ).json()["prompts"] == [] + # A caller that passes nothing gets what it always got + plain = client.get("/api/prompts").json() + assert "minimax/Song" in plain["prompts"] + assert plain["details"]["minimax/Fox"]["text"] == "a red fox at dawn" + assert "text_chars" not in plain["details"]["minimax/Fox"] + + # include_text=false swaps the payload for its size + slim = client.get("/api/prompts?include_text=false").json() + entry = slim["details"]["minimax/Fox"] + assert "text" not in entry + assert entry["text_chars"] == len("a red fox at dawn") + assert entry["description"] == "a fox" + + # A filter narrows the names, the origins and the details together + by_model = client.get("/api/prompts?intended_model=MINIMAX-MUSIC3").json() + assert by_model["prompts"] == ["minimax/Song"] + assert list(by_model["details"]) == ["minimax/Song"] + assert list(by_model["origins"]) == ["minimax/Song"] + + by_tag = client.get("/api/prompts?tag=Wildlife").json() + assert by_tag["prompts"] == ["minimax/Fox"] + + # Both at once, and a miss is an empty listing rather than a 404 + assert ( + client.get("/api/prompts?tag=music&intended_model=minimax-h3").json()[ + "prompts" + ] + == [] + ) - # The writable directory is reported whatever the filter - assert client.get("/api/prompts?tag=music").json()["prompt_dir"] + # The writable directory is reported whatever the filter + assert client.get("/api/prompts?tag=music").json()["prompt_dir"] ``` - [ ] **Step 2: Run the test to verify it fails** @@ -630,7 +636,9 @@ def test_a_generated_audio_track_carries_has_audio(): "name": "generate_music", "pipeline": { "configuration": {"component_type": "ModularPipeline"}, - "from_pretrained_arguments": {"model_name": "MiniMaxAI/MiniMax-Music3"}, + "from_pretrained_arguments": { + "model_name": "MiniMaxAI/MiniMax-Music3" + }, "arguments": {"prompt": "variable:prompt"}, }, "result": {"content_type": "audio/mpeg"}, @@ -990,8 +998,7 @@ def test_a_skill_is_enumerated_where_the_plugin_describes_itself(path): ): with open(document) as file: assert name in file.read(), ( - f"skill {name!r} is not named in " - f"{os.path.relpath(document, REPO_ROOT)}" + f"skill {name!r} is not named in {os.path.relpath(document, REPO_ROOT)}" ) ``` @@ -1101,7 +1108,9 @@ def test_the_stated_tool_count_is_the_registered_one(): with open(os.path.join(REPO_ROOT, "README.md")) as file: stated["README.md"] = re.search(r"The agent has (\d+) tools", file.read()) with open(os.path.join(REPO_ROOT, "dw", "server", "guides.py")) as file: - stated["dw/server/guides.py"] = re.search(r"(\d+)-tool MCP surface", file.read()) + stated["dw/server/guides.py"] = re.search( + r"(\d+)-tool MCP surface", file.read() + ) for where, found in stated.items(): assert found, f"{where} no longer states a tool count in the expected form" @@ -1200,28 +1209,30 @@ TASKS_TEXT = ( Then add to `class TestFetching`: ```python - def test_a_section_linking_an_example_says_how_to_reach_it(self, checkout): - """A guide's link to an example is a repo path. The reader these - guides exist for has no checkout - it has the MCP and nothing else - - so a payload holding such a path carries the translation rule, rather - than 114 links being rewritten in prose people read too.""" - body = guides.get_guide("tasks", "Frame Interpolation") - - # The note is one fixed sentence - it states the rule, it does not - # name the paths it saw - assert body["catalog_paths"] == guides.CATALOG_PATH_NOTE - assert "get_workflow" in body["catalog_paths"] - - def test_a_section_linking_nothing_carries_no_note(self, checkout): - body = guides.get_guide("tasks", "Speech Generation") - - assert "catalog_paths" not in body - - def test_an_index_holding_a_path_is_noted_too(self, checkout): - """The index carries the guide's opening and its whole first section, - which is as able to link an example as any other.""" - assert "catalog_paths" not in guides.get_guide("tasks") - assert "catalog_paths" in guides.get_guide("workflows") +def test_a_section_linking_an_example_says_how_to_reach_it(self, checkout): + """A guide's link to an example is a repo path. The reader these + guides exist for has no checkout - it has the MCP and nothing else - + so a payload holding such a path carries the translation rule, rather + than 114 links being rewritten in prose people read too.""" + body = guides.get_guide("tasks", "Frame Interpolation") + + # The note is one fixed sentence - it states the rule, it does not + # name the paths it saw + assert body["catalog_paths"] == guides.CATALOG_PATH_NOTE + assert "get_workflow" in body["catalog_paths"] + + +def test_a_section_linking_nothing_carries_no_note(self, checkout): + body = guides.get_guide("tasks", "Speech Generation") + + assert "catalog_paths" not in body + + +def test_an_index_holding_a_path_is_noted_too(self, checkout): + """The index carries the guide's opening and its whole first section, + which is as able to link an example as any other.""" + assert "catalog_paths" not in guides.get_guide("tasks") + assert "catalog_paths" in guides.get_guide("workflows") ``` The last assertion needs the fixture's `WORKFLOW_GUIDE.md` to link one. In the diff --git a/dw/server/app.py b/dw/server/app.py index 7689ee83..51a9f67f 100644 --- a/dw/server/app.py +++ b/dw/server/app.py @@ -2208,7 +2208,9 @@ def list_prompts( # than no filter at all wanted = _matching_prompts(details, tag, intended_model) if wanted is not None: - details = {name: detail for name, detail in details.items() if name in wanted} + details = { + name: detail for name, detail in details.items() if name in wanted + } # The three parallel keys agree by construction, filter or no filter. # `prompt_details` drops a path whose mtime it cannot read - the file # went away between the walk and the read - and listing a name that diff --git a/dw_mcp/authoring.py b/dw_mcp/authoring.py index 882a472d..7c18a600 100644 --- a/dw_mcp/authoring.py +++ b/dw_mcp/authoring.py @@ -6,6 +6,8 @@ re-implements it - a second, subtly different check is how the two drift. """ +import json + from dw_mcp.client import DwApiError, api_path @@ -75,12 +77,39 @@ def save_workflow(client, name, workflow=None, patch=None): "Provide exactly one of `workflow` (a full replacement) or " "`patch` (a JSON merge patch onto the stored version)." ) + workflow = _coerce_json_object(workflow, "workflow") + patch = _coerce_json_object(patch, "patch") if patch is not None: current = client.get_json(api_path("api", "workflows", name)) workflow = _merge_patch(current, patch) return client.put_json(api_path("api", "workflows", name), {"workflow": workflow}) +def _coerce_json_object(value, param_name): + """A tool argument typed as an object can still arrive as a JSON-encoded + string (a caller that serialized it before handing it over, or a client + that couldn't parse a malformed document and passed the raw text + through). Accept that case rather than letting it reach the server as a + string, where the schema rejection names the wrong problem - not "this + isn't an object" but a bare pydantic `type=dict_type, input_type=str`, + which reads as if the field itself were misdeclared.""" + if value is None or isinstance(value, dict): + return value + if not isinstance(value, str): + raise DwApiError( + f"`{param_name}` must be a JSON object, not {type(value).__name__}." + ) + try: + parsed = json.loads(value) + except json.JSONDecodeError as e: + raise DwApiError(f"`{param_name}` is not valid JSON: {e}") from e + if not isinstance(parsed, dict): + raise DwApiError( + f"`{param_name}` must be a JSON object, not {type(parsed).__name__}." + ) + return parsed + + def _merge_patch(target, patch): """RFC 7396 JSON Merge Patch: each dict key in `patch` merges recursively into `target`; any other value replaces `target` outright; diff --git a/dw_mcp/server.py b/dw_mcp/server.py index 485f0aad..fc7bce57 100644 --- a/dw_mcp/server.py +++ b/dw_mcp/server.py @@ -855,7 +855,9 @@ def validate_workflow( ) def save_workflow( - name: str, workflow: dict | None = None, patch: dict | None = None + name: str, + workflow: dict | str | None = None, + patch: dict | str | None = None, ) -> dict: """Save a workflow to the server's writable workflow directory, overwriting any existing workflow of that name there. Validate it @@ -873,7 +875,10 @@ def save_workflow( workflow. A patch key set to `null` deletes that key from the stored document. A list is replaced whole, never merged - a merge patch has no notion of list position, so changing one `shots` entry - still means sending the whole `shots` list. + still means sending the whole `shots` list. Either may also be + given as a JSON-encoded string, which is parsed before saving; a + string that fails to parse is reported as invalid JSON rather than + as a type mismatch. A workflow stored for reuse should mark each saving step's `result.subfolder` - `final` for the step whose output the user will diff --git a/tests/test_catalog_shape.py b/tests/test_catalog_shape.py index 520107bd..dc02fba0 100644 --- a/tests/test_catalog_shape.py +++ b/tests/test_catalog_shape.py @@ -676,7 +676,9 @@ def test_a_generated_audio_track_carries_has_audio(): "name": "generate_music", "pipeline": { "configuration": {"component_type": "ModularPipeline"}, - "from_pretrained_arguments": {"model_name": "MiniMaxAI/MiniMax-Music3"}, + "from_pretrained_arguments": { + "model_name": "MiniMaxAI/MiniMax-Music3" + }, "arguments": {"prompt": "variable:prompt"}, }, "result": {"content_type": "audio/mpeg"}, diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index d205dd6c..d8d124f1 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -1080,7 +1080,9 @@ def test_the_stated_tool_count_is_the_registered_one(): with open(os.path.join(REPO_ROOT, "README.md")) as file: stated["README.md"] = re.search(r"The agent has (\d+) tools", file.read()) with open(os.path.join(REPO_ROOT, "dw", "server", "guides.py")) as file: - stated["dw/server/guides.py"] = re.search(r"(\d+)-tool MCP surface", file.read()) + stated["dw/server/guides.py"] = re.search( + r"(\d+)-tool MCP surface", file.read() + ) for where, found in stated.items(): assert found, f"{where} no longer states a tool count in the expected form" diff --git a/tests/test_plugin_skills.py b/tests/test_plugin_skills.py index 755fcd6f..d7c7b636 100644 --- a/tests/test_plugin_skills.py +++ b/tests/test_plugin_skills.py @@ -511,6 +511,5 @@ def test_a_skill_is_enumerated_where_the_plugin_describes_itself(path): with open(document) as file: content = file.read() assert f"`{name}`" in content, ( - f"skill {name!r} is not named in " - f"{os.path.relpath(document, REPO_ROOT)}" + f"skill {name!r} is not named in {os.path.relpath(document, REPO_ROOT)}" ) diff --git a/tests/test_server.py b/tests/test_server.py index 7c73d968..a69bb0b2 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -2977,7 +2977,10 @@ def test_save_prompt_roundtrip_and_confinement(self, server, tmp_path): assert not (tmp_path / "escape.json").exists() def _store(self, client, name, prompt): - assert client.put(f"/api/prompts/{name}", json={"prompt": prompt}).status_code == 200 + assert ( + client.put(f"/api/prompts/{name}", json={"prompt": prompt}).status_code + == 200 + ) def test_the_listing_narrows_by_tag_and_model_and_can_omit_the_text( self, server, tmp_path @@ -3027,9 +3030,12 @@ def test_the_listing_narrows_by_tag_and_model_and_can_omit_the_text( assert by_tag["prompts"] == ["minimax/Fox"] # Both at once, and a miss is an empty listing rather than a 404 - assert client.get( - "/api/prompts?tag=music&intended_model=minimax-h3" - ).json()["prompts"] == [] + assert ( + client.get("/api/prompts?tag=music&intended_model=minimax-h3").json()[ + "prompts" + ] + == [] + ) # The writable directory is reported whatever the filter assert client.get("/api/prompts?tag=music").json()["prompt_dir"]