Skip to content

Commit e135cf5

Browse files
authored
fix reconnection needing oauth flow (#649)
* fix: reconnecting servers * feat: add button to demo to interact with server * changeset
1 parent c365bd6 commit e135cf5

File tree

5 files changed

+140
-0
lines changed

5 files changed

+140
-0
lines changed

.changeset/deep-sloths-sit.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"agents": patch
3+
---
4+
5+
fix auth url not being cleared on a successful oauth callback causing endless reconnection

examples/mcp-client/src/client.tsx

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,36 @@ function App() {
9393
);
9494
};
9595

96+
const handleGetTools = async (serverId: string) => {
97+
try {
98+
const response = await agentFetch(
99+
{
100+
agent: "my-agent",
101+
host: agent.host,
102+
name: sessionId!,
103+
path: "get-tools"
104+
},
105+
{
106+
body: JSON.stringify({ serverId }),
107+
method: "POST"
108+
}
109+
);
110+
const data = (await response.json()) as { tools: any[]; error?: string };
111+
112+
if (data.error) {
113+
throw new Error(data.error);
114+
}
115+
116+
console.log("Server tools:", data.tools);
117+
alert(`Server Tools:\n\n${JSON.stringify(data.tools, null, 2)}`);
118+
} catch (error) {
119+
console.error("Failed to get tools:", error);
120+
alert(
121+
`Failed to get tools: ${error instanceof Error ? error.message : String(error)}`
122+
);
123+
}
124+
};
125+
96126
return (
97127
<div className="container">
98128
<div className="status-indicator">
@@ -140,6 +170,11 @@ function App() {
140170
Authorize
141171
</button>
142172
)}
173+
{server.state === "ready" && (
174+
<button type="button" onClick={() => handleGetTools(id)}>
175+
List Tools
176+
</button>
177+
)}
143178
<button type="button" onClick={() => handleDisconnect(id)}>
144179
Disconnect
145180
</button>

examples/mcp-client/src/server.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,28 @@ export class MyAgent extends Agent<Env, never> {
4747
return new Response("Ok", { status: 200 });
4848
}
4949

50+
if (reqUrl.pathname.endsWith("get-tools") && request.method === "POST") {
51+
try {
52+
const { serverId } = (await request.json()) as { serverId: string };
53+
const allTools = this.mcp.listTools();
54+
const tools = allTools.filter((tool) => tool.serverId === serverId);
55+
return new Response(JSON.stringify({ tools }), {
56+
headers: { "Content-Type": "application/json" },
57+
status: 200
58+
});
59+
} catch (error) {
60+
return new Response(
61+
JSON.stringify({
62+
error: error instanceof Error ? error.message : String(error)
63+
}),
64+
{
65+
headers: { "Content-Type": "application/json" },
66+
status: 500
67+
}
68+
);
69+
}
70+
}
71+
5072
return new Response("Not found", { status: 404 });
5173
}
5274
}

packages/agents/src/index.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,8 @@ export class Agent<
502502
this.broadcastMcpServers();
503503

504504
if (result.authSuccess) {
505+
this.clearMcpServerAuthUrl(result.serverId);
506+
505507
this.mcp
506508
.establishConnection(result.serverId)
507509
.catch((error) => {
@@ -1609,6 +1611,19 @@ export class Agent<
16091611
this.broadcastMcpServers();
16101612
}
16111613

1614+
/**
1615+
* Clear the auth_url for an MCP server after successful OAuth authentication
1616+
* This prevents the agent from continuously asking for OAuth on reconnect
1617+
* @param id The server ID to clear auth_url for
1618+
*/
1619+
private clearMcpServerAuthUrl(id: string) {
1620+
this.sql`
1621+
UPDATE cf_agents_mcp_servers
1622+
SET auth_url = NULL
1623+
WHERE id = ${id}
1624+
`;
1625+
}
1626+
16121627
getMcpServers(): MCPServersState {
16131628
const mcpState: MCPServersState = {
16141629
prompts: this.mcp.listPrompts(),

packages/agents/src/tests/mcp/oauth2-mcp-client.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,69 @@ describe("OAuth2 MCP Client", () => {
291291
});
292292
});
293293

294+
it("should clear auth_url from database after successful OAuth callback", async () => {
295+
const agentId = env.TestOAuthAgent.newUniqueId();
296+
const agentStub = env.TestOAuthAgent.get(agentId);
297+
298+
await agentStub.setName("default");
299+
await agentStub.onStart();
300+
301+
const serverId = nanoid(8);
302+
const serverName = "test-oauth-server";
303+
const serverUrl = "http://example.com/mcp";
304+
const clientId = "test-client-id";
305+
const authUrl = "http://example.com/oauth/authorize";
306+
const callbackBaseUrl = `http://example.com/agents/test-o-auth-agent/${agentId.toString()}/callback`;
307+
308+
// Insert MCP server with auth_url
309+
agentStub.sql`
310+
INSERT INTO cf_agents_mcp_servers (id, name, server_url, client_id, auth_url, callback_url, server_options)
311+
VALUES (
312+
${serverId},
313+
${serverName},
314+
${serverUrl},
315+
${clientId},
316+
${authUrl},
317+
${callbackBaseUrl},
318+
${null}
319+
)
320+
`;
321+
322+
// Verify auth_url exists before callback
323+
const serverBefore = await agentStub.getMcpServerFromDb(serverId);
324+
expect(serverBefore).not.toBeNull();
325+
expect(serverBefore?.auth_url).toBe(authUrl);
326+
327+
// Setup mock connection and OAuth state
328+
await agentStub.setupMockMcpConnection(
329+
serverId,
330+
serverName,
331+
serverUrl,
332+
callbackBaseUrl
333+
);
334+
await agentStub.setupMockOAuthState(serverId, "test-code", "test-state");
335+
336+
// Simulate successful OAuth callback
337+
const authCode = "test-auth-code";
338+
const state = "test-state";
339+
const callbackUrl = `${callbackBaseUrl}/${serverId}?code=${authCode}&state=${state}`;
340+
const request = new Request(callbackUrl, { method: "GET" });
341+
342+
const response = await agentStub.fetch(request);
343+
expect(response.status).toBe(200);
344+
345+
// Verify auth_url is cleared after successful callback
346+
const serverAfter = await agentStub.getMcpServerFromDb(serverId);
347+
expect(serverAfter).not.toBeNull();
348+
expect(serverAfter?.auth_url).toBeNull();
349+
350+
// Verify the server record still exists with other data intact
351+
expect(serverAfter?.id).toBe(serverId);
352+
expect(serverAfter?.name).toBe(serverName);
353+
expect(serverAfter?.server_url).toBe(serverUrl);
354+
expect(serverAfter?.client_id).toBe(clientId);
355+
});
356+
294357
describe("OAuth Redirect Behavior", () => {
295358
async function setupOAuthTest(config: {
296359
successRedirect?: string;

0 commit comments

Comments
 (0)