fix(e2b): close remote lifecycle gaps

packages/lsp/lsp-local/src/index.ts

@@ -1,16 +1,19 @@
 /**
  * Generic stdio language-server backend for `ctx.lsp`. One plugin instance configures a named table
  * of server commands and registers one isolated provider for each entry. Every provider lazily
- * single-flights one server process per canonical workspace target, serves transient-open queries
+ * single-flights one server process per canonical workspace realpath, serves transient-open queries
  * through it, and replaces a selected transport that fails before or during the next read-only
- * query. Providers read sources through `ctx.fs` and launch servers through
- * `ctx.subprocess`, so both local and remote implementations share one host.
+ * query. Providers read sources through Node APIs in the host namespace (not `ctx.fs`)
+ * and trust their configured servers — no sandbox confinement.
  *
  * Namespace plugin (named exports, no default export). Lifecycle is effect-scoped: disposal
  * unregisters from `ctx.lsp` and tears down every live server.
  * @module @deepseek-ai/dsh-lsp-local
  */
 
+import { accessSync, constants, statSync } from 'node:fs'
+import { delimiter, isAbsolute, join } from 'node:path'
+import { pathToFileURL } from 'node:url'
 import type { Context } from 'cordis'
 import z from 'schemastery'
 import { LspError, LspProviderId } from '@deepseek-ai/dsh-lsp'
