Optional streamable-HTTP transport for remote hosting

[jamiew]

adds an optional streamable-HTTP transport so the server can be hosted remotely (homelab, Modal, etc.) instead of only running locally over stdio. stdio stays the default — http is fully opt-in via env. also lands the competitive research that motivated this plus a PLAN.md deployment guide.

• SPOTIFY_MCP_TRANSPORT=streamable-http serves over HTTP (SPOTIFY_MCP_HOST/PORT/STATELESS knobs); stdio remains the zero-config default
• headless OAuth: seed SPOTIFY_REFRESH_TOKEN (in-memory) or SPOTIFY_CACHE_PATH (file on a volume) so hosted deploys never need a browser — local stdio still auto-opens one
• optional SPOTIFY_MCP_BEARER gates the endpoint with a constant-time bearer check; otherwise keep it behind a private network
• deploy/modal_app.py for a one-command Modal deploy (stateless, since Modal's 150s request cap breaks stateful SSE) + Dockerfile EXPOSE/env notes
• research/spotify-mcp-landscape.md: survey of other Spotify MCPs — most are stdio-only, so remote hosting is a real differentiator
• adds uvicorn as a direct dep (used for the bearer path)

to test:

• SPOTIFY_MCP_TRANSPORT=streamable-http uv run spotify-mcp then claude mcp add --transport http spotify http://127.0.0.1:8000/mcp → list tools, run a search
• set SPOTIFY_MCP_BEARER=… and confirm requests without the header get a 401

[claude]

Code Review: PR #15 — Optional streamable-HTTP transport

Overall this is solid, well-structured work. The transport dispatch is clean, the bearer guard uses constant-time comparison correctly, and the headless OAuth design is sound. A few things worth addressing before merge:

---

Security

_bearer_guard is missing WWW-Authenticate in the 401 response (fastmcp_server.py)

Per RFC 7235, a 401 response should include WWW-Authenticate to tell clients what auth scheme is expected. Without it, some clients/proxies may not handle the rejection cleanly. Simple fix:

"headers": [
    (b"content-type", b"text/plain; charset=utf-8"),
    (b"www-authenticate", b"Bearer realm=\"spotify-mcp\""),
],

Using hmac.compare_digest for constant-time comparison is the right call — good.

---

Potential Bug: stateless HTTP mode with no SPOTIFY_REFRESH_TOKEN

In Modal (stateless) mode, is_headless() returns True (transport is http), correctly suppressing the browser open. But build_cache_handler() returns None when SPOTIFY_REFRESH_TOKEN is absent, so spotipy falls back to its default CacheFileHandler. On a stateless Modal container, that file cache is lost between requests — meaning the first request can succeed (seeded from the refresh token that comes from... the browser, which is suppressed). This is a subtle footgun.

The modal_app.py docstring only mentions setting SPOTIFY_REFRESH_TOKEN in setup step 2. Consider making it a hard check or at least a loud warning at startup:

# In _run_http() or build_cache_handler():
if _env_bool("SPOTIFY_MCP_STATELESS") and not os.getenv("SPOTIFY_REFRESH_TOKEN"):
    import warnings
    warnings.warn("SPOTIFY_MCP_STATELESS is set but SPOTIFY_REFRESH_TOKEN is missing — token cache will not persist across requests")

---

Dependency weight: uvicorn as a hard dep

uvicorn is only used in one narrow path: bearer-gated HTTP (_run_http() → uvicorn.run()). The non-bearer HTTP path goes through mcp.run(transport="streamable-http"), which handles its own serving internally. So every stdio user now carries uvicorn in their install for a code path they'll never hit.

Options in order of preference:

1. Check if mcp[cli]>=1.27.1 already pulls uvicorn transitively (if so, the explicit dep is just documentation — acceptable).
2. Make it an optional dep with an [http] extra in pyproject.toml.
3. Keep it as-is if the transitive dep means the overhead is already paid.

---

Minor Issues

import os placement in modal_app.py (line 268)
The import os is inside the function body, after other statements. Move it to the top of the function:

def mcp_web():
    import os
    from spotify_mcp.fastmcp_server import build_http_app, mcp
    ...

No port validation (__init__.py)

port = int(os.getenv("SPOTIFY_MCP_PORT", "8000"))

A non-numeric value raises ValueError with no helpful message. Consider:

raw_port = os.getenv("SPOTIFY_MCP_PORT", "8000")
try:
    port = int(raw_port)
except ValueError:
    raise SystemExit(f"SPOTIFY_MCP_PORT must be a number, got {raw_port!r}")

mcp.settings.host/port is set but redundant in the bearer path
In _run_http(), both paths set mcp.settings.host/port, but the bearer path passes these directly to uvicorn.run() and FastMCP settings aren't consulted. Not a bug, just slightly confusing. A comment clarifying why the settings are set in both cases would help.

---

Test Coverage

Tests are comprehensive and well-structured. Two gaps:

1. test_run_http_no_bearer doesn't assert that mcp.settings.stateless_http was set correctly — worth adding since that's a load-bearing behavior for Modal deploys.

2. test_client_disables_browser_when_headless / test_client_enables_browser_for_local both create a Client() with SpotifyOAuth mocked but without mocking credentials. These may be silently sensitive to whether load_config() returns something for SPOTIFY_CLIENT_ID in the test environment. Other tests in the file do pytest.raises(SpotifyOauthError) for missing creds — consider either adding credential fixtures or noting why these tests are safe without them.

---

Documentation / Repo Hygiene

PLAN.md in the repo root is a planning artifact rather than end-user docs. Once this is merged it'll go stale quickly (the "Implementation" section is already done). Consider either:

• Moving deployment guidance into README.md or a docs/ directory
• Deleting it post-merge (the reasoning is preserved in this PR and git history)

research/spotify-mcp-landscape.md is useful competitive context — reasonable to keep, but probably belongs in a docs/ or research/ directory rather than the root.

---

Summary

The core implementation is correct and well-tested. The bearer guard, headless OAuth, and transport dispatch are all solid. The main things to address before merge:

1. Add WWW-Authenticate header to 401 responses (small, clear win)
2. Either confirm uvicorn is already a transitive dep or make it optional
3. Clarify the stateless-without-refresh-token footgun, at minimum in docs

The rest are nice-to-haves.