Skip to content

fix: remove CUSTOM memory strategy temporarily - #266

Merged
jesseturner21 merged 1 commit into
mainfrom
fix/issue-235-remove-custom-memory
Feb 11, 2026
Merged

fix: remove CUSTOM memory strategy temporarily#266
jesseturner21 merged 1 commit into
mainfrom
fix/issue-235-remove-custom-memory

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

  • Remove CUSTOM from MemoryStrategyType as it is not yet supported
  • This is a P0 fix to prevent users from selecting an unsupported option
  • Add comprehensive tests for memory strategy validation

Changes

  • Remove CUSTOM from MemoryStrategyTypeSchema enum in schema
  • Update validation logic to reject CUSTOM strategy
  • Update CLI help text and documentation
  • Add schema-level tests for MemoryStrategyTypeSchema
  • Add validation tests for CUSTOM rejection
  • Add integration test for CLI rejection of CUSTOM

Test plan

  • All existing tests pass (307 passed)
  • New tests verify CUSTOM is rejected at schema level
  • New tests verify CUSTOM is rejected at validation level
  • New integration test verifies CLI rejects CUSTOM strategy
  • Build succeeds

Closes #235

@tejaskash
tejaskash requested a review from a team February 10, 2026 23:34
@github-actions

github-actions Bot commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 8.09% 532 / 6568
🔵 Statements 7.78% 543 / 6978
🔵 Functions 5.47% 73 / 1333
🔵 Branches 6.05% 230 / 3800
Generated in workflow #257 for commit 714adc8 by the Vitest Coverage Report Action

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
@tejaskash
tejaskash force-pushed the fix/issue-235-remove-custom-memory branch from 7d71fa9 to 714adc8 Compare February 10, 2026 23:40

@aidandaly24 aidandaly24 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jesseturner21
jesseturner21 merged commit b2fc32b into main Feb 11, 2026
14 checks passed
@jesseturner21
jesseturner21 deleted the fix/issue-235-remove-custom-memory branch February 11, 2026 00:13
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the diff post-merge — changes look good and well-scoped:

  • Schema enum updated cleanly in src/schema/schemas/primitives/memory.ts
  • CLI help text, validation logic, and TUI strategy descriptions all kept in sync
  • Solid test coverage at both the schema level (MemoryStrategyTypeSchema/MemoryStrategySchema) and the CLI level (add-memory.test.ts, validate.test.ts)
  • LLM-compacted schema docs (src/schema/llm-compacted/agentcore.ts) and AGENTS.md were also updated, so AI agents using this CLI won't be misled into suggesting CUSTOM

No new issues to flag. 👍

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the report, @agentcore-cli-automation — feedback like this is exactly
how we catch the things we missed. Because this PR is already
closed, the team won't see follow-up comments here.

Would you mind opening a new issue so we can track it properly?
https://github.com/aws/agentcore-cli/issues/new/choose

If this is a security issue, please report it privately via
https://aws.amazon.com/security/vulnerability-reporting/ instead
of a public issue.

notgitika added a commit to notgitika/agentcore-cli that referenced this pull request Aug 18, 2026
Reverts 87be86e. I added CUSTOM because the CDK schema already had it in
MemoryStrategyTypeSchema, which turns out to be the argument PR aws#694 made --
and aws#713 reverted a day later.

The CLI has removed CUSTOM twice on purpose. Offering the type without
somewhere to put its extraction configuration is aws#241 ("select custom memory
strategy, note there is no option to add prompts"); aws#266 removed it as a P0 to
stop users picking an unsupported option, aws#694/aws#696 added it back with
semanticOverride, and aws#713 reverted both as premature. aws#676 tracks doing it
properly. The CDK keeping CUSTOM in its enum without a configuration field is
the same hole, not a licence.

So both forms are rejected again, now with an error that says why and points
at aws#676. The one thing kept from the reverted commit: memory validation errors
name the offending field, since issue.path was being dropped.
notgitika added a commit to notgitika/agentcore-cli that referenced this pull request Aug 19, 2026
Reverts 87be86e. I added CUSTOM because the CDK schema already had it in
MemoryStrategyTypeSchema, which turns out to be the argument PR aws#694 made --
and aws#713 reverted a day later.

