From 6071732dcbdfe1c8416304e2e17a8b1764fc8133 Mon Sep 17 00:00:00 2001 From: agentcore-cli-bot Date: Thu, 7 May 2026 17:50:16 +0000 Subject: [PATCH 1/2] fix: resolve Dockerfile path relative to cwd in agent BYO wizard (#1128) The 'add Dockerfile' step in the agent BYO wizard was rooting its PathInput at projectRoot+codeLocation (e.g. /app//), which often did not exist yet and produced 'not a valid path' errors for files the user could see in the directory they invoked agentcore from. The reporter requested that this match the 'add policy' wizard, which resolves paths from process.cwd(). Changes: - AddAgentScreen: drop the basePath override on the dockerfile PathInput (now defaults to process.cwd()) and stop collapsing the user input to basename() prematurely so the downstream copy step can find the source file. - useAddAgent (handleCreatePath, handleByoPath): resolve the dockerfile source from process.cwd() instead of projectRoot. - GenerateWizardUI: stop calling basename() on submit so the existing copy logic in handleCreatePath becomes reachable for user-supplied Dockerfiles. Fixes aws/agentcore-cli#1128 --- src/cli/tui/screens/agent/AddAgentScreen.tsx | 11 ++++++++--- src/cli/tui/screens/agent/useAddAgent.ts | 12 ++++++++---- src/cli/tui/screens/generate/GenerateWizardUI.tsx | 7 +++++-- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/cli/tui/screens/agent/AddAgentScreen.tsx b/src/cli/tui/screens/agent/AddAgentScreen.tsx index c8961c065..13c953144 100644 --- a/src/cli/tui/screens/agent/AddAgentScreen.tsx +++ b/src/cli/tui/screens/agent/AddAgentScreen.tsx @@ -45,7 +45,6 @@ import { } from './types'; import { Box, Text, useInput } from 'ink'; import Spinner from 'ink-spinner'; -import { basename, resolve } from 'path'; import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; // Helper to get provider display name and env var name from ModelProvider @@ -1086,12 +1085,18 @@ export function AddAgentScreen({ existingAgentNames, onComplete, onExit }: AddAg {byoStep === 'dockerfile' && ( { - setByoConfig(c => ({ ...c, dockerfile: value ? basename(value) : '' })); + // Preserve the full user-supplied path so handleByoPath can + // locate and copy the source file. The file is collapsed to a + // basename only after a successful copy into the code dir. + setByoConfig(c => ({ ...c, dockerfile: value })); goToNextByoStep('dockerfile'); }} onCancel={handleByoBack} diff --git a/src/cli/tui/screens/agent/useAddAgent.ts b/src/cli/tui/screens/agent/useAddAgent.ts index e2fabe140..1c1e5444a 100644 --- a/src/cli/tui/screens/agent/useAddAgent.ts +++ b/src/cli/tui/screens/agent/useAddAgent.ts @@ -260,9 +260,11 @@ async function handleCreatePath( const renderer = createRenderer(renderConfig); await renderer.render({ outputDir: projectRoot }); - // If dockerfile is a path (contains /), copy it into the agent directory (overwriting template default) + // If dockerfile is a path (contains /), copy it into the agent directory (overwriting template default). + // The path is interpreted relative to the directory the user invoked agentcore from (process.cwd()), + // matching the behavior of the "add policy" file picker (issue #1128). if (generateConfig.dockerfile?.includes('/')) { - const sourcePath = resolve(projectRoot, generateConfig.dockerfile); + const sourcePath = resolve(process.cwd(), generateConfig.dockerfile); if (!existsSync(sourcePath)) { return { ok: false, error: `Dockerfile not found at ${sourcePath}` }; } @@ -369,10 +371,12 @@ async function handleByoPath( const codeDir = join(projectRoot, config.codeLocation.replace(/\/$/, '')); mkdirSync(codeDir, { recursive: true }); - // If dockerfile is a path (contains /), copy it into the code directory and use the filename + // If dockerfile is a path (contains /), copy it into the code directory and use the filename. + // The user-supplied path is resolved relative to process.cwd() — the directory the user + // invoked agentcore from — matching the "add policy" file picker (issue #1128). let dockerfileName = config.dockerfile; if (dockerfileName?.includes('/')) { - const sourcePath = resolve(projectRoot, dockerfileName); + const sourcePath = resolve(process.cwd(), dockerfileName); if (!existsSync(sourcePath)) { return { ok: false, error: `Dockerfile not found at ${sourcePath}` }; } diff --git a/src/cli/tui/screens/generate/GenerateWizardUI.tsx b/src/cli/tui/screens/generate/GenerateWizardUI.tsx index 9c6c79599..c2339cc5e 100644 --- a/src/cli/tui/screens/generate/GenerateWizardUI.tsx +++ b/src/cli/tui/screens/generate/GenerateWizardUI.tsx @@ -36,7 +36,6 @@ import { } from './types'; import type { useGenerateWizard } from './useGenerateWizard'; import { Box, Text, useInput } from 'ink'; -import { basename } from 'path'; // Helper to get provider display name and env var name from ModelProvider function getProviderInfo(provider: ModelProvider): { name: string; envVarName: string } { @@ -234,7 +233,11 @@ export function GenerateWizardUI({ allowEmpty emptyHelpText="Press Enter to use the default Dockerfile" onSubmit={value => { - wizard.setDockerfile(value ? basename(value) : undefined); + // Preserve the full user-supplied path so handleCreatePath can + // locate and copy the source file. The path is collapsed to a + // basename only after a successful copy into the agent dir + // (issue #1128). + wizard.setDockerfile(value || undefined); }} onCancel={onBack} /> From a5db80e70c4c63a4426f30b6c300e54492a76e72 Mon Sep 17 00:00:00 2001 From: agentcore-cli-bot Date: Thu, 7 May 2026 17:56:03 +0000 Subject: [PATCH 2/2] test: add resolveDockerfileSource helper + regression tests for #1128 Extract the Dockerfile path-resolution logic into a small exported helper so it can be unit-tested without spinning up the BYO wizard's state machine. Both handleCreatePath and handleByoPath now delegate to the helper. Adds 9 regression tests covering: - empty/undefined/bare-filename inputs (no-op) - relative paths resolved against cwd (the fix) - ./ prefix and absolute paths - explicit assertion that resolution does NOT use a project root (regression guard for #1128) --- .../__tests__/resolveDockerfileSource.test.ts | 66 +++++++++++++++++++ src/cli/tui/screens/agent/useAddAgent.ts | 48 ++++++++++---- 2 files changed, 102 insertions(+), 12 deletions(-) create mode 100644 src/cli/tui/screens/agent/__tests__/resolveDockerfileSource.test.ts diff --git a/src/cli/tui/screens/agent/__tests__/resolveDockerfileSource.test.ts b/src/cli/tui/screens/agent/__tests__/resolveDockerfileSource.test.ts new file mode 100644 index 000000000..2f69342fd --- /dev/null +++ b/src/cli/tui/screens/agent/__tests__/resolveDockerfileSource.test.ts @@ -0,0 +1,66 @@ +import { resolveDockerfileSource } from '../useAddAgent'; +import { resolve } from 'path'; +import { describe, expect, it } from 'vitest'; + +/** + * Regression tests for issue #1128: the Dockerfile picker in the agent BYO + * wizard must resolve user-supplied paths relative to the directory the user + * invoked `agentcore` from, NOT relative to the (possibly not-yet-created) + * project's code directory. + */ +describe('resolveDockerfileSource', () => { + it('returns null for undefined (no dockerfile selected)', () => { + expect(resolveDockerfileSource(undefined)).toBeNull(); + }); + + it('returns null for empty string (user pressed Enter for default)', () => { + expect(resolveDockerfileSource('')).toBeNull(); + }); + + it('returns null for a bare filename (no slash)', () => { + // A bare filename means the file is expected to already live inside the + // build context's code directory; no copy is needed. + expect(resolveDockerfileSource('Dockerfile')).toBeNull(); + expect(resolveDockerfileSource('Dockerfile.dev')).toBeNull(); + }); + + it('resolves a relative path against the supplied cwd, not against any project root', () => { + const cwd = '/home/user/workplace/.github'; + expect(resolveDockerfileSource('docker/MyDockerfile.dev', cwd)).toBe(resolve(cwd, 'docker/MyDockerfile.dev')); + }); + + it('resolves a nested relative path against the supplied cwd', () => { + const cwd = '/home/user/workplace/.github'; + expect(resolveDockerfileSource('subdir/MyDockerfile', cwd)).toBe(resolve(cwd, 'subdir/MyDockerfile')); + }); + + it('resolves "./Dockerfile" against the supplied cwd', () => { + const cwd = '/home/user/workplace/.github'; + // "./Dockerfile" contains "/", so it is treated as a real path, not a + // bare filename. resolve() collapses the "./" segment. + expect(resolveDockerfileSource('./Dockerfile', cwd)).toBe(resolve(cwd, 'Dockerfile')); + }); + + it('preserves absolute paths (they are not re-rooted)', () => { + const cwd = '/home/user/workplace/.github'; + const abs = '/tmp/elsewhere/Dockerfile'; + expect(resolveDockerfileSource(abs, cwd)).toBe(abs); + }); + + it('defaults to process.cwd() when no cwd argument is supplied', () => { + const original = process.cwd(); + expect(resolveDockerfileSource('subdir/Dockerfile')).toBe(resolve(original, 'subdir/Dockerfile')); + }); + + it('does NOT resolve relative to a project root (regression for #1128)', () => { + // The bug: previously the picker rooted at /, + // e.g. /home/user/.github/myproj/app/myagent/. A user typing + // "MyDockerfile" from /home/user/.github would have it looked up at + // /home/user/.github/myproj/app/myagent/MyDockerfile — wrong. + const cwd = '/home/user/workplace/.github'; + const resolved = resolveDockerfileSource('subdir/MyDockerfile', cwd); + expect(resolved).toBe('/home/user/workplace/.github/subdir/MyDockerfile'); + expect(resolved).not.toContain('/app/'); + expect(resolved).not.toContain('/myproj/'); + }); +}); diff --git a/src/cli/tui/screens/agent/useAddAgent.ts b/src/cli/tui/screens/agent/useAddAgent.ts index 1c1e5444a..3d9ea0fa3 100644 --- a/src/cli/tui/screens/agent/useAddAgent.ts +++ b/src/cli/tui/screens/agent/useAddAgent.ts @@ -59,6 +59,30 @@ export interface AddAgentError { export type AddAgentOutcome = AddAgentCreateResult | AddAgentByoResult | AddAgentError; +// ───────────────────────────────────────────────────────────────────────────── +// Dockerfile source resolution +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Resolve the absolute source path of a user-supplied Dockerfile. + * + * The path is interpreted relative to the directory the user invoked + * `agentcore` from (process.cwd()), matching the "add policy" file picker + * (issue #1128). Absolute paths are returned unchanged. A `null` return + * value means the caller should not attempt to copy the file (the user + * either left the field empty, indicating they want the default + * Dockerfile, or supplied a bare filename indicating the file already + * lives inside the build context). + */ +export function resolveDockerfileSource(dockerfile: string | undefined, cwd: string = process.cwd()): string | null { + // Empty / undefined / bare-filename inputs do not need resolution: empty + // means "use default", bare filename means "already in code dir". + if (!dockerfile?.includes('/')) { + return null; + } + return resolve(cwd, dockerfile); +} + // ───────────────────────────────────────────────────────────────────────────── // Config Mappers // ───────────────────────────────────────────────────────────────────────────── @@ -263,13 +287,13 @@ async function handleCreatePath( // If dockerfile is a path (contains /), copy it into the agent directory (overwriting template default). // The path is interpreted relative to the directory the user invoked agentcore from (process.cwd()), // matching the behavior of the "add policy" file picker (issue #1128). - if (generateConfig.dockerfile?.includes('/')) { - const sourcePath = resolve(process.cwd(), generateConfig.dockerfile); - if (!existsSync(sourcePath)) { - return { ok: false, error: `Dockerfile not found at ${sourcePath}` }; + const createSourcePath = resolveDockerfileSource(generateConfig.dockerfile); + if (createSourcePath !== null) { + if (!existsSync(createSourcePath)) { + return { ok: false, error: `Dockerfile not found at ${createSourcePath}` }; } - const filename = basename(sourcePath); - copyFileSync(sourcePath, join(agentPath, filename)); + const filename = basename(createSourcePath); + copyFileSync(createSourcePath, join(agentPath, filename)); generateConfig.dockerfile = filename; } @@ -375,13 +399,13 @@ async function handleByoPath( // The user-supplied path is resolved relative to process.cwd() — the directory the user // invoked agentcore from — matching the "add policy" file picker (issue #1128). let dockerfileName = config.dockerfile; - if (dockerfileName?.includes('/')) { - const sourcePath = resolve(process.cwd(), dockerfileName); - if (!existsSync(sourcePath)) { - return { ok: false, error: `Dockerfile not found at ${sourcePath}` }; + const byoSourcePath = resolveDockerfileSource(dockerfileName); + if (byoSourcePath !== null) { + if (!existsSync(byoSourcePath)) { + return { ok: false, error: `Dockerfile not found at ${byoSourcePath}` }; } - dockerfileName = basename(sourcePath); - copyFileSync(sourcePath, join(codeDir, dockerfileName)); + dockerfileName = basename(byoSourcePath); + copyFileSync(byoSourcePath, join(codeDir, dockerfileName)); } const project = await configIO.readProjectSpec();