feat: add memory primitive

[opieter-aws]

Description

Related Issues

Documentation PR

Type of Change

Bug fix
New feature
Breaking change
Documentation update
Other (please describe):

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

• I ran hatch run prepare

Checklist

• I have read the CONTRIBUTING document
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Assessment: Request Changes

This PR introduces a well-designed MemoryManager primitive for the TypeScript SDK. The architecture is clean — implementing the Plugin interface, exposing tools via getTools(), and providing both programmatic and tool-based access. However, the PR description is empty and this introduces a new public API primitive that requires more thorough documentation and review.

Review Categories

• API Review Process: This introduces a new public primitive (MemoryManager, MemoryStore, etc.) that customers will directly use. It needs needs-api-review label and the PR description should include use cases, example code, and API design rationale.
• PR Description: The description template is unfilled — no motivation, no linked issue, no testing verification. For a feature of this scope, the "WHY" is critical for reviewers.
• Code Quality: The implementation is solid with good error handling, graceful partial failures, and proper scoping. A few edge cases in the tool callbacks could be improved.
• Testing: Comprehensive unit test coverage. A few minor gaps around search tool callback shape and error reporting to the model.

The implementation quality is high — clean separation of concerns, good use of Promise.allSettled for resilience, and thoughtful scoping logic in tools.

[github-actions]

Issue: The MemoryStore interface requires only search as mandatory, with add being optional. However, there's no mechanism for stores to declare other capabilities (e.g., delete, update, list). If these are planned for follow-up PRs, that's fine, but it's worth noting.

More importantly: the search method only accepts SearchOptions with limit. Real vector stores typically need additional parameters like score_threshold, filter (metadata-based filtering), etc. The SearchOptions interface is very minimal.

Suggestion: Consider whether SearchOptions should be generic/extensible (e.g., SearchOptions & Record<string, unknown>) or if the intent is for store implementations to accept additional params through their own constructors. Documenting this design choice would help implementors.

[github-actions]

Issue: The entries schema allows an empty array (z.array(z.string())). If the model passes entries: [], the tool will return { stored: 0, failed: 0 } — a no-op that wastes a tool call.

Suggestion: Add .min(1) to prevent empty invocations:

entries: z.array(z.string()).min(1).describe('Data to store in long-term memory'),

[github-actions]

Issue: The metadata loop silently drops values that aren't string, number, or boolean. Since MemoryStore.add accepts Record<string, JSONValue>, callers can pass arrays and objects which will be lost without warning. This could be confusing when debugging why metadata isn't appearing in the knowledge base.

Suggestion: Either log a warning for skipped keys, or serialize complex values (e.g., JSON.stringify) into a STRING attribute so they're not silently lost:

} else {
  logger.warn(`key=<${key}> | skipping non-primitive metadata value for Bedrock KB attribute`)
}

[github-actions]

Issue: This PR introduces a new public API primitive (MemoryManager, MemoryStore interface, BedrockKnowledgeBaseStore, MemoryManagerConfig, injection system, etc.) that customers will directly use. Per the team's API Bar Raising process, this qualifies as a substantial change ("Adding a new primitive or introducing a new abstraction that customers are expected to frequently use") requiring explicit API reviewer evaluation.

The PR description template is unfilled — no motivation, no linked issue, no use cases, no API design documentation.

Suggestion:

1. Add the needs-api-review label
2. Fill in the PR description with:
   • Expected use cases for memory management
   • Example code snippets from the customer's perspective (config shorthand, class instantiation, custom stores)
   • Complete API signatures with defaults
   • Module exports
   • Design rationale: Why search is opt-out but add is opt-in? Why injection uses the last assistant message as query? Why MemoryStore doesn't extend Plugin/HookProvider?
   • Alternatives considered

[github-actions]

Issue: AgentConfig.sessionManager only accepts a SessionManager instance, while memoryManager accepts both MemoryManager | MemoryManagerConfig. This ergonomic shorthand is nice but creates an inconsistency in the Agent constructor API.

Suggestion: Document why this inconsistency exists (presumably because MemoryManagerConfig is a pure data object while SessionManager requires a storage instance that can't be auto-constructed). If this pattern is intended to be retroactively applied to sessionManager in a follow-up, note that in the PR description.

[github-actions]

Note: Several of my earlier comments (the ones referencing storeToolConfig, store_memory, empty array behavior, and .min(1)) appear to have been posted against an older revision of this code that has since been updated. Please disregard those comments — the current code addresses those issues with _resolveToolTargets, the rename to addToolConfig/add_memory, and the .min(1) constraint already in place. The remaining active concerns are:

1. API Review Process — needs needs-api-review label and PR description (see separate comment)
2. BedrockKnowledgeBaseStore — no unit tests and silent metadata dropping
3. SearchOptions extensibility — (types.ts:70 comment still relevant)
4. AgentConfig inconsistency — sessionManager vs memoryManager pattern

[github-actions]

Issue: The store can be configured with writable: true but without dataSourceId, which means add() will throw at runtime with "dataSourceId is required for write operations." This creates a configuration-time success but runtime failure pattern. Since the MemoryManager validates writable stores have add at construction, users would expect the store to actually be writable.

Suggestion: Consider validating that dataSourceId is present when writable: true in the constructor:

if (this.writable && !this._dataSourceId) {
  throw new Error('BedrockKnowledgeBaseStore: dataSourceId is required when writable is true')
}

This gives immediate feedback rather than a cryptic runtime error on first write.

[github-actions]

Assessment: Request Changes

This PR introduces a well-architected MemoryManager primitive with injection support, multi-store scoping, and a concrete BedrockKnowledgeBaseStore. The core MemoryManager code is solid — good error handling, graceful partial failures, and comprehensive test coverage (657 lines). However, there are process and completeness gaps that should be addressed before merging.

Review Categories

• API Review Process: This is a substantial new public primitive (new interface, new config types, new agent constructor parameter, new subpath export) that requires needs-api-review label and a filled-out PR description per the team's bar-raising process.
• Test Coverage: BedrockKnowledgeBaseStore ships with zero unit tests despite having non-trivial logic (scope filtering, metadata mapping, lazy clients, validation).
• Fail-Fast Validation: The KB store allows writable: true without dataSourceId, deferring the error to runtime. This violates tenet ci: update pre-commit requirement from <4.2.0,>=3.2.0 to >=3.2.0,<4.3.0 #4 ("the obvious path is the happy path").
• Silent Data Loss: Complex metadata values (arrays, objects) are silently dropped in the KB store's add() method.

The MemoryManager itself is well-implemented with strong test coverage and thoughtful design choices (injection stripping, _resolveToolTargets throwing on fully-invalid scopes).