Implements P1 and P2 from the shared-memory MCP brief, plus the shared-write-path cleanup that fell out of it.
memory.get no longer returns the embedding and tsvector
memory.get used a bare select() and returned the raw DB row, while memory.list and memory.search already projected an explicit 9-field shape. It was the only outlier.
Measured against the live server on a ~13k-char memory:
chars
share
content
27,662
44.1%
contentTsv
29,912
47.7%
embedding
4,688
7.5%
other 12 fields
433
0.7%
total
62,695
embedding is a fixed ~4,690-char tax on every read (384 dims regardless of content), so it dominates small memories — about 58% of an ~8k-char response. contentTsv scales super-linearly and dominates large ones.
This wasn't only token waste: fetching that memory exceeded the MCP tool-output cap and spilled to a file, even though content alone is well under it. memory.get was effectively unusable on exactly the large living documents memory.patch exists to serve.
user_id is still selected for the authorization check and stripped before responding.
memory.patch
memory.update only accepts full replacement, so adding one line to a 13k-char document meant resending all of it. That is expensive enough that edits were being skipped rather than risk silently truncating shared team history.
error naming the count — ambiguity never resolves arbitrarily
matches exactly once
replace, bump version, re-embed
stale version
concurrent-edit error
Both failure modes refuse rather than clobber; that is what makes it safe to hand to an agent. Semantics live in lib/memory-patch.ts as a pure function.
memory_append was considered and dropped — memory_patch(id, "## RECENTLY SHIPPED", "## RECENTLY SHIPPED\n- entry") already covers the heading-insert case with the uniqueness guarantee, and a section parameter would mean defining heading-match and insert-position semantics for no gain.
Shared mutation layer
The MCP tools and the Web UI Server Actions each reimplemented authorize → mutate → re-embed → CAS → audit. Both now route through lib/memory-mutations.ts; the callers are thin adapters (MCP maps Outcome → ToolResult, the Web UI throws then revalidates/redirects).
⚠️ Behaviour change worth a close look:memory.delete over MCP skipped the project ACL whenever the caller authored the row. So a memory you wrote while a share was rw stayed deletable by you after an owner downgraded that share to ro — while the Web UI and both update paths correctly refused. Authoring a row now grants no standing write privilege anywhere. This is written as a test that fails on the old code.
The one deliberate difference between the surfaces is injected as a ProjectResolver: MCP refuses an unknown project key (call project.identify first) so an agent can't spawn near-miss projects off a typo; the Web UI creates one, because a person typing a name into a form means to.
Net: -519 lines from the two callers, plus a 357-line shared module.
Tests
First test infrastructure in the repo — vitest, with integration tests against a real Postgres, not a mocked DB. Setup instructions are in the README.
The embedder sidecar is the only stub, and it's deterministic per-text, so re-embedding is verified by asserting the stored vector changed rather than that a mock was called.
One test is worth calling out: it pins that content_tsv is a GENERATED ALWAYS ... STORED column, so full-text search cannot rot after a patch — only the embedding needs an explicit recompute. This matters because the obvious acceptance check ("after patching, does search find the new text?") would pass on an implementation that skipped re-embedding entirely, since the FTS ranker finds the literal text regardless.
35 tests. Both memory.get and memory.patch were verified to fail without their implementation before being marked done.
Lint
pnpm lint previously dropped into an interactive next lint setup prompt and exited 1 — ESLint had never been configured here. Replaced with the ESLint CLI plus a flat config bridging eslint-config-next (still legacy-format) through FlatCompat. Also clears 4 pre-existing warnings (3 unused imports, 1 anonymous default export). Clean at --max-warnings=0.
Verification
35/35 tests passing
pnpm -r typecheck clean across all three packages
pnpm build succeeds
pnpm lint clean
Not verified: the P1 payload reduction is a property of the deployed server. Post-deploy, confirm memory_get on aaea192c-edce-4372-b011-5113a02dea16 returns inline instead of spilling, and drops ~62,695 → ~28,100 chars.
Follow-ups not in this PR
P3 (making file→memory mirroring derived rather than conventional) — deliberately deferred. Much of that problem was a symptom of the update primitive being expensive; worth re-evaluating now that patch exists.
Adding --max-warnings=0 to the lint script — a policy call, left to the team.
Implements P1 and P2 from the `shared-memory` MCP brief, plus the shared-write-path cleanup that fell out of it.
## `memory.get` no longer returns the embedding and tsvector
`memory.get` used a bare `select()` and returned the raw DB row, while `memory.list` and `memory.search` already projected an explicit 9-field shape. It was the only outlier.
Measured against the live server on a ~13k-char memory:
| | chars | share |
|---|---|---|
| `content` | 27,662 | 44.1% |
| `contentTsv` | 29,912 | 47.7% |
| `embedding` | 4,688 | 7.5% |
| other 12 fields | 433 | 0.7% |
| **total** | **62,695** | |
`embedding` is a fixed ~4,690-char tax on every read (384 dims regardless of content), so it dominates *small* memories — about 58% of an ~8k-char response. `contentTsv` scales super-linearly and dominates large ones.
This wasn't only token waste: fetching that memory **exceeded the MCP tool-output cap and spilled to a file**, even though `content` alone is well under it. `memory.get` was effectively unusable on exactly the large living documents `memory.patch` exists to serve.
`user_id` is still selected for the authorization check and stripped before responding.
## `memory.patch`
`memory.update` only accepts full replacement, so adding one line to a 13k-char document meant resending all of it. That is expensive enough that edits were being *skipped* rather than risk silently truncating shared team history.
```
memory_patch(id, old_string, new_string, version?)
```
| condition | behaviour |
|---|---|
| `old_string` absent | error — never a silent no-op |
| `old_string` matches >1 | error naming the count — ambiguity never resolves arbitrarily |
| matches exactly once | replace, bump `version`, re-embed |
| stale `version` | concurrent-edit error |
Both failure modes refuse rather than clobber; that is what makes it safe to hand to an agent. Semantics live in `lib/memory-patch.ts` as a pure function.
`memory_append` was considered and dropped — `memory_patch(id, "## RECENTLY SHIPPED", "## RECENTLY SHIPPED\n- entry")` already covers the heading-insert case *with* the uniqueness guarantee, and a `section` parameter would mean defining heading-match and insert-position semantics for no gain.
## Shared mutation layer
The MCP tools and the Web UI Server Actions each reimplemented authorize → mutate → re-embed → CAS → audit. Both now route through `lib/memory-mutations.ts`; the callers are thin adapters (MCP maps `Outcome` → `ToolResult`, the Web UI throws then revalidates/redirects).
**⚠️ Behaviour change worth a close look:** `memory.delete` over MCP skipped the project ACL whenever the caller authored the row. So a memory you wrote while a share was `rw` stayed deletable by you after an owner downgraded that share to `ro` — while the Web UI and *both* update paths correctly refused. Authoring a row now grants no standing write privilege anywhere. This is written as a test that fails on the old code.
The one deliberate difference between the surfaces is injected as a `ProjectResolver`: MCP refuses an unknown project key (`call project.identify first`) so an agent can't spawn near-miss projects off a typo; the Web UI creates one, because a person typing a name into a form means to.
Net: **-519 lines** from the two callers, plus a 357-line shared module.
## Tests
First test infrastructure in the repo — vitest, with integration tests against a **real Postgres**, not a mocked DB. Setup instructions are in the README.
The embedder sidecar is the only stub, and it's deterministic per-text, so re-embedding is verified by asserting the **stored vector changed** rather than that a mock was called.
One test is worth calling out: it pins that `content_tsv` is a `GENERATED ALWAYS ... STORED` column, so full-text search **cannot** rot after a patch — only the embedding needs an explicit recompute. This matters because the obvious acceptance check ("after patching, does search find the new text?") would pass on an implementation that skipped re-embedding entirely, since the FTS ranker finds the literal text regardless.
35 tests. Both `memory.get` and `memory.patch` were verified to fail without their implementation before being marked done.
## Lint
`pnpm lint` previously dropped into an interactive `next lint` setup prompt and exited 1 — ESLint had never been configured here. Replaced with the ESLint CLI plus a flat config bridging `eslint-config-next` (still legacy-format) through `FlatCompat`. Also clears 4 pre-existing warnings (3 unused imports, 1 anonymous default export). Clean at `--max-warnings=0`.
## Verification
- 35/35 tests passing
- `pnpm -r typecheck` clean across all three packages
- `pnpm build` succeeds
- `pnpm lint` clean
**Not verified:** the P1 payload reduction is a property of the *deployed* server. Post-deploy, confirm `memory_get` on `aaea192c-edce-4372-b011-5113a02dea16` returns inline instead of spilling, and drops ~62,695 → ~28,100 chars.
## Follow-ups not in this PR
- **P3** (making file→memory mirroring derived rather than conventional) — deliberately deferred. Much of that problem was a symptom of the update primitive being expensive; worth re-evaluating now that patch exists.
- Adding `--max-warnings=0` to the `lint` script — a policy call, left to the team.
memory.get no longer returns the embedding and content_tsv
----------------------------------------------------------
It used a bare select() and returned the raw DB row, while memory.list
and memory.search already projected an explicit 9-field shape. On a
~13k-char memory those two internal columns were 55% of the response
and pushed it past the MCP tool-output cap, so large memories could not
be fetched inline at all. memory.get now returns the same 9 fields as
its siblings; user_id is still selected for the authorization check and
stripped before responding.
memory.patch
------------
memory.update only accepts full replacement, so adding one line to a
large document meant resending the whole document — expensive enough
that edits were being skipped rather than risk silently truncating
shared team documents.
memory.patch replaces one exact occurrence of old_string. An absent or
ambiguous match is an error, never a silent no-op and never an
arbitrary pick; that refusal is what makes the operation safe to hand
to an agent. The semantics live in lib/memory-patch.ts as a pure
function, free of DB and auth, so both surfaces share them.
Shared mutation layer
---------------------
The MCP tools and the Web UI Server Actions each reimplemented
authorize -> mutate -> re-embed -> CAS -> audit, and had drifted. Both
now route through lib/memory-mutations.ts.
BEHAVIOUR CHANGE: memory.delete over MCP skipped the project ACL
whenever the caller authored the row, so a memory written while a share
was rw stayed deletable by its author after an owner downgraded that
share to ro. memory.update and the whole Web UI always checked.
Authoring a row now grants no standing write privilege on any path.
The one deliberate difference between the surfaces is injected as a
ProjectResolver: MCP refuses an unknown project key so an agent cannot
spawn near-miss projects off a typo, while the Web UI creates one
because a person typing a name into a form means to.
Tests and lint
--------------
Adds vitest. The integration tests run against a real Postgres rather
than a mocked DB. The embedder sidecar is the only stub and it is
deterministic per-text, so re-embedding is verified by asserting the
stored vector actually changed rather than that a mock was called. One
test pins that content_tsv is a generated column and therefore cannot
rot after a patch — only the embedding needs an explicit recompute.
pnpm lint previously dropped into an interactive `next lint` setup
prompt and exited 1; ESLint had never been configured here. Replaced
with the ESLint CLI and a flat config bridging eslint-config-next
through FlatCompat. Clean at --max-warnings=0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keeps the reasoning behind PR #19 next to the code, since none of it is
recoverable from the diff: why memory_get stopped returning the embedding
and tsvector, why memory_patch refuses ambiguous matches rather than
picking one, and why memory_append was dropped as redundant with patch.
Also records P3 (mechanising file->memory mirroring) as DECLINED with its
reasoning and, more usefully, the condition that would reopen it — the
mirror going stale again now that patching is cheap. The evidence we had
pointed at edit cost, which P2 fixed; if drift recurs the cause was
attention instead, and the answer is probably to remove the duplication
rather than build a drift detector for it.
Notes two traps for anyone extending this: content_tsv is a generated
column so full-text search cannot rot after a patch (only the embedding
needs recomputing), and the obvious "does search find the patched text"
acceptance check therefore passes on an implementation that skips
re-embedding entirely.
The brief previously lived outside the repo. Moved rather than copied —
two hand-maintained copies is the exact drift problem described in the
document.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jknapp
merged commit d4d478d2f3 into main2026-08-11 22:16:56 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements P1 and P2 from the
shared-memoryMCP brief, plus the shared-write-path cleanup that fell out of it.memory.getno longer returns the embedding and tsvectormemory.getused a bareselect()and returned the raw DB row, whilememory.listandmemory.searchalready projected an explicit 9-field shape. It was the only outlier.Measured against the live server on a ~13k-char memory:
contentcontentTsvembeddingembeddingis a fixed ~4,690-char tax on every read (384 dims regardless of content), so it dominates small memories — about 58% of an ~8k-char response.contentTsvscales super-linearly and dominates large ones.This wasn't only token waste: fetching that memory exceeded the MCP tool-output cap and spilled to a file, even though
contentalone is well under it.memory.getwas effectively unusable on exactly the large living documentsmemory.patchexists to serve.user_idis still selected for the authorization check and stripped before responding.memory.patchmemory.updateonly accepts full replacement, so adding one line to a 13k-char document meant resending all of it. That is expensive enough that edits were being skipped rather than risk silently truncating shared team history.old_stringabsentold_stringmatches >1version, re-embedversionBoth failure modes refuse rather than clobber; that is what makes it safe to hand to an agent. Semantics live in
lib/memory-patch.tsas a pure function.memory_appendwas considered and dropped —memory_patch(id, "## RECENTLY SHIPPED", "## RECENTLY SHIPPED\n- entry")already covers the heading-insert case with the uniqueness guarantee, and asectionparameter would mean defining heading-match and insert-position semantics for no gain.Shared mutation layer
The MCP tools and the Web UI Server Actions each reimplemented authorize → mutate → re-embed → CAS → audit. Both now route through
lib/memory-mutations.ts; the callers are thin adapters (MCP mapsOutcome→ToolResult, the Web UI throws then revalidates/redirects).⚠️ Behaviour change worth a close look:
memory.deleteover MCP skipped the project ACL whenever the caller authored the row. So a memory you wrote while a share wasrwstayed deletable by you after an owner downgraded that share toro— while the Web UI and both update paths correctly refused. Authoring a row now grants no standing write privilege anywhere. This is written as a test that fails on the old code.The one deliberate difference between the surfaces is injected as a
ProjectResolver: MCP refuses an unknown project key (call project.identify first) so an agent can't spawn near-miss projects off a typo; the Web UI creates one, because a person typing a name into a form means to.Net: -519 lines from the two callers, plus a 357-line shared module.
Tests
First test infrastructure in the repo — vitest, with integration tests against a real Postgres, not a mocked DB. Setup instructions are in the README.
The embedder sidecar is the only stub, and it's deterministic per-text, so re-embedding is verified by asserting the stored vector changed rather than that a mock was called.
One test is worth calling out: it pins that
content_tsvis aGENERATED ALWAYS ... STOREDcolumn, so full-text search cannot rot after a patch — only the embedding needs an explicit recompute. This matters because the obvious acceptance check ("after patching, does search find the new text?") would pass on an implementation that skipped re-embedding entirely, since the FTS ranker finds the literal text regardless.35 tests. Both
memory.getandmemory.patchwere verified to fail without their implementation before being marked done.Lint
pnpm lintpreviously dropped into an interactivenext lintsetup prompt and exited 1 — ESLint had never been configured here. Replaced with the ESLint CLI plus a flat config bridgingeslint-config-next(still legacy-format) throughFlatCompat. Also clears 4 pre-existing warnings (3 unused imports, 1 anonymous default export). Clean at--max-warnings=0.Verification
pnpm -r typecheckclean across all three packagespnpm buildsucceedspnpm lintcleanNot verified: the P1 payload reduction is a property of the deployed server. Post-deploy, confirm
memory_getonaaea192c-edce-4372-b011-5113a02dea16returns inline instead of spilling, and drops ~62,695 → ~28,100 chars.Follow-ups not in this PR
--max-warnings=0to thelintscript — a policy call, left to the team.