Skip to content

Commit 0291f9e

Browse files
committed
fix(mcp): use session-level context for login polling, not per-request
The per-request ctx passed to MCP tool handlers is cancelled when the handler returns. The previous commit selected on that ctx in the login polling goroutine, which killed the poll immediately after returning the browser URL — breaking in-MCP authentication. Fix: add a sessionCtx field to Server, set it in Run() (called by RunStdio and tests), and select on that instead. The session context is only cancelled when the MCP transport closes (stdin EOF), which is the correct signal to abandon login polling. Also adds TestLoginTool_PollSurvivesAcrossToolCalls: a regression test that starts a login flow, lets the mock auth complete between tool calls, and verifies the client is authenticated on the second call. This would have caught the per-request ctx bug. https://claude.ai/code/session_01EpXZqTmgybtjgmSALukH8d
1 parent 5e61acb commit 0291f9e

File tree

3 files changed

+95
-12
lines changed

3 files changed

+95
-12
lines changed

pkg/gateway/mcp/server.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ type Server struct {
1818
client *hookdeck.Client
1919
cfg *config.Config
2020
mcpServer *mcpsdk.Server
21+
22+
// sessionCtx is the context passed to RunStdio. It is cancelled when the
23+
// MCP transport closes (stdin EOF). Background goroutines (e.g. login
24+
// polling) should select on this — NOT on the per-request ctx passed to
25+
// tool handlers, which is cancelled when the handler returns.
26+
sessionCtx context.Context
2127
}
2228

2329
// NewServer creates an MCP server with all Hookdeck tools registered.
@@ -56,7 +62,7 @@ func (s *Server) registerTools() {
5662
"reauth": {Type: "boolean", Desc: "If true, clear stored credentials and start a new browser login. Use when project listing fails — complete login in the browser, then retry hookdeck_projects."},
5763
}),
5864
},
59-
s.wrapWithTelemetry("hookdeck_login", handleLogin(s.client, s.cfg)),
65+
s.wrapWithTelemetry("hookdeck_login", handleLogin(s)),
6066
)
6167
}
6268

@@ -126,5 +132,13 @@ func extractAction(req *mcpsdk.CallToolRequest) string {
126132
// RunStdio starts the MCP server on stdin/stdout and blocks until the
127133
// connection is closed (i.e. stdin reaches EOF).
128134
func (s *Server) RunStdio(ctx context.Context) error {
129-
return s.mcpServer.Run(ctx, &mcpsdk.StdioTransport{})
135+
return s.Run(ctx, &mcpsdk.StdioTransport{})
136+
}
137+
138+
// Run starts the MCP server on the given transport. It stores ctx as the
139+
// session-level context so background goroutines (e.g. login polling) can
140+
// detect when the session ends.
141+
func (s *Server) Run(ctx context.Context, transport mcpsdk.Transport) error {
142+
s.sessionCtx = ctx
143+
return s.mcpServer.Run(ctx, transport)
130144
}

pkg/gateway/mcp/server_test.go

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/url"
1010
"strings"
1111
"testing"
12+
"time"
1213

1314
mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp"
1415
"github.com/stretchr/testify/assert"
@@ -45,7 +46,7 @@ func connectInMemory(t *testing.T, client *hookdeck.Client) *mcpsdk.ClientSessio
4546
ctx, cancel := context.WithCancel(context.Background())
4647
t.Cleanup(cancel)
4748
go func() {
48-
_ = srv.mcpServer.Run(ctx, serverTransport)
49+
_ = srv.Run(ctx, serverTransport)
4950
}()
5051

5152
mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{
@@ -1143,7 +1144,7 @@ func TestLoginTool_ReauthStartsFreshLogin(t *testing.T) {
11431144
serverTransport, clientTransport := mcpsdk.NewInMemoryTransports()
11441145
ctx, cancel := context.WithCancel(context.Background())
11451146
t.Cleanup(cancel)
1146-
go func() { _ = srv.mcpServer.Run(ctx, serverTransport) }()
1147+
go func() { _ = srv.Run(ctx, serverTransport) }()
11471148

11481149
mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil)
11491150
session, err := mcpClient.Connect(ctx, clientTransport, nil)
@@ -1182,7 +1183,7 @@ func TestLoginTool_ReturnsURLImmediately(t *testing.T) {
11821183
serverTransport, clientTransport := mcpsdk.NewInMemoryTransports()
11831184
ctx, cancel := context.WithCancel(context.Background())
11841185
t.Cleanup(cancel)
1185-
go func() { _ = srv.mcpServer.Run(ctx, serverTransport) }()
1186+
go func() { _ = srv.Run(ctx, serverTransport) }()
11861187

11871188
mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil)
11881189
session, err := mcpClient.Connect(ctx, clientTransport, nil)
@@ -1218,7 +1219,7 @@ func TestLoginTool_InProgressShowsURL(t *testing.T) {
12181219
serverTransport, clientTransport := mcpsdk.NewInMemoryTransports()
12191220
ctx, cancel := context.WithCancel(context.Background())
12201221
t.Cleanup(cancel)
1221-
go func() { _ = srv.mcpServer.Run(ctx, serverTransport) }()
1222+
go func() { _ = srv.Run(ctx, serverTransport) }()
12221223

12231224
mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil)
12241225
session, err := mcpClient.Connect(ctx, clientTransport, nil)
@@ -1236,6 +1237,70 @@ func TestLoginTool_InProgressShowsURL(t *testing.T) {
12361237
assert.Contains(t, text, "https://hookdeck.com/auth?code=xyz")
12371238
}
12381239

1240+
func TestLoginTool_PollSurvivesAcrossToolCalls(t *testing.T) {
1241+
// Regression: the login polling goroutine must use the session-level
1242+
// context, not the per-request ctx (which is cancelled when the handler
1243+
// returns). If the goroutine selected on per-request ctx, it would be
1244+
// cancelled immediately and the second hookdeck_login call would see a
1245+
// "login cancelled" error instead of "Already authenticated".
1246+
pollCount := 0
1247+
api := mockAPI(t, map[string]http.HandlerFunc{
1248+
"/2025-07-01/cli-auth": func(w http.ResponseWriter, r *http.Request) {
1249+
json.NewEncoder(w).Encode(map[string]any{
1250+
"browser_url": "https://hookdeck.com/auth?code=survive",
1251+
"poll_url": "http://" + r.Host + "/2025-07-01/cli-auth/poll?key=survive",
1252+
})
1253+
},
1254+
"/2025-07-01/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) {
1255+
pollCount++
1256+
if pollCount >= 2 {
1257+
// Simulate user completing browser auth on 2nd poll.
1258+
json.NewEncoder(w).Encode(map[string]any{
1259+
"claimed": true,
1260+
"key": "sk_test_survive12345",
1261+
"team_id": "proj_survive",
1262+
"team_name": "Survive Project",
1263+
"team_mode": "console",
1264+
"user_name": "test-user",
1265+
"organization_name": "test-org",
1266+
})
1267+
return
1268+
}
1269+
json.NewEncoder(w).Encode(map[string]any{"claimed": false})
1270+
},
1271+
})
1272+
1273+
unauthClient := newTestClient(api.URL, "")
1274+
cfg := &config.Config{APIBaseURL: api.URL}
1275+
srv := NewServer(unauthClient, cfg)
1276+
1277+
serverTransport, clientTransport := mcpsdk.NewInMemoryTransports()
1278+
ctx, cancel := context.WithCancel(context.Background())
1279+
t.Cleanup(cancel)
1280+
go func() { _ = srv.Run(ctx, serverTransport) }()
1281+
1282+
mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil)
1283+
session, err := mcpClient.Connect(ctx, clientTransport, nil)
1284+
require.NoError(t, err)
1285+
t.Cleanup(func() { _ = session.Close() })
1286+
1287+
// First call initiates the flow — handler returns immediately.
1288+
result := callTool(t, session, "hookdeck_login", map[string]any{})
1289+
assert.False(t, result.IsError)
1290+
assert.Contains(t, textContent(t, result), "https://hookdeck.com/auth?code=survive")
1291+
1292+
// Wait briefly for the polling goroutine to complete (poll interval is 2s
1293+
// in production, but the mock returns instantly so it completes quickly).
1294+
time.Sleep(500 * time.Millisecond)
1295+
1296+
// Second call — if the goroutine survived, the client is now authenticated.
1297+
result2 := callTool(t, session, "hookdeck_login", map[string]any{})
1298+
assert.False(t, result2.IsError)
1299+
text := textContent(t, result2)
1300+
assert.Contains(t, text, "Already authenticated")
1301+
assert.Equal(t, "sk_test_survive12345", unauthClient.APIKey)
1302+
}
1303+
12391304
// ---------------------------------------------------------------------------
12401305
// API error scenarios (shared across tools)
12411306
// ---------------------------------------------------------------------------

