Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/core/src/tools/mcp-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,34 @@ describe('mcp-client', () => {
});
});

it('skips OAuth without opening a browser in non-interactive mode on 401', async () => {
const { authenticate, connect, workspaceContext } = setupHttpOAuthRetry(
new Error(
'HTTP 401 Unauthorized\nwww-authenticate: Bearer realm="example", resource_metadata="https://example.com/.well-known/oauth-protected-resource"',
),
);

await expect(
connectToMcpServer(
'http-server',
{ httpUrl: 'http://test-server/mcp' },
false,
workspaceContext,
undefined,
false, // non-interactive (`-p`)
),
).rejects.toThrow(
// Pin the non-interactive branch specifically: the dialog-instruction
// substring alone appears in many OAuth error paths.
/non-interactive mode/,
);

// The interactive OAuth flow (which opens a browser) must not run.
expect(authenticate).not.toHaveBeenCalled();
// Only the initial connect attempt happens; no retry after OAuth.
expect(connect).toHaveBeenCalledTimes(1);
});

it('falls back to base-url OAuth discovery when www-authenticate lacks resource metadata', async () => {
const { authenticate, connect, discoverOAuthConfig, workspaceContext } =
setupHttpOAuthRetry(
Expand Down
18 changes: 18 additions & 0 deletions packages/core/src/tools/mcp-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ export async function connectAndDiscover(
debugMode,
workspaceContext,
sendSdkMcpMessage,
cliConfig.isInteractive(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] isInteractive() is also false for ACP / stream-json sessions, not just one-shot -p runs. Those modes still support user interaction: the existing permission gates explicitly exempt getExperimentalZedIntegration() and InputFormat.STREAM_JSON (for example coreToolScheduler.ts:2500-2503). With an HTTP MCP server that returns 401 and needs first-time OAuth, this call now passes false; the new guard throws before authenticate() can open the browser. The server remains disconnected for every ACP/stream-json user although this flow worked before the change. Derive this flag from whether the session can handle interaction (isInteractive || experimental Zed || stream-json) and cover that configuration path in the test.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The new test calls connectToMcpServer(..., false) directly, so it does not cover this production propagation. Because the callee defaults interactive to true, accidentally omitting this argument later would remain type-correct and leave the test green while the startup regression returns. Please add a test through connectAndDiscover or discoverMcpTools with cliConfig.isInteractive() returning false, then assert that OAuth authentication is not attempted and the server follows the disconnected path.

— GPT-5 via Qwen Code /review

);

mcpClient.onerror = (error) => {
Expand Down Expand Up @@ -1291,6 +1292,7 @@ export function hasNetworkTransport(config: MCPServerConfig): boolean {
* @param mcpServerName The name of the MCP server, used for logging and identification.
* @param mcpServerConfig The configuration specifying how to connect to the server.
* @param sendSdkMcpMessage Optional callback for SDK MCP servers to route messages via control plane.
* @param interactive When false (non-interactive/headless `-p` mode), skip OAuth flows that open a browser; the connection is rejected instead of blocking on a callback.
* @returns A promise that resolves to a connected MCP `Client` instance.
* @throws An error if the connection fails or the configuration is invalid.
*/
Expand All @@ -1300,6 +1302,10 @@ export async function connectToMcpServer(
debugMode: boolean,
workspaceContext: WorkspaceContext,
sendSdkMcpMessage?: SendSdkMcpMessage,
// When false (non-interactive `-p` mode), never open a browser for MCP OAuth.
// A server that would require an interactive OAuth flow is skipped instead of
// blocking startup on a browser callback.
interactive: boolean = true,
): Promise<Client> {
const mcpClient = new Client({
name: 'qwen-code-mcp-client',
Expand Down Expand Up @@ -1439,6 +1445,18 @@ export async function connectToMcpServer(
throw new Error(oauthMessage);
}

// In non-interactive mode (`-p`) we must never open a browser for OAuth,
// as it would block startup on a callback the user can't complete. Skip
// this server instead — it surfaces via the normal failed-connection path.
if (!interactive) {
const oauthMessage =
`The MCP server '${mcpServerName}' requires OAuth authentication, ` +
`but Qwen Code is running in non-interactive mode. Skipping this server. ` +
getMcpOAuthDialogInstruction('authenticate', mcpServerName);
debugLogger.warn(oauthMessage);
throw new Error(oauthMessage);
}

// Try to extract www-authenticate header from the error
let wwwAuthenticate = extractWWWAuthenticateHeader(errorString);

Expand Down
Loading