mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-01 15:32:11 +08:00
Fix/vscode mcp oauth sign in (#8910)
* fix(vscode): restore MCP OAuth sign-in from settings UI
- Add Sign in control when MCP status is needs_auth; posts authenticateMcp
to run CLI POST /mcp/{name}/auth/authenticate.
- On SSE mcp.browser.open.failed, open the auth URL via
vscode.env.openExternal (CLI subprocess open() often fails under VS Code).
- Dedupe openExternal when multiple webviews receive the same event.
Fixes UI regression described in GitHub issue #8904.
Made-with: Cursor
* fix(vscode): satisfy ESLint max-lines and complexity for MCP OAuth
- Move MCP connect/disconnect/authenticate and openExternal dedupe to
kilo-provider/mcp-oauth.ts.
- eslint-disable-next-line complexity for the webview message router.
Made-with: Cursor
* fix: restore MCP OAuth sign-in in VS Code
* fix: simplify MCP OAuth callback delegation
* fix: clarify MCP OAuth callback delegation
---------
Co-authored-by: e.olbrych <e.olbrych@mkmc.pl>
Co-authored-by: marius-kilocode <marius@kilocode.ai>
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"kilo-code": patch
|
||||
"@kilocode/cli": patch
|
||||
---
|
||||
|
||||
Restore the Sign in action for MCP servers that require OAuth authentication in VS Code settings.
|
||||
@@ -66,6 +66,7 @@ import {
|
||||
import * as ModelState from "./kilo-provider/model-state"
|
||||
import { handleForkSession } from "./kilo-provider/fork-session"
|
||||
import { openConfig } from "./kilo-provider/open-config"
|
||||
import * as McpOAuth from "./kilo-provider/mcp-oauth"
|
||||
import { retryable, backoff, MAX_RETRIES } from "./util/retry"
|
||||
import { hasGit } from "./kilo-provider/git-status"
|
||||
// legacy-migration start
|
||||
@@ -796,14 +797,33 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
case "requestMcpStatus":
|
||||
this.fetchAndSendMcpStatus().catch((e) => console.error("[Kilo New] fetchAndSendMcpStatus failed:", e))
|
||||
break
|
||||
case "connectMcp":
|
||||
this.handleConnectMcp(message.name).catch((e) => console.error("[Kilo New] handleConnectMcp failed:", e))
|
||||
case "connectMcp": {
|
||||
const c1 = this.client
|
||||
if (c1) {
|
||||
void McpOAuth.connectMcpServer(c1, message.name, this.getWorkspaceDirectory(), () =>
|
||||
this.fetchAndSendMcpStatus(),
|
||||
).catch((e) => console.error("[Kilo New] connectMcpServer failed:", e))
|
||||
}
|
||||
break
|
||||
case "disconnectMcp":
|
||||
this.handleDisconnectMcp(message.name).catch((e) =>
|
||||
console.error("[Kilo New] handleDisconnectMcp failed:", e),
|
||||
)
|
||||
}
|
||||
case "disconnectMcp": {
|
||||
const c2 = this.client
|
||||
if (c2) {
|
||||
void McpOAuth.disconnectMcpServer(c2, message.name, this.getWorkspaceDirectory(), () =>
|
||||
this.fetchAndSendMcpStatus(),
|
||||
).catch((e) => console.error("[Kilo New] disconnectMcpServer failed:", e))
|
||||
}
|
||||
break
|
||||
}
|
||||
case "authenticateMcp": {
|
||||
const c = this.client
|
||||
if (c) {
|
||||
void McpOAuth.authenticateMcpServer(c, message.name, this.getWorkspaceDirectory(), () =>
|
||||
this.fetchAndSendMcpStatus(),
|
||||
).catch((e) => console.error("[Kilo New] authenticateMcpServer failed:", e))
|
||||
}
|
||||
break
|
||||
}
|
||||
|
||||
case "questionReply":
|
||||
this.noteFollowup(message.answers, message.sessionID)
|
||||
@@ -1950,30 +1970,6 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
}
|
||||
}
|
||||
|
||||
private async handleConnectMcp(name: string): Promise<void> {
|
||||
if (!this.client) return
|
||||
try {
|
||||
const directory = this.getWorkspaceDirectory()
|
||||
await this.client.mcp.connect({ name, directory })
|
||||
await this.fetchAndSendMcpStatus()
|
||||
} catch (error) {
|
||||
console.error("[Kilo New] KiloProvider: Failed to connect MCP:", name, error)
|
||||
await this.fetchAndSendMcpStatus()
|
||||
}
|
||||
}
|
||||
|
||||
private async handleDisconnectMcp(name: string): Promise<void> {
|
||||
if (!this.client) return
|
||||
try {
|
||||
const directory = this.getWorkspaceDirectory()
|
||||
await this.client.mcp.disconnect({ name, directory })
|
||||
await this.fetchAndSendMcpStatus()
|
||||
} catch (error) {
|
||||
console.error("[Kilo New] KiloProvider: Failed to disconnect MCP:", name, error)
|
||||
await this.fetchAndSendMcpStatus()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a marketplace item from a single scope and invalidate CLI caches.
|
||||
*/
|
||||
@@ -2978,6 +2974,11 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
this.permissionDirectories.delete(event.properties.requestID)
|
||||
}
|
||||
|
||||
if (event.type === "mcp.browser.open.failed") {
|
||||
McpOAuth.openMcpOAuthUrlOnce(event.properties.url)
|
||||
return
|
||||
}
|
||||
|
||||
if (event.type === "message.updated") {
|
||||
this.confirmations.confirm(event.properties.info.id)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
import * as vscode from "vscode"
|
||||
import type { KiloClient, McpStatus } from "@kilocode/sdk/v2/client"
|
||||
import { getErrorMessage } from "../kilo-provider-utils"
|
||||
|
||||
let lastMcpBrowserOpen: { url: string; at: number } | null = null
|
||||
|
||||
/** Dedupe when several webviews receive the same `mcp.browser.open.failed` SSE. */
|
||||
export function openMcpOAuthUrlOnce(url: string): void {
|
||||
const now = Date.now()
|
||||
if (lastMcpBrowserOpen && lastMcpBrowserOpen.url === url && now - lastMcpBrowserOpen.at < 4000) return
|
||||
lastMcpBrowserOpen = { url, at: now }
|
||||
void vscode.env.openExternal(vscode.Uri.parse(url)).then(
|
||||
(opened) => {
|
||||
if (opened) return
|
||||
void vscode.window.showErrorMessage(
|
||||
"MCP sign-in failed to open the browser. Check the Kilo logs for the authentication URL.",
|
||||
)
|
||||
},
|
||||
(error) => {
|
||||
console.error("[Kilo New] Failed to open MCP OAuth URL:", error)
|
||||
void vscode.window.showErrorMessage(
|
||||
"MCP sign-in failed to open the browser. Check the Kilo logs for the authentication URL.",
|
||||
)
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
export async function connectMcpServer(
|
||||
client: KiloClient,
|
||||
name: string,
|
||||
directory: string,
|
||||
refreshStatus: () => Promise<void>,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await client.mcp.connect({ name, directory })
|
||||
await refreshStatus()
|
||||
} catch (error) {
|
||||
console.error("[Kilo New] Failed to connect MCP:", name, error)
|
||||
await refreshStatus()
|
||||
}
|
||||
}
|
||||
|
||||
export async function disconnectMcpServer(
|
||||
client: KiloClient,
|
||||
name: string,
|
||||
directory: string,
|
||||
refreshStatus: () => Promise<void>,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await client.mcp.disconnect({ name, directory })
|
||||
await refreshStatus()
|
||||
} catch (error) {
|
||||
console.error("[Kilo New] Failed to disconnect MCP:", name, error)
|
||||
await refreshStatus()
|
||||
}
|
||||
}
|
||||
|
||||
export async function authenticateMcpServer(
|
||||
client: KiloClient,
|
||||
name: string,
|
||||
directory: string,
|
||||
refreshStatus: () => Promise<void>,
|
||||
): Promise<void> {
|
||||
try {
|
||||
const { data, error } = await client.mcp.auth.authenticate({ name, directory })
|
||||
if (error) {
|
||||
vscode.window.showErrorMessage(`MCP sign-in failed: ${getErrorMessage(error)}`)
|
||||
return
|
||||
}
|
||||
const status = data as McpStatus | undefined
|
||||
if (status?.status === "failed") {
|
||||
vscode.window.showErrorMessage(status.error || "MCP OAuth failed")
|
||||
} else if (status?.status === "needs_client_registration") {
|
||||
vscode.window.showErrorMessage(status.error || "MCP server requires client registration in config")
|
||||
}
|
||||
} catch (error) {
|
||||
console.error("[Kilo New] Failed to authenticate MCP:", name, error)
|
||||
vscode.window.showErrorMessage(getErrorMessage(error) || "MCP sign-in failed")
|
||||
} finally {
|
||||
await refreshStatus()
|
||||
}
|
||||
}
|
||||
@@ -658,6 +658,18 @@ const AgentBehaviourTab: Component = () => {
|
||||
</span>
|
||||
</div>
|
||||
<div style={{ display: "flex", gap: "4px", "align-items": "center" }}>
|
||||
<Show when={session.mcpStatus()[name]?.status === "needs_auth"}>
|
||||
<div onClick={(e: MouseEvent) => e.stopPropagation()}>
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="small"
|
||||
disabled={session.mcpLoading() === name}
|
||||
onClick={() => session.authenticateMcp(name)}
|
||||
>
|
||||
{language.t("common.signIn")}
|
||||
</Button>
|
||||
</div>
|
||||
</Show>
|
||||
<div onClick={(e: MouseEvent) => e.stopPropagation()}>
|
||||
<Switch
|
||||
checked={isConnected(name)}
|
||||
|
||||
@@ -175,6 +175,7 @@ interface SessionContextValue {
|
||||
mcpLoading: Accessor<string | null>
|
||||
connectMcp: (name: string) => void
|
||||
disconnectMcp: (name: string) => void
|
||||
authenticateMcp: (name: string) => void
|
||||
refreshMcpStatus: () => void
|
||||
selectedAgent: Accessor<string>
|
||||
selectAgent: (name: string) => void
|
||||
@@ -356,6 +357,13 @@ export const SessionProvider: ParentComponent = (props) => {
|
||||
vscode.postMessage({ type: "disconnectMcp", name })
|
||||
}
|
||||
|
||||
const authenticateMcp = (name: string) => {
|
||||
if (mcpLoading()) return
|
||||
if (!server.isConnected()) return
|
||||
setMcpLoading(name)
|
||||
vscode.postMessage({ type: "authenticateMcp", name })
|
||||
}
|
||||
|
||||
const refreshMcpStatus = () => {
|
||||
vscode.postMessage({ type: "requestMcpStatus" })
|
||||
}
|
||||
@@ -2219,6 +2227,7 @@ export const SessionProvider: ParentComponent = (props) => {
|
||||
mcpLoading,
|
||||
connectMcp,
|
||||
disconnectMcp,
|
||||
authenticateMcp,
|
||||
refreshMcpStatus,
|
||||
selectedAgent: selectedAgentName,
|
||||
selectAgent,
|
||||
|
||||
@@ -259,6 +259,11 @@ export interface DisconnectMcpMessage {
|
||||
name: string
|
||||
}
|
||||
|
||||
export interface AuthenticateMcpMessage {
|
||||
type: "authenticateMcp"
|
||||
name: string
|
||||
}
|
||||
|
||||
export interface SetLanguageRequest {
|
||||
type: "setLanguage"
|
||||
locale: string
|
||||
@@ -979,6 +984,7 @@ export type WebviewMessage =
|
||||
| RequestMcpStatusMessage
|
||||
| ConnectMcpMessage
|
||||
| DisconnectMcpMessage
|
||||
| AuthenticateMcpMessage
|
||||
| SetLanguageRequest
|
||||
| QuestionReplyRequest
|
||||
| QuestionRejectRequest
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
import type { Server } from "http"
|
||||
|
||||
const host = "127.0.0.1"
|
||||
|
||||
type State = {
|
||||
server: Server | undefined
|
||||
port: number
|
||||
path: string
|
||||
}
|
||||
|
||||
type Deps = {
|
||||
redirectUri?: string
|
||||
parse: (uri?: string) => { port: number; path: string }
|
||||
state: () => State
|
||||
set: (state: State) => void
|
||||
create: () => Server
|
||||
stop: () => Promise<void>
|
||||
info: (msg: string, data?: Record<string, unknown>) => void
|
||||
error: (msg: string, data?: Record<string, unknown>) => void
|
||||
}
|
||||
|
||||
let active = host
|
||||
let start: Promise<void> | null = null
|
||||
|
||||
export function parseHost(uri?: string): string {
|
||||
if (!uri) return host
|
||||
try {
|
||||
return new URL(uri).hostname || host
|
||||
} catch {
|
||||
return host
|
||||
}
|
||||
}
|
||||
|
||||
export function listen(srv: Server, host: string, port: number): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const fail = (err: Error & { code?: string }) => {
|
||||
srv.off("error", fail)
|
||||
if (err.code === "EADDRINUSE") {
|
||||
reject(
|
||||
new Error(
|
||||
`OAuth callback port ${port} is already in use. Close the other Kilo process or configure a different MCP OAuth redirect URI, then retry.`,
|
||||
),
|
||||
)
|
||||
return
|
||||
}
|
||||
reject(err)
|
||||
}
|
||||
|
||||
srv.once("error", fail)
|
||||
srv.listen(port, host, () => {
|
||||
srv.off("error", fail)
|
||||
resolve()
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
export async function ensureRunning(deps: Deps): Promise<void> {
|
||||
const cfg = deps.parse(deps.redirectUri)
|
||||
const nextHost = parseHost(deps.redirectUri)
|
||||
|
||||
if (start) await start
|
||||
|
||||
const state = deps.state()
|
||||
if (state.server && (active !== nextHost || state.port !== cfg.port || state.path !== cfg.path)) {
|
||||
deps.info("stopping oauth callback server to reconfigure", {
|
||||
oldHost: active,
|
||||
oldPort: state.port,
|
||||
newHost: nextHost,
|
||||
newPort: cfg.port,
|
||||
})
|
||||
await deps.stop()
|
||||
}
|
||||
|
||||
if (deps.state().server) return
|
||||
|
||||
active = nextHost
|
||||
const srv = deps.create()
|
||||
start = listen(srv, active, cfg.port).then(() => {
|
||||
deps.set({ server: srv, port: cfg.port, path: cfg.path })
|
||||
deps.info("oauth callback server started", { host: active, port: cfg.port, path: cfg.path })
|
||||
})
|
||||
|
||||
try {
|
||||
await start
|
||||
} catch (err) {
|
||||
if (err instanceof Error && err.message.includes("already in use")) {
|
||||
deps.error("oauth callback bind failed: port already in use", { host: active, port: cfg.port, path: cfg.path })
|
||||
}
|
||||
throw err
|
||||
} finally {
|
||||
start = null
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@ import { createConnection } from "net"
|
||||
import { createServer } from "http"
|
||||
import { Log } from "../util"
|
||||
import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH, parseRedirectUri } from "./oauth-provider"
|
||||
import * as KiloOAuthCallback from "../kilocode/mcp-oauth-callback" // kilocode_change
|
||||
|
||||
const log = Log.create({ service: "mcp.oauth-callback" })
|
||||
|
||||
@@ -145,6 +146,24 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
|
||||
}
|
||||
|
||||
export async function ensureRunning(redirectUri?: string): Promise<void> {
|
||||
// kilocode_change start - delegate Kilo-specific callback binding from here because OAuth state lives in this module
|
||||
await KiloOAuthCallback.ensureRunning({
|
||||
redirectUri,
|
||||
parse: parseRedirectUri,
|
||||
state: () => ({ server, port: currentPort, path: currentPath }),
|
||||
set: (next) => {
|
||||
server = next.server
|
||||
currentPort = next.port
|
||||
currentPath = next.path
|
||||
},
|
||||
create: () => createServer(handleRequest),
|
||||
stop,
|
||||
info: (msg, data) => log.info(msg, data),
|
||||
error: (msg, data) => log.error(msg, data),
|
||||
})
|
||||
return
|
||||
// kilocode_change end
|
||||
|
||||
// Parse the redirect URI to get port and path (uses defaults if not provided)
|
||||
const { port, path } = parseRedirectUri(redirectUri)
|
||||
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, test, afterEach } from "bun:test"
|
||||
import { createServer } from "http"
|
||||
import { McpOAuthCallback } from "../../src/mcp/oauth-callback"
|
||||
|
||||
describe("Kilo MCP OAuth callback", () => {
|
||||
afterEach(async () => {
|
||||
await McpOAuthCallback.stop()
|
||||
})
|
||||
|
||||
test("fails fast when the callback port belongs to another process", async () => {
|
||||
const blocker = createServer((_req, res) => {
|
||||
res.writeHead(200)
|
||||
res.end("occupied")
|
||||
})
|
||||
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
blocker.once("error", reject)
|
||||
blocker.listen(0, "127.0.0.1", resolve)
|
||||
})
|
||||
|
||||
try {
|
||||
const address = blocker.address()
|
||||
if (!address || typeof address === "string") throw new Error("missing blocker address")
|
||||
|
||||
await expect(
|
||||
McpOAuthCallback.ensureRunning(`http://127.0.0.1:${address.port}/mcp/oauth/callback`),
|
||||
).rejects.toThrow("already in use")
|
||||
expect(McpOAuthCallback.isRunning()).toBe(false)
|
||||
} finally {
|
||||
await new Promise<void>((resolve) => blocker.close(() => resolve()))
|
||||
}
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user