pkg/gateway/mcp/tool_login.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ type loginState struct {
3636
err error // non-nil if polling failed
3737
}
3838

39-
func handleLogin(client *hookdeck.Client, cfg *config.Config) mcpsdk.ToolHandler {
39+
func handleLogin(srv *Server) mcpsdk.ToolHandler {
40+
client := srv.client
41+
cfg := srv.cfg
4042
var stateMu sync.Mutex
4143
var state *loginState
4244

@@ -121,9 +123,11 @@ func handleLogin(client *hookdeck.Client, cfg *config.Config) mcpsdk.ToolHandler
121123
}
122124

123125
// Poll in the background so we return the URL to the agent immediately.
124-
// WaitForAPIKey blocks with time.Sleep; run it in a goroutine and
125-
// select on ctx so we abandon the attempt when the session closes.
126-
go func(s *loginState, ctx context.Context) {
126+
// WaitForAPIKey blocks with time.Sleep internally, so we run it in an
127+
// inner goroutine and select on the session-level context (not the
128+
// per-request ctx, which is cancelled when this handler returns).
129+
sessionCtx := srv.sessionCtx
130+
go func(s *loginState) {
127131
defer close(s.done)
128132

129133
type pollResult struct {
@@ -138,7 +142,7 @@ func handleLogin(client *hookdeck.Client, cfg *config.Config) mcpsdk.ToolHandler
138142

139143
var response *hookdeck.PollAPIKeyResponse
140144
select {
141-
case <-ctx.Done():
145+
case <-sessionCtx.Done():
142146
s.err = fmt.Errorf("login cancelled: MCP session closed")
143147
log.Debug("Login polling cancelled — MCP session closed")
144148
return
@@ -187,7 +191,7 @@ func handleLogin(client *hookdeck.Client, cfg *config.Config) mcpsdk.ToolHandler
187191
"user": response.UserName,
188192
"project": response.ProjectName,
189193
}).Info("MCP login completed successfully")
190-
}(state, ctx)
194+
}(state)
191195

192196
// Return the URL immediately so the agent can show it to the user.
193197
return TextResult(fmt.Sprintf(

0 commit comments

Comments
 (0)