Skip to content

Commit 03a1e2e

Browse files
claudeclaude
committed
fix(mcp): make hookdeck_login non-blocking so browser URL is shown immediately
Previously, handleLogin blocked for up to 4 minutes while polling for the user to complete browser auth. The browser URL was only returned in the tool result after polling finished, meaning the user never saw it — they just saw a spinner with no way to authenticate. Now the handler returns the browser URL immediately and polls in a background goroutine. Subsequent calls to hookdeck_login report "in progress" (with the URL) or the final result. Once auth completes, the background goroutine updates the shared client and removes the login tool. https://claude.ai/code/session_01Y2eJZgKG78nDyN6Uw2tWQx
1 parent 0d9c6ff commit 03a1e2e

File tree

2 files changed

+167
-36
lines changed

2 files changed

+167
-36
lines changed

‎pkg/gateway/mcp/server_test.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,85 @@ func TestLoginTool_AlreadyAuthenticated(t *testing.T) {
10561056
_ = client // suppress unused warning
10571057
}
10581058

1059+
func TestLoginTool_ReturnsURLImmediately(t *testing.T) {
1060+
// Mock the /cli-auth endpoint to return a browser URL and a poll URL
1061+
// that never completes (simulates user not yet opening browser).
1062+
authCalled := false
1063+
api := mockAPI(t, map[string]http.HandlerFunc{
1064+
"/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) {
1065+
authCalled = true
1066+
json.NewEncoder(w).Encode(map[string]any{
1067+
"browser_url": "https://hookdeck.com/auth?code=abc123",
1068+
"poll_url": "http://" + r.Host + "/2025-07-01/cli-auth/poll?key=abc123",
1069+
})
1070+
},
1071+
"/2025-07-01/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) {
1072+
// Never claimed — user hasn't opened the browser yet.
1073+
json.NewEncoder(w).Encode(map[string]any{"claimed": false})
1074+
},
1075+
})
1076+
1077+
unauthClient := newTestClient(api.URL, "")
1078+
cfg := &config.Config{APIBaseURL: api.URL}
1079+
srv := NewServer(unauthClient, cfg)
1080+
1081+
serverTransport, clientTransport := mcpsdk.NewInMemoryTransports()
1082+
ctx, cancel := context.WithCancel(context.Background())
1083+
t.Cleanup(cancel)
1084+
go func() { _ = srv.mcpServer.Run(ctx, serverTransport) }()
1085+
1086+
mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil)
1087+
session, err := mcpClient.Connect(ctx, clientTransport, nil)
1088+
require.NoError(t, err)
1089+
t.Cleanup(func() { _ = session.Close() })
1090+
1091+
// The call should return immediately (not block for 4 minutes).
1092+
result := callTool(t, session, "hookdeck_login", map[string]any{})
1093+
assert.True(t, authCalled, "should have called /cli-auth")
1094+
assert.False(t, result.IsError)
1095+
text := textContent(t, result)
1096+
assert.Contains(t, text, "https://hookdeck.com/auth?code=abc123")
1097+
assert.Contains(t, text, "browser")
1098+
}
1099+
1100+
func TestLoginTool_InProgressShowsURL(t *testing.T) {
1101+
api := mockAPI(t, map[string]http.HandlerFunc{
1102+
"/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) {
1103+
json.NewEncoder(w).Encode(map[string]any{
1104+
"browser_url": "https://hookdeck.com/auth?code=xyz",
1105+
"poll_url": "http://" + r.Host + "/2025-07-01/cli-auth/poll?key=xyz",
1106+
})
1107+
},
1108+
"/2025-07-01/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) {
1109+
json.NewEncoder(w).Encode(map[string]any{"claimed": false})
1110+
},
1111+
})
1112+
1113+
unauthClient := newTestClient(api.URL, "")
1114+
cfg := &config.Config{APIBaseURL: api.URL}
1115+
srv := NewServer(unauthClient, cfg)
1116+
1117+
serverTransport, clientTransport := mcpsdk.NewInMemoryTransports()
1118+
ctx, cancel := context.WithCancel(context.Background())
1119+
t.Cleanup(cancel)
1120+
go func() { _ = srv.mcpServer.Run(ctx, serverTransport) }()
1121+
1122+
mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil)
1123+
session, err := mcpClient.Connect(ctx, clientTransport, nil)
1124+
require.NoError(t, err)
1125+
t.Cleanup(func() { _ = session.Close() })
1126+
1127+
// First call starts the flow.
1128+
_ = callTool(t, session, "hookdeck_login", map[string]any{})
1129+
1130+
// Second call should report "in progress" with the URL.
1131+
result := callTool(t, session, "hookdeck_login", map[string]any{})
1132+
assert.False(t, result.IsError)
1133+
text := textContent(t, result)
1134+
assert.Contains(t, text, "already in progress")
1135+
assert.Contains(t, text, "https://hookdeck.com/auth?code=xyz")
1136+
}
1137+
10591138
// ---------------------------------------------------------------------------
10601139
// API error scenarios (shared across tools)
10611140
// ---------------------------------------------------------------------------

