From 366492aa33762b91e59da91943b0fca9ea9b5f4a Mon Sep 17 00:00:00 2001 From: Adam Creeger Date: Thu, 5 Feb 2026 00:03:02 -0500 Subject: [PATCH] feat(dev-server): add Docker-based dev server support for framework-agnostic port mapping Add opt-in Docker mode for dev servers where the app runs on its default port inside a container and Docker maps it to the workspace-specific port on the host. This solves the problem of frameworks like Angular CLI that don't respect the PORT environment variable. New DockerManager utility class handles Docker CLI operations (build, run, stop, inspect, port detection). DevServerManager gains Docker execution paths alongside existing process-based paths. ResourceCleanup handles Docker container cleanup. Configuration via capabilities.web.devServer setting in .iloom/settings.json. Closes #548 --- docs/iloom-commands.md | 73 +++ src/commands/dev-server.test.ts | 236 ++++++++- src/commands/dev-server.ts | 20 +- src/commands/open.test.ts | 9 +- src/commands/open.ts | 17 +- src/commands/run.test.ts | 3 +- src/commands/run.ts | 17 +- src/lib/DevServerManager.test.ts | 395 ++++++++++++++- src/lib/DevServerManager.ts | 156 +++++- src/lib/DockerManager.test.ts | 796 +++++++++++++++++++++++++++++++ src/lib/DockerManager.ts | 491 +++++++++++++++++++ src/lib/ResourceCleanup.test.ts | 211 ++++++++ src/lib/ResourceCleanup.ts | 40 +- src/lib/SettingsManager.ts | 44 ++ 14 files changed, 2477 insertions(+), 31 deletions(-) create mode 100644 src/lib/DockerManager.test.ts create mode 100644 src/lib/DockerManager.ts diff --git a/docs/iloom-commands.md b/docs/iloom-commands.md index d2d7ae1d..4fcd2fb7 100644 --- a/docs/iloom-commands.md +++ b/docs/iloom-commands.md @@ -654,6 +654,79 @@ il dev-server feat/my-branch - Use Ctrl+C to stop the server - Respects `sourceEnvOnStart` setting for environment loading +#### Docker Dev Server Mode + +By default, `il dev-server` runs your dev server as a native process. For frameworks that ignore the `PORT` environment variable (e.g., Angular CLI), you can use Docker mode to remap ports via Docker's `-p` flag. + +**Requirements:** +- Docker must be installed and the Docker daemon must be running +- A `Dockerfile` in your project (or a custom path configured) + +**Configuration:** + +Set `capabilities.web.devServer` to `"docker"` in your `.iloom/settings.json`: + +```json +{ + "capabilities": { + "web": { + "devServer": "docker", + "dockerFile": "./Dockerfile", + "containerPort": 4200, + "dockerBuildArgs": { + "NODE_ENV": "development" + }, + "dockerRunArgs": ["-v", "./src:/app/src"] + } + } +} +``` + +**Configuration Fields:** + +| Field | Type | Default | Description | +|-------|------|---------|-------------| +| `devServer` | `"process"` \| `"docker"` | `"process"` | Dev server execution mode. `"process"` runs natively, `"docker"` runs inside a Docker container with port mapping. | +| `dockerFile` | `string` | `"./Dockerfile"` | Path to the Dockerfile relative to the worktree root. Only used when `devServer` is `"docker"`. | +| `containerPort` | `number` | Auto-detected | Port the application listens on inside the Docker container. If not set, iloom attempts to detect it from `EXPOSE` directives in the built Docker image (via `docker image inspect`), falling back to Dockerfile parsing. | +| `dockerBuildArgs` | `Record` | - | Build arguments passed to `docker build` (e.g., `{"NODE_ENV": "development"}`). | +| `dockerRunArgs` | `string[]` | - | Additional flags passed to `docker run`. Use this for volume mounts, environment variables, user mapping, and other Docker options. | + +**How Port Mapping Works:** + +Each loom workspace gets a unique port calculated as `basePort + issue/PR number` (e.g., issue #25 gets port 3025). In Docker mode: + +1. The Docker image is built from your Dockerfile +2. The container runs with `-p :` +3. Your app runs on `containerPort` inside the container (e.g., 4200 for Angular) +4. Docker maps that to the workspace port on the host (e.g., 3025) +5. You access the app at `http://localhost:3025` as usual + +This means frameworks that hardcode their listen port work correctly -- Docker handles the port translation transparently. + +**Container Naming:** + +Containers are named `iloom-dev-` where the identifier is derived from the issue/PR number or branch name. Special characters (slashes, etc.) are replaced with hyphens. + +**Known Limitations:** + +- **macOS volume mount performance:** Docker Desktop on macOS has known slow file-watching performance through bind mounts. This can affect hot reload when using volume mounts via `dockerRunArgs`. Consider using tools like `mutagen` or Docker's `synchronized` file sharing if performance is an issue. +- **Linux file permissions:** Docker containers typically run as root, so files created by the container via volume mounts may be owned by root on the host. Mitigate by passing `--user` via `dockerRunArgs`: + ```json + { + "dockerRunArgs": ["--user", "1000:1000"] + } + ``` +- **Container orphaning on crash:** If the `il dev-server` process is killed ungracefully (e.g., SIGKILL), the Docker container may keep running. Use `docker stop` to clean up orphaned containers: + ```bash + # List any orphaned iloom containers + docker ps --filter "name=iloom-dev-" + + # Stop all orphaned iloom containers + docker stop $(docker ps -q --filter "name=iloom-dev-") + ``` + Normal cleanup via `il cleanup`, `il finish`, or Ctrl+C handles container shutdown automatically. + --- ### il build diff --git a/src/commands/dev-server.test.ts b/src/commands/dev-server.test.ts index a565181e..16d24073 100644 --- a/src/commands/dev-server.test.ts +++ b/src/commands/dev-server.test.ts @@ -3,6 +3,7 @@ import { DevServerCommand } from './dev-server.js' import { GitWorktreeManager } from '../lib/GitWorktreeManager.js' import { ProjectCapabilityDetector } from '../lib/ProjectCapabilityDetector.js' import { DevServerManager } from '../lib/DevServerManager.js' +import { DockerManager } from '../lib/DockerManager.js' import { SettingsManager } from '../lib/SettingsManager.js' import { IdentifierParser } from '../utils/IdentifierParser.js' import { loadWorkspaceEnv, isNoEnvFilesFoundError } from '../utils/env.js' @@ -14,6 +15,7 @@ import fs from 'fs-extra' vi.mock('../lib/GitWorktreeManager.js') vi.mock('../lib/ProjectCapabilityDetector.js') vi.mock('../lib/DevServerManager.js') +vi.mock('../lib/DockerManager.js') vi.mock('../utils/IdentifierParser.js') vi.mock('fs-extra') vi.mock('../lib/SettingsManager.js') @@ -209,7 +211,8 @@ describe('DevServerCommand', () => { 3087, false, expect.any(Function), - expect.any(Object) + expect.any(Object), + undefined ) }) @@ -293,7 +296,8 @@ describe('DevServerCommand', () => { 4500, false, expect.any(Function), - expect.any(Object) + expect.any(Object), + undefined ) }) @@ -315,7 +319,8 @@ describe('DevServerCommand', () => { 3087, false, expect.any(Function), - expect.any(Object) + expect.any(Object), + undefined ) }) }) @@ -402,7 +407,8 @@ describe('DevServerCommand', () => { 3087, false, expect.any(Function), - expect.any(Object) + expect.any(Object), + undefined ) }) @@ -417,7 +423,8 @@ describe('DevServerCommand', () => { 3087, true, expect.any(Function), - expect.any(Object) + expect.any(Object), + undefined ) }) }) @@ -458,7 +465,8 @@ describe('DevServerCommand', () => { 3087, false, expect.any(Function), - { DATABASE_URL: 'postgres://test', API_KEY: 'secret' } + { DATABASE_URL: 'postgres://test', API_KEY: 'secret' }, + undefined ) }) @@ -475,7 +483,8 @@ describe('DevServerCommand', () => { 3087, false, expect.any(Function), - {} + {}, + undefined ) }) @@ -490,7 +499,8 @@ describe('DevServerCommand', () => { 3087, false, expect.any(Function), - {} + {}, + undefined ) }) @@ -513,7 +523,8 @@ describe('DevServerCommand', () => { 3087, false, expect.any(Function), - {} + {}, + undefined ) }) @@ -533,4 +544,211 @@ describe('DevServerCommand', () => { expect(mockDevServerManager.runServerForeground).toHaveBeenCalled() }) }) + + describe('Docker mode', () => { + beforeEach(() => { + vi.mocked(mockIdentifierParser.parseForPatternDetection).mockResolvedValue({ + type: 'issue', + number: 87, + originalInput: '87', + }) + + vi.mocked(mockGitWorktreeManager.findWorktreeForIssue).mockResolvedValue(mockWorktree) + + const mockCapabilities: ProjectCapabilities = { + capabilities: ['web'], + binEntries: {}, + } + vi.mocked(mockCapabilityDetector.detectCapabilities).mockResolvedValue(mockCapabilities) + + vi.mocked(fs.pathExists).mockResolvedValue(true) + vi.mocked(fs.readFile).mockResolvedValue('PORT=3087\n') + + // Docker is available by default in Docker mode tests + vi.mocked(DockerManager.assertAvailable).mockResolvedValue(undefined) + }) + + it('should construct DockerConfig and pass to isServerRunning and runServerForeground when devServer is "docker"', async () => { + const expectedDockerConfig = { + dockerFile: './Dockerfile.dev', + containerPort: 4200, + dockerBuildArgs: { NODE_ENV: 'development' }, + dockerRunArgs: ['-v', './src:/app/src'], + identifier: '87', + } + + vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue({ + capabilities: { + web: { + devServer: 'docker', + dockerFile: './Dockerfile.dev', + containerPort: 4200, + dockerBuildArgs: { NODE_ENV: 'development' }, + dockerRunArgs: ['-v', './src:/app/src'], + }, + }, + }) + + vi.mocked(DockerManager.buildDockerConfigFromSettings).mockReturnValue(expectedDockerConfig) + + await command.execute({ identifier: '87' }) + + expect(DockerManager.assertAvailable).toHaveBeenCalled() + expect(mockDevServerManager.isServerRunning).toHaveBeenCalledWith( + 3087, + expectedDockerConfig + ) + expect(mockDevServerManager.runServerForeground).toHaveBeenCalledWith( + mockWorktree.path, + 3087, + false, + expect.any(Function), + {}, + expectedDockerConfig + ) + }) + + it('should use default dockerFile when not specified in settings', async () => { + const expectedDockerConfig = { + dockerFile: './Dockerfile', + containerPort: undefined, + dockerBuildArgs: undefined, + dockerRunArgs: undefined, + identifier: '87', + } + + vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue({ + capabilities: { + web: { + devServer: 'docker', + }, + }, + }) + + vi.mocked(DockerManager.buildDockerConfigFromSettings).mockReturnValue(expectedDockerConfig) + + await command.execute({ identifier: '87' }) + + expect(mockDevServerManager.runServerForeground).toHaveBeenCalledWith( + mockWorktree.path, + 3087, + false, + expect.any(Function), + {}, + expect.objectContaining({ + dockerFile: './Dockerfile', + identifier: '87', + }) + ) + }) + + it('should use branchName as identifier when number is not available', async () => { + vi.mocked(mockIdentifierParser.parseForPatternDetection).mockResolvedValue({ + type: 'branch', + branchName: 'feat/docker-support', + originalInput: 'feat/docker-support', + }) + + vi.mocked(mockGitWorktreeManager.findWorktreeForBranch).mockResolvedValue(mockWorktree) + + vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue({ + capabilities: { + web: { + devServer: 'docker', + }, + }, + }) + + vi.mocked(DockerManager.buildDockerConfigFromSettings).mockReturnValue({ + dockerFile: './Dockerfile', + containerPort: undefined, + dockerBuildArgs: undefined, + dockerRunArgs: undefined, + identifier: 'feat/docker-support', + }) + + await command.execute({ identifier: 'feat/docker-support' }) + + expect(mockDevServerManager.runServerForeground).toHaveBeenCalledWith( + mockWorktree.path, + 3087, + false, + expect.any(Function), + {}, + expect.objectContaining({ + identifier: 'feat/docker-support', + }) + ) + }) + + it('should throw when Docker is not available in Docker mode', async () => { + vi.mocked(DockerManager.assertAvailable).mockRejectedValue( + new Error( + 'Docker is not available. Please ensure Docker is installed and the Docker daemon is running.' + ) + ) + + vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue({ + capabilities: { + web: { + devServer: 'docker', + }, + }, + }) + + // Return a config so that the code path reaches assertAvailable + vi.mocked(DockerManager.buildDockerConfigFromSettings).mockReturnValue({ + dockerFile: './Dockerfile', + containerPort: undefined, + dockerBuildArgs: undefined, + dockerRunArgs: undefined, + identifier: '87', + }) + + await expect(command.execute({ identifier: '87' })).rejects.toThrow( + 'Docker is not available' + ) + + // Should not proceed to run server + expect(mockDevServerManager.runServerForeground).not.toHaveBeenCalled() + }) + + it('should not call DockerManager.assertAvailable when devServer is "process"', async () => { + vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue({ + capabilities: { + web: { + devServer: 'process', + }, + }, + }) + + await command.execute({ identifier: '87' }) + + expect(DockerManager.assertAvailable).not.toHaveBeenCalled() + expect(mockDevServerManager.isServerRunning).toHaveBeenCalledWith( + 3087, + undefined + ) + expect(mockDevServerManager.runServerForeground).toHaveBeenCalledWith( + mockWorktree.path, + 3087, + false, + expect.any(Function), + {}, + undefined + ) + }) + + it('should not call DockerManager.assertAvailable when devServer is not configured', async () => { + vi.mocked(mockSettingsManager.loadSettings).mockResolvedValue({}) + + await command.execute({ identifier: '87' }) + + expect(DockerManager.assertAvailable).not.toHaveBeenCalled() + expect(mockDevServerManager.isServerRunning).toHaveBeenCalledWith( + 3087, + undefined + ) + }) + }) }) diff --git a/src/commands/dev-server.ts b/src/commands/dev-server.ts index c1739f72..80685619 100644 --- a/src/commands/dev-server.ts +++ b/src/commands/dev-server.ts @@ -2,6 +2,7 @@ import path from 'path' import { GitWorktreeManager } from '../lib/GitWorktreeManager.js' import { ProjectCapabilityDetector } from '../lib/ProjectCapabilityDetector.js' import { DevServerManager } from '../lib/DevServerManager.js' +import { DockerManager } from '../lib/DockerManager.js' import { SettingsManager } from '../lib/SettingsManager.js' import { IdentifierParser } from '../utils/IdentifierParser.js' import { loadWorkspaceEnv, isNoEnvFilesFoundError } from '../utils/env.js' @@ -65,10 +66,22 @@ export class DevServerCommand { logger.debug(`Found worktree at: ${worktree.path}`) - // 3. Load settings to check sourceEnvOnStart + // 3. Load settings to check sourceEnvOnStart and Docker config const settings = await this.settingsManager.loadSettings() const shouldLoadEnv = settings.sourceEnvOnStart ?? false + // 3a. Extract Docker configuration if Docker mode is enabled + const identifier = parsed.number?.toString() ?? parsed.branchName ?? parsed.originalInput + const dockerConfig = DockerManager.buildDockerConfigFromSettings( + settings.capabilities?.web, + identifier + ) + + if (dockerConfig) { + await DockerManager.assertAvailable() + logger.debug(`Docker mode enabled with config: ${JSON.stringify(dockerConfig)}`) + } + // Build environment variables let envOverrides: Record = {} @@ -117,7 +130,7 @@ export class DevServerCommand { const url = `http://localhost:${port}` // 6. Check if server already running - const isRunning = await this.devServerManager.isServerRunning(port) + const isRunning = await this.devServerManager.isServerRunning(port, dockerConfig) if (isRunning) { const message = `Dev server already running at ${url}` @@ -165,7 +178,8 @@ export class DevServerCommand { this.outputJson(finalResult) } }, - envOverrides + envOverrides, + dockerConfig ) if (processInfo.pid) { diff --git a/src/commands/open.test.ts b/src/commands/open.test.ts index cedf63db..a5bbd189 100644 --- a/src/commands/open.test.ts +++ b/src/commands/open.test.ts @@ -657,7 +657,8 @@ describe('OpenCommand', () => { expect(mockDevServerManager.ensureServerRunning).toHaveBeenCalledWith( mockWorktree.path, - 3087 + 3087, + undefined ) }) @@ -687,7 +688,8 @@ describe('OpenCommand', () => { // Should calculate port as 3000 + 87 = 3087 expect(mockDevServerManager.ensureServerRunning).toHaveBeenCalledWith( mockWorktree.path, - 3087 + 3087, + undefined ) }) @@ -703,7 +705,8 @@ describe('OpenCommand', () => { // Should use PORT from .env expect(mockDevServerManager.ensureServerRunning).toHaveBeenCalledWith( mockWorktree.path, - 4500 + 4500, + undefined ) }) diff --git a/src/commands/open.ts b/src/commands/open.ts index f94ed2d7..59f886f3 100644 --- a/src/commands/open.ts +++ b/src/commands/open.ts @@ -4,6 +4,7 @@ import { execa } from 'execa' import { GitWorktreeManager } from '../lib/GitWorktreeManager.js' import { ProjectCapabilityDetector } from '../lib/ProjectCapabilityDetector.js' import { DevServerManager } from '../lib/DevServerManager.js' +import { DockerManager } from '../lib/DockerManager.js' import { SettingsManager } from '../lib/SettingsManager.js' import { IdentifierParser } from '../utils/IdentifierParser.js' import { openBrowser } from '../utils/browser.js' @@ -225,10 +226,24 @@ export class OpenCommand { checkEnvFile: true, }) + // Extract Docker configuration if Docker mode is enabled + const issueNumber = extractIssueNumber(worktree.branch) + const dockerIdentifier = issueNumber?.toString() ?? worktree.branch + const dockerConfig = DockerManager.buildDockerConfigFromSettings( + settings.capabilities?.web, + dockerIdentifier + ) + + if (dockerConfig) { + await DockerManager.assertAvailable() + logger.debug(`Docker mode enabled with config: ${JSON.stringify(dockerConfig)}`) + } + // Ensure dev server is running on the port const serverReady = await this.devServerManager.ensureServerRunning( worktree.path, - port + port, + dockerConfig ) if (!serverReady) { diff --git a/src/commands/run.test.ts b/src/commands/run.test.ts index a56ae10b..5fdb675c 100644 --- a/src/commands/run.test.ts +++ b/src/commands/run.test.ts @@ -592,7 +592,8 @@ describe('RunCommand', () => { expect(mockDevServerManager.ensureServerRunning).toHaveBeenCalledWith( mockWorktree.path, - 3087 + 3087, + undefined ) }) diff --git a/src/commands/run.ts b/src/commands/run.ts index 7bf83334..f609968f 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -4,6 +4,7 @@ import { execa } from 'execa' import { GitWorktreeManager } from '../lib/GitWorktreeManager.js' import { ProjectCapabilityDetector } from '../lib/ProjectCapabilityDetector.js' import { DevServerManager } from '../lib/DevServerManager.js' +import { DockerManager } from '../lib/DockerManager.js' import { SettingsManager } from '../lib/SettingsManager.js' import { IdentifierParser } from '../utils/IdentifierParser.js' import { openBrowser } from '../utils/browser.js' @@ -267,10 +268,24 @@ export class RunCommand { checkEnvFile: true, }) + // Extract Docker configuration if Docker mode is enabled + const issueNumber = extractIssueNumber(worktree.branch) + const dockerIdentifier = issueNumber?.toString() ?? worktree.branch + const dockerConfig = DockerManager.buildDockerConfigFromSettings( + settings.capabilities?.web, + dockerIdentifier + ) + + if (dockerConfig) { + await DockerManager.assertAvailable() + logger.debug(`Docker mode enabled with config: ${JSON.stringify(dockerConfig)}`) + } + // Ensure dev server is running on the port const serverReady = await this.devServerManager.ensureServerRunning( worktree.path, - port + port, + dockerConfig ) if (!serverReady) { diff --git a/src/lib/DevServerManager.test.ts b/src/lib/DevServerManager.test.ts index 5d045b36..258a808c 100644 --- a/src/lib/DevServerManager.test.ts +++ b/src/lib/DevServerManager.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { DevServerManager } from './DevServerManager.js' +import { DevServerManager, type DockerConfig } from './DevServerManager.js' import { ProcessManager } from './process/ProcessManager.js' +import { DockerManager } from './DockerManager.js' import { execa, type ExecaChildProcess } from 'execa' import { setTimeout } from 'timers/promises' import * as devServerUtils from '../utils/dev-server.js' @@ -11,6 +12,7 @@ import * as packageJsonUtils from '../utils/package-json.js' vi.mock('execa') vi.mock('timers/promises') vi.mock('./process/ProcessManager.js') +vi.mock('./DockerManager.js') vi.mock('../utils/dev-server.js') vi.mock('../utils/package-manager.js') vi.mock('../utils/package-json.js') @@ -582,6 +584,397 @@ describe('DevServerManager', () => { }) }) + describe('Docker mode', () => { + const dockerConfig: DockerConfig = { + dockerFile: './Dockerfile', + containerPort: 4200, + identifier: '548', + } + + beforeEach(() => { + // Set up default return values for DockerManager static methods + vi.mocked(DockerManager.buildContainerName).mockReturnValue('iloom-dev-548') + vi.mocked(DockerManager.buildImageName).mockReturnValue('iloom-dev-548') + }) + + describe('ensureServerRunning', () => { + it('should detect running container and skip start', async () => { + const port = 3548 + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(true) + + const result = await manager.ensureServerRunning(mockWorktreePath, port, dockerConfig) + + expect(result).toBe(true) + expect(DockerManager.isContainerRunning).toHaveBeenCalledWith('iloom-dev-548') + expect(DockerManager.buildImage).not.toHaveBeenCalled() + }) + + it('should build image then run container in background when not running', async () => { + const port = 3548 + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(false) + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runDetached).mockResolvedValue('abc123') + + // Server becomes ready on first poll + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce({ + pid: 12345, + name: 'docker-proxy', + command: 'docker', + port, + isDevServer: true, + }) + + const result = await manager.ensureServerRunning(mockWorktreePath, port, dockerConfig) + + expect(result).toBe(true) + expect(DockerManager.buildImage).toHaveBeenCalledWith( + mockWorktreePath, + 'iloom-dev-548', + './Dockerfile', + undefined + ) + expect(DockerManager.resolveContainerPort).toHaveBeenCalledWith( + 4200, + expect.stringContaining('Dockerfile'), + 'iloom-dev-548' + ) + expect(DockerManager.runDetached).toHaveBeenCalledWith( + 'iloom-dev-548', + 'iloom-dev-548', + port, + 4200, + undefined + ) + }) + + it('should return false when Docker build fails', async () => { + const port = 3548 + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(false) + vi.mocked(DockerManager.buildImage).mockRejectedValue(new Error('Build failed')) + + const result = await manager.ensureServerRunning(mockWorktreePath, port, dockerConfig) + + expect(result).toBe(false) + }) + + it('should clean up container and return false when server fails to start within timeout', async () => { + const port = 3548 + + manager = new DevServerManager(mockProcessManager, { + startupTimeout: 500, + checkInterval: 100, + }) + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(false) + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runDetached).mockResolvedValue('abc123') + vi.mocked(DockerManager.stopAndRemoveContainer).mockResolvedValue(true) + + // Server never starts + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValue(null) + vi.mocked(setTimeout).mockResolvedValue(undefined) + + const result = await manager.ensureServerRunning(mockWorktreePath, port, dockerConfig) + + expect(result).toBe(false) + // Should clean up the container on timeout + expect(DockerManager.stopAndRemoveContainer).toHaveBeenCalledWith('iloom-dev-548') + }) + + it('should pass build args and run args to Docker', async () => { + const port = 3548 + const configWithArgs: DockerConfig = { + ...dockerConfig, + dockerBuildArgs: { NODE_ENV: 'development' }, + dockerRunArgs: ['-v', './src:/app/src'], + } + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(false) + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runDetached).mockResolvedValue('abc123') + + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce({ + pid: 12345, + name: 'docker-proxy', + command: 'docker', + port, + isDevServer: true, + }) + + await manager.ensureServerRunning(mockWorktreePath, port, configWithArgs) + + expect(DockerManager.buildImage).toHaveBeenCalledWith( + mockWorktreePath, + 'iloom-dev-548', + './Dockerfile', + { NODE_ENV: 'development' } + ) + expect(DockerManager.runDetached).toHaveBeenCalledWith( + 'iloom-dev-548', + 'iloom-dev-548', + port, + 4200, + ['-v', './src:/app/src'] + ) + }) + + it('should not use process-based detection in Docker mode', async () => { + const port = 3548 + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(true) + + await manager.ensureServerRunning(mockWorktreePath, port, dockerConfig) + + // Process-based detection should NOT be called in Docker mode + expect(mockProcessManager.detectDevServer).not.toHaveBeenCalled() + }) + }) + + describe('isServerRunning', () => { + it('should check Docker container status when dockerConfig provided', async () => { + const port = 3548 + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(true) + + const result = await manager.isServerRunning(port, dockerConfig) + + expect(result).toBe(true) + expect(DockerManager.buildContainerName).toHaveBeenCalledWith('548') + expect(mockProcessManager.detectDevServer).not.toHaveBeenCalled() + }) + + it('should return false when Docker container is not running', async () => { + const port = 3548 + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(false) + + const result = await manager.isServerRunning(port, dockerConfig) + + expect(result).toBe(false) + }) + + it('should fall back to process detection when no dockerConfig', async () => { + const port = 3548 + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValue({ + pid: 12345, + name: 'node', + command: 'pnpm dev', + port, + isDevServer: true, + }) + + const result = await manager.isServerRunning(port) + + expect(result).toBe(true) + expect(mockProcessManager.detectDevServer).toHaveBeenCalledWith(port) + expect(DockerManager.isContainerRunning).not.toHaveBeenCalled() + }) + }) + + describe('runServerForeground', () => { + it('should build image then run container in foreground', async () => { + const port = 3548 + + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runForeground).mockResolvedValue(undefined) + + const result = await manager.runServerForeground( + mockWorktreePath, port, false, undefined, undefined, dockerConfig + ) + + expect(result).toEqual({}) + expect(DockerManager.buildImage).toHaveBeenCalledWith( + mockWorktreePath, + 'iloom-dev-548', + './Dockerfile', + undefined + ) + expect(DockerManager.runForeground).toHaveBeenCalledWith( + 'iloom-dev-548', + 'iloom-dev-548', + port, + 4200, + undefined, + false + ) + }) + + it('should pass redirectToStderr to DockerManager.runForeground', async () => { + const port = 3548 + + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runForeground).mockResolvedValue(undefined) + + await manager.runServerForeground( + mockWorktreePath, port, true, undefined, undefined, dockerConfig + ) + + expect(DockerManager.runForeground).toHaveBeenCalledWith( + 'iloom-dev-548', + 'iloom-dev-548', + port, + 4200, + undefined, + true + ) + }) + + it('should pass dockerRunArgs to container', async () => { + const port = 3548 + const configWithRunArgs: DockerConfig = { + ...dockerConfig, + dockerRunArgs: ['-v', './src:/app/src', '--env', 'DEBUG=true'], + } + + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runForeground).mockResolvedValue(undefined) + + await manager.runServerForeground( + mockWorktreePath, port, false, undefined, undefined, configWithRunArgs + ) + + expect(DockerManager.runForeground).toHaveBeenCalledWith( + 'iloom-dev-548', + 'iloom-dev-548', + port, + 4200, + ['-v', './src:/app/src', '--env', 'DEBUG=true'], + false + ) + }) + + it('should use containerPort from config or Dockerfile EXPOSE', async () => { + const port = 3548 + const configWithoutPort: DockerConfig = { + dockerFile: './Dockerfile', + identifier: '548', + } + + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(8080) + vi.mocked(DockerManager.runForeground).mockResolvedValue(undefined) + + await manager.runServerForeground( + mockWorktreePath, port, false, undefined, undefined, configWithoutPort + ) + + // resolveContainerPort should receive undefined for containerPort + expect(DockerManager.resolveContainerPort).toHaveBeenCalledWith( + undefined, + expect.stringContaining('Dockerfile'), + 'iloom-dev-548' + ) + // runForeground should use the resolved port + expect(DockerManager.runForeground).toHaveBeenCalledWith( + 'iloom-dev-548', + 'iloom-dev-548', + port, + 8080, + undefined, + false + ) + }) + + it('should call onProcessStarted with undefined pid in Docker mode', async () => { + const port = 3548 + const onStart = vi.fn() + + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runForeground).mockResolvedValue(undefined) + + await manager.runServerForeground( + mockWorktreePath, port, false, onStart, undefined, dockerConfig + ) + + // Docker containers don't have a host PID to report + expect(onStart).toHaveBeenCalledWith(undefined) + }) + + it('should not use runScript or buildDevServerCommand in Docker mode', async () => { + const port = 3548 + + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runForeground).mockResolvedValue(undefined) + + await manager.runServerForeground( + mockWorktreePath, port, false, undefined, undefined, dockerConfig + ) + + expect(packageManagerUtils.runScript).not.toHaveBeenCalled() + expect(devServerUtils.buildDevServerCommand).not.toHaveBeenCalled() + expect(execa).not.toHaveBeenCalled() + }) + }) + + describe('cleanup', () => { + it('should stop and remove tracked Docker containers', async () => { + const port = 3548 + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(false) + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runDetached).mockResolvedValue('abc123') + vi.mocked(DockerManager.stopAndRemoveContainer).mockResolvedValue(true) + + // Server becomes ready + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce({ + pid: 12345, + name: 'docker-proxy', + command: 'docker', + port, + isDevServer: true, + }) + + await manager.ensureServerRunning(mockWorktreePath, port, dockerConfig) + + // Reset mock to track cleanup call + vi.mocked(DockerManager.stopAndRemoveContainer).mockClear() + vi.mocked(DockerManager.stopAndRemoveContainer).mockResolvedValue(true) + + await manager.cleanup() + + expect(DockerManager.stopAndRemoveContainer).toHaveBeenCalledWith('iloom-dev-548') + }) + + it('should handle Docker cleanup errors gracefully', async () => { + const port = 3548 + + vi.mocked(DockerManager.isContainerRunning).mockResolvedValue(false) + vi.mocked(DockerManager.buildImage).mockResolvedValue(undefined) + vi.mocked(DockerManager.resolveContainerPort).mockResolvedValue(4200) + vi.mocked(DockerManager.runDetached).mockResolvedValue('abc123') + + // Server becomes ready + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce({ + pid: 12345, + name: 'docker-proxy', + command: 'docker', + port, + isDevServer: true, + }) + + await manager.ensureServerRunning(mockWorktreePath, port, dockerConfig) + + // Make cleanup fail + vi.mocked(DockerManager.stopAndRemoveContainer).mockRejectedValue( + new Error('Docker daemon not responding') + ) + + // Should not throw + await expect(manager.cleanup()).resolves.not.toThrow() + }) + }) + }) + describe('default options', () => { it('should use default timeout (180s) and interval if not specified', () => { const defaultManager = new DevServerManager() diff --git a/src/lib/DevServerManager.ts b/src/lib/DevServerManager.ts index b225863d..4464a88a 100644 --- a/src/lib/DevServerManager.ts +++ b/src/lib/DevServerManager.ts @@ -1,6 +1,8 @@ import { execa, type ExecaChildProcess } from 'execa' +import path from 'path' import { setTimeout } from 'timers/promises' import { ProcessManager } from './process/ProcessManager.js' +import { DockerManager, type DockerConfig } from './DockerManager.js' import { buildDevServerCommand } from '../utils/dev-server.js' import { runScript } from '../utils/package-manager.js' import { getPackageScripts } from '../utils/package-json.js' @@ -38,6 +40,9 @@ export interface DevServerManagerOptions { checkInterval?: number } +// Re-export DockerConfig from DockerManager for backward compatibility +export type { DockerConfig } from './DockerManager.js' + /** * DevServerManager handles auto-starting and monitoring dev servers * Used by open/run commands to ensure dev server is running before opening browser @@ -46,6 +51,7 @@ export class DevServerManager { private readonly processManager: ProcessManager private readonly options: Required private runningServers: Map = new Map() + private runningDockerContainers: Map = new Map() constructor( processManager?: ProcessManager, @@ -64,12 +70,34 @@ export class DevServerManager { * * @param worktreePath - Path to the worktree * @param port - Port the server should run on + * @param dockerConfig - Optional Docker configuration for container-based server * @returns true if server is ready, false if startup failed/timed out */ - async ensureServerRunning(worktreePath: string, port: number): Promise { + async ensureServerRunning(worktreePath: string, port: number, dockerConfig?: DockerConfig): Promise { logger.debug(`Checking if dev server is running on port ${port}...`) - // Check if already running + // Docker mode: check if container is already running + if (dockerConfig) { + const containerName = DockerManager.buildContainerName(dockerConfig.identifier) + const isRunning = await DockerManager.isContainerRunning(containerName) + if (isRunning) { + logger.debug(`Docker container "${containerName}" already running on port ${port}`) + return true + } + + logger.info(`Docker dev server not running on port ${port}, starting...`) + try { + await this.startDockerServer(worktreePath, port, dockerConfig) + return true + } catch (error) { + logger.error( + `Failed to start Docker dev server: ${error instanceof Error ? error.message : 'Unknown error'}` + ) + return false + } + } + + // Process mode: check if a process is listening on the port const existingProcess = await this.processManager.detectDevServer(port) if (existingProcess) { logger.debug( @@ -139,6 +167,59 @@ export class DevServerManager { logger.success(`Dev server started successfully on port ${port}`) } + /** + * Start dev server in Docker container (background) and wait for it to be ready. + * Builds the image, resolves the container port, starts the container detached, + * and polls the host port for readiness. + */ + private async startDockerServer(worktreePath: string, port: number, dockerConfig: DockerConfig): Promise { + const imageName = DockerManager.buildImageName(dockerConfig.identifier) + const containerName = DockerManager.buildContainerName(dockerConfig.identifier) + const dockerfilePath = path.resolve(worktreePath, dockerConfig.dockerFile) + + // Build image + await DockerManager.buildImage( + worktreePath, + imageName, + dockerConfig.dockerFile, + dockerConfig.dockerBuildArgs + ) + + // Resolve container port (config > image inspect > Dockerfile EXPOSE) + const containerPort = await DockerManager.resolveContainerPort( + dockerConfig.containerPort, + dockerfilePath, + imageName + ) + + // Run container detached + await DockerManager.runDetached( + imageName, + containerName, + port, + containerPort, + dockerConfig.dockerRunArgs + ) + + // Track for cleanup + this.runningDockerContainers.set(port, containerName) + + // Wait for server to be ready (Docker proxy listens on host port) + logger.info(`Waiting for Docker dev server to start on port ${port}...`) + const ready = await this.waitForServerReady(port) + + if (!ready) { + // Clean up the container if startup failed + await DockerManager.stopAndRemoveContainer(containerName) + this.runningDockerContainers.delete(port) + throw new Error( + `Docker dev server failed to start within ${this.options.startupTimeout}ms timeout` + ) + } + + logger.success(`Docker dev server started successfully on port ${port}`) + } + /** * Wait for server to be ready by polling the port */ @@ -174,9 +255,14 @@ export class DevServerManager { * Check if a dev server is running on the specified port * * @param port - Port to check + * @param dockerConfig - Optional Docker configuration; when provided, checks container status * @returns true if server is running, false otherwise */ - async isServerRunning(port: number): Promise { + async isServerRunning(port: number, dockerConfig?: DockerConfig): Promise { + if (dockerConfig) { + const containerName = DockerManager.buildContainerName(dockerConfig.identifier) + return DockerManager.isContainerRunning(containerName) + } const existingProcess = await this.processManager.detectDevServer(port) return existingProcess !== null } @@ -196,8 +282,56 @@ export class DevServerManager { port: number, redirectToStderr = false, onProcessStarted?: (pid?: number) => void, - envOverrides?: Record + envOverrides?: Record, + dockerConfig?: DockerConfig ): Promise<{ pid?: number }> { + // Docker mode: build image and run container in foreground + if (dockerConfig) { + logger.debug(`Starting Docker dev server in foreground on port ${port}`) + + const imageName = DockerManager.buildImageName(dockerConfig.identifier) + const containerName = DockerManager.buildContainerName(dockerConfig.identifier) + const dockerfilePath = path.resolve(worktreePath, dockerConfig.dockerFile) + + // Build image + await DockerManager.buildImage( + worktreePath, + imageName, + dockerConfig.dockerFile, + dockerConfig.dockerBuildArgs + ) + + // Resolve container port + const containerPort = await DockerManager.resolveContainerPort( + dockerConfig.containerPort, + dockerfilePath, + imageName + ) + + if (onProcessStarted) { + onProcessStarted(undefined) + } + + // Track container for cleanup + this.runningDockerContainers.set(port, containerName) + try { + // Run container in foreground (blocks until stopped) + // DockerManager.runForeground handles signal forwarding internally + await DockerManager.runForeground( + imageName, + containerName, + port, + containerPort, + dockerConfig.dockerRunArgs, + redirectToStderr + ) + } finally { + this.runningDockerContainers.delete(port) + } + + return {} + } + logger.debug(`Starting dev server in foreground on port ${port}`) // Use runScript for foreground mode to support multi-language projects @@ -244,6 +378,7 @@ export class DevServerManager { * This should be called when the manager is being disposed */ async cleanup(): Promise { + // Clean up process-based servers for (const [port, serverProcess] of this.runningServers.entries()) { try { logger.debug(`Cleaning up server process on port ${port}`) @@ -255,5 +390,18 @@ export class DevServerManager { } } this.runningServers.clear() + + // Clean up Docker containers + for (const [port, containerName] of this.runningDockerContainers.entries()) { + try { + logger.debug(`Cleaning up Docker container "${containerName}" on port ${port}`) + await DockerManager.stopAndRemoveContainer(containerName) + } catch (error) { + logger.warn( + `Failed to stop Docker container "${containerName}" on port ${port}: ${error instanceof Error ? error.message : 'Unknown error'}` + ) + } + } + this.runningDockerContainers.clear() } } diff --git a/src/lib/DockerManager.test.ts b/src/lib/DockerManager.test.ts new file mode 100644 index 00000000..4b3fa9f7 --- /dev/null +++ b/src/lib/DockerManager.test.ts @@ -0,0 +1,796 @@ +import { describe, it, expect, vi } from 'vitest' +import { DockerManager } from './DockerManager.js' +import { execa } from 'execa' +import { readFile } from 'fs/promises' + +// Mock dependencies +vi.mock('execa') +vi.mock('fs/promises') + +// Mock the logger +vi.mock('../utils/logger.js', () => ({ + logger: { + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + success: vi.fn(), + }, +})) + +describe('DockerManager', () => { + describe('isAvailable', () => { + it('should return true when docker CLI is accessible', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '', + stderr: '', + } as never) + + const result = await DockerManager.isAvailable() + + expect(result).toBe(true) + expect(execa).toHaveBeenCalledWith('docker', ['info'], { reject: false }) + }) + + it('should return false when docker CLI is not found', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 1, + stdout: '', + stderr: 'Cannot connect to the Docker daemon', + } as never) + + const result = await DockerManager.isAvailable() + + expect(result).toBe(false) + }) + + it('should return false when execa throws', async () => { + vi.mocked(execa).mockRejectedValue(new Error('command not found: docker')) + + const result = await DockerManager.isAvailable() + + expect(result).toBe(false) + }) + }) + + describe('assertAvailable', () => { + it('should not throw when Docker is available', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '', + stderr: '', + } as never) + + await expect(DockerManager.assertAvailable()).resolves.toBeUndefined() + }) + + it('should throw with install instructions when Docker is not available', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 1, + stdout: '', + stderr: '', + } as never) + + await expect(DockerManager.assertAvailable()).rejects.toThrow( + 'Docker is not available' + ) + await expect(DockerManager.assertAvailable()).rejects.toThrow( + 'https://docs.docker.com/get-docker/' + ) + }) + }) + + describe('buildImage', () => { + it('should build with default Dockerfile path', async () => { + vi.mocked(execa).mockResolvedValue({ exitCode: 0 } as never) + + await DockerManager.buildImage('/test/worktree', 'my-image', './Dockerfile') + + expect(execa).toHaveBeenCalledWith( + 'docker', + ['build', '-t', 'my-image', '-f', './Dockerfile', '.'], + { cwd: '/test/worktree', stdio: 'inherit' } + ) + }) + + it('should build with custom Dockerfile path', async () => { + vi.mocked(execa).mockResolvedValue({ exitCode: 0 } as never) + + await DockerManager.buildImage('/test/worktree', 'my-image', './docker/Dockerfile.dev') + + expect(execa).toHaveBeenCalledWith( + 'docker', + ['build', '-t', 'my-image', '-f', './docker/Dockerfile.dev', '.'], + { cwd: '/test/worktree', stdio: 'inherit' } + ) + }) + + it('should pass build args when configured', async () => { + vi.mocked(execa).mockResolvedValue({ exitCode: 0 } as never) + + await DockerManager.buildImage( + '/test/worktree', + 'my-image', + './Dockerfile', + { NODE_ENV: 'development', API_URL: 'http://localhost:3000' } + ) + + expect(execa).toHaveBeenCalledWith( + 'docker', + [ + 'build', '-t', 'my-image', '-f', './Dockerfile', + '--build-arg', 'NODE_ENV=development', + '--build-arg', 'API_URL=http://localhost:3000', + '.', + ], + { cwd: '/test/worktree', stdio: 'inherit' } + ) + }) + + it('should throw on build failure with clear error message', async () => { + vi.mocked(execa).mockRejectedValue(new Error('Step 3/7 : RUN npm install\n---> Running in abc123\nnpm ERR! code E404')) + + await expect( + DockerManager.buildImage('/test/worktree', 'my-image', './Dockerfile') + ).rejects.toThrow('Docker build failed for image "my-image"') + }) + }) + + describe('runDetached', () => { + it('should run with port mapping -p hostPort:containerPort', async () => { + // First call: force-remove existing container (reject: false) + // Second call: docker run + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockResolvedValueOnce({ exitCode: 0, stdout: 'abc123def456' } as never) // run -d + + const containerId = await DockerManager.runDetached( + 'my-image', 'iloom-dev-123', 3123, 4200 + ) + + expect(containerId).toBe('abc123def456') + // First call: force-remove + expect(execa).toHaveBeenNthCalledWith(1, 'docker', ['rm', '-f', 'iloom-dev-123'], { reject: false }) + // Second call: run + expect(execa).toHaveBeenNthCalledWith(2, 'docker', [ + 'run', '-d', + '--name', 'iloom-dev-123', + '-p', '3123:4200', + 'my-image', + ]) + }) + + it('should pass additional dockerRunArgs', async () => { + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockResolvedValueOnce({ exitCode: 0, stdout: 'abc123' } as never) // run + + await DockerManager.runDetached( + 'my-image', 'iloom-dev-123', 3123, 4200, + ['-v', './src:/app/src', '--env', 'DEBUG=true'] + ) + + expect(execa).toHaveBeenNthCalledWith(2, 'docker', [ + 'run', '-d', + '--name', 'iloom-dev-123', + '-p', '3123:4200', + '-v', './src:/app/src', '--env', 'DEBUG=true', + 'my-image', + ]) + }) + + it('should use named container iloom-dev-{identifier}', async () => { + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockResolvedValueOnce({ exitCode: 0, stdout: 'containerid' } as never) // run + + await DockerManager.runDetached( + 'my-image', 'iloom-dev-548', 3548, 4200 + ) + + expect(execa).toHaveBeenNthCalledWith(2, 'docker', expect.arrayContaining([ + '--name', 'iloom-dev-548', + ])) + }) + + it('should throw on run failure with error details', async () => { + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockRejectedValueOnce(new Error('port already allocated')) + + await expect( + DockerManager.runDetached('my-image', 'iloom-dev-123', 3123, 4200) + ).rejects.toThrow('Failed to start Docker container "iloom-dev-123"') + }) + }) + + describe('runForeground', () => { + it('should run attached with --rm flag and port mapping', async () => { + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockResolvedValueOnce({ exitCode: 0 } as never) // run (foreground) + + await DockerManager.runForeground( + 'my-image', 'iloom-dev-123', 3123, 4200 + ) + + // First call: force-remove + expect(execa).toHaveBeenNthCalledWith(1, 'docker', ['rm', '-f', 'iloom-dev-123'], { reject: false }) + // Second call: run in foreground + expect(execa).toHaveBeenNthCalledWith(2, 'docker', [ + 'run', + '--name', 'iloom-dev-123', + '--rm', + '-p', '3123:4200', + 'my-image', + ], { stdio: 'inherit' }) + }) + + it('should redirect to stderr when redirectToStderr is true', async () => { + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockResolvedValueOnce({ exitCode: 0 } as never) // run + + await DockerManager.runForeground( + 'my-image', 'iloom-dev-123', 3123, 4200, + undefined, true + ) + + expect(execa).toHaveBeenNthCalledWith(2, 'docker', expect.any(Array), { + stdio: [process.stdin, process.stderr, process.stderr], + }) + }) + + it('should pass additional args', async () => { + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockResolvedValueOnce({ exitCode: 0 } as never) // run + + await DockerManager.runForeground( + 'my-image', 'iloom-dev-123', 3123, 4200, + ['-v', './src:/app/src'] + ) + + expect(execa).toHaveBeenNthCalledWith(2, 'docker', [ + 'run', + '--name', 'iloom-dev-123', + '--rm', + '-p', '3123:4200', + '-v', './src:/app/src', + 'my-image', + ], { stdio: 'inherit' }) + }) + + it('should set up signal forwarding for SIGINT and SIGTERM', async () => { + const processOnSpy = vi.spyOn(process, 'on') + const processRemoveListenerSpy = vi.spyOn(process, 'removeListener') + + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockResolvedValueOnce({ exitCode: 0 } as never) // run + + await DockerManager.runForeground( + 'my-image', 'iloom-dev-123', 3123, 4200 + ) + + // Should have registered signal handlers + expect(processOnSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function)) + expect(processOnSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function)) + + // Should have cleaned up signal handlers + expect(processRemoveListenerSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function)) + expect(processRemoveListenerSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function)) + }) + + it('should clean up signal handlers even when docker run fails', async () => { + const processRemoveListenerSpy = vi.spyOn(process, 'removeListener') + + vi.mocked(execa) + .mockResolvedValueOnce({ exitCode: 0 } as never) // rm -f + .mockRejectedValueOnce(new Error('container failed')) // run + + await expect( + DockerManager.runForeground('my-image', 'iloom-dev-123', 3123, 4200) + ).rejects.toThrow('container failed') + + // Signal handlers should still be cleaned up + expect(processRemoveListenerSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function)) + expect(processRemoveListenerSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function)) + }) + }) + + describe('stopAndRemoveContainer', () => { + it('should force-remove a running container', async () => { + // isContainerRunning check + vi.mocked(execa) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: 'iloom-dev-123', + } as never) // docker ps + .mockResolvedValueOnce({ exitCode: 0 } as never) // docker rm -f + + const result = await DockerManager.stopAndRemoveContainer('iloom-dev-123') + + expect(result).toBe(true) + // Should have used docker rm -f (atomic force-remove) + expect(execa).toHaveBeenNthCalledWith(2, 'docker', ['rm', '-f', 'iloom-dev-123'], { reject: false }) + }) + + it('should not throw if container does not exist', async () => { + // isContainerRunning returns false + vi.mocked(execa) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: '', + } as never) // docker ps (empty = not running) + .mockResolvedValueOnce({ exitCode: 1 } as never) // docker rm -f (not found, ok) + + const result = await DockerManager.stopAndRemoveContainer('iloom-dev-nonexistent') + + expect(result).toBe(false) + }) + + it('should still try rm -f for stopped containers', async () => { + // isContainerRunning returns false (container stopped but exists) + vi.mocked(execa) + .mockResolvedValueOnce({ + exitCode: 0, + stdout: '', + } as never) // docker ps (not running) + .mockResolvedValueOnce({ exitCode: 0 } as never) // docker rm -f (removes stopped container) + + const result = await DockerManager.stopAndRemoveContainer('iloom-dev-stopped') + + expect(result).toBe(false) + expect(execa).toHaveBeenNthCalledWith(2, 'docker', ['rm', '-f', 'iloom-dev-stopped'], { reject: false }) + }) + }) + + describe('isContainerRunning', () => { + it('should return true for running named container', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: 'iloom-dev-123', + } as never) + + const result = await DockerManager.isContainerRunning('iloom-dev-123') + + expect(result).toBe(true) + expect(execa).toHaveBeenCalledWith( + 'docker', + ['ps', '--filter', 'name=^iloom-dev-123$', '--format', '{{.Names}}'], + { reject: false } + ) + }) + + it('should return false for stopped/missing container', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '', + } as never) + + const result = await DockerManager.isContainerRunning('iloom-dev-missing') + + expect(result).toBe(false) + }) + + it('should return false when docker ps fails', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 1, + stdout: '', + } as never) + + const result = await DockerManager.isContainerRunning('iloom-dev-123') + + expect(result).toBe(false) + }) + + it('should return false when execa throws', async () => { + vi.mocked(execa).mockRejectedValue(new Error('Docker not available')) + + const result = await DockerManager.isContainerRunning('iloom-dev-123') + + expect(result).toBe(false) + }) + + it('should use exact name matching with anchored regex', async () => { + // Ensure "iloom-dev-1" doesn't match "iloom-dev-123" + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: 'iloom-dev-123', // returned name does not match queried name + } as never) + + const result = await DockerManager.isContainerRunning('iloom-dev-1') + + expect(result).toBe(false) // Names don't match + }) + }) + + describe('parseExposeFromDockerfile', () => { + it('should extract port from EXPOSE directive', async () => { + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nWORKDIR /app\nCOPY . .\nEXPOSE 4200\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.parseExposeFromDockerfile('/test/Dockerfile') + + expect(port).toBe(4200) + }) + + it('should return first EXPOSE if multiple exist', async () => { + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nEXPOSE 4200\nEXPOSE 8080\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.parseExposeFromDockerfile('/test/Dockerfile') + + expect(port).toBe(4200) + }) + + it('should return null if no EXPOSE directive', async () => { + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nWORKDIR /app\nCOPY . .\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.parseExposeFromDockerfile('/test/Dockerfile') + + expect(port).toBeNull() + }) + + it('should handle EXPOSE with protocol (e.g., 4200/tcp)', async () => { + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nEXPOSE 4200/tcp\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.parseExposeFromDockerfile('/test/Dockerfile') + + expect(port).toBe(4200) + }) + + it('should handle EXPOSE with udp protocol', async () => { + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nEXPOSE 8080/udp\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.parseExposeFromDockerfile('/test/Dockerfile') + + expect(port).toBe(8080) + }) + + it('should return null if Dockerfile does not exist', async () => { + vi.mocked(readFile).mockRejectedValue( + Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + ) + + const port = await DockerManager.parseExposeFromDockerfile('/nonexistent/Dockerfile') + + expect(port).toBeNull() + }) + + it('should not match EXPOSE in comments', async () => { + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\n# EXPOSE 4200\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.parseExposeFromDockerfile('/test/Dockerfile') + + expect(port).toBeNull() + }) + }) + + describe('inspectImagePorts', () => { + it('should detect ports from docker image inspect', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '{"4200/tcp":{}}', + } as never) + + const port = await DockerManager.inspectImagePorts('my-image') + + expect(port).toBe(4200) + expect(execa).toHaveBeenCalledWith( + 'docker', + ['image', 'inspect', 'my-image', '--format', '{{json .Config.ExposedPorts}}'], + { reject: false } + ) + }) + + it('should return first port when multiple are exposed', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '{"4200/tcp":{},"8080/tcp":{}}', + } as never) + + const port = await DockerManager.inspectImagePorts('my-image') + + expect(port).toBe(4200) + }) + + it('should return null when no ports are exposed', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: 'null', + } as never) + + const port = await DockerManager.inspectImagePorts('my-image') + + expect(port).toBeNull() + }) + + it('should return null for empty exposed ports', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '{}', + } as never) + + const port = await DockerManager.inspectImagePorts('my-image') + + expect(port).toBeNull() + }) + + it('should return null when inspect fails', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 1, + stdout: '', + } as never) + + const port = await DockerManager.inspectImagePorts('nonexistent-image') + + expect(port).toBeNull() + }) + + it('should return null for output', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '', + } as never) + + const port = await DockerManager.inspectImagePorts('my-image') + + expect(port).toBeNull() + }) + + it('should handle execa throwing', async () => { + vi.mocked(execa).mockRejectedValue(new Error('Docker not available')) + + const port = await DockerManager.inspectImagePorts('my-image') + + expect(port).toBeNull() + }) + }) + + describe('resolveContainerPort', () => { + it('should return config port when provided', async () => { + const port = await DockerManager.resolveContainerPort(4200, '/test/Dockerfile') + + expect(port).toBe(4200) + // Should not call execa or readFile since config port is provided + expect(execa).not.toHaveBeenCalled() + expect(readFile).not.toHaveBeenCalled() + }) + + it('should use image inspect when config port is not set and imageName provided', async () => { + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: '{"8080/tcp":{}}', + } as never) + + const port = await DockerManager.resolveContainerPort( + undefined, '/test/Dockerfile', 'my-image' + ) + + expect(port).toBe(8080) + }) + + it('should fall back to Dockerfile regex when image inspect returns nothing', async () => { + // Image inspect returns null + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: 'null', + } as never) + + // Dockerfile has EXPOSE + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nEXPOSE 3000\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.resolveContainerPort( + undefined, '/test/Dockerfile', 'my-image' + ) + + expect(port).toBe(3000) + }) + + it('should use Dockerfile regex when no imageName provided', async () => { + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nEXPOSE 4200\nCMD ["npm", "start"]' + ) + + const port = await DockerManager.resolveContainerPort(undefined, '/test/Dockerfile') + + expect(port).toBe(4200) + }) + + it('should throw when no port source provides a port', async () => { + // Image inspect returns null + vi.mocked(execa).mockResolvedValue({ + exitCode: 0, + stdout: 'null', + } as never) + + // Dockerfile has no EXPOSE + vi.mocked(readFile).mockResolvedValue( + 'FROM node:18\nCMD ["npm", "start"]' + ) + + await expect( + DockerManager.resolveContainerPort(undefined, '/test/Dockerfile', 'my-image') + ).rejects.toThrow('Cannot determine container port') + }) + + it('should include helpful error message with resolution steps', async () => { + vi.mocked(readFile).mockResolvedValue('FROM node:18\nCMD ["npm", "start"]') + + await expect( + DockerManager.resolveContainerPort(undefined, '/test/Dockerfile') + ).rejects.toThrow('capabilities.web') + }) + }) + + describe('sanitizeContainerName', () => { + it('should replace slashes with hyphens', () => { + expect(DockerManager.sanitizeContainerName('feat/issue-548')).toBe('feat-issue-548') + }) + + it('should remove invalid characters', () => { + expect(DockerManager.sanitizeContainerName('my@container!name')).toBe('my-container-name') + }) + + it('should handle branch names like feat/issue-548__docker', () => { + // Double underscores are valid Docker container name characters + expect(DockerManager.sanitizeContainerName('iloom-dev-feat/issue-548__docker')) + .toBe('iloom-dev-feat-issue-548__docker') + }) + + it('should collapse consecutive hyphens', () => { + expect(DockerManager.sanitizeContainerName('a---b---c')).toBe('a-b-c') + }) + + it('should remove leading non-alphanumeric characters', () => { + expect(DockerManager.sanitizeContainerName('---my-container')).toBe('my-container') + }) + + it('should remove trailing hyphens', () => { + expect(DockerManager.sanitizeContainerName('my-container---')).toBe('my-container') + }) + + it('should truncate to 63 characters', () => { + const longName = 'a'.repeat(100) + const result = DockerManager.sanitizeContainerName(longName) + + expect(result.length).toBeLessThanOrEqual(63) + }) + + it('should clean trailing hyphen after truncation', () => { + // Create a name that will have a hyphen at position 63 after replacement + const name = 'a'.repeat(62) + '/b' + const result = DockerManager.sanitizeContainerName(name) + + expect(result).not.toMatch(/-$/) + expect(result.length).toBeLessThanOrEqual(63) + }) + + it('should preserve dots and underscores', () => { + expect(DockerManager.sanitizeContainerName('my_container.v1')).toBe('my_container.v1') + }) + + it('should handle empty string with fallback', () => { + expect(DockerManager.sanitizeContainerName('---')).toBe('iloom-container') + }) + + it('should handle string with only invalid characters', () => { + expect(DockerManager.sanitizeContainerName('@@@')).toBe('iloom-container') + }) + + it('should handle spaces in names', () => { + expect(DockerManager.sanitizeContainerName('my container name')).toBe('my-container-name') + }) + + it('should preserve double underscores (valid Docker name chars)', () => { + // Underscores are valid in Docker container names + expect(DockerManager.sanitizeContainerName('feat__docker-server')) + .toBe('feat__docker-server') + }) + }) + + describe('buildContainerName', () => { + it('should build name with numeric identifier', () => { + expect(DockerManager.buildContainerName(548)).toBe('iloom-dev-548') + }) + + it('should build name with string identifier', () => { + expect(DockerManager.buildContainerName('my-branch')).toBe('iloom-dev-my-branch') + }) + + it('should sanitize branch names with slashes', () => { + // Slashes replaced with hyphens, underscores preserved + expect(DockerManager.buildContainerName('feat/issue-548__docker')) + .toBe('iloom-dev-feat-issue-548__docker') + }) + }) + + describe('buildImageName', () => { + it('should build image name with numeric identifier', () => { + expect(DockerManager.buildImageName(548)).toBe('iloom-dev-548') + }) + + it('should build image name with string identifier', () => { + expect(DockerManager.buildImageName('my-branch')).toBe('iloom-dev-my-branch') + }) + }) + + describe('buildDockerConfigFromSettings', () => { + it('should return undefined when webSettings is undefined', () => { + const result = DockerManager.buildDockerConfigFromSettings(undefined, '548') + expect(result).toBeUndefined() + }) + + it('should return undefined when devServer is "process"', () => { + const result = DockerManager.buildDockerConfigFromSettings( + { devServer: 'process' }, + '548' + ) + expect(result).toBeUndefined() + }) + + it('should return undefined when devServer is not set', () => { + const result = DockerManager.buildDockerConfigFromSettings({}, '548') + expect(result).toBeUndefined() + }) + + it('should return DockerConfig when devServer is "docker"', () => { + const result = DockerManager.buildDockerConfigFromSettings( + { devServer: 'docker' }, + '548' + ) + + expect(result).toEqual({ + dockerFile: './Dockerfile', + containerPort: undefined, + dockerBuildArgs: undefined, + dockerRunArgs: undefined, + identifier: '548', + }) + }) + + it('should use custom dockerFile when provided', () => { + const result = DockerManager.buildDockerConfigFromSettings( + { devServer: 'docker', dockerFile: './docker/Dockerfile.dev' }, + '548' + ) + + expect(result?.dockerFile).toBe('./docker/Dockerfile.dev') + }) + + it('should pass through containerPort, buildArgs, and runArgs', () => { + const result = DockerManager.buildDockerConfigFromSettings( + { + devServer: 'docker', + containerPort: 4200, + dockerBuildArgs: { NODE_ENV: 'development' }, + dockerRunArgs: ['-v', './src:/app/src'], + }, + 'my-branch' + ) + + expect(result).toEqual({ + dockerFile: './Dockerfile', + containerPort: 4200, + dockerBuildArgs: { NODE_ENV: 'development' }, + dockerRunArgs: ['-v', './src:/app/src'], + identifier: 'my-branch', + }) + }) + }) +}) diff --git a/src/lib/DockerManager.ts b/src/lib/DockerManager.ts new file mode 100644 index 00000000..552c1aa5 --- /dev/null +++ b/src/lib/DockerManager.ts @@ -0,0 +1,491 @@ +import { execa } from 'execa' +import { readFile } from 'fs/promises' +import { logger } from '../utils/logger.js' + +/** + * Configuration for Docker-based dev server mode. + * When provided to DevServerManager methods, the server runs inside a Docker container + * with port mapping instead of as a local process. + */ +export interface DockerConfig { + /** Path to Dockerfile (relative to worktree) */ + dockerFile: string + /** Port inside the container (auto-detected from image inspect or Dockerfile EXPOSE if not set) */ + containerPort?: number | undefined + /** Build arguments passed as --build-arg to docker build */ + dockerBuildArgs?: Record | undefined + /** Additional docker run flags (e.g., volume mounts) */ + dockerRunArgs?: string[] | undefined + /** Identifier for container naming (issue number, branch name) */ + identifier: string +} + +/** + * Web settings shape accepted by buildDockerConfigFromSettings. + * Matches the capabilities.web section of IloomSettings. + * Uses `| undefined` for optional properties to satisfy exactOptionalPropertyTypes. + */ +interface WebSettings { + devServer?: 'process' | 'docker' | undefined + dockerFile?: string | undefined + containerPort?: number | undefined + dockerBuildArgs?: Record | undefined + dockerRunArgs?: string[] | undefined +} + +/** + * Maximum length for Docker container names. + * Docker allows up to 63 characters for container names. + */ +const MAX_CONTAINER_NAME_LENGTH = 63 + +/** + * DockerManager encapsulates all Docker CLI interactions. + * Used by DevServerManager and ResourceCleanup for Docker-based dev server operations. + * + * All methods are static since no instance state is needed -- Docker CLI is stateless. + */ +export class DockerManager { + /** + * Check if Docker CLI is available and daemon is running. + * @returns true if Docker is ready, false otherwise + */ + static async isAvailable(): Promise { + try { + const result = await execa('docker', ['info'], { reject: false }) + return result.exitCode === 0 + } catch { + return false + } + } + + /** + * Assert that Docker is available, throwing a clear error if not. + * Call this early in any workflow that requires Docker. + */ + static async assertAvailable(): Promise { + const available = await DockerManager.isAvailable() + if (!available) { + throw new Error( + 'Docker is not available. Please ensure Docker is installed and the Docker daemon is running.\n' + + 'Install Docker: https://docs.docker.com/get-docker/' + ) + } + } + + /** + * Build a Docker image. + * + * @param cwd - Working directory (worktree path) + * @param imageName - Image tag name + * @param dockerFile - Path to Dockerfile (relative to cwd) + * @param buildArgs - Optional build arguments passed as --build-arg + */ + static async buildImage( + cwd: string, + imageName: string, + dockerFile: string, + buildArgs?: Record + ): Promise { + const args = ['build', '-t', imageName, '-f', dockerFile] + + if (buildArgs) { + for (const [key, value] of Object.entries(buildArgs)) { + args.push('--build-arg', `${key}=${value}`) + } + } + + // Context directory is always cwd + args.push('.') + + logger.info(`Building Docker image "${imageName}" from ${dockerFile}...`) + + try { + await execa('docker', args, { + cwd, + stdio: 'inherit', + }) + } catch (error) { + const message = error instanceof Error ? error.message : 'Unknown error' + throw new Error(`Docker build failed for image "${imageName}": ${message}`) + } + + logger.success(`Docker image "${imageName}" built successfully`) + } + + /** + * Run a container in detached mode (background). + * If a container with the same name already exists, it is force-removed first. + * + * @param imageName - Image to run + * @param containerName - Container name (must be sanitized) + * @param hostPort - Port on the host to map + * @param containerPort - Port inside the container + * @param additionalArgs - Additional docker run flags (e.g., volume mounts) + * @returns Container ID + */ + static async runDetached( + imageName: string, + containerName: string, + hostPort: number, + containerPort: number, + additionalArgs?: string[] + ): Promise { + // Force-remove any existing container with same name to avoid name collision + await DockerManager.forceRemoveContainer(containerName) + + const args = [ + 'run', '-d', + '--name', containerName, + '-p', `${hostPort}:${containerPort}`, + ] + + if (additionalArgs) { + args.push(...additionalArgs) + } + + args.push(imageName) + + logger.info(`Starting Docker container "${containerName}" (${hostPort}:${containerPort})...`) + + try { + const result = await execa('docker', args) + const containerId = result.stdout.trim() + logger.success(`Docker container "${containerName}" started (ID: ${containerId.substring(0, 12)})`) + return containerId + } catch (error) { + const message = error instanceof Error ? error.message : 'Unknown error' + throw new Error(`Failed to start Docker container "${containerName}": ${message}`) + } + } + + /** + * Run a container in foreground mode (attached, blocking). + * The container is automatically removed on exit (--rm flag). + * Stdout/stderr are streamed to the terminal. + * + * Signal forwarding: Captures SIGINT/SIGTERM on the host process and + * forwards them to the container via `docker kill --signal` for graceful + * shutdown of the framework running inside the container. + * + * @param imageName - Image to run + * @param containerName - Container name (must be sanitized) + * @param hostPort - Port on the host to map + * @param containerPort - Port inside the container + * @param additionalArgs - Additional docker run flags + * @param redirectToStderr - If true, redirect stdout/stderr to stderr + */ + static async runForeground( + imageName: string, + containerName: string, + hostPort: number, + containerPort: number, + additionalArgs?: string[], + redirectToStderr?: boolean + ): Promise { + // Force-remove any existing container with same name to avoid name collision + await DockerManager.forceRemoveContainer(containerName) + + const args = [ + 'run', + '--name', containerName, + '--rm', + '-p', `${hostPort}:${containerPort}`, + ] + + if (additionalArgs) { + args.push(...additionalArgs) + } + + args.push(imageName) + + logger.info(`Running Docker container "${containerName}" in foreground (${hostPort}:${containerPort})...`) + + const stdio = redirectToStderr + ? [process.stdin, process.stderr, process.stderr] as const + : 'inherit' as const + + // Set up signal forwarding to gracefully shut down the container + const forwardSignal = (signal: string): void => { + logger.debug(`Forwarding ${signal} to container "${containerName}"`) + void execa('docker', ['kill', `--signal=${signal}`, containerName], { reject: false }) + } + + const onSigint = (): void => forwardSignal('SIGINT') + const onSigterm = (): void => forwardSignal('SIGTERM') + + process.on('SIGINT', onSigint) + process.on('SIGTERM', onSigterm) + + try { + await execa('docker', args, { stdio }) + } finally { + // Clean up signal handlers to avoid leaks + process.removeListener('SIGINT', onSigint) + process.removeListener('SIGTERM', onSigterm) + } + } + + /** + * Stop and remove a container by name using atomic force-remove. + * No-op if the container doesn't exist or is already removed. + * Uses `docker rm -f` which handles both running and stopped containers + * atomically, eliminating race conditions between stop and remove. + * + * @param containerName - Name of the container to remove + * @returns true if a container was removed, false if no container was found + */ + static async stopAndRemoveContainer(containerName: string): Promise { + const isRunning = await DockerManager.isContainerRunning(containerName) + + if (!isRunning) { + logger.debug(`No running container found with name "${containerName}"`) + // Still try to remove in case container exists but is stopped + await execa('docker', ['rm', '-f', containerName], { reject: false }) + return false + } + + logger.info(`Removing Docker container "${containerName}"...`) + + // Atomic force-remove: handles running and stopped containers + await execa('docker', ['rm', '-f', containerName], { reject: false }) + + logger.success(`Docker container "${containerName}" removed`) + return true + } + + /** + * Check if a named container is currently running. + * + * @param containerName - Name of the container to check + * @returns true if the container is running, false otherwise + */ + static async isContainerRunning(containerName: string): Promise { + try { + const result = await execa('docker', [ + 'ps', + '--filter', `name=^${containerName}$`, + '--format', '{{.Names}}', + ], { reject: false }) + + return result.exitCode === 0 && result.stdout.trim() === containerName + } catch { + return false + } + } + + /** + * Parse the EXPOSE directive from a Dockerfile. + * Returns the first exposed port number, or null if none found. + * + * Handles formats like: + * - EXPOSE 4200 + * - EXPOSE 4200/tcp + * - EXPOSE 8080/udp + * + * @param dockerfilePath - Absolute path to the Dockerfile + * @returns First exposed port number, or null if no EXPOSE directive found + */ + static async parseExposeFromDockerfile( + dockerfilePath: string + ): Promise { + try { + const content = await readFile(dockerfilePath, 'utf-8') + const match = /^EXPOSE\s+(\d+)/m.exec(content) + + if (match?.[1]) { + const port = parseInt(match[1], 10) + if (!isNaN(port) && port >= 1 && port <= 65535) { + return port + } + } + + return null + } catch { + return null + } + } + + /** + * Sanitize a string for use as a Docker container name. + * + * Docker container names must match: [a-zA-Z0-9][a-zA-Z0-9_.-] + * - Replace slashes, spaces, and other invalid characters with hyphens + * - Ensure the name starts with an alphanumeric character + * - Truncate to a maximum of 63 characters + * + * @param name - Raw name to sanitize + * @returns Sanitized container name + */ + static sanitizeContainerName(name: string): string { + // Replace any character that isn't alphanumeric, underscore, dot, or hyphen with a hyphen + let sanitized = name.replace(/[^a-zA-Z0-9_.-]/g, '-') + + // Collapse consecutive hyphens into one + sanitized = sanitized.replace(/-{2,}/g, '-') + + // Remove leading non-alphanumeric characters (Docker requires starting with [a-zA-Z0-9]) + sanitized = sanitized.replace(/^[^a-zA-Z0-9]+/, '') + + // Remove trailing hyphens + sanitized = sanitized.replace(/-+$/, '') + + // Truncate to max length + if (sanitized.length > MAX_CONTAINER_NAME_LENGTH) { + sanitized = sanitized.substring(0, MAX_CONTAINER_NAME_LENGTH) + // Remove trailing hyphen if truncation created one + sanitized = sanitized.replace(/-+$/, '') + } + + // Fallback if everything was stripped + if (sanitized.length === 0) { + sanitized = 'iloom-container' + } + + return sanitized + } + + /** + * Build the standard container name for an iloom dev server. + * + * @param identifier - Issue number, branch name, or other identifier + * @returns Sanitized container name in the format "iloom-dev-{identifier}" + */ + static buildContainerName(identifier: string | number): string { + return DockerManager.sanitizeContainerName(`iloom-dev-${identifier}`) + } + + /** + * Build the standard image name for an iloom dev server. + * + * @param identifier - Issue number, branch name, or other identifier + * @returns Sanitized image name + */ + static buildImageName(identifier: string | number): string { + return DockerManager.sanitizeContainerName(`iloom-dev-${identifier}`) + } + + /** + * Detect exposed ports from a built Docker image using `docker image inspect`. + * This is the source of truth for exposed ports as it handles multi-stage builds, + * inherited images, and runtime configuration that Dockerfile regex cannot capture. + * + * @param imageName - Name of the built Docker image to inspect + * @returns First exposed port number, or null if none found + */ + static async inspectImagePorts(imageName: string): Promise { + try { + const result = await execa('docker', [ + 'image', 'inspect', imageName, + '--format', '{{json .Config.ExposedPorts}}', + ], { reject: false }) + + if (result.exitCode !== 0 || !result.stdout.trim()) { + return null + } + + const output = result.stdout.trim() + // Output format: {"4200/tcp":{},"8080/tcp":{}} or null + if (output === 'null' || output === '' || output === '{}') { + return null + } + + const parsed: unknown = JSON.parse(output) + if (typeof parsed !== 'object' || parsed === null) { + return null + } + + // Get first key (e.g., "4200/tcp") and extract port number + const firstKey = Object.keys(parsed)[0] + if (!firstKey) { + return null + } + + const portMatch = /^(\d+)/.exec(firstKey) + if (portMatch?.[1]) { + const port = parseInt(portMatch[1], 10) + if (!isNaN(port) && port >= 1 && port <= 65535) { + return port + } + } + + return null + } catch { + return null + } + } + + /** + * Resolve the container port from config, image inspection, or Dockerfile EXPOSE directive. + * Priority: config > docker image inspect > Dockerfile regex + * Throws a clear error if no source provides a port. + * + * @param configPort - Port from settings (containerPort field) + * @param dockerfilePath - Absolute path to the Dockerfile + * @param imageName - Optional built image name for inspection (preferred over Dockerfile regex) + * @returns Resolved container port number + */ + static async resolveContainerPort( + configPort: number | undefined, + dockerfilePath: string, + imageName?: string + ): Promise { + if (configPort !== undefined) { + return configPort + } + + // Try image inspection first (most accurate - handles multi-stage builds, inherited images) + if (imageName) { + const inspectedPort = await DockerManager.inspectImagePorts(imageName) + if (inspectedPort !== null) { + logger.debug(`Auto-detected container port ${inspectedPort} from Docker image inspect`) + return inspectedPort + } + } + + // Fall back to Dockerfile regex + const exposedPort = await DockerManager.parseExposeFromDockerfile(dockerfilePath) + if (exposedPort !== null) { + logger.debug(`Auto-detected container port ${exposedPort} from Dockerfile EXPOSE directive`) + return exposedPort + } + + throw new Error( + `Cannot determine container port. No "containerPort" configured in settings and no EXPOSE directive found in ${dockerfilePath}.\n` + + 'Either add an EXPOSE directive to your Dockerfile or set "containerPort" in capabilities.web settings.' + ) + } + + /** + * Force-remove a container by name (no-op if it doesn't exist). + * Used internally before running a new container to avoid name collisions. + */ + private static async forceRemoveContainer(containerName: string): Promise { + await execa('docker', ['rm', '-f', containerName], { reject: false }) + } + + /** + * Build a DockerConfig from iloom web settings and an identifier. + * Centralizes the Docker config extraction logic used by dev-server, open, and run commands. + * + * @param webSettings - The capabilities.web section from IloomSettings + * @param identifier - Identifier for container naming (issue number, branch name, etc.) + * @returns DockerConfig if Docker mode is enabled, undefined otherwise + */ + static buildDockerConfigFromSettings( + webSettings: WebSettings | undefined, + identifier: string + ): DockerConfig | undefined { + if (!webSettings || webSettings.devServer !== 'docker') { + return undefined + } + + return { + dockerFile: webSettings.dockerFile ?? './Dockerfile', + containerPort: webSettings.containerPort, + dockerBuildArgs: webSettings.dockerBuildArgs, + dockerRunArgs: webSettings.dockerRunArgs, + identifier, + } + } +} diff --git a/src/lib/ResourceCleanup.test.ts b/src/lib/ResourceCleanup.test.ts index 34a95387..56ec2db8 100644 --- a/src/lib/ResourceCleanup.test.ts +++ b/src/lib/ResourceCleanup.test.ts @@ -4,6 +4,7 @@ import { GitWorktreeManager } from './GitWorktreeManager.js' import { DatabaseManager } from './DatabaseManager.js' import { ProcessManager } from './process/ProcessManager.js' import { SettingsManager } from './SettingsManager.js' +import { DockerManager } from './DockerManager.js' import type { GitWorktree } from '../types/worktree.js' import type { ResourceCleanupOptions } from '../types/cleanup.js' import { executeGitCommand, findMainWorktreePathWithSettings, hasUncommittedChanges, isBranchMergedIntoMain, checkRemoteBranchStatus, getMergeTargetBranch, findWorktreeForBranch } from '../utils/git.js' @@ -14,6 +15,7 @@ vi.mock('./GitWorktreeManager.js') vi.mock('./DatabaseManager.js') vi.mock('./process/ProcessManager.js') vi.mock('./SettingsManager.js') +vi.mock('./DockerManager.js') // Mock MetadataManager to prevent real file creation during tests vi.mock('./MetadataManager.js', () => ({ @@ -467,6 +469,215 @@ describe('ResourceCleanup', () => { /may still be running/ ) }) + + it('should stop Docker container when dockerIdentifier is provided and container is running', async () => { + vi.mocked(DockerManager.buildContainerName).mockReturnValue('iloom-dev-25') + vi.mocked(DockerManager.isContainerRunning).mockResolvedValueOnce(true) + vi.mocked(DockerManager.stopAndRemoveContainer).mockResolvedValueOnce(true) + + const result = await resourceCleanup.terminateDevServer(3025, 25) + + expect(DockerManager.buildContainerName).toHaveBeenCalledWith(25) + expect(DockerManager.isContainerRunning).toHaveBeenCalledWith('iloom-dev-25') + expect(DockerManager.stopAndRemoveContainer).toHaveBeenCalledWith('iloom-dev-25') + expect(result).toBe(true) + // Should NOT fall through to process detection + expect(mockProcessManager.detectDevServer).not.toHaveBeenCalled() + }) + + it('should fall back to process detection when dockerIdentifier is provided but no container found', async () => { + vi.mocked(DockerManager.buildContainerName).mockReturnValue('iloom-dev-25') + vi.mocked(DockerManager.isContainerRunning).mockResolvedValueOnce(false) + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce({ + pid: 12345, + name: 'node', + command: 'node next dev', + port: 3025, + isDevServer: true, + }) + vi.mocked(mockProcessManager.terminateProcess).mockResolvedValueOnce(true) + vi.mocked(mockProcessManager.verifyPortFree).mockResolvedValueOnce(true) + + const result = await resourceCleanup.terminateDevServer(3025, 25) + + expect(DockerManager.isContainerRunning).toHaveBeenCalledWith('iloom-dev-25') + expect(mockProcessManager.detectDevServer).toHaveBeenCalledWith(3025) + expect(result).toBe(true) + }) + + it('should return false when dockerIdentifier provided but no container and no process found', async () => { + vi.mocked(DockerManager.buildContainerName).mockReturnValue('iloom-dev-42') + vi.mocked(DockerManager.isContainerRunning).mockResolvedValueOnce(false) + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce(null) + + const result = await resourceCleanup.terminateDevServer(3042, 42) + + expect(DockerManager.isContainerRunning).toHaveBeenCalledWith('iloom-dev-42') + expect(result).toBe(false) + }) + + it('should not check Docker when no dockerIdentifier is provided', async () => { + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce(null) + + const result = await resourceCleanup.terminateDevServer(3025) + + expect(DockerManager.isContainerRunning).not.toHaveBeenCalled() + expect(DockerManager.stopAndRemoveContainer).not.toHaveBeenCalled() + expect(result).toBe(false) + }) + + it('should handle string dockerIdentifier (branch name)', async () => { + vi.mocked(DockerManager.buildContainerName).mockReturnValue('iloom-dev-feat-issue-25') + vi.mocked(DockerManager.isContainerRunning).mockResolvedValueOnce(true) + vi.mocked(DockerManager.stopAndRemoveContainer).mockResolvedValueOnce(true) + + const result = await resourceCleanup.terminateDevServer(3025, 'feat/issue-25') + + expect(DockerManager.buildContainerName).toHaveBeenCalledWith('feat/issue-25') + expect(result).toBe(true) + }) + }) + + describe('cleanupWorktree - Docker mode', () => { + const mockWorktree: GitWorktree = { + path: '/path/to/worktree', + branch: 'feat/issue-25', + commit: 'abc123', + bare: false, + detached: false, + locked: false, + } + + it('should pass dockerIdentifier to terminateDevServer when Docker mode is configured', async () => { + // Configure Docker mode in settings + mockSettingsManager.loadSettings = vi.fn().mockResolvedValue({ + capabilities: { + web: { + basePort: 3000, + devServer: 'docker', + }, + }, + }) + + vi.mocked(mockGitWorktree.findWorktreeForIssue).mockResolvedValueOnce(mockWorktree) + vi.mocked(mockProcessManager.calculatePort).mockReturnValue(3025) + + // Docker container is found and removed + vi.mocked(DockerManager.buildContainerName).mockReturnValue('iloom-dev-25') + vi.mocked(DockerManager.isContainerRunning).mockResolvedValueOnce(true) + vi.mocked(DockerManager.stopAndRemoveContainer).mockResolvedValueOnce(true) + + vi.mocked(mockGitWorktree.removeWorktree).mockResolvedValueOnce(undefined) + + const parsedInput = { + type: 'issue' as const, + number: 25, + originalInput: 'issue-25', + } + + const result = await resourceCleanup.cleanupWorktree(parsedInput, { + keepDatabase: true, + }) + + expect(result.success).toBe(true) + expect(DockerManager.buildContainerName).toHaveBeenCalledWith('25') + expect(DockerManager.isContainerRunning).toHaveBeenCalledWith('iloom-dev-25') + expect(DockerManager.stopAndRemoveContainer).toHaveBeenCalledWith('iloom-dev-25') + expect(result.operations[0]?.type).toBe('dev-server') + expect(result.operations[0]?.success).toBe(true) + expect(result.operations[0]?.message).toContain('terminated') + }) + + it('should NOT pass dockerIdentifier when process mode is configured', async () => { + // Configure process mode (default) + mockSettingsManager.loadSettings = vi.fn().mockResolvedValue({ + capabilities: { + web: { + basePort: 3000, + devServer: 'process', + }, + }, + }) + + vi.mocked(mockGitWorktree.findWorktreeForIssue).mockResolvedValueOnce(mockWorktree) + vi.mocked(mockProcessManager.calculatePort).mockReturnValue(3025) + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce(null) + vi.mocked(mockGitWorktree.removeWorktree).mockResolvedValueOnce(undefined) + + const parsedInput = { + type: 'issue' as const, + number: 25, + originalInput: 'issue-25', + } + + await resourceCleanup.cleanupWorktree(parsedInput, { + keepDatabase: true, + }) + + // Docker methods should NOT be called + expect(DockerManager.isContainerRunning).not.toHaveBeenCalled() + expect(DockerManager.stopAndRemoveContainer).not.toHaveBeenCalled() + }) + + it('should NOT pass dockerIdentifier when devServer setting is not configured (defaults to process)', async () => { + // Settings without devServer field - should default to 'process' + mockSettingsManager.loadSettings = vi.fn().mockResolvedValue({}) + + vi.mocked(mockGitWorktree.findWorktreeForIssue).mockResolvedValueOnce(mockWorktree) + vi.mocked(mockProcessManager.calculatePort).mockReturnValue(3025) + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce(null) + vi.mocked(mockGitWorktree.removeWorktree).mockResolvedValueOnce(undefined) + + const parsedInput = { + type: 'issue' as const, + number: 25, + originalInput: 'issue-25', + } + + await resourceCleanup.cleanupWorktree(parsedInput, { + keepDatabase: true, + }) + + // Docker methods should NOT be called + expect(DockerManager.isContainerRunning).not.toHaveBeenCalled() + }) + + it('should fall back to process cleanup when Docker mode is configured but no container found', async () => { + // Configure Docker mode in settings + mockSettingsManager.loadSettings = vi.fn().mockResolvedValue({ + capabilities: { + web: { + basePort: 3000, + devServer: 'docker', + }, + }, + }) + + vi.mocked(mockGitWorktree.findWorktreeForIssue).mockResolvedValueOnce(mockWorktree) + vi.mocked(mockProcessManager.calculatePort).mockReturnValue(3025) + + // Docker container not found + vi.mocked(DockerManager.buildContainerName).mockReturnValue('iloom-dev-25') + vi.mocked(DockerManager.isContainerRunning).mockResolvedValueOnce(false) + + // Falls back to process detection - no process found either + vi.mocked(mockProcessManager.detectDevServer).mockResolvedValueOnce(null) + + vi.mocked(mockGitWorktree.removeWorktree).mockResolvedValueOnce(undefined) + + const parsedInput = { + type: 'issue' as const, + number: 25, + originalInput: 'issue-25', + } + + const result = await resourceCleanup.cleanupWorktree(parsedInput, { + keepDatabase: true, + }) + + expect(result.success).toBe(true) + expect(result.operations[0]?.message).toContain('No dev server running') + }) }) describe('deleteBranch', () => { diff --git a/src/lib/ResourceCleanup.ts b/src/lib/ResourceCleanup.ts index b081c159..566e0f11 100644 --- a/src/lib/ResourceCleanup.ts +++ b/src/lib/ResourceCleanup.ts @@ -5,6 +5,7 @@ import { ProcessManager } from './process/ProcessManager.js' import { CLIIsolationManager } from './CLIIsolationManager.js' import { SettingsManager } from './SettingsManager.js' import { MetadataManager } from './MetadataManager.js' +import { DockerManager } from './DockerManager.js' import { getLogger } from '../utils/logger-context.js' import { hasUncommittedChanges, executeGitCommand, findMainWorktreePathWithSettings, extractIssueNumber, isBranchMergedIntoMain, checkRemoteBranchStatus, getMergeTargetBranch, findWorktreeForBranch, type RemoteBranchStatus } from '../utils/git.js' import { calculatePortFromIdentifier } from '../utils/port.js' @@ -56,15 +57,20 @@ export class ResourceCleanup { const displayIdentifier = parsed.branchName ?? parsed.number?.toString() ?? parsed.originalInput getLogger().info(`Starting cleanup for: ${displayIdentifier}`) - // Extract number from ParsedInput for port calculation - const number = parsed.number + // Derive identifier for port calculation and Docker container lookup + // Uses the same fallback logic as dev-server.ts: number > branchName > originalInput + const portIdentifier = parsed.number ?? parsed.branchName ?? parsed.originalInput // Step 1: Terminate dev server if applicable - if (number !== undefined) { - // Load settings to get basePort + { + // Load settings to get basePort and Docker mode const settings = await this.settingsManager.loadSettings() const basePort = settings?.capabilities?.web?.basePort ?? 3000 - const port = calculatePortFromIdentifier(number, basePort) + const port = calculatePortFromIdentifier(portIdentifier, basePort) + const devServerMode = settings?.capabilities?.web?.devServer ?? 'process' + const dockerIdentifier = devServerMode === 'docker' + ? (parsed.number?.toString() ?? parsed.branchName ?? parsed.originalInput) + : undefined if (options.dryRun) { operations.push({ @@ -74,7 +80,7 @@ export class ResourceCleanup { }) } else { try { - const terminated = await this.terminateDevServer(port) + const terminated = await this.terminateDevServer(port, dockerIdentifier) operations.push({ type: 'dev-server', success: true, @@ -472,11 +478,29 @@ export class ResourceCleanup { } /** - * Terminate dev server on specified port + * Terminate dev server on specified port. + * When a dockerIdentifier is provided, attempts Docker container cleanup first + * before falling back to process-based detection. + * + * @param port - Port the dev server is running on + * @param dockerIdentifier - Optional identifier for Docker container lookup (issue number, branch name, etc.) */ - async terminateDevServer(port: number): Promise { + async terminateDevServer(port: number, dockerIdentifier?: string | number): Promise { getLogger().debug(`Checking for dev server on port ${port}`) + // Try Docker container cleanup first if identifier provided + if (dockerIdentifier !== undefined) { + const containerName = DockerManager.buildContainerName(dockerIdentifier) + const isRunning = await DockerManager.isContainerRunning(containerName) + if (isRunning) { + getLogger().info(`Terminating Docker container: ${containerName}`) + await DockerManager.stopAndRemoveContainer(containerName) + return true + } + getLogger().debug(`No Docker container found with name "${containerName}", falling back to process detection`) + } + + // Existing process-based detection const processInfo = await this.processManager.detectDevServer(port) if (!processInfo) { diff --git a/src/lib/SettingsManager.ts b/src/lib/SettingsManager.ts index 9afd4d59..f0d7348f 100644 --- a/src/lib/SettingsManager.ts +++ b/src/lib/SettingsManager.ts @@ -181,6 +181,28 @@ export const CapabilitiesSettingsSchema = z .max(65535, 'Base port must be <= 65535') .optional() .describe('Base port for web workspace port calculations (default: 3000)'), + devServer: z + .enum(['process', 'docker']) + .default('process') + .describe('Dev server mode: "process" runs natively, "docker" runs inside a Docker container with port mapping'), + dockerFile: z + .string() + .default('./Dockerfile') + .describe('Path to Dockerfile relative to worktree root (only used when devServer is "docker")'), + containerPort: z + .number() + .min(1, 'Container port must be >= 1') + .max(65535, 'Container port must be <= 65535') + .optional() + .describe('Port the app runs on inside the Docker container (auto-detected from EXPOSE directive if not set)'), + dockerBuildArgs: z + .record(z.string()) + .optional() + .describe('Build arguments to pass to docker build (e.g., {"NODE_ENV": "development"})'), + dockerRunArgs: z + .array(z.string()) + .optional() + .describe('Additional arguments for docker run (e.g., ["-v", "./src:/app/src"] for volume mounts)'), }) .optional(), database: z @@ -214,6 +236,28 @@ export const CapabilitiesSettingsSchemaNoDefaults = z .max(65535, 'Base port must be <= 65535') .optional() .describe('Base port for web workspace port calculations (default: 3000)'), + devServer: z + .enum(['process', 'docker']) + .optional() + .describe('Dev server mode: "process" runs natively, "docker" runs inside a Docker container with port mapping'), + dockerFile: z + .string() + .optional() + .describe('Path to Dockerfile relative to worktree root (only used when devServer is "docker")'), + containerPort: z + .number() + .min(1, 'Container port must be >= 1') + .max(65535, 'Container port must be <= 65535') + .optional() + .describe('Port the app runs on inside the Docker container (auto-detected from EXPOSE directive if not set)'), + dockerBuildArgs: z + .record(z.string()) + .optional() + .describe('Build arguments to pass to docker build (e.g., {"NODE_ENV": "development"})'), + dockerRunArgs: z + .array(z.string()) + .optional() + .describe('Additional arguments for docker run (e.g., ["-v", "./src:/app/src"] for volume mounts)'), }) .optional(), database: z