refactor(core): replace legacy logger with Effect logging (#31310)
This commit is contained in:
@@ -15,7 +15,6 @@ import {
|
||||
} from "@modelcontextprotocol/sdk/types.js"
|
||||
import { Config } from "@/config/config"
|
||||
import { ConfigMCPV1 } from "@opencode-ai/core/v1/config/mcp"
|
||||
import * as Log from "@opencode-ai/core/util/log"
|
||||
import { NamedError } from "@opencode-ai/core/util/error"
|
||||
import { InstallationVersion } from "@opencode-ai/core/installation/version"
|
||||
import { withTimeout } from "@/util/timeout"
|
||||
@@ -33,7 +32,6 @@ import { InstanceState } from "@/effect/instance-state"
|
||||
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
|
||||
import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
|
||||
|
||||
const log = Log.create({ service: "mcp" })
|
||||
const DEFAULT_TIMEOUT = 30_000
|
||||
|
||||
const TolerantListToolsResultSchema = ListToolsResultSchema.extend({
|
||||
@@ -117,7 +115,6 @@ const sanitize = (s: string) => s.replace(/[^a-zA-Z0-9_-]/g, "_")
|
||||
|
||||
function remoteURL(key: string, value: string) {
|
||||
if (URL.canParse(value)) return new URL(value)
|
||||
log.warn("invalid remote mcp url", { key })
|
||||
}
|
||||
|
||||
function isOutputSchemaValidationError(error: Error) {
|
||||
@@ -135,7 +132,6 @@ function listTools(key: string, client: MCPClient, timeout: number) {
|
||||
Effect.catch((error) => {
|
||||
if (!isOutputSchemaValidationError(error)) return Effect.fail(error)
|
||||
|
||||
log.warn("failed to validate MCP tool output schemas, retrying without output schema validation", { key, error })
|
||||
return Effect.tryPromise({
|
||||
try: () =>
|
||||
client.request({ method: "tools/list" }, TolerantListToolsResultSchema, {
|
||||
@@ -189,7 +185,6 @@ function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number
|
||||
function defs(key: string, client: MCPClient, timeout?: number) {
|
||||
return listTools(key, client, timeout ?? DEFAULT_TIMEOUT).pipe(
|
||||
Effect.catch((err) => {
|
||||
log.error("failed to get tools from client", { key, error: err })
|
||||
return Effect.succeed(undefined)
|
||||
}),
|
||||
)
|
||||
@@ -204,7 +199,6 @@ function fetchFromClient<T extends { name: string }>(
|
||||
return Effect.tryPromise({
|
||||
try: () => listFn(client),
|
||||
catch: (e: any) => {
|
||||
log.warn(`failed to get ${label}`, { clientName, error: e.message })
|
||||
return e
|
||||
},
|
||||
}).pipe(
|
||||
@@ -330,9 +324,7 @@ export const layer = Layer.effect(
|
||||
redirectUri: oauthConfig?.redirectUri,
|
||||
},
|
||||
{
|
||||
onRedirect: async (url) => {
|
||||
log.info("oauth redirect requested", { key, url: url.toString() })
|
||||
},
|
||||
onRedirect: async () => {},
|
||||
},
|
||||
auth,
|
||||
)
|
||||
@@ -367,8 +359,6 @@ export const layer = Layer.effect(
|
||||
error instanceof UnauthorizedError || (authProvider && lastError.message.includes("OAuth"))
|
||||
|
||||
if (isAuthError) {
|
||||
log.info("mcp server requires authentication", { key, transport: name })
|
||||
|
||||
if (lastError.message.includes("registration") || lastError.message.includes("client_id")) {
|
||||
lastStatus = {
|
||||
status: "needs_client_registration" as const,
|
||||
@@ -396,18 +386,11 @@ export const layer = Layer.effect(
|
||||
}
|
||||
}
|
||||
|
||||
log.debug("transport connection failed", {
|
||||
key,
|
||||
transport: name,
|
||||
url: mcp.url,
|
||||
error: lastError.message,
|
||||
})
|
||||
lastStatus = { status: "failed" as const, error: lastError.message }
|
||||
return Effect.succeed(undefined)
|
||||
}),
|
||||
)
|
||||
if (result) {
|
||||
log.info("connected", { key, transport: result.transportName })
|
||||
return { client: result.client as MCPClient | undefined, status: { status: "connected" } as Status }
|
||||
}
|
||||
// If this was an auth error, stop trying other transports
|
||||
@@ -437,9 +420,6 @@ export const layer = Layer.effect(
|
||||
...mcp.environment,
|
||||
},
|
||||
})
|
||||
transport.stderr?.on("data", (chunk: Buffer) => {
|
||||
log.info(`mcp stderr: ${chunk.toString()}`, { key })
|
||||
})
|
||||
|
||||
const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT
|
||||
return yield* connectTransport(transport, connectTimeout).pipe(
|
||||
@@ -449,7 +429,6 @@ export const layer = Layer.effect(
|
||||
})),
|
||||
Effect.catch((error): Effect.Effect<{ client: MCPClient | undefined; status: Status }> => {
|
||||
const msg = error instanceof Error ? error.message : String(error)
|
||||
log.error("local mcp startup failed", { key, command: mcp.command, cwd, error: msg })
|
||||
return Effect.succeed({ client: undefined, status: { status: "failed", error: msg } })
|
||||
}),
|
||||
)
|
||||
@@ -457,11 +436,9 @@ export const layer = Layer.effect(
|
||||
|
||||
const create = Effect.fn("MCP.create")(function* (key: string, mcp: ConfigMCPV1.Info) {
|
||||
if (mcp.enabled === false) {
|
||||
log.info("mcp server disabled", { key })
|
||||
return DISABLED_RESULT
|
||||
}
|
||||
|
||||
log.info("found", { key, type: mcp.type })
|
||||
|
||||
const { client: mcpClient, status } =
|
||||
mcp.type === "remote"
|
||||
@@ -478,7 +455,6 @@ export const layer = Layer.effect(
|
||||
return { status: { status: "failed", error: "Failed to get tools" } } satisfies CreateResult
|
||||
}
|
||||
|
||||
log.info("create() successfully created client", { key, toolCount: listed.length })
|
||||
return { mcpClient, status, defs: listed } satisfies CreateResult
|
||||
})
|
||||
const cfgSvc = yield* Config.Service
|
||||
@@ -510,7 +486,6 @@ export const layer = Layer.effect(
|
||||
function watch(s: State, name: string, client: MCPClient, bridge: EffectBridge.Shape, timeout?: number) {
|
||||
if (!client.getServerCapabilities()?.tools) return
|
||||
client.setNotificationHandler(ToolListChangedNotificationSchema, async () => {
|
||||
log.info("tools list changed notification received", { server: name })
|
||||
if (s.clients[name] !== client || s.status[name]?.status !== "connected") return
|
||||
|
||||
const listed = await bridge.promise(defs(name, client, timeout))
|
||||
@@ -539,7 +514,6 @@ export const layer = Layer.effect(
|
||||
([key, mcp]) =>
|
||||
Effect.gen(function* () {
|
||||
if (!isMcpConfigured(mcp)) {
|
||||
log.error("Ignoring MCP config entry without type", { key })
|
||||
return
|
||||
}
|
||||
|
||||
@@ -690,7 +664,6 @@ export const layer = Layer.effect(
|
||||
|
||||
const listed = s.defs[clientName]
|
||||
if (!listed) {
|
||||
log.warn("missing cached tools for connected server", { clientName })
|
||||
return
|
||||
}
|
||||
|
||||
@@ -745,13 +718,11 @@ export const layer = Layer.effect(
|
||||
const s = yield* InstanceState.get(state)
|
||||
const client = s.clients[clientName]
|
||||
if (!client) {
|
||||
log.warn(`client not found for ${label}`, { clientName })
|
||||
return undefined
|
||||
}
|
||||
return yield* Effect.tryPromise({
|
||||
try: () => fn(client),
|
||||
catch: (e: any) => {
|
||||
log.error(`failed to ${label}`, { clientName, ...meta, error: e?.message })
|
||||
return e
|
||||
},
|
||||
}).pipe(Effect.orElseSucceed(() => undefined))
|
||||
@@ -873,7 +844,6 @@ export const layer = Layer.effect(
|
||||
return yield* storeClient(s, mcpName, client, listed, mcpConfig.timeout)
|
||||
}
|
||||
|
||||
log.info("opening browser for oauth", { mcpName, url: result.authorizationUrl, state: result.oauthState })
|
||||
|
||||
const callbackPromise = McpOAuthCallback.waitForCallback(result.oauthState, mcpName)
|
||||
|
||||
@@ -894,7 +864,6 @@ export const layer = Layer.effect(
|
||||
}),
|
||||
),
|
||||
Effect.catch(() => {
|
||||
log.warn("failed to open browser, user must open URL manually", { mcpName })
|
||||
return events.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl }).pipe(Effect.ignore)
|
||||
}),
|
||||
)
|
||||
@@ -918,7 +887,6 @@ export const layer = Layer.effect(
|
||||
const result = yield* Effect.tryPromise({
|
||||
try: () => transport.finishAuth(authorizationCode).then(() => true as const),
|
||||
catch: (error) => {
|
||||
log.error("failed to finish oauth", { mcpName, error })
|
||||
return error
|
||||
},
|
||||
}).pipe(Effect.option)
|
||||
@@ -939,7 +907,6 @@ export const layer = Layer.effect(
|
||||
yield* auth.remove(mcpName)
|
||||
McpOAuthCallback.cancelPending(mcpName)
|
||||
pendingOAuthTransports.delete(mcpName)
|
||||
log.info("removed oauth credentials", { mcpName })
|
||||
})
|
||||
|
||||
const supportsOAuth = Effect.fn("MCP.supportsOAuth")(function* (mcpName: string) {
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
import { createConnection } from "net"
|
||||
import { createServer } from "http"
|
||||
import * as Log from "@opencode-ai/core/util/log"
|
||||
import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH, parseRedirectUri } from "./oauth-provider"
|
||||
|
||||
const log = Log.create({ service: "mcp.oauth-callback" })
|
||||
|
||||
// Current callback server configuration (may differ from defaults if custom redirectUri is used)
|
||||
let currentPort = OAUTH_CALLBACK_PORT
|
||||
@@ -87,12 +85,10 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
|
||||
const error = url.searchParams.get("error")
|
||||
const errorDescription = url.searchParams.get("error_description")
|
||||
|
||||
log.info("received oauth callback", { hasCode: !!code, state, error })
|
||||
|
||||
// Enforce state parameter presence
|
||||
if (!state) {
|
||||
const errorMsg = "Missing required state parameter - potential CSRF attack"
|
||||
log.error("oauth callback missing state parameter", { url: url.toString() })
|
||||
res.writeHead(400, { "Content-Type": "text/html" })
|
||||
res.end(HTML_ERROR(errorMsg))
|
||||
return
|
||||
@@ -121,7 +117,6 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
|
||||
// Validate state parameter
|
||||
if (!pendingAuths.has(state)) {
|
||||
const errorMsg = "Invalid or expired state parameter - potential CSRF attack"
|
||||
log.error("oauth callback with invalid state", { state, pendingStates: Array.from(pendingAuths.keys()) })
|
||||
res.writeHead(400, { "Content-Type": "text/html" })
|
||||
res.end(HTML_ERROR(errorMsg))
|
||||
return
|
||||
@@ -144,7 +139,6 @@ export async function ensureRunning(redirectUri?: string): Promise<void> {
|
||||
|
||||
// If server is running on a different port/path, stop it first
|
||||
if (server && (currentPort !== port || currentPath !== path)) {
|
||||
log.info("stopping oauth callback server to reconfigure", { oldPort: currentPort, newPort: port })
|
||||
await stop()
|
||||
}
|
||||
|
||||
@@ -152,7 +146,6 @@ export async function ensureRunning(redirectUri?: string): Promise<void> {
|
||||
|
||||
const running = await isPortInUse(port)
|
||||
if (running) {
|
||||
log.info("oauth callback server already running on another instance", { port })
|
||||
return
|
||||
}
|
||||
|
||||
@@ -162,7 +155,6 @@ export async function ensureRunning(redirectUri?: string): Promise<void> {
|
||||
server = createServer(handleRequest)
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server!.listen(currentPort, () => {
|
||||
log.info("oauth callback server started", { port: currentPort, path: currentPath })
|
||||
resolve()
|
||||
})
|
||||
server!.on("error", reject)
|
||||
@@ -214,7 +206,6 @@ export async function stop(): Promise<void> {
|
||||
if (server) {
|
||||
await new Promise<void>((resolve) => server!.close(() => resolve()))
|
||||
server = undefined
|
||||
log.info("oauth callback server stopped")
|
||||
}
|
||||
|
||||
for (const [_name, pending] of pendingAuths) {
|
||||
|
||||
@@ -7,9 +7,7 @@ import type {
|
||||
} from "@modelcontextprotocol/sdk/shared/auth.js"
|
||||
import { Effect } from "effect"
|
||||
import { McpAuth } from "./auth"
|
||||
import * as Log from "@opencode-ai/core/util/log"
|
||||
|
||||
const log = Log.create({ service: "mcp.oauth" })
|
||||
|
||||
const OAUTH_CALLBACK_PORT = 19876
|
||||
const OAUTH_CALLBACK_PATH = "/mcp/oauth/callback"
|
||||
@@ -70,7 +68,6 @@ export class McpOAuthProvider implements OAuthClientProvider {
|
||||
if (entry?.clientInfo) {
|
||||
// Check if client secret has expired
|
||||
if (entry.clientInfo.clientSecretExpiresAt && entry.clientInfo.clientSecretExpiresAt < Date.now() / 1000) {
|
||||
log.info("client secret expired, need to re-register", { mcpName: this.mcpName })
|
||||
return undefined
|
||||
}
|
||||
return {
|
||||
@@ -96,10 +93,6 @@ export class McpOAuthProvider implements OAuthClientProvider {
|
||||
this.serverUrl,
|
||||
),
|
||||
)
|
||||
log.info("saved dynamically registered client", {
|
||||
mcpName: this.mcpName,
|
||||
clientId: info.client_id,
|
||||
})
|
||||
}
|
||||
|
||||
async tokens(): Promise<OAuthTokens | undefined> {
|
||||
@@ -131,11 +124,9 @@ export class McpOAuthProvider implements OAuthClientProvider {
|
||||
this.serverUrl,
|
||||
),
|
||||
)
|
||||
log.info("saved oauth tokens", { mcpName: this.mcpName })
|
||||
}
|
||||
|
||||
async redirectToAuthorization(authorizationUrl: URL): Promise<void> {
|
||||
log.info("redirecting to authorization", { mcpName: this.mcpName, url: authorizationUrl.toString() })
|
||||
await this.callbacks.onRedirect(authorizationUrl)
|
||||
}
|
||||
|
||||
@@ -173,7 +164,6 @@ export class McpOAuthProvider implements OAuthClientProvider {
|
||||
}
|
||||
|
||||
async invalidateCredentials(type: "all" | "client" | "tokens"): Promise<void> {
|
||||
log.info("invalidating credentials", { mcpName: this.mcpName, type })
|
||||
const entry = await Effect.runPromise(this.auth.get(this.mcpName))
|
||||
if (!entry) {
|
||||
return
|
||||
|
||||
Reference in New Issue
Block a user