feat(slots): editable pinned memory slots + idle reflection + global scope#182
Merged
feat(slots): editable pinned memory slots + idle reflection + global scope#182
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds a KV-backed "memory slots" feature with CRUD and reflect operations, SDK and HTTP endpoints, MCP tools and registry entries, startup/event hooks, types/schema updates, README/plugin documentation edits, and tests for seeding, CRUD, limits, scope shadowing, and reflect behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant App as AppStartup
participant Events as event::session::stopped
participant Summ as mem::summarize
participant SlotReflect as mem::slot-reflect
participant Observations as Observations_KV
participant Slots as Slots_KV
Client->>App: start
App->>Slots: register mem::slot-* endpoints (if AGENTMEMORY_SLOTS)
Client->>Events: session stops
Events->>Summ: trigger(mem::summarize)
Summ-->>Events: summary
alt AGENTMEMORY_REFLECT enabled
Events->>SlotReflect: triggerVoid(mem::slot-reflect, {sessionId})
SlotReflect->>Observations: read compressed observations
SlotReflect->>SlotReflect: extract TODOs, patterns, touched files
SlotReflect->>Slots: acquire keyed lock -> update pending_items (truncate to sizeLimit)
SlotReflect->>Slots: acquire keyed lock -> update session_patterns
SlotReflect->>Slots: acquire keyed lock -> update project_context
SlotReflect-->>SlotReflect: record slot_reflect audit
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/index.ts (1)
334-335:⚠️ Potential issue | 🟡 MinorStale MCP tool count in startup log.
getAllTools()now returns 50 entries (withV010_SLOTS_TOOLS), and the plugin manifest/README badge were updated to 50, but this startup banner still reports 44.Proposed fix
- `[agentmemory] Endpoints: 107 REST + 44 MCP tools + 6 MCP resources + 3 MCP prompts`, + `[agentmemory] Endpoints: 107 REST + 50 MCP tools + 6 MCP resources + 3 MCP prompts`,Based on learnings: "When adding or removing MCP tools, update ALL of: tools-registry.ts, server.ts, triggers/api.ts, index.ts, test/mcp-standalone.test.ts, README.md, and plugin/.claude-plugin/plugin.json".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 334 - 335, The startup banner in index.ts currently hardcodes "44 MCP tools" which is stale; use the actual MCP tools count from getAllTools() instead of a magic number. Update the log construction where the string `[agentmemory] Endpoints: 107 REST + 44 MCP tools + 6 MCP resources + 3 MCP prompts` is emitted to compute the tools count dynamically (e.g., call getAllTools() and use its length) or otherwise replace 44 with the current getAllTools() result so the banner always reflects the real number of MCP tools.README.md (1)
596-620:⚠️ Potential issue | 🟡 MinorStale tool counts throughout this section.
The badge on line 28 and the prose on line 596 say "50 tools", but:
- Line 598:
### 44 Tools- Line 620:
<summary>Extended tools (44 total — set AGENTMEMORY_TOOLS=all)</summary>The new
memory_slot_*tools (6 total) are also not listed in the extended-tools table, so readers ofAGENTMEMORY_TOOLS=allwon't discover them from the README. Update the header/summary to 50 and add a short slots row block (or point toV010_SLOTS_TOOLS).Additionally:
- Lines 344 and 366 still say "all 43 memory tools" in the OpenClaw and Hermes paste-this prompts.
- Lines 904 and 924 still say "646 tests" — the PR summary says 800/800 passing.
Based on learnings: "When adding or removing MCP tools, update ALL of: tools-registry.ts, server.ts, triggers/api.ts, index.ts, test/mcp-standalone.test.ts, README.md, and plugin/.claude-plugin/plugin.json".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 596 - 620, Update README.md to reflect the new total tool and test counts and to list the new slot tools: change the "### 44 Tools" and the extended-tools summary text ("Extended tools (44 total — set AGENTMEMORY_TOOLS=all)") to 50, and ensure the top badge/prose and those summaries are consistent; add a short table row or a collapsible block describing the six new memory_slot_* tools (or add a pointer to V010_SLOTS_TOOLS) inside the extended-tools section so they appear when AGENTMEMORY_TOOLS=all; replace the two occurrences of "all 43 memory tools" in the OpenClaw and Hermes paste-this prompt blocks with the correct phrase (e.g., "all 50 memory tools"); and update the test count strings "646 tests" to "800 tests" (or "800/800 passing") to match the PR summary—search for these literal strings to find all sites to change.
🧹 Nitpick comments (4)
src/triggers/events.ts (1)
44-50: Use the exportedisReflectEnabled()helper instead of re-reading the env directly.
src/functions/slots.tsalready exportsisReflectEnabled(), which is the source of truth used insrc/index.ts. Readingprocess.env["AGENTMEMORY_REFLECT"]inline here duplicates that check and can drift (e.g., if the helper ever normalizes values or adds a second flag).Also worth noting:
triggerVoidhere is truly fire-and-forget and has no.catch()— if the slot-reflect function rejects (or the slots feature is disabled butAGENTMEMORY_REFLECT=truewas set), the rejection will surface as an unhandled promise rejection. Considersdk.triggerVoid(...).catch(() => {})or attaching a logger.Proposed fix
+import { isReflectEnabled } from "../functions/slots.js"; ... sdk.registerFunction("event::session::stopped", async (data: { sessionId: string }) => { const summary = await sdk.trigger({ function_id: "mem::summarize", payload: data }); - if (process.env["AGENTMEMORY_REFLECT"] === "true") { - sdk.triggerVoid("mem::slot-reflect", { sessionId: data.sessionId }); + if (isReflectEnabled()) { + void Promise.resolve(sdk.triggerVoid("mem::slot-reflect", { sessionId: data.sessionId })) + .catch(() => {}); } return summary; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/triggers/events.ts` around lines 44 - 50, Replace the direct env check in the "event::session::stopped" handler with the exported helper isReflectEnabled() from src/functions/slots.ts (i.e., call isReflectEnabled() instead of reading process.env["AGENTMEMORY_REFLECT"]), and make the fire-and-forget call to sdk.triggerVoid("mem::slot-reflect", { sessionId: data.sessionId }) resilient by attaching a .catch(() => {}) or logging the error so rejections don't become unhandled; keep the rest of the handler (calling sdk.trigger for "mem::summarize" and returning summary) unchanged.src/mcp/server.ts (1)
1111-1176: Slot handlers: consistent validation pattern, but consider masking not-found as 404.Right now, a non-existent label bubbles up through the
catchon line 1184 and returns a generic500 Internal error, which hides the real cause (Slot "X" not foundthrown bymem::slot-get/-append/-replace/-delete). Consider returning404with the thrown message for slot operations so agents get actionable feedback instead of a 500.Otherwise, validation + payload construction looks correct and aligns with the slot function signatures in
src/functions/slots.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcp/server.ts` around lines 1111 - 1176, The slot handlers (memory_slot_get, memory_slot_append, memory_slot_replace, memory_slot_delete) currently let "slot not found" errors bubble up and become 500s; change each handler to catch errors from its sdk.trigger call (the calls to mem::slot-get, mem::slot-append, mem::slot-replace, mem::slot-delete) and, if the thrown error message indicates the slot was not found (e.g. contains 'Slot "' or 'not found'), return status_code 404 with body.error set to the error message; otherwise rethrow or return the existing 500 behavior so other failures remain unchanged.src/functions/slots.ts (1)
138-142: Reuse one timestamp for new slot metadata.
createdAtandupdatedAtshould be identical for a newly-created slot; capturenowIso()once and reuse it.Suggested fix
+ const timestamp = nowIso(); const slot: MemorySlot = { ...tmpl, - createdAt: nowIso(), - updatedAt: nowIso(), + createdAt: timestamp, + updatedAt: timestamp, };+ const timestamp = nowIso(); const slot: MemorySlot = { label, content, sizeLimit, description, pinned: data?.pinned !== false, readOnly: false, scope, - createdAt: nowIso(), - updatedAt: nowIso(), + createdAt: timestamp, + updatedAt: timestamp, };As per coding guidelines, capture timestamps once with
new Date().toISOString()and reuse instead of calling Date multiple times.Also applies to: 227-237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/slots.ts` around lines 138 - 142, The code calls nowIso() twice when constructing a new MemorySlot causing createdAt and updatedAt to differ; capture the timestamp once (e.g., const ts = nowIso() or const now = new Date().toISOString()) and use that single value for both createdAt and updatedAt when building the slot object (see MemorySlot/slot creation where createdAt and updatedAt are set) and apply the same change to the other slot-creation block later in the file (the similar construct around lines ~227-237).test/slots.test.ts (1)
128-151: Exercise scope shadowing through the public slot functions.This test bypasses
mem::slot-create, so it won’t catch regressions where creating a project slot with the same label as a global slot is rejected. Use the handlers to set up the override.Suggested test setup
- await kv.set(KV.globalSlots, "persona", { - label: "persona", - content: "global-persona", - sizeLimit: 1000, - description: "", - pinned: true, - readOnly: false, - scope: "global", - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - await kv.set(KV.slots, "persona", { - label: "persona", - content: "project-override", - sizeLimit: 1000, - description: "", - pinned: true, - readOnly: false, - scope: "project", - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); + await handlers["mem::slot-replace"]({ + label: "persona", + content: "global-persona", + }); + const created = (await handlers["mem::slot-create"]({ + label: "persona", + content: "project-override", + scope: "project", + })) as { success: boolean }; + expect(created.success).toBe(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/slots.test.ts` around lines 128 - 151, The test currently bypasses the public API by writing directly to KV.slots/KV.globalSlots; instead create the slots via the handlers to exercise scope validation—call handlers["mem::slot-create"] to create the global slot (scope:"global") and again to create the project-scoped override (scope:"project") with the same label, then call handlers["mem::slot-get"] to assert the project slot shadows the global one; this ensures regressions in mem::slot-create scope/rejection logic are caught rather than masked by direct kv.set usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/slots.ts`:
- Around line 370-408: The code reads slots via readSlot("pending_items") and
readSlot("session_patterns") before taking locks and then writes back via
kv.set, which can cause lost updates; fix by moving the readSlot call inside the
withKeyedLock for "pending_items" (i.e., call readSlot(kv, "pending_items") from
within the async callback passed to withKeyedLock(`slot:pending_items`)) and
also wrap the session_patterns update in a withKeyedLock (e.g.,
withKeyedLock(`slot:session_patterns`, async () => { const { slot, scope } =
await readSlot(kv, "session_patterns"); ... kv.set(scopeKv(scope),
"session_patterns", {...}) })), ensuring you re-read slot content under the lock
before computing fresh/truncated content and before calling kv.set to avoid
stale overwrites.
- Around line 315-323: The delete path must acquire the same slot-level lock
used by writers to avoid the race where mem::slot-delete removes a slot while
mem::slot-append/mem::slot-replace later recreate it; wrap the
readSlot/check/readOnly/kv.delete/recordAudit sequence inside the same slot lock
(the helper used by append/replace, e.g., withSlotLock or acquireSlotLock for
the slot/scope) so you re-check existence/readOnly while holding the lock, then
call kv.delete(scopeKv(scope), label) and recordAudit("slot_delete",
"mem::slot-delete", ...) before releasing the lock.
- Around line 213-221: The create-slot flow uses validateLabel and readSlot but
checks for duplicates across all scopes and silently defaults invalid sizeLimit
values; update the logic so scope is explicitly validated from data.scope
(allowed values "global" or "project") into SlotScope and use that scope when
calling readSlot(kv, label, scope) (or equivalent) so duplicate checks only
apply within the target scope and permit project shadowing of global slots;
change sizeLimit handling in the slot creation path (currently using
DEFAULT_SIZE_LIMIT) to validate that data.sizeLimit is an integer between 1 and
20000 and return a validation error if invalid instead of defaulting; keep
validateLabel usage and ensure all validation failures return { success: false,
error: ... } as before.
In `@src/mcp/server.ts`:
- Around line 1129-1143: The memory_slot_create branch currently only recognizes
the string forms ("false" for args.pinned and "global" for args.scope), so
boolean false/true from clients are ignored; update the checks around
args.pinned and args.scope before building payload so they accept both boolean
and string forms (e.g., treat args.pinned === false || args.pinned === "false"
as payload.pinned = false, and treat args.pinned === true || args.pinned ===
"true" as payload.pinned = true), and similarly treat args.scope === "global" ||
args.scope === true as payload.scope = "global" (or otherwise normalize allowed
scope values) so the payload passed to sdk.trigger({ function_id:
"mem::slot-create", payload }) reflects the intended boolean/string inputs;
adjust logic in the memory_slot_create case to coerce/normalize these fields
rather than only checking for specific string literals.
In `@src/triggers/api.ts`:
- Around line 1319-1329: The handler currently silently drops invalid fields;
instead validate and reject malformed inputs before calling
sdk.trigger("mem::slot-create"): if body["content"] exists and is not a string
return 400; if body["scope"] exists and is not "project" or "global" return 400;
ensure body["pinned"] if present is boolean or return 400; use
parseOptionalPositiveInt(body["sizeLimit"]) but also enforce an upper bound
(20*1024 bytes) returning 400 when exceeded; only populate payload with
validated fields and then call sdk.trigger({ function_id: "mem::slot-create",
payload }).
In `@src/types.ts`:
- Around line 206-216: The MemorySlot interface currently types sizeLimit as
number but slot creation in src/functions/slots.ts may assign null; update the
type or the creator to be consistent: either change MemorySlot.sizeLimit to
number | null to accept null from the create/seed/reflect flows, or modify the
slot creation logic (the function(s) that set sizeLimit in
src/functions/slots.ts) to default to a numeric value (e.g., 2000) so
MemorySlot.sizeLimit remains a non-nullable number; pick one approach and apply
it everywhere (MemorySlot interface and the create/seed/reflect codepaths) so
the runtime shape and the TypeScript type match.
---
Outside diff comments:
In `@README.md`:
- Around line 596-620: Update README.md to reflect the new total tool and test
counts and to list the new slot tools: change the "### 44 Tools" and the
extended-tools summary text ("Extended tools (44 total — set
AGENTMEMORY_TOOLS=all)") to 50, and ensure the top badge/prose and those
summaries are consistent; add a short table row or a collapsible block
describing the six new memory_slot_* tools (or add a pointer to
V010_SLOTS_TOOLS) inside the extended-tools section so they appear when
AGENTMEMORY_TOOLS=all; replace the two occurrences of "all 43 memory tools" in
the OpenClaw and Hermes paste-this prompt blocks with the correct phrase (e.g.,
"all 50 memory tools"); and update the test count strings "646 tests" to "800
tests" (or "800/800 passing") to match the PR summary—search for these literal
strings to find all sites to change.
In `@src/index.ts`:
- Around line 334-335: The startup banner in index.ts currently hardcodes "44
MCP tools" which is stale; use the actual MCP tools count from getAllTools()
instead of a magic number. Update the log construction where the string
`[agentmemory] Endpoints: 107 REST + 44 MCP tools + 6 MCP resources + 3 MCP
prompts` is emitted to compute the tools count dynamically (e.g., call
getAllTools() and use its length) or otherwise replace 44 with the current
getAllTools() result so the banner always reflects the real number of MCP tools.
---
Nitpick comments:
In `@src/functions/slots.ts`:
- Around line 138-142: The code calls nowIso() twice when constructing a new
MemorySlot causing createdAt and updatedAt to differ; capture the timestamp once
(e.g., const ts = nowIso() or const now = new Date().toISOString()) and use that
single value for both createdAt and updatedAt when building the slot object (see
MemorySlot/slot creation where createdAt and updatedAt are set) and apply the
same change to the other slot-creation block later in the file (the similar
construct around lines ~227-237).
In `@src/mcp/server.ts`:
- Around line 1111-1176: The slot handlers (memory_slot_get, memory_slot_append,
memory_slot_replace, memory_slot_delete) currently let "slot not found" errors
bubble up and become 500s; change each handler to catch errors from its
sdk.trigger call (the calls to mem::slot-get, mem::slot-append,
mem::slot-replace, mem::slot-delete) and, if the thrown error message indicates
the slot was not found (e.g. contains 'Slot "' or 'not found'), return
status_code 404 with body.error set to the error message; otherwise rethrow or
return the existing 500 behavior so other failures remain unchanged.
In `@src/triggers/events.ts`:
- Around line 44-50: Replace the direct env check in the
"event::session::stopped" handler with the exported helper isReflectEnabled()
from src/functions/slots.ts (i.e., call isReflectEnabled() instead of reading
process.env["AGENTMEMORY_REFLECT"]), and make the fire-and-forget call to
sdk.triggerVoid("mem::slot-reflect", { sessionId: data.sessionId }) resilient by
attaching a .catch(() => {}) or logging the error so rejections don't become
unhandled; keep the rest of the handler (calling sdk.trigger for
"mem::summarize" and returning summary) unchanged.
In `@test/slots.test.ts`:
- Around line 128-151: The test currently bypasses the public API by writing
directly to KV.slots/KV.globalSlots; instead create the slots via the handlers
to exercise scope validation—call handlers["mem::slot-create"] to create the
global slot (scope:"global") and again to create the project-scoped override
(scope:"project") with the same label, then call handlers["mem::slot-get"] to
assert the project slot shadows the global one; this ensures regressions in
mem::slot-create scope/rejection logic are caught rather than masked by direct
kv.set usage.
🪄 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: ae8a1faa-d3e9-48f2-9324-fd2c539db492
📒 Files selected for processing (11)
README.mdplugin/.claude-plugin/plugin.jsonsrc/functions/slots.tssrc/index.tssrc/mcp/server.tssrc/mcp/tools-registry.tssrc/state/schema.tssrc/triggers/api.tssrc/triggers/events.tssrc/types.tstest/slots.test.ts
rohitg00
added a commit
that referenced
this pull request
Apr 21, 2026
## Concurrency - mem::slot-create now wraps the duplicate check + set + audit inside withKeyedLock. Previously a racing slot-append could create a slot that slot-create assumed was absent. - mem::slot-delete acquires the same slot-level lock writers use, so an in-flight append/replace cannot resurrect a deleted slot. - mem::slot-reflect now reads pending_items / session_patterns / project_context INSIDE their per-slot locks, computes the new content under the lock, and writes back under the same lock. The old path read-then-locked-then-wrote, which could clobber a concurrent slot-replace on the same label. ## Scope + validation - mem::slot-create duplicate check is now scope-local via readSlotInScope(). A project slot with the same label as a global slot is allowed and shadows the global on read (matches the read precedence). Before, create rejected any label that already existed in either scope. - validateScope() returns null on unknown scope values; handler returns "scope must be 'project' or 'global'" 400 instead of silently defaulting. - validateSizeLimit() returns null on non-integers, negatives, or values > 20000; handler returns "sizeLimit must be an integer between 1 and 20000" 400 instead of silently falling back to DEFAULT_SIZE_LIMIT. - Timestamp captured once (ts) then used for createdAt + updatedAt, both in the create path and in seedDefaults. ## HTTP surface - api::slot-create now rejects bad types up front: content / description must be strings, pinned must be a boolean, scope must be "project" or "global", sizeLimit must be a positive integer <= 20000. Malformed inputs get a 400 with a specific error message instead of being silently dropped. ## MCP surface - memory_slot_create accepts both boolean and string-boolean forms for pinned (MCP clients bind either depending on their schema wrapper). Same for scope, which now also accepts explicit "project". ## Other cleanup - event::session::stopped uses isReflectEnabled() helper instead of poking process.env directly. triggerVoid call wrapped in try/catch with a warn log so a reflect failure doesn't surface as an unhandled rejection. - src/index.ts boot banner computes MCP tool count via getAllTools().length instead of hardcoding 44 — banner now tracks the actual registry size. - README counts bumped: "all 43 memory tools" → "all 50 memory tools" (x2), "### 44 Tools" → "### 50 Tools", "Extended tools (44 total" → "(50 total", "646 tests" → "800 tests" (x2). ## Tests - test/slots.test.ts: shadow test now goes through the public handler API instead of direct kv.set. Two new cases: invalid sizeLimit (over max, negative, non-integer) rejected; unknown scope rejected. - Full suite: 802/802 pass (+2 new).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
619-656:⚠️ Potential issue | 🟡 MinorExtended tools list doesn't include the new
memory_slot_*tools (or several other recent additions).The badge and configuration blocks now advertise “50 tools /
AGENTMEMORY_TOOLS=all(50 tools)” and mentionmemory_slot_*by name, but the<details>table under "Extended tools (50 total — set AGENTMEMORY_TOOLS=all)" still shows the old entries and omitsmemory_slot_list,memory_slot_get,memory_slot_create,memory_slot_append,memory_slot_replace,memory_slot_delete(and, pre-existing gap:memory_lesson_save,memory_lesson_recall,memory_reflect,memory_insight_list,memory_obsidian_export). Users enabling the slots feature won’t find them documented here.♻️ Suggested additions
| `memory_verify` | Trace provenance | +| `memory_slot_list` | List memory slots (merged global + project) | +| `memory_slot_get` | Read a slot by label | +| `memory_slot_create` | Create a new slot (requires `AGENTMEMORY_SLOTS=true`) | +| `memory_slot_append` | Append text to a slot | +| `memory_slot_replace` | Replace a slot's content | +| `memory_slot_delete` | Delete a non-readOnly slot |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 619 - 656, Update the "Extended tools (50 total — set AGENTMEMORY_TOOLS=all)" details table to include the missing memory_slot_* entries (memory_slot_list, memory_slot_get, memory_slot_create, memory_slot_append, memory_slot_replace, memory_slot_delete) plus the other omitted tools (memory_lesson_save, memory_lesson_recall, memory_reflect, memory_insight_list, memory_obsidian_export), and ensure the summary/badge count and the parentheses after "AGENTMEMORY_TOOLS=all" reflect the new total; modify the table rows (the | Tool | Description | entries) to add one line per missing tool with concise descriptions and update the header text "Extended tools (50 total — set AGENTMEMORY_TOOLS=all)" to the correct total and matching string.
🧹 Nitpick comments (1)
src/functions/slots.ts (1)
154-167: Parallelize default-slot seeding.Eight independent
kv.get+kv.setpairs run serially on startup. Per repo guidelines, independent KV writes/reads should usePromise.all.♻️ Proposed refactor
-async function seedDefaults(kv: StateKV): Promise<void> { - const ts = nowIso(); - for (const tmpl of DEFAULT_SLOTS) { - const target = scopeKv(tmpl.scope); - const existing = await kv.get<MemorySlot>(target, tmpl.label); - if (existing) continue; - const slot: MemorySlot = { - ...tmpl, - createdAt: ts, - updatedAt: ts, - }; - await kv.set(target, tmpl.label, slot); - } -} +async function seedDefaults(kv: StateKV): Promise<void> { + const ts = nowIso(); + await Promise.all( + DEFAULT_SLOTS.map(async (tmpl) => { + const target = scopeKv(tmpl.scope); + const existing = await kv.get<MemorySlot>(target, tmpl.label); + if (existing) return; + await kv.set(target, tmpl.label, { ...tmpl, createdAt: ts, updatedAt: ts }); + }), + ); +}As per coding guidelines: "Use parallel operations where possible with Promise.all for independent kv writes/reads".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/slots.ts` around lines 154 - 167, seedDefaults currently runs kv.get and kv.set sequentially for each entry in DEFAULT_SLOTS; change it to run each slot's work in parallel by mapping DEFAULT_SLOTS to an array of async tasks (each task computes target via scopeKv(tmpl.scope), does the kv.get<MemorySlot>(target, tmpl.label) and if missing does kv.set(target, tmpl.label, slot) with createdAt/updatedAt timestamps) and then await Promise.all(tasks). Keep the same variable names (seedDefaults, DEFAULT_SLOTS, scopeKv, kv.get, kv.set) and ensure the timestamp is computed per slot or shared before Promise.all as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/slots.ts`:
- Around line 384-397: The reflect heuristics are doing sloppy substring
matches: replace the naive narrative/title check (narrative.includes("todo") ||
title.includes("todo")) inside the mem::slot-reflect loop with a
word-boundary/regex test (e.g. /\b(todo|fixme)\b/i) so only real TODO/FIXME
markers are considered before pushing into pendingLines; and fix the
duplicate-file logic that uses already.includes(f) by splitting the multi-line
already string into exact lines (already.split(/\r?\n/)), checking exact
equality against those lines (not substring), and only appending to
project_context when no exact match is found to avoid false dedupes like
src/a.ts vs src/a.tsx.
---
Outside diff comments:
In `@README.md`:
- Around line 619-656: Update the "Extended tools (50 total — set
AGENTMEMORY_TOOLS=all)" details table to include the missing memory_slot_*
entries (memory_slot_list, memory_slot_get, memory_slot_create,
memory_slot_append, memory_slot_replace, memory_slot_delete) plus the other
omitted tools (memory_lesson_save, memory_lesson_recall, memory_reflect,
memory_insight_list, memory_obsidian_export), and ensure the summary/badge count
and the parentheses after "AGENTMEMORY_TOOLS=all" reflect the new total; modify
the table rows (the | Tool | Description | entries) to add one line per missing
tool with concise descriptions and update the header text "Extended tools (50
total — set AGENTMEMORY_TOOLS=all)" to the correct total and matching string.
---
Nitpick comments:
In `@src/functions/slots.ts`:
- Around line 154-167: seedDefaults currently runs kv.get and kv.set
sequentially for each entry in DEFAULT_SLOTS; change it to run each slot's work
in parallel by mapping DEFAULT_SLOTS to an array of async tasks (each task
computes target via scopeKv(tmpl.scope), does the kv.get<MemorySlot>(target,
tmpl.label) and if missing does kv.set(target, tmpl.label, slot) with
createdAt/updatedAt timestamps) and then await Promise.all(tasks). Keep the same
variable names (seedDefaults, DEFAULT_SLOTS, scopeKv, kv.get, kv.set) and ensure
the timestamp is computed per slot or shared before Promise.all as appropriate.
🪄 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: 83606e49-2539-4a86-b15d-6d63c02a1a29
📒 Files selected for processing (7)
README.mdsrc/functions/slots.tssrc/index.tssrc/mcp/server.tssrc/triggers/api.tssrc/triggers/events.tstest/slots.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/slots.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/triggers/events.ts
- src/index.ts
- src/triggers/api.ts
Comment on lines
+384
to
+397
| for (const obs of recent) { | ||
| const title = (obs.title || "").toLowerCase(); | ||
| const narrative = (obs.narrative || "").toLowerCase(); | ||
| if (narrative.includes("todo") || title.includes("todo")) { | ||
| pendingLines.push(`- ${obs.title || obs.id}`); | ||
| } | ||
| if (obs.type === "error") { | ||
| patternCounts.set("errors", (patternCounts.get("errors") ?? 0) + 1); | ||
| } | ||
| if (obs.type === "command_run") { | ||
| patternCounts.set("commands", (patternCounts.get("commands") ?? 0) + 1); | ||
| } | ||
| if (obs.files) for (const f of obs.files) files.add(f); | ||
| } |
There was a problem hiding this comment.
Reflect heuristics have sloppy substring matches that will misclassify observations.
Two spots silently produce false positives/negatives in mem::slot-reflect:
- Line 387:
narrative.includes("todo") || title.includes("todo")matches any substring —"todoist","mastodon","stodolist", words mid-sentence — and pushes them intopending_items. Use a word-boundary match (or a structured TODO signal) so only real TODO/FIXME markers land in pending. - Lines 450–452 (same function):
already.includes(f)treats file paths as substrings — oncesrc/a.tsis recorded,src/a.ts.bakandsrc/a.tsxare incorrectly considered already-present and never appended toproject_context. Splitalreadyon newlines (matching the format written at line 457–459) and dedupe on exact line equality.
🩹 Proposed fix
- if (narrative.includes("todo") || title.includes("todo")) {
+ if (/\btodo\b|\bfixme\b/i.test(narrative) || /\btodo\b|\bfixme\b/i.test(title)) {
pendingLines.push(`- ${obs.title || obs.id}`);
}
@@
- const already = slot.content;
- const fresh = Array.from(files)
- .filter((f) => !already.includes(f))
- .slice(0, 20);
+ const already = slot.content;
+ const alreadyFiles = new Set(
+ already.split("\n").map((l) => l.replace(/^- /, "").trim()).filter(Boolean),
+ );
+ const fresh = Array.from(files)
+ .filter((f) => !alreadyFiles.has(f))
+ .slice(0, 20);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/slots.ts` around lines 384 - 397, The reflect heuristics are
doing sloppy substring matches: replace the naive narrative/title check
(narrative.includes("todo") || title.includes("todo")) inside the
mem::slot-reflect loop with a word-boundary/regex test (e.g.
/\b(todo|fixme)\b/i) so only real TODO/FIXME markers are considered before
pushing into pendingLines; and fix the duplicate-file logic that uses
already.includes(f) by splitting the multi-line already string into exact lines
(already.split(/\r?\n/)), checking exact equality against those lines (not
substring), and only appending to project_context when no exact match is found
to avoid false dedupes like src/a.ts vs src/a.tsx.
…bal scope
Introduces memory slots as a new first-class memory primitive, orthogonal
to observations / memories / graph. A slot is a labelled, size-limited,
agent-editable chunk of context. 8 defaults seed on first run
(persona, user_preferences, tool_guidelines, project_context, guidance,
pending_items, session_patterns, self_notes). Agent reads/writes via
MCP tools; pinned slots serialise cleanly into SessionStart context.
Functions (`src/functions/slots.ts`):
- mem::slot-list
- mem::slot-get
- mem::slot-create
- mem::slot-append (refuses writes over sizeLimit)
- mem::slot-replace (refuses content over sizeLimit)
- mem::slot-delete
- mem::slot-reflect (used by idle reflection)
REST (`src/triggers/api.ts`):
- GET /agentmemory/slots
- GET /agentmemory/slot?label=...
- POST /agentmemory/slot
- POST /agentmemory/slot/append
- POST /agentmemory/slot/replace
- DELETE /agentmemory/slot?label=...
- POST /agentmemory/slot/reflect
MCP tools (`src/mcp/tools-registry.ts`, `src/mcp/server.ts`):
- memory_slot_list
- memory_slot_get
- memory_slot_create
- memory_slot_append
- memory_slot_replace
- memory_slot_delete
event::session::stopped fires mem::slot-reflect via triggerVoid when
AGENTMEMORY_REFLECT=true. The reflector reads the last N observations,
extracts TODO-flavoured titles into pending_items, counts error /
command types into session_patterns, and records touched files in
project_context. Fire-and-forget: does not block summarise.
Slots carry a scope field ("project" | "global"). Global slots live at
KV.globalSlots ("mem:slots:global") — one brain across all projects for
things like persona / user_preferences / tool_guidelines. Project-
scoped slot with the same label shadows the global one on read.
- AGENTMEMORY_SLOTS=true — enable the whole primitive
- AGENTMEMORY_REFLECT=true — enable Stop-hook reflection (requires SLOTS)
Both default OFF. Matches the opt-in convention of auto-compress (#138),
inject-context (#143), vision embeddings (#179).
44 MCP tools → 50 across README (stats badge, install instructions,
config block) and plugin/.claude-plugin/plugin.json description. New
AGENTMEMORY_SLOTS and AGENTMEMORY_REFLECT entries in the config block.
test/slots.test.ts — 12 new cases: default seeding by scope, label
validation, create/get/append/replace/delete round-trip, duplicate-
create rejection, sizeLimit enforcement on append + replace, project
shadows global, listPinnedSlots + renderPinnedContext, reflect no-op
on empty session, reflect updates pending_items / session_patterns /
project_context from TODO + error + file signals.
Full suite: 800/800 pass (+12 new). Build clean.
## Concurrency - mem::slot-create now wraps the duplicate check + set + audit inside withKeyedLock. Previously a racing slot-append could create a slot that slot-create assumed was absent. - mem::slot-delete acquires the same slot-level lock writers use, so an in-flight append/replace cannot resurrect a deleted slot. - mem::slot-reflect now reads pending_items / session_patterns / project_context INSIDE their per-slot locks, computes the new content under the lock, and writes back under the same lock. The old path read-then-locked-then-wrote, which could clobber a concurrent slot-replace on the same label. ## Scope + validation - mem::slot-create duplicate check is now scope-local via readSlotInScope(). A project slot with the same label as a global slot is allowed and shadows the global on read (matches the read precedence). Before, create rejected any label that already existed in either scope. - validateScope() returns null on unknown scope values; handler returns "scope must be 'project' or 'global'" 400 instead of silently defaulting. - validateSizeLimit() returns null on non-integers, negatives, or values > 20000; handler returns "sizeLimit must be an integer between 1 and 20000" 400 instead of silently falling back to DEFAULT_SIZE_LIMIT. - Timestamp captured once (ts) then used for createdAt + updatedAt, both in the create path and in seedDefaults. ## HTTP surface - api::slot-create now rejects bad types up front: content / description must be strings, pinned must be a boolean, scope must be "project" or "global", sizeLimit must be a positive integer <= 20000. Malformed inputs get a 400 with a specific error message instead of being silently dropped. ## MCP surface - memory_slot_create accepts both boolean and string-boolean forms for pinned (MCP clients bind either depending on their schema wrapper). Same for scope, which now also accepts explicit "project". ## Other cleanup - event::session::stopped uses isReflectEnabled() helper instead of poking process.env directly. triggerVoid call wrapped in try/catch with a warn log so a reflect failure doesn't surface as an unhandled rejection. - src/index.ts boot banner computes MCP tool count via getAllTools().length instead of hardcoding 44 — banner now tracks the actual registry size. - README counts bumped: "all 43 memory tools" → "all 50 memory tools" (x2), "### 44 Tools" → "### 50 Tools", "Extended tools (44 total" → "(50 total", "646 tests" → "800 tests" (x2). ## Tests - test/slots.test.ts: shadow test now goes through the public handler API instead of direct kv.set. Two new cases: invalid sizeLimit (over max, negative, non-integer) rejected; unknown scope rejected. - Full suite: 802/802 pass (+2 new).
rohitg00
force-pushed
the
feat/memory-slots-v010
branch
from
b813d30 to
02d13c5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds memory slots as a new first-class memory primitive, orthogonal to observations / memories / graph. A slot is a labelled, size-limited, agent-editable chunk of context. 8 defaults seed on first run:
persona— how the agent should see itself (tone, role)user_preferences— coding style, tool habitstool_guidelines— rules for tool picking / sequencingproject_context— architecture, conventions, build/test commandsguidance— active advice for the next sessionpending_items— explicit TODOssession_patterns— recurring behaviours across sessionsself_notes— free-form scratch for the agentAgent edits via MCP tools; pinned slots serialise cleanly and can be injected into SessionStart context.
Why this is different from existing
memory_saveprojectvsglobal— one brain across all projects for persona / user_preferences / tool_guidelinesSurfaces
Functions (
src/functions/slots.ts)mem::slot-list,mem::slot-get,mem::slot-create,mem::slot-append(refuses writes over sizeLimit),mem::slot-replace,mem::slot-delete,mem::slot-reflect.REST (
src/triggers/api.ts)GET /agentmemory/slotsGET /agentmemory/slot?label=...POST /agentmemory/slotPOST /agentmemory/slot/appendPOST /agentmemory/slot/replaceDELETE /agentmemory/slot?label=...POST /agentmemory/slot/reflectMCP (
src/mcp/tools-registry.ts,src/mcp/server.ts)memory_slot_list,memory_slot_get,memory_slot_create,memory_slot_append,memory_slot_replace,memory_slot_delete.Idle reflection
event::session::stoppedfiresmem::slot-reflectviatriggerVoidwhenAGENTMEMORY_REFLECT=true. The reflector reads the last N observations, extracts TODO-flavoured titles intopending_items, counts error/command types intosession_patterns, and records touched files inproject_context. Fire-and-forget — does not block the summarise path.Scope: project vs global
Slots carry a
scopefield (\"project\" | \"global\"). Global slots live atKV.globalSlots(mem:slots:global) — one brain across all projects for things likepersona,user_preferences,tool_guidelines. A project-scoped slot with the same label shadows the global one on read.Opt-in
AGENTMEMORY_SLOTS=true— enable the whole primitiveAGENTMEMORY_REFLECT=true— enable Stop-hook reflection (requires SLOTS)Both default OFF. Matches the opt-in convention of #138 (auto-compress), #143 (inject-context), #160 (health thresholds), #179 (vision embeddings).
Counts bumped
44 MCP tools → 50 across README stats badge, install instructions, config block; plugin.json description. New env flag docs for
AGENTMEMORY_SLOTS/AGENTMEMORY_REFLECT.Test plan
npm run buildcleannpm test— 800/800 pass (+12 new cases intest/slots.test.ts)pending_items/session_patterns/project_contextfrom TODO + error + file signalsAGENTMEMORY_SLOTS=true npx @agentmemory/agentmemory,curl :3111/agentmemory/slots→ 8 default slotspending_items, start new session, verify Stop hook withAGENTMEMORY_REFLECT=trueupdates slot contentNext
Phase B (
.af-compatible import/export) and a viewer "Slots" tab are filed as follow-ups. Wanted this PR to stay focused on the primitive.Summary by CodeRabbit
New Features
Documentation
Tests