Skip to content

refactor: memory adapter and pi-memory - #145

Open
maxy-shpfy wants to merge 1 commit into
maxy/feat--shared-foldersfrom
maxy/refactor--memory-adapters
Open

maxy-shpfy wants to merge 1 commit into
maxy/feat--shared-foldersfrom
maxy/refactor--memory-adapters

Conversation

@maxy-shpfy

@maxy-shpfy maxy-shpfy commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Refactors memory from a single hardcoded SimpleMD store 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 SimpleMdMemory store, optionally composed with a session MemoryBinding supplied by the agent bundle's default (memoryBinding in its manifest) or overridden by CreateSessionRequest.memory. The resolved binding is persisted on the session.

What changed

  • New apps/server/src/memory/ module built around a MemoryBackend interface (init / preamble / tools / spawn / execute):
    • simpleMd.ts — the always-on session + global markdown store (the former MemoryManager).
    • piMemory.ts — the pi-memory leaf: wires the pi-memory npm extension to a directory, sets PI_MEMORY_DIR, allowlists the extension's memory_* tools by role, and maps PiMemoryConfig onto the extension's env knobs.
    • sharedMarkdown.ts — a directory-rooted simple-md leaf.
    • 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's root to the session root (traversal/absolute paths rejected, degrading to baseline SimpleMD), and projects the spawn plan (env / --extension paths / --tools allowlist) the manager needs.
    • types.ts — shared backend/tool/effect types.
  • pi/memory.ts is now a thin back-compat facade re-exporting SimpleMdMemory as MemoryManager, so the global-memory route, chat handlers, and host-resource writers keep importing unchanged.
  • Dynamic memory tool grants: read_memory / remember / suggest_memory are no longer hardcoded in SHARED_AGENT_TOOLS / PRIME_MEMORY_TOOLS. They're granted at spawn from the session's ResolvedMemory, so a pi-memory-bound session also gets that backend's tools — the resolution composes them.
  • Contracts (packages/shared): new MemoryBinding (simple-md | pi-memory | composite) and PiMemoryConfig, plus CreateSessionRequest.memory. Mirrored by a shared zod validator (memoryBindingSchema) used by both the bundle manifest and the create-session route.
  • Persistence: new nullable sessions.memory column (migration 0025), with getSessionMemory / setSessionMemory on the session store (SQLite + in-memory). The create-session handler persists the binding, then resolves memory so a root pointing into a mount lands inside the shared folder.
  • Config knobs (PiMemoryConfig): exitSummary, snapshot, qmdUpdate, search, exitSummaryModel — omitted fields default to "max memory power".
  • Embed: embed-react / embed runtime types extended to carry the memory binding.
  • Docs: updated memory.md, sessions-and-storage.md, extensions-and-prompts.md, and overview/09-agent-memory.md for 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)"]
Loading

2. Spawn flow (Prime and sub-agents)

sequenceDiagram
  autonumber
  participant Caller as Route or Trigger
  participant Manager as PiAgentManager
  participant Resolver as MemoryResolver
  participant Resolved as ResolvedMemory
  participant Pi as Pi process

  Caller->>Manager: ensure(sessionId, root)
  Manager->>Resolver: resolve(sessionId, root)
  Resolver-->>Manager: ResolvedMemory (cached on process record)
  Manager->>Resolved: init()
  Manager->>Resolved: preamble(role) + spawnPlan(role)
  Resolved-->>Manager: env, extensionPaths, allowlistTools, deniedTools
  Manager->>Pi: pi --mode rpc --extension ... --tools ... (PI_MEMORY_DIR, INDEX_PATH, QMD_CONFIG_DIR)
  Note over Manager,Pi: Sub-agents reuse the cached resolution, get read-only tools, no exit summary or qmd upkeep
Loading

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")]
Loading

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")]
Loading

How to test

pnpm --filter @tangent/server test
pnpm validate

End-to-end (the pluggable pi-memory path), per .local/pi-memory-verification.md:

  1. Ensure qmd is on the server process PATH (optional — write/read work without it; only memory_search + background indexing need it).
  2. POST /api/shared-folders { name } → keep folder.id.
  3. Create writer session mounting the folder with "memory": { "kind": "pi-memory", "root": "project/pi-memory" }; over the socket, prompt Prime to memory_write a unique VERIFY-<uuid> marker.
  4. Create a second session mounting the same folder with the same binding; prompt Prime to memory_search/memory_read for the marker and echo it verbatim → it should surface, proving the shared store.
  5. Negative: a session-local pi-memory session (no mount/root) must not find the marker; a root that escapes the session root ("../escape") must be ignored (server logs ignoring invalid memory binding) and fall back to baseline SimpleMD.

Notes

  • Stacked on top of maxy/feat--shared-folders (shared-folder mounts are what make pi-memory shareable across sessions).
  • pi-memory tools run inside the Pi process and emit no server-side highlight — assert on the agent's reply text, not a socket event.
  • Scope: simple-md / pi-memory / composite leaves only; LLM Wiki / StructuredEntity are not yet supported.

Stack created with GitHub Stacks CLI • Give Feedback 💬

@maxy-shpfy
maxy-shpfy added this pull request to stack #139 October 31, 2026 04:34
@maxy-shpfy
maxy-shpfy force-pushed the maxy/refactor--memory-adapters branch from bd03da6 to 8f2c11c Compare October 31, 2026 04:51
@maxy-shpfy
maxy-shpfy force-pushed the maxy/refactor--memory-adapters branch 3 times, most recently from 055464e to bf1b278 Compare October 31, 2026 05:45
@maxy-shpfy
maxy-shpfy marked this pull request as ready for review October 31, 2026 06:40

@Mbeaulne Mbeaulne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two blockers and the Prime-only write rule:

  • Backend image crashes at startup: the Cloud Run Dockerfile doesn't install pi-memory, which piMemory.ts resolves 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.ts was 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.

Comment thread apps/server/src/memory/piMemory.ts Resolved
Comment thread apps/server/src/memory/piMemory.ts Resolved
Comment thread apps/server/src/memory/piMemory.ts Resolved
Comment thread apps/server/src/pi/extensions/memory.ts Resolved
Comment thread apps/server/src/pi/piAgentManager.ts Resolved
Comment thread apps/server/src/memory/composite.ts Resolved
Comment thread apps/server/src/memory/piMemory.ts Resolved
Comment thread apps/server/src/memory/piMemory.ts Resolved
Comment thread docs/server/index.md Resolved
Comment thread Dockerfile.fullstack Resolved
@maxy-shpfy
maxy-shpfy force-pushed the maxy/refactor--memory-adapters branch from bf1b278 to 0d8bcfa Compare November 3, 2026 03:10
@maxy-shpfy

Copy link
Copy Markdown
Collaborator Author

AI facilitated response

On the item outside the diff: fixed in 0d8bcfa. InMemorySessionStore.deleteSession now also clears the session's memory binding.

@maxy-shpfy
maxy-shpfy requested a review from Mbeaulne November 3, 2026 03:14
@camielvs camielvs mentioned this pull request Nov 3, 2026
);
}
const real = canonicalPath(resolved);
const inMount = mountedFolders

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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.

Comment thread Dockerfile
# 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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 = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants