Skip to content

Commit e0b2171

Browse files
Merge branch 'main' into sou-1870-scoped-access-tokens
2 parents 2d6c321 + 472692a commit e0b2171

31 files changed

Lines changed: 2198 additions & 70 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111
- [EE] Added one-hour repository-scoped access tokens with public mint and revoke APIs. [#1549](https://github.com/sourcebot-dev/sourcebot/pull/1549)
12+
- [EE] Added guided reconnection for MCP connector authentication failures during Ask Sourcebot agent turns. [#1548](https://github.com/sourcebot-dev/sourcebot/pull/1548)
1213

1314
### Removed
1415
- Removed the Langfuse integration. [#1536](https://github.com/sourcebot-dev/sourcebot/pull/1536)

packages/web/src/app/api/(client)/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ export const getOffers = async (): Promise<OffersResponse | ServiceError> => {
320320
return result as OffersResponse | ServiceError;
321321
}
322322

323-
export const connectMcpToAsk = async (body: { serverId: string; returnTo?: string }): Promise<ConnectMcpResponse | ServiceError> => {
323+
export const connectMcpToAsk = async (body: { serverId: string; returnTo?: string; forceAuthorization?: boolean }): Promise<ConnectMcpResponse | ServiceError> => {
324324
const result = await fetch('/api/ee/askmcp/connect', {
325325
method: 'POST',
326326
headers: {

packages/web/src/app/api/(server)/ee/askmcp/connect/route.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ vi.mock('@ai-sdk/mcp', () => ({
4848
const { POST } = await import('./route');
4949
const { getMcpOAuthReturnToFromState } = await import('@/ee/features/chat/mcp/mcpOAuthReturnTo');
5050

51-
function createRequest(body: { serverId: string; returnTo?: string } = { serverId: 'server-1' }) {
51+
function createRequest(body: { serverId: string; returnTo?: string; forceAuthorization?: boolean } = { serverId: 'server-1' }) {
5252
return new NextRequest('https://sourcebot.example.com/api/ee/askmcp/connect', {
5353
method: 'POST',
5454
headers: { 'content-type': 'application/json' },
@@ -229,6 +229,60 @@ describe('POST /api/ee/askmcp/connect', () => {
229229
});
230230
});
231231

232+
test('forces an interactive OAuth redirect for reconnect recovery', async () => {
233+
const prisma = createPrismaMock();
234+
const tx = createTransactionMock();
235+
tx.userMcpServer.findUnique.mockResolvedValue({
236+
tokens: 'encrypted:{"access_token":"stale-token"}',
237+
codeVerifier: null,
238+
state: null,
239+
});
240+
mocks.authContext = {
241+
org: { id: 1 },
242+
user: { id: 'user-1' },
243+
prisma,
244+
};
245+
mocks.unsafePrisma.$transaction.mockImplementation(async (callback, _options) => callback(tx));
246+
mocks.mcpAuth.mockImplementation(async (provider) => {
247+
await expect(provider.tokens()).resolves.toBeUndefined();
248+
provider.authorizationUrl = 'https://oauth.example.com/authorize';
249+
return 'REDIRECT';
250+
});
251+
252+
const response = await POST(createRequest({
253+
serverId: 'server-1',
254+
returnTo: '/chat/abc123',
255+
forceAuthorization: true,
256+
}));
257+
258+
expect(await response.json()).toEqual({
259+
authorizationUrl: 'https://oauth.example.com/authorize',
260+
});
261+
});
262+
263+
test('does not report a forced reconnect as successful without an OAuth redirect', async () => {
264+
const prisma = createPrismaMock();
265+
const tx = createTransactionMock();
266+
mocks.authContext = {
267+
org: { id: 1 },
268+
user: { id: 'user-1' },
269+
prisma,
270+
};
271+
mocks.unsafePrisma.$transaction.mockImplementation(async (callback, _options) => callback(tx));
272+
mocks.mcpAuth.mockResolvedValue('AUTHORIZED');
273+
274+
const response = await POST(createRequest({
275+
serverId: 'server-1',
276+
returnTo: '/chat/abc123',
277+
forceAuthorization: true,
278+
}));
279+
280+
expect(response.status).toBe(502);
281+
expect(await response.json()).toMatchObject({
282+
message: 'Could not start connector reauthorization.',
283+
});
284+
});
285+
232286
test('ignores unsafe return paths', async () => {
233287
const prisma = createPrismaMock();
234288
const tx = createTransactionMock();

packages/web/src/app/api/(server)/ee/askmcp/connect/route.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { getEnabledMcpOAuthScopeNames } from '@/ee/features/chat/mcp/oauthScopeU
2424
const bodySchema = z.object({
2525
serverId: z.string(),
2626
returnTo: z.string().optional(),
27+
forceAuthorization: z.boolean().optional().default(false),
2728
});
2829
const logger = createLogger('mcp-connect');
2930
const MCP_AUTH_FETCH_TIMEOUT_MS = Math.min(env.SOURCEBOT_MCP_TOOL_CALL_TIMEOUT_MS, 30000);
@@ -146,6 +147,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
146147
callbackReturnTo,
147148
allowClientRegistration: true,
148149
requestedOAuthScopes: getEnabledMcpOAuthScopeNames(mcpServer.oauthScopes),
150+
forceAuthorization: parsed.data.forceAuthorization,
149151
});
150152

151153
let authResult: Awaited<ReturnType<typeof mcpAuth>>;
@@ -200,7 +202,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
200202
throw error;
201203
}
202204

203-
if (connectResult.authResult === 'AUTHORIZED') {
205+
if (connectResult.authResult === 'AUTHORIZED' && !parsed.data.forceAuthorization) {
204206
// Already has valid tokens (e.g., refreshed)
205207
void captureEvent('ask_mcp_connector_connection_completed', {
206208
...eventProperties,
@@ -209,6 +211,18 @@ export const POST = apiHandler(async (request: NextRequest) => {
209211
return { authorizationUrl: null } satisfies ConnectMcpResponse;
210212
}
211213

214+
if (connectResult.authResult === 'AUTHORIZED') {
215+
void captureEvent('ask_mcp_connector_connection_failed', {
216+
...eventProperties,
217+
failureReason: 'missing_authorization_url',
218+
});
219+
throw new ServiceErrorException({
220+
statusCode: StatusCodes.BAD_GATEWAY,
221+
errorCode: ErrorCode.UNEXPECTED_ERROR,
222+
message: 'Could not start connector reauthorization.',
223+
});
224+
}
225+
212226
if (!connectResult.authorizationUrl) {
213227
void captureEvent('ask_mcp_connector_connection_failed', {
214228
...eventProperties,

packages/web/src/ee/features/chat/agent.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,50 @@ describe('createMessageStream approval continuation', () => {
321321
});
322322
});
323323

324+
test('streams the connector ID when its tools fail to load', async () => {
325+
const { getConnectedMcpClients } = await import('@/ee/features/chat/mcp/mcpClientFactory');
326+
const { getMcpTools } = await import('@/ee/features/chat/mcp/mcpToolSets');
327+
vi.mocked(getConnectedMcpClients).mockResolvedValueOnce([
328+
{ serverId: 'server-linear', serverName: 'Linear' },
329+
] as never);
330+
vi.mocked(getMcpTools).mockResolvedValueOnce({
331+
tools: {},
332+
failedServers: [{ serverId: 'server-linear', serverName: 'Linear' }],
333+
serverFaviconUrls: {},
334+
toolDisplayNames: {},
335+
cleanup: vi.fn(),
336+
});
337+
mockAi.streamText.mockReturnValue(createFakeStreamResult());
338+
339+
await createMessageStream({
340+
chatId: 'chat-id',
341+
messages: [createUserMessage()],
342+
selectedRepos: [],
343+
disabledMcpServerIds: [],
344+
prisma: {},
345+
model: {},
346+
modelName: 'test-model',
347+
promptCacheStrategy: noopStrategy,
348+
onFinish: vi.fn(),
349+
onError: () => 'error',
350+
userId: 'user-id',
351+
orgId: 1,
352+
} as unknown as Parameters<typeof createMessageStream>[0]);
353+
354+
const execute = mockAi.latestCreateUIMessageStreamOptions?.execute;
355+
if (!execute) {
356+
throw new Error('Expected createUIMessageStream to capture execute callback.');
357+
}
358+
359+
const write = vi.fn();
360+
await execute({ writer: { merge: vi.fn(), write } });
361+
362+
expect(write).toHaveBeenCalledWith({
363+
type: 'data-mcp-failed-server',
364+
data: { serverId: 'server-linear', serverName: 'Linear' },
365+
});
366+
});
367+
324368
test.each([
325369
['dynamic', dynamicApprovalRespondedPart],
326370
['static', staticApprovalRespondedPart],

packages/web/src/ee/features/chat/agent.ts

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ import { addLineNumbers, fileReferenceToString, formatAttachmentsForPrompt, getA
2626
import { createTools } from "./tools";
2727
import { getConnectedMcpClients } from "@/ee/features/chat/mcp/mcpClientFactory";
2828
import { getMcpTools, McpToolsResult } from "@/ee/features/chat/mcp/mcpToolSets";
29+
import {
30+
createMcpAuthInterruptionDirective,
31+
denyApprovedToolApprovalsForAuthInterruption,
32+
getMcpAuthRequiredFailureFromAssistantMessage,
33+
McpToolAuthFailure,
34+
} from "@/ee/features/chat/mcp/mcpAuthFailure";
2935
import { buildMcpToolRegistry, McpToolRegistryEntry } from "@/ee/features/chat/mcp/mcpToolRegistry";
3036
import { PromptCacheStrategy, mergeProviderOptions, detectPromptCacheBreak, detectUnexpectedCacheMiss } from "./promptCaching";
3137
import { hasEntitlement } from '@/lib/entitlements';
@@ -332,9 +338,21 @@ export const createMessageStream = async ({
332338
? (lastMsg.metadata as SBChatMessageMetadata | undefined)
333339
: undefined;
334340

341+
// When the response was interrupted by a reconnect-required authentication
342+
// failure (detected via the safe tool error's marker text), the
343+
// continuation must run its final step with tool use disabled. Any
344+
// approval that was still approved is rewritten to a denial: once the
345+
// response is authentication-terminal, later approval actions are invalid.
346+
const priorMcpAuthFailure = hasApprovalContinuationReady
347+
? getMcpAuthRequiredFailureFromAssistantMessage(lastMsg)
348+
: undefined;
349+
335350
if (hasApprovalContinuationReady) {
351+
const continuationMessage = priorMcpAuthFailure
352+
? denyApprovedToolApprovalsForAuthInterruption(lastMsg, priorMcpAuthFailure.serverName)
353+
: lastMsg;
336354
const fullLastTurn = await convertToModelMessages(
337-
[lastMsg],
355+
[continuationMessage],
338356
{ ignoreIncompleteToolCalls: true }
339357
);
340358
messageHistory = [...messageHistory, ...fullLastTurn];
@@ -375,12 +393,22 @@ export const createMessageStream = async ({
375393
data: { modelToolName, rawToolName },
376394
});
377395
},
378-
onMcpServerFailed: (serverName) => {
396+
onMcpServerFailed: (server) => {
379397
writer.write({
380398
type: 'data-mcp-failed-server',
381-
data: { serverName },
399+
data: server,
382400
});
383401
},
402+
onMcpAuthRequired: (failure) => {
403+
// Transient: consumed live by the client to surface the
404+
// connector reconnect UI, never folded into persisted parts.
405+
writer.write({
406+
type: 'data-mcp-auth-required',
407+
data: failure,
408+
transient: true,
409+
});
410+
},
411+
priorMcpAuthFailure,
384412
traceId,
385413
chatId,
386414
prisma,
@@ -508,7 +536,14 @@ interface AgentOptions {
508536
onWriteSource: (source: Source) => void;
509537
onMcpServerDiscovered: (sanitizedName: string, faviconUrl: string) => void;
510538
onMcpToolDiscovered: (modelToolName: string, rawToolName: string) => void;
511-
onMcpServerFailed: (serverName: string) => void;
539+
onMcpServerFailed: (server: { serverId: string; serverName: string }) => void;
540+
// Fired at most once per connector per response when a tool call fails
541+
// with a reconnect-required authentication failure.
542+
onMcpAuthRequired: (failure: McpToolAuthFailure) => void;
543+
// Set when the incoming messages show this response was already
544+
// interrupted by an authentication failure (approval continuation): the
545+
// stream must run its final step with tool use disabled from step one.
546+
priorMcpAuthFailure?: { serverName: string };
512547
traceId: string;
513548
chatId: string;
514549
prisma: PrismaClient;
@@ -529,6 +564,8 @@ const createAgentStream = async ({
529564
onMcpServerDiscovered,
530565
onMcpToolDiscovered,
531566
onMcpServerFailed,
567+
onMcpAuthRequired,
568+
priorMcpAuthFailure,
532569
traceId,
533570
chatId,
534571
prisma,
@@ -564,6 +601,15 @@ const createAgentStream = async ({
564601
}))
565602
).filter((source) => source !== undefined);
566603

604+
// Mutable, response-scoped authentication failure state. `serverName` is
605+
// the first failed connector's display name (V1 supports recovery for a
606+
// single failed connector). Failures are deduplicated by connector so the
607+
// client sees at most one transient event per connector per response.
608+
const mcpAuthFailureState: { failure?: { serverName: string } } = {
609+
...(priorMcpAuthFailure ? { failure: { serverName: priorMcpAuthFailure.serverName } } : {}),
610+
};
611+
const reportedMcpAuthFailureServerIds = new Set<string>();
612+
567613
let mcpToolSetsObj: McpToolsResult = { tools: {}, failedServers: [], serverFaviconUrls: {}, toolDisplayNames: {}, cleanup: async () => {} };
568614
if (userId && orgId && await hasEntitlement('ask') && disabledMcpServerIds !== undefined) {
569615
try {
@@ -573,6 +619,16 @@ const createAgentStream = async ({
573619
chatId,
574620
traceId,
575621
source: 'sourcebot-ask-agent',
622+
}, {
623+
onAuthFailure: (failure) => {
624+
if (!mcpAuthFailureState.failure) {
625+
mcpAuthFailureState.failure = { serverName: failure.serverName };
626+
}
627+
if (!reportedMcpAuthFailureServerIds.has(failure.serverId)) {
628+
reportedMcpAuthFailureServerIds.add(failure.serverId);
629+
onMcpAuthRequired(failure);
630+
}
631+
},
576632
});
577633

578634
for (const [sanitizedName, faviconUrl] of Object.entries(mcpToolSetsObj.serverFaviconUrls)) {
@@ -590,8 +646,8 @@ const createAgentStream = async ({
590646
}
591647
}
592648

593-
for (const serverName of mcpToolSetsObj.failedServers) {
594-
onMcpServerFailed(serverName);
649+
for (const server of mcpToolSetsObj.failedServers) {
650+
onMcpServerFailed(server);
595651
}
596652

597653
const mcpRegistry = buildMcpToolRegistry(mcpToolSetsObj.tools);
@@ -715,14 +771,34 @@ const createAgentStream = async ({
715771
// rebuilds the step's messages each time as the original input plus
716772
// its own accumulated response messages. Re-applying the moving tail marker
717773
// to the new last message each step is safe and does not accumulate.
718-
prepareStep: (tailMarker || hasMcpTools) ? ({ steps, messages }) => {
774+
prepareStep: (tailMarker || hasMcpTools || mcpAuthFailureState.failure) ? ({ steps, messages }) => {
719775
const stepMessages = (tailMarker && messages.length > 0)
720776
? messages.map((message, index) =>
721777
index === messages.length - 1
722778
? { ...message, providerOptions: mergeProviderOptions(message.providerOptions, tailMarker) }
723779
: message)
724780
: undefined;
725781

782+
// Once a reconnect-required authentication failure occurs, the
783+
// response is terminal for tool use: every remaining step runs
784+
// with tool calling disabled (`toolChoice: 'none'` keeps the
785+
// tool definitions byte-stable for prompt caching) plus an
786+
// ephemeral directive to summarize completed work and prompt
787+
// the user to reconnect. In-flight tool calls of the failing
788+
// step have already run to completion by the time this fires.
789+
if (mcpAuthFailureState.failure) {
790+
return {
791+
messages: [
792+
...(stepMessages ?? messages),
793+
{
794+
role: 'user' as const,
795+
content: createMcpAuthInterruptionDirective(mcpAuthFailureState.failure.serverName),
796+
},
797+
],
798+
toolChoice: 'none' as const,
799+
};
800+
}
801+
726802
if (!hasMcpTools) {
727803
return stepMessages ? { messages: stepMessages } : {};
728804
}

0 commit comments

Comments
 (0)