fix(stremio_mcp): report ADB failures as actionable, redacted categories

[netixc]

Intent

Implement the captain-approved bounded response to GitHub issue #21 in stremio-mcp. Correct the macOS ADB Local Network documentation to identify adb, document the permitted GUI-terminal ADB server plus localhost-client pattern, and forbid automated kill-server/start-server lifecycle management. Add typed, bounded, redacted ADB failure categories with actionable caller-visible messages for unreachable/ambiguous network failures, unauthorized/offline devices, timeouts, and transport failures; preserve the native ADB transport and defer the raw-TCP differential probe. Sanitize endpoint/raw stderr/credential/URL/command-payload logging, fix set_volume false success, invalidate stale device state after failed operations, and bound reconnect attempts. Validate with mocked regression tests, locked repository checks, documentation/changelog updates, and CI.

What Changed

• Added a typed AdbFailureCategory/AdbFailure pair and classify_adb_failure() that maps ADB output to bounded categories (unreachable, ambiguous network, unauthorized, offline, timeout, transport, command). tv_control and play tool responses now return actionable guidance via _adb_failure_text() instead of a bare "Failed", and controller logs record only the category — device endpoints, raw ADB stderr, intent URIs, and command payloads are no longer logged or returned.
• Hardened controller state handling: failed operations clear the possibly stale device handle and retain the failure, connect() is serialized behind a lock with a reconnect cooldown, and set_volume now derives success from a new per-call _run_shell() helper so a failing shell command can no longer report success (the earlier shared-flag approach was replaced after review flagged a concurrency race).
• Documented the macOS pattern in README/AGENTS/CHANGELOG: Local Network permission applies to the adb binary, the shared server should be started from a permitted GUI terminal with other tools acting as localhost clients, and automated tooling must not run adb kill-server/start-server. Covered by new NativeAdbControllerTests and AdbToolFailureTests cases; the full locked unit-test suite (99 tests) and compileall pass.

Risk Assessment

✅ Low: The follow-up commit is a tightly scoped fix of the two prior findings — removing the shared shell-success flag in favor of a per-call return and reordering connect-scoped network markers ahead of the generic timeout branch — with mocked regression tests for both and no collateral impact on the other failure categories or on any acceptance criterion.

Testing

Ran the repo's locked setup, full unit suite, and source compilation (all clean), then went beyond tests by driving the real MCP tool handlers against a fake adb binary through ADB_PATH so the actual subprocess, classification, and response path executed for five realistic ADB failure modes; the captured transcripts show actionable category-tagged guidance with no endpoint, port, or secret leaking into user messages or logs, the previously false "Volume set to 8" success now correctly reported as a failure, five concurrent calls collapsing to a single bounded adb connect attempt, and automatic recovery once the device returned. A base-commit run of the identical driver documents the prior leaky, unactionable behavior. This change is CLI/MCP-text and documentation only with no rendered UI surface, so the evidence is command transcripts rather than screenshots.

Evidence: After: MCP tool responses and logs for five ADB failure modes (changed code)

=== scenario: local_network_denied — macOS Local Network denial / silently dropped packets MCP call: tv_control({'category': 'volume', 'action': 'set', 'value': 8}) user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb. server logs: ERROR:stremio-mcp:ADB connect failed: category=ambiguous_network leak check (endpoint/port/secret in message or logs): none === scenario: no_route — TV on another LAN (no route to host) user sees > ADB failure (category=unreachable): network reachability is ambiguous; verify the TV is on the same LAN and that macOS Local Network access is granted to adb. leak check: none === scenario: unauthorized — TV has not authorized this computer (stderr carries a secret) user sees > ADB failure (category=unauthorized): the TV has not authorized this computer; accept the debugging prompt on the TV. server logs: ERROR:stremio-mcp:ADB connect failed: category=unauthorized leak check: none === scenario: offline — TV connected but device offline MCP call: play({'source': 'direct', 'imdb_id': 'tt0111161'}) user sees > Failed to play: ADB failure (category=offline): the TV is offline; wake it and reconnect using the current connection port. leak check: none === scenario: shell_transport — transport dies mid-session user sees > ADB failure (category=transport): the ADB transport failed; verify the TV connection and try again. leak check: none

=== scenario: local_network_denied — macOS Local Network denial / silently dropped packets
    MCP call: tv_control({'category': 'volume', 'action': 'set', 'value': 8})
    user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb.
    server logs:
      ERROR:stremio-mcp:ADB connect failed: category=ambiguous_network
    leak check (endpoint/port/secret in message or logs): none

