From 714adc8a6740a19a64b66dbefec7f8713eda2ead Mon Sep 17 00:00:00 2001 From: Tejas Kashinath Date: Tue, 10 Feb 2026 18:33:44 -0500 Subject: [PATCH] fix: remove CUSTOM memory strategy temporarily (#235) Remove CUSTOM from MemoryStrategyType as it is not yet supported. This is a P0 fix to prevent users from selecting an unsupported option. Changes: - Remove CUSTOM from MemoryStrategyTypeSchema enum - Update validation to reject CUSTOM strategy - Update CLI help text and documentation - Add comprehensive tests for memory strategy validation Closes #235 --- .../assets.snapshot.test.ts.snap | 2 +- src/assets/agents/AGENTS.md | 2 +- .../commands/add/__tests__/add-memory.test.ts | 12 +++ .../commands/add/__tests__/validate.test.ts | 44 ++++++++- src/cli/commands/add/command.tsx | 2 +- src/cli/commands/add/validate.ts | 2 +- src/cli/tui/screens/memory/types.ts | 1 - src/schema/llm-compacted/agentcore.ts | 2 +- .../primitives/__tests__/memory.test.ts | 89 +++++++++++++++++++ src/schema/schemas/primitives/memory.ts | 4 +- 10 files changed, 150 insertions(+), 10 deletions(-) create mode 100644 src/schema/schemas/primitives/__tests__/memory.test.ts diff --git a/src/assets/__tests__/__snapshots__/assets.snapshot.test.ts.snap b/src/assets/__tests__/__snapshots__/assets.snapshot.test.ts.snap index 753b7d114..a55177be0 100644 --- a/src/assets/__tests__/__snapshots__/assets.snapshot.test.ts.snap +++ b/src/assets/__tests__/__snapshots__/assets.snapshot.test.ts.snap @@ -2829,7 +2829,7 @@ file maps to a JSON config file and includes validation constraints as comments. - **BuildType**: \`'CodeZip'\` | \`'Container'\` - **NetworkMode**: \`'PUBLIC'\` | \`'PRIVATE'\` - **RuntimeVersion**: \`'PYTHON_3_12'\` | \`'PYTHON_3_13'\` | \`'NODE_18'\` | \`'NODE_20'\` | \`'NODE_22'\` -- **MemoryStrategyType**: \`'SEMANTIC'\` | \`'SUMMARIZATION'\` | \`'USER_PREFERENCE'\` | \`'CUSTOM'\` +- **MemoryStrategyType**: \`'SEMANTIC'\` | \`'SUMMARIZATION'\` | \`'USER_PREFERENCE'\` ### Supported Frameworks (for template agents) diff --git a/src/assets/agents/AGENTS.md b/src/assets/agents/AGENTS.md index a8ae79c2d..1a8ee68e1 100644 --- a/src/assets/agents/AGENTS.md +++ b/src/assets/agents/AGENTS.md @@ -63,7 +63,7 @@ file maps to a JSON config file and includes validation constraints as comments. - **BuildType**: `'CodeZip'` | `'Container'` - **NetworkMode**: `'PUBLIC'` | `'PRIVATE'` - **RuntimeVersion**: `'PYTHON_3_12'` | `'PYTHON_3_13'` | `'NODE_18'` | `'NODE_20'` | `'NODE_22'` -- **MemoryStrategyType**: `'SEMANTIC'` | `'SUMMARIZATION'` | `'USER_PREFERENCE'` | `'CUSTOM'` +- **MemoryStrategyType**: `'SEMANTIC'` | `'SUMMARIZATION'` | `'USER_PREFERENCE'` ### Supported Frameworks (for template agents) diff --git a/src/cli/commands/add/__tests__/add-memory.test.ts b/src/cli/commands/add/__tests__/add-memory.test.ts index f235cbacf..ed0958795 100644 --- a/src/cli/commands/add/__tests__/add-memory.test.ts +++ b/src/cli/commands/add/__tests__/add-memory.test.ts @@ -50,6 +50,18 @@ describe('add memory command', () => { expect(json.success).toBe(false); expect(json.error.includes('INVALID'), `Error: ${json.error}`).toBeTruthy(); }); + + // Issue #235: CUSTOM strategy has been removed + it('rejects CUSTOM strategy', async () => { + const result = await runCLI( + ['add', 'memory', '--name', 'testCustom', '--strategies', 'CUSTOM', '--json'], + projectDir + ); + expect(result.exitCode).toBe(1); + const json = JSON.parse(result.stdout); + expect(json.success).toBe(false); + expect(json.error.includes('CUSTOM'), `Error: ${json.error}`).toBeTruthy(); + }); }); describe('memory creation', () => { diff --git a/src/cli/commands/add/__tests__/validate.test.ts b/src/cli/commands/add/__tests__/validate.test.ts index 6e8596e5b..cefc1a733 100644 --- a/src/cli/commands/add/__tests__/validate.test.ts +++ b/src/cli/commands/add/__tests__/validate.test.ts @@ -313,9 +313,51 @@ describe('validate', () => { expect(validateAddMemoryOptions(validMemoryOptions)).toEqual({ valid: true }); // Test all valid strategies expect( - validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SEMANTIC,SUMMARIZATION,USER_PREFERENCE,CUSTOM' }) + validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SEMANTIC,SUMMARIZATION,USER_PREFERENCE' }) ).toEqual({ valid: true }); }); + + // AC23: CUSTOM strategy is not supported (Issue #235) + it('rejects CUSTOM strategy', () => { + const result = validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'CUSTOM' }); + expect(result.valid).toBe(false); + expect(result.error).toContain('Invalid strategy: CUSTOM'); + }); + + it('rejects CUSTOM even when mixed with valid strategies', () => { + const result = validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SEMANTIC,CUSTOM' }); + expect(result.valid).toBe(false); + expect(result.error).toContain('Invalid strategy: CUSTOM'); + }); + + // AC24: Each individual valid strategy should pass + it('accepts each valid strategy individually', () => { + expect(validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SEMANTIC' })).toEqual({ valid: true }); + expect(validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SUMMARIZATION' })).toEqual({ valid: true }); + expect(validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'USER_PREFERENCE' })).toEqual({ + valid: true, + }); + }); + + // AC25: Valid strategy combinations should pass + it('accepts valid strategy combinations', () => { + expect(validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SEMANTIC,SUMMARIZATION' })).toEqual({ + valid: true, + }); + expect(validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SEMANTIC,USER_PREFERENCE' })).toEqual({ + valid: true, + }); + expect(validateAddMemoryOptions({ ...validMemoryOptions, strategies: 'SUMMARIZATION,USER_PREFERENCE' })).toEqual({ + valid: true, + }); + }); + + // AC26: Strategies with whitespace should be handled + it('handles strategies with whitespace', () => { + expect(validateAddMemoryOptions({ ...validMemoryOptions, strategies: ' SEMANTIC , SUMMARIZATION ' })).toEqual({ + valid: true, + }); + }); }); describe('validateAddIdentityOptions', () => { diff --git a/src/cli/commands/add/command.tsx b/src/cli/commands/add/command.tsx index 85faf9f20..d9f1aa1ec 100644 --- a/src/cli/commands/add/command.tsx +++ b/src/cli/commands/add/command.tsx @@ -316,7 +316,7 @@ export function registerAdd(program: Command) { .option('--name ', 'Memory name [non-interactive]') .option( '--strategies ', - 'Comma-separated strategies: SEMANTIC, SUMMARIZATION, USER_PREFERENCE, CUSTOM [non-interactive]' + 'Comma-separated strategies: SEMANTIC, SUMMARIZATION, USER_PREFERENCE [non-interactive]' ) .option('--expiry ', 'Event expiry duration in days (default: 30) [non-interactive]', parseInt) .option('--json', 'Output as JSON [non-interactive]') diff --git a/src/cli/commands/add/validate.ts b/src/cli/commands/add/validate.ts index fa6debae9..89ca71426 100644 --- a/src/cli/commands/add/validate.ts +++ b/src/cli/commands/add/validate.ts @@ -22,7 +22,7 @@ export interface ValidationResult { // Constants const MEMORY_OPTIONS = ['none', 'shortTerm', 'longAndShortTerm'] as const; const OIDC_WELL_KNOWN_SUFFIX = '/.well-known/openid-configuration'; -const VALID_STRATEGIES = ['SEMANTIC', 'SUMMARIZATION', 'USER_PREFERENCE', 'CUSTOM']; +const VALID_STRATEGIES = ['SEMANTIC', 'SUMMARIZATION', 'USER_PREFERENCE']; // Agent validation export function validateAddAgentOptions(options: AddAgentOptions): ValidationResult { diff --git a/src/cli/tui/screens/memory/types.ts b/src/cli/tui/screens/memory/types.ts index e86226a63..b7ca0f710 100644 --- a/src/cli/tui/screens/memory/types.ts +++ b/src/cli/tui/screens/memory/types.ts @@ -32,7 +32,6 @@ const STRATEGY_DESCRIPTIONS: Record = { SEMANTIC: 'Vector-based semantic search over memories', SUMMARIZATION: 'Compress and summarize conversation context', USER_PREFERENCE: 'Track and recall user preferences', - CUSTOM: 'Custom memory strategy implementation', }; export const MEMORY_STRATEGY_OPTIONS = MemoryStrategyTypeSchema.options.map(type => ({ diff --git a/src/schema/llm-compacted/agentcore.ts b/src/schema/llm-compacted/agentcore.ts index 17b4c3ffb..15eddd66b 100644 --- a/src/schema/llm-compacted/agentcore.ts +++ b/src/schema/llm-compacted/agentcore.ts @@ -27,7 +27,7 @@ type PythonRuntime = 'PYTHON_3_10' | 'PYTHON_3_11' | 'PYTHON_3_12' | 'PYTHON_3_1 type NodeRuntime = 'NODE_18' | 'NODE_20' | 'NODE_22'; type RuntimeVersion = PythonRuntime | NodeRuntime; type NetworkMode = 'PUBLIC' | 'PRIVATE'; -type MemoryStrategyType = 'SEMANTIC' | 'SUMMARIZATION' | 'USER_PREFERENCE' | 'CUSTOM'; +type MemoryStrategyType = 'SEMANTIC' | 'SUMMARIZATION' | 'USER_PREFERENCE'; // ───────────────────────────────────────────────────────────────────────────── // AGENT diff --git a/src/schema/schemas/primitives/__tests__/memory.test.ts b/src/schema/schemas/primitives/__tests__/memory.test.ts new file mode 100644 index 000000000..c8eef9f4c --- /dev/null +++ b/src/schema/schemas/primitives/__tests__/memory.test.ts @@ -0,0 +1,89 @@ +import { DEFAULT_STRATEGY_NAMESPACES, MemoryStrategySchema, MemoryStrategyTypeSchema } from '../memory'; +import { describe, expect, it } from 'vitest'; + +describe('MemoryStrategyTypeSchema', () => { + describe('valid strategy types', () => { + it('accepts SEMANTIC', () => { + expect(MemoryStrategyTypeSchema.safeParse('SEMANTIC').success).toBe(true); + }); + + it('accepts SUMMARIZATION', () => { + expect(MemoryStrategyTypeSchema.safeParse('SUMMARIZATION').success).toBe(true); + }); + + it('accepts USER_PREFERENCE', () => { + expect(MemoryStrategyTypeSchema.safeParse('USER_PREFERENCE').success).toBe(true); + }); + }); + + describe('invalid strategy types', () => { + // Issue #235: CUSTOM strategy has been removed + it('rejects CUSTOM strategy', () => { + const result = MemoryStrategyTypeSchema.safeParse('CUSTOM'); + expect(result.success).toBe(false); + }); + + it('rejects arbitrary invalid strategies', () => { + expect(MemoryStrategyTypeSchema.safeParse('INVALID').success).toBe(false); + expect(MemoryStrategyTypeSchema.safeParse('').success).toBe(false); + expect(MemoryStrategyTypeSchema.safeParse('semantic').success).toBe(false); // lowercase + }); + }); + + describe('schema options', () => { + it('only contains three valid strategies', () => { + expect(MemoryStrategyTypeSchema.options).toEqual(['SEMANTIC', 'SUMMARIZATION', 'USER_PREFERENCE']); + expect(MemoryStrategyTypeSchema.options).not.toContain('CUSTOM'); + }); + }); +}); + +describe('MemoryStrategySchema', () => { + it('validates strategy with required type field', () => { + const result = MemoryStrategySchema.safeParse({ type: 'SEMANTIC' }); + expect(result.success).toBe(true); + }); + + it('validates strategy with optional fields', () => { + const result = MemoryStrategySchema.safeParse({ + type: 'SEMANTIC', + name: 'myStrategy', + description: 'A description', + namespaces: ['/users/{actorId}/facts'], + }); + expect(result.success).toBe(true); + }); + + it('rejects strategy with CUSTOM type', () => { + const result = MemoryStrategySchema.safeParse({ type: 'CUSTOM' }); + expect(result.success).toBe(false); + }); + + it('rejects strategy with invalid type', () => { + const result = MemoryStrategySchema.safeParse({ type: 'INVALID' }); + expect(result.success).toBe(false); + }); + + it('rejects strategy without type', () => { + const result = MemoryStrategySchema.safeParse({ name: 'myStrategy' }); + expect(result.success).toBe(false); + }); +}); + +describe('DEFAULT_STRATEGY_NAMESPACES', () => { + it('has default namespaces for SEMANTIC', () => { + expect(DEFAULT_STRATEGY_NAMESPACES.SEMANTIC).toEqual(['/users/{actorId}/facts']); + }); + + it('has default namespaces for USER_PREFERENCE', () => { + expect(DEFAULT_STRATEGY_NAMESPACES.USER_PREFERENCE).toEqual(['/users/{actorId}/preferences']); + }); + + it('has default namespaces for SUMMARIZATION', () => { + expect(DEFAULT_STRATEGY_NAMESPACES.SUMMARIZATION).toEqual(['/summaries/{actorId}/{sessionId}']); + }); + + it('does not have default namespaces for CUSTOM (removed)', () => { + expect(DEFAULT_STRATEGY_NAMESPACES).not.toHaveProperty('CUSTOM'); + }); +}); diff --git a/src/schema/schemas/primitives/memory.ts b/src/schema/schemas/primitives/memory.ts index 8dc36e3f2..0d3a809c7 100644 --- a/src/schema/schemas/primitives/memory.ts +++ b/src/schema/schemas/primitives/memory.ts @@ -10,16 +10,14 @@ import { z } from 'zod'; * - SEMANTIC → SemanticMemoryStrategy * - SUMMARIZATION → SummaryMemoryStrategy (note: CloudFormation uses "Summary") * - USER_PREFERENCE → UserPreferenceMemoryStrategy - * - CUSTOM → CustomMemoryStrategy * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/aws-properties-bedrockagentcore-memory-memorystrategy.html */ -export const MemoryStrategyTypeSchema = z.enum(['SEMANTIC', 'SUMMARIZATION', 'USER_PREFERENCE', 'CUSTOM']); +export const MemoryStrategyTypeSchema = z.enum(['SEMANTIC', 'SUMMARIZATION', 'USER_PREFERENCE']); export type MemoryStrategyType = z.infer; /** * Default namespaces for each memory strategy type. * These match the patterns generated in CLI session.py templates. - * CUSTOM strategy intentionally has no default namespace. */ export const DEFAULT_STRATEGY_NAMESPACES: Partial> = { SEMANTIC: ['/users/{actorId}/facts'],