feat(agent-memory): add Redis Agent Memory API module - #6424
booleanhunter wants to merge 1 commit into
Conversation
5c289be to
68cbd32
Compare
Code Coverage - Backend unit tests
Test suite run success3795 tests passing in 327 suites. Report generated by 🧪jest coverage report action from 4b2749b |
Code Coverage - Integration Tests
|
68cbd32 to
3dc5bc5
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dc5bc5121
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const data = await this.sdkCall(() => | ||
| this.sdk.searchLongTermMemory({ text: MATCH_ALL_TEXT }), | ||
| ); | ||
| const items = data?.items ?? []; |
There was a problem hiding this comment.
When a store contains more memories than one search response can return, this scans only the first items page and silently omits owners and namespaces that occur later. The normal search path explicitly uses a bounded limit, while this discovery endpoint exposes no pagination, so its filter options become incomplete; paginate through all results or use a server-side aggregation endpoint.
Useful? React with 👍 / 👎.
| serviceName: store.name ?? store.serviceName ?? this.endpoint.name, | ||
| storeId: store.id ?? this.endpoint.storeId, | ||
| database: store.databaseName ?? store.database ?? store.databaseId, | ||
| endpoint: store.endpoint ?? store.publicEndpoint ?? this.endpoint.url, | ||
| }; |
There was a problem hiding this comment.
For stores with working- or long-term-memory TTLs configured, GET /agent-memory/:id/config can never return those settings even though the controller advertises “general settings + memory TTLs”: this mapping and AgentMemoryConfiguration contain only identity and endpoint fields. Add the TTL fields to the response contract and populate them, or narrow the documented contract if TTLs are intentionally unavailable.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| items.map((m) => m.namespace).filter((ns): ns is string => Boolean(ns)), | ||
| ), | ||
| ]; | ||
| return { users, namespaces }; |
There was a problem hiding this comment.
Discovery scan misses most store facets
High Severity
discoverFilters builds distinct owners and namespaces from a single searchLongTermMemory call with only a match-all text and no limit or pagination. That returns one default-sized page, so owners and namespaces that appear only on later pages never show up in discovery.
Reviewed by Cursor Bugbot for commit 3dc5bc5. Configure here.
3dc5bc5 to
d93f849
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d93f849a02
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| async ensureAuth(): Promise<void> { | ||
| // Static bearer token - nothing to refresh. | ||
| } |
There was a problem hiding this comment.
When a client is already pooled, AgentMemoryClientProvider.getOrCreate() calls this no-op and the /agent-memory/:id/connect endpoint returns 200 without making any request. If the store becomes unavailable or its API key is revoked after the initial connection, repeated connect checks continue reporting success while the pool remains active; perform an authenticated probe for explicit connect requests or revalidate cached clients here.
Useful? React with 👍 / 👎.
| this.sdk.listSessions( | ||
| SESSIONS_LIST_LIMIT, | ||
| undefined, | ||
| filter.userId || undefined, | ||
| filter.userId ? undefined : true, |
There was a problem hiding this comment.
For stores containing more than 50 sessions, this request always returns only the first page because the SDK offset argument is left undefined and the controller exposes no pagination parameters. Consequently, later sessions cannot be discovered or opened through this API; iterate through the SDK pages or expose pagination in the REST contract.
Useful? React with 👍 / 👎.
| @UsePipes( | ||
| new ValidationPipe({ | ||
| transform: true, | ||
| transformOptions: { groups: ['security'] }, | ||
| }), |
There was a problem hiding this comment.
Because this ValidationPipe does not enable whitelist, undeclared properties survive transformation even though the DTO uses OmitType. A request such as PATCH /agent-memory/A with an id field therefore reaches deepMerge, replaces the loaded model's primary key, and can make the repository save data under a different endpoint ID instead of updating A; enable whitelisting or explicitly reject non-DTO fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d93f849. Configure here.
| ), | ||
| ); | ||
| return (data?.items ?? []).filter((id): id is string => Boolean(id)); | ||
| } |
There was a problem hiding this comment.
Session list silently truncates after fifty
Medium Severity
listSessions always requests SESSIONS_LIST_LIMIT (50) and never follows pagination, so GET /agent-memory/:id/sessions cannot return more than the first page of session ids even when the store has more.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d93f849. Configure here.
| }), | ||
| ); | ||
| const items = (data?.items ?? []).map(fromCloudMemory); | ||
| return { memories: items, total: items.length }; |
There was a problem hiding this comment.
Search total reflects page size only
Medium Severity
Long-term search always passes limit: 50 and sets total to the returned page length, with no pageToken handling. Clients treating total as the store hit count will under-count matches and cannot fetch further pages.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d93f849. Configure here.
|
Related to #6226 |
d93f849 to
e0f4271
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0f4271444
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| default: | ||
| return new InternalServerErrorException(message); |
There was a problem hiding this comment.
When the Agent Memory service responds with an unlisted status such as 429, this default branch converts it to 500. Clients then cannot distinguish throttling from an internal failure or apply the appropriate retry behavior; preserve valid upstream 4xx/5xx statuses or explicitly map operational statuses such as 409 and 429.
Useful? React with 👍 / 👎.
| ): AgentMemoryMessage => ({ | ||
| id: event?.eventId, | ||
| role: (event?.role ?? 'user').toLowerCase(), | ||
| content: event?.content?.[0]?.text ?? '', |
There was a problem hiding this comment.
When a RAM event contains multiple content blocks, this transformer returns only the first block's text and silently discards the rest. Since the SDK represents event content as an array, externally produced multipart messages will appear truncated in the inspector; combine all applicable text blocks or retain their structured representation.
Useful? React with 👍 / 👎.
Add a NestJS module that connects RedisInsight to Redis Agent Memory (RAM) stores through the @redis-iris/agent-memory SDK (0.2.0). Endpoints: - CRUD for endpoint connections, credentials encrypted at rest - Verify reachability and credentials on connect - backendType discriminator fixed to Cloud, leaving room for other transports without a schema migration Working memory: - Read a session's message log, running summary, and namespace - Add events (creating the session if needed) - Clear a session Long-term memory: - Semantic + keyword search with a similarity-threshold control, filtered by owner, namespace, session, memory type, and topics - Delete records - Discover the distinct owners and namespaces present in a store Store configuration: - Read the store's general settings and memory TTLs
e0f4271 to
4b2749b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b2749b058
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| case 'SDKValidationError': | ||
| case 'ResponseValidationError': | ||
| return new BadRequestException(message); |
There was a problem hiding this comment.
When the SDK accepts a request but cannot validate the Agent Memory service's response—for example, after an upstream response-schema change—it throws ResponseValidationError; mapping that error to 400 incorrectly tells clients their request was invalid. Return a server/upstream failure such as 502 or 500 for this case, while reserving 400 for SDKValidationError.
Useful? React with 👍 / 👎.
1 participant
Add a NestJS module that connects RedisInsight to Redis Agent Memory (RAM) stores through the @redis-iris/agent-memory SDK (0.2.0).
Endpoints:
Working memory:
Long-term memory:
Store configuration:
Note
High Risk
New persistence of encrypted API keys plus proxy APIs that read, append, and delete session and long-term memory on Redis Cloud. Credential handling, encryption, and destructive data operations are security-sensitive.
Overview
Adds a backend Agent Memory module so RedisInsight can connect to Redis Cloud RAM stores and inspect or mutate working and long-term memory.
Connections: CRUD under
/agent-memorywith a SQLiteagent_memory_endpointtable.apiKeyis encrypted at rest, omitted from list/get responses, and never returned to the client. Create/update/connect validate reachability via the@redis-iris/agent-memorySDK. Clients are pooled per session and idle-swept.Data APIs (
/agent-memory/:id): list sessions; get/append/clear working memory; hybrid long-term search (filters + similarity threshold); bulk delete memories; discover owners/namespaces; fetch store config. Cloud responses are normalized to a shared camelCase contract.backendTypeis Cloud-only for now.Reviewed by Cursor Bugbot for commit 4b2749b. Bugbot is set up for automated code reviews on this repo. Configure here.