=== scenario: no_route — TV on another LAN (no route to host)
    MCP call: tv_control({'category': 'playback', 'action': 'toggle'})
    user sees > ADB failure (category=unreachable): network reachability is ambiguous; verify the TV is on the same LAN and that macOS Local Network access is granted to adb.
    server logs:
      ERROR:stremio-mcp:ADB connect failed: category=unreachable
    leak check (endpoint/port/secret in message or logs): none

=== scenario: unauthorized — TV has not authorized this computer (stderr carries a secret)
    MCP call: tv_control({'category': 'volume', 'action': 'up'})
    user sees > ADB failure (category=unauthorized): the TV has not authorized this computer; accept the debugging prompt on the TV.
    server logs:
      ERROR:stremio-mcp:ADB connect failed: category=unauthorized
    leak check (endpoint/port/secret in message or logs): none

=== scenario: offline — TV connected but device offline
    MCP call: play({'source': 'direct', 'imdb_id': 'tt0111161'})
    user sees > Failed to play: ADB failure (category=offline): the TV is offline; wake it and reconnect using the current connection port.
    server logs:
      INFO:stremio-mcp:Connected to Android TV
      ERROR:stremio-mcp:ADB intent failed: category=offline
    leak check (endpoint/port/secret in message or logs): none

=== scenario: shell_transport — transport dies mid-session
    MCP call: tv_control({'category': 'navigate', 'action': 'home'})
    user sees > ADB failure (category=transport): the ADB transport failed; verify the TV connection and try again.
    server logs:
      INFO:stremio-mcp:Connected to Android TV
      ERROR:stremio-mcp:ADB key event failed: category=transport
    leak check (endpoint/port/secret in message or logs): none

Evidence: Before (base commit 45c269d): same scenarios leaked endpoint/secret and reported false success

=== scenario: local_network_denied user sees > Volume set to 8 <-- false success on a total failure server logs: ERROR:stremio-mcp:Failed to connect to Android TV: failed to connect to '10.0.0.8:37139': Operation timed out leak check: LEAKED 10.0.0.8,37139 === scenario: unauthorized user sees > Failed <-- no guidance server logs: ERROR:stremio-mcp:Failed to connect to Android TV: failed to connect to '10.0.0.8:37139': device unauthorized. bearer=secret-token-12345678 leak check: LEAKED 10.0.0.8,37139,secret-token-12345678 === scenario: offline user sees > Failed to play: movie leak check: LEAKED 10.0.0.8,37139 === scenario: shell_transport user sees > Failed leak check: LEAKED 10.0.0.8,37139

=== scenario: local_network_denied — macOS Local Network denial / silently dropped packets
    MCP call: tv_control({'category': 'volume', 'action': 'set', 'value': 8})
    user sees > Volume set to 8
    server logs:
      ERROR:stremio-mcp:Failed to connect to Android TV: failed to connect to '10.0.0.8:37139': Operation timed out
    leak check (endpoint/port/secret in message or logs): LEAKED 10.0.0.8,37139

=== scenario: no_route — TV on another LAN (no route to host)
    MCP call: tv_control({'category': 'playback', 'action': 'toggle'})
    user sees > Failed
    server logs:
      ERROR:stremio-mcp:Failed to connect to Android TV: failed to connect to '10.0.0.8:37139': No route to host
    leak check (endpoint/port/secret in message or logs): LEAKED 10.0.0.8,37139

=== scenario: unauthorized — TV has not authorized this computer (stderr carries a secret)
    MCP call: tv_control({'category': 'volume', 'action': 'up'})
    user sees > Failed
    server logs:
      ERROR:stremio-mcp:Failed to connect to Android TV: failed to connect to '10.0.0.8:37139': device unauthorized. bearer=secret-token-12345678
    leak check (endpoint/port/secret in message or logs): LEAKED 10.0.0.8,37139,secret-token-12345678

=== scenario: offline — TV connected but device offline
    MCP call: play({'source': 'direct', 'imdb_id': 'tt0111161'})
    user sees > Failed to play: movie
    server logs:
      INFO:stremio-mcp:Connected to Android TV at 10.0.0.8:37139
      ERROR:stremio-mcp:Failed to send intent: error: device offline
    leak check (endpoint/port/secret in message or logs): LEAKED 10.0.0.8,37139

