Repository navigation
refactor: memory adapter and pi-memory - #145
maxy-shpfy wants to merge 1 commit into
Conversation
bd03da6 to
8f2c11c
Compare
055464e to
bf1b278
Compare
Mbeaulne
left a comment
There was a problem hiding this comment.
Requesting changes for two blockers and the Prime-only write rule:
- Backend image crashes at startup: the Cloud Run
Dockerfiledoesn't install pi-memory, whichpiMemory.tsresolves at module load. - Memory leaks between sessions: every session shares one qmd collection named
pi-memory. - Sub-agents can write memory: through pi-memory's exit summary, and because the role check in
extensions/memory.tswas removed.
Details and smaller items are inline. One more is outside the diff: InMemorySessionStore.deleteSession (inMemorySessionStore.ts:214) clears envs but not the new memories map, so a deleted session's binding survives. That store is test-only, so the impact is small.
bf1b278 to
0d8bcfa
Compare
On the item outside the diff: fixed in |
| ); | ||
| } | ||
| const real = canonicalPath(resolved); | ||
| const inMount = mountedFolders |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
Unmounting silently drops a shared memory binding while the symlink stays
With #137's one-way unmount, removing a mount resource drops the catalog row but keeps the symlink. mountedFolderPaths reads only catalog rows, so on the next Prime spawn resolveRoot throws for a root inside that folder, buildLeafSafely drops the leaf with a console.warn, and the session falls back to SimpleMD-only — while Prime can still read and write the same folder through the symlink. Neither the user nor the agent is told.
Suggest making the two authorities agree: either have unmount remove the symlink (so losing the binding is the expected outcome), or reject/warn when removing a mount the session's memory binding points into. Surfacing a dropped leaf in the memory preamble would also help.
| # Writable locations on Cloud Run for session workspaces and Pi's lockfiles/config. | ||
| ENV SESSIONS_ROOT=/tmp/sessions | ||
| # pi-memory qmd indexes; keep on the same storage as the stores they index. qmd | ||
| # models (~2 GB) download to $HOME/.cache/qmd/models on first semantic search. |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
qmd models land in memory-backed /tmp
Search is on by default (PI_MEMORY_NO_SEARCH is only set when search === false), and with HOME=/tmp on Cloud Run's in-memory filesystem the first memory_search downloads ~2 GB of models into instance RAM — on top of inference memory — and again on every cold start. That's a real OOM risk.
Options: bake the models into the image, point qmd's cache at persistent storage, or default search: false for deployments until memory sizing is confirmed. Same applies to Dockerfile.fullstack.
|
|
||
| /** The dispatcher tools SimpleMD exposes, by role. Descriptions live here (not | ||
| * in the Pi extension) so tool behavior and copy stay with the backend. */ | ||
| const READ_MEMORY_TOOL: MemoryToolSpec = { |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
Memory tool copy is duplicated; spec fields are unused
Only .name of a MemoryToolSpec is ever read (the allowlist in resolver.ts, and composite.ts), while pi/extensions/memory.ts still hardcodes the same descriptions, prompt snippets, and typebox params. The two copies will drift, and the "tool copy stays with the backend" intent isn't actually realized.
Suggest trimming MemoryToolSpec to what's consumed (name), or generating the extension's registerTool calls from the spec so there's a single source.
| "## Project memory", | ||
| "", | ||
| "No project memory note exists yet. When there is something durable to", | ||
| `record, create \`${editPath}\` with your file tools; it then becomes`, |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
No availability guard or size cap for the shared note
If a simple-md root names a mount that isn't present, resolveRoot accepts the non-existent path (it canonicalizes inside the session root), and this preamble tells Prime to create <root>/MEMORY.md — which creates a real directory that a later mount of that name then fails on ("A file or folder already exists at the mount name"). pi-memory has storeAvailable for exactly this; consider applying it to simple-md roots in buildSessionBackend.
Also, the full note is injected into every spawn's system prompt with no cap, and any mounting session can grow it — worth truncating with a pointer to the file.
| return suggestion; | ||
| } | ||
| } | ||
| export { |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
Back-compat alias re-export (optional)
This leaves one class under two names (MemoryManager / SimpleMdMemory) and adds a re-export barrel, which the repo conventions discourage. Fine to keep for this PR's diff size, but consider a follow-up that moves the 13 importers to SimpleMdMemory from memory/simpleMd.ts and deletes the facade.
Description
Refactors memory from a single hardcoded
SimpleMDstore into a pluggable memory backend tree, and promotes pi-memory to a first-class, per-session memory adapter that can be shared across sessions via mounted shared folders.Memory is now an attribute of the session, not the server. A session resolves to the always-on
SimpleMdMemorystore, optionally composed with a sessionMemoryBindingsupplied by the agent bundle's default (memoryBindingin its manifest) or overridden byCreateSessionRequest.memory. The resolved binding is persisted on the session.What changed
apps/server/src/memory/module built around aMemoryBackendinterface (init/preamble/tools/spawn/execute):simpleMd.ts— the always-on session + global markdown store (the formerMemoryManager).piMemory.ts— thepi-memoryleaf: wires thepi-memorynpm extension to a directory, setsPI_MEMORY_DIR, allowlists the extension'smemory_*tools by role, and mapsPiMemoryConfigonto the extension's env knobs.sharedMarkdown.ts— a directory-rootedsimple-mdleaf.composite.ts— unions several backends, dispatching tool calls to the first child that handles them.resolver.ts—MemoryResolver→ResolvedMemory: builds the backend tree from a binding, confines each leaf'srootto the session root (traversal/absolute paths rejected, degrading to baseline SimpleMD), and projects the spawn plan (env /--extensionpaths /--toolsallowlist) the manager needs.types.ts— shared backend/tool/effect types.pi/memory.tsis now a thin back-compat facade re-exportingSimpleMdMemory as MemoryManager, so the global-memory route, chat handlers, and host-resource writers keep importing unchanged.read_memory/remember/suggest_memoryare no longer hardcoded inSHARED_AGENT_TOOLS/PRIME_MEMORY_TOOLS. They're granted at spawn from the session'sResolvedMemory, so a pi-memory-bound session also gets that backend's tools — the resolution composes them.packages/shared): newMemoryBinding(simple-md|pi-memory|composite) andPiMemoryConfig, plusCreateSessionRequest.memory. Mirrored by a shared zod validator (memoryBindingSchema) used by both the bundle manifest and the create-session route.sessions.memorycolumn (migration0025), withgetSessionMemory/setSessionMemoryon the session store (SQLite + in-memory). The create-session handler persists the binding, then resolves memory so arootpointing into a mount lands inside the shared folder.PiMemoryConfig):exitSummary,snapshot,qmdUpdate,search,exitSummaryModel— omitted fields default to "max memory power".embed-react/ embed runtime types extended to carry the memory binding.memory.md,sessions-and-storage.md,extensions-and-prompts.md, andoverview/09-agent-memory.mdfor the pluggable model.Screenshots
Server/API refactor, so there's no UI to show. These diagrams describe the memory approach instead.
1. Backend tree resolution
flowchart TD CreateReq["CreateSessionRequest.memory"] --> Pick{"body.memory ?? bundle memoryBinding"} Manifest["Bundle manifest memoryBinding"] --> Pick Pick --> Settle["settleBinding at creation"] Settle --> Column[("sessions.memory")] Column --> Resolver["MemoryResolver.resolve"] Resolver -->|"no binding / invalid / >1 pi-memory"| Default["resolveDefault: SimpleMdMemory only"] Resolver --> Composite["CompositeMemory"] Composite --> SimpleMd["SimpleMdMemory (always on)"] Composite --> Leaf{"binding.kind"} Leaf -->|"simple-md + root"| SharedMd["SharedMarkdownMemory"] Leaf -->|"pi-memory"| PiMem["PiMemory"] Leaf -->|"composite"| Nested["nested children"] Default --> Resolved["ResolvedMemory"] Composite --> Resolved Resolved --> Init["init()"] Resolved --> Preamble["preamble(role)"] Resolved --> Plan["spawnPlan(role)"] Resolved --> Exec["execute(name, params, role)"]2. Spawn flow (Prime and sub-agents)
3. Two tool paths
flowchart LR subgraph piProc [Pi process] DispTools["read_memory / remember / suggest_memory"] PiTools["memory_write / memory_read / memory_search / scratchpad"] end DispTools -->|"POST /internal/memory/call"| Internal["InternalMemoryAPI (role check: writes Prime-only)"] Internal --> SimpleMdExec["SimpleMdMemory.execute"] SimpleMdExec --> SessionFile[("root/MEMORY.md")] SimpleMdExec --> GlobalFile[("GLOBAL_MEMORY.md")] Internal --> Highlight["chat:message highlight / memory:suggestion card"] PiTools -->|"in-process, no server highlight"| Store[("PI_MEMORY_DIR store")] PiTools --> Qmd[("qmd index in .qmd-state")]4. Sharing a pi-memory store across sessions
flowchart TD SessA["Session A (mount: project)"] -->|"root: project/pi-memory"| SharedStore[("SHARED_ROOT/folderId/pi-memory")] SessB["Session B (mount: project)"] -->|"root: project/pi-memory"| SharedStore SessC["Session C (no root)"] --> LocalStore[("SESSIONS_ROOT/id/.pi-memory")] SharedStore --> SharedIdx[(".qmd-state/shared/folderId/pi-memory")] LocalStore --> LocalIdx[(".qmd-state/sessions/id/.pi-memory")]How to test
pnpm --filter @tangent/server test pnpm validateEnd-to-end (the pluggable pi-memory path), per
.local/pi-memory-verification.md:qmdis on the server processPATH(optional — write/read work without it; onlymemory_search+ background indexing need it).POST /api/shared-folders { name }→ keepfolder.id."memory": { "kind": "pi-memory", "root": "project/pi-memory" }; over the socket, prompt Prime tomemory_writea uniqueVERIFY-<uuid>marker.memory_search/memory_readfor the marker and echo it verbatim → it should surface, proving the shared store.root) must not find the marker; arootthat escapes the session root ("../escape") must be ignored (server logsignoring invalid memory binding) and fall back to baseline SimpleMD.Notes
maxy/feat--shared-folders(shared-folder mounts are what make pi-memory shareable across sessions).pi-memorytools run inside the Pi process and emit no server-side highlight — assert on the agent's reply text, not a socket event.simple-md/pi-memory/compositeleaves only; LLM Wiki / StructuredEntity are not yet supported.Stack created with GitHub Stacks CLI • Give Feedback 💬