Memory names, slash-canonical paths, and the onConflict model#85
Memory names, slash-canonical paths, and the onConflict model#85cevian wants to merge 27 commits into
Conversation
denormalizeTreePath renders the display form shell-style: the caller's home anchors with `~` (`home.<id>.a.b` → `~/a/b`), every other path is absolute with a leading `/` (`share.auth` → `/share/auth`), and the root (empty path) is `/`. ltree storage and the SQL layer stay dot-native; normalizeTreePath strips a leading separator and accepts both `/` and `.`, so a displayed path fed back in round-trips. First step toward one canonical separator across API/CLI/docs. The web's tree logic is ltree-dot-native (sentinels, lquery building, path splitting), so incoming wire paths are converted at the fetch boundary (queries.ts): drop the leading `/`, then `/` → `.`. The web goes slash-native for display later. Updated server memory + management integration assertions to expect canonical slash output (inputs stay dotted since input accepts both). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a nullable, mutable `name` to me_<slug>.memory — a filename-like leaf slug, unique within its tree path via a partial unique index (where name is not null). The UUID stays the immutable identity; `name` is additive addressing and the future (tree,name) idempotency key. A CHECK enforces the slug shape (dots allowed; never an ltree label, so it can't collide with the tree separator). Bumps SPACE_SCHEMA_VERSION to 0.0.2. No reads or writes of `name` yet — the SQL functions and wire come next. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
batch_create_memory / create_memory key idempotency on the explicit id when given (import/export and deterministic importers preserve identity), else on (tree, name) when named, else anonymous insert. On conflict the action is onConflict: 'error' (default → raise unique_violation, for single create and batch alike), 'replace' (content-aware — a no-op unless an updated field differs, so re-import is idempotent and an importer-version bump re-renders), or 'ignore' (skip). raise_conflict() raises from the ON CONFLICT WHERE; the (tree, name) unique index is enforced on every path. replaceIfMetaDiffers is kept transitionally and takes precedence when set. get_memory and search/hybrid_search_memory return `name` (return-type change, guarded drops). patch_memory accepts a `name` key. New resolve_memory_id(tree, name) translates a folder/name ref to an id, read-gated. mapSpaceError maps 23505 -> CONFLICT. The git importer now submits with replaceIfMetaDiffers so re-import stays idempotent under the raise default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds memoryNameSchema (filename-like slug: starts alphanumeric, then
[A-Za-z0-9._-], <=128 — dots allowed since a name is never an ltree
label) and onConflictSchema ('error' | 'replace' | 'ignore'). memory.create
and batchCreate gain optional name and onConflict; memory.update gains
name (null clears, a string sets/renames). id stays optional on create
for import/export identity. The response name field and the
id-XOR-(tree,name) addressing on get/update/delete land with commit 6
(the server that implements them).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Memory gains `name`; CreateMemoryParams gains `name` + `onConflict`
('error'|'replace'|'ignore'); MemoryPatch gains `name` (null clears).
createMemory/batchCreateMemories pass the per-row name array + batch
onConflict (and the explicit id) to the SQL; getMemory and
search/hybridSearch map `name`; patchMemory threads a name patch. New
resolveMemoryId(tree, name) wraps resolve_memory_id (read-gated).
Existing 3-arg batchCreateMemories and no-name createMemory callers are
unaffected (defaults).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
memoryResponse gains `name` (toMemoryResponse maps it). New
memory.getByPath / memory.deleteByPath address a named memory by its
`folder/name` path (single `path` string; the server splits at the final
`/`, expands `~`, normalizes, then resolve_memory_id -> NOT_FOUND if it
doesn't resolve) -- two explicit methods rather than overloading
get/delete, which keeps each unambiguous (notably for MCP). get/delete
stay id-only.
create/batchCreate thread `name` + `onConflict`; a single create the
store skips ('ignore' or a 'replace' no-op) returns the existing memory
(idempotent) while a bare conflict raises CONFLICT (23505). update threads
a name patch (null clears, a string renames; it stays id-addressed, so the
CLI resolves a folder/name ref to an id for update). Client gains
getByPath/deleteByPath.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
create gains --name and --replace/--ignore (-> onConflict). get/update/ delete accept a folder/name path as well as a UUID: get dispatches to getByPath; update resolves the ref to an id (it's id-addressed) and gains --name (rename, or "" to clear); delete auto-detects a named memory vs a subtree and errors when the arg matches both (--name/--tree to force). Markdown export writes a nested <dir>/<tree>/<name-or-id>.md tree and carries `name` in frontmatter. memoryNameSchema gains min(1) (empty is never a valid name); the CLI maps `update --name ""` to a name clear (null), so no separate flag is needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
me_memory_create gains `name` and `on_conflict` ('error'|'replace'|
'ignore'). New me_memory_get_by_path / me_memory_delete_by_path address a
named memory by its folder/name path. me_memory_update gains `name`
(rename; "" clears). Markdown export mirrors the tree
(<dir>/<tree>/<name-or-id>.md) and carries `name`. Paths in tool
descriptions use the canonical leading-slash form. Adds the two new MCP
tool doc pages (required by the doc-link integrity test).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface an optional `name` through the import parsers (JSON / YAML / NDJSON / Markdown frontmatter), validated as a filename-like slug. `me import memories`, the MCP me_memory_import tool, and `me pack install` now pass `onConflict: 'ignore'` to batchCreateChunked so a re-import or re-install skips rows whose idempotency key (id, or (tree, name)) already exists — restoring the silent-skip behavior that the name-aware SQL conflict model replaced with raise-by-default. Pack install in particular would otherwise have started erroring on re-install. batchCreateChunked gains an `onConflict` option. The transcript/git importers were already safe via replaceIfMetaDiffers. e2e proves `me import memories` is idempotent for both named and explicit-id records. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
onConflict: 'replace' already overwrites in place only when content/meta/temporal differ, so the version-conditional replaceIfMetaDiffers override is redundant: importers stamp meta.importer_version, so a version bump makes meta differ and re-renders, while an unchanged re-import is a content-aware no-op. - SQL: drop _replace_if_meta_differs from batch_create_memory / create_memory and its CASE arms; guarded drops for the old overloads so the boot-time migration doesn't leave an ambiguous signature. - protocol/engine/server/chunk: drop the param across every layer. - importers (transcript + git): pass onConflict 'replace' and remove the per-run imported_at meta stamp — meta must be deterministic for the no-op path (the row's created_at/updated_at carry import timing). - tests + CLAUDE.md updated to the single-mechanism model. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`name` joins tree/meta/temporal as an editable frontmatter field: the editor emits and parses it (filename-slug validation; omit to clear, a slug to set/rename) and the save threads it through the update mutation; view mode renders a name row. In the left sidebar, the browse-mode tree leaf label and the search-mode result row both show a named memory's name (falling back to the first content line, then the id). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document the name feature (optional `name` slug, id-vs-path addressing via getByPath/deleteByPath, the onConflict error/replace/ignore model that replaced replaceIfMetaDiffers, nested Markdown export) across concepts, formats, the CLI and MCP/TS-client references, getting-started, and the CLAUDE.md memory-table summary. Also a path-format consistency pass: all example tree paths now use the canonical leading-slash form (`/share/auth`, `/home/<id>`), and the legacy dot-separated path language is removed. lquery filter tokens, code identifiers, and filesystem paths are left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…it id batch_create_memory now routes any row carrying a `name` to the (tree, name) arbiter — the name takes precedence over the id as the idempotency key. A supplied id is still used as the row's identity on insert (so importers can mint a timestamp-prefixed v7 for chronological sort) and the existing row's id is kept on a (tree,name) conflict. The with_id partition is now unnamed-only; unnamed-with-id still dedups on id (import/export identity). Lets the transcript/git importers key on a stable name instead of a hash-derived id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Importers no longer derive a SHA-256 deterministic id. They mint a timestamp-prefixed UUIDv7 (random tail) via uuidv7At and rely on the (tree, name) idempotency key: - Transcripts: each session is its own tree node (.../<sessionsNode>/<sessionId>) and each message is named msg_<messageId>; re-import collapses on (tree, name). The id keeps the message-time prefix, so the live-capture watermark (newest-by-id) is unchanged. - Git: commits are named by <sha> under .../git_history; the id keeps the commit-date prefix (incremental high-water lookup unchanged). uuid.ts drops deterministicUuidV7/deterministicMessageUuidV7 for uuidv7At. The id-keyed dedup helper becomes a generic dedupBy(items, key); callers collapse on the (tree, name) slot. Importer + e2e tests updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Transcript imports now nest each session as its own tree node with messages named msg_<message_id>; git commits are named leaves under git_history keyed by <sha>. Update the tree-layout + idempotency sections to (tree, name) keying (id is a timestamp-prefixed v7 for ordering) and drop the removed imported_at meta field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rebase onto main brought in upstream's copy/move/export e2e tests, which assert the returned `tree` in dotted form. This branch's slash-wire flip makes the API return the canonical leading-slash form, so convert the expected values via a toSlashPath helper (countUnder keeps the dotted ltree query). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Potential missed name plumbing in the interactive editor: |
memory-edit.ts had its own frontmatter handling that predated names, so a name edited in `me memory edit` was silently dropped: formatForEdit didn't emit the existing name, hasChanges didn't compare it, and the update params omitted it. Now the editor shows the name, detects a rename/clear as a change, and sends it (a value sets/renames; removing the line clears it). Adds a unit test for the formatForEdit/hasChanges name round-trip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch — fixed in 05d81f1. It was a bit broader than the one-liner:
All three now handle it, matching the web editor's frontmatter semantics: a value sets/renames, and removing the |
There was a problem hiding this comment.
Pull request overview
Adds first-class memory names, introduces folder/name path addressing, switches returned tree paths to canonical slash form, and replaces the old conflict behavior with an explicit onConflict model across create/batchCreate—spanning the database schema/functions, engine store API, server RPC surface, client/CLI/Web, and docs.
Changes:
- Add optional
memory.name(validated, unique per(tree, name)when non-null) and surface it through RPC/client/engine/web/CLI. - Add path-addressed RPC methods (
memory.getByPath,memory.deleteByPath) and update CLI/MCP/docs accordingly. - Make returned/display
treevalues slash-canonical (/share/auth,~/notes,/) and addonConflict: error|replace|ignoreto create/batchCreate (defaulterror).
Reviewed changes
Copilot reviewed 73 out of 74 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/lib/tree-build.ts | Prefer name for leaf titles |
| packages/web/src/lib/tree-build.test.ts | Add name title preference test |
| packages/web/src/lib/frontmatter.ts | Support name in editor frontmatter |
| packages/web/src/lib/frontmatter.test.ts | Add name frontmatter coverage |
| packages/web/src/components/viewer/FrontmatterBlock.tsx | Display name in viewer panel |
| packages/web/src/components/SearchResultRow.tsx | Show name in search rows/menu |
| packages/web/src/components/editor/EditorPane.tsx | Send name in update mutations |
| packages/web/src/api/queries.ts | Convert slash paths → dot internally |
| packages/server/rpc/memory/memory.ts | Add name/path methods + onConflict handling |
| packages/server/rpc/memory/memory.integration.test.ts | Extend integration tests for name/paths/slash trees |
| packages/server/rpc/memory/management.integration.test.ts | Update grant path expectations to slash form |
| packages/protocol/memory.ts | Add name/onConflict + get/deleteByPath schemas |
| packages/protocol/memory.test.ts | Add schema unit tests |
| packages/protocol/fields.ts | Add memoryNameSchema + onConflictSchema |
| packages/engine/space/types.ts | Add name + OnConflict types |
| packages/engine/space/db.ts | Plumb name/onConflict; add resolveMemoryId |
| packages/engine/space/db.integration.test.ts | Cover name + onConflict semantics |
| packages/database/space/version.ts | Bump SPACE_SCHEMA_VERSION |
| packages/database/space/path.ts | Slash-canonical denormalized tree paths |
| packages/database/space/path.test.ts | Update tests for slash form |
| packages/database/space/migrate/migrate.ts | Add incremental 005 migration |
| packages/database/space/migrate/migrate.integration.test.ts | Expect new migration/function/index |
| packages/database/space/migrate/incremental/005_memory_name.sql | Add memory.name + unique index + check |
| packages/database/space/migrate/idempotent/002_search.sql | Add name return column; guarded drops |
| packages/database/space/migrate/idempotent/001_memory.sql | Add name columns + resolve/raise_conflict + conflict model |
| packages/client/memory.ts | Add getByPath/deleteByPath client methods |
| packages/cli/parsers/validation.ts | Validate/emit name in imports |
| packages/cli/parsers/index.ts | Extend ParsedMemory with name |
| packages/cli/parsers/import.test.ts | Add name parsing tests |
| packages/cli/mcp/server.ts | Add MCP tools + on_conflict/name support |
| packages/cli/importers/uuid.ts | Switch to timestamp-prefixed uuidv7 minting |
| packages/cli/importers/uuid.test.ts | Update tests for uuidv7At behavior |
| packages/cli/importers/index.ts | Re-key transcript import to (tree,name) + replace |
| packages/cli/importers/index.test.ts | Generalize dedup helper tests |
| packages/cli/importers/git.ts | Name commits by sha; use uuidv7At |
| packages/cli/importers/git.test.ts | Update expectations for name/id behavior |
| packages/cli/commands/pack.ts | Pack install uses onConflict ignore |
| packages/cli/commands/memory.ts | Add --name, path get/delete, onConflict flags |
| packages/cli/commands/memory-import.ts | Import uses onConflict ignore + carries name |
| packages/cli/commands/import-git.ts | Git import dedups by sha + replace |
| packages/cli/chunk.ts | Replace replaceIfMetaDiffers with onConflict option |
| packages/cli/chunk.test.ts | Update chunking tests for onConflict |
| e2e/cli.e2e.test.ts | Update e2e for slash trees + named-path flows |
| docs/typescript-client.md | Document slash trees, name, onConflict, getByPath |
| docs/memory-packs.md | Update examples to slash paths |
| docs/mcp/me_memory_update.md | Document name field + slash tree |
| docs/mcp/me_memory_tree.md | Update tree examples to slash form |
| docs/mcp/me_memory_search.md | Update tree filter examples to slash form |
| docs/mcp/me_memory_mv.md | Update move examples to slash form |
| docs/mcp/me_memory_import.md | Document name + onConflict ignore behavior |
| docs/mcp/me_memory_get.md | Add name field + point to get_by_path |
| docs/mcp/me_memory_get_by_path.md | New doc for get-by-path tool |
| docs/mcp/me_memory_export.md | Document tree-mirroring markdown export |
| docs/mcp/me_memory_delete.md | Point to delete_by_path/delete_tree |
| docs/mcp/me_memory_delete_tree.md | Update examples to slash form |
| docs/mcp/me_memory_delete_by_path.md | New doc for delete-by-path tool |
| docs/mcp/me_memory_create.md | Document name + on_conflict + slash trees |
| docs/mcp-integration.md | Add new MCP tools + slash examples |
| docs/getting-started.md | Update quickstart for slash trees + name |
| docs/formats.md | Add name field + slash-canonical tree docs |
| docs/concepts.md | Add name/addressing/conflict model + slash trees |
| docs/cli/me-pack.md | Update semantics to onConflict ignore |
| docs/cli/me-memory.md | Update CLI docs for name/path + slash trees |
| docs/cli/me-mcp.md | Update defaults to slash paths |
| docs/cli/me-import.md | Update git layout/idempotency docs |
| docs/cli/me-claude.md | Update CLAUDE.md pointer tree examples |
| docs/cli/me-agent.md | Update home path examples to slash form |
| docs/cli/me-access.md | Update grant examples to slash form |
| docs/cli/agent-session-imports.md | Update transcript importer layout/idempotency |
| docs/access-control.md | Update access-control examples to slash form |
| CLAUDE.md | Update project model summary for name/paths/onConflict |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const id = | ||
| created?.id ?? | ||
| params.id ?? | ||
| (params.name != null | ||
| ? await guard(() => | ||
| store.resolveMemoryId(treeAccess, tree, params.name as string), | ||
| ) | ||
| : null); |
There was a problem hiding this comment.
Fixed in 278f070. create_memory now returns the stored id (the kept existing id on a (tree, name) update/skip), and the handler uses it directly instead of created?.id ?? params.id ?? resolve(...). So a named re-submit with a fresh explicit id resolves to the existing row — no INTERNAL_ERROR.
| export const memoryGetByPathParams = z.object({ | ||
| path: treePathSchema.min(1, "path is required"), | ||
| }); |
There was a problem hiding this comment.
Fixed in 06e67e7 — getByPath now uses a new memoryPathSchema that validates the leaf (segment after the final /) with memoryNameSchema and rejects a trailing /. So a typo'd path fails with VALIDATION_ERROR instead of masquerading as NOT_FOUND. The tree portion stays lenient.
| export const memoryDeleteByPathParams = z.object({ | ||
| path: treePathSchema.min(1, "path is required"), | ||
| }); |
There was a problem hiding this comment.
Fixed in 06e67e7 — same memoryPathSchema applied to deleteByPath.
| const params: Record<string, unknown> = { content }; | ||
| params.tree = opts.tree; | ||
| if (opts.name) params.name = opts.name; | ||
| if (opts.meta) params.meta = parseMeta(opts.meta); |
There was a problem hiding this comment.
Fixed in 06e67e7 — me memory create --name "" now errors immediately ("omit --name for an unnamed memory") instead of silently creating an unnamed memory. Clearing a name remains an update-only op (update --name "").
The unnamed→named transition was untested (existing update/editor tests start from an already-named memory). Add it at both levels: hasChanges detects a name added where there was none, and a server integration test creates an unnamed memory, updates it with a name, and confirms the name is then resolvable via getByPath. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Protocol docs appear to contradict the implemented name-precedence behavior for create/batchCreate conflicts. The comments here say the idempotency key is the explicit
But the SQL implementation makes These protocol comments should probably say that named rows use |
| -- only so it can sit in a WHERE expression; | ||
| -- it never actually returns. | ||
| ------------------------------------------------------------------------------- | ||
| drop function if exists {{schema}}.raise_conflict(ltree, text); |
There was a problem hiding this comment.
we don't need this drop function do we?
There was a problem hiding this comment.
You're right — removed in 40fa547. The (ltree, text) signature never existed on main or anywhere in the branch (only the 0-arg raise_conflict() was ever created), so the drop was a no-op running every migration.
| -- Returns one row (id, inserted) per insert/replace — inserted distinguishes a | ||
| -- fresh insert (true, xmax = 0) from a replace (false); skipped rows are absent. |
There was a problem hiding this comment.
Say I'm using (tree,name) for idempotency and I pass in 125 memories. 1 gets skipped. I get 124 rows of results, but I have no way of knowing which one was skipped. Also, because I only get 124 rows back, I cannot match up the returned ids with the source rows--I don't know which id goes with which (tree,path).
Ideally, I'd get 125 rows back in the same order i inserted them with an indication of which one was skipped. Something like this:
returns table
( ord bigint
, id uuid
, status text -- inserted | updated | skipped
)There was a problem hiding this comment.
another option is to treat duplicate explicit ids and duplicate (tree,name)s in input as an error
There was a problem hiding this comment.
Done — and thank you for the exact shape. 278f070 makes batch_create_memory return (ord, id, status) (one row per input, in input order; status inserted | updated | skipped). f3360a9 surfaces it on the wire as { results: [{ id, status }] }, so callers can map each result back to its input and see skips.
There was a problem hiding this comment.
names are parsed but not inserted. they are dropped. see line 233
There was a problem hiding this comment.
Fixed in 8f8ce9f — pack install now passes each memory's name through to batchCreate (it was parsed but dropped at the createParams step).
| const treeDir = | ||
| typeof mem.tree === "string" | ||
| ? mem.tree.replace(/^\//, "") // drop the absolute leading slash | ||
| : ""; | ||
| const base = | ||
| typeof mem.name === "string" && mem.name | ||
| ? mem.name | ||
| : String(mem.id); | ||
| const filename = base.endsWith(".md") ? base : `${base}.md`; | ||
| const dir = treeDir ? join(file, treeDir) : file; | ||
| mkdirSync(dir, { recursive: true }); | ||
| writeFileSync( | ||
| join(dir, filename), | ||
| formatMemoryAsMarkdown(mem), | ||
| "utf-8", | ||
| ); |
There was a problem hiding this comment.
Names are unique in the database, but filesystem names can still collide. For example, foo and foo.md are distinct memory names in one tree but both export to foo.md.
Case-insensitive filesystems introduce more possible collisions.
There was a problem hiding this comment.
Fixed in 4705be0 — added uniqueExportFilename, which tracks claimed filenames per directory (compared lowercased, so Foo/foo are treated as colliding for portability) and inserts the memory's unique id before .md on a clash, e.g. foo + foo.md → foo.md and foo.<id>.md. If even the id-disambiguated name is taken it throws rather than guessing.
| const treeDir = | ||
| typeof mem.tree === "string" ? mem.tree.replace(/^\//, "") : ""; | ||
| const base = | ||
| typeof mem.name === "string" && mem.name | ||
| ? mem.name | ||
| : String(mem.id); | ||
| const fname = base.endsWith(".md") ? base : `${base}.md`; | ||
| const dir = treeDir ? join(resolved, treeDir) : resolved; | ||
| mkdirSync(dir, { recursive: true }); | ||
| writeFileSync( | ||
| join(dir, fname), | ||
| formatMemoryAsMarkdown(mem), | ||
| "utf-8", | ||
| ); |
There was a problem hiding this comment.
Export uses <tree>/<name-or-id>.md, appending .md unless the name already ends with .md.
Names are unique in the database, but filesystem names can still collide. For example, foo and foo.md are distinct memory names in one tree but both export to foo.md.
Case-insensitive filesystems introduce more possible collisions.
There was a problem hiding this comment.
Fixed in 4705be0 — the MCP directory export uses the same uniqueExportFilename helper.
| /** | ||
| * The ltree node for one session: `<root>.<slug>.<sessionsNode>.<sessionId>`. | ||
| * The session id is normalized to a valid ltree label. Each session is its own | ||
| * node so its messages are browsable as named leaves under it. | ||
| */ | ||
| function sessionTree( | ||
| options: WriteOptions, | ||
| slug: string, | ||
| sessionId: string, | ||
| ): string { | ||
| return `${options.treeRoot}.${slug}.${options.sessionsNodeName}.${normalizeSlug(sessionId)}`; | ||
| } | ||
|
|
||
| /** | ||
| * A message's leaf name within its session node: `msg_<messageId>`, with any | ||
| * character outside the name charset replaced. `(tree, name)` is the idempotency | ||
| * key, so the same message always lands in the same slot across re-imports. | ||
| */ | ||
| function messageName(messageId: string): string { | ||
| return `msg_${messageId.replace(/[^A-Za-z0-9._-]/g, "_")}`; | ||
| } | ||
|
|
There was a problem hiding this comment.
Generated Importer Names Can Be Invalid or Collapse Distinct IDs
messageName()replaces invalid chars with_, but does not cap at the 128-character name limit.- Distinct message ids like
a/b,a:b, anda_bcollapse to the same name. sessionTree()usesnormalizeSlug(sessionId), which can similarly conflate session ids into one tree.
There was a problem hiding this comment.
Fixed in 8f8ce9f. messageName and sessionTree now route through a new boundedUniqueLabel: it caps at the 128-char name limit and appends a hash of the full original id whenever the slug is lossy or over-length — so a/b, a:b, a_b (and a UUID session id whose dashes merge in an ltree label) no longer collapse to one slot. Clean, fitting ids are unchanged. Covered by new unit tests.
There was a problem hiding this comment.
Don't we also need to consider named memories that didn't have explicit ids when computing how many were skipped?
There was a problem hiding this comment.
Good catch — fixed in f3360a9. Skip counting now reads the server's per-row status instead of diffing explicit ids, so a named, id-less row skipped on its (tree, name) slot is counted. The old computeSkippedIds helper is gone.
batch_create_memory now returns (ord, id, status) per input in input
order; create_memory returns (id, status) — both report the stored id
(the kept existing id on a (tree,name) update/skip) so a caller can read
the row back even on a skip. Reject a duplicate idempotency key within one
batch (repeated explicit id, or (tree,name)) up front, which also catches
an id shared across a named and an unnamed row that the per-key partitions
would otherwise miss.
The return-type changes use guarded do-block drops keyed on proargnames
(matching get_memory), so the live function isn't dropped/recreated every
boot. The server create handler now uses the returned stored id directly,
fixing the name-wins resolution bug (a named re-submit with a fresh id no
longer resolves to a never-inserted id → INTERNAL_ERROR). The batchCreate
wire result stays {ids, updatedIds} for now (derived from status); R2
surfaces status to the wire + external consumers.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
memory.batchCreate now returns { results: [{ id, status }] } — one entry
per submitted memory in request order — replacing { ids, updatedIds }, so
a caller maps each result to its input and sees inserted/updated/skipped
directly.
The chunker keeps that one-row-per-input contract across chunk boundaries:
its results is a superset that adds an 'error' status for inputs whose
chunk call threw (id echoed when present, else null), with the per-chunk
message in errors[]. The redundant failedIds field is dropped. The
transcript / git / file / pack / MCP importers derive their counts by
filtering on status — which now also counts a named-but-id-less skip and
id-less failed rows that the old explicit-id-only tallies missed.
Drops the obsolete computeSkippedIds helper; classifySkips takes the
skipped ids directly. Corrects the onConflict doc to name-wins and notes
ignore governs the idempotency-key conflict only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pack install now passes each memory's name through to batchCreate (it was parsed but dropped), so installed pack memories keep their human name. Importer name generation could produce invalid or colliding names: messageName didn't cap at the 128-char name limit, and both messageName and sessionTree mapped distinct ids to the same slot (a/b, a:b, a_b → a_b; a UUID session id's dashes merge in an ltree label). Both now route through boundedUniqueLabel, which appends a hash of the full original id when the slug is lossy or over-length — deterministic, so it stays a stable (tree, name) idempotency key — and caps length. Clean, fitting ids are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
copy_tree's INSERT omitted the name column, so copying a subtree dropped every memory's name (copies landed nameless at the destination). Copy it alongside the other columns: a copied memory keeps its name at the new path (with a fresh id); a (dst, name) clash raises 23505 → CONFLICT, as move_tree already does. move_tree was unaffected — it updates rows in place, so name carries over. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`me memory delete <id-or-path>` now deletes exactly one memory — by UUID or by its tree/name path — with no flags. Subtree deletion moves to a new `me memory deltree <tree>` (alias rmtree), which owns --dry-run and --yes and always previews first, so --dry-run can never delete (the old auto-detect/--name path deleted immediately despite --dry-run). This drops the delete-time ambiguity between a named memory and a subtree. When `delete` is given a path that names no memory but has memories beneath it, the NOT_FOUND error now points at `deltree` rather than leaving the user guessing. Also renames the user-facing "folder/name" wording to "tree/name" across CLI help, MCP tool descriptions, and docs, matching the model's `tree` term. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getByPath/deleteByPath now validate the leaf of a tree/name path with memoryNameSchema (via a new memoryPathSchema) and reject a trailing '/', so a typo'd path fails fast with VALIDATION_ERROR instead of looking like NOT_FOUND. The tree portion stays lenient as before. `me memory create --name ""` now errors immediately with a clear message (omit --name for an unnamed memory) instead of silently creating an unnamed memory. Clearing a name remains an update-only operation (`update --name ""`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Memory names are unique in the database, but distinct names can map to the same file on disk — `foo` and `foo.md` both export to `foo.md`, and a case-insensitive filesystem also conflates `Foo` and `foo` — silently overwriting one export with another. Add uniqueExportFilename, which tracks the filenames claimed per directory (lowercased) and inserts the memory's unique id before `.md` on a clash; the common no-collision case still writes a clean `<name>.md`. If even the id-disambiguated name is taken (a memory literally named like another's id), it throws rather than silently guessing. Used by both the CLI and MCP Markdown directory export. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ct overload Correct the conflict-model docs to match the implementation: the idempotency key is a named row's (tree, name) slot (name takes precedence over id), not "id when given, else (tree, name)". Document that onConflict governs that key only — a named row whose explicit id collides with a different row still raises (#11, #12). Updated across concepts, formats, the CLI/MCP create references, and the protocol doc comments. Also refresh stale docs surfaced en route: the TS-client batchCreate example now returns { results } (per-row { id, status }); the import skip-tracking note reflects that named id-less skips are now counted; and `me delete --tree` is now `me deltree`. Drop the `raise_conflict(ltree, text)` overload `drop` — that signature never existed (only the 0-arg raise_conflict() was ever created), so the drop was a no-op running on every migration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 79 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (3)
docs/mcp/me_memory_create.md:40
- In the space RPC model,
createdByis alwaysnull(the space schema doesn’t track per-memory creators; the server sets it to null in responses). The example response should reflect that to avoid misleading API/MCP consumers.
"hasEmbedding": false,
"createdAt": "2025-04-15T12:00:00Z",
"createdBy": "user_abc",
"updatedAt": null
}
docs/mcp/me_memory_get.md:28
- The space RPC layer always returns
createdBy: null(there’s no per-memory creator in the space schema). This example currently shows a user id, which doesn’t match actual responses.
"temporal": null,
"hasEmbedding": true,
"createdAt": "2025-04-15T12:00:00Z",
"createdBy": "user_abc",
"updatedAt": null
docs/mcp/me_memory_update.md:40
- The space RPC layer always returns
createdBy: null(no per-memory creator tracking). This example response should usenullto match real output.
"hasEmbedding": true,
"createdAt": "2025-04-15T12:00:00Z",
"createdBy": "user_abc",
"updatedAt": "2025-04-15T14:00:00Z"
| export interface ParsedFrontmatter { | ||
| /** Editable fields extracted from frontmatter. Missing → null. */ | ||
| /** Editable fields extracted from frontmatter. Missing → null/"". */ | ||
| name: string | null; |
There was a problem hiding this comment.
Fixed in 8151819 — the comment now reads "an absent or empty name → null", matching coerceName (which normalizes both to null).
| - **Header constants** (`CLIENT_VERSION_HEADER`, `SPACE_HEADER`) live in `@memory.build/protocol/headers`. | ||
| - **MCP compatibility**: all tool parameters are required (nullable for optional). Uses `z.record(z.string(), z.any())` for meta instead of `z.record(z.unknown())` (which crashes the MCP SDK). | ||
| - **batchCreate conflict semantics**: a duplicate explicit id is skipped, or — with `replaceIfMetaDiffers: "<metaKey>"` — replaced in place when the stored row's value for that key differs (the session importers pass `importer_version` so version bumps re-render server-side). Result is `{ids, updatedIds}` (inserted / replaced); ids in neither were skipped. Single `memory.create` on a duplicate id errors with CONFLICT. | ||
| - **create / batchCreate conflict semantics**: the idempotency key is the explicit id when given, else the `(tree, name)` slot. `onConflict` governs a clash: `error` (default) raises CONFLICT, `replace` overwrites in place when content/meta/temporal differ (a no-op when identical; the id-path also compares tree/name since it can move/rename), `ignore` skips. batchCreate returns `{ids, updatedIds}` (inserted / replaced); ids in neither were skipped. The session/git importers pass `onConflict: 'replace'` and stamp `meta.importer_version` (deterministic meta, no per-run timestamp), so an unchanged re-import is a no-op while a version bump makes meta differ and re-renders. The file importers (`me import memories`, `me_memory_import`, `me pack install`) pass `onConflict: 'ignore'` so a re-import/re-install is a no-op. (There is no `replaceIfMetaDiffers` — content-aware `replace` subsumed it.) |
There was a problem hiding this comment.
Fixed in 8151819 — CLAUDE.md now describes the per-row { results: [{ id, status }] } shape, and the idempotency-key note reads name-wins (this file was the one the R8 doc sweep missed).
| |------|------|----------|-------------| | ||
| | `id` | `string` | yes | The UUID of the memory to update. | | ||
| | `content` | `string \| null` | no | New content. Omit or pass `null` to keep existing. | | ||
| | `name` | `string \| null` | no | Set or rename the leaf name. Pass an empty string (`""`) to clear it; omit or pass `null` to keep existing. Same slug rules as `me_memory_create`. | |
There was a problem hiding this comment.
This one's actually working as intended for the MCP tool. The tool's name input is z.string().optional().nullable() (no min(1)), and the handler maps "" → null before calling the RPC (server.ts:429) — so passing "" does clear the name via me_memory_update, mirroring the CLI's update --name "". The min-1 memoryNameSchema you spotted is on the RPC param, which the tool never sees "" for. So the doc is accurate; leaving it.
| - id: "019b0300-0001-7000-8000-000000000001" | ||
| tree: "pack.typescript.naming" | ||
| tree: "/pack/typescript/naming" | ||
| meta: | ||
| pack: | ||
| name: "my-pack" |
There was a problem hiding this comment.
Correct — per-memory meta.pack is rejected by validatePackConstraints and the installer injects the pack.<name> prefix. This is actually broader: the whole example here is the old v1 flat-array format (header comments + bare array), which the current parser rejects in favor of the v2 envelope (name/version/id-prefix/format/memories). That staleness predates this PR (it's already on main; this branch only slash-converted these lines), so I'm leaving it out of the memory-names PR and will fix the pack-authoring docs to v2 in a separate change.
| - id: "019b0300-0002-7000-8000-000000000002" | ||
| tree: "pack.typescript.error_handling" | ||
| tree: "/pack/typescript/error_handling" | ||
| meta: | ||
| pack: | ||
| name: "my-pack" |
There was a problem hiding this comment.
Same as the sibling example — meta.pack rejection and the relative-tree / pack.<name> prefix are part of the same pre-existing v1→v2 pack-doc staleness (on main already). Tracking it as a separate pack-docs fix rather than expanding the scope of this PR.
CLAUDE.md still described batchCreate as returning {ids, updatedIds}; it's
now the per-row {results: [{id, status}]} shape, and the idempotency-key
note now reads name-wins (the R8 doc sweep missed CLAUDE.md). The web
ParsedFrontmatter comment claimed a missing name maps to null/"" —
coerceName only ever yields null.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| , content = excluded.content | ||
| , name = excluded.name | ||
| where case | ||
| when not {{schema}}.has_tree_access(_tree_access, m.tree, 2) then false |
There was a problem hiding this comment.
Here we skip if there is a collision on ids but we don't have write access the existing memory's tree. This behavior seems to contradict _on_conflict = 'error'
There was a problem hiding this comment.
Explicit-id collision against an existing row outside the caller’s writable tree silently becomes skipped. In memory.create, packages/server/rpc/memory/memory.ts:233-246 then tries to read that inaccessible id and returns INTERNAL_ERROR instead of CONFLICT.
| -- Rows actually written this statement: (id, inserted=fresh-insert vs replace). | ||
| , acted as | ||
| ( | ||
| select id, inserted from ins_id | ||
| union all | ||
| select id, inserted from ins_named | ||
| union all | ||
| select id, inserted from ins_anon | ||
| ) | ||
| select d.id, d.tree, d.meta, d.temporal, d.content | ||
| from d | ||
| on conflict (id) do update set | ||
| tree = excluded.tree | ||
| , meta = excluded.meta | ||
| , temporal = excluded.temporal | ||
| , content = excluded.content | ||
| where _replace_if_meta_differs is not null | ||
| and m.meta->>_replace_if_meta_differs | ||
| is distinct from excluded.meta->>_replace_if_meta_differs | ||
| and {{schema}}.has_tree_access(_tree_access, m.tree, 2) | ||
| returning m.id, (m.xmax = 0) | ||
| -- The stored id per input. For a named row it is resolved by (tree, name): | ||
| -- this subquery reads the PRE-statement snapshot (data-modifying CTEs aren't | ||
| -- visible here), so an EXISTING row (update/skip) yields its kept id, while a | ||
| -- fresh insert yields null → fall back to the row's own (minted) id. Unnamed | ||
| -- and anonymous rows always keep their own id. | ||
| , resolved as | ||
| ( | ||
| select | ||
| r.ord | ||
| , case | ||
| when r.name is not null then coalesce | ||
| ( ( select mm.id | ||
| from {{schema}}.memory mm | ||
| where mm.tree = r.tree and mm.name = r.name ) | ||
| , r.id | ||
| ) | ||
| else r.id | ||
| end as id | ||
| from r | ||
| ) | ||
| -- One row per input, in order: present in `acted` → inserted/updated; absent | ||
| -- → skipped (onConflict ignore, or a replace no-op). | ||
| select | ||
| res.ord | ||
| , res.id | ||
| , case | ||
| when a.id is null then 'skipped' | ||
| when a.inserted then 'inserted' | ||
| else 'updated' | ||
| end as status | ||
| from resolved res | ||
| left join acted a on a.id = res.id | ||
| order by res.ord |
There was a problem hiding this comment.
This attributes batch results by joining acted back to inputs on id only. If two inputs resolve to the same stored id through different keys, e.g. existing named row X, one input {id: X} and another {tree, name} with no explicit id, one row’s update marks both inputs as updated. The duplicate checks at 267-286 don’t catch this because the idempotency keys differ. This breaks the “one status per input” contract.
There was a problem hiding this comment.
I think we need a check that goes something like this:
select exists
(
select x.id, count(*)
from
(
select m.id
from unnest(_ids) u(id)
inner join {{schema}}.memory m on (u.id = m.id)
union all
select m.id
from unnest(_trees, _names) u(tree, name)
inner join {{schema}}.memory m on (m.tree = u.tree and m.name = u.name)
where u.name is not null
and m.name is not null
) x
group by x.id
having count(*) > 1
)There was a problem hiding this comment.
i.e. find any occurences where both an explicit id input and a tree name input would update the same existing memory
Summary
Adds an optional, human-chosen
nameto memories and reworks how memories are addressed, how tree paths are represented on the wire, and how write conflicts are handled. The UUID remains the immutable identity — everything here changes the contract around it.This PR is best read by its semantic changes rather than its commits.
Semantic changes
Names & addressing
name— a filename-like slug (^[A-Za-z0-9][A-Za-z0-9._-]*$, ≤128 chars, dots allowed / slashes not), unique within itstree.memory.getByPath/memory.deleteByPathtake afolder/namepath (split on the final/).memory.get/deletestay id-addressed. CLIme get/me deleteauto-detect UUID-vs-path;me updateresolves a path to an id first.create/batchCreatestill accept an explicitid(import/export identity);updategainsname(set/rename;nullor""clears).Tree paths are now slash-canonical — breaking (wire/display)
/,/share/auth, home~/notes(previously dotted,share.auth). ltree storage stays dotted internally./share/auth,share/auth, and dottedshare.authall normalize the same. Only output changed — anything that string-matches a returnedtreemust now expect the slash form.Conflict handling:
onConflict— breaking (default behavior)create/batchCreatetakeonConflict: 'error' | 'replace' | 'ignore', defaulterror. Previously a duplicate explicit id was silently skipped; it now raisesCONFLICTunlessreplace/ignoreis requested.idwhen given, else the(tree, name)slot — and anametakes precedence over theid(a named row dedups on(tree, name)even when it also carries an id; the id is then just the row's identity on insert).replaceis content-aware: a no-op unless content/meta/temporal differ, and re-embeds only when content changes.replaceIfMetaDiffersparameter is removed — content-awarereplacesubsumes it.me import memories,me_memory_import,me pack install) submit withignore(idempotent re-import); transcript/git importers withreplaceplus a deterministicmeta.importer_versionstamp, so a version bump re-renders.Importers re-keyed to
(tree, name)— breaking (data layout)(tree, name):…/agent_sessions/<session_id>), each message namedmsg_<message_id>;<sha>under…/git_history.imported_atmeta stamp was dropped (it would have defeated the content-aware re-import no-op).Export
<dir>/<tree>/<name-or-id>.md(was a flat{id}.md) — withnamein frontmatter. Round-trips re-import idempotently.Web
nameis an editable frontmatter field; the browse-tree leaf label and search-result rows display it.Breaking changes / migration notes
treestrings are slash-form now — update any consumer that compares them as strings.onConflict: 'ignore'/'replace'for the old behavior.replaceIfMetaDiffersis gone frombatchCreate.agent_sessionsnode; after this, a re-import writes to the new nested(tree, name)layout with fresh ids, so old rows aren't recognized (and would be duplicated under the new paths) rather than updated in place. Pre-production, so likely fine — but worth a one-time reimport/wipe where it matters.Verification
./bun run check:fullis green against local Postgres: 620 unit · 869 integration · 24 e2e, 0 failures. One new incremental migration (005_memory_name.sql);SPACE_SCHEMA_VERSION→0.0.4.🤖 Generated with Claude Code