fix(create): resolve Dockerfile paths against the invocation cwd, not the project root#1144
fix(create): resolve Dockerfile paths against the invocation cwd, not the project root#1144aidandaly24 wants to merge 4 commits intomainfrom
Conversation
The BYO/Container 'agentcore create' wizard previously resolved user-entered Dockerfile paths against the project root (where agentcore.json lives), causing 'not found' errors when users specified a path relative to the directory they ran the command from. Two related fixes: 1. AddAgentScreen PathInput basePath now uses getWorkingDirectory() instead of <projectRoot>/<codeLocation>, so completion + validation match the user's invocation directory. This mirrors the pattern used by 'agentcore policy add --source' and CodePathScreen. 2. The wizard no longer strips the user-entered path via basename() at submit time. The full path is preserved so that the downstream copy logic in useAddAgent can resolve it. basename() is now applied at the persistence/render boundary instead. 3. useAddAgent now resolves Dockerfile sources against getWorkingDirectory() (not projectRoot) and broadens the path detection to include absolute paths. The same submit-time basename() bug is also fixed in GenerateWizardUI so the 'agentcore generate --dockerfile <path>' flow actually copies the file as intended. Fixes #1128
Extract the path-resolution logic for user-provided Dockerfile paths into a small pure helper so it can be unit-tested without mocking ConfigIO, telemetry, credential primitives, etc. Tests cover the cases that motivated issue #1128: - bare filename -> no copy (existing in place) - relative path -> resolved against the invocation cwd, not projectRoot - absolute path -> used verbatim - parent-relative paths - empty/undefined input
[MEDIUM] GenerateWizardUI: pass basePath={getWorkingDirectory()} to the
Dockerfile PathInput so completion/validation/downstream resolution
all agree on the invocation cwd (previously the PathInput defaulted
to process.cwd() while resolveDockerfileSource resolved against
INIT_CWD, causing 'not found' errors under npm/bun script wrappers).
[LOW] useAddAgent.resolveDockerfileSource: detect both forward- and
back-slash separators when classifying a value as a path vs bare
filename, so Windows users entering 'subdir\Dockerfile' aren't
silently treated as referencing a non-existent bare file.
[LOW] useAddAgent.handleCreatePath: resolve+normalize generateConfig
.dockerfile to its basename BEFORE calling
mapGenerateConfigToRenderConfig so render templates that interpolate
{{dockerfile}} never see a host path string. The actual file copy
still happens after render so the template default is overwritten
correctly.
[LOW] useAddAgent.handleByoPath: when the user supplies a bare filename
(no copy path), surface a clear error if that file is not present
in the code directory, instead of letting it fail later at deploy
time with a less helpful message.
[LOW] types.ts (agent + generate): expand the JSDoc on the dockerfile
field to document its dual lifecycle (full path during the wizard,
basename after handle*Path normalization).
[LOW] resolveDockerfileSource.test.ts:
- Replace tautological default-cwd test with vi.stubEnv('INIT_CWD',
'/sentinel/...') so the test actually exercises getWorkingDirectory.
- Add a second case stubbing INIT_CWD to undefined to cover the
process.cwd() fallback.
- Add Windows-style separator tests covering 'subdir\\Dockerfile'
and '.\\sub\\My.Dockerfile' to lock in cross-platform behavior.
[MEDIUM] resolveDockerfileSource.test.ts (Windows separators): Tightened the cross-platform-detection describe block to clarify it locks in *classification* (shouldCopy) only — basename extraction on Windows is the responsibility of node's platform-default `path` module. Added a new test that uses path.win32 directly to document the expected end-to-end basename behavior on Windows hosts, so a future regression in win32 path handling would be caught. [MEDIUM] handleByoPath bare-filename strict check: Reverted the new `else if (dockerfileName) -> existsSync(codeDir/...)` branch added in round 1. While well-intentioned, it broke the legitimate workflow where a user adds the agent config first and produces / places the Dockerfile in a separate step. The deploy/build path will still surface a clear error if the file is missing at that time. Added an explanatory NOTE comment so future contributors don't re-introduce the check. [MEDIUM] handleByoPath / handleCreatePath error-path test coverage: Extracted a new `validateDockerfileSource(value, cwd?, fileExists?)` helper that bundles `resolveDockerfileSource` + the existsSync gate used by both call sites, with an injectable `fileExists` predicate for testing without fs mocks. Both handle*Path functions now delegate to the helper, eliminating duplicated logic. Added 7 new unit tests covering: success path, missing-source error path (asserts the exact user-visible 'Dockerfile not found at <sourcePath>' message), bare-filename short-circuit (verifies fileExists is NOT called), absolute-path success+failure, and a regression guard ensuring the error message references the cwd-resolved path (issue #1128). [LOW] resolveDockerfileSource.test.ts (INIT_CWD handling): - Replaced the `vi.stubEnv('INIT_CWD', undefined as unknown as string)` pattern with explicit beforeEach/afterEach save+restore via `delete process.env.INIT_CWD`, which doesn't depend on the vitest deletion-via-undefined contract. - Added a new test documenting that INIT_CWD='' (empty string) is honored by getWorkingDirectory()'s `??` semantics rather than falling through to process.cwd(). [LOW] GenerateWizardUI summary: Now displays `basename(config.dockerfile)` instead of the raw value (which post-fix may be a long absolute or relative host path). Mirrors the equivalent normalization already in AddAgentScreen.tsx for UX consistency. Re-imported `basename` from 'path'.
Coverage Report
|
|
The wizard-side changes in Trace
Why this breaks post-PRBefore the PR, In
So for This seems inconsistent with the manual verification step #1 in the PR description ("file is copied into Options
Option (1) is probably the smallest-blast-radius fix; (2) or (3) would be better long-term. Also, consider adding at least one test exercising the |
Description
Fixes the
agentcore createBYO/Container wizard so that the Dockerfile path is resolved against the directory the user ranagentcorefrom, rather than the (newly-created) project root. Today, when a user runsagentcore createfrom~/work/.github/and then types a relative Dockerfile path during the "Add a Dockerfile" step, the wizard validates against<projectRoot>/<codeLocation>/instead of the invocation cwd, producing a confusing "not a valid path" error and (even on accept) silently dropping the user's path so the file is never copied.This PR mirrors the existing pattern used by
agentcore policy add --source <path>(andCodePathScreen, etc.) where user-entered paths are resolved against the invocation directory viagetWorkingDirectory().Changes
src/cli/tui/screens/agent/AddAgentScreen.tsxPathInputnow usesbasePath={getWorkingDirectory()}(was<projectRoot>/<codeLocation>), so completion + validation match the user's invocation cwd.onSubmitno longer strips the user-entered path viabasename(); the full path flows through touseAddAgentwhich handles resolution and copy.src/cli/tui/screens/agent/useAddAgent.tsresolveDockerfileSource(value, cwd?)pure helper: classifies a user-entered value as either a path-to-copy (absolute, or contains/or\) or a bare filename. Resolves relative paths againstgetWorkingDirectory()by default.validateDockerfileSource(value, cwd?, fileExists?)helper: bundlesresolveDockerfileSource+ theexistsSyncgate that bothhandleCreatePathandhandleByoPathperform. Accepts an injectablefileExistspredicate so the not-found error path is unit-testable without mockingfs.handleCreatePathnow resolves+normalizesgenerateConfig.dockerfileto a basename before callingmapGenerateConfigToRenderConfig, so future templates that interpolate{{dockerfile}}never receive a host path. The actual file copy still happens after the renderer writes its template default, so the override behavior is preserved.handleByoPathnow uses the validated helper; reverted an over-eager round-1 check that pre-validated bare-filename references againstcodeDir(it broke the legitimate "add agent first, place Dockerfile later" workflow). The deploy/build path will still surface a clear error if the file is missing at that time./and\so Windows users enteringsubdir\Dockerfilearen't silently misclassified as referencing a bare filename.src/cli/tui/screens/generate/GenerateWizardUI.tsxPathInputnow usesbasePath={getWorkingDirectory()}(matching the AddAgentScreen change).onSubmitno longer strips viabasename()— that pre-existing behavior meantagentcore generate --dockerfile <path>silently dropped the path before the copy logic could run.basename(config.dockerfile)for UX parity.src/cli/tui/screens/agent/types.tsandsrc/cli/tui/screens/generate/types.tsdockerfilefield documenting its dual lifecycle (full path during the wizard → basename afterhandleCreatePath/handleByoPathmutation).Tests added:
src/cli/tui/screens/agent/__tests__/resolveDockerfileSource.test.ts(22 tests)resolveDockerfileSource: undefined/empty input, bare filenames, relative/nested/parent paths, absolute paths, basename extraction, cross-platform separator detection (including apath.win32end-to-end case),INIT_CWDset/unset/empty-string semantics.validateDockerfileSource: success path, missing-source error path (asserts the exactDockerfile not found at <sourcePath>user-visible message — regression guard for The path to dockerfile should be fixed to where theagentcorecommand was run #1128), bare-filename short-circuit (verifiesfileExistsis not called), absolute-path success and failure cases, plus an explicit regression guard that the error message references the cwd-resolved path (not the user-typed input).Related Issue
Closes #1128
Documentation PR
N/A — no user-facing documentation changes.
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integ(targeted unit tests for the changed files; CI will run the full suite)npm run typechecknpm run lint(via pre-commit prettier + eslint hooks)src/assets/, I rannpm run test:update-snapshotsand committed the updated snapshots (N/A — no asset changes)Targeted test files run locally:
src/cli/tui/screens/agent/__tests__/resolveDockerfileSource.test.ts— 22/22 pass (new)src/cli/tui/screens/agent/__tests__/computeByoSteps.test.ts— 4/4 passsrc/cli/tui/screens/generate/__tests__/useGenerateWizard.test.tsx— 36/36 passsrc/cli/tui/components/__tests__/PathInput.test.tsx— passManual verification
Dockerfile.testfile, runagentcore create→ BYO + Container → "Add a Dockerfile" → type./Dockerfile.test. Wizard accepts; file is copied into<newProject>/app/<agent>/Dockerfile.test;agentcore.jsonreferencesDockerfile.test.agentcore generate --dockerfile ./relative/path— file is copied as intended (parallel fix in the generate wizard).Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.