=== scenario: shell_transport — transport dies mid-session
    MCP call: tv_control({'category': 'navigate', 'action': 'home'})
    user sees > Failed
    server logs:
      INFO:stremio-mcp:Connected to Android TV at 10.0.0.8:37139
      ERROR:stremio-mcp:Failed to send key event: error: closed
    leak check (endpoint/port/secret in message or logs): LEAKED 10.0.0.8,37139

Evidence: Bounded reconnect and stale-state recovery through the MCP tool path

--- 5 concurrent tv_control calls while the TV is unreachable user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb. (x5) actual adb subprocess invocations for 5 tool calls: 1 (serialized + cooldown-bounded, not one connect per call) --- TV comes back; next tool call reconnects without a restart user sees > Navigate: home controller device handle restored: '10.0.0.8:37139', last_failure cleared: True

--- 5 concurrent tv_control calls while the TV is unreachable
    user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb.
    user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb.
    user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb.
    user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb.
    user sees > ADB failure (category=ambiguous_network): network connection failed; verify the TV is online and use its current connection port, and on macOS check that Local Network access is granted to adb.
    actual `adb` subprocess invocations for 5 tool calls: 1   (serialized + cooldown-bounded, not one connect per call)

--- TV comes back; next tool call reconnects without a restart
    user sees > Navigate: home
    controller device handle restored: '10.0.0.8:37139', last_failure cleared: True

Evidence: Evidence driver scripts and fake adb binary used for the manual end-to-end runs

"""Drive the real MCP tool handlers against a fake `adb` binary and print what an
end user sees, plus every log line the server emitted."""
import asyncio, importlib.util, io, logging, os, sys

SRC = sys.argv[1]
SCENARIOS = [
    ("local_network_denied", "macOS Local Network denial / silently dropped packets",
     ("tv_control", {"category": "volume", "action": "set", "value": 8})),
    ("no_route", "TV on another LAN (no route to host)",
     ("tv_control", {"category": "playback", "action": "toggle"})),
    ("unauthorized", "TV has not authorized this computer (stderr carries a secret)",
     ("tv_control", {"category": "volume", "action": "up"})),
    ("offline", "TV connected but device offline",
     ("play", {"source": "direct", "imdb_id": "tt0111161"})),
    ("shell_transport", "transport dies mid-session",
     ("tv_control", {"category": "navigate", "action": "home"})),
]

async def main():
    for scen, desc, (tool, args) in SCENARIOS:
        os.environ.update(
            FAKE_ADB_SCENARIO=scen,
            ADB_PATH=os.path.abspath("fake/adb"),
            ANDROID_TV_HOST="10.0.0.8",
            ANDROID_TV_PORT="37139",
        )
        spec = importlib.util.spec_from_file_location(f"m_{scen}", SRC)
        m = importlib.util.module_from_spec(spec)
        spec.loader.exec_module(m)
        m.initialize()  # same startup path the real MCP server runs

        buf = io.StringIO()
        h = logging.StreamHandler(buf)
        h.setFormatter(logging.Formatter("%(levelname)s:%(name)s:%(message)s"))
        root = logging.getLogger()
        root.addHandler(h); root.setLevel(logging.DEBUG)
        for lg in (m.logger,):
            lg.addHandler(h); lg.setLevel(logging.DEBUG); lg.propagate = False
        try:
            resp = await m.call_tool(tool, args)
        finally:
            root.removeHandler(h); m.logger.removeHandler(h)

        print(f"=== scenario: {scen} — {desc}")
        print(f"    MCP call: {tool}({args})")
        print(f"    user sees > {resp[0].text}")
        logs = [l for l in buf.getvalue().splitlines() if l.strip()]
        print("    server logs:")
        for l in logs or ["      (none)"]:
            print(f"      {l}")
        leaked = [n for n in ("10.0.0.8", "37139", "secret-token-12345678")
                  if n in resp[0].text or any(n in l for l in logs)]
        print(f"    leak check (endpoint/port/secret in message or logs): "
              f"{'LEAKED ' + ','.join(leaked) if leaked else 'none'}")
        print()

asyncio.run(main())

Evidence: README macOS Local Network guidance (identifies adb, GUI-terminal server pattern, forbids automated lifecycle management)