@@ -22,9 +25,9 @@ import type {
 import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout'
 import { abortable, abortError } from './abort.ts'
 import { canonicalizeWorkspace, readHostSource } from './host.ts'
-import type { HostWorkspace } from './host.ts'
 import { LspInstance } from './instance.ts'
 import type { ConnectionSpawner } from './connection.ts'
+import { scrubbedParentEnv } from '@deepseek-ai/dsh-subprocess'
 import type { InstanceSpec } from './instance.ts'
 
 export { canonicalizeWorkspace, readHostSource } from './host.ts'
@@ -44,7 +47,10 @@ export { LspConnection } from './connection.ts'
 export const name = 'lsp-local'
 
 /** Services required by this plugin. */
-export const inject = ['fs', 'lsp', 'subprocess']
+export const inject = ['lsp', 'subprocess']
+
+/** Credential-shaped ambient env vars are NOT forwarded to the child by default. */
+
 
 const DEFAULT_MAX_MESSAGE_BYTES = 16_000_000
 const DEFAULT_MAX_STDERR_BYTES = 1_000_000
@@ -86,7 +92,6 @@ export interface Config {
 
 /** One server config after schemastery fills every default. */
 type ResolvedServerConfig = Required<LspLocalServerConfig>
-type WorkspaceKey = HostWorkspace['target']['targetKey']
 
 const LspLocalServerConfig: z<LspLocalServerConfig> = z.object({
   command: z.string().required(),
@@ -106,67 +111,27 @@ export const Config: z<Config> = z.object({
   servers: z.dict(LspLocalServerConfig).required(),
 })
 
-/** Propagate teardown failures only after every sibling has settled. */
-function throwTeardownFailures(results: readonly PromiseSettledResult<void>[], message: string): void {
-  const failures: unknown[] = []
-  for (const result of results) {
-    if (result.status === 'rejected') failures.push(result.reason)
-  }
-  if (failures.length === 1) throw failures[0]
-  if (failures.length > 1) throw new AggregateError(failures, message)
-}
-
 /**
  * Register the configured stdio LSP providers. Resolves every executable at load (after credential
  * scrubbing) before publishing any provider; each process launches lazily on its first matching
  * query.
- * @param ctx - the plugin context carrying `fs`, `lsp`, and `subprocess`.
+ * @param ctx - the plugin context (must inject `lsp`).
  * @param config - the resolved plugin configuration (schemastery has filled every default).
  */
-export async function apply(ctx: Context, config: Config): Promise<void> {
+export function apply(ctx: Context, config: Config): void {
   const entries = Object.entries(config.servers)
   if (entries.length === 0) throw new Error('lsp-local: servers must contain at least one server')
 
-  const setupAbort = new AbortController()
-  const stopSetupCancellation = ctx.on('internal/plugin', (fiber) => {
-    // An async plugin callback must observe its own disposal before Cordis can
-    // run effect cleanup, because unload otherwise waits for this callback.
-    if (fiber === ctx.fiber && fiber.uid === null) {
-      setupAbort.abort(new Error('lsp-local setup disposed'))
-    }
-  })
-
   // Resolve every server-local setting before registration so a bad later command or bound cannot
   // publish an earlier provider. Registry-level mapping conflicts are rolled back below.
-  const providers = await (async () => {
-    const lookups = entries.map(async ([providerId, rawConfig]) => {
-      if (providerId.trim() === '') throw new Error('lsp-local: server ids must be non-empty strings')
-      const resolved = rawConfig as ResolvedServerConfig
-      validateServerConfig(providerId, resolved)
-      const executable = await ctx.subprocess.resolveExecutable(
-        resolved.command,
-        resolved.env,
-        setupAbort.signal,
-      )
-      setupAbort.signal.throwIfAborted()
-      return new LocalLspProvider(
-        providerId,
-        ctx.fs,
-        resolved,
-        executable,
-        spec => ctx.subprocess.spawn(spec),
-      )
-    })
-    try {
-      return await Promise.all(lookups)
-    } catch (error: unknown) {
-      setupAbort.abort(error)
-      await Promise.allSettled(lookups)
-      throw error
-    } finally {
-      stopSetupCancellation()
-    }
-  })()
+  const providers = entries.map(([providerId, rawConfig]) => {
+    if (providerId.trim() === '') throw new Error('lsp-local: server ids must be non-empty strings')
+    const resolved = rawConfig as ResolvedServerConfig
+    validateServerConfig(providerId, resolved)
+    const childEnv = buildChildEnv(resolved.env)
+    const executable = resolveExecutable(resolved.command, childEnv)
+    return new LocalLspProvider(providerId, resolved, childEnv, executable, spec => ctx.subprocess.spawn(spec))
+  })
 
   ctx.effect(() => {
     const disposers: Array<() => void> = []
@@ -179,8 +144,7 @@ export async function apply(ctx: Context, config: Config): Promise<void> {
     return async () => {
       // Remove every route before process teardown so no new query can enter a draining provider.
       for (const dispose of disposers.reverse()) dispose()
-      const results = await Promise.allSettled(providers.map(provider => provider.disposeAll()))
-      throwTeardownFailures(results, 'lsp-local provider teardown failed')
+      await Promise.all(providers.map(provider => provider.disposeAll()))
     }
   }, 'lsp-local.registerProviders')
 }
@@ -217,19 +181,16 @@ function assertPositiveInteger(providerId: string, name: string, value: number):
 class LocalLspProvider implements LspProvider {
   readonly id: LspProviderId
   readonly extensionToLanguage: Readonly<Record<string, string>>
-  /** One live instance per stable canonical workspace identity. */
-  private readonly instances = new Map<WorkspaceKey, LspInstance>()
+  /** One live instance per canonical workspace realpath. */
+  private readonly instances = new Map<string, LspInstance>()
   /** One complete source-read→open→query→close serialization tail per canonical workspace. */
-  private readonly queues = new Map<WorkspaceKey, Promise<void>>()
-  /** Workspace canonicalizations that have not entered a provider-owned queue yet. */
-  private readonly workspaceLookups = new Set<Promise<void>>()
-  private readonly lifetime = new AbortController()
+  private readonly queues = new Map<string, Promise<void>>()
   private disposed = false
 
   constructor(
     providerId: string,
-    private readonly fs: Context['fs'],
     private readonly config: ResolvedServerConfig,
+    private readonly childEnv: Record<string, string>,
     private readonly executable: string,
     private readonly spawner: ConnectionSpawner,
   ) {
@@ -250,60 +211,43 @@ class LocalLspProvider implements LspProvider {
     if (signal?.aborted) throw abortError(signal)
   }
 
-  /** Fuse caller cancellation with provider disposal for every filesystem and protocol await. */
-  private querySignal(signal?: AbortSignal): AbortSignal {
-    return signal === undefined
-      ? this.lifetime.signal
-      : AbortSignal.any([signal, this.lifetime.signal])
-  }
-
   async query(request: LspProviderQuery, signal?: AbortSignal): Promise<LspQueryResult> {
-    // Honor an already-aborted signal before provider I/O so a canceled request never starts a server.
+    // Honor an already-aborted signal before host I/O so a canceled request never starts a server.
     this.assertActive(signal)
-    const querySignal = this.querySignal(signal)
-    const workspaceResult = canonicalizeWorkspace(this.fs, request.workspaceRoot, querySignal)
-    const workspaceLookup = workspaceResult.then(() => undefined, () => undefined)
-    this.workspaceLookups.add(workspaceLookup)
-    let workspace: HostWorkspace
-    try {
-      workspace = await workspaceResult
-    } finally {
-      this.workspaceLookups.delete(workspaceLookup)
-    }
-    this.assertActive(querySignal)
-    const workspaceKey = workspace.target.targetKey
-    return this.enqueue(workspaceKey, querySignal, async () => {
-      this.assertActive(querySignal)
+    const workspace = await canonicalizeWorkspace(request.workspaceRoot, signal)
+    this.assertActive(signal)
+    return this.enqueue(workspace, signal, async () => {
+      this.assertActive(signal)
       // Read inside the workspace queue but before spawning: a queued query sees current bytes when
       // its turn starts, while an invalid source still cannot leave an idle process pooled.
-      const source = await readHostSource(this.fs, request.filePath, workspace, this.config.maxDocumentBytes, querySignal)
+      const source = await readHostSource(request.filePath, workspace, this.config.maxDocumentBytes, signal)
       // Disposal may have snapshotted the instance map while host I/O was pending. Re-check before a
       // synchronous get-or-create so every spawned process remains owned by teardown.
-      this.assertActive(querySignal)
-      let instance = this.instanceFor(workspaceKey, workspace)
+      this.assertActive(signal)
+      let instance = this.instanceFor(workspace)
       try {
-        return await instance.query(request, source, querySignal)
+        return await instance.query(request, source, signal)
       } catch (error) {
         // A selected child can have died while idle or fail during the next write. Queries are
         // read-only, so replace that transport once and retry transparently.
         if (!instance.isTransportFailure(error)) throw error
         await instance.dispose()
-        this.evictIfCurrent(workspaceKey, instance)
-        this.assertActive(querySignal)
-        instance = this.instanceFor(workspaceKey, workspace)
-        return await instance.query(request, source, querySignal)
+        this.evictIfCurrent(workspace, instance)
+        this.assertActive(signal)
+        instance = this.instanceFor(workspace)
+        return await instance.query(request, source, signal)
       } finally {
         // Reach quiescence before dropping a dead slot; a replacement must survive this ownership check.
         if (instance.dead) {
           await instance.dispose()
-          this.evictIfCurrent(workspaceKey, instance)
+          this.evictIfCurrent(workspace, instance)
         }
       }
     })
   }
 
   /** Serialize one complete query lifecycle for a canonical workspace. */
-  private enqueue<T>(workspace: WorkspaceKey, signal: AbortSignal | undefined, run: () => Promise<T>): Promise<T> {
+  private enqueue<T>(workspace: string, signal: AbortSignal | undefined, run: () => Promise<T>): Promise<T> {
     const previous = this.queues.get(workspace) ?? Promise.resolve()
     const result = abortable(previous, signal).then(run)
     // The tail follows the actual prior work even when this caller aborts its wait. It never rejects,
@@ -317,34 +261,34 @@ class LocalLspProvider implements LspProvider {
   }
 
   /** Return or synchronously publish the one instance for a canonical workspace. */
-  private instanceFor(workspaceKey: WorkspaceKey, workspace: HostWorkspace): LspInstance {
+  private instanceFor(workspace: string): LspInstance {
     this.assertActive()
-    const existing = this.instances.get(workspaceKey)
+    const existing = this.instances.get(workspace)
     if (existing !== undefined) return existing
     const created = this.createInstance(workspace)
-    this.instances.set(workspaceKey, created)
+    this.instances.set(workspace, created)
     return created
   }
 
   /** Drop the slot iff it still contains this instance. */
-  private evictIfCurrent(workspace: WorkspaceKey, instance: LspInstance): void {
+  private evictIfCurrent(workspace: string, instance: LspInstance): void {
     /* v8 ignore next -- mismatch requires another query to replace the slot before this finally runs. */
     if (this.instances.get(workspace) === instance) this.instances.delete(workspace)
   }
 
-  private createInstance(workspace: HostWorkspace): LspInstance {
+  private createInstance(workspace: string): LspInstance {
     const spec: InstanceSpec = {
       command: this.executable,
       args: this.config.args,
-      cwd: workspace.canonicalPath,
-      workspaceUri: workspace.fileUrl,
-      env: this.config.env,
+      cwd: workspace,
+      env: this.childEnv,
       configuration: this.config.configuration,
       initializationOptions: this.config.initializationOptions,
       maxMessageBytes: this.config.maxMessageBytes,
       maxStderrBytes: this.config.maxStderrBytes,
       shutdownTimeoutMs: this.config.shutdownTimeoutMs,
       killGraceMs: this.config.killGraceMs,
+      pathToFileUri: path => pathToFileURL(path).href,
     }
     return new LspInstance(spec, this.spawner)
   }
@@ -352,18 +296,51 @@ class LocalLspProvider implements LspProvider {
   /** Dispose every live instance and block further queries. */
   async disposeAll(): Promise<void> {
     this.disposed = true
-    this.lifetime.abort(new LspError('lsp-local provider is disposed', 'LSP_DISPOSED'))
     const live = [...this.instances.values()]
     const draining = [...this.queues.values()]
-    const resolving = [...this.workspaceLookups]
     this.instances.clear()
-    const results = await Promise.allSettled([
+    await Promise.all([
       ...live.map(instance => instance.dispose()),
       ...draining,
-      ...resolving,
     ])
     this.queues.clear()
-    this.workspaceLookups.clear()
-    throwTeardownFailures(results, 'lsp-local instance teardown failed')
+  }
+}
+
+/** The seam's scrubbed parent env (credential-shaped and DSH_* names dropped), plus the config's explicit env. */
+function buildChildEnv(extra: Record<string, string>): Record<string, string> {
+  return { ...scrubbedParentEnv(), ...extra }
+}
+
+/**
+ * Resolve the server executable to an absolute path: an absolute command is verified directly; a
+ * bare command is looked up on the child's PATH. Fails loudly when nothing is executable.
+ */
+function resolveExecutable(command: string, childEnv: Record<string, string>): string {
+  if (isAbsolute(command)) {
+    // Verify an absolute command too, so an unavailable one fails at load, not on the first query.
+    if (!isExecutableFileSync(command)) {
+      throw new Error(`lsp-local: command "${command}" is not an executable file`)
+    }
+    return command
+  }
+  /* v8 ignore next -- buildChildEnv always sets PATH from the ambient env; the further fallbacks are defensive. */
+  const pathValue = childEnv.PATH ?? process.env.PATH ?? ''
+  for (const dir of pathValue.split(delimiter)) {
+    if (dir === '') continue
+    const candidate = join(dir, command)
+    if (isExecutableFileSync(candidate)) return candidate
+  }
+  throw new Error(`lsp-local: command "${command}" was not found on PATH`)
+}
+
+/** Synchronous regular-file and executable check used only at load-time resolution. */
+function isExecutableFileSync(path: string): boolean {
+  try {
+    if (!statSync(path).isFile()) return false
+    accessSync(path, constants.X_OK)
+    return true
+  } catch {
+    return false
   }
 }

packages/lsp/lsp-local/src/instance.ts

@@ -7,7 +7,6 @@
  * @module @deepseek-ai/dsh-lsp-local/instance
  */
 
-import { pathToFileURL } from 'node:url'
 import { LspError } from '@deepseek-ai/dsh-lsp'
 import type {
   LspOperation,
@@ -37,6 +36,12 @@ export interface InstanceSpec extends ConnectionSpec {
   readonly shutdownTimeoutMs: number
   /** PID advertised to the server; `null` when client and server do not share a process namespace. */
   readonly clientProcessId?: number | null
+  /**
+   * Encode one implementation-native absolute path as a file URI.
+   * @param path - Canonical workspace or source path.
+   * @returns A file URI interpreted in the server's filesystem namespace.
+   */
+  readonly pathToFileUri: (path: string) => string
 }
 
 /**
@@ -111,8 +116,8 @@ export class LspInstance {
   private async initialize(): Promise<void> {
     const initializeResult = await this.connection.request('initialize', {
       processId: this.spec.clientProcessId === undefined ? process.pid : this.spec.clientProcessId,
-      rootUri: pathToFileURL(this.spec.cwd).href,
-      workspaceFolders: [{ uri: pathToFileURL(this.spec.cwd).href, name: 'workspace' }],
+      rootUri: this.spec.pathToFileUri(this.spec.cwd),
+      workspaceFolders: [{ uri: this.spec.pathToFileUri(this.spec.cwd), name: 'workspace' }],
       capabilities: CLIENT_CAPABILITIES,
       initializationOptions: this.spec.initializationOptions,
     }) as WireInitializeResult
@@ -149,7 +154,7 @@ export class LspInstance {
       throw new LspError('server does not support the transient textDocument/didOpen this host requires', 'LSP_UNSUPPORTED_OPERATION')
     }
 
-    const uri = pathToFileURL(source.canonicalPath).href
+    const uri = this.spec.pathToFileUri(source.canonicalPath)
     let opened = false
     try {
       /* v8 ignore next -- guards an abort landing between the ready wait and didOpen; not deterministically reproducible. */

packages/lsp/lsp-local/tests/instance.spec.ts

@@ -56,6 +56,7 @@ function makeInstance(
     maxStderrBytes: 100_000,
     shutdownTimeoutMs: 200,
     killGraceMs: 200,
+    pathToFileUri: path => pathToFileURL(path).href,
     ...overrides,
   }, spawnSubprocess, writer)
   live.push(instance)
@@ -91,6 +92,7 @@ function scriptInstance(script: string, overrides: Partial<InstanceSpec> = {}):
     maxStderrBytes: 100_000,
     shutdownTimeoutMs: 150,
     killGraceMs: 150,
+    pathToFileUri: path => pathToFileURL(path).href,
     ...overrides,
   }, spawnSubprocess)
   live.push(instance)