refactor(import): address PR review feedback

- Add `red` to ANSI constants, replace inline escape codes
- Type GetEvaluatorResult.level as EvaluationLevel at boundary
- Combine ARN_RESOURCE_TYPE_MAP, collectionKeyMap, idFieldMap into
  single RESOURCE_TYPE_CONFIG to prevent drift
- Export IMPORTABLE_RESOURCES as const array, derive type from it,
  replace || chains with .includes()
- Fix samplingPercentage === 0 false positive (use == null)
- Document closure state sequencing contract on descriptor hooks

Author: jesseturner21

src/cli/aws/agentcore-control.ts

@@ -1,3 +1,4 @@
+import type { EvaluationLevel } from '../../schema/schemas/primitives/evaluator';
 import { getCredentialProvider } from './account';
 import {
   BedrockAgentCoreControlClient,
@@ -455,7 +456,7 @@ export interface GetEvaluatorResult {
   evaluatorId: string;
   evaluatorArn: string;
   evaluatorName: string;
-  level: string;
+  level: EvaluationLevel;
   status: string;
   description?: string;
   evaluatorConfig?: {
@@ -536,7 +537,7 @@ export async function getEvaluator(options: GetEvaluatorOptions): Promise<GetEva
     evaluatorId: response.evaluatorId,
     evaluatorArn: response.evaluatorArn ?? '',
     evaluatorName: response.evaluatorName ?? '',
-    level: response.level ?? 'SESSION',
+    level: (response.level ?? 'SESSION') as EvaluationLevel,
     status: response.status ?? 'UNKNOWN',
     description: response.description,
     evaluatorConfig,

src/cli/commands/import/__tests__/import-evaluator.test.ts

@@ -211,12 +211,14 @@ describe('toEvaluatorSpec', () => {
     expect(result.tags).toBeUndefined();
   });
 
-  it('defaults level to SESSION when not provided', () => {
-    const detail: GetEvaluatorResult = {
+  it('defaults level to SESSION when empty string reaches toEvaluatorSpec', () => {
+    // In practice, getEvaluator defaults empty level to 'SESSION' via the as EvaluationLevel cast.
+    // This tests the fallback in toEvaluatorSpec for defensive coverage.
+    const detail = {
       evaluatorId: 'eval-no-level',
       evaluatorArn: 'arn:aws:bedrock-agentcore:us-west-2:123456789012:evaluator/eval-no-level',
       evaluatorName: 'no_level_eval',
-      level: '',
+      level: '' as GetEvaluatorResult['level'],
       status: 'ACTIVE',
       evaluatorConfig: {
         llmAsAJudge: {
@@ -225,11 +227,10 @@ describe('toEvaluatorSpec', () => {
           ratingScale: { numerical: [{ value: 1, label: 'Low', definition: 'Low' }] },
         },
       },
-    };
+    } as GetEvaluatorResult;
 
-    // level defaults to 'SESSION' via the ?? in getEvaluator, but toEvaluatorSpec takes what it gets
     const result = toEvaluatorSpec(detail, 'no_level_eval');
-    expect(result.level).toBe('');
+    expect(result.level).toBe('SESSION');
   });
 });
 

src/cli/commands/import/constants.ts

@@ -3,6 +3,7 @@ export const NAME_REGEX = /^[a-zA-Z][a-zA-Z0-9_]{0,47}$/;
 
 /** ANSI escape codes for console output. */
 export const ANSI = {
+  red: '\x1b[31m',
   green: '\x1b[32m',
   yellow: '\x1b[33m',
   cyan: '\x1b[36m',

src/cli/commands/import/import-evaluator.ts

@@ -16,7 +16,7 @@ import type { Command } from '@commander-js/extra-typings';
  * Map an AWS GetEvaluator response to the CLI Evaluator spec format.
  */
 export function toEvaluatorSpec(detail: GetEvaluatorResult, localName: string): Evaluator {
-  const level = (detail.level ?? 'SESSION') as Evaluator['level'];
+  const level = detail.level || 'SESSION';
 
   let config: Evaluator['config'];
 
@@ -143,7 +143,7 @@ export function registerImportEvaluator(importCmd: Command): void {
         console.log(`  ID: ${result.resourceId}`);
         console.log('');
       } else {
-        console.error(`\n\x1b[31m[error]${ANSI.reset} ${result.error}`);
+        console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`);
         if (result.logPath) {
           console.error(`Log: ${result.logPath}`);
         }

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

@@ -121,7 +121,7 @@ export function registerImportMemory(importCmd: Command): void {
         console.log(`  ID: ${result.resourceId}`);
         console.log('');
       } else {
-        console.error(`\n\x1b[31m[error]${ANSI.reset} ${result.error}`);
+        console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`);
         if (result.logPath) {
           console.error(`Log: ${result.logPath}`);
         }

src/cli/commands/import/import-online-eval.ts

@@ -32,7 +32,7 @@ export function toOnlineEvalConfigSpec(
   agentName: string,
   evaluatorArns: string[]
 ): OnlineEvalConfig {
-  if (!detail.samplingPercentage) {
+  if (detail.samplingPercentage == null) {
     throw new Error(`Online eval config "${detail.configName}" has no sampling configuration. Cannot import.`);
   }
 
@@ -59,6 +59,7 @@ function buildEvaluatorArns(evaluatorIds: string[], region: string, account: str
  * Create an online-eval descriptor with closed-over state for reference resolution.
  */
 function createOnlineEvalDescriptor(): ResourceImportDescriptor<GetOnlineEvalConfigResult, OnlineEvalConfigSummary> {
+  // Set by beforeConfigWrite, read by addToProjectSpec. Ordering guaranteed by executeResourceImport.
   let resolvedAgentName = '';
   let resolvedEvaluatorArns: string[] = [];
 
@@ -207,7 +208,7 @@ export function registerImportOnlineEval(importCmd: Command): void {
         console.log(`  ID: ${result.resourceId}`);
         console.log('');
       } else {
-        console.error(`\n\x1b[31m[error]${ANSI.reset} ${result.error}`);
+        console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`);
         if (result.logPath) {
           console.error(`Log: ${result.logPath}`);
         }

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

@@ -225,7 +225,7 @@ export function registerImportRuntime(importCmd: Command): void {
         console.log(`  agentcore invoke     ${ANSI.dim}Test your agent${ANSI.reset}`);
         console.log('');
       } else {
-        console.error(`\n\x1b[31m[error]${ANSI.reset} ${result.error}`);
+        console.error(`\n${ANSI.red}[error]${ANSI.reset} ${result.error}`);
         if (result.logPath) {
           console.error(`Log: ${result.logPath}`);
         }

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

@@ -212,12 +212,23 @@ export interface ParsedArn {
 const ARN_PATTERN =
   /^arn:aws:bedrock-agentcore:([^:]+):([^:]+):(runtime|memory|evaluator|online-evaluation-config)\/(.+)$/;
 
-/** Map from ImportableResourceType to the resource type string used in ARNs. */
-const ARN_RESOURCE_TYPE_MAP: Record<ImportableResourceType, string> = {
-  runtime: 'runtime',
-  memory: 'memory',
-  evaluator: 'evaluator',
-  'online-eval': 'online-evaluation-config',
+/** Unified config for each importable resource type — ARN mapping, deployed state keys. */
+const RESOURCE_TYPE_CONFIG: Record<
+  ImportableResourceType,
+  {
+    arnType: string;
+    collectionKey: string;
+    idField: string;
+  }
+> = {
+  runtime: { arnType: 'runtime', collectionKey: 'runtimes', idField: 'runtimeId' },
+  memory: { arnType: 'memory', collectionKey: 'memories', idField: 'memoryId' },
+  evaluator: { arnType: 'evaluator', collectionKey: 'evaluators', idField: 'evaluatorId' },
+  'online-eval': {
+    arnType: 'online-evaluation-config',
+    collectionKey: 'onlineEvalConfigs',
+    idField: 'onlineEvaluationConfigId',
+  },
 };
 
 /**
@@ -230,7 +241,7 @@ export function parseAndValidateArn(
   target: { region: string; account: string }
 ): ParsedArn {
   const match = ARN_PATTERN.exec(arn);
-  const expectedArnType = ARN_RESOURCE_TYPE_MAP[expectedResourceType];
+  const expectedArnType = RESOURCE_TYPE_CONFIG[expectedResourceType].arnType;
   if (!match) {
     throw new Error(
       `Invalid ARN format: "${arn}". Expected format: arn:aws:bedrock-agentcore:<region>:<account>:${expectedArnType}/<id>`
@@ -289,23 +300,10 @@ export async function findResourceInDeployedState(
   const targetState = state.targets?.[targetName];
   if (!targetState?.resources) return undefined;
 
-  const collectionKeyMap: Record<ImportableResourceType, string> = {
-    runtime: 'runtimes',
-    memory: 'memories',
-    evaluator: 'evaluators',
-    'online-eval': 'onlineEvalConfigs',
-  };
-  const idFieldMap: Record<ImportableResourceType, string> = {
-    runtime: 'runtimeId',
-    memory: 'memoryId',
-    evaluator: 'evaluatorId',
-    'online-eval': 'onlineEvaluationConfigId',
-  };
+  const { collectionKey, idField } = RESOURCE_TYPE_CONFIG[resourceType];
 
-  const collection = targetState.resources[collectionKeyMap[resourceType]];
+  const collection = targetState.resources[collectionKey];
   if (!collection) return undefined;
-
-  const idField = idFieldMap[resourceType];
   for (const [name, entry] of Object.entries(collection)) {
     if ((entry as any)[idField] === resourceId) return name;
   }

src/cli/commands/import/types.ts

@@ -72,8 +72,10 @@ export interface ParsedStarterToolkitConfig {
 
 /**
  * Resource types supported by the import subcommands.
+ * Use the array for runtime checks (e.g., IMPORTABLE_RESOURCES.includes(x)).
  */
-export type ImportableResourceType = 'runtime' | 'memory' | 'evaluator' | 'online-eval';
+export const IMPORTABLE_RESOURCES = ['runtime', 'memory', 'evaluator', 'online-eval'] as const;
+export type ImportableResourceType = (typeof IMPORTABLE_RESOURCES)[number];
 
 /**
  * Resource to be imported via CloudFormation IMPORT change set.
@@ -203,7 +205,10 @@ export interface ResourceImportDescriptor<TDetail, TSummary> {
   /** Get the array of existing resource names from the project spec. */
   getExistingNames: (projectSpec: AgentCoreProjectSpec) => string[];
 
-  /** Convert the AWS detail to local spec and add it to the project spec. */
+  /**
+   * Convert the AWS detail to local spec and add it to the project spec.
+   * Called after beforeConfigWrite — descriptor factories may rely on state set during that hook.
+   */
   addToProjectSpec: (detail: TDetail, localName: string, projectSpec: AgentCoreProjectSpec) => void;
 
   // ---- CFN template matching ----
@@ -226,6 +231,8 @@ export interface ResourceImportDescriptor<TDetail, TSummary> {
 
   /**
    * Called after detail fetch + name validation but before config write.
+   * Always runs before addToProjectSpec — descriptor factories can use this
+   * to set closed-over state that addToProjectSpec later reads.
    * Return an ImportResourceResult to abort, or void to continue.
    */
   beforeConfigWrite?: (ctx: BeforeWriteContext<TDetail>) => Promise<ImportResourceResult | void>;

src/cli/tui/screens/import/ArnInputScreen.tsx

@@ -1,3 +1,4 @@
+import type { ImportableResourceType } from '../../../commands/import/types';
 import { Panel } from '../../components/Panel';
 import { Screen } from '../../components/Screen';
 import { TextInput } from '../../components/TextInput';
@@ -13,7 +14,7 @@ function validateArn(value: string): true | string {
 }
 
 interface ArnInputScreenProps {
-  resourceType: 'runtime' | 'memory' | 'evaluator' | 'online-eval';
+  resourceType: ImportableResourceType;
   onSubmit: (arn: string) => void;
   onExit: () => void;
 }