- On macOS, grant **Local Network** permission under **Privacy & Security → Local
Network** to the `adb` binary itself. A reliable pattern is to start the ADB
server once from a permitted GUI terminal, then let the MCP server and other
tools act as localhost clients of that existing server.
- Do not run `adb kill-server` or `adb start-server` from automated tooling: that
can discard a permitted server and recreate it under a process without the
required macOS permission.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed ✅

• ⚠️ src/stremio_mcp.py:469 - set_volume determines success by reading the controller-instance flag self._last_shell_succeeded that send_shell_command mutates (lines 435/446) rather than from a value returned by the call it made. MCP tool calls can be served concurrently on the same shared controller object, so two overlapping shell operations interleave on this single flag: if set_volume's own media volume --set returns non-zero (flag -> False) while a concurrent get_tv_state/get_playback_status shell command completes successfully afterwards (flag -> True), set_volume returns True and the tool replies "Volume set to N" — reintroducing exactly the false-success this change set out to remove. It also makes the self._last_shell_succeeded = None line at 469 dead in production (send_shell_command overwrites it to False immediately), existing only so the mocked test at tests/test_stremio_mcp.py:1645 passes. Prefer a private helper returning (ok, stdout) (e.g. _run_shell(command) -> tuple[bool, str]) that send_shell_command and set_volume both consume, so success is carried per-call instead of on shared state.
• ⚠️ src/stremio_mcp.py:272 - classify_adb_failure tests "timed out" in output or "timeout" in output before any of the network markers, so failed to connect to '<tv>:<port>': Operation timed out — the other common macOS Local Network denial symptom, alongside the No route to host seen in issue ADB: actionable preflight for macOS Local Network denial, and document the working pattern #21, since the denied packets are silently blackholed rather than rejected — is classified TIMEOUT and answered with "the operation timed out; verify the TV is online and try again", which omits the Local Network guidance that is the point of the issue. The generic timeout branch also cannot distinguish this from the internal 20s asyncio.wait_for timeout in _run_adb. Consider testing the connect-scoped network markers (failed to connect, no route to host, ...) before the generic timeout, or adding the Local Network hint to the TIMEOUT guidance.
• ℹ️ src/stremio_mcp.py:233 - The two network categories carry guidance that reads inverted against their names: UNREACHABLE says "network reachability is ambiguous..." while AMBIGUOUS_NETWORK says "network connection failed...". Per the comment at line 279 this is deliberate (a No route to host really is ambiguous between a route failure and a macOS denial), but a future maintainer editing these strings is likely to "fix" them backwards. Noting only; renaming or a short comment on the mapping would remove the trap.

🔧 Fix: fix ADB shell success race and connect-timeout classification
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

• uv sync --locked
• uv run --locked python -m unittest discover -s tests -v (99 tests, all pass, including the new NativeAdbControllerTests classification/redaction/cooldown cases and AdbToolFailureTests)
• uv run --locked python -m compileall -q src tests
• Manual end-to-end: drove real stremio_mcp.call_tool (tv_control volume/playback/navigate, play) with ADB_PATH pointed at a fake adb emitting real ADB stderr for local-network-denial, no-route, unauthorized (with an embedded sentinel secret), offline, and mid-session transport failure; captured user-visible tool text plus all server log records and asserted no host/port/secret leakage
• Manual before/after: ran the same driver against base commit 45c269d source to contrast messages and log leakage
• Manual bounded-reconnect/recovery: 5 concurrent tv_control calls against an unreachable TV while counting actual adb subprocess invocations, then healed the fake adb and confirmed the next call reconnected and cleared stale failure state
• Doc check: grep -rn "kill-server|start-server" README.md AGENTS.md CLAUDE.md docs/ and read of the README macOS troubleshooting section

🔧 **Document** - 1 issue found → auto-fixed ✅

• ℹ️ src/stremio_mcp.py:231 - The user-facing guidance strings for AdbFailureCategory.UNREACHABLE and AMBIGUOUS_NETWORK appear transposed: UNREACHABLE (matched on explicit "no route to host"/"network is unreachable") says "network reachability is ambiguous", while AMBIGUOUS_NETWORK (matched on connect-scoped failures) says "network connection failed". Both are safe and actionable, so this is cosmetic, but a caller reading category=unreachable alongside "ambiguous" text gets a confusing signal. Left unchanged because these strings are asserted by mocked regression tests and editing them would change caller-visible behavior.

🔧 Fix: align ADB failure messages with their categories
✅ Re-checked - no issues remain.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.