diff --git a/packages/devcontainer-worker/src/parts/CommandMap/CommandMap.ts b/packages/devcontainer-worker/src/parts/CommandMap/CommandMap.ts index 5d260ff..0fc6c08 100644 --- a/packages/devcontainer-worker/src/parts/CommandMap/CommandMap.ts +++ b/packages/devcontainer-worker/src/parts/CommandMap/CommandMap.ts @@ -1,6 +1,7 @@ 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' @@ -8,6 +9,7 @@ import * as HandleNodeMessagePort from '../HandleNodeMessagePort/HandleNodeMessa 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, diff --git a/packages/devcontainer-worker/src/parts/ContainerTerminal/ContainerTerminal.ts b/packages/devcontainer-worker/src/parts/ContainerTerminal/ContainerTerminal.ts index a776a91..115cba8 100644 --- a/packages/devcontainer-worker/src/parts/ContainerTerminal/ContainerTerminal.ts +++ b/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' }, } } diff --git a/packages/devcontainer-worker/src/parts/ContainerTerminalSession/ContainerTerminalSession.ts b/packages/devcontainer-worker/src/parts/ContainerTerminalSession/ContainerTerminalSession.ts new file mode 100644 index 0000000..42ca50e --- /dev/null +++ b/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() + const closing = new Map>() + 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 => { + try { + await execute(session, cleanupScript) + } finally { + closing.delete(token) + } + } + const dispose = (token: string): Promise => { + 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('; ') diff --git a/packages/devcontainer-worker/test/ContainerTerminal.test.ts b/packages/devcontainer-worker/test/ContainerTerminal.test.ts index 6ce0dae..0fda312 100644 --- a/packages/devcontainer-worker/test/ContainerTerminal.test.ts +++ b/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', ) }) diff --git a/packages/devcontainer-worker/test/ContainerTerminalSession.test.ts b/packages/devcontainer-worker/test/ContainerTerminalSession.test.ts new file mode 100644 index 0000000..d0a16a1 --- /dev/null +++ b/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() + 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 +}) diff --git a/packages/e2e/helpers/testContainerTerminal.ts b/packages/e2e/helpers/testContainerTerminal.ts index eda7995..3dbaf75 100644 --- a/packages/e2e/helpers/testContainerTerminal.ts +++ b/packages/e2e/helpers/testContainerTerminal.ts @@ -13,6 +13,7 @@ export const testContainerTerminal = async ( Workspace, }: Parameters[0], label = 'Dev Containers: Reopen in Container', + verifyDisposal = false, ): Promise => { 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() diff --git a/packages/e2e/src/devcontainer.terminal-disposal.ts b/packages/e2e/src/devcontainer.terminal-disposal.ts new file mode 100644 index 0000000..b8d41f2 --- /dev/null +++ b/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, + ) +} diff --git a/packages/extension/src/parts/DevContainerCommands/DevContainerCommands.ts b/packages/extension/src/parts/DevContainerCommands/DevContainerCommands.ts index dbe383e..318fc26 100644 --- a/packages/extension/src/parts/DevContainerCommands/DevContainerCommands.ts +++ b/packages/extension/src/parts/DevContainerCommands/DevContainerCommands.ts @@ -152,3 +152,7 @@ export const installDocker = async (): Promise => { export const getTerminalSpawnOptions = (workspaceUri: string, cwd = '') => { return Rpc.invoke('DevContainer.getTerminalSpawnOptions', workspaceUri, cwd) } + +export const disposeTerminal = (token: string) => { + return Rpc.invoke('DevContainer.disposeTerminal', token) +} diff --git a/packages/extension/src/parts/RegisterCommands/RegisterCommands.ts b/packages/extension/src/parts/RegisterCommands/RegisterCommands.ts index c28af7c..ae4c4c7 100644 --- a/packages/extension/src/parts/RegisterCommands/RegisterCommands.ts +++ b/packages/extension/src/parts/RegisterCommands/RegisterCommands.ts @@ -3,6 +3,10 @@ import * as BuildError from '../BuildError/BuildError.ts' import * as DevContainerCommands from '../DevContainerCommands/DevContainerCommands.ts' const commands = [ + { + execute: DevContainerCommands.disposeTerminal, + id: 'devcontainer.disposeTerminal', + }, { execute: DevContainerCommands.getTerminalSpawnOptions, id: 'devcontainer.getTerminalSpawnOptions',