[gmacario]

Summary

Closes #116.

Clears the five code-quality items from that issue. No behaviour changes intended beyond the --mcp-transport fix, which is the point of item 1.

1. --mcp-transport was a no-op

mcp_server_pupper/main.py read the raw env var in the run block rather than the parsed argument:

-    if transport == "http":
-        mcp.run(transport=transport)
-    else:
-        mcp.run(transport="stdio")
+    mcp.run(transport=args.mcp_transport)

args.mcp_transport was parsed and never used, and "http" isn't among the argparse choices, so --mcp-transport sse / streamable-http silently ran stdio.

MCP_TRANSPORT still supplies the default. Worth noting: argparse does not validate defaults against choices, so MCP_TRANSPORT=http still reaches mcp.run(transport="http") exactly as before — this change doesn't break that path.

2. response.json missing its call

-        res_content = response.json if response.content else None
+        res_content = response.json() if response.content else None

3. Mutable default arguments (B006)

publish_once(msg={}) and publish_for_durations(messages=[], durations=[]) now default to None, with the guards changed from msg == {} / == [] to falsy checks. Verified the guards accept exactly the same inputs:

value	old guard	new guard
{} / []	rejected	rejected
None (new default)	n/a	rejected
populated	accepted	accepted

4. zip() strictness made explicit (B905)

• publish_for_durations → strict=True; equal lengths are already validated immediately above, so it can't fire.
• get_message_details → strict=False with a comment. A malformed rosbridge typedef keeps truncating rather than raising out of a tool call — deliberately preserving current behaviour.

5. os.system() → subprocess.run()

In puppy_state_manager.py and puppy_head.py:

-            os.system(f"mpg123 {sound_path}")
+            subprocess.run(["mpg123", str(sound_path)], check=False)

To be clear on severity, as noted in #116: these were not remotely injectable — both call sites allow-list the requested name against configuration first. The fix is about paths containing spaces or shell metacharacters. check=False preserves the previous ignore-the-exit-status behaviour. import os became unused in puppy_head.py and was dropped.

Heads-up: MCP tool schema change

A consequence of item 3 — the schema the LLM sees for these parameters changes:

	before	after
publish_once.msg	{"type":"object","default":{}}	{"anyOf":[{"type":"object"},{"type":"null"}],"default":null}
publish_for_durations.messages	{"type":"array","default":[]}	{"anyOf":[{"type":"array"},{"type":"null"}],"default":null}

Omitting the argument was rejected before and is still rejected, with the same error — so there's no functional difference. Flagging it because it's user-visible to the model. If you'd rather keep the original schema exactly, Annotated[dict, Field(default_factory=dict)] also satisfies B006; say the word and I'll switch.

Verification

• All 9 MCP tools still register; schemas captured before and after for the table above.
• --mcp-transport stdio|sse|streamable-http each confirmed to reach args.mcp_transport.
• Guard and zip() semantics verified against the old comparisons (table above).
• ruff --select B006,B905 and --select S605 → clean. Repo-wide default ruleset (what Super-Linter runs) reports only two pre-existing F401s in code/training-yolo/*.ipynb, untouched here.
• python3.13 -m py_compile clean on all four files. (py_compile under 3.11 fails on puppy_state_manager.py — a pre-existing PEP 701 multi-line f-string at line 139, present on main and unrelated to this PR; that package requires Python ≥ 3.13.)
• Not hardware-tested: the subprocess.run() audio changes need a robot with a sound device to exercise. They're mechanical translations, but worth a sanity check on a puppy before relying on them.

Leaves #111, #112, #113, #114, #115 and #117 untouched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Mc57S5DG9vCz16a8g7Xxqi

---

Generated by Claude Code

[gmacario]

Publish Docker Image is red here, and it is not this PR's failure. Filed as #119.

The build dies at Dockerfile:3 with ~13 bullseye-security packages returning 404 Not Found:

E: Failed to fetch .../libtiff5-dev_4.2.0-1+deb11u8_amd64.deb  404  Not Found
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
exit code: 100

Evidence that it is independent of this PR:

• I ran the same workflow against main with no PR content (run 34765657297, workflow_dispatch on 1a0e71d) and it failed identically.
• The failing step is Dockerfile:3; the first COPY is Dockerfile:19. No repository file is in the image yet when it fails, so no diff can reach it.
• The two runs hit different mirror backends (146.75.30.132, 140.248.146.132) and failed the same way, so it is not one bad CDN node.

This PR changes four Python files (mcp_server_pupper/main.py, puppy_interaction.py, puppy_state_manager.py, puppy_head.py) and touches neither the Dockerfile nor requirements.txt.

No fix exists yet to port into this PR, so I am not widening it — the fix belongs in #119, which needs a decision about the base image (move to bookworm, repoint at archive.debian.org, or trim the package list). Once #119 lands on main, this check should go green here with no change to the branch.

Everything else on this PR is green: Lint ✅ · REUSE ✅ · Markdown Link Check ✅ · WIP ✅, no open review threads.

I will re-run the failed job once to confirm it reproduces rather than leaving it on a single observation.

---

Generated by Claude Code