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/__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 e2fabe140..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 // ───────────────────────────────────────────────────────────────────────────── @@ -260,14 +284,16 @@ 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 (generateConfig.dockerfile?.includes('/')) { - const sourcePath = resolve(projectRoot, generateConfig.dockerfile); - if (!existsSync(sourcePath)) { - return { ok: false, error: `Dockerfile not found at ${sourcePath}` }; + // 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). + 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; } @@ -369,15 +395,17 @@ 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); - 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(); 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} />