Skip to content

Commit e467c75

Browse files
jesseturner21jesseturner21
committed
fix(import): address bugbash issues for import commands
- Invalid ARN now returns "Not a valid ARN" before target resolution - Failed imports roll back agentcore.json and clean up copied app/ dirs - Discovery listings show ARNs (not just IDs) so users can copy them - Remove --target flag from import runtime/memory subcommands - Add description field to AgentEnvSpec schema and wire through import Constraint: Commander validates requiredOption before action handlers Constraint: Rollback is best-effort to avoid masking the original error Rejected: Keep --target on subcommands | silently falls back to default, confusing UX Confidence: high Scope-risk: moderate
1 parent 63348c3 commit e467c75

5 files changed

Lines changed: 80 additions & 9 deletions

File tree

‎src/cli/commands/import/__tests__/import-runtime-handler.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -364,10 +364,10 @@ describe('handleImportRuntime', () => {
364364

365365
mockCopyAgentSource.mockResolvedValue(undefined);
366366

367-
// Capture what's written to project spec
367+
// Capture the first write to project spec (before any rollback)
368368
let writtenSpec: Record<string, unknown> | undefined;
369369
mockConfigIO.writeProjectSpec.mockImplementation((spec: Record<string, unknown>) => {
370-
writtenSpec = spec;
370+
if (!writtenSpec) writtenSpec = JSON.parse(JSON.stringify(spec)) as Record<string, unknown>;
371371
return Promise.resolve();
372372
});
373373

@@ -409,7 +409,7 @@ describe('handleImportRuntime', () => {
409409

410410
let writtenSpec: Record<string, unknown> | undefined;
411411
mockConfigIO.writeProjectSpec.mockImplementation((spec: Record<string, unknown>) => {
412-
writtenSpec = spec;
412+
if (!writtenSpec) writtenSpec = JSON.parse(JSON.stringify(spec)) as Record<string, unknown>;
413413
return Promise.resolve();
414414
});
415415

@@ -454,7 +454,7 @@ describe('handleImportRuntime', () => {
454454

455455
let writtenSpec: Record<string, unknown> | undefined;
456456
mockConfigIO.writeProjectSpec.mockImplementation((spec: Record<string, unknown>) => {
457-
writtenSpec = spec;
457+
if (!writtenSpec) writtenSpec = JSON.parse(JSON.stringify(spec)) as Record<string, unknown>;
458458
return Promise.resolve();
459459
});
460460

‎src/cli/commands/import/import-memory.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { Memory } from '../../../schema';
1+
import type { AgentCoreProjectSpec, Memory } from '../../../schema';
2+
import type { ConfigIO } from '../../../lib';
23
import type { MemoryDetail } from '../../aws/agentcore-control';
34
import { getMemoryDetail, listAllMemories } from '../../aws/agentcore-control';
45
import { LocalCdkProject } from '../../cdk/local-cdk-project';
@@ -91,10 +92,26 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
9192
console.log(`${green}[done]${reset} ${message}`);
9293
};
9394

95+
// Rollback state
96+
let configSnapshot: AgentCoreProjectSpec | undefined;
97+
let configWritten = false;
98+
let configIORef: ConfigIO | undefined;
99+
100+
const rollback = async () => {
101+
if (configWritten && configSnapshot && configIORef) {
102+
try {
103+
await configIORef.writeProjectSpec(configSnapshot);
104+
} catch {
105+
// best-effort rollback
106+
}
107+
}
108+
};
109+
94110
try {
95111
// 1. Validate project context
96112
logger.startStep('Validate project context');
97113
const ctx = await resolveProjectContext();
114+
configIORef = ctx.configIO;
98115
logger.endStep('success');
99116

100117
// 2. Resolve deployment target
@@ -142,6 +159,7 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
142159
for (let i = 0; i < memories.length; i++) {
143160
const m = memories[i]!;
144161
console.log(` ${dim}[${i + 1}]${reset} ${m.memoryId} — ${m.status}`);
162+
console.log(` ${dim}${m.memoryArn}${reset}`);
145163
}
146164
console.log('');
147165

@@ -203,9 +221,11 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
203221

204222
// 5. Add to project config
205223
logger.startStep('Update project config');
224+
configSnapshot = JSON.parse(JSON.stringify(projectSpec)) as AgentCoreProjectSpec;
206225
const memorySpec = toMemorySpec(memoryDetail, localName);
207226
(projectSpec.memories ??= []).push(memorySpec);
208227
await ctx.configIO.writeProjectSpec(projectSpec);
228+
configWritten = true;
209229
onProgress(`Added memory "${localName}" to agentcore.json`);
210230
logger.endStep('success');
211231

@@ -232,6 +252,7 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
232252
if (files.length === 0) {
233253
await toolkitWrapper.dispose();
234254
const error = 'No CloudFormation template found in CDK assembly';
255+
await rollback();
235256
logger.endStep('error', error);
236257
logger.finalize(false);
237258
return {
@@ -275,6 +296,7 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
275296

276297
if (!phase1Result.success) {
277298
const error = `Phase 1 failed: ${phase1Result.error}`;
299+
await rollback();
278300
logger.endStep('error', error);
279301
logger.finalize(false);
280302
return {
@@ -293,6 +315,7 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
293315
const deployedTemplate = await getDeployedTemplate(target.region, stackName);
294316
if (!deployedTemplate) {
295317
const error = 'Could not read deployed template after Phase 1';
318+
await rollback();
296319
logger.endStep('error', error);
297320
logger.finalize(false);
298321
return {
@@ -322,6 +345,7 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
322345

323346
if (!logicalId) {
324347
const error = `Could not find logical ID for memory "${localName}" in CloudFormation template`;
348+
await rollback();
325349
logger.endStep('error', error);
326350
logger.finalize(false);
327351
return {
@@ -354,6 +378,7 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
354378

355379
if (!phase2Result.success) {
356380
const error = `Phase 2 failed: ${phase2Result.error}`;
381+
await rollback();
357382
logger.endStep('error', error);
358383
logger.finalize(false);
359384
return {
@@ -389,6 +414,7 @@ export async function handleImportMemory(options: ImportResourceOptions): Promis
389414
};
390415
} catch (err: unknown) {
391416
const message = err instanceof Error ? err.message : String(err);
417+
await rollback();
392418
logger.log(message, 'error');
393419
logger.finalize(false);
394420
return {
@@ -409,7 +435,6 @@ export function registerImportMemory(importCmd: Command): void {
409435
.command('memory')
410436
.description('Import an existing AgentCore Memory from your AWS account')
411437
.option('--arn <memoryArn>', 'Memory ARN to import')
412-
.option('--target <target>', 'Deployment target name')
413438
.option('--name <name>', 'Local name for the imported memory')
414439
.option('-y, --yes', 'Auto-confirm prompts')
415440
.action(async (cliOptions: ImportResourceOptions) => {

‎src/cli/commands/import/import-runtime.ts‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { AgentEnvSpec } from '../../../schema';
1+
import type { AgentCoreProjectSpec, AgentEnvSpec } from '../../../schema';
2+
import type { ConfigIO } from '../../../lib';
23
import type { AgentRuntimeDetail } from '../../aws/agentcore-control';
34
import { getAgentRuntimeDetail, listAllAgentRuntimes } from '../../aws/agentcore-control';
45
import { LocalCdkProject } from '../../cdk/local-cdk-project';
@@ -52,6 +53,7 @@ function toAgentEnvSpec(
5253
runtime.build === 'Container' ? runtime.runtimeVersion : (runtime.runtimeVersion ?? 'PYTHON_3_12');
5354
const spec: AgentEnvSpec = {
5455
name: localName,
56+
...(runtime.description && { description: runtime.description }),
5557
build: runtime.build,
5658
entrypoint: entrypoint as any,
5759
codeLocation: codeLocation as any,
@@ -105,10 +107,36 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
105107
console.log(`${green}[done]${reset} ${message}`);
106108
};
107109

110+
// Rollback state
111+
let configSnapshot: AgentCoreProjectSpec | undefined;
112+
let configWritten = false;
113+
let copiedAppDir: string | undefined;
114+
let configIORef: ConfigIO | undefined;
115+
116+
const rollback = async () => {
117+
// Rollback config
118+
if (configWritten && configSnapshot && configIORef) {
119+
try {
120+
await configIORef.writeProjectSpec(configSnapshot);
121+
} catch {
122+
// best-effort rollback
123+
}
124+
}
125+
// Cleanup copied source directory
126+
if (copiedAppDir && fs.existsSync(copiedAppDir)) {
127+
try {
128+
fs.rmSync(copiedAppDir, { recursive: true, force: true });
129+
} catch {
130+
// best-effort cleanup
131+
}
132+
}
133+
};
134+
108135
try {
109136
// 1. Validate project context
110137
logger.startStep('Validate project context');
111138
const ctx = await resolveProjectContext();
139+
configIORef = ctx.configIO;
112140
logger.endStep('success');
113141

114142
// 2. Resolve deployment target
@@ -155,7 +183,8 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
155183
console.log(`\nFound ${runtimes.length} runtime(s):\n`);
156184
for (let i = 0; i < runtimes.length; i++) {
157185
const r = runtimes[i]!;
158-
console.log(` ${dim}[${i + 1}]${reset} ${r.agentRuntimeName} (${r.agentRuntimeId}) — ${r.status}`);
186+
console.log(` ${dim}[${i + 1}]${reset} ${r.agentRuntimeName} — ${r.status}`);
187+
console.log(` ${dim}${r.agentRuntimeArn}${reset}`);
159188
}
160189
console.log('');
161190

@@ -289,6 +318,7 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
289318
// 7. Copy source code
290319
logger.startStep('Copy agent source');
291320
const codeLocation = `app/${localName}/`;
321+
copiedAppDir = path.join(ctx.projectRoot, 'app', localName);
292322
await copyAgentSource({
293323
sourcePath,
294324
agentName: localName,
@@ -301,9 +331,11 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
301331

302332
// 8. Add to project config
303333
logger.startStep('Update project config');
334+
configSnapshot = JSON.parse(JSON.stringify(projectSpec)) as AgentCoreProjectSpec;
304335
const agentSpec = toAgentEnvSpec(runtimeDetail, localName, codeLocation, entrypoint);
305336
projectSpec.runtimes.push(agentSpec);
306337
await ctx.configIO.writeProjectSpec(projectSpec);
338+
configWritten = true;
307339
onProgress(`Added runtime "${localName}" to agentcore.json`);
308340
logger.endStep('success');
309341

@@ -330,6 +362,7 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
330362
if (files.length === 0) {
331363
await toolkitWrapper.dispose();
332364
const error = 'No CloudFormation template found in CDK assembly';
365+
await rollback();
333366
logger.endStep('error', error);
334367
logger.finalize(false);
335368
return {
@@ -373,6 +406,7 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
373406

374407
if (!phase1Result.success) {
375408
const error = `Phase 1 failed: ${phase1Result.error}`;
409+
await rollback();
376410
logger.endStep('error', error);
377411
logger.finalize(false);
378412
return {
@@ -391,6 +425,7 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
391425
const deployedTemplate = await getDeployedTemplate(target.region, stackName);
392426
if (!deployedTemplate) {
393427
const error = 'Could not read deployed template after Phase 1';
428+
await rollback();
394429
logger.endStep('error', error);
395430
logger.finalize(false);
396431
return {
@@ -420,6 +455,7 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
420455

421456
if (!logicalId) {
422457
const error = `Could not find logical ID for runtime "${localName}" in CloudFormation template`;
458+
await rollback();
423459
logger.endStep('error', error);
424460
logger.finalize(false);
425461
return {
@@ -452,6 +488,7 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
452488

453489
if (!phase2Result.success) {
454490
const error = `Phase 2 failed: ${phase2Result.error}`;
491+
await rollback();
455492
logger.endStep('error', error);
456493
logger.finalize(false);
457494
return {
@@ -487,6 +524,7 @@ export async function handleImportRuntime(options: ImportResourceOptions): Promi
487524
};
488525
} catch (err: unknown) {
489526
const message = err instanceof Error ? err.message : String(err);
527+
await rollback();
490528
logger.log(message, 'error');
491529
logger.finalize(false);
492530
return {
@@ -509,7 +547,6 @@ export function registerImportRuntime(importCmd: Command): void {
509547
.option('--arn <runtimeArn>', 'Runtime ARN to import')
510548
.option('--code <path>', 'Path to the directory containing the entrypoint file (e.g., the folder with main.py)')
511549
.option('--entrypoint <file>', 'Entrypoint file (auto-detected from runtime, e.g. main.py)')
512-
.option('--target <target>', 'Deployment target name')
513550
.option('--name <name>', 'Local name for the imported runtime')
514551
.option('-y, --yes', 'Auto-confirm prompts')
515552
.action(async (cliOptions: ImportResourceOptions) => {

‎src/cli/commands/import/import-utils.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ export interface ResolveTargetOptions {
6060
export async function resolveImportTarget(options: ResolveTargetOptions): Promise<AwsDeploymentTarget> {
6161
const { configIO, targetName, arn, onProgress } = options;
6262

63+
// Validate ARN format early if provided
64+
if (arn && !/^arn:aws:bedrock-agentcore:([^:]+):([^:]+):(runtime|memory)\/(.+)$/.test(arn)) {
65+
throw new Error(
66+
`Not a valid ARN: "${arn}".\nExpected format: arn:aws:bedrock-agentcore:<region>:<account>:<runtime|memory>/<id>`
67+
);
68+
}
69+
6370
let targets = await configIO.readAWSDeploymentTargets();
6471

6572
if (targets.length === 0) {

‎src/schema/schemas/agent-env.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ export type LifecycleConfiguration = z.infer<typeof LifecycleConfigurationSchema
178178
export const AgentEnvSpecSchema = z
179179
.object({
180180
name: AgentNameSchema,
181+
/** Optional description for the runtime. */
182+
description: z.string().max(200).optional(),
181183
build: BuildTypeSchema,
182184
entrypoint: EntrypointSchema,
183185
codeLocation: DirectoryPathSchema,

0 commit comments

Comments
 (0)