fix: persist memories to disk immediately after memory_save

[JasonLandbridge]

Problem

In standalone.ts, kv.persist() (which writes the in-memory store to ~/.agentmemory/standalone.json) was only called on SIGINT/SIGTERM signal handlers:

process.on("SIGINT", () => { kv.persist(); process.exit(0); });
process.on("SIGTERM", () => { kv.persist(); process.exit(0); });

If the MCP server process is killed with SIGKILL, crashes unexpectedly, or the host terminates it forcefully (e.g. when an AI coding agent session ends), all memories saved during that session are permanently lost because standalone.json is never written.

Fix

Call kv.persist() immediately after kv.set() in the memory_save handler, so data is written to disk on every save:

await kvInstance.set("mem:memories", id, { ... });
kvInstance.persist();  // ← ensure data survives unexpected termination

Additional Changes

• Exported handleToolCall with an injectable kvInstance parameter (defaults to the module-level kv) to enable unit testing
• Mocked transport.js and config.js in test/mcp-standalone.test.ts to prevent side effects when importing the module
• Added 4 new tests in a handleToolCall describe block:
  • memory_save persists to disk immediately after saving — verifies writeFileSync is called
  • memory_save without persist path does not call writeFileSync — no unnecessary writes when no path configured
  • memory_save throws when content is missing — validates error handling
  • memory_recall returns matching memories — end-to-end recall test

All 625 existing tests continue to pass.

Summary by CodeRabbit

• Bug Fixes

  • Improved memory save and retrieval so saved items are consistently persisted, timestamped, and retrievable across sessions; save now reliably triggers disk persistence when configured.
  • Validation tightened so attempts to save without required content produce clear errors.
  • Memory listing/recall now reads from the active storage instance for accurate results.

• Tests

  • Added isolated tests covering persistence behavior, save/recall flows, and error cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 59e6a1af-4a45-4b8c-be5c-5cbf20801b53

📥 Commits

Reviewing files that changed from the base of the PR and between 71a1a7d and 7e1d197.

📒 Files selected for processing (1)

• src/mcp/standalone.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/mcp/standalone.ts

---

📝 Walkthrough

Walkthrough

handleToolCall was exported and now accepts an optional kvInstance: InMemoryKV = kv. Memory save uses generateId("mem"), a single isoNow timestamp for created/updated, writes via kvInstance.set(...) and calls kvInstance.persist(). Read-only handlers use kvInstance.list(...).

Changes

Cohort / File(s)	Summary
Standalone MCP logic src/mcp/standalone.ts	Exported handleToolCall(toolName, args, kvInstance = kv); memory_save now generates IDs with generateId("mem"), uses one isoNow for timestamps, writes via kvInstance.set(...) and calls kvInstance.persist(); read-only handlers (memory_recall, memory_sessions, memory_export, memory_audit) read via kvInstance.list(...) instead of module kv.
Tests for injectable KV behavior test/mcp-standalone.test.ts	Added mocks for transport/config and a describe("handleToolCall") suite; tests exercise memory_save/memory_recall with injected InMemoryKV, assert writeFileSync persistence when persist path present, assert no disk write when absent, validate missing content rejection, and verify recall filtering.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped from module fields into the light,
A handler freed, with KV held tight.
IDs and timestamps snug in a row,
I saved and recalled — watch the memories grow! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately describes the main fix: ensuring memories persist to disk immediately after the memory_save operation, which is the core issue addressed in the PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/mcp/standalone.ts (1)

34-47: 🛠️ Refactor suggestion | 🟠 Major

Align memory_save with repository ID and timestamp rules.

Use generateId() (and fingerprintId() for content-addressable dedup) instead of manual Date.now()/Math.random() ID construction, and capture ISO time once for both createdAt/updatedAt.

As per coding guidelines: src/**/*.ts: Use fingerprintId() for content-addressable deduplication and generateId() for unique IDs in TypeScript code; capture timestamps once with new Date().toISOString() and reuse the value rather than calling multiple times.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/mcp/standalone.ts` around lines 34 - 47, Replace the ad-hoc ID and
timestamp logic in the memory save code (the block that creates id and sets the
object in kvInstance.set) with the standard helpers: call generateId() to
produce the unique memory ID and use fingerprintId(content) if you need
content-addressable deduplication, and capture a single const isoNow = new
Date().toISOString() to set both createdAt and updatedAt rather than calling
Date.now()/Math.random() and multiple Date calls; update the object fields (id,
createdAt, updatedAt) accordingly in the same kvInstance.set call.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/mcp/standalone.ts`:
- Around line 34-47: Replace the ad-hoc ID and timestamp logic in the memory
save code (the block that creates id and sets the object in kvInstance.set) with
the standard helpers: call generateId() to produce the unique memory ID and use
fingerprintId(content) if you need content-addressable deduplication, and
capture a single const isoNow = new Date().toISOString() to set both createdAt
and updatedAt rather than calling Date.now()/Math.random() and multiple Date
calls; update the object fields (id, createdAt, updatedAt) accordingly in the
same kvInstance.set call.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 818ba2b7-6298-4137-9c93-67e6937c7fbf

📥 Commits

Reviewing files that changed from the base of the PR and between fa2aa98 and 71a1a7d.

📒 Files selected for processing (2)

• src/mcp/standalone.ts
• test/mcp-standalone.test.ts

[rohitg00]

Thanks for this — SIGKILL data loss was a real hole and the injectable kvInstance refactor + test mocks make this easy to merge confidently. Merging now.

Nice touches worth calling out:

• Switching to the shared generateId("mem") helper instead of the ad-hoc concat
• Single isoNow shared by createdAt/updatedAt so they can't drift
• The rationale for not using fingerprintId (memory_save is append-only, content-addressable would silently overwrite) matched the project's existing behavior exactly

Landing as v0.8.4 in the next patch bump.

[JasonLandbridge]

Thanks for this — SIGKILL data loss was a real hole and the injectable kvInstance refactor + test mocks make this easy to merge confidently. Merging now.

Nice touches worth calling out:

* Switching to the shared `generateId("mem")` helper instead of the ad-hoc concat

* Single `isoNow` shared by `createdAt`/`updatedAt` so they can't drift

* The rationale for _not_ using `fingerprintId` (memory_save is append-only, content-addressable would silently overwrite) matched the project's existing behavior exactly

Landing as v0.8.4 in the next patch bump.

Thanks for the quick merge, and thank you for this awesome project!