Skip to content

fix(create): resolve Dockerfile paths against the invocation cwd, not the project root#1144

Open
aidandaly24 wants to merge 4 commits intomainfrom
fix/1128-c86de5c9
Open

fix(create): resolve Dockerfile paths against the invocation cwd, not the project root#1144
aidandaly24 wants to merge 4 commits intomainfrom
fix/1128-c86de5c9

Conversation

@aidandaly24
Copy link
Copy Markdown
Contributor

Description

Fixes the agentcore create BYO/Container wizard so that the Dockerfile path is resolved against the directory the user ran agentcore from, rather than the (newly-created) project root. Today, when a user runs agentcore create from ~/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> (and CodePathScreen, etc.) where user-entered paths are resolved against the invocation directory via getWorkingDirectory().

Changes

src/cli/tui/screens/agent/AddAgentScreen.tsx

  • Dockerfile PathInput now uses basePath={getWorkingDirectory()} (was <projectRoot>/<codeLocation>), so completion + validation match the user's invocation cwd.
  • onSubmit no longer strips the user-entered path via basename(); the full path flows through to useAddAgent which handles resolution and copy.
  • Confirmation summary now displays only the basename (UX consistency).

src/cli/tui/screens/agent/useAddAgent.ts

  • New resolveDockerfileSource(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 against getWorkingDirectory() by default.
  • New validateDockerfileSource(value, cwd?, fileExists?) helper: bundles resolveDockerfileSource + the existsSync gate that both handleCreatePath and handleByoPath perform. Accepts an injectable fileExists predicate so the not-found error path is unit-testable without mocking fs.
  • handleCreatePath now resolves+normalizes generateConfig.dockerfile to a basename before calling mapGenerateConfigToRenderConfig, 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.
  • handleByoPath now uses the validated helper; reverted an over-eager round-1 check that pre-validated bare-filename references against codeDir (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.
  • Path-separator detection now matches both / and \ so Windows users entering subdir\Dockerfile aren't silently misclassified as referencing a bare filename.

src/cli/tui/screens/generate/GenerateWizardUI.tsx

  • Dockerfile PathInput now uses basePath={getWorkingDirectory()} (matching the AddAgentScreen change).
  • onSubmit no longer strips via basename() — that pre-existing behavior meant agentcore generate --dockerfile <path> silently dropped the path before the copy logic could run.
  • Wizard summary displays basename(config.dockerfile) for UX parity.

src/cli/tui/screens/agent/types.ts and src/cli/tui/screens/generate/types.ts

  • Expanded JSDoc on the dockerfile field documenting its dual lifecycle (full path during the wizard → basename after handleCreatePath/handleByoPath mutation).

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 a path.win32 end-to-end case), INIT_CWD set/unset/empty-string semantics.
  • validateDockerfileSource: success path, missing-source error path (asserts the exact Dockerfile not found at <sourcePath> user-visible message — regression guard for The path to dockerfile should be fixed to where the agentcore command was run #1128), bare-filename short-circuit (verifies fileExists is 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

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Testing

How have you tested the change?

  • I ran npm run test:unit and npm run test:integ (targeted unit tests for the changed files; CI will run the full suite)
  • I ran npm run typecheck
  • I ran npm run lint (via pre-commit prettier + eslint hooks)
  • If I modified src/assets/, I ran npm run test:update-snapshots and 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 pass
  • src/cli/tui/screens/generate/__tests__/useGenerateWizard.test.tsx — 36/36 pass
  • src/cli/tui/components/__tests__/PathInput.test.tsx — pass

Manual verification

  1. From a directory with a Dockerfile.test file, run agentcore create → BYO + Container → "Add a Dockerfile" → type ./Dockerfile.test. Wizard accepts; file is copied into <newProject>/app/<agent>/Dockerfile.test; agentcore.json references Dockerfile.test.
  2. Same flow with an absolute path — works.
  3. Same flow with a non-existent path — clear error referencing the cwd-resolved location.
  4. agentcore generate --dockerfile ./relative/path — file is copied as intended (parallel fix in the generate wizard).

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly (JSDoc on the affected types)
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

agentcore-bot added 4 commits May 6, 2026 20:50
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'.
@aidandaly24 aidandaly24 requested a review from a team May 6, 2026 21:15
@github-actions github-actions Bot added size/m PR size: M agentcore-harness-reviewing AgentCore Harness review in progress labels May 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 43.14% 9029 / 20927
🔵 Statements 42.42% 9587 / 22597
🔵 Functions 39.94% 1555 / 3893
🔵 Branches 40.04% 5824 / 14544
Generated in workflow #2561 for commit 5db3e02 by the Vitest Coverage Report Action

@agentcore-cli-automation
Copy link
Copy Markdown

The wizard-side changes in AddAgentScreen.tsx look correct, and the useAddAgent.ts fix is solid for agentcore add. However, I believe the fix misses the flow the PR is targetingagentcore create does not go through useAddAgent.

Trace

  • agentcore createCreateScreen → renders AddAgentScreen (phase create-wizard) → onComplete(config)useCreateFlow.handleAddAgentCompleteuseCreateFlow "running" effect (useCreateFlow.ts:181–460).
  • useCreateFlow.ts has its own copy of the create/BYO logic and never imports useAddAgent, handleCreatePath, handleByoPath, or validateDockerfileSource. It also never calls copyFileSync for a Dockerfile.

Why this breaks post-PR

Before the PR, AddAgentScreen's Dockerfile PathInput.onSubmit stored basename(value) on byoConfig.dockerfile. The PR changes this to store the full user-entered path (intentionally, so useAddAgent can resolve it). That value is then copied into AddAgentConfig.dockerfile in handleGenerateComplete / handleByoComplete and passed to onComplete, which in the create flow lands in useCreateFlow.

In useCreateFlow.ts:

  • BYO branch (lines 368–425): calls mapByoConfigToAgent(addAgentConfig) directly (line 378). mapByoConfigToAgent (useAddAgent.ts:163) does ...(config.dockerfile && { dockerfile: config.dockerfile }) with no normalization, so the full user-entered path gets written to agentcore.json and no file is ever copied into the code directory.
  • create branch (lines 259–347): builds generateConfig with addAgentConfig.dockerfile verbatim (line 264), renders, then writeAgentToProject(generateConfig, …). mapGenerateConfigToAgent (schema-mapper.ts:118) again stores config.dockerfile unchanged, and no copy happens.

So for agentcore create (the exact repro in #1128), the wizard now accepts the user's path without a confusing error, but the persisted agent spec ends up with an absolute/relative host path as its dockerfile, and the file is never copied into <projectRoot>/app/<agentName>/. That's arguably a worse state than the original bug.

This seems inconsistent with the manual verification step #1 in the PR description ("file is copied into <newProject>/app/<agent>/Dockerfile.test; agentcore.json references Dockerfile.test"), which makes me wonder if that was actually run against agentcore add rather than agentcore create. Worth re-running against agentcore create specifically.

Options

  1. Apply validateDockerfileSource + copy in useCreateFlow.ts for both the create and byo branches, mirroring what handleCreatePath / handleByoPath now do. Simplest targeted fix.
  2. De-duplicate by having useCreateFlow delegate to handleCreatePath / handleByoPath (or share a common helper). Larger refactor but removes the duplicated agent-creation logic that caused this divergence in the first place.
  3. Move the validation/copy earlier — e.g., have AddAgentScreen (or a shared post-wizard step) resolve the Dockerfile path and perform the copy before calling onComplete, so both useAddAgent and useCreateFlow downstream receive a normalized basename. This also simplifies the dual-state JSDoc you added to AddAgentConfig.dockerfile / GenerateConfig.dockerfile.

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 agentcore create → BYO + Container → ./Dockerfile.test path so this can't regress silently again.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The path to dockerfile should be fixed to where the agentcore command was run

2 participants