feat(stremio_mcp): diagnose masked adb denials with a raw TCP preflight - #26
Merged
Merged
Conversation
When adb connect reports the TV as unreachable or the network as ambiguous, run one bounded raw TCP probe (single attempt, no bytes written, 2 s cap) against the configured endpoint from inside the serialized connect path. If the probe connects while adb could not, report local_network_denied with guidance to grant adb Local Network access on macOS or reuse an ADB server started from a permitted GUI terminal. A failed probe keeps the original diagnosis because the probing process may itself be denied. The probe never logs or returns the endpoint and never runs an ADB server lifecycle command. Closes #21. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Close the one remaining item from netixc/stremio-mcp issue #21: a bounded raw-TCP differential preflight that separates a masked adb-side denial (macOS Local Network permission, or an ADB server started by a process without it) from a genuine network failure, because macOS reports the denial as EHOSTUNREACH ('No route to host') which is textually identical to a real routing failure and cost a multi-hour misdiagnosis during live dogfooding. Deliberate design, all captain-approved: when classify_adb_failure returns UNREACHABLE or AMBIGUOUS_NETWORK inside connect()'s existing serialized/cooldown failure path, run exactly one asyncio.open_connection probe to the configured endpoint bounded by asyncio.wait_for (new module constant ADB_PROBE_TIMEOUT=2.0 - deliberately a plain constant like ADB_RECONNECT_COOLDOWN, not an env knob, since none was needed); zero protocol bytes are written, TCP acceptance alone is the differential, and the socket closes immediately. Probe success upgrades the failure to a new category local_network_denied whose message names the macOS Local Network permission and the permitted-GUI-terminal server-reuse pattern without ever containing the endpoint. Probe failure of any kind (refused, timeout, EHOSTUNREACH, resolver error) deliberately keeps the original diagnosis because the probing interpreter may itself be denied - the probe can confirm a denial but can never rule one out; this asymmetry is the core constraint and is documented in the probe docstring. The probe is deliberately only in connect() (not disconnect/_operation_failed) because every recovery path re-enters connect() via _ensure_connected. The raw client socket deliberately does not use AsyncHTTPClient (that rule governs HTTP; this is not HTTP) and never invokes adb, so no server lifecycle command (kill-server/start-server) can occur - preserving the externally started permitted GUI server is a hard project constraint. Existing behavior outside this diagnosis is unchanged, including the cooldown assignment semantics. Tests: new AdbLocalNetworkPreflightTests with a fail-before/pass-after headline regression driving a real fake-adb executable via ADB_PATH against a real loopback listener (also proves at socket level: one connection, zero bytes before EOF, argv transcript free of lifecycle commands); coverage for every probe outcome including mocked EHOSTUNREACH and gaierror; probe-eligibility gating; one-probe-per-failed-connect under concurrency; bounded wall-clock proof; sentinel-endpoint privacy tests at log and tool layers per the repo's sentinel-secret rule. In the one test that runs the real probe under captured logs, the endpoint assertion is deliberately scoped to the stremio-mcp logger's records because unittest's debug-mode event loop makes asyncio itself emit DEBUG reprs of the test's own loopback listener - production does not run loop debug, and the project's stated privacy boundary is what this module logs. Three pre-existing connect-failure tests gained a stubbed probe so they cannot perform real network I/O. Docs deliberately minimal: one README troubleshooting bullet tying local_network_denied to the existing macOS guidance and one CHANGELOG Added entry; AGENTS.md/CLAUDE.md untouched because issue #21's documentation item already landed via PR #22. Verified on Linux: uv sync --locked, 122 unit tests (up from 112), compileall, uv build all green; also validated end-to-end on the captain's real macOS 26 machine via an assisted redacted session (real GUI-started ADB server reused without restart, real stdio MCP playback_status truthful, live raw-TCP differential produced local_network_denied with zero endpoint disclosure and zero lifecycle commands) - that evidence lives outside the repo by design and must stay out; no endpoint, serial, pairing code, or credential may enter the repo, PR, or logs. Ship to netixc/stremio-mcp only. Closes issue #21.
What Changed
connect(): whenclassify_adb_failurereturnsUNREACHABLEorAMBIGUOUS_NETWORK, a singleasyncio.open_connectionprobe (bounded by the newADB_PROBE_TIMEOUT=2.0constant) writes zero bytes and closes immediately — TCP acceptance alone upgrades the failure to a newlocal_network_deniedcategory that names the macOS Local Network permission and the permitted-GUI-terminal server-reuse pattern without ever disclosing the endpoint. Any probe failure deliberately preserves the original diagnosis, since the probing interpreter may itself be denied.adband does not useAsyncHTTPClient, so no server lifecycle command can occur; it lives only inconnect()because every recovery path re-enters there via_ensure_connected.AdbLocalNetworkPreflightTests(10 new tests, 122 total up from 112) covering a fail-before/pass-after regression against a real fake-adb executable and loopback listener, every probe outcome including mockedEHOSTUNREACH/gaierror, one-probe-per-failed-connect under concurrency, bounded wall-clock, and sentinel-endpoint privacy at the log and tool layers; documented via one README troubleshooting bullet and a CHANGELOG Added entry.Risk Assessment
✅ Low: The change adds a well-bounded, single-attempt, no-bytes raw TCP differential probe strictly within the existing serialized connect() failure path, with exhaustive tests covering every outcome and privacy boundary, and no observable regression to existing behavior.
Testing
Ran the CI-equivalent unit suite (122 tests, up from the 112 baseline) and the newAdbLocalNetworkPreflightTestsclass — all green. Beyond tests, I drove the real code path end-to-end with a fakeadbbinary emulating the macOS masked denial ('No route to host', exit 1) and a live loopback listener standing in for the reachable TV, then called the actual MCPtv_controltool. The verbatim response the user receives isADB failure (category=local_network_denied): ...on macOS grant adb Local Network access ... or reuse an ADB server started from a GUI terminal.... The captured proof points confirm every intent constraint: the probe made exactly one connection and wrote zero bytes before EOF, the adb argv transcript wasconnect 127.0.0.1:<port>with no kill-server/start-server, and neither the host127.0.0.1nor the port appeared in the tool response or the module log line. This is a CLI/MCP-tool surface (no visual UI), so the reviewer-visible evidence is the captured tool-response transcript rather than a screenshot.Evidence: End-to-end MCP tool-response transcript (masked denial → local_network_denied, with privacy/one-probe/no-lifecycle proof points)
--- verbatim MCP tool response the user receives --- ADB failure (category=local_network_denied): the TV accepted a direct connection from this process while adb could not reach it, so this is an adb permission or ADB-server problem, not a network failure; on macOS grant adb Local Network access (System Settings > Privacy & Security > Local Network) or reuse an ADB server started from a GUI terminal that has the permission. PROOF POINTS reported failure category : local_network_denied probe wrote before EOF : [b''] (zero protocol bytes) probe connections accepted: 1 (single attempt) adb argv transcript : 'connect 127.0.0.1:38503' contains 'kill-server' : False contains 'start-server' : False endpoint '127.0.0.1' leaked into response : False port 38503 leaked into response : FalseEvidence: End-to-end driver script (real fake-adb + real loopback listener + real MCP call_tool)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
uv sync --locked(locked env setup)uv run --locked python -m unittest discover -s tests— 122 tests pass (up from 112)uv run --locked python -m unittest tests.test_stremio_mcp.AdbLocalNetworkPreflightTests -v— all 10 new preflight tests passEnd-to-end MCP driver: real fake-adb executable emitting 'No route to host' + real loopback listener, calling the actualtv_controltool; captured verbatim end-user response, probe byte count, adb argv transcript, and endpoint-leak checksInspected the module's own (stremio-mcp) log record to confirm the production-relevant log line carries onlycategory=local_network_deniedwith no endpoint or raw adb stderr✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.