fix: stop container shells when terminals close

packages/devcontainer-worker/src/parts/CommandMap/CommandMap.ts

@@ -1,13 +1,15 @@
 import { setDockerPath } from '@lvce-editor/devcontainer-node/devcontainer-cli'
 import * as ContainerFileSystem from '../ContainerFileSystem/ContainerFileSystem.ts'
 import * as ContainerTerminal from '../ContainerTerminal/ContainerTerminal.ts'
+import * as ContainerTerminalSession from '../ContainerTerminalSession/ContainerTerminalSession.ts'
 import * as DevContainer from '../DevContainer/DevContainer.ts'
 import * as DevContainerCommandType from '../DevContainerCommandType/DevContainerCommandType.ts'
 import * as HandleElectronMessagePort from '../HandleElectronMessagePort/HandleElectronMessagePort.ts'
 import * as HandleNodeMessagePort from '../HandleNodeMessagePort/HandleNodeMessagePort.ts'
 import * as HandleWebSocket from '../HandleWebSocket/HandleWebSocket.ts'
 
 export const commandMap = {
+  'DevContainer.disposeTerminal': ContainerTerminalSession.dispose,
   'DevContainer.getTerminalSpawnOptions': ContainerTerminal.getSpawnOptions,
   [DevContainerCommandType.DevContainerDetect]: DevContainer.detect,
   [DevContainerCommandType.DevContainerExec]: DevContainer.exec,

packages/devcontainer-worker/src/parts/ContainerTerminal/ContainerTerminal.ts

@@ -1,9 +1,21 @@
 import { getDevcontainerCliPath } from '@lvce-editor/devcontainer-node/devcontainer-cli'
 import { posix } from 'node:path'
+import * as ContainerTerminalSession from '../ContainerTerminalSession/ContainerTerminalSession.ts'
 import * as ContainerUri from '../ContainerUri/ContainerUri.ts'
 import * as DevContainerState from '../DevContainerState/DevContainerState.ts'
 
-export const getSpawnOptions = async (workspaceUri: string, cwd = '') => {
+export const getSpawnOptions = async (
+  workspaceUri: string,
+  cwd = '',
+  createSession: (
+    id: string,
+    cli: string,
+    user?: string,
+  ) => Promise<{
+    directory: string
+    token: string
+  }> = ContainerTerminalSession.create,
+) => {
   const workspace = ContainerUri.parse(workspaceUri)
   const location = ContainerUri.parse(cwd || workspaceUri)
   if (location.id !== workspace.id) {
@@ -19,6 +31,28 @@ export const getSpawnOptions = async (workspaceUri: string, cwd = '') => {
   ) {
     throw new Error('Devcontainer is not running')
   }
+  let session: { directory: string; token: string }
+  try {
+    session = await createSession(
+      state.containerId,
+      state.containerCli || 'docker',
+      state.remoteUser,
+    )
+  } catch (error) {
+    // Keep preparation failures in the terminal's normal startup-error UI.
+    // The message is an argument to fixed code, never executable source.
+    return {
+      args: [
+        '--eval',
+        'console.error(process.argv[1]); process.exitCode = 1',
+        error instanceof Error ? error.message : String(error),
+      ],
+      command: process.execPath,
+      cwd: workspaceFolder,
+      disposeCommand: undefined,
+      env: { ELECTRON_RUN_AS_NODE: '1' },
+    }
+  }
   // The host PTY runs the CLI, which applies remoteUser, remoteEnv and the
   // user's environment probe. The shell and its working directory are remote.
   return {
@@ -31,14 +65,21 @@ export const getSpawnOptions = async (workspaceUri: string, cwd = '') => {
       state.containerCli || 'docker',
       '--container-id',
       state.containerId,
+      'env',
+      `LVCE_TERMINAL_SESSION=${session.token}`,
       'sh',
       '-c',
-      'cd -- "$1" && exec "${SHELL:-/bin/sh}" -il',
+      ContainerTerminalSession.shellScript,
       'devcontainer-terminal',
       posix.join(state.remoteWorkspaceFolder, location.path),
+      session.directory,
     ],
     command: process.execPath,
     cwd: workspaceFolder,
+    disposeCommand: {
+      args: [session.token],
+      command: 'devcontainer.disposeTerminal',
+    },
     env: { ELECTRON_RUN_AS_NODE: '1' },
   }
 }

packages/devcontainer-worker/src/parts/ContainerTerminalSession/ContainerTerminalSession.ts

@@ -0,0 +1,101 @@
+import { execFile } from 'node:child_process'
+import { randomUUID } from 'node:crypto'
+import { promisify } from 'node:util'
+
+interface Session {
+  containerCli: string
+  containerId: string
+  directory: string
+  remoteUser?: string
+  token: string
+}
+
+const execFileAsync = promisify(execFile)
+
+const run = async (session: Session, script: string) => {
+  const user = session.remoteUser ? ['--user', session.remoteUser] : []
+  await execFileAsync(
+    session.containerCli,
+    [
+      'exec',
+      ...user,
+      session.containerId,
+      'sh',
+      '-c',
+      script,
+      'lvce-terminal-session',
+      session.directory,
+      session.token,
+    ],
+    { timeout: 10_000 },
+  )
+}
+
+export const createSessionManager = (execute = run) => {
+  const sessions = new Map<string, Session>()
+  const closing = new Map<string, Promise<void>>()
+  const create = async (
+    containerId: string,
+    containerCli: string,
+    remoteUser?: string,
+  ) => {
+    const token = randomUUID()
+    const session: Session = {
+      containerCli,
+      containerId,
+      directory: `/tmp/lvce-terminal-${token}`,
+      remoteUser,
+      token,
+    }
+    await execute(session, 'umask 077; mkdir -- "$1"')
+    sessions.set(token, session)
+    return session
+  }
+
+  const finishDispose = async (
+    session: Session,
+    token: string,
+  ): Promise<void> => {
+    try {
+      await execute(session, cleanupScript)
+    } finally {
+      closing.delete(token)
+    }
+  }
+  const dispose = (token: string): Promise<void> => {
+    const pending = closing.get(token)
+    if (pending) return pending
+    const session = sessions.get(token)
+    if (!session) return Promise.resolve()
+    sessions.delete(token)
+    const task = finishDispose(session, token)
+    closing.set(token, task)
+    return task
+  }
+  return { create, dispose }
+}
+
+// Rename closes the startup gate before inspecting the atomically published
+// owner. A late CLI launch cannot publish its PID into the removed directory.
+// The nonce in the process environment also prevents killing a reused PID.
+export const cleanupScript = [
+  'set -e',
+  'mv -- "$1" "$1.closed" 2>/dev/null || exit 0',
+  'pid=',
+  'if [ -f "$1.closed/pid" ]; then read -r pid < "$1.closed/pid"; fi',
+  'case "$pid" in ""|*[!0-9]*) pid=0;; esac',
+  'if [ "$pid" -gt 1 ] && [ -r "/proc/$pid/environ" ] && tr "\\000" "\\n" < "/proc/$pid/environ" | grep -Fxq "LVCE_TERMINAL_SESSION=$2"; then kill -HUP -"$pid" 2>/dev/null || true; fi',
+  'rm -r -- "$1.closed"',
+].join('; ')
+
+const manager = createSessionManager()
+export const { create } = manager
+export const { dispose } = manager
+
+export const shellScript = [
+  'set -e',
+  'cd -- "$1"',
+  'printf "%s\\n" "$$" > "$2/pid.tmp"',
+  'mv -- "$2/pid.tmp" "$2/pid"',
+  'exec "${SHELL:-/bin/sh}" -il',
+].join('; ')

packages/devcontainer-worker/test/ContainerTerminal.test.ts

@@ -1,7 +1,15 @@
-import { afterEach, expect, test } from '@jest/globals'
+import { afterEach, expect, jest, test } from '@jest/globals'
 import * as ContainerTerminal from '../src/parts/ContainerTerminal/ContainerTerminal.ts'
+import * as ContainerTerminalSession from '../src/parts/ContainerTerminalSession/ContainerTerminalSession.ts'
 import * as DevContainerState from '../src/parts/DevContainerState/DevContainerState.ts'
 
+const createSession = jest.fn(async () => ({
+  directory: '/tmp/lvce-terminal-test-token',
+  token: 'test-token',
+}))
+const getOptions = (uri: string, cwd = '') =>
+  ContainerTerminal.getSpawnOptions(uri, cwd, createSession)
+
 const workspaceFolder = '/host/project with spaces'
 const workspaceUri = 'devcontainers:///abc123'
 const connect = () =>
@@ -17,10 +25,14 @@ afterEach(() => DevContainerState.reset())
 
 test('launches a PTY through devcontainer exec with the configured environment and container id', async () => {
   connect()
-  const options = await ContainerTerminal.getSpawnOptions(workspaceUri)
+  const options = await getOptions(workspaceUri)
   expect(options.command).toBe(process.execPath)
   expect(options.env).toEqual({ ELECTRON_RUN_AS_NODE: '1' })
   expect(options.cwd).toBe(workspaceFolder)
+  expect(options.disposeCommand).toEqual({
+    args: ['test-token'],
+    command: 'devcontainer.disposeTerminal',
+  })
   expect(options.args).toEqual([
     expect.stringContaining('devcontainer.js'),
     'exec',
@@ -30,38 +42,50 @@ test('launches a PTY through devcontainer exec with the configured environment a
     '/custom/podman',
     '--container-id',
     'abc123',
+    'env',
+    'LVCE_TERMINAL_SESSION=test-token',
     'sh',
     '-c',
-    'cd -- "$1" && exec "${SHELL:-/bin/sh}" -il',
+    ContainerTerminalSession.shellScript,
     'devcontainer-terminal',
     '/workspaces/project',
+    '/tmp/lvce-terminal-test-token',
   ])
 })
 
 test('opens an Explorer directory and passes special characters as a positional argument', async () => {
   connect()
-  const options = await ContainerTerminal.getSpawnOptions(
+  const options = await getOptions(
     workspaceUri,
     `${workspaceUri}/src/a%20b%3B%24x`,
   )
-  expect(options.args.at(-1)).toBe('/workspaces/project/src/a b;$x')
+  expect(options.args.at(-2)).toBe('/workspaces/project/src/a b;$x')
+})
+
+test('preparation failures use the normal terminal error path without evaluating the message', async () => {
+  connect()
+  const message = 'Docker is unavailable; $(unexpected) "quoted"'
+  createSession.mockRejectedValueOnce(new Error(message))
+  const options = await getOptions(workspaceUri)
+  expect(options.command).toBe(process.execPath)
+  expect(options.env).toEqual({ ELECTRON_RUN_AS_NODE: '1' })
+  expect(options.args.at(-1)).toBe(message)
+  expect(options.args[1]).not.toContain(message)
+  expect(options.disposeCommand).toBeUndefined()
 })
 
 test('refuses a terminal in another container', async () => {
   connect()
   await expect(
-    ContainerTerminal.getSpawnOptions(
-      workspaceUri,
-      'devcontainers:///def456/src',
-    ),
+    getOptions(workspaceUri, 'devcontainers:///def456/src'),
   ).rejects.toThrow('different workspace')
 })
 
 test('refuses host directories in a container workspace', async () => {
   connect()
-  await expect(
-    ContainerTerminal.getSpawnOptions(workspaceUri, 'file:///host'),
-  ).rejects.toThrow('Invalid devcontainer URI')
+  await expect(getOptions(workspaceUri, 'file:///host')).rejects.toThrow(
+    'Invalid devcontainer URI',
+  )
 })
 
 test('does not launch a host shell for a stopped container', async () => {
@@ -70,7 +94,7 @@ test('does not launch a host shell for a stopped container', async () => {
     ...DevContainerState.get(workspaceFolder)!,
     status: 'stopped',
   })
-  await expect(ContainerTerminal.getSpawnOptions(workspaceUri)).rejects.toThrow(
+  await expect(getOptions(workspaceUri)).rejects.toThrow(
     'Devcontainer is not running',
   )
 })

packages/devcontainer-worker/test/ContainerTerminalSession.test.ts

@@ -0,0 +1,40 @@
+import { expect, jest, test } from '@jest/globals'
+import * as Session from '../src/parts/ContainerTerminalSession/ContainerTerminalSession.ts'
+
+test('disposal retains its owned container and user and is idempotent', async () => {
+  const execute = jest.fn(async (_session: unknown, _script: string) => {})
+  const manager = Session.createSessionManager(execute)
+  const first = await manager.create(
+    'first-container',
+    '/custom/podman',
+    'node',
+  )
+  const other = await manager.create('other-container', 'docker')
+  execute.mockClear()
+  await manager.dispose(first.token)
+  expect(execute).toHaveBeenCalledWith(first, Session.cleanupScript)
+  expect(first).toMatchObject({
+    containerCli: '/custom/podman',
+    containerId: 'first-container',
+    remoteUser: 'node',
+  })
+  await manager.dispose(first.token)
+  expect(execute).toHaveBeenCalledTimes(1)
+  await manager.dispose(other.token)
+  expect(execute).toHaveBeenCalledTimes(2)
+})
+
+test('concurrent disposal waits for the same cleanup operation', async () => {
+  const execute = jest.fn(async (_session: unknown, _script: string) => {})
+  const manager = Session.createSessionManager(execute)
+  const session = await manager.create('container', 'docker')
+  execute.mockClear()
+  const gate = Promise.withResolvers<void>()
+  execute.mockImplementationOnce(() => gate.promise)
+  const first = manager.dispose(session.token)
+  const second = manager.dispose(session.token)
+  expect(second).toBe(first)
+  expect(execute).toHaveBeenCalledTimes(1)
+  gate.resolve()
+  await first
+})

packages/e2e/helpers/testContainerTerminal.ts

@@ -13,6 +13,7 @@ export const testContainerTerminal = async (
     Workspace,
   }: Parameters<Test>[0],
   label = 'Dev Containers: Reopen in Container',
+  verifyDisposal = false,
 ): Promise<void> => {
   const localUri = await getWorkspaceUri({ Command }, 'reopen')
   await Workspace.setPath(localUri)
@@ -77,6 +78,22 @@ export const testContainerTerminal = async (
       ['/container-workspace/sub folder/marker.txt'],
       'subfolder-created',
     )
+    if (verifyDisposal) {
+      await Command.execute(
+        'Terminals.sendText',
+        'echo $$ > terminal-shell-pid; printf "dispose-%s\\n" ready\r',
+      )
+      await expect(terminal).toContainText('dispose-ready')
+      await Command.execute('Terminals.killTerminal')
+      await Devcontainer.shouldHaveExecOutput(
+        'sh',
+        [
+          '-c',
+          'pid=$(cat "/container-workspace/sub folder/terminal-shell-pid"); state=$(awk \'{print $3}\' "/proc/$pid/stat" 2>/dev/null || true); if [ -z "$state" ] || [ "$state" = Z ]; then printf disposed; else printf alive; fi',
+        ],
+        'disposed',
+      )
+    }
   } finally {
     await Workspace.setPath(localUri)
     await Devcontainer.remove()

packages/e2e/src/devcontainer.terminal-disposal.ts

@@ -0,0 +1,13 @@
+import type { Test } from '@lvce-editor/test-worker'
+import { testContainerTerminal } from '../helpers/testContainerTerminal.ts'
+
+// Requires the renderer's disposeCommand support in the published server.
+export const skip = 1
+
+export const test: Test = async (context) => {
+  await testContainerTerminal(
+    context,
+    'Dev Containers: Reopen in Container',
+    true,
+  )
+}

packages/extension/src/parts/DevContainerCommands/DevContainerCommands.ts

@@ -152,3 +152,7 @@ export const installDocker = async (): Promise<void> => {
 export const getTerminalSpawnOptions = (workspaceUri: string, cwd = '') => {
   return Rpc.invoke('DevContainer.getTerminalSpawnOptions', workspaceUri, cwd)
 }
+
+export const disposeTerminal = (token: string) => {
+  return Rpc.invoke('DevContainer.disposeTerminal', token)
+}