From 987a5098de5b257bb69dbfe19bbd630259fb5eb6 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Mon, 10 Aug 2026 14:17:25 -0400 Subject: [PATCH 1/3] feat(dev): add container dev runner --- src/core/dev/container.test.ts | 524 +++++++++++++++++++++++++++++++++ src/core/dev/container.ts | 202 +++++++++++++ 2 files changed, 726 insertions(+) create mode 100644 src/core/dev/container.test.ts create mode 100644 src/core/dev/container.ts diff --git a/src/core/dev/container.test.ts b/src/core/dev/container.test.ts new file mode 100644 index 000000000..fda2e3005 --- /dev/null +++ b/src/core/dev/container.test.ts @@ -0,0 +1,524 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { InputValidationError } from "../../errors"; +import type { DevEvent, DevServerInput } from "../../handlers/project/dev/types"; +import { + MissingToolError, + type ProcessEvent, + type ProcessStreamer, + type StreamProcessOptions, +} from "../../io"; +import type { ProjectRuntime } from "../project/schema"; +import { ContainerDevRunner } from "./container"; + +type ProcessCall = { + command: string[]; + options: StreamProcessOptions; +}; + +type StreamBehavior = ( + command: string[], + options: StreamProcessOptions, +) => AsyncIterable | Iterable; + +const tempDirectories: string[] = []; + +afterEach(async () => { + await Promise.all( + tempDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true })), + ); +}); + +type RuntimeOverrides = Partial> & { + buildContextPath?: string; + dockerfile?: string; +}; + +function runtime(overrides: RuntimeOverrides = {}): ProjectRuntime { + return { + name: "Hello_World", + build: "Container", + entrypoint: "main.py", + codeLocation: "app/hello-world", + protocol: "HTTP", + ...overrides, + } as ProjectRuntime; +} + +async function projectRoot(projectRuntime: ProjectRuntime = runtime()): Promise { + const root = await mkdtemp(join(tmpdir(), "agentcore-container-")); + tempDirectories.push(root); + const context = join(root, projectRuntime.buildContextPath ?? projectRuntime.codeLocation); + await mkdir(context, { recursive: true }); + await writeFile(join(context, projectRuntime.dockerfile ?? "Dockerfile"), "FROM scratch\n"); + return root; +} + +function harness( + config: { + available?: (tool: string, probeArgs?: string[]) => Promise; + stream?: StreamBehavior; + } = {}, +) { + const calls: ProcessCall[] = []; + const fakeStreamProcess: ProcessStreamer = async function* (command, options) { + calls.push({ command, options }); + if (config.stream) yield* config.stream(command, options); + }; + return { + calls, + runner: new ContainerDevRunner({ + streamProcess: fakeStreamProcess, + toolAvailable: + config.available ?? + (async (tool) => { + return tool === "docker"; + }), + }), + }; +} + +function input( + root: string, + projectRuntime: ProjectRuntime, + signal = new AbortController().signal, +): DevServerInput { + return { + runtime: projectRuntime, + projectRoot: root, + port: 3000, + env: { API_KEY: "super-secret" }, + signal, + }; +} + +async function collect(events: AsyncIterable): Promise { + const collected: DevEvent[] = []; + for await (const event of events) collected.push(event); + return collected; +} + +async function* rejectedEvents(error: unknown): AsyncGenerator { + yield* []; + throw error; +} + +function projectIdentifier(projectRoot: string): string { + return createHash("sha256").update(resolve(projectRoot)).digest("hex").slice(0, 12); +} + +function imageTag(projectRoot: string): string { + return `agentcore-dev/hello_world-${projectIdentifier(projectRoot)}`; +} + +function containerName(projectRoot: string): string { + return `agentcore-dev-hello_world-${projectIdentifier(projectRoot)}`; +} + +function commandCall(calls: ProcessCall[], operation: "build" | "run"): ProcessCall { + const call = calls.find(({ command }) => command[1] === operation); + if (!call) throw new Error(`${operation} command was not called`); + return call; +} + +describe("ContainerDevRunner", () => { + test("builds with a widened context and keeps build arg values out of argv", async () => { + const projectRuntime = runtime({ + buildContextPath: ".", + dockerfile: "docker/Dockerfile", + customDockerBuildArgs: { AGENT_NAME: "hello-world", TARGET: "development" }, + }); + const root = await mkdtemp(join(tmpdir(), "agentcore-container-")); + tempDirectories.push(root); + await mkdir(join(root, "docker"), { recursive: true }); + await writeFile(join(root, "docker", "Dockerfile"), "FROM scratch\n"); + const { calls, runner } = harness(); + + await collect(runner.run(input(root, projectRuntime))); + + const build = commandCall(calls, "build"); + expect(build.command).toEqual([ + "docker", + "build", + "-f", + "docker/Dockerfile", + "-t", + imageTag(root), + "--build-arg", + "AGENT_NAME", + "--build-arg", + "TARGET", + ".", + ]); + expect(build.options.cwd).toBe(root); + expect(build.options.env).toMatchObject({ + AGENT_NAME: "hello-world", + TARGET: "development", + }); + expect(build.command.join(" ")).not.toContain("hello-world"); + expect(build.command.join(" ")).not.toContain("development"); + + const dockerignore = await readFile(join(root, ".dockerignore"), "utf8"); + for (const pattern of [".env", "**/.env", "**/node_modules", "agentcore/"]) { + expect(dockerignore).toContain(pattern); + } + }); + + test.each([ + ["HTTP", 8080, "HTTP"], + ["MCP", 8000, "MCP"], + ["A2A", 9000, "A2A"], + ["AGUI", 8080, "AGUI"], + ["the default protocol", 8080, undefined], + ] as const)("maps %s to container port %d", async (_label, containerPort, protocol) => { + const projectRuntime = runtime({ protocol }); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness(); + + await collect(runner.run(input(root, projectRuntime))); + + const run = commandCall(calls, "run"); + expect(run.command).toEqual([ + "docker", + "run", + "--rm", + "--name", + containerName(root), + "-p", + `127.0.0.1:3000:${containerPort}`, + "-e", + "API_KEY", + "-e", + "PORT", + "-e", + "LOCAL_DEV", + ...(protocol === "MCP" ? ["-e", "FASTMCP_PORT"] : []), + imageTag(root), + ]); + expect(run.options.env).toMatchObject({ + API_KEY: "super-secret", + PORT: String(containerPort), + LOCAL_DEV: "1", + }); + expect(run.options.env?.FASTMCP_PORT).toBe(protocol === "MCP" ? "8000" : undefined); + expect(run.command.join(" ")).not.toContain("super-secret"); + }); + + test("preserves an existing build context .dockerignore", async () => { + const projectRuntime = runtime({ buildContextPath: "." }); + const root = await projectRoot(projectRuntime); + const dockerignore = join(root, ".dockerignore"); + await writeFile(dockerignore, "# user owned\ncustom-pattern\n"); + const { runner } = harness(); + + await collect(runner.run(input(root, projectRuntime))); + + expect(await readFile(dockerignore, "utf8")).toBe("# user owned\ncustom-pattern\n"); + }); + + test("scopes image and container names to the project root", async () => { + const projectRuntime = runtime(); + const firstRoot = await projectRoot(projectRuntime); + const secondRoot = await projectRoot(projectRuntime); + const first = harness(); + const second = harness(); + + await collect(first.runner.run(input(firstRoot, projectRuntime))); + await collect(second.runner.run(input(secondRoot, projectRuntime))); + + expect(commandCall(first.calls, "build").command).toContain(imageTag(firstRoot)); + expect(commandCall(second.calls, "build").command).toContain(imageTag(secondRoot)); + expect(imageTag(firstRoot)).not.toBe(imageTag(secondRoot)); + expect(first.calls[0]?.command).toContain(containerName(firstRoot)); + expect(second.calls[0]?.command).toContain(containerName(secondRoot)); + expect(containerName(firstRoot)).not.toBe(containerName(secondRoot)); + }); + + test("selects the first tool that supports container builds", async () => { + const probes: Array<[string, string[] | undefined]> = []; + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness({ + available: async (tool, probeArgs) => { + probes.push([tool, probeArgs]); + return tool === "podman"; + }, + }); + + await collect(runner.run(input(root, projectRuntime))); + + expect(probes).toEqual([ + ["docker", undefined], + ["podman", undefined], + ["podman", ["build", "--help"]], + ]); + expect(commandCall(calls, "build").command[0]).toBe("podman"); + expect(commandCall(calls, "run").command[0]).toBe("podman"); + }); + + test("skips a version shim that cannot build", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness({ + available: async (tool, probeArgs) => { + if (tool === "docker") return probeArgs === undefined; + return tool === "podman"; + }, + }); + + await collect(runner.run(input(root, projectRuntime))); + + expect(commandCall(calls, "build").command[0]).toBe("podman"); + }); + + test("falls back to finch when docker and podman are unavailable", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness({ + available: async (tool) => tool === "finch", + }); + + await collect(runner.run(input(root, projectRuntime))); + + expect(commandCall(calls, "build").command[0]).toBe("finch"); + }); + + test("throws a useful error when no container runtime is available", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { runner } = harness({ available: async () => false }); + + const promise = collect(runner.run(input(root, projectRuntime))); + + await expect(promise).rejects.toBeInstanceOf(MissingToolError); + await expect(promise).rejects.toThrow(/Docker.*Podman.*Finch/); + }); + + test("does not probe tools or mutate the project when already aborted", async () => { + const projectRuntime = runtime({ buildContextPath: "." }); + const root = await projectRoot(projectRuntime); + const controller = new AbortController(); + controller.abort(); + const probes: string[] = []; + const { calls, runner } = harness({ + available: async (tool) => { + probes.push(tool); + return true; + }, + }); + + await expect( + collect(runner.run(input(root, projectRuntime, controller.signal))), + ).rejects.toMatchObject({ name: "AbortError" }); + + expect(probes).toHaveLength(0); + expect(calls).toHaveLength(0); + await expect(readFile(join(root, ".dockerignore"), "utf8")).rejects.toThrow(); + }); + + test("stops after tool detection when aborted during probing", async () => { + const projectRuntime = runtime({ buildContextPath: "." }); + const root = await projectRoot(projectRuntime); + const controller = new AbortController(); + const probes: string[] = []; + const { calls, runner } = harness({ + available: async (tool) => { + probes.push(tool); + controller.abort(); + return false; + }, + }); + + await expect( + collect(runner.run(input(root, projectRuntime, controller.signal))), + ).rejects.toMatchObject({ name: "AbortError" }); + + expect(probes).toEqual(["docker"]); + expect(calls).toHaveLength(0); + await expect(readFile(join(root, ".dockerignore"), "utf8")).rejects.toThrow(); + }); + + test("rejects a build context that is not a directory", async () => { + const root = await mkdtemp(join(tmpdir(), "agentcore-container-")); + tempDirectories.push(root); + await mkdir(join(root, "app"), { recursive: true }); + await writeFile(join(root, "app", "hello-world"), "not a directory"); + const probes: string[] = []; + const { calls, runner } = harness({ + available: async (tool) => { + probes.push(tool); + return true; + }, + }); + + const promise = collect(runner.run(input(root, runtime()))); + + await expect(promise).rejects.toBeInstanceOf(InputValidationError); + await expect(promise).rejects.toThrow(/build context directory not found/); + expect(probes).toHaveLength(0); + expect(calls).toHaveLength(0); + }); + + test("rejects a Dockerfile that is not a file", async () => { + const projectRuntime = runtime(); + const root = await mkdtemp(join(tmpdir(), "agentcore-container-")); + tempDirectories.push(root); + await mkdir(join(root, projectRuntime.codeLocation, "Dockerfile"), { recursive: true }); + const probes: string[] = []; + const { calls, runner } = harness({ + available: async (tool) => { + probes.push(tool); + return true; + }, + }); + + const promise = collect(runner.run(input(root, projectRuntime))); + + await expect(promise).rejects.toBeInstanceOf(InputValidationError); + await expect(promise).rejects.toThrow(/Dockerfile not found/); + expect(probes).toHaveLength(0); + expect(calls).toHaveLength(0); + }); + + test("removes stale containers and cleans up after a normal exit", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness(); + const runInput = input(root, projectRuntime); + + await collect(runner.run(runInput)); + + expect(calls.map(({ command }) => command.slice(0, 3))).toEqual([ + ["docker", "rm", "-f"], + ["docker", "build", "-f"], + ["docker", "run", "--rm"], + ["docker", "rm", "-f"], + ]); + expect(calls[0]?.command).toEqual(["docker", "rm", "-f", containerName(root)]); + expect(calls[3]?.command).toEqual(calls[0]!.command); + expect(calls[0]?.options.signal).not.toBe(runInput.signal); + expect(calls[1]?.options.signal).toBe(runInput.signal); + expect(calls[2]?.options.signal).toBe(runInput.signal); + expect(calls[3]?.options.signal).not.toBe(runInput.signal); + }); + + test("cleans up when the container process fails", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness({ + stream: (command) => { + return command[1] === "run" ? rejectedEvents(new Error("container failed")) : []; + }, + }); + + await expect(collect(runner.run(input(root, projectRuntime)))).rejects.toThrow( + "container failed", + ); + + expect(calls.filter(({ command }) => command[1] === "rm")).toHaveLength(2); + }); + + test("cleans up when container execution is aborted", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const controller = new AbortController(); + const { calls, runner } = harness({ + stream: (command, options) => { + if (command[1] === "run") { + controller.abort(); + return rejectedEvents(options.signal?.reason); + } + return []; + }, + }); + + await expect( + collect(runner.run(input(root, projectRuntime, controller.signal))), + ).rejects.toMatchObject({ name: "AbortError" }); + + expect(calls.filter(({ command }) => command[1] === "rm")).toHaveLength(2); + }); + + test("cleans up when the consumer stops iterating", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + let runIteratorClosed = false; + const { calls, runner } = harness({ + stream: (command) => { + if (command[1] !== "run") return []; + return (async function* () { + try { + yield { type: "stdout", line: "container ready" } as const; + } finally { + runIteratorClosed = true; + } + })(); + }, + }); + const iterator = runner.run(input(root, projectRuntime)); + + expect(await iterator.next()).toMatchObject({ + value: { type: "status", message: "Building image with docker" }, + }); + expect(await iterator.next()).toMatchObject({ + value: { type: "status", message: "Starting container" }, + }); + expect(await iterator.next()).toMatchObject({ + value: { type: "stdout", line: "container ready" }, + }); + await iterator.return(undefined); + + expect(runIteratorClosed).toBe(true); + expect(calls.filter(({ command }) => command[1] === "rm")).toHaveLength(2); + }); + + test("ignores stale and final cleanup failures", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness({ + stream: (command) => { + return command[1] === "rm" ? rejectedEvents(new Error("container missing")) : []; + }, + }); + + await expect(collect(runner.run(input(root, projectRuntime)))).resolves.toBeDefined(); + expect(commandCall(calls, "run")).toBeDefined(); + expect(calls.filter(({ command }) => command[1] === "rm")).toHaveLength(2); + }); + + test("does not run a container after a failed build", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness({ + stream: (command) => { + return command[1] === "build" ? rejectedEvents(new Error("build failed")) : []; + }, + }); + + await expect(collect(runner.run(input(root, projectRuntime)))).rejects.toThrow("build failed"); + expect(calls.some(({ command }) => command[1] === "run")).toBe(false); + }); + + test("interleaves status, build output, and run output", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { runner } = harness({ + stream: async function* (command) { + if (command[1] === "build") yield { type: "stdout", line: "build output" }; + if (command[1] === "run") yield { type: "stderr", line: "run output" }; + }, + }); + + const events = await collect(runner.run(input(root, projectRuntime))); + + expect(events).toEqual([ + { type: "status", message: "Building image with docker" }, + { type: "stdout", line: "build output" }, + { type: "status", message: "Starting container" }, + { type: "stderr", line: "run output" }, + ]); + }); +}); diff --git a/src/core/dev/container.ts b/src/core/dev/container.ts new file mode 100644 index 000000000..b2f0c6333 --- /dev/null +++ b/src/core/dev/container.ts @@ -0,0 +1,202 @@ +import { createHash } from "node:crypto"; +import { existsSync, statSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { InputValidationError } from "../../errors"; +import type { DevEvent, DevRunner, DevServerInput } from "../../handlers/project/dev/types"; +import { + MissingToolError, + streamProcess, + toolAvailable, + type ProcessStreamer, + type StreamProcessOptions, +} from "../../io"; + +const CONTAINER_TOOLS = ["docker", "podman", "finch"] as const; +const CLEANUP_TIMEOUT_MS = 2_000; +const DOCKERFILE_NAME = "Dockerfile"; +const CONTAINER_RUNTIME_INSTALL_HINT = + "Install Docker (https://docs.docker.com/get-docker/), Podman (https://podman.io/), " + + "or Finch (https://runfinch.com/)."; + +const BUILD_CONTEXT_DOCKERIGNORE = `# Generated by agentcore because buildContextPath widens the Docker build context. +.env +.env.* +**/.env +**/.env.* +.git +**/.git +.venv +**/.venv +node_modules +**/node_modules +__pycache__ +**/__pycache__ +.pytest_cache +**/.pytest_cache +.DS_Store +**/.DS_Store +agentcore/ +`; + +type ContainerTool = (typeof CONTAINER_TOOLS)[number]; +type ToolAvailable = typeof toolAvailable; + +type ContainerDevRunnerConfig = { + streamProcess?: ProcessStreamer; + toolAvailable?: ToolAvailable; +}; + +export class ContainerDevRunner implements DevRunner { + private readonly streamProcess: ProcessStreamer; + private readonly toolAvailable: ToolAvailable; + + constructor(config: ContainerDevRunnerConfig = {}) { + this.streamProcess = config.streamProcess ?? streamProcess; + this.toolAvailable = config.toolAvailable ?? toolAvailable; + } + + public async *run(input: DevServerInput): AsyncGenerator { + input.signal.throwIfAborted(); + const context = join( + input.projectRoot, + input.runtime.buildContextPath ?? input.runtime.codeLocation, + ); + if (!isDirectory(context)) { + throw new InputValidationError(`container build context directory not found: ${context}`); + } + + const dockerfile = input.runtime.dockerfile ?? DOCKERFILE_NAME; + const dockerfilePath = join(context, dockerfile); + if (!isFile(dockerfilePath)) { + throw new InputValidationError(`container Dockerfile not found: ${dockerfilePath}`); + } + + const tool = await this.resolveContainerTool(input.signal); + input.signal.throwIfAborted(); + if (input.runtime.buildContextPath) { + const dockerignore = ensureBuildContextDockerignore(context); + if (dockerignore) { + yield { type: "status", message: `Created protective ${dockerignore}` }; + } + } + + const runtimeName = input.runtime.name.toLowerCase(); + const projectId = projectIdentifier(input.projectRoot); + const imageTag = `agentcore-dev/${runtimeName}-${projectId}`; + const containerName = `agentcore-dev-${runtimeName}-${projectId}`; + input.signal.throwIfAborted(); + await this.removeContainer(tool, containerName, context); + + const buildArgs = input.runtime.customDockerBuildArgs ?? {}; + const buildArgFlags = Object.keys(buildArgs).flatMap((key) => ["--build-arg", key]); + const buildOptions: StreamProcessOptions = { + cwd: context, + env: { ...process.env, ...buildArgs }, + signal: input.signal, + }; + + yield { type: "status", message: `Building image with ${tool}` }; + yield* this.streamProcess( + [tool, "build", "-f", dockerfile, "-t", imageTag, ...buildArgFlags, "."], + buildOptions, + ); + + const containerPort = portForProtocol(input.runtime.protocol); + const forwardedEnv: Record = { + ...input.env, + PORT: String(containerPort), + LOCAL_DEV: "1", + }; + if (input.runtime.protocol === "MCP") { + forwardedEnv.FASTMCP_PORT = String(containerPort); + } + const envNameFlags = Object.keys(forwardedEnv).flatMap((key) => ["-e", key]); + + yield { type: "status", message: "Starting container" }; + try { + yield* this.streamProcess( + [ + tool, + "run", + "--rm", + "--name", + containerName, + "-p", + `127.0.0.1:${input.port}:${containerPort}`, + ...envNameFlags, + imageTag, + ], + { + cwd: context, + env: { ...process.env, ...forwardedEnv }, + signal: input.signal, + }, + ); + } finally { + await this.removeContainer(tool, containerName, context); + } + } + + private async resolveContainerTool(signal: AbortSignal): Promise { + for (const tool of CONTAINER_TOOLS) { + const hasVersion = await this.toolAvailable(tool); + signal.throwIfAborted(); + if (!hasVersion) continue; + + const canBuild = await this.toolAvailable(tool, ["build", "--help"]); + signal.throwIfAborted(); + if (canBuild) return tool; + } + throw new MissingToolError("container runtime", CONTAINER_RUNTIME_INSTALL_HINT); + } + + private async removeContainer( + tool: ContainerTool, + containerName: string, + cwd: string, + ): Promise { + try { + for await (const _event of this.streamProcess([tool, "rm", "-f", containerName], { + cwd, + signal: AbortSignal.timeout(CLEANUP_TIMEOUT_MS), + })) { + // Best-effort cleanup intentionally discards command output. + } + } catch { + // A missing container and an unavailable daemon are both safe to ignore here. + } + } +} + +function portForProtocol(protocol: DevServerInput["runtime"]["protocol"]): number { + if (protocol === "MCP") return 8000; + if (protocol === "A2A") return 9000; + return 8080; +} + +function isDirectory(path: string): boolean { + try { + return statSync(path).isDirectory(); + } catch { + return false; + } +} + +function isFile(path: string): boolean { + try { + return statSync(path).isFile(); + } catch { + return false; + } +} + +function ensureBuildContextDockerignore(context: string): string | undefined { + const dockerignore = join(context, ".dockerignore"); + if (existsSync(dockerignore)) return undefined; + writeFileSync(dockerignore, BUILD_CONTEXT_DOCKERIGNORE); + return dockerignore; +} + +function projectIdentifier(projectRoot: string): string { + return createHash("sha256").update(resolve(projectRoot)).digest("hex").slice(0, 12); +} From 67438879d7604ec395115de7b6d8f08119ae76c6 Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Tue, 11 Aug 2026 16:42:32 -0400 Subject: [PATCH 2/3] fix(dev): address container runner review --- src/core/dev/container.test.ts | 29 +++++++++++++++++++++++++---- src/core/dev/container.ts | 8 ++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/core/dev/container.test.ts b/src/core/dev/container.test.ts index fda2e3005..15b078b17 100644 --- a/src/core/dev/container.test.ts +++ b/src/core/dev/container.test.ts @@ -106,16 +106,16 @@ async function* rejectedEvents(error: unknown): AsyncGenerator { await expect(readFile(join(root, ".dockerignore"), "utf8")).rejects.toThrow(); }); + test("does not build when aborted during stale container cleanup", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const controller = new AbortController(); + const { calls, runner } = harness({ + stream: async function* (command) { + if (command[1] === "rm") { + await Promise.resolve(); + controller.abort(); + } + yield* []; + }, + }); + + await expect( + collect(runner.run(input(root, projectRuntime, controller.signal))), + ).rejects.toMatchObject({ name: "AbortError" }); + + expect(calls.map(({ command }) => command[1])).toEqual(["rm"]); + }); + test("rejects a build context that is not a directory", async () => { const root = await mkdtemp(join(tmpdir(), "agentcore-container-")); tempDirectories.push(root); diff --git a/src/core/dev/container.ts b/src/core/dev/container.ts index b2f0c6333..c50720c9e 100644 --- a/src/core/dev/container.ts +++ b/src/core/dev/container.ts @@ -81,11 +81,11 @@ export class ContainerDevRunner implements DevRunner { } const runtimeName = input.runtime.name.toLowerCase(); - const projectId = projectIdentifier(input.projectRoot); + const projectId = hashString(resolve(input.projectRoot)); const imageTag = `agentcore-dev/${runtimeName}-${projectId}`; const containerName = `agentcore-dev-${runtimeName}-${projectId}`; - input.signal.throwIfAborted(); await this.removeContainer(tool, containerName, context); + input.signal.throwIfAborted(); const buildArgs = input.runtime.customDockerBuildArgs ?? {}; const buildArgFlags = Object.keys(buildArgs).flatMap((key) => ["--build-arg", key]); @@ -197,6 +197,6 @@ function ensureBuildContextDockerignore(context: string): string | undefined { return dockerignore; } -function projectIdentifier(projectRoot: string): string { - return createHash("sha256").update(resolve(projectRoot)).digest("hex").slice(0, 12); +function hashString(value: string): string { + return createHash("sha256").update(value).digest("hex").slice(0, 12); } From 113dcdee7153789e21ce0f6e78f1df4f929d391e Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Wed, 12 Aug 2026 09:04:46 -0400 Subject: [PATCH 3/3] fix(dev): address container runtime feedback --- src/core/dev/container.test.ts | 89 ++++++++++++++++++++++++++-------- src/core/dev/container.ts | 76 ++++++++++++++++++++++------- src/io/exec.test.ts | 14 ++++++ src/io/exec.ts | 11 +++-- 4 files changed, 147 insertions(+), 43 deletions(-) diff --git a/src/core/dev/container.test.ts b/src/core/dev/container.test.ts index 15b078b17..3767c88aa 100644 --- a/src/core/dev/container.test.ts +++ b/src/core/dev/container.test.ts @@ -3,7 +3,7 @@ import { createHash } from "node:crypto"; import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; -import { InputValidationError } from "../../errors"; +import { InputValidationError, InvalidEnvironmentError } from "../../errors"; import type { DevEvent, DevServerInput } from "../../handlers/project/dev/types"; import { MissingToolError, @@ -125,7 +125,7 @@ function commandCall(calls: ProcessCall[], operation: "build" | "run"): ProcessC } describe("ContainerDevRunner", () => { - test("builds with a widened context and keeps build arg values out of argv", async () => { + test("builds with a widened context and redacts build arg values from errors", async () => { const projectRuntime = runtime({ buildContextPath: ".", dockerfile: "docker/Dockerfile", @@ -148,18 +148,17 @@ describe("ContainerDevRunner", () => { "-t", imageTag(root), "--build-arg", - "AGENT_NAME", + "AGENT_NAME=hello-world", "--build-arg", - "TARGET", + "TARGET=development", ".", ]); expect(build.options.cwd).toBe(root); - expect(build.options.env).toMatchObject({ - AGENT_NAME: "hello-world", - TARGET: "development", - }); - expect(build.command.join(" ")).not.toContain("hello-world"); - expect(build.command.join(" ")).not.toContain("development"); + expect(build.options.env).toBe(process.env); + expect(build.options.redactedCommand).toContain("AGENT_NAME="); + expect(build.options.redactedCommand).toContain("TARGET="); + expect(build.options.redactedCommand?.join(" ")).not.toContain("hello-world"); + expect(build.options.redactedCommand?.join(" ")).not.toContain("development"); const dockerignore = await readFile(join(root, ".dockerignore"), "utf8"); for (const pattern of [".env", "**/.env", "**/node_modules", "agentcore/"]) { @@ -190,21 +189,17 @@ describe("ContainerDevRunner", () => { "-p", `127.0.0.1:3000:${containerPort}`, "-e", - "API_KEY", + "API_KEY=super-secret", "-e", - "PORT", + `PORT=${containerPort}`, "-e", - "LOCAL_DEV", - ...(protocol === "MCP" ? ["-e", "FASTMCP_PORT"] : []), + "LOCAL_DEV=1", + ...(protocol === "MCP" ? ["-e", "FASTMCP_PORT=8000"] : []), imageTag(root), ]); - expect(run.options.env).toMatchObject({ - API_KEY: "super-secret", - PORT: String(containerPort), - LOCAL_DEV: "1", - }); - expect(run.options.env?.FASTMCP_PORT).toBe(protocol === "MCP" ? "8000" : undefined); - expect(run.command.join(" ")).not.toContain("super-secret"); + expect(run.options.env).toBe(process.env); + expect(run.options.redactedCommand).toContain("API_KEY="); + expect(run.options.redactedCommand?.join(" ")).not.toContain("super-secret"); }); test("preserves an existing build context .dockerignore", async () => { @@ -237,6 +232,33 @@ describe("ContainerDevRunner", () => { expect(containerName(firstRoot)).not.toBe(containerName(secondRoot)); }); + test("limits image names to two consecutive underscores", async () => { + const projectRuntime = runtime({ name: "Hello___World" }); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness(); + + await collect(runner.run(input(root, projectRuntime))); + + expect(commandCall(calls, "build").command).toContain( + `agentcore-dev/hello__world-${hashString(resolve(root))}`, + ); + }); + + test("keeps app variables out of the container CLI environment", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness(); + const runInput = input(root, projectRuntime); + runInput.env = { ...runInput.env, DOCKER_HOST: "tcp://application-value" }; + + await collect(runner.run(runInput)); + + const run = commandCall(calls, "run"); + expect(run.command).toContain("DOCKER_HOST=tcp://application-value"); + expect(run.options.env).toBe(process.env); + expect(run.options.redactedCommand).toContain("DOCKER_HOST="); + }); + test("selects the first tool that supports container builds", async () => { const probes: Array<[string, string[] | undefined]> = []; const projectRuntime = runtime(); @@ -286,6 +308,31 @@ describe("ContainerDevRunner", () => { expect(commandCall(calls, "build").command[0]).toBe("finch"); }); + test("passes explicit build arg values to finch", async () => { + const projectRuntime = runtime({ customDockerBuildArgs: { AGENT_NAME: "hello-world" } }); + const root = await projectRoot(projectRuntime); + const { calls, runner } = harness({ + available: async (tool) => tool === "finch", + }); + + await collect(runner.run(input(root, projectRuntime))); + + expect(commandCall(calls, "build").command).toContain("AGENT_NAME=hello-world"); + }); + + test("suggests initializing the Finch VM when its build probe fails", async () => { + const projectRuntime = runtime(); + const root = await projectRoot(projectRuntime); + const { runner } = harness({ + available: async (tool, probeArgs) => tool === "finch" && probeArgs === undefined, + }); + + const promise = collect(runner.run(input(root, projectRuntime))); + + await expect(promise).rejects.toBeInstanceOf(InvalidEnvironmentError); + await expect(promise).rejects.toThrow("finch vm init"); + }); + test("throws a useful error when no container runtime is available", async () => { const projectRuntime = runtime(); const root = await projectRoot(projectRuntime); diff --git a/src/core/dev/container.ts b/src/core/dev/container.ts index c50720c9e..720d6e8dd 100644 --- a/src/core/dev/container.ts +++ b/src/core/dev/container.ts @@ -1,7 +1,7 @@ import { createHash } from "node:crypto"; import { existsSync, statSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; -import { InputValidationError } from "../../errors"; +import { InputValidationError, InvalidEnvironmentError } from "../../errors"; import type { DevEvent, DevRunner, DevServerInput } from "../../handlers/project/dev/types"; import { MissingToolError, @@ -82,24 +82,39 @@ export class ContainerDevRunner implements DevRunner { const runtimeName = input.runtime.name.toLowerCase(); const projectId = hashString(resolve(input.projectRoot)); - const imageTag = `agentcore-dev/${runtimeName}-${projectId}`; + const imageTag = `agentcore-dev/${sanitizeImageNameComponent(runtimeName)}-${projectId}`; const containerName = `agentcore-dev-${runtimeName}-${projectId}`; await this.removeContainer(tool, containerName, context); input.signal.throwIfAborted(); const buildArgs = input.runtime.customDockerBuildArgs ?? {}; - const buildArgFlags = Object.keys(buildArgs).flatMap((key) => ["--build-arg", key]); + const buildArgFlags = Object.entries(buildArgs).flatMap(([key, value]) => [ + "--build-arg", + `${key}=${value}`, + ]); + const redactedBuildArgFlags = Object.keys(buildArgs).flatMap((key) => [ + "--build-arg", + `${key}=`, + ]); + const buildCommand = [tool, "build", "-f", dockerfile, "-t", imageTag, ...buildArgFlags, "."]; const buildOptions: StreamProcessOptions = { cwd: context, - env: { ...process.env, ...buildArgs }, + env: process.env, + redactedCommand: [ + tool, + "build", + "-f", + dockerfile, + "-t", + imageTag, + ...redactedBuildArgFlags, + ".", + ], signal: input.signal, }; yield { type: "status", message: `Building image with ${tool}` }; - yield* this.streamProcess( - [tool, "build", "-f", dockerfile, "-t", imageTag, ...buildArgFlags, "."], - buildOptions, - ); + yield* this.streamProcess(buildCommand, buildOptions); const containerPort = portForProtocol(input.runtime.protocol); const forwardedEnv: Record = { @@ -110,12 +125,32 @@ export class ContainerDevRunner implements DevRunner { if (input.runtime.protocol === "MCP") { forwardedEnv.FASTMCP_PORT = String(containerPort); } - const envNameFlags = Object.keys(forwardedEnv).flatMap((key) => ["-e", key]); + const envFlags = Object.entries(forwardedEnv).flatMap(([key, value]) => [ + "-e", + `${key}=${value}`, + ]); + const redactedEnvFlags = Object.keys(forwardedEnv).flatMap((key) => [ + "-e", + `${key}=`, + ]); + const runCommand = [ + tool, + "run", + "--rm", + "--name", + containerName, + "-p", + `127.0.0.1:${input.port}:${containerPort}`, + ...envFlags, + imageTag, + ]; yield { type: "status", message: "Starting container" }; try { - yield* this.streamProcess( - [ + yield* this.streamProcess(runCommand, { + cwd: context, + env: process.env, + redactedCommand: [ tool, "run", "--rm", @@ -123,15 +158,11 @@ export class ContainerDevRunner implements DevRunner { containerName, "-p", `127.0.0.1:${input.port}:${containerPort}`, - ...envNameFlags, + ...redactedEnvFlags, imageTag, ], - { - cwd: context, - env: { ...process.env, ...forwardedEnv }, - signal: input.signal, - }, - ); + signal: input.signal, + }); } finally { await this.removeContainer(tool, containerName, context); } @@ -146,6 +177,11 @@ export class ContainerDevRunner implements DevRunner { const canBuild = await this.toolAvailable(tool, ["build", "--help"]); signal.throwIfAborted(); if (canBuild) return tool; + if (tool === "finch") { + throw new InvalidEnvironmentError( + "Finch is installed but its VM is not initialized. Run 'finch vm init' and retry.", + ); + } } throw new MissingToolError("container runtime", CONTAINER_RUNTIME_INSTALL_HINT); } @@ -200,3 +236,7 @@ function ensureBuildContextDockerignore(context: string): string | undefined { function hashString(value: string): string { return createHash("sha256").update(value).digest("hex").slice(0, 12); } + +function sanitizeImageNameComponent(value: string): string { + return value.replace(/_{3,}/g, "__"); +} diff --git a/src/io/exec.test.ts b/src/io/exec.test.ts index a4b9cc433..d20a8a090 100644 --- a/src/io/exec.test.ts +++ b/src/io/exec.test.ts @@ -111,6 +111,20 @@ describe("streamProcess", () => { await expect(iterator.next()).rejects.toThrow(/exit code 3/); }); + test("redacts sensitive command arguments from process errors", async () => { + const failing = await script("stream-redacted-fail.js", "process.exit(3)"); + const iterator = streamProcess(["node", failing, "super-secret"], { + cwd: process.cwd(), + redactedCommand: ["node", failing, ""], + }); + + const error = await iterator.next().catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ProcessFailedError); + expect(String(error)).toContain(""); + expect(String(error)).not.toContain("super-secret"); + }); + test("throws ProcessFailedError when the executable cannot spawn", async () => { await expect( collect(streamProcess(["definitely-not-a-real-tool-xyz"], { cwd: process.cwd() })), diff --git a/src/io/exec.ts b/src/io/exec.ts index 1a05ac6d3..94ad19f59 100644 --- a/src/io/exec.ts +++ b/src/io/exec.ts @@ -66,6 +66,8 @@ export type StreamProcessOptions = { cwd: string; env?: NodeJS.ProcessEnv; signal?: AbortSignal; + /** Command rendered in errors when the actual arguments contain sensitive values. */ + redactedCommand?: string[]; /** Required on Windows for command scripts such as npm.cmd. */ shell?: boolean; }; @@ -115,8 +117,9 @@ export async function* streamProcess( options: StreamProcessOptions, ): AsyncGenerator { const [executable, ...args] = command; + const errorCommand = options.redactedCommand ?? command; if (!executable) { - throw new ProcessFailedError(command, options.cwd, null, "command is empty"); + throw new ProcessFailedError(errorCommand, options.cwd, null, "command is empty"); } if (options.signal?.aborted) throw abortReason(options.signal); @@ -130,7 +133,7 @@ export async function* streamProcess( detached: !useShell, }); } catch (error) { - throw new ProcessFailedError(command, options.cwd, null, String(error)); + throw new ProcessFailedError(errorCommand, options.cwd, null, String(error)); } const events: ProcessEvent[] = []; @@ -203,12 +206,12 @@ export async function* streamProcess( if (options.signal?.aborted) throw abortReason(options.signal); if (spawnError) { - throw new ProcessFailedError(command, options.cwd, null, String(spawnError)); + throw new ProcessFailedError(errorCommand, options.cwd, null, String(spawnError)); } if (exitCode !== 0) { const signalMessage = exitSignal ? `terminated by ${exitSignal}` : ""; throw new ProcessFailedError( - command, + errorCommand, options.cwd, exitCode, [...recentOutput, signalMessage].filter(Boolean).join("\n"),