fix: P1 quick-win findings from comprehensive review

[hybridindie]

Summary

Addresses 9 high-priority quick-win findings from the comprehensive code review (P1 items that are small effort):

• Expanded workflow template library #13 ModelManager negative TTL: Failed detection now expires after 5 minutes instead of caching permanently, so a late-starting Model Manager plugin is picked up without server restart
• Streamable HTTP transport support #19 Path injection tests: 7 new tests covering prompt_id traversal, non-UUID rejection, path segment injection, empty values, and HTTP method injection
• feat: add model search and download tools #20 Fix integration tests: Rewrote to use register_*_tools() return dicts instead of private _tool_manager._tools SDK attributes (CLAUDE.md rule 12)
• fix: ComfyUI-Model-Manager response envelope normalization and previewFile compatibility #22 Gate Docker push on CI: Added test job with lint+type-check+pytest that docker job needs: before pushing
• feat: Mermaid workflow visualization and img2img/inpaint/upscale tools #23 Docker volume paths: Updated docker-compose.yml mounts from /root/ to /home/app/ to match non-root container user
• feat: sanitized system stats tool (get_system_info) #24 SSE security docs: Added warning callout about unauthenticated SSE transport exposure
• feat: add get_model_presets and get_prompting_guide discovery tools #25 Audit mode guidance: Added comparison table for audit vs enforce mode with migration recommendation
• feat: expand workflow template library #26 Log rotation docs: Added logrotate config example to prevent unbounded audit log growth
• #52 Duplicate table row: Removed duplicate "Information disclosure" row in threat model

Based on: PR #27 (P0 critical security fixes)

Test plan

• All 359 tests pass (8 new tests)
• ruff check clean
• ruff format --check clean
• mypy clean
• All pre-commit hooks pass
• Verify CI passes on GitHub

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR addresses a set of P1 “quick-win” items from a comprehensive review, spanning Model Manager availability caching, path-injection test coverage, integration-test correctness, CI gating for Docker publishes, Docker compose volume paths for a non-root container, and targeted security/ops documentation updates.

Changes:

• Add an expiring negative cache (5 min TTL) for ComfyUI-Model-Manager detection and cover it with a regression test.
• Add new path-injection prevention tests and refactor integration tests to use register_*_tools() return dicts rather than private SDK internals.
• Gate Docker image publishing on a new CI test job; update Docker compose mounts for the non-root runtime; expand README security/ops guidance and remove a duplicate threat-model row.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/comfyui_mcp/model_manager.py	Adds negative-cache TTL logic for Model Manager detection so late-starting plugins can be detected without restart.
tests/test_model_manager.py	Adds a regression test verifying negative cache expiry and re-probing behavior.
tests/test_client.py	Adds tests validating prompt_id/path-segment validation and HTTP method allowlisting.
tests/test_integration.py	Refactors integration tests to call functions returned by register_*_tools() instead of private _tool_manager._tools.
docker-compose.yml	Updates bind/volume mount locations to /home/app/... to align with non-root container usage.
README.md	Adds SSE unauthenticated exposure warning, audit vs enforce guidance, log rotation example, and removes a duplicated threat-model row.
.github/workflows/docker.yml	Adds a test job (lint/typecheck/pytest) and makes the Docker publish job depend on it.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

The docker-compose volume mounts were switched to /home/app/.comfyui-mcp/..., but the image’s app user currently appears to be created without a home directory (see Dockerfile useradd --system), so Path.home()/~ may resolve to /nonexistent and /home/app may not exist or be writable. This can cause the server to ignore the mounted config.yaml and/or fail to write logs. Consider either creating /home/app for the app user in the image (and setting HOME), or update the compose mounts to match the actual runtime home/config paths used by the container.

Suggested change

	- ./config.yaml:/home/app/.comfyui-mcp/config.yaml:ro
	- comfyui-mcp-data:/home/app/.comfyui-mcp/logs
	- ./config.yaml:/nonexistent/.comfyui-mcp/config.yaml:ro
	- comfyui-mcp-data:/nonexistent/.comfyui-mcp/logs

[Copilot]

test_prompt_id_valid_uuid_accepted makes a real HTTP call (no respx.mock) and asserts on httpx.ConnectError. Because the client retries with backoff, this can be slow and potentially flaky depending on DNS/network behavior in CI. Prefer mocking the /history/{prompt_id} route with respx (and asserting the request was made) so the test stays deterministic while still verifying that UUID-shaped prompt_ids are accepted by the validator.

Suggested change

	async def test_prompt_id_valid_uuid_accepted(self, client):
	# Should not raise ValueError (will fail on HTTP, but that's fine)
	with pytest.raises(httpx.ConnectError):
	await client.get_history_item("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
	@respx.mock
	async def test_prompt_id_valid_uuid_accepted(self, client):
	# Valid UUID-shaped prompt_id should be accepted by the validator
	route = respx.get(
	"http://test-comfyui:8188/history/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
	).mock(
	return_value=httpx.Response(
	200,
	json={"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee": {"outputs": {}}},
	)
	)
	result = await client.get_history_item("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
	assert "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" in result
	assert route.call_count == 1

[Copilot]

The monkeypatch fixture is included in this test signature but never used. Please remove it to keep the test minimal and avoid suggesting that time or environment is being patched when it isn’t.

Suggested change

	async def test_negative_cache_expires(self, client, monkeypatch):
	async def test_negative_cache_expires(self, client):