The CLI has removed CUSTOM twice on purpose. Offering the type without
somewhere to put its extraction configuration is aws#241 ("select custom memory
strategy, note there is no option to add prompts"); aws#266 removed it as a P0 to
stop users picking an unsupported option, aws#694/aws#696 added it back with
semanticOverride, and aws#713 reverted both as premature. aws#676 tracks doing it
properly. The CDK keeping CUSTOM in its enum without a configuration field is
the same hole, not a licence.

So both forms are rejected again, now with an error that says why and points
at aws#676. The one thing kept from the reverted commit: memory validation errors
name the offending field, since issue.path was being dropped.
notgitika added a commit to notgitika/agentcore-cli that referenced this pull request Aug 20, 2026
Reverts 87be86e. I added CUSTOM because the CDK schema already had it in
MemoryStrategyTypeSchema, which turns out to be the argument PR aws#694 made --
and aws#713 reverted a day later.

The CLI has removed CUSTOM twice on purpose. Offering the type without
somewhere to put its extraction configuration is aws#241 ("select custom memory
strategy, note there is no option to add prompts"); aws#266 removed it as a P0 to
stop users picking an unsupported option, aws#694/aws#696 added it back with
semanticOverride, and aws#713 reverted both as premature. aws#676 tracks doing it
properly. The CDK keeping CUSTOM in its enum without a configuration field is
the same hole, not a licence.

So both forms are rejected again, now with an error that says why and points
at aws#676. The one thing kept from the reverted commit: memory validation errors
name the offending field, since issue.path was being dropped.
notgitika added a commit to notgitika/agentcore-cli that referenced this pull request Aug 21, 2026
Reverts 87be86e. I added CUSTOM because the CDK schema already had it in
MemoryStrategyTypeSchema, which turns out to be the argument PR aws#694 made --
and aws#713 reverted a day later.

The CLI has removed CUSTOM twice on purpose. Offering the type without
somewhere to put its extraction configuration is aws#241 ("select custom memory
strategy, note there is no option to add prompts"); aws#266 removed it as a P0 to
stop users picking an unsupported option, aws#694/aws#696 added it back with
semanticOverride, and aws#713 reverted both as premature. aws#676 tracks doing it
properly. The CDK keeping CUSTOM in its enum without a configuration field is
the same hole, not a licence.

So both forms are rejected again, now with an error that says why and points
at aws#676. The one thing kept from the reverted commit: memory validation errors
name the offending field, since issue.path was being dropped.
notgitika added a commit to notgitika/agentcore-cli that referenced this pull request Aug 21, 2026
Reverts 87be86e. I added CUSTOM because the CDK schema already had it in
MemoryStrategyTypeSchema, which turns out to be the argument PR aws#694 made --
and aws#713 reverted a day later.

The CLI has removed CUSTOM twice on purpose. Offering the type without
somewhere to put its extraction configuration is aws#241 ("select custom memory
strategy, note there is no option to add prompts"); aws#266 removed it as a P0 to
stop users picking an unsupported option, aws#694/aws#696 added it back with
semanticOverride, and aws#713 reverted both as premature. aws#676 tracks doing it
properly. The CDK keeping CUSTOM in its enum without a configuration field is
the same hole, not a licence.

So both forms are rejected again, now with an error that says why and points
at aws#676. The one thing kept from the reverted commit: memory validation errors
name the offending field, since issue.path was being dropped.
jariy17 pushed a commit that referenced this pull request Aug 24, 2026
* feat(project): add `project add memory`

Registers a `memory` leaf under `project add`, following the same
SDK-union -> flat project-schema conversion pattern as `project add
harness`. A memory scaffolds no files, so the command only appends an
entry to `spec.memories` in agentcore.json; the L3 CDK turns that into
an `AWS::BedrockAgentCore::Memory` at deploy time.

