Skip to content
Merged
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
7 changes: 4 additions & 3 deletions apps/sidecar/src/services/mcp/workspace-mcp-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import { createDiagnosticLogSummary, createLogger, type Logger } from "../infra/

export interface WorkspaceSdkMcpManager {
sync(configs: Record<string, NormalizedMcpServerConfig>): void;
connect?(serverId: string): Promise<void>;
ensureConnected?(serverId: string): Promise<void>;
connect?(serverId: string, options?: { force?: boolean }): Promise<void>;
ensureConnected?(serverId: string, options?: { force?: boolean }): Promise<void>;
disconnect(serverId: string): Promise<void>;
dispose(): Promise<void>;
getStatus(): Record<string, McpClientServerStatus>;
Expand Down Expand Up @@ -377,7 +377,8 @@ export class WorkspaceMcpManager {
const state = this.ensureWorkspaceState(workspaceSlug);
state.sdk.sync(this.normalizedConfigs(config));
try {
await (state.sdk.connect ?? state.sdk.ensureConnected)?.call(state.sdk, serverId);
// Manual probes bypass the manager's reconnect backoff (#312).
await (state.sdk.connect ?? state.sdk.ensureConnected)?.call(state.sdk, serverId, { force: true });
} catch (error) {
// 连接失败的底层错误常内嵌 URL/凭据片段;与其余 MCP 路径一致经脱敏再下发,
// 且错误码归一,不把原文直抛 renderer(#403)
Expand Down
58 changes: 57 additions & 1 deletion packages/sdk/src/lsp/adapters.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { describe, expect, test } from 'bun:test'
import { parseSwiftLintDiagnostics } from './adapters.js'
import { chmod, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join, resolve } from 'node:path'
import { collectLspAdapterDiagnostics, parseSwiftLintDiagnostics } from './adapters.js'
import { resolveShellInvocation, shellKind } from '../utils/shell-invocation.js'

describe('SwiftLint LSP adapter', () => {
test('normalizes JSON diagnostics to LSP positions and severities', () => {
Expand Down Expand Up @@ -41,4 +45,56 @@ describe('SwiftLint LSP adapter', () => {
expect(parseSwiftLintDiagnostics('swiftlint: unknown output')).toBeUndefined()
expect(parseSwiftLintDiagnostics('{}')).toBeUndefined()
})

test('shellKind classifies resolved shell executables (#328)', () => {
expect(shellKind('powershell.exe')).toBe('powershell')
expect(shellKind('C:\\Windows\\System32\\WindowsPowerShell\\1.0\\powershell.exe')).toBe('powershell')
expect(shellKind('/usr/bin/pwsh')).toBe('powershell')
expect(shellKind('bash.exe')).toBe('bash')
expect(shellKind('/usr/bin/bash')).toBe('bash')
expect(shellKind('C:\\Program Files\\Git\\bin\\bash.exe')).toBe('bash')
expect(shellKind('sh')).toBe('bash')
})

test('picks the PowerShell call operator from the resolved shell, not the platform (#328)', async () => {
const root = await mkdtemp(join(tmpdir(), 'lume-lsp-adapter-'))
try {
await writeFile(join(root, '.swiftlint.yml'), '')
const swiftlint = join(root, process.platform === 'win32' ? 'swiftlint.cmd' : 'swiftlint')
await writeFile(swiftlint, '')
// resolveLspExecutable requires the exec bit on POSIX; without it the
// adapter silently finds no swiftlint and returns undefined.
if (process.platform !== 'win32') await chmod(swiftlint, 0o755)

let captured: string | undefined
const context = {
cwd: root,
// Pin the executable explicitly so resolution never falls through to
// a real swiftlint installed on the machine's PATH.
toolConfig: { lsp: { servers: { swiftlint: { command: swiftlint } } } },
executeNestedTool: async (invocation: { params: { command: string } }) => {
captured = invocation.params.command
return { content: '[]' }
},
}
const filePath = join(root, 'Sources', 'Demo.swift')
const result = await collectLspAdapterDiagnostics(filePath, context as any)

expect(result).toBeDefined()
expect(captured).toBeDefined()
const command = captured!
const withoutOperator = command.replace(/^& /, '')
expect(withoutOperator).toBe(`${swiftlint} lint --path ${resolve(filePath)} --quiet --reporter json`)
// The operator decision must match exactly what the Bash tool will
// resolve for this command line: PowerShell dialect gets '& ', Git Bash
// must not (a leading '&' is a syntax error there).
const expectedPowerShell = shellKind(resolveShellInvocation(withoutOperator).command) === 'powershell'
expect(command.startsWith('& ')).toBe(expectedPowerShell)
if (process.platform !== 'win32') {
expect(command.startsWith('& ')).toBe(false)
}
} finally {
await rm(root, { recursive: true, force: true })
}
})
})
8 changes: 7 additions & 1 deletion packages/sdk/src/lsp/adapters.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { resolve } from 'node:path'
import type { LspDiagnosticBatch, ToolContext } from '../types.js'
import { findLspWorkspaceRoot, resolveLspExecutable } from './registry.js'
import { resolveShellInvocation, shellKind } from '../utils/shell-invocation.js'

export async function collectLspAdapterDiagnostics(
filePath: string,
Expand All @@ -19,10 +20,15 @@ export async function collectLspAdapterDiagnostics(
if (!root) return undefined
const command = await resolveLspExecutable(configured.command ?? 'swiftlint', root, configured.cwd)
if (!command) return undefined
const cliCommand = `${quote(command)} lint --path ${quote(resolve(filePath))} --quiet --reporter json`
// PowerShell needs an explicit call operator to invoke a quoted path, but a
// leading '&' is a syntax error in Git Bash — pick the operator from the
// shell the Bash tool will actually resolve, not from the platform (#328).
const callOperator = shellKind(resolveShellInvocation(cliCommand).command) === 'powershell' ? '& ' : ''
const result = await context.executeNestedTool({
toolName: 'Bash',
params: {
command: `${process.platform === 'win32' ? '& ' : ''}${quote(command)} lint --path ${quote(resolve(filePath))} --quiet --reporter json`,
command: `${callOperator}${cliCommand}`,
purpose: 'lsp-diagnostics',
description: 'SwiftLint diagnostics',
},
Expand Down
114 changes: 114 additions & 0 deletions packages/sdk/src/lsp/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
LspClient,
parseLspMessages,
resolveLspServerConfigsForFile,
setLspWriteTimeout,
shutdownLspClients,
warmupLspClients,
} from './client.js'
Expand Down Expand Up @@ -163,6 +164,54 @@ describe('LSP protocol helpers', () => {
])
})

test('fails the client when the server never drains stdin writes (#327)', async () => {
setLspWriteTimeout(30)
try {
const client = Object.create(LspClient.prototype) as any
client.writeQueue = Promise.resolve()
client.lastActivity = 0
client.dead = false
client.initialized = true
client.disposed = false
client.pending = new Map()
let dead = false
client.onDead = () => { dead = true }
// Accepts the write but never invokes the callback: a wedged pipe.
client.process = { stdin: { writable: true, write: () => undefined } }

await expect(client['send']({ jsonrpc: '2.0', method: 'test/hang' })).rejects.toThrow(/timed out/i)
expect(dead).toBe(true)
expect(client.dead).toBe(true)
} finally {
setLspWriteTimeout(10_000)
}
})

test('resolves send once the write callback fires and leaves the client alive (#327)', async () => {
setLspWriteTimeout(30)
try {
const client = Object.create(LspClient.prototype) as any
client.writeQueue = Promise.resolve()
client.lastActivity = 0
client.dead = false
client.initialized = true
client.disposed = false
client.pending = new Map()
let dead = false
client.onDead = () => { dead = true }
client.process = { stdin: { writable: true, write: (_body: Buffer, callback: () => void) => callback() } }

await expect(client['send']({ jsonrpc: '2.0', method: 'test/ok' })).resolves.toBeUndefined()
expect(dead).toBe(false)

// The timer must be released after success rather than firing later.
await new Promise((resolve) => setTimeout(resolve, 60))
expect(dead).toBe(false)
} finally {
setLspWriteTimeout(10_000)
}
})

test('spawns .cmd server shims through cmd.exe on Windows', async () => {
if (process.platform !== 'win32') return
const root = await mkdtemp(join(tmpdir(), 'lume-lsp-cmd-shim-'))
Expand Down Expand Up @@ -277,6 +326,71 @@ process.stdin.on('data', (chunk) => {
}
})

test('spawns servers with the minimal default environment plus explicit env, not the host environment (#380)', async () => {
const root = await mkdtemp(join(tmpdir(), 'lume-lsp-env-'))
const secretName = 'LUME_LSP_ENV_PROBE_SECRET'
const previousSecret = process.env[secretName]
process.env[secretName] = 'top-secret-value'
try {
const script = join(root, 'server.mjs')
const source = join(root, 'index.ts')
await writeFile(join(root, 'package.json'), '{}')
await writeFile(source, '')
await writeFile(script, `
let buffer = Buffer.alloc(0)
const send = (value) => {
const body = Buffer.from(JSON.stringify(value))
process.stdout.write(Buffer.concat([Buffer.from('Content-Length: ' + body.length + '\\r\\n\\r\\n'), body]))
}
process.stdin.on('data', (chunk) => {
buffer = Buffer.concat([buffer, chunk])
while (true) {
const end = buffer.indexOf('\\r\\n\\r\\n')
if (end < 0) return
const match = buffer.subarray(0, end).toString().match(/Content-Length:\\s*(\\d+)/i)
if (!match) process.exit(2)
const message = JSON.parse(buffer.subarray(end + 4, end + 4 + Number(match[1])))
buffer = buffer.subarray(end + 4 + Number(match[1]))
if (message.method === 'initialize') send({ jsonrpc: '2.0', id: message.id, result: { capabilities: {} } })
else if (message.method === 'shutdown' || (message.id !== undefined && !message.method)) send({ jsonrpc: '2.0', id: message.id, result: null })
else if (message.method === 'test/env') {
send({ jsonrpc: '2.0', id: message.id, result: {
secret: process.env.LUME_LSP_ENV_PROBE_SECRET ?? null,
marker: process.env.LUME_LSP_ENV_PROBE_MARKER ?? null,
hasPath: typeof process.env.PATH === 'string' && process.env.PATH.length > 0,
} })
}
}
})
`)
const config = {
lsp: {
servers: {
test: {
command: process.execPath,
args: [script],
fileTypes: ['.ts'],
rootMarkers: ['package.json'],
env: { LUME_LSP_ENV_PROBE_MARKER: 'passed-through' },
},
},
},
}
const client = await getLspClient(root, config, source)
const env = await client.request<{ secret: string | null; marker: string | null; hasPath: boolean }>('test/env', {})
// Host-only variables must not leak into the project-configured server;
// the explicit per-server env is the opt-in passthrough.
expect(env.secret).toBeNull()
expect(env.marker).toBe('passed-through')
expect(env.hasPath).toBe(true)
} finally {
if (previousSecret === undefined) delete process.env[secretName]
else process.env[secretName] = previousSecret
await shutdownLspClients(root)
await rm(root, { recursive: true, force: true })
}
}, 20_000)

test('warms up a configured server and never rejects when none matches', async () => {
const root = await mkdtemp(join(tmpdir(), 'lume-lsp-warmup-'))
const empty = await mkdtemp(join(tmpdir(), 'lume-lsp-warmup-empty-'))
Expand Down
47 changes: 44 additions & 3 deletions packages/sdk/src/lsp/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
supportsLspFile,
type LspServerRole,
} from './registry.js'
import { wrapRustAnalyzerWithLspmux } from './lspmux.js'
import { invalidateLspmuxCache, wrapRustAnalyzerWithLspmux } from './lspmux.js'

export interface LspPosition {
line: number
Expand Down Expand Up @@ -121,6 +121,13 @@ const failedStarts = new Map<string, { until: number; error: Error }>()
const resolutionCache = new Map<string, { until: number; value: Promise<ResolvedLspServerConfig[]> }>()
let idleTimeoutMs: number | null = 10 * 60_000
let idleChecker: ReturnType<typeof setInterval> | undefined
// Upper bound for one stdin write; a wedged language-server pipe must fail
// the client instead of hanging Write/Edit forever (#327).
let writeTimeoutMs = 10_000

export function setLspWriteTimeout(timeoutMs: number): void {
writeTimeoutMs = Math.max(1, timeoutMs)
}

export type LspClientState = 'initializing' | 'ready' | 'failed' | 'restarting' | 'disposed'

Expand Down Expand Up @@ -403,6 +410,15 @@ function normalizeServerConfig(name: string, value: unknown, workspaceRoot: stri
priority: typeof record.priority === 'number' ? record.priority : 0,
role: record.role === 'linter' ? 'linter' : record.role === 'primary' ? 'primary' : undefined,
adapter: record.adapter === 'swiftlint' ? 'swiftlint' : undefined,
// Explicit per-server environment passthrough; merged over the minimal
// default spawn environment, never the host environment (#380).
...(record.env && typeof record.env === 'object' && !Array.isArray(record.env)
? {
env: Object.fromEntries(
Object.entries(record.env).filter((entry): entry is [string, string] => typeof entry[1] === 'string')
),
}
: {}),
...(typeof record.cwd === 'string' && record.cwd.trim() ? { cwd: resolve(workspaceRoot, record.cwd) } : {}),
}
}
Expand Down Expand Up @@ -432,7 +448,8 @@ async function resolveAvailableServer(
command: shim?.command ?? wrapped.command,
args: shim?.args ?? wrapped.args,
cwd: server.cwd ?? root,
...(wrapped.env ? { env: wrapped.env } : {}),
// Merge instead of replace so configured env survives mux wrapping (#380).
...(server.env || wrapped.env ? { env: { ...server.env, ...wrapped.env } } : {}),
lspmux: wrapped.lspmux,
}
}
Expand Down Expand Up @@ -682,6 +699,9 @@ export class LspClient {
await client.initialize()
} catch (error) {
killLspProcessTree(child)
// A failed mux'd start must not keep serving a stale "running" probe;
// invalidate so the next resolution falls back to a direct connection (#374).
if (server.lspmux) invalidateLspmuxCache(resolve(server.cwd ?? cwd))
const detail = error instanceof Error ? error.message : String(error)
throw new Error(`Unable to start LSP server "${server.command}": ${detail}`)
}
Expand Down Expand Up @@ -942,7 +962,28 @@ export class LspClient {
})
})
this.writeQueue = write.catch(() => undefined)
return write
// Every notification path funnels through here, so one timeout + fail()
// covers them all: a wedged pipe marks the client dead and routes every
// caller into the existing restart chain instead of hanging forever (#327).
let timer: ReturnType<typeof setTimeout> | undefined
return Promise.race([
write,
new Promise<never>((_, reject) => {
// No unref here: bun's test runner never fires unref'd timers, and
// this one is always released by the settle handlers below anyway.
timer = setTimeout(() => reject(new Error(`LSP server write timed out after ${writeTimeoutMs}ms`)), writeTimeoutMs)
}),
]).then(
(value) => {
clearTimeout(timer)
return value
},
(error) => {
clearTimeout(timer)
this.fail(error instanceof Error ? error : new Error(String(error)))
throw error
},
)
}

private onOutput(chunk: Buffer): void {
Expand Down
Loading
Loading