feat: add memory_update tool for memory versioning (#1018)#1038
feat: add memory_update tool for memory versioning (#1018)#1038ziishanahmad wants to merge 2 commits into
Conversation
- Add mem::update engine function: updates existing memory by ID, increments version, preserves fields not provided, re-indexes BM25 + vector index, records audit trail - Add REST API endpoint PUT /agentmemory/memories/update - Add memory_update MCP tool definition in tools-registry.ts - Add memory_update to standalone server (validate, handleLocal, handleProxy) so it works in both connected and fallback modes - Add 9 tests covering: version increment, in-place update (no duplicate), field preservation, type/concept overwrite, not-found error, missing memoryId, missing content, multi-update versioning, no-duplicate guarantee - Update existing standalone test counts (CORE_TOOLS 14→15, IMPLEMENTED_TOOLS 7→8)
|
@ziishanahmad is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Changesmemory_update feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant API as api::memory-update
participant MemUpdate as mem::update
participant KV as KV Store
participant Search as BM25/Vector Index
Client->>API: PUT /agentmemory/memories/update
API->>API: validate memoryId, content
API->>MemUpdate: forward update payload
MemUpdate->>KV: load existing memory
MemUpdate->>KV: save old (isLatest=false) and updated memory
MemUpdate->>Search: remove old entry, re-index updated memory
MemUpdate-->>API: success, updated memory
API-->>Client: 200 with updated memory (or 404 if not found)
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mcp/tools-registry.ts (1)
962-971: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
memory_updatemissing fromESSENTIAL_TOOLS.
memory_updateis registered inCORE_TOOLSandIMPLEMENTED_TOOLS, butESSENTIAL_TOOLS(used to filter tools whenAGENTMEMORY_TOOLS=core) still omits it. Users running in core/minimal mode won't have access to this tool and will fall back to the exact save+delete workaround this PR is meant to eliminate (per issue#1018).🛡️ Proposed fix
export const ESSENTIAL_TOOLS = new Set([ "memory_save", + "memory_update", "memory_recall",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mcp/tools-registry.ts` around lines 962 - 971, Add `memory_update` to `ESSENTIAL_TOOLS` so core/minimal tool filtering includes it; update the `ESSENTIAL_TOOLS` set in `tools-registry.ts` to match the already-registered `CORE_TOOLS`/`IMPLEMENTED_TOOLS` entries. Use the `ESSENTIAL_TOOLS` symbol to locate the set and ensure `memory_update` is included alongside the other memory tools.
🧹 Nitpick comments (1)
test/memory-update.test.ts (1)
1-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWell-structured test suite for
mem::updateversioning.The 9 test cases comprehensively cover version increment, in-place ID preservation, field preservation/overwrite, all three error paths (matching upstream error strings), repeated updates, and duplicate prevention. The mock strategy is sound — all mocked modules (
logger,keyed-mutex,audit,access-tracker,config) are runtime dependencies ofremember.ts, and the DI approach viamakeMockSdk()/makeMockKV()is valid sinceregisterRememberFunctionacceptssdk/kvas parameters. Based on learnings,vi.mockis only effective for modules used at runtime; here the mocked modules are all runtime deps, so the mocks are correctly applied.Consider adding coverage for:
ttlDaysupdate — the upstream code computes a newforgetAftertimestamp, which is currently untested.supersedesarray — each update appendsexisting.idtosupersedes; verifying this after multiple updates would catch potential duplication in the array.agentIdoverride — the upstream code usesdata.agentIdwhen provided, falling back togetAgentId().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/memory-update.test.ts` around lines 1 - 247, The mem::update test suite is missing coverage for a few behaviors in registerRememberFunction/update handling. Add tests that verify ttlDays produces the expected updated forgetAfter value, that repeated updates append the previous ID into the supersedes array without breaking existing history, and that providing agentId in the update payload overrides the getAgentId fallback. Use the existing mem::update trigger flow and the mem::remember/mem::update symbols to locate the relevant path.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/functions/remember.ts`:
- Around line 216-219: The agentId handling in remember/update paths can
overwrite or drop the existing owner tag when data.agentId is missing, because
it falls back to getAgentId() instead of preserving existing.agentId. Update the
logic around callAgentId in remember.ts so memory_update keeps the prior agentId
unless an explicit new agentId is provided, matching the preservation behavior
used for fields like sessionIds, strength, and project. Apply the same fix
anywhere the update flow reuses this fallback so isolated agent scope does not
accidentally reassign or hide memories.
- Around line 250-257: The version update in remember.ts writes the same memory
twice through kv.set, so the pre-write on existing.id is redundant and can
briefly expose a stale isLatest state. Remove the first write that persists
existing after setting isLatest to false and updatedAt, and keep only the final
kv.set for updated.id in the update flow around existing and updated so the
latest record is stored once atomically from the caller’s perspective.
- Around line 196-201: The memory write paths are using different lock keys even
though they both mutate the same KV.memories record, so concurrent
remember/update operations can interleave and clobber versioning state. Update
the locking in the mem:update flow inside remember.ts to use the same shared
lock as the mem:remember path (or a single common lock helper) so both
read-modify-write sequences serialize on the same key. Keep the fix localized
around withKeyedLock and the memory write methods that touch KV.memories[id].
- Line 234: The supersedes chain in the in-place update path is incorrectly
appending the current record’s ID, creating a self-reference and distorting
lineage. Update the logic in remember.ts where the object is rebuilt so that the
`supersedes` field preserves the existing chain only, without adding
`existing.id` again; use the update flow around `updated`/`existing` to locate
it.
- Around line 176-194: The mem::update handler in remember.ts validates memoryId
and content but does not verify concepts or files before persisting them. Add
the same array-type checks used in mem::remember inside the update function so
api::memory-update rejects non-array payloads for concepts/files early. Keep the
validation alongside the existing input guards in the async handler to ensure
only arrays are stored.
In `@src/mcp/standalone.ts`:
- Around line 129-146: The memory_update validation is always populating
concepts and files with empty arrays, which causes preserved values to be wiped
on every update. In validate() for the memory_update case in standalone.ts, only
assign v.concepts and v.files when the corresponding args["concepts"] /
args["files"] are actually provided, matching the existing conditional handling
used for v.type. Then ensure handleProxy and handleLocal can distinguish “not
provided” from “provided empty list” so existing memory fields are preserved
unless the caller explicitly sends values.
- Around line 322-327: The local fallback in standalone.ts is adding the updated
record’s own identifier to its supersedes list, causing a self-referential
entry. Update the logic around the object built from existing["supersedes"] so
it excludes v.memoryId! when the record is being written back under the same
key, and only append prior version IDs or other distinct superseded IDs. Check
the fallback update path in the same area that mirrors the mem::update behavior
from remember.ts.
---
Outside diff comments:
In `@src/mcp/tools-registry.ts`:
- Around line 962-971: Add `memory_update` to `ESSENTIAL_TOOLS` so core/minimal
tool filtering includes it; update the `ESSENTIAL_TOOLS` set in
`tools-registry.ts` to match the already-registered
`CORE_TOOLS`/`IMPLEMENTED_TOOLS` entries. Use the `ESSENTIAL_TOOLS` symbol to
locate the set and ensure `memory_update` is included alongside the other memory
tools.
---
Nitpick comments:
In `@test/memory-update.test.ts`:
- Around line 1-247: The mem::update test suite is missing coverage for a few
behaviors in registerRememberFunction/update handling. Add tests that verify
ttlDays produces the expected updated forgetAfter value, that repeated updates
append the previous ID into the supersedes array without breaking existing
history, and that providing agentId in the update payload overrides the
getAgentId fallback. Use the existing mem::update trigger flow and the
mem::remember/mem::update symbols to locate the relevant path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b3c7ddc-ff08-4a33-9bd4-04c42c799b5c
📒 Files selected for processing (7)
src/functions/remember.tssrc/mcp/standalone.tssrc/mcp/tools-registry.tssrc/triggers/api.tstest/mcp-standalone-proxy.test.tstest/mcp-standalone.test.tstest/memory-update.test.ts
| async (data: { | ||
| memoryId: string; | ||
| content: string; | ||
| type?: string; | ||
| concepts?: string[]; | ||
| files?: string[]; | ||
| ttlDays?: number; | ||
| agentId?: string; | ||
| }) => { | ||
| if (!data.memoryId || typeof data.memoryId !== "string") { | ||
| return { success: false, error: "memoryId is required" }; | ||
| } | ||
| if ( | ||
| !data.content || | ||
| typeof data.content !== "string" || | ||
| !data.content.trim() | ||
| ) { | ||
| return { success: false, error: "content is required" }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/functions/remember.ts outline ==\n'
ast-grep outline src/functions/remember.ts --view expanded || true
printf '\n== src/triggers/api.ts outline ==\n'
ast-grep outline src/triggers/api.ts --view expanded || true
printf '\n== search for memoryToObservation and normalizeList ==\n'
rg -n "memoryToObservation|normalizeList|api::memory-update|mem::remember|mem::update|concepts|files" src/functions/remember.ts src/triggers/api.ts || true
printf '\n== relevant slices from src/functions/remember.ts ==\n'
sed -n '1,280p' src/functions/remember.ts | cat -n | sed -n '1,280p'
printf '\n== relevant slices from src/triggers/api.ts ==\n'
sed -n '1,260p' src/triggers/api.ts | cat -n | sed -n '1,260p'Repository: rohitg00/agentmemory
Length of output: 27900
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate memoryToObservation ==\n'
rg -n "function memoryToObservation|const memoryToObservation|memoryToObservation\\(" src || true
printf '\n== locate array iteration/join on concepts/files ==\n'
rg -n "concepts\\.|files\\.|join\\(|forEach\\(|map\\(" src/functions src/triggers || trueRepository: rohitg00/agentmemory
Length of output: 23234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/state/memory-utils.ts ==\n'
wc -l src/state/memory-utils.ts
sed -n '1,220p' src/state/memory-utils.ts | cat -n
printf '\n== api::memory-remember and api::memory-update slice ==\n'
sed -n '940,1055p' src/triggers/api.ts | cat -nRepository: rohitg00/agentmemory
Length of output: 6312
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CompressedObservation / Memory types ==\n'
rg -n "type CompressedObservation|interface CompressedObservation|type Memory|interface Memory" src/types.* src || true
printf '\n== memory observation consumers ==\n'
rg -n "concepts\\.join\\(|files\\.join\\(|concepts\\?\\.|files\\?\\.|\\.concepts\\b|\\.files\\b" src/state src/functions | sed -n '1,220p'
printf '\n== focused memory-related search consumers ==\n'
sed -n '1,380p' src/state/hybrid-search.ts | cat -n | sed -n '250,340p'
sed -n '1,220p' src/state/memory-utils.ts | cat -n
sed -n '1,260p' src/state/types.* 2>/dev/null | cat -n || trueRepository: rohitg00/agentmemory
Length of output: 9364
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/state/search-index.ts slice ==\n'
sed -n '220,280p' src/state/search-index.ts | cat -n
printf '\n== src/types.ts Memory / CompressedObservation ==\n'
sed -n '40,120p' src/types.ts | cat -nRepository: rohitg00/agentmemory
Length of output: 4441
Validate concepts and files in mem::update src/functions/remember.ts:176-194
Mirror the mem::remember array checks here. api::memory-update forwards concepts/files directly, so a non-array payload can still be persisted and later break search/index behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/functions/remember.ts` around lines 176 - 194, The mem::update handler in
remember.ts validates memoryId and content but does not verify concepts or files
before persisting them. Add the same array-type checks used in mem::remember
inside the update function so api::memory-update rejects non-array payloads for
concepts/files early. Keep the validation alongside the existing input guards in
the async handler to ensure only arrays are stored.
Source: Path instructions
| return withKeyedLock("mem:update", async () => { | ||
| const existing = await kv.get<Memory>(KV.memories, data.memoryId); | ||
| if (!existing) { | ||
| return { success: false, error: "memory not found" }; | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "withKeyedLock" src --type=ts -C2Repository: rohitg00/agentmemory
Length of output: 27870
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the memory function around remember/update and the keyed lock helper.
sed -n '1,260p' src/functions/remember.ts
printf '\n--- keyed mutex ---\n'
sed -n '1,220p' src/state/keyed-mutex.tsRepository: rohitg00/agentmemory
Length of output: 10354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the rest of the mem::update implementation and any related helpers in remember.ts.
sed -n '260,420p' src/functions/remember.tsRepository: rohitg00/agentmemory
Length of output: 4365
Use a shared lock for memory writes
mem::update and mem::remember both read-modify-write KV.memories[id], but they serialize on different lock keys (mem:update vs mem:remember). A concurrent supersede can overwrite the same record’s version/isLatest state and drop a just-written update.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/functions/remember.ts` around lines 196 - 201, The memory write paths are
using different lock keys even though they both mutate the same KV.memories
record, so concurrent remember/update operations can interleave and clobber
versioning state. Update the locking in the mem:update flow inside remember.ts
to use the same shared lock as the mem:remember path (or a single common lock
helper) so both read-modify-write sequences serialize on the same key. Keep the
fix localized around withKeyedLock and the memory write methods that touch
KV.memories[id].
| const callAgentId = | ||
| typeof data.agentId === "string" && data.agentId.trim().length > 0 | ||
| ? data.agentId.trim().slice(0, 128) | ||
| : getAgentId(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
agentId fallback can silently reassign/drop the memory's owner tag.
When data.agentId isn't supplied, callAgentId falls back to getAgentId() (the current process's agent identity), not existing.agentId. If the record was tagged with a different agent (or getAgentId() returns undefined), a plain memory_update call will overwrite or entirely drop the original agentId — unlike every other preserved field (sessionIds, strength, project, etc.). In AGENTMEMORY_AGENT_SCOPE=isolated mode this can move a memory out of its owner's visible scope or make it disappear from agent-filtered listings (see api::sessions/api::memories agentId filtering).
🛡️ Proposed fix
const callAgentId =
typeof data.agentId === "string" && data.agentId.trim().length > 0
? data.agentId.trim().slice(0, 128)
- : getAgentId();
+ : existing.agentId;Also applies to: 240-240
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/functions/remember.ts` around lines 216 - 219, The agentId handling in
remember/update paths can overwrite or drop the existing owner tag when
data.agentId is missing, because it falls back to getAgentId() instead of
preserving existing.agentId. Update the logic around callAgentId in remember.ts
so memory_update keeps the prior agentId unless an explicit new agentId is
provided, matching the preservation behavior used for fields like sessionIds,
strength, and project. Apply the same fix anywhere the update flow reuses this
fallback so isolated agent scope does not accidentally reassign or hide
memories.
| strength: existing.strength, | ||
| version: newVersion, | ||
| parentId: existing.parentId, | ||
| supersedes: [...(existing.supersedes ?? []), existing.id], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant file and nearby usages of `supersedes`
git ls-files 'src/functions/remember.ts' 'src/**/*.ts' | sed -n '1,20p'
echo '--- remember.ts excerpt ---'
nl -ba src/functions/remember.ts | sed -n '180,280p'
echo '--- search supersedes usages ---'
rg -n "supersedes" src --glob '*.ts'Repository: rohitg00/agentmemory
Length of output: 776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- remember.ts lines 210-270 ---'
sed -n '210,270p' src/functions/remember.ts
echo '--- all supersedes usages ---'
grep -RIn --include='*.ts' 'supersedes' src || trueRepository: rohitg00/agentmemory
Length of output: 4942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- remember.ts lines 80-130 ---'
sed -n '80,130p' src/functions/remember.ts
echo '--- relations.ts lines 130-180 ---'
sed -n '130,180p' src/functions/relations.ts
echo '--- diagnostics.ts supersedes checks ---'
sed -n '320,340p' src/functions/diagnostics.tsRepository: rohitg00/agentmemory
Length of output: 4670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- remember.ts update path around in-place save ---'
sed -n '220,255p' src/functions/remember.ts
echo '--- docs/comments mentioning same ID or supersedes semantics ---'
grep -RIn --include='*.ts' -E 'same ID|same id|supersedes.*self|lineage|version' src/functions src/mcp src/types.ts | sed -n '1,200p'Repository: rohitg00/agentmemory
Length of output: 6279
Avoid appending the current ID to supersedes on in-place updates. updated.id is still existing.id, so this creates a self-reference and distorts lineage; keep the previous chain unchanged instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/functions/remember.ts` at line 234, The supersedes chain in the in-place
update path is incorrectly appending the current record’s ID, creating a
self-reference and distorting lineage. Update the logic in remember.ts where the
object is rebuilt so that the `supersedes` field preserves the existing chain
only, without adding `existing.id` again; use the update flow around
`updated`/`existing` to locate it.
| // Mark the old version as no longer latest. | ||
| existing.isLatest = false; | ||
| existing.updatedAt = now; | ||
| await kv.set(KV.memories, existing.id, existing); | ||
|
|
||
| // Save the updated memory in-place (same ID, new version). | ||
| await kv.set(KV.memories, updated.id, updated); | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the referenced lines.
file='src/functions/remember.ts'
wc -l "$file"
sed -n '200,320p' "$file"
# Find the update function and any lock / search-list code that may observe intermediate state.
rg -n "mem:update|isLatest|updatedAt|kv\.set\(KV\.memories|KV\.memories|search|list" src/functions/remember.ts src -g '*.ts'Repository: rohitg00/agentmemory
Length of output: 50376
Remove the redundant pre-write. existing.id and updated.id are the same, so the first kv.set() is overwritten immediately. It only adds I/O and briefly exposes a stale isLatest: false record to unlocked readers between awaits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/functions/remember.ts` around lines 250 - 257, The version update in
remember.ts writes the same memory twice through kv.set, so the pre-write on
existing.id is redundant and can briefly expose a stale isLatest state. Remove
the first write that persists existing after setting isLatest to false and
updatedAt, and keep only the final kv.set for updated.id in the update flow
around existing and updated so the latest record is stored once atomically from
the caller’s perspective.
| case "memory_update": { | ||
| const memoryId = args["memoryId"]; | ||
| if (typeof memoryId !== "string" || !memoryId.trim()) { | ||
| throw new Error("memoryId is required"); | ||
| } | ||
| const content = args["content"]; | ||
| if (typeof content !== "string" || !content.trim()) { | ||
| throw new Error("content is required"); | ||
| } | ||
| v.memoryId = memoryId; | ||
| v.content = content; | ||
| if (typeof args["type"] === "string" && args["type"].trim()) { | ||
| v.type = args["type"] as string; | ||
| } | ||
| v.concepts = normalizeList(args["concepts"]); | ||
| v.files = normalizeList(args["files"]); | ||
| return v; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
concepts/files are always sent as [], silently wiping preserved fields on every memory_update call.
normalizeList() returns [] (never undefined) when the argument is missing. In validate(), v.concepts/v.files are unconditionally set via normalizeList(args[...]) for memory_update (unlike v.type, which is only set when actually provided). Downstream:
handleProxychecksif (v.concepts) body["concepts"] = v.concepts;—[]is truthy, soconcepts: []is always sent toPUT /agentmemory/memories/update, andmem::update'sdata.concepts ?? existing.conceptswon't fall back (since[]isn't nullish) — existing concepts/files get erased on every call unless the caller explicitly repeats them.handleLocalhas the same problem:concepts: v.concepts ?? existing["concepts"]never falls back becausev.conceptsis neverundefined.
This defeats the field-preservation behavior that the SDK-layer tests (test/memory-update.test.ts) verify for mem::update itself — the standalone MCP shim doesn't honor the same contract.
🛡️ Proposed fix
case "memory_update": {
const memoryId = args["memoryId"];
if (typeof memoryId !== "string" || !memoryId.trim()) {
throw new Error("memoryId is required");
}
const content = args["content"];
if (typeof content !== "string" || !content.trim()) {
throw new Error("content is required");
}
v.memoryId = memoryId;
v.content = content;
if (typeof args["type"] === "string" && args["type"].trim()) {
v.type = args["type"] as string;
}
- v.concepts = normalizeList(args["concepts"]);
- v.files = normalizeList(args["files"]);
+ if (args["concepts"] !== undefined) {
+ v.concepts = normalizeList(args["concepts"]);
+ }
+ if (args["files"] !== undefined) {
+ v.files = normalizeList(args["files"]);
+ }
return v;
}Also applies to: 207-220, 317-318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mcp/standalone.ts` around lines 129 - 146, The memory_update validation
is always populating concepts and files with empty arrays, which causes
preserved values to be wiped on every update. In validate() for the
memory_update case in standalone.ts, only assign v.concepts and v.files when the
corresponding args["concepts"] / args["files"] are actually provided, matching
the existing conditional handling used for v.type. Then ensure handleProxy and
handleLocal can distinguish “not provided” from “provided empty list” so
existing memory fields are preserved unless the caller explicitly sends values.
| supersedes: [ | ||
| ...(Array.isArray(existing["supersedes"]) | ||
| ? (existing["supersedes"] as string[]) | ||
| : []), | ||
| v.memoryId!, | ||
| ], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Self-referential supersedes in the local fallback (same conceptual bug as mem::update in remember.ts).
The updated record is stored under the same key (v.memoryId!), yet supersedes appends v.memoryId! to its own list — the memory ends up superseding itself.
🛡️ Proposed fix
- supersedes: [
- ...(Array.isArray(existing["supersedes"])
- ? (existing["supersedes"] as string[])
- : []),
- v.memoryId!,
- ],
+ supersedes: Array.isArray(existing["supersedes"])
+ ? (existing["supersedes"] as string[])
+ : [],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| supersedes: [ | |
| ...(Array.isArray(existing["supersedes"]) | |
| ? (existing["supersedes"] as string[]) | |
| : []), | |
| v.memoryId!, | |
| ], | |
| supersedes: Array.isArray(existing["supersedes"]) | |
| ? (existing["supersedes"] as string[]) | |
| : [], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mcp/standalone.ts` around lines 322 - 327, The local fallback in
standalone.ts is adding the updated record’s own identifier to its supersedes
list, causing a self-referential entry. Update the logic around the object built
from existing["supersedes"] so it excludes v.memoryId! when the record is being
written back under the same key, and only append prior version IDs or other
distinct superseded IDs. Check the fallback update path in the same area that
mirrors the mem::update behavior from remember.ts.
- Package name: ziiagentmemory (was @agentmemory/agentmemory) - Author: Zeeshan Ahmad - Repository: github.com/ziishanahmad/ziiagentmemory - Bin: ziiagentmemory + agentmemory (backward compat) - Public access, npm registry
1 participant
Problem
The data model supports
versionandsupersedesfields, but there's no tool to update existing memories. Callingmemory_savetwice with the same concepts creates separate memories (both v1, bothisLatest=true) instead of versioning. Users who update memories get duplicates instead of versions, requiring manual cleanup viamemory_governance_delete.Fix
1. Engine:
mem::updatefunction (src/functions/remember.ts)memoryId+content(required), plus optionaltype,concepts,files,ttlDays,agentIdmemory not foundif missingversion, updatescontent/title, preserves all fields not provided (concepts, files, project, etc.)type/concepts/filesonly when explicitly providedoperation: "update")withKeyedLockfor concurrency safety2. REST API:
PUT /agentmemory/memories/update(src/triggers/api.ts)memoryIdandcontentare present3. MCP tool:
memory_update(src/mcp/tools-registry.ts)memory_savememoryId,content4. Standalone server (
src/mcp/standalone.ts)IMPLEMENTED_TOOLSsetvalidate()handleLocal()(works without engine connection)handleProxy()(calls PUT endpoint when engine is connected)Tests
test/memory-update.test.ts(9 tests):Updated existing tests:
mcp-standalone.test.ts: CORE_TOOLS count 14→15mcp-standalone-proxy.test.ts: IMPLEMENTED_TOOLS count 7→8, addedmemory_updateto expected tool listAll 65 tests across 5 test files pass. Build succeeds.
Closes #1018
Summary by CodeRabbit
New Features
PUT /agentmemory/memories/updateendpoint.Bug Fixes
Tests