feat(import): extract and pass through executionRoleArn from starter toolkit YAML#680
Closed
jesseturner21 wants to merge 27 commits intoaws:mainfrom
Closed
feat(import): extract and pass through executionRoleArn from starter toolkit YAML#680jesseturner21 wants to merge 27 commits intoaws:mainfrom
jesseturner21 wants to merge 27 commits intoaws:mainfrom
Conversation
Implements a 3-phase import flow that migrates Bedrock AgentCore Starter Toolkit projects to the agentcore-cli: - Phase 1: Creates companion CloudFormation resources (IAM roles/policies) - Phase 2: Imports existing AWS resources via CFN IMPORT change set - Phase 3: User runs `agentcore deploy` to reconcile the stack Parses .bedrock_agentcore.yaml, scaffolds CLI project structure, copies agent source code, publishes CDK assets, and adopts pre-existing runtimes and memories into the CLI's CloudFormation stack. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove scaffold.ts; import now requires `agentcore create` first - Resolve target from project's aws-targets.json (auto-select single, error on multiple without --target, create default from YAML if empty) - Replace dangling Fn::GetAtt/Ref to removed primary resources with "*" in Phase 1 companion template (fixes IAM policy ARN validation) - Fix memoryArn placeholder in deployed state (construct ARN from ID) - Make --target optional (no default value) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Starter toolkit projects have multiple top-level directories (model/, mcp_client/) which causes setuptools to fail with "Multiple top-level packages discovered". Fix by appending [tool.setuptools] py-modules = [] to pyproject.toml after copy. Also run uv venv + uv sync so agentcore dev works immediately after import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Parse OAuth and API key credential providers from .bedrock_agentcore.yaml identity config and add them to project.json during import. Fix YAML parser to handle list items with nested key-value pairs (e.g., credential_providers). Fix eslint no-base-to-string errors by adding explicit type casts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… mode from creating invalid memory
The simple YAML parser turns bare "mode:" (no value) into an empty object {},
which is truthy and passes the existing `!== 'NO_MEMORY' && memoryConfig.mode`
check. This caused a memory with mode={} to be created, which is invalid.
Added typeof check to ensure mode is actually a string before proceeding.
Also adds comprehensive unit tests for import memory handling (35 tests).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oups
The YAML parser creates empty objects {} for keys with no list items
(e.g., `subnets:` with nothing underneath). The previous code used
`(networkModeConfig.subnets as string[]) ?? []` which doesn't protect
against this since {} is truthy. This resulted in subnets being {} instead
of [] when the YAML had empty subnet/security_group keys under VPC mode.
Now uses Array.isArray() to ensure we always get a proper array, falling
back to [] when the parser returns a non-array value.
Also adds comprehensive VPC import tests (18 tests covering parsing,
edge cases, mixed agents, quoted values, and starter toolkit format).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, running `agentcore import` twice with the same YAML would attempt to re-import already-managed CloudFormation resources in Phase 2, causing errors like "resource already exists in stack". The bug was that agentsToImport and memoriesToImport were filtered from all parsed YAML agents (regardless of whether they were skipped during merge), rather than from only the newly-added ones. Fix: track which agents/memories are actually added during the merge step (newlyAddedAgentNames / newlyAddedMemoryNames) and filter agentsToImport / memoriesToImport to only include those newly-added resources. Adds 17 unit tests covering first import, second import idempotency, partial overlap, credential idempotency, and edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When importing a starter toolkit YAML with no physical IDs (agent_id and memory_id are null), the import command would fail unnecessarily if no deployment targets existed and the YAML had null account/region. This is incorrect because the no-deploy path only needs config merge and source copy -- no CloudFormation operations. The fix computes hasPhysicalIds early and only performs strict target resolution (requiring account/region) when physical IDs are present. When no physical IDs exist, target resolution is lenient -- it uses an existing target if available, or falls back to 'default' for stackName. Also adds 22 unit tests covering the no-deploy path: YAML parsing of null IDs, early return behavior, config merge, Python setup, absence of CloudFormation operations, and target resolution edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
63 tests covering YAML parsing, toAgentEnvSpec conversion, merge logic, source copy, Phase 1/Phase 2 template operations, findLogicalId helpers, sanitize/toStackName, and integration for a single agent with no memory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests cover: - YAML parsing with 2, 3, and similar-named agents - Memory deduplication across shared agents - Credential extraction from identity.credential_providers - findLogicalIdByProperty with multiple runtimes - findLogicalIdByProperty Fn::Sub false substring match regression test - filterCompanionOnlyTemplate with multiple primary resources - buildImportTemplate with multiple import targets - Stack name sanitization and source code directory structure - Partial import (deployed + undeployed agents mixed) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 25a1458125d9ecdae1dd3e8c2f6e17336fc7498f.
…enario 16 tests covering agent/memory/credential deduplication, Set-based name matching, toMemorySpec clamping, source copy skip logic, append-only merge behavior, and edge cases for projects with existing agents and memories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…enario 16 tests covering agent/memory/credential deduplication, Set-based name matching, toMemorySpec clamping, source copy skip logic, append-only merge behavior, and edge cases for projects with existing agents and memories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Publish CDK assets to S3 before Phase 1 (CodeBuild needs source zip) - Handle directory assets in publishCdkAssets (zip before upload) - Export publishCdkAssets for use in actions.ts - Copy Dockerfile from starter toolkit config for Container builds - Generate fallback Dockerfile if toolkit config not found - Preserve previously imported resources during Phase 1 UPDATE by merging deployed template back into companion template Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change toCredentialSpec() to map OAuth providers to OAuthCredentialProvider instead of ApiKeyCredentialProvider, so the CLI wires CLIENT_ID/CLIENT_SECRET env vars correctly (not API_KEY) - Make discoveryUrl optional in OAuthCredentialSchema for imported providers that already exist in Identity service - Skip Identity service create/update for OAuth providers without discoveryUrl during deploy (provider already exists) - Set AWS_REGION/AWS_DEFAULT_REGION to target.region before import operations to prevent region mismatch when env var differs from aws-targets.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n authorizer config BUG-001: copyDirRecursive now skips symlinks and common excluded directories (.venv, .git, __pycache__, node_modules, etc.) preventing EISDIR crash when importing projects with virtualenvs containing symlinks like .venv/lib64 -> lib. BUG-004: Import now warns when an agent has authorizer_configuration set in the starter toolkit YAML, since custom JWT authorizer config is not automatically imported and must be manually recreated via `agentcore add gateway`. Constraint: CLI gateway model is separate from agent config, so full authorizer import requires design work Rejected: Auto-create gateway during import | gateway-agent linking logic is complex and warrants its own feature Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…containers
Two fixes:
1. Memory name prefix mismatch: CDK prefixes memory names with project
name (e.g. "myproject_Agent_mem") but import searched for unprefixed
YAML name. Added fallback lookup with project name prefix.
2. Container agents no longer trigger unnecessary Python venv setup
during import, since dependencies are installed inside the Docker image.
Constraint: CDK BasePrimitiveConstruct generates physicalName as ${projectName}_${name}
Rejected: Stripping prefix from CDK names | would break other CDK conventions
Confidence: high
Scope-risk: narrow
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
esbuild does not read tsconfig.json, so it defaults to classic JSX transform (React.createElement) while tsconfig specifies react-jsx (automatic). This produces 432 bare React.createElement calls in the bundle that reference a nonexistent React global, crashing all TUI commands (status, deploy, etc.) with "React is not defined". Constraint: esbuild ignores tsconfig jsx settings by default Rejected: Adding React as external | would require React in node_modules at runtime Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Starter Toolkit injects memory ID via BEDROCK_AGENTCORE_MEMORY_ID,
but CDK constructs use MEMORY_{NAME}_ID pattern. After import, agent
code still references the old env var, causing memory to silently fail.
Add a yellow warning during import telling users the correct env var
name to update in their agent code.
Constraint: CDK env var naming is controlled by AgentCoreMemory.getEnvVarName()
Rejected: Auto-rewrite agent source code | too fragile across frameworks
Confidence: high
Scope-risk: narrow
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Starter Toolkit injects memory ID via BEDROCK_AGENTCORE_MEMORY_ID,
but CDK constructs use MEMORY_{NAME}_ID pattern. After import, agent
code still references the old env var, causing memory to silently fail.
Show a git-diff-style warning during import with red/green highlighting
so users see exactly what to change in their agent code.
Constraint: CDK env var naming is controlled by AgentCoreMemory.getEnvVarName()
Rejected: Auto-rewrite agent source code | too fragile across frameworks
Confidence: high
Scope-risk: narrow
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Import is a CLI-only command that requires --source flag and doesn't have a TUI screen. Remove it from the interactive command picker. Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Import fails with "The specified bucket does not exist" when the AWS account hasn't been CDK-bootstrapped. The deploy command handles this via useCdkPreflight, but import bypasses that since it's CLI-only. Check bootstrap status after CDK synth and auto-bootstrap if needed, before disposing the toolkit wrapper. Tested on two unbootstrapped accounts (509471412906, 126432121770). Constraint: Must bootstrap before disposing toolkitWrapper since bootstrap requires it Rejected: Prompt user to manually run cdk bootstrap | poor UX for import flow Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused waitUntilChangeSetCreateComplete import - Prefix unused assetHash with underscore - Add eslint-disable for necessary any cast in STS credentials - Remove unnecessary type assertion in test - Fix prettier formatting in multi-agent test Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
OAuthCredentialProvider.discoveryUrl was made optional to support imported providers that already exist in Identity service. Update tests to match: - Schema test: expect success when discoveryUrl is omitted - Pre-deploy identity tests: add discoveryUrl to fixtures that test update and error paths (without it, the code now skips the provider) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rAllMocks checkBootstrapNeeded and bootstrapEnvironment were created as inline vi.fn() inside the vi.mock factory. vi.clearAllMocks() in beforeEach wipes their mockResolvedValue, causing handleImport to fail in CI where test execution order differs from local runs. Hoist the mocks and configure them in setupCommonMocks like all other mock functions in the file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…toolkit YAML Parse the aws.execution_role field from .bedrock_agentcore.yaml and propagate it through ParsedStarterToolkitAgent → toAgentEnvSpec → agentcore.json so the CDK constructs can import the existing role instead of creating a new one. - Schema: add optional executionRoleArn to AgentEnvSpecSchema - Types: add executionRoleArn to ParsedStarterToolkitAgent - YAML parser: extract execution_role from aws config - Actions: spread executionRoleArn into AgentEnvSpec Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
aws.execution_rolefield from.bedrock_agentcore.yamland propagate it through the import pipeline intoagentcore.jsonexecutionRoleArntoAgentEnvSpecSchema,ParsedStarterToolkitAgent, YAML parser, andtoAgentEnvSpec()User Experience
Before (broken)
After (fixed)
Reverting to a CDK-managed role
Remove
executionRoleArnfromagentcore.jsonand runagentcore deploy— CDK will create a new role with standard policies.Test plan
execution_role, spec generation with/withoutexecutionRoleArnagentcore importwrites correctexecutionRoleArntoagentcore.json🤖 Generated with Claude Code