Flags: --name, --event-expiry-duration, --strategies, --indexed-keys,
--stream-delivery-resources, --encryption-key-arn, --execution-role-arn,
--tags.

--strategies accepts two forms: a comma-separated list of strategy types
expanded with the CLI's default namespace templates, or a JSON
MemoryStrategyInput[] mirroring the CreateMemory API for strategies that
need explicit names, descriptions, or namespaces.

clientToken is excluded (it is CreateMemory idempotency and this command
makes no API call), and description is excluded until the L3 CDK schema
supports it.

* feat: add --description to 'project add memory'

Stores an optional memory description in agentcore.json, matching the
CreateMemory API's description field (max 4096 characters).

The generated CDK app pins @aws/agentcore-cdk 0.1.0-alpha.45, whose
MemorySchema is a non-strict z.object with no description field, so the key
is stripped at synth rather than rejected until
aws/agentcore-l3-cdk-constructs#325 ships and that pin is bumped. The flag
help text says so.

* feat: accept a CUSTOM memory strategy in 'project add memory'

The CDK's memory schema already models CUSTOM (@aws/agentcore-cdk
0.1.0-alpha.45 maps it to CFN customMemoryStrategy), so the CLI's four-type
enum was the outlier. A customMemoryStrategy in the --strategies JSON now
converts to { type: 'CUSTOM', name, description, namespaceTemplates }.

The shorthand form still takes managed types only: CUSTOM has no default
namespaces to expand. An extraction configuration or memoryRecordSchema is
rejected rather than dropped, since the CDK schema carries neither.

Also names the offending field in the memory validation error.

* revert: drop CUSTOM memory strategy from "project add memory"

Reverts 87be86e. I added CUSTOM because the CDK schema already had it in
MemoryStrategyTypeSchema, which turns out to be the argument PR #694 made --
and #713 reverted a day later.

The CLI has removed CUSTOM twice on purpose. Offering the type without
somewhere to put its extraction configuration is #241 ("select custom memory
strategy, note there is no option to add prompts"); #266 removed it as a P0 to
stop users picking an unsupported option, #694/#696 added it back with
semanticOverride, and #713 reverted both as premature. #676 tracks doing it
properly. The CDK keeping CUSTOM in its enum without a configuration field is
the same hole, not a licence.

So both forms are rejected again, now with an error that says why and points
at #676. The one thing kept from the reverted commit: memory validation errors
name the offending field, since issue.path was being dropped.

* refactor: drop the long-form help for --description

The one-line flag description is enough; the deploy-time caveat lives in the
PR discussion rather than in help output.

* docs: comment change

* fix: change function name and add comment for clarity

* test: add uncovered unsupported stream content type test

* style: make json example concrete, remove comments

* refactor(project): reuse shared spec validation for memory

* fix(project): validate memory JSON inputs

* fix(project): harden memory input validation

* test(project): colocate memory tests with the add/memory handler

Upstream moved the per-resource `project add` tests out of the monolithic
project.test.ts into colocated add/<resource>/index.test.ts suites (harness
in #2034, online-eval in #2048). Move the memory tests to match, with the
same locally-duplicated run/inProject helpers those suites use.

project.test.ts is now identical to upstream/refactor again, so this PR no
longer touches it. Also drops the DeserializationError, FsReadWriteJson and
ReadWriteJson imports, left dead there once the harness tests that used them
moved to add/harness/index.test.ts.

No test content changed: 187 project tests still pass, now across 10 files
instead of 9.

* refactor(project): match --strategies JSON to the agentcore.json schema

The --strategies flag re-declared its own strategy input schema, modelled
on the CreateMemory API's tagged union (semanticMemoryStrategy et al.) and
requiring a name. agentcore.json stores strategies flat with an optional
name, so the flag accepted a shape the project file never holds and
rejected one it does.

Parse the JSON form with MemoryStrategySchema itself, wrapped only for the
unsupported-field diagnostics, so the flag cannot drift from the schema.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P0] Remove CUSTOM memory strategy temporarily

4 participants