‎pkg/gateway/mcp/tool_login.go‎

Lines changed: 88 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ import (
55
"fmt"
66
"net/url"
77
"os"
8+
"sync"
89
"time"
910

11+
log "github.com/sirupsen/logrus"
12+
1013
mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp"
1114

1215
"github.com/hookdeck/hookdeck-cli/pkg/config"
@@ -19,13 +22,49 @@ const (
1922
loginMaxAttempts = 120 // ~4 minutes
2023
)
2124

25+
// loginState tracks a background login poll so that repeated calls to
26+
// hookdeck_login don't start duplicate auth flows.
27+
type loginState struct {
28+
mu sync.Mutex
29+
browserURL string // URL the user must open
30+
done chan struct{} // closed when polling finishes
31+
err error // non-nil if polling failed
32+
}
33+
2234
func handleLogin(client *hookdeck.Client, cfg *config.Config, mcpServer *mcpsdk.Server) mcpsdk.ToolHandler {
35+
var state *loginState
36+
2337
return func(ctx context.Context, req *mcpsdk.CallToolRequest) (*mcpsdk.CallToolResult, error) {
24-
// If already authenticated, let the caller know.
38+
// Already authenticated — nothing to do.
2539
if client.APIKey != "" {
2640
return TextResult("Already authenticated. All Hookdeck tools are available."), nil
2741
}
2842

43+
// If a login flow is already in progress, check its status.
44+
if state != nil {
45+
select {
46+
case <-state.done:
47+
// Polling finished — check result.
48+
if state.err != nil {
49+
errMsg := state.err.Error()
50+
browserURL := state.browserURL
51+
state = nil // allow a fresh retry
52+
return ErrorResult(fmt.Sprintf(
53+
"Authentication failed: %s\n\nPlease call hookdeck_login again to retry.\nThe user needs to open this URL in their browser:\n\n%s",
54+
errMsg, browserURL,
55+
)), nil
56+
}
57+
// Success was already handled by the goroutine (client.APIKey set).
58+
return TextResult("Already authenticated. All Hookdeck tools are available."), nil
59+
default:
60+
// Still polling — remind the agent about the URL.
61+
return TextResult(fmt.Sprintf(
62+
"Login is already in progress. Waiting for the user to complete authentication.\n\nThe user needs to open this URL in their browser:\n\n%s\n\nCall hookdeck_login again to check status.",
63+
state.browserURL,
64+
)), nil
65+
}
66+
}
67+
2968
parsedBaseURL, err := url.Parse(cfg.APIBaseURL)
3069
if err != nil {
3170
return ErrorResult(fmt.Sprintf("Invalid API base URL: %s", err)), nil
@@ -40,49 +79,62 @@ func handleLogin(client *hookdeck.Client, cfg *config.Config, mcpServer *mcpsdk.
4079
return ErrorResult(fmt.Sprintf("Failed to start login: %s", err)), nil
4180
}
4281

43-
// Poll until the user completes login or we time out.
44-
response, err := session.WaitForAPIKey(loginPollInterval, loginMaxAttempts)
45-
if err != nil {
46-
return &mcpsdk.CallToolResult{
47-
Content: []mcpsdk.Content{
48-
&mcpsdk.TextContent{Text: fmt.Sprintf(
49-
"Authentication timed out or failed: %s\n\nPlease try again by calling hookdeck_login.\nTo authenticate, the user needs to open this URL in their browser:\n\n%s",
50-
err, session.BrowserURL,
51-
)},
52-
},
53-
IsError: true,
54-
}, nil
82+
// Set up background polling state.
83+
state = &loginState{
84+
browserURL: session.BrowserURL,
85+
done: make(chan struct{}),
5586
}
5687

57-
if err := validators.APIKey(response.APIKey); err != nil {
58-
return ErrorResult(fmt.Sprintf("Received invalid API key: %s", err)), nil
59-
}
88+
// Poll in the background so we return the URL to the agent immediately.
89+
go func(s *loginState) {
90+
defer close(s.done)
6091

61-
// Persist credentials so future MCP sessions start authenticated.
62-
cfg.Profile.APIKey = response.APIKey
63-
cfg.Profile.ProjectId = response.ProjectID
64-
cfg.Profile.ProjectMode = response.ProjectMode
65-
cfg.Profile.GuestURL = "" // Clear guest URL for permanent accounts.
92+
response, err := session.WaitForAPIKey(loginPollInterval, loginMaxAttempts)
93+
if err != nil {
94+
s.mu.Lock()
95+
s.err = err
96+
s.mu.Unlock()
97+
log.WithError(err).Debug("Login polling failed")
98+
return
99+
}
66100

67-
if err := cfg.Profile.SaveProfile(); err != nil {
68-
return ErrorResult(fmt.Sprintf("Login succeeded but failed to save profile: %s", err)), nil
69-
}
70-
if err := cfg.Profile.UseProfile(); err != nil {
71-
return ErrorResult(fmt.Sprintf("Login succeeded but failed to activate profile: %s", err)), nil
72-
}
101+
if err := validators.APIKey(response.APIKey); err != nil {
102+
s.mu.Lock()
103+
s.err = fmt.Errorf("received invalid API key: %s", err)
104+
s.mu.Unlock()
105+
return
106+
}
107+
108+
// Persist credentials so future MCP sessions start authenticated.
109+
cfg.Profile.APIKey = response.APIKey
110+
cfg.Profile.ProjectId = response.ProjectID
111+
cfg.Profile.ProjectMode = response.ProjectMode
112+
cfg.Profile.GuestURL = ""
113+
114+
if err := cfg.Profile.SaveProfile(); err != nil {
115+
log.WithError(err).Error("Login succeeded but failed to save profile")
116+
}
117+
if err := cfg.Profile.UseProfile(); err != nil {
118+
log.WithError(err).Error("Login succeeded but failed to activate profile")
119+
}
120+
121+
// Update the shared client so all resource tools start working.
122+
client.APIKey = response.APIKey
123+
client.ProjectID = response.ProjectID
73124

74-
// Update the shared client so all resource tools start working.
75-
client.APIKey = response.APIKey
76-
client.ProjectID = response.ProjectID
125+
// Remove the login tool now that auth is complete.
126+
mcpServer.RemoveTools("hookdeck_login")
77127

78-
// Remove the login tool now that auth is complete. This sends
79-
// notifications/tools/list_changed to clients that support it.
80-
mcpServer.RemoveTools("hookdeck_login")
128+
log.WithFields(log.Fields{
129+
"user": response.UserName,
130+
"project": response.ProjectName,
131+
}).Info("MCP login completed successfully")
132+
}(state)
81133

134+
// Return the URL immediately so the agent can show it to the user.
82135
return TextResult(fmt.Sprintf(
83-
"Successfully authenticated as %s (%s).\nActive project: %s in organization %s.\nAll Hookdeck tools are now available.",
84-
response.UserName, response.UserEmail,
85-
response.ProjectName, response.OrganizationName,
136+
"Login initiated. The user must open the following URL in their browser to authenticate:\n\n%s\n\nOnce the user completes authentication in the browser, all Hookdeck tools will become available.\nCall hookdeck_login again to check if authentication has completed.",
137+
session.BrowserURL,
86138
)), nil
87139
}
88140
}

0 commit comments

Comments
 (0)