Skip to content

fix(mcp-v2): don't claim relay-tunnel disconnects mean toggle is off#4095

Merged
saddlepaddle merged 1 commit into
mainfrom
fix/mcp-tunnel-error-message
May 5, 2026
Merged

fix(mcp-v2): don't claim relay-tunnel disconnects mean toggle is off#4095
saddlepaddle merged 1 commit into
mainfrom
fix/mcp-tunnel-error-message

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented May 5, 2026

Summary

  • The relay returns 503 {"error":"Host not connected"} whenever tunnelManager.hasTunnel(hostId) is false (apps/relay/src/index.ts:55-56). That fires for several distinct conditions: toggle off, desktop app closed, WebSocket dropped mid-session, tunnel auth failed during WS upgrade, or tunnel not yet established.
  • describeRelayFailure in packages/mcp-v2/src/host-service-client.ts named only the toggle case and pushed a remediation step that's wrong in the other scenarios.
  • Replaces the message with a terse statement of fact (Host <id> relay tunnel is not connected.) matching the surrounding 'You are not authenticated' / Host ${hostId} not found style.

Skipped the optional relay-side enrichment ({ error: 'host_not_connected', reason: ... }) — it's a non-trivial cross-package change, would need new types, and wasn't required for the fix. Leaving for a follow-up if we want to surface the actual disconnect reason.

Test plan

  • bun run lint:fix clean
  • bun run typecheck clean
  • Verified no other consumers in the repo key on the old string (the toggle UI label in apps/desktop SecuritySettings is a separate independent string)

Summary by cubic

Fix inaccurate error text for relay 503 "Host not connected" responses. Now shows "Host relay tunnel is not connected." instead of telling users to enable remote access, which was wrong for cases like app closed, dropped WS, auth failure, or a not-yet-established tunnel.

Written for commit b6a7b6f. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error messaging for host connection failures to provide clearer feedback when a host becomes unavailable.

The 503 "Host not connected" branch from the relay fires for several
distinct conditions (toggle off, desktop closed, WS dropped, tunnel
auth failed, tunnel not yet established). The previous message named
only the toggle case and pushed a misleading remediation step.

Replace with a terse statement of fact matching the surrounding error
strings.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a2ea07ba-f8df-4883-974c-3f60334aa3ae

📥 Commits

Reviewing files that changed from the base of the PR and between 611ab0c and b6a7b6f.

📒 Files selected for processing (1)
  • packages/mcp-v2/src/host-service-client.ts

📝 Walkthrough

Walkthrough

A single error message in the host relay failure handler was updated to provide clearer indication of tunnel connectivity status. The 503 error response message for disconnected hosts now reports the specific host ID and relay tunnel state instead of settings guidance.

Changes

Relay Error Message Update

Layer / File(s) Summary
Error Message
packages/mcp-v2/src/host-service-client.ts
503 "host not connected" error message in describeRelayFailure changed to report Host ${hostId} relay tunnel is not connected. instead of settings guidance.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A rabbit hops with glee so bright, 🐰
Error messages now shine with light!
No more confusion, just the truth—
When tunnels fail, we speak the proof.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: fixing an inaccurate error message that incorrectly claimed relay-tunnel disconnects mean the toggle is off.
Description check ✅ Passed The description provides comprehensive context, explains the problem, describes the solution, justifies design decisions, and includes test verification. All critical sections are well-documented.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/mcp-tunnel-error-message

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Greptile Summary

  • Narrows the scope of describeRelayFailure in host-service-client.ts: the 503 "Host not connected" branch previously claimed the desktop toggle was off and prescribed a specific remediation, but that 503 fires for several other conditions (WebSocket drop, tunnel auth failure, app closed, not-yet-established tunnel). The new message is a neutral statement of fact.
  • The change is a one-line error-message correction with no behavioral side effects; it only affects the human-readable string surfaced in HostServiceCallError.message.
  • No new code paths, no type changes, no consumer breakage (the PR description confirms no callers key on the old string).

Confidence Score: 5/5

Safe to merge — one-line error message correction with no logic, API, or type changes.

The change is a pure string replacement in a single branch of a private helper function. It cannot introduce regressions, break types, or affect control flow. The PR description confirms no callers rely on the old string.

No files require special attention.

Important Files Changed

Filename Overview
packages/mcp-v2/src/host-service-client.ts Single-line fix replacing a misleading, toggle-specific error message with a neutral "relay tunnel is not connected" fact statement; no logic changes.

Sequence Diagram

sequenceDiagram
    participant Client as MCP Client
    participant Relay as Relay Server
    participant Host as Desktop Host

    Client->>Relay: POST /hosts/{routingKey}/trpc/{procedure}
    alt tunnel connected
        Relay->>Host: forward request
        Host-->>Relay: tRPC response
        Relay-->>Client: 200 OK
    else tunnel not connected (toggle off / WS dropped / auth fail / app closed)
        Relay-->>Client: 503 {"error":"Host not connected"}
        Note over Client: describeRelayFailure()<br/>OLD: "Host X has not enabled remote access. Toggle..."<br/>NEW: "Host X relay tunnel is not connected."
    end
Loading

Reviews (1): Last reviewed commit: "fix(mcp-v2): don't claim relay-tunnel di..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

@saddlepaddle saddlepaddle merged commit 1bb6688 into main May 5, 2026
17 checks passed
saddlepaddle added a commit that referenced this pull request May 6, 2026
…4095)

The 503 "Host not connected" branch from the relay fires for several
distinct conditions (toggle off, desktop closed, WS dropped, tunnel
auth failed, tunnel not yet established). The previous message named
only the toggle case and pushed a misleading remediation step.

Replace with a terse statement of fact matching the surrounding error
strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant