Compare commits

..
Author SHA1 Message Date
shadowdaoandClaude Opus 5 6e9628b073 docs: record the memory API decisions, including the one declined
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>
2026-08-11 15:10:36 -07:00
shadowdaoandClaude Opus 5 c3bbea5134 feat: add memory.patch, trim memory.get, unify the memory write path
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>
2026-08-11 14:58:11 -07:00
jknapp d9306884d3 Merge pull request 'docs: instance branches should be orphan branches' (#18) from docs/instance-branch-note into main 2026-07-27 14:05:13 +00:00
shadowdaoandClaude Opus 5 d319f00227 docs: instance branches should be orphan branches
Records why, so the next person doesn't rebuild the trap: a branch off main
carries a full copy of the app it has no reason to have and drifts behind it,
and syncing it via `git merge origin/main` silently replaces the instance
manifests with main's placeholders — no conflict, because only main touches
those paths.

instance/dnspegasus has been converted accordingly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 07:05:12 -07:00
jknapp fc0cb453d3 Merge pull request 'fix: accept the issuer with or without a trailing slash' (#17) from fix/issuer-trailing-slash into main 2026-07-27 13:42:31 +00:00
21 changed files with 2730 additions and 527 deletions
+93
View File
@@ -410,6 +410,25 @@ claude plugin marketplace add https://your-git-host/you/shared-memory.git#instan
The `#ref` suffix is undocumented in `claude plugin marketplace add --help` but is
honored and persisted in `known_marketplaces.json` (verified on Claude Code 2.1.220).
**Make that an orphan branch, not a branch off `main`.** `marketplace add` reads
only the manifests, so the branch needs nothing else:
```bash
git checkout --orphan instance/<name>
git rm -rf --cached . && find . -mindepth 1 -maxdepth 1 ! -name .git -exec rm -rf {} +
# restore just .claude-plugin/marketplace.json, plugin/.claude-plugin/plugin.json,
# plugin/.mcp.json — fill in your URL and client ID
claude plugin validate . && git add -A && git commit && git push
```
A branch off `main` carries a full copy of the application it has no reason to
have, so it drifts and someone can cut a stale deploy from it. Worse, syncing it
means `git merge origin/main`, which **silently replaces those manifests** with
the placeholders below — no conflict is raised, because only `main` ever touches
those paths. With no shared history there is nothing to sync and nothing to
clobber; if the manifest format changes upstream, hand-edit the three files and
re-run `claude plugin validate .`.
### B. OAuth flow (manual, per-machine)
```bash
@@ -576,6 +595,80 @@ The OIDC client you use locally must accept
---
## Running the tests
```bash
pnpm test # all packages
pnpm --filter @shared-memory/web test:watch
```
Unit tests (e.g. `lib/memory-patch.test.ts`) need nothing but `pnpm install`.
The integration tests in `lib/mcp/tools.integration.test.ts` exercise the
real tool handlers against a **real Postgres with pgvector** — they assert on
stored rows, so there is no mock DB to drift from production behaviour. Spin
one up:
```bash
docker run -d --name sm-test-db \
-e POSTGRES_USER=test -e POSTGRES_PASSWORD=test \
-e POSTGRES_DB=shared_memory_test \
-p 55432:5432 pgvector/pgvector:pg16
for f in apps/web/drizzle/*.sql; do
docker exec -i sm-test-db psql -U test -d shared_memory_test -v ON_ERROR_STOP=1 -q < "$f"
done
pnpm test
```
The default `DATABASE_URL` assumes the published port is reachable on
localhost. If your test runner is itself inside a container, point it at the
database container's address instead:
```bash
DATABASE_URL="postgres://test:test@$(docker inspect -f \
'{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' sm-test-db):5432/shared_memory_test" \
pnpm test
```
The embedder sidecar is stubbed in tests (it's an external ML service); the
stub is deterministic per-text, so re-embedding is verified by asserting the
stored vector actually changed — not by asserting a mock was called.
Teardown: `docker rm -f sm-test-db`.
### Linting
```bash
pnpm lint
```
Runs the ESLint CLI directly against `eslint.config.mjs`. Note that `next
lint` is deprecated (it goes away in Next 16) and had never been configured
here, so this replaces it. `eslint-config-next` is still published in the
legacy `.eslintrc` format, so the config bridges it through `FlatCompat`;
that bridge can be dropped once the package ships a native flat export.
The tree is currently clean at `--max-warnings=0`, so adding that flag to
the `lint` script is a cheap way to keep it that way.
---
## Design notes
`docs/` holds decision records for changes whose reasoning isn't recoverable
from the diff — what was built, what was deliberately rejected, and what would
reopen a closed question.
- [`docs/memory-api-improvements.md`](docs/memory-api-improvements.md) — why
`memory_get` stopped returning the embedding and tsvector, why `memory_patch`
refuses ambiguous matches instead of guessing, why `memory_append` was
dropped, and why file-mirroring was left as a convention rather than
mechanised.
---
## Troubleshooting
- **`401 claim invalid: aud`** from `/api/mcp` — your MCP client isn't
+1 -1
View File
@@ -5,7 +5,7 @@ import { db } from "@/lib/db/client";
import { memories, projects, projectShares } from "@/lib/db/schema";
import { getUserGroupNames, readableProjectIds } from "@/lib/access";
import { Container, PageHeader } from "@/app/_components/ui/container";
import { Card, CardBody, CardHeader } from "@/app/_components/ui/card";
import { Card, CardBody } from "@/app/_components/ui/card";
import { Badge } from "@/app/_components/ui/badge";
import { Button } from "@/app/_components/ui/button";
import { EmptyState } from "@/app/_components/ui/empty-state";
@@ -6,7 +6,7 @@ import { db, pg } from "@/lib/db/client";
import { projects, users } from "@/lib/db/schema";
import { getProjectAccess, getUserGroupNames, readableProjectIds } from "@/lib/access";
import { Container, PageHeader } from "@/app/_components/ui/container";
import { Card, CardBody } from "@/app/_components/ui/card";
import { Card } from "@/app/_components/ui/card";
import { Badge } from "@/app/_components/ui/badge";
import { Button } from "@/app/_components/ui/button";
import { EmptyState } from "@/app/_components/ui/empty-state";
@@ -1,5 +1,5 @@
import { revalidatePath } from "next/cache";
import { and, asc, desc, eq, isNull } from "drizzle-orm";
import { and, asc, desc, eq } from "drizzle-orm";
import { auth } from "@/auth";
import { env } from "@/lib/env";
import { db } from "@/lib/db/client";
+33
View File
@@ -0,0 +1,33 @@
import { dirname } from "node:path";
import { fileURLToPath } from "node:url";
import { FlatCompat } from "@eslint/eslintrc";
/**
* ESLint flat config.
*
* `next lint` is deprecated (removed in Next 16) and was never configured
* here, so `pnpm lint` used to drop into an interactive setup prompt and
* exit non-zero. This runs the ESLint CLI directly instead.
*
* `eslint-config-next` is still published in the legacy .eslintrc format,
* so FlatCompat bridges it into flat config. That bridge goes away when
* the config ships a native flat export.
*/
const compat = new FlatCompat({
baseDirectory: dirname(fileURLToPath(import.meta.url)),
});
const config = [
{
ignores: [
".next/**",
"node_modules/**",
"next-env.d.ts",
// Generated SQL/journal artifacts from drizzle-kit.
"drizzle/**",
],
},
...compat.extends("next/core-web-vitals", "next/typescript"),
];
export default config;
+258
View File
@@ -0,0 +1,258 @@
import { afterAll, beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
/**
* Integration tests for memory.get and memory.patch against a REAL
* Postgres (pgvector). See CONTRIBUTING/README for spinning up the test
* database; without it these tests fail to connect rather than silently
* passing.
*
* The embedder sidecar is the one thing stubbed — it's an external HTTP
* service running an ML model. The stub is deterministic per-text, which
* lets the re-embedding test assert on the STORED VECTOR CHANGING (real
* DB state) rather than on "was the mock called".
*/
vi.mock("@/lib/embedder", () => ({
embedText: async (text: string) => {
// Deterministic pseudo-vector: distinct texts produce distinct vectors.
let h = 0;
for (let i = 0; i < text.length; i++) h = (h * 31 + text.charCodeAt(i)) | 0;
return Array.from({ length: 384 }, (_, i) => ((h + i * 7919) % 1000) / 1000);
},
embedTexts: async (texts: string[]) => texts.map(() => Array(384).fill(0.1)),
embedderReady: async () => true,
EmbedderError: class extends Error {},
}));
const { db, pg } = await import("@/lib/db/client");
const { memories, projects, users } = await import("@/lib/db/schema");
const { toolMap } = await import("@/lib/mcp/tools");
const { eq } = await import("drizzle-orm");
type UserContext = import("@/lib/mcp/context").UserContext;
const ORIGINAL = [
"# Roadmap",
"",
"## RECENTLY SHIPPED",
"- v1.0 initial release",
"",
"## IN PROGRESS",
"- patch primitive",
"",
].join("\n");
let userId: string;
let projectId: string;
let memoryId: string;
let ctx: UserContext;
async function seedMemory(content = ORIGINAL): Promise<string> {
const row = await db
.insert(memories)
.values({
userId,
projectId,
scope: "project",
content,
tags: ["roadmap"],
embedding: Array(384).fill(0.5),
})
.returning({ id: memories.id });
return row[0]!.id;
}
async function readContent(id: string): Promise<string> {
const r = await db
.select({ content: memories.content })
.from(memories)
.where(eq(memories.id, id));
return r[0]!.content;
}
beforeAll(async () => {
const u = await db
.insert(users)
.values({ oidcSub: "test-sub", oidcIss: "http://test", email: "t@example.com" })
.onConflictDoNothing()
.returning({ id: users.id });
userId =
u[0]?.id ??
(await db.select({ id: users.id }).from(users).limit(1))[0]!.id;
const p = await db
.insert(projects)
.values({ userId, key: "test-project", displayName: "Test Project" })
.onConflictDoNothing()
.returning({ id: projects.id });
projectId =
p[0]?.id ??
(await db.select({ id: projects.id }).from(projects).limit(1))[0]!.id;
ctx = {
userId,
sub: "test-sub",
iss: "http://test",
email: null,
name: null,
groups: [],
};
});
beforeEach(async () => {
memoryId = await seedMemory();
});
afterAll(async () => {
await db.delete(memories);
await pg.end();
});
describe("memory.get response shape (P1)", () => {
test("does not leak the embedding or the tsvector to the caller", async () => {
const res = await toolMap["memory.get"]!.handler({ id: memoryId }, ctx);
const fields = Object.keys(res.structuredContent as object);
expect(fields).not.toContain("embedding");
expect(fields).not.toContain("contentTsv");
});
test("returns exactly the same 9 fields as memory.list", async () => {
const res = await toolMap["memory.get"]!.handler({ id: memoryId }, ctx);
const fields = Object.keys(res.structuredContent as object).sort();
expect(fields).toEqual(
[
"content",
"createdAt",
"id",
"lastEditedBy",
"projectId",
"scope",
"tags",
"updatedAt",
"version",
].sort(),
);
});
test("still returns the full content", async () => {
const res = await toolMap["memory.get"]!.handler({ id: memoryId }, ctx);
expect((res.structuredContent as { content: string }).content).toBe(ORIGINAL);
});
});
describe("memory.patch (P2)", () => {
test("applies a unique patch and increments version by exactly 1", async () => {
const before = await db
.select({ version: memories.version })
.from(memories)
.where(eq(memories.id, memoryId));
const res = await toolMap["memory.patch"]!.handler(
{
id: memoryId,
old_string: "## RECENTLY SHIPPED",
new_string: "## RECENTLY SHIPPED\n- v1.1 patch primitive",
},
ctx,
);
expect(res.isError).toBeFalsy();
const after = res.structuredContent as { version: number };
expect(after.version).toBe(before[0]!.version + 1);
expect(await readContent(memoryId)).toContain("- v1.1 patch primitive");
// The rest of the document survived.
expect(await readContent(memoryId)).toContain("- v1.0 initial release");
expect(await readContent(memoryId)).toContain("## IN PROGRESS");
});
test("refuses an absent old_string and leaves content byte-identical", async () => {
const res = await toolMap["memory.patch"]!.handler(
{ id: memoryId, old_string: "## NOT PRESENT", new_string: "x" },
ctx,
);
expect(res.isError).toBe(true);
expect(await readContent(memoryId)).toBe(ORIGINAL);
});
test("refuses an ambiguous old_string, naming the count, leaving content unchanged", async () => {
const id = await seedMemory("alpha\nalpha\nbeta\n");
const res = await toolMap["memory.patch"]!.handler(
{ id, old_string: "alpha", new_string: "gamma" },
ctx,
);
expect(res.isError).toBe(true);
expect(res.content[0]!.text).toMatch(/2/);
expect(await readContent(id)).toBe("alpha\nalpha\nbeta\n");
});
test("refuses a stale version and leaves content unchanged", async () => {
const current = await db
.select({ version: memories.version })
.from(memories)
.where(eq(memories.id, memoryId));
const res = await toolMap["memory.patch"]!.handler(
{
id: memoryId,
old_string: "## IN PROGRESS",
new_string: "## DONE",
version: current[0]!.version + 99,
},
ctx,
);
expect(res.isError).toBe(true);
expect(await readContent(memoryId)).toBe(ORIGINAL);
});
test("rejects a patch that would push content past the 64,000-char limit", async () => {
const id = await seedMemory("A".repeat(63_950) + "ANCHOR");
const res = await toolMap["memory.patch"]!.handler(
{ id, old_string: "ANCHOR", new_string: "B".repeat(100) },
ctx,
);
expect(res.isError).toBe(true);
expect(await readContent(id)).toBe("A".repeat(63_950) + "ANCHOR");
});
test("re-embeds: the stored vector changes after a patch", async () => {
const before = await pg<{ embedding: string }[]>`
SELECT embedding::text AS embedding FROM memories WHERE id = ${memoryId}
`;
await toolMap["memory.patch"]!.handler(
{ id: memoryId, old_string: "- patch primitive", new_string: "- shipped it" },
ctx,
);
const after = await pg<{ embedding: string }[]>`
SELECT embedding::text AS embedding FROM memories WHERE id = ${memoryId}
`;
expect(after[0]!.embedding).not.toBe(before[0]!.embedding);
});
test("full-text index updates itself, because content_tsv is a generated column", async () => {
// This is the claim that a patch cannot rot FTS. Postgres maintains
// content_tsv; only the embedding needs an explicit recompute.
await toolMap["memory.patch"]!.handler(
{
id: memoryId,
old_string: "- patch primitive",
new_string: "- kumquat marmalade",
},
ctx,
);
const hit = await pg<{ n: number }[]>`
SELECT count(*)::int AS n FROM memories
WHERE id = ${memoryId} AND content_tsv @@ plainto_tsquery('english', 'kumquat')
`;
expect(hit[0]!.n).toBe(1);
});
});
+244
View File
@@ -0,0 +1,244 @@
import { afterAll, beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
/**
* Integration cover for the memory MUTATION paths (write / update / delete)
* against a real Postgres. These exist mainly as a safety net for the
* shared-mutation refactor: the MCP tools and the Web UI Server Actions
* used to reimplement the same authorize → CAS → re-embed → audit sequence
* separately, and these assertions pin the behaviour that must survive
* being pulled into one place.
*/
vi.mock("@/lib/embedder", () => ({
embedText: async (text: string) => {
let h = 0;
for (let i = 0; i < text.length; i++) h = (h * 31 + text.charCodeAt(i)) | 0;
return Array.from({ length: 384 }, (_, i) => ((h + i * 7919) % 1000) / 1000);
},
embedTexts: async (texts: string[]) => texts.map(() => Array(384).fill(0.1)),
embedderReady: async () => true,
EmbedderError: class extends Error {},
}));
const { db, pg } = await import("@/lib/db/client");
const { memories, projects, users, groups, userGroups, projectShares } = await import(
"@/lib/db/schema"
);
const { toolMap } = await import("@/lib/mcp/tools");
const { eq } = await import("drizzle-orm");
type UserContext = import("@/lib/mcp/context").UserContext;
const ISS = "http://test";
let author: UserContext;
let projectOwnerId: string;
let ownProjectId: string;
let sharedProjectId: string;
let sharedGroupId: string;
function ctxFor(userId: string, sub: string, groupNames: string[] = []): UserContext {
return { userId, sub, iss: ISS, email: null, name: null, groups: groupNames };
}
async function upsertUser(sub: string): Promise<string> {
const r = await db
.insert(users)
.values({ oidcSub: sub, oidcIss: ISS })
.onConflictDoUpdate({ target: [users.oidcIss, users.oidcSub], set: { oidcSub: sub } })
.returning({ id: users.id });
return r[0]!.id;
}
async function seedMemory(
userId: string,
projectId: string | null,
content = "seed content",
): Promise<string> {
const r = await db
.insert(memories)
.values({
userId,
projectId,
scope: projectId ? "project" : "user",
content,
tags: [],
embedding: Array(384).fill(0.5),
lastEditedBy: userId,
})
.returning({ id: memories.id });
return r[0]!.id;
}
async function setShareAccess(access: "ro" | "rw") {
await db
.insert(projectShares)
.values({ projectId: sharedProjectId, groupId: sharedGroupId, access })
.onConflictDoUpdate({
target: [projectShares.projectId, projectShares.groupId],
set: { access },
});
}
async function isDeleted(id: string): Promise<boolean> {
const r = await db
.select({ deletedAt: memories.deletedAt })
.from(memories)
.where(eq(memories.id, id));
return r[0]!.deletedAt !== null;
}
beforeAll(async () => {
const authorId = await upsertUser("author-sub");
projectOwnerId = await upsertUser("owner-sub");
const own = await db
.insert(projects)
.values({ userId: authorId, key: "author-own", displayName: "Author Own" })
.onConflictDoNothing()
.returning({ id: projects.id });
ownProjectId =
own[0]?.id ??
(
await db
.select({ id: projects.id })
.from(projects)
.where(eq(projects.key, "author-own"))
)[0]!.id;
const shared = await db
.insert(projects)
.values({ userId: projectOwnerId, key: "team-shared", displayName: "Team Shared" })
.onConflictDoNothing()
.returning({ id: projects.id });
sharedProjectId =
shared[0]?.id ??
(
await db
.select({ id: projects.id })
.from(projects)
.where(eq(projects.key, "team-shared"))
)[0]!.id;
const g = await db
.insert(groups)
.values({ oidcIss: ISS, name: "team" })
.onConflictDoNothing()
.returning({ id: groups.id });
sharedGroupId =
g[0]?.id ??
(await db.select({ id: groups.id }).from(groups).where(eq(groups.name, "team")))[0]!
.id;
await db
.insert(userGroups)
.values({ userId: authorId, groupId: sharedGroupId })
.onConflictDoNothing();
author = ctxFor(authorId, "author-sub", ["team"]);
});
beforeEach(async () => {
await db.delete(memories);
await setShareAccess("rw");
});
afterAll(async () => {
await db.delete(memories);
await pg.end();
});
describe("memory.write", () => {
test("writes into a project the caller owns", async () => {
const res = await toolMap["memory.write"]!.handler(
{ content: "hello", scope: "project", project: "author-own" },
author,
);
expect(res.isError).toBeFalsy();
});
test("refuses an unknown project rather than creating one", async () => {
const res = await toolMap["memory.write"]!.handler(
{ content: "hello", scope: "project", project: "does-not-exist" },
author,
);
expect(res.isError).toBe(true);
expect(res.content[0]!.text).toMatch(/project\.identify/);
});
});
describe("memory.update", () => {
test("updates content and increments version", async () => {
const id = await seedMemory(author.userId, ownProjectId);
const before = await db
.select({ version: memories.version })
.from(memories)
.where(eq(memories.id, id));
const res = await toolMap["memory.update"]!.handler(
{ id, content: "revised content" },
author,
);
expect(res.isError).toBeFalsy();
expect((res.structuredContent as { version: number }).version).toBe(
before[0]!.version + 1,
);
});
test("refuses a stale version", async () => {
const id = await seedMemory(author.userId, ownProjectId);
const res = await toolMap["memory.update"]!.handler(
{ id, content: "revised", version: 99 },
author,
);
expect(res.isError).toBe(true);
});
test("denies updating a memory in a project shared read-only", async () => {
const id = await seedMemory(author.userId, sharedProjectId);
await setShareAccess("ro");
const res = await toolMap["memory.update"]!.handler(
{ id, content: "sneaky edit" },
author,
);
expect(res.isError).toBe(true);
});
});
describe("memory.delete authorization", () => {
test("allows deleting a memory in a project shared read-write", async () => {
const id = await seedMemory(author.userId, sharedProjectId);
const res = await toolMap["memory.delete"]!.handler({ id }, author);
expect(res.isError).toBeFalsy();
expect(await isDeleted(id)).toBe(true);
});
test("denies deleting a memory in a project shared read-only, even to its author", async () => {
// The realistic path here: the memory was written while the share was
// rw, then an owner downgraded the group to ro. Authoring the row must
// not grant a standing write privilege the project ACL has revoked —
// memory.update already refuses this, and delete must agree.
const id = await seedMemory(author.userId, sharedProjectId);
await setShareAccess("ro");
const res = await toolMap["memory.delete"]!.handler({ id }, author);
expect(res.isError).toBe(true);
expect(await isDeleted(id)).toBe(false);
});
test("denies deleting another user's user-scope memory", async () => {
const id = await seedMemory(projectOwnerId, null);
const res = await toolMap["memory.delete"]!.handler({ id }, author);
expect(res.isError).toBe(true);
expect(await isDeleted(id)).toBe(false);
});
});
+109 -222
View File
@@ -11,6 +11,7 @@ import {
MemoryIdInput,
MemoryDeleteInput,
MemoryListInput,
MemoryPatchInput,
MemorySearchInput,
MemoryUpdateInput,
MemoryWriteInput,
@@ -20,20 +21,22 @@ import {
SnippetListInput,
SnippetDeleteInput,
} from "@shared-memory/schemas";
import { embedText } from "@/lib/embedder";
import { searchMemories } from "@/lib/memories";
import {
createMemory,
patchMemory,
softDeleteMemory,
updateMemory,
type Actor,
type ProjectResolver,
} from "@/lib/memory-mutations";
import {
getSnippet,
putSnippet,
listSnippets,
softDeleteSnippet,
} from "@/lib/snippets";
import {
CONCURRENT_EDIT_ERROR,
canWriteProject,
getProjectAccess,
readableProjectIds,
} from "@/lib/access";
import { getProjectAccess, readableProjectIds } from "@/lib/access";
import type { UserContext } from "./context";
/**
@@ -151,6 +154,27 @@ function withDefaultProject(
return { ...obj, project: ctx.defaultProjectKey };
}
/** Adapt an MCP request context to the shared mutation layer. */
function mcpActor(ctx: UserContext): Actor {
return { userId: ctx.userId, groups: ctx.groups, via: "mcp" };
}
/**
* Project resolution for MCP writes. Unlike the Web UI, the MCP surface
* never auto-creates a project — an unknown key is an error telling the
* caller to run project.identify first, which keeps agents from silently
* spawning near-miss projects off a typo'd key.
*/
function mcpProjectResolver(ctx: UserContext): ProjectResolver {
return async (key: string) => {
const id = await resolveProjectId(ctx, key);
if (!id) {
return { ok: false, error: `unknown project '${key}'; call project.identify first` };
}
return { ok: true, value: id };
};
}
// ---------- tools ----------
const projectIdentify: ToolDef = {
@@ -373,55 +397,18 @@ const memoryWrite: ToolDef = {
const parsed = MemoryWriteInput.safeParse(withDefaultProject(args, ctx));
if (!parsed.success) return err(parsed.error.message);
const scope = parsed.data.scope;
let projectId: string | null = null;
let projectKey: string | undefined = undefined;
if (scope === "project") {
projectKey = projectKeyOrDefault(ctx, parsed.data.project);
if (!projectKey) {
return err("scope=project requires `project` key (or X-Project-Key header)");
}
projectId = await resolveProjectId(ctx, projectKey);
if (!projectId) {
return err(`unknown project '${projectKey}'; call project.identify first`);
}
// Authorize write. Owner always allowed; otherwise require rw.
const allowed = await canWriteProject(ctx.userId, ctx.groups, projectId);
if (!allowed) {
return err(`no write access to project '${projectKey}'`);
}
// Fold the X-Project-Key fallback in before the shared path sees it.
const input = {
...parsed.data,
project: projectKeyOrDefault(ctx, parsed.data.project),
};
if (input.scope === "project" && !input.project) {
return err("scope=project requires `project` key (or X-Project-Key header)");
}
// Embed inline so the new memory is searchable immediately. Slower
// writes (~50150 ms) are an acceptable price for that guarantee; if
// embedder pressure ever forces an async path, only this section
// needs to change.
const embedding = await embedText(parsed.data.content);
const inserted = await db
.insert(memories)
.values({
userId: ctx.userId,
projectId,
scope,
content: parsed.data.content,
tags: parsed.data.tags ?? [],
embedding,
lastEditedBy: ctx.userId,
})
.returning({ id: memories.id, createdAt: memories.createdAt });
const m = inserted[0]!;
await db.insert(auditLog).values({
userId: ctx.userId,
actor: "mcp",
action: "memory.write",
entityType: "memory",
entityId: m.id,
payload: { scope, projectKey: projectKey ?? null, tags: parsed.data.tags ?? [] },
});
return ok({ id: m.id, createdAt: m.createdAt }, `wrote memory ${m.id}`);
const res = await createMemory(mcpActor(ctx), input, mcpProjectResolver(ctx));
if (!res.ok) return err(res.error);
return ok(res.value, `wrote memory ${res.value.id}`);
},
};
@@ -508,8 +495,28 @@ const memoryGet: ToolDef = {
const parsed = MemoryIdInput.safeParse(args);
if (!parsed.success) return err(parsed.error.message);
// Project explicitly rather than `select()`-ing the raw row. The
// table carries `embedding` (384 floats) and `content_tsv` (the full
// lexeme index, which outgrows `content` itself on large memories) —
// both are Postgres retrieval internals that no MCP client can use,
// and together they were the majority of every response. Returning
// them also pushed large memories past the tool-output cap. This is
// the same 9-field shape memory.list and memory.search return.
const row = await db
.select()
.select({
id: memories.id,
scope: memories.scope,
projectId: memories.projectId,
content: memories.content,
tags: memories.tags,
version: memories.version,
lastEditedBy: memories.lastEditedBy,
createdAt: memories.createdAt,
updatedAt: memories.updatedAt,
// Needed for the authorization check below; stripped before the
// response so the payload matches list/search exactly.
userId: memories.userId,
})
.from(memories)
.where(and(eq(memories.id, parsed.data.id), isNull(memories.deletedAt)))
.limit(1);
@@ -518,8 +525,8 @@ const memoryGet: ToolDef = {
// Authorize read: own row, OR project-scope row in an accessible
// project. Anything else looks "not found" to the caller.
const m = row[0];
if (m.userId !== ctx.userId) {
const { userId, ...m } = row[0];
if (userId !== ctx.userId) {
if (!m.projectId) return err("not found");
const access = await getProjectAccess(ctx.userId, ctx.groups, m.projectId);
if (access === null) return err("not found");
@@ -550,57 +557,9 @@ const memoryDelete: ToolDef = {
const parsed = MemoryDeleteInput.safeParse(args);
if (!parsed.success) return err(parsed.error.message);
// Look up the row first to authorize and capture its current version
// for the CAS. Shared-project writes need a per-project access check.
const target = await db
.select({
id: memories.id,
userId: memories.userId,
projectId: memories.projectId,
scope: memories.scope,
version: memories.version,
})
.from(memories)
.where(and(eq(memories.id, parsed.data.id), isNull(memories.deletedAt)))
.limit(1);
const m = target[0];
if (!m) return err("not found");
if (m.userId !== ctx.userId) {
// Not the owner. User-scope memories can only be deleted by their
// owner; project-scope require rw access on the project.
if (m.scope === "user" || !m.projectId) return err("not found");
const allowed = await canWriteProject(ctx.userId, ctx.groups, m.projectId);
if (!allowed) return err("no write access to this project");
}
// Optimistic-lock CAS: pin to the caller-supplied version when given,
// else the version we just read in this handler. The 0-row response
// tells us a peer raced us.
const expectedVersion = parsed.data.version ?? m.version;
const updated = await db
.update(memories)
.set({ deletedAt: new Date(), lastEditedBy: ctx.userId })
.where(
and(
eq(memories.id, parsed.data.id),
eq(memories.version, expectedVersion),
isNull(memories.deletedAt),
),
)
.returning({ id: memories.id });
if (!updated[0]) return err(CONCURRENT_EDIT_ERROR);
await db.insert(auditLog).values({
userId: ctx.userId,
actor: "mcp",
action: "memory.delete",
entityType: "memory",
entityId: updated[0].id,
});
return ok({ id: updated[0].id, deleted: true }, `deleted memory ${updated[0].id}`);
const res = await softDeleteMemory(mcpActor(ctx), parsed.data);
if (!res.ok) return err(res.error);
return ok({ id: res.value.id, deleted: true }, `deleted memory ${res.value.id}`);
},
};
@@ -638,124 +597,51 @@ const memoryUpdate: ToolDef = {
const parsed = MemoryUpdateInput.safeParse(withDefaultProject(args, ctx));
if (!parsed.success) return err(parsed.error.message);
const existingRows = await db
.select({
id: memories.id,
content: memories.content,
scope: memories.scope,
projectId: memories.projectId,
projectKey: projects.key,
version: memories.version,
userId: memories.userId,
})
.from(memories)
.leftJoin(projects, eq(memories.projectId, projects.id))
.where(and(eq(memories.id, parsed.data.id), isNull(memories.deletedAt)))
.limit(1);
const existing = existingRows[0];
if (!existing) return err("not found");
const res = await updateMemory(mcpActor(ctx), parsed.data, mcpProjectResolver(ctx));
if (!res.ok) return err(res.error);
return ok(res.value, `updated memory ${res.value.id}`);
},
};
// Authorize write.
if (existing.scope === "user") {
if (existing.userId !== ctx.userId) return err("not found");
} else if (existing.projectId) {
const allowed = await canWriteProject(ctx.userId, ctx.groups, existing.projectId);
if (!allowed) return err("no write access to this project");
}
const memoryPatch: ToolDef = {
name: "memory.patch",
description:
"Replace one exact snippet of a memory's content, leaving the rest untouched — the same mental model as editing a file. Use this INSTEAD of memory.update whenever you're making a small edit to a large memory: adding an entry under a heading, correcting a line, updating a status. memory.update requires you to resend the entire document, which risks silently dropping content you didn't mean to touch; memory.patch only needs the fragment you're changing. `old_string` must appear EXACTLY once — if it's missing or ambiguous the call fails and nothing is changed, so include enough surrounding context to make it unique. Pass an empty `new_string` to delete the matched text. Re-embeds automatically, preserves the memory's id, and accepts `version` for the same concurrent-edit protection as memory.update.",
inputSchema: {
type: "object",
properties: {
id: { type: "string", format: "uuid" },
old_string: {
type: "string",
description:
"The exact text to replace. Must occur exactly once in the memory's content — include surrounding lines if the fragment alone would be ambiguous.",
},
new_string: {
type: "string",
description:
"The replacement text. May be empty to delete the matched text (the memory itself may not be left empty).",
},
version: {
type: "integer",
minimum: 0,
description:
"Optimistic-locking token from memory.get / memory.list. When supplied, the patch is rejected if the row was edited by someone else since you read it.",
},
},
required: ["id", "old_string", "new_string"],
},
async handler(args, ctx) {
const parsed = MemoryPatchInput.safeParse(args);
if (!parsed.success) return err(parsed.error.message);
const update: Record<string, unknown> = {
updatedAt: new Date(),
lastEditedBy: ctx.userId,
version: existing.version + 1,
};
if (parsed.data.tags !== undefined) update.tags = parsed.data.tags;
if (parsed.data.content !== undefined && parsed.data.content !== existing.content) {
update.content = parsed.data.content;
update.embedding = await embedText(parsed.data.content);
}
const res = await patchMemory(mcpActor(ctx), parsed.data);
if (!res.ok) return err(res.error);
let scopeChanged = false;
let projectChanged = false;
let newProjectKey: string | null = existing.projectKey ?? null;
if (parsed.data.scope !== undefined) {
if (parsed.data.scope === "user") {
if (existing.scope !== "user") {
update.scope = "user";
scopeChanged = true;
}
if (existing.projectId !== null) {
update.projectId = null;
projectChanged = true;
newProjectKey = null;
}
} else {
// scope === 'project' — schema refine guarantees `project` is set.
const projectKey = parsed.data.project!;
const projectId = await resolveProjectId(ctx, projectKey);
if (!projectId) {
return err(`unknown project '${projectKey}'; call project.identify first`);
}
// Moving INTO a project requires write access there.
const allowedTarget = await canWriteProject(ctx.userId, ctx.groups, projectId);
if (!allowedTarget) {
return err(`no write access to project '${projectKey}'`);
}
if (existing.scope !== "project") {
update.scope = "project";
scopeChanged = true;
}
if (existing.projectId !== projectId) {
update.projectId = projectId;
projectChanged = true;
newProjectKey = projectKey;
}
}
}
const expectedVersion = parsed.data.version ?? existing.version;
const updated = await db
.update(memories)
.set(update)
.where(
and(
eq(memories.id, parsed.data.id),
eq(memories.version, expectedVersion),
),
)
.returning({
id: memories.id,
updatedAt: memories.updatedAt,
version: memories.version,
});
if (!updated[0]) return err(CONCURRENT_EDIT_ERROR);
const auditFields = Object.keys(update).filter(
(k) => k !== "updatedAt" && k !== "version" && k !== "lastEditedBy",
const { id, delta, contentLength } = res.value;
return ok(
res.value,
`patched memory ${id} (${delta >= 0 ? "+" : ""}${delta} chars, now ${contentLength})`,
);
const auditPayload: Record<string, unknown> = { fields: auditFields };
if (scopeChanged || projectChanged) {
auditPayload.scope = {
from: existing.scope,
to: update.scope ?? existing.scope,
};
auditPayload.projectKey = {
from: existing.projectKey ?? null,
to: newProjectKey,
};
}
await db.insert(auditLog).values({
userId: ctx.userId,
actor: "mcp",
action: "memory.update",
entityType: "memory",
entityId: updated[0]!.id,
payload: auditPayload,
});
return ok(updated[0]!, `updated memory ${updated[0]!.id}`);
},
};
@@ -1113,6 +999,7 @@ export const tools: ToolDef[] = [
projectIdentify,
memoryWrite,
memoryUpdate,
memoryPatch,
memoryList,
memoryGet,
memorySearch,
+72 -289
View File
@@ -2,37 +2,36 @@
import { revalidatePath } from "next/cache";
import { redirect } from "next/navigation";
import { and, eq, inArray, isNull } from "drizzle-orm";
import { and, eq, inArray } from "drizzle-orm";
import { auth } from "@/auth";
import { db } from "@/lib/db/client";
import { memories, projects, auditLog } from "@/lib/db/schema";
import { embedText } from "@/lib/embedder";
import { projects } from "@/lib/db/schema";
import { resolveProjectId, upsertProject } from "@/lib/projects";
import {
MemoryWriteInput,
MemoryUpdateInput,
MemoryDeleteInput,
} from "@shared-memory/schemas";
import { getUserGroupNames, readableProjectIds } from "@/lib/access";
import {
CONCURRENT_EDIT_ERROR,
canWriteProject,
getUserGroupNames,
readableProjectIds,
} from "@/lib/access";
createMemory,
softDeleteMemory,
updateMemory,
type Actor,
type Outcome,
type ProjectResolver,
} from "@/lib/memory-mutations";
/**
* Server Actions for memory CRUD from the Web UI. Mirrors the MCP tools
* but writes through the same DB layer, so updates and deletes here are
* indistinguishable from those made via Claude Code.
* Server Actions for memory CRUD from the Web UI.
*
* These are thin adapters: form parsing, then `lib/memory-mutations`,
* then revalidate/redirect. The authorize → mutate → re-embed → CAS →
* audit sequence lives in that shared module so this surface and the MCP
* tools cannot drift apart — they previously did, and the sharing rules
* ended up subtly different between them.
*
* `actor` is "web" in audit_log so we can tell the two paths apart later.
*
* Sharing: project-scope memories may live under projects shared with
* the user's groups. Reads include those projects; writes require the
* user to own the project or have an `rw` share. Cross-user concurrent
* edits use the `version` column for optimistic locking — if the stored
* version no longer matches what the form submitted, we surface
* `CONCURRENT_EDIT_ERROR` rather than clobber.
*/
async function requireUserId(): Promise<string> {
@@ -41,6 +40,52 @@ async function requireUserId(): Promise<string> {
return session.user.id;
}
/** Server Actions signal failure by throwing; the shared layer returns Outcome. */
function must<T>(outcome: Outcome<T>): T {
if (!outcome.ok) throw new Error(outcome.error);
return outcome.value;
}
async function webActor(): Promise<{ actor: Actor; resolveProject: ProjectResolver }> {
const userId = await requireUserId();
const groups = await getUserGroupNames(userId);
return {
actor: { userId, groups, via: "web" },
resolveProject: webProjectResolver(userId, groups),
};
}
/**
* Project resolution for Web UI writes. Unlike the MCP surface, an
* unknown key is CREATED rather than rejected — a person typing a project
* name into a form means to make one. Shared projects are matched only
* within the set the user can actually read, because `projects.key` is
* unique per user rather than globally: an unscoped key match could
* otherwise select someone else's project.
*
* Write access to whatever this returns is enforced centrally by the
* mutation layer, so it deliberately isn't re-checked here.
*/
function webProjectResolver(userId: string, groupNames: string[]): ProjectResolver {
return async (key: string) => {
const owned = await resolveProjectId(userId, key);
if (owned) return { ok: true, value: owned };
const readableIds = await readableProjectIds(userId, groupNames);
const shared =
readableIds.length > 0
? await db
.select({ id: projects.id })
.from(projects)
.where(and(eq(projects.key, key), inArray(projects.id, readableIds)))
.limit(1)
: [];
if (shared[0]) return { ok: true, value: shared[0].id };
return { ok: true, value: await upsertProject(userId, key) };
};
}
function parseTags(raw: FormDataEntryValue | null): string[] {
if (typeof raw !== "string") return [];
return raw
@@ -50,95 +95,26 @@ function parseTags(raw: FormDataEntryValue | null): string[] {
}
export async function createMemoryAction(formData: FormData) {
const userId = await requireUserId();
const groupNames = await getUserGroupNames(userId);
const { actor, resolveProject } = await webActor();
const payload = {
const parsed = MemoryWriteInput.safeParse({
content: String(formData.get("content") ?? "").trim(),
scope: (formData.get("scope") as "project" | "user") || "project",
project: (formData.get("project") as string | null)?.trim() || undefined,
tags: parseTags(formData.get("tags")),
};
const parsed = MemoryWriteInput.safeParse(payload);
});
if (!parsed.success) {
throw new Error(parsed.error.issues.map((i) => i.message).join("; "));
}
let projectId: string | null = null;
if (parsed.data.scope === "project") {
if (!parsed.data.project) throw new Error("scope=project requires `project`");
// Same priority as memory.update's reclassification path: prefer an
// owned project; otherwise check for a shared one we have rw on;
// otherwise auto-upsert as owner.
const owned = await resolveProjectId(userId, parsed.data.project);
if (owned) {
projectId = owned;
} else {
// Restrict the by-key lookup to projects the user can actually
// read. Without this, a different user's project with the same
// key string could be selected (`projects.key` is unique per user,
// not globally), opening a cross-user write hazard.
const readableIds = await readableProjectIds(userId, groupNames);
const sharedRow =
readableIds.length > 0
? await db
.select({ id: projects.id })
.from(projects)
.where(
and(
eq(projects.key, parsed.data.project),
inArray(projects.id, readableIds),
),
)
.limit(1)
: [];
if (sharedRow[0]) {
const allowed = await canWriteProject(userId, groupNames, sharedRow[0].id);
if (!allowed) {
throw new Error(`no write access to project '${parsed.data.project}'`);
}
projectId = sharedRow[0].id;
} else {
projectId = await upsertProject(userId, parsed.data.project);
}
}
}
const embedding = await embedText(parsed.data.content);
const inserted = await db
.insert(memories)
.values({
userId,
projectId,
scope: parsed.data.scope,
content: parsed.data.content,
tags: parsed.data.tags ?? [],
embedding,
lastEditedBy: userId,
})
.returning({ id: memories.id });
await db.insert(auditLog).values({
userId,
actor: "web",
action: "memory.write",
entityType: "memory",
entityId: inserted[0]!.id,
payload: {
scope: parsed.data.scope,
projectKey: parsed.data.project ?? null,
tags: parsed.data.tags ?? [],
},
});
const created = must(await createMemory(actor, parsed.data, resolveProject));
revalidatePath("/memories");
redirect(`/memories/${inserted[0]!.id}`);
redirect(`/memories/${created.id}`);
}
export async function updateMemoryAction(formData: FormData) {
const userId = await requireUserId();
const groupNames = await getUserGroupNames(userId);
const { actor, resolveProject } = await webActor();
const id = String(formData.get("id") ?? "");
const rawScope = formData.get("scope");
@@ -164,153 +140,7 @@ export async function updateMemoryAction(formData: FormData) {
throw new Error(parsed.error.issues.map((i) => i.message).join("; "));
}
// Fetch the row regardless of ownership — we may be editing a shared
// memory. Authorization is enforced below against the project, not
// by `user_id`.
const existingRows = await db
.select({
id: memories.id,
content: memories.content,
scope: memories.scope,
projectId: memories.projectId,
projectKey: projects.key,
version: memories.version,
userId: memories.userId,
})
.from(memories)
.leftJoin(projects, eq(memories.projectId, projects.id))
.where(and(eq(memories.id, parsed.data.id), isNull(memories.deletedAt)))
.limit(1);
const existing = existingRows[0];
if (!existing) throw new Error("not found");
// Authorize write. For user-scope memories, only the owner can edit.
// For project-scope memories, owner OR a group with rw access.
if (existing.scope === "user") {
if (existing.userId !== userId) throw new Error("not found");
} else if (existing.projectId) {
const allowed = await canWriteProject(userId, groupNames, existing.projectId);
if (!allowed) {
throw new Error("you don't have write access to this project");
}
}
const update: Record<string, unknown> = {
updatedAt: new Date(),
lastEditedBy: userId,
version: existing.version + 1,
};
if (parsed.data.tags !== undefined) update.tags = parsed.data.tags;
if (parsed.data.content !== undefined && parsed.data.content !== existing.content) {
update.content = parsed.data.content;
update.embedding = await embedText(parsed.data.content);
}
let scopeChanged = false;
let projectChanged = false;
let newProjectKey: string | null = existing.projectKey ?? null;
if (parsed.data.scope !== undefined) {
if (parsed.data.scope === "user") {
if (existing.scope !== "user") {
update.scope = "user";
scopeChanged = true;
}
if (existing.projectId !== null) {
update.projectId = null;
projectChanged = true;
newProjectKey = null;
}
} else {
// scope === 'project' — schema refine guarantees project is set.
// Moving INTO a project requires write access there. Owners get
// a fresh project upsert; non-owners must target an existing one
// they have rw on.
const projectKey = parsed.data.project!;
let projectId: string;
const existingId = await resolveProjectId(userId, projectKey);
if (existingId) {
projectId = existingId;
} else {
// Restrict the shared-project lookup to projects the user can
// actually read (`projects.key` is unique per user, not globally,
// so an unscoped key match could resolve another user's project).
const readableIds = await readableProjectIds(userId, groupNames);
const sharedRow =
readableIds.length > 0
? await db
.select({ id: projects.id })
.from(projects)
.where(
and(eq(projects.key, projectKey), inArray(projects.id, readableIds)),
)
.limit(1)
: [];
if (sharedRow[0]) {
const allowed = await canWriteProject(userId, groupNames, sharedRow[0].id);
if (!allowed) {
throw new Error(`no write access to project '${projectKey}'`);
}
projectId = sharedRow[0].id;
} else {
// Auto-upsert as owner — user becomes the project owner of a
// brand-new private project.
projectId = await upsertProject(userId, projectKey);
}
}
if (existing.scope !== "project") {
update.scope = "project";
scopeChanged = true;
}
if (existing.projectId !== projectId) {
update.projectId = projectId;
projectChanged = true;
newProjectKey = projectKey;
}
}
}
// Optimistic-locking guard. When `version` is supplied, the UPDATE
// matches on (id, version); a 0-row result means the caller's view
// is stale. When `version` is NOT supplied, we still match on the
// pre-fetched version to keep behaviour deterministic.
const expectedVersion = parsed.data.version ?? existing.version;
const updated = await db
.update(memories)
.set(update)
.where(
and(
eq(memories.id, parsed.data.id),
eq(memories.version, expectedVersion),
),
)
.returning({ id: memories.id });
if (!updated[0]) throw new Error(CONCURRENT_EDIT_ERROR);
const auditFields = Object.keys(update).filter(
(k) => k !== "updatedAt" && k !== "version" && k !== "lastEditedBy",
);
const auditPayload: Record<string, unknown> = { fields: auditFields };
if (scopeChanged || projectChanged) {
auditPayload.scope = {
from: existing.scope,
to: update.scope ?? existing.scope,
};
auditPayload.projectKey = {
from: existing.projectKey ?? null,
to: newProjectKey,
};
}
await db.insert(auditLog).values({
userId,
actor: "web",
action: "memory.update",
entityType: "memory",
entityId: parsed.data.id,
payload: auditPayload,
});
must(await updateMemory(actor, parsed.data, resolveProject));
revalidatePath(`/memories/${parsed.data.id}`);
revalidatePath("/memories");
@@ -318,8 +148,7 @@ export async function updateMemoryAction(formData: FormData) {
}
export async function deleteMemoryAction(formData: FormData) {
const userId = await requireUserId();
const groupNames = await getUserGroupNames(userId);
const { actor } = await webActor();
const id = String(formData.get("id") ?? "");
const rawVersion = formData.get("version");
const version =
@@ -332,53 +161,7 @@ export async function deleteMemoryAction(formData: FormData) {
});
if (!parsed.success) throw new Error(parsed.error.issues[0]!.message);
// Authorize delete: same rule as update — owner OR rw on the project.
const existing = await db
.select({
id: memories.id,
scope: memories.scope,
projectId: memories.projectId,
userId: memories.userId,
version: memories.version,
})
.from(memories)
.where(and(eq(memories.id, parsed.data.id), isNull(memories.deletedAt)))
.limit(1);
const row = existing[0];
if (!row) throw new Error("not found");
if (row.scope === "user") {
if (row.userId !== userId) throw new Error("not found");
} else if (row.projectId) {
const allowed = await canWriteProject(userId, groupNames, row.projectId);
if (!allowed) throw new Error("you don't have write access to this project");
}
// CAS on version so a peer's concurrent edit can't be silently overwritten
// by this delete. Form may or may not supply version; fall back to the row
// we just read to keep behaviour deterministic.
const expectedVersion = parsed.data.version ?? row.version;
const updated = await db
.update(memories)
.set({ deletedAt: new Date(), lastEditedBy: userId })
.where(
and(
eq(memories.id, parsed.data.id),
eq(memories.version, expectedVersion),
isNull(memories.deletedAt),
),
)
.returning({ id: memories.id });
if (!updated[0]) throw new Error(CONCURRENT_EDIT_ERROR);
await db.insert(auditLog).values({
userId,
actor: "web",
action: "memory.delete",
entityType: "memory",
entityId: updated[0].id,
});
must(await softDeleteMemory(actor, parsed.data));
revalidatePath("/memories");
redirect("/memories");
+263
View File
@@ -0,0 +1,263 @@
import { afterAll, beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
/**
* Tests for the shared mutation layer itself — the code both the MCP
* tools and the Web UI Server Actions now route through.
*
* Two things matter here:
* 1. The ProjectResolver seam really is the ONLY behavioural difference
* between the two surfaces.
* 2. The authorization rule is uniform across update / patch / delete.
* It previously wasn't: delete-over-MCP let a row's author bypass the
* project ACL.
*/
vi.mock("@/lib/embedder", () => ({
embedText: async (text: string) => {
let h = 0;
for (let i = 0; i < text.length; i++) h = (h * 31 + text.charCodeAt(i)) | 0;
return Array.from({ length: 384 }, (_, i) => ((h + i * 7919) % 1000) / 1000);
},
embedTexts: async (texts: string[]) => texts.map(() => Array(384).fill(0.1)),
embedderReady: async () => true,
EmbedderError: class extends Error {},
}));
const { db, pg } = await import("@/lib/db/client");
const { memories, projects, users, groups, userGroups, projectShares } = await import(
"@/lib/db/schema"
);
const { createMemory, updateMemory, patchMemory, softDeleteMemory } = await import(
"@/lib/memory-mutations"
);
const { and, eq } = await import("drizzle-orm");
type Actor = import("@/lib/memory-mutations").Actor;
type ProjectResolver = import("@/lib/memory-mutations").ProjectResolver;
const ISS = "http://test-mutations";
let actor: Actor;
let otherUserId: string;
let sharedProjectId: string;
let sharedGroupId: string;
/** Mirrors the MCP surface: unknown project keys are refused. */
const refusingResolver: ProjectResolver = async (key) => ({
ok: false,
error: `unknown project '${key}'; call project.identify first`,
});
/** Mirrors the Web UI surface: unknown project keys are created. */
function creatingResolver(userId: string): ProjectResolver {
return async (key) => {
const existing = await db
.select({ id: projects.id })
.from(projects)
.where(and(eq(projects.key, key), eq(projects.userId, userId)))
.limit(1);
if (existing[0]) return { ok: true, value: existing[0].id };
const created = await db
.insert(projects)
.values({ userId, key, displayName: key })
.returning({ id: projects.id });
return { ok: true, value: created[0]!.id };
};
}
async function seedMemory(userId: string, projectId: string | null): Promise<string> {
const r = await db
.insert(memories)
.values({
userId,
projectId,
scope: projectId ? "project" : "user",
content: "line one\nline two\n",
tags: [],
embedding: Array(384).fill(0.5),
lastEditedBy: userId,
})
.returning({ id: memories.id });
return r[0]!.id;
}
async function setShareAccess(access: "ro" | "rw") {
await db
.insert(projectShares)
.values({ projectId: sharedProjectId, groupId: sharedGroupId, access })
.onConflictDoUpdate({
target: [projectShares.projectId, projectShares.groupId],
set: { access },
});
}
beforeAll(async () => {
const me = await db
.insert(users)
.values({ oidcSub: "mut-me", oidcIss: ISS })
.onConflictDoNothing()
.returning({ id: users.id });
const myId =
me[0]?.id ??
(
await db.select({ id: users.id }).from(users).where(eq(users.oidcSub, "mut-me"))
)[0]!.id;
const other = await db
.insert(users)
.values({ oidcSub: "mut-other", oidcIss: ISS })
.onConflictDoNothing()
.returning({ id: users.id });
otherUserId =
other[0]?.id ??
(
await db.select({ id: users.id }).from(users).where(eq(users.oidcSub, "mut-other"))
)[0]!.id;
const p = await db
.insert(projects)
.values({ userId: otherUserId, key: "mut-shared", displayName: "Shared" })
.onConflictDoNothing()
.returning({ id: projects.id });
sharedProjectId =
p[0]?.id ??
(
await db
.select({ id: projects.id })
.from(projects)
.where(eq(projects.key, "mut-shared"))
)[0]!.id;
const g = await db
.insert(groups)
.values({ oidcIss: ISS, name: "mut-team" })
.onConflictDoNothing()
.returning({ id: groups.id });
sharedGroupId =
g[0]?.id ??
(
await db.select({ id: groups.id }).from(groups).where(eq(groups.name, "mut-team"))
)[0]!.id;
await db
.insert(userGroups)
.values({ userId: myId, groupId: sharedGroupId })
.onConflictDoNothing();
actor = { userId: myId, groups: ["mut-team"], via: "mcp" };
});
beforeEach(async () => {
await db.delete(memories);
await setShareAccess("rw");
});
afterAll(async () => {
await db.delete(memories);
await pg.end();
});
describe("the ProjectResolver seam", () => {
test("a refusing resolver rejects an unknown project without creating one", async () => {
const res = await createMemory(
actor,
{ content: "x", scope: "project", project: "brand-new-key", tags: [] },
refusingResolver,
);
expect(res.ok).toBe(false);
const rows = await db
.select({ id: projects.id })
.from(projects)
.where(eq(projects.key, "brand-new-key"));
expect(rows).toHaveLength(0);
});
test("a creating resolver makes the project and writes into it", async () => {
const res = await createMemory(
actor,
{ content: "x", scope: "project", project: "made-on-demand", tags: [] },
creatingResolver(actor.userId),
);
expect(res.ok).toBe(true);
const rows = await db
.select({ id: projects.id })
.from(projects)
.where(eq(projects.key, "made-on-demand"));
expect(rows).toHaveLength(1);
});
});
describe("authorization is uniform across mutations", () => {
// Each of these seeds a memory the actor AUTHORED, then downgrades the
// share to read-only. Authoring must not survive as a write privilege.
test("update is denied on a read-only share", async () => {
const id = await seedMemory(actor.userId, sharedProjectId);
await setShareAccess("ro");
const res = await updateMemory(actor, { id, content: "edited" }, refusingResolver);
expect(res.ok).toBe(false);
});
test("patch is denied on a read-only share", async () => {
const id = await seedMemory(actor.userId, sharedProjectId);
await setShareAccess("ro");
const res = await patchMemory(actor, {
id,
old_string: "line one",
new_string: "line uno",
});
expect(res.ok).toBe(false);
});
test("delete is denied on a read-only share", async () => {
const id = await seedMemory(actor.userId, sharedProjectId);
await setShareAccess("ro");
const res = await softDeleteMemory(actor, { id });
expect(res.ok).toBe(false);
});
test("all three are allowed again once the share is read-write", async () => {
const id = await seedMemory(actor.userId, sharedProjectId);
expect((await updateMemory(actor, { id, content: "a\nb\n" }, refusingResolver)).ok).toBe(
true,
);
expect((await patchMemory(actor, { id, old_string: "a", new_string: "c" })).ok).toBe(
true,
);
expect((await softDeleteMemory(actor, { id })).ok).toBe(true);
});
test("another user's user-scope memory is invisible to all three", async () => {
const id = await seedMemory(otherUserId, null);
expect((await updateMemory(actor, { id, content: "x" }, refusingResolver)).ok).toBe(
false,
);
expect(
(await patchMemory(actor, { id, old_string: "line one", new_string: "y" })).ok,
).toBe(false);
expect((await softDeleteMemory(actor, { id })).ok).toBe(false);
});
});
describe("audit trail records the originating surface", () => {
test("via: 'web' and via: 'mcp' are both preserved", async () => {
const webRes = await createMemory(
{ ...actor, via: "web" },
{ content: "from the web", scope: "user", tags: [] },
refusingResolver,
);
expect(webRes.ok).toBe(true);
const rows = await pg<{ actor: string }[]>`
SELECT actor FROM audit_log WHERE action = 'memory.write' ORDER BY created_at DESC LIMIT 1
`;
expect(rows[0]!.actor).toBe("web");
});
});
+357
View File
@@ -0,0 +1,357 @@
import { and, eq, isNull } from "drizzle-orm";
import { db } from "@/lib/db/client";
import { memories, projects, auditLog } from "@/lib/db/schema";
import { embedText } from "@/lib/embedder";
import { applyPatch } from "@/lib/memory-patch";
import { CONCURRENT_EDIT_ERROR, canWriteProject } from "@/lib/access";
import type {
MemoryDeleteInput,
MemoryPatchInput,
MemoryUpdateInput,
MemoryWriteInput,
} from "@shared-memory/schemas";
/**
* The single write path for memories.
*
* Both surfaces — the MCP tools and the Web UI Server Actions — used to
* reimplement authorize → mutate → re-embed → CAS → audit independently.
* They drifted: `memory.delete` over MCP skipped the project ACL whenever
* the caller happened to author the row, which `memory.update` and the
* whole Web UI did not. Consolidating here is what keeps those rules in
* one place, so a change to the sharing model can't be half-applied.
*
* Callers keep their own presentation concerns: MCP maps Outcome to a
* ToolResult, the Web UI throws and then revalidates/redirects.
*/
export interface Actor {
userId: string;
/** Group names, for project-share authorization. */
groups: string[];
/** Recorded as audit_log.actor so the two surfaces stay distinguishable. */
via: "web" | "mcp";
}
export type Outcome<T> = { ok: true; value: T } | { ok: false; error: string };
const fail = (error: string): Outcome<never> => ({ ok: false, error });
const succeed = <T>(value: T): Outcome<T> => ({ ok: true, value });
/**
* Resolves a project key to an id for a write. Injected because this is
* the one place the two surfaces genuinely, deliberately differ: MCP
* refuses unknown projects (the caller is expected to run project.identify
* first), while the Web UI creates one owned by the user. Everything else
* about a write is identical.
*/
export type ProjectResolver = (key: string) => Promise<Outcome<string>>;
interface WriteTarget {
scope: "project" | "user";
projectId: string | null;
userId: string;
}
/**
* The authorization rule for every mutating operation:
* - user-scope → only the owner may write (anything else reads as 404)
* - project-scope → owner of the project, or a group with `rw`
*
* Authoring a row grants nothing on its own. A memory you wrote while a
* share was `rw` becomes read-only to you when an owner downgrades that
* share to `ro` — the project ACL is the authority, not the byline.
*/
async function authorizeWrite(actor: Actor, row: WriteTarget): Promise<Outcome<null>> {
if (row.scope === "user") {
return row.userId === actor.userId ? succeed(null) : fail("not found");
}
if (row.projectId) {
const allowed = await canWriteProject(actor.userId, actor.groups, row.projectId);
if (!allowed) return fail("no write access to this project");
}
return succeed(null);
}
export async function createMemory(
actor: Actor,
input: MemoryWriteInput,
resolveProject: ProjectResolver,
): Promise<Outcome<{ id: string; createdAt: Date }>> {
let projectId: string | null = null;
const projectKey = input.scope === "project" ? input.project : undefined;
if (input.scope === "project") {
if (!projectKey) return fail("scope=project requires `project`");
const resolved = await resolveProject(projectKey);
if (!resolved.ok) return resolved;
projectId = resolved.value;
const allowed = await canWriteProject(actor.userId, actor.groups, projectId);
if (!allowed) return fail(`no write access to project '${projectKey}'`);
}
// Embed inline so the new memory is searchable immediately. Slower
// writes (~50150 ms) are an acceptable price for that guarantee.
const embedding = await embedText(input.content);
const inserted = await db
.insert(memories)
.values({
userId: actor.userId,
projectId,
scope: input.scope,
content: input.content,
tags: input.tags ?? [],
embedding,
lastEditedBy: actor.userId,
})
.returning({ id: memories.id, createdAt: memories.createdAt });
const row = inserted[0]!;
await db.insert(auditLog).values({
userId: actor.userId,
actor: actor.via,
action: "memory.write",
entityType: "memory",
entityId: row.id,
payload: {
scope: input.scope,
projectKey: projectKey ?? null,
tags: input.tags ?? [],
},
});
return succeed(row);
}
export interface MutatedMemory {
id: string;
updatedAt: Date;
version: number;
}
export async function updateMemory(
actor: Actor,
input: MemoryUpdateInput,
resolveProject: ProjectResolver,
): Promise<Outcome<MutatedMemory>> {
const existingRows = await db
.select({
id: memories.id,
content: memories.content,
scope: memories.scope,
projectId: memories.projectId,
projectKey: projects.key,
version: memories.version,
userId: memories.userId,
})
.from(memories)
.leftJoin(projects, eq(memories.projectId, projects.id))
.where(and(eq(memories.id, input.id), isNull(memories.deletedAt)))
.limit(1);
const existing = existingRows[0];
if (!existing) return fail("not found");
const authorized = await authorizeWrite(actor, existing);
if (!authorized.ok) return authorized;
const update: Record<string, unknown> = {
updatedAt: new Date(),
lastEditedBy: actor.userId,
version: existing.version + 1,
};
if (input.tags !== undefined) update.tags = input.tags;
if (input.content !== undefined && input.content !== existing.content) {
update.content = input.content;
update.embedding = await embedText(input.content);
}
let scopeChanged = false;
let projectChanged = false;
let newProjectKey: string | null = existing.projectKey ?? null;
if (input.scope !== undefined) {
if (input.scope === "user") {
if (existing.scope !== "user") {
update.scope = "user";
scopeChanged = true;
}
if (existing.projectId !== null) {
update.projectId = null;
projectChanged = true;
newProjectKey = null;
}
} else {
// scope === 'project' — the schema refine guarantees `project` is set.
// Moving INTO a project requires write access there.
const projectKey = input.project!;
const resolved = await resolveProject(projectKey);
if (!resolved.ok) return resolved;
const targetId = resolved.value;
const allowed = await canWriteProject(actor.userId, actor.groups, targetId);
if (!allowed) return fail(`no write access to project '${projectKey}'`);
if (existing.scope !== "project") {
update.scope = "project";
scopeChanged = true;
}
if (existing.projectId !== targetId) {
update.projectId = targetId;
projectChanged = true;
newProjectKey = projectKey;
}
}
}
const updated = await casUpdate(input.id, update, input.version ?? existing.version);
if (!updated) return fail(CONCURRENT_EDIT_ERROR);
const auditFields = Object.keys(update).filter(
(k) => k !== "updatedAt" && k !== "version" && k !== "lastEditedBy",
);
const auditPayload: Record<string, unknown> = { fields: auditFields };
if (scopeChanged || projectChanged) {
auditPayload.scope = { from: existing.scope, to: update.scope ?? existing.scope };
auditPayload.projectKey = { from: existing.projectKey ?? null, to: newProjectKey };
}
await db.insert(auditLog).values({
userId: actor.userId,
actor: actor.via,
action: "memory.update",
entityType: "memory",
entityId: updated.id,
payload: auditPayload,
});
return succeed(updated);
}
export async function patchMemory(
actor: Actor,
input: MemoryPatchInput,
): Promise<Outcome<MutatedMemory & { contentLength: number; delta: number }>> {
const existingRows = await db
.select({
content: memories.content,
scope: memories.scope,
projectId: memories.projectId,
version: memories.version,
userId: memories.userId,
})
.from(memories)
.where(and(eq(memories.id, input.id), isNull(memories.deletedAt)))
.limit(1);
const existing = existingRows[0];
if (!existing) return fail("not found");
const authorized = await authorizeWrite(actor, existing);
if (!authorized.ok) return authorized;
const patch = applyPatch(existing.content, input.old_string, input.new_string);
if (!patch.ok) return fail(patch.error);
const updated = await casUpdate(
input.id,
{
content: patch.content,
embedding: await embedText(patch.content),
updatedAt: new Date(),
lastEditedBy: actor.userId,
version: existing.version + 1,
},
input.version ?? existing.version,
);
if (!updated) return fail(CONCURRENT_EDIT_ERROR);
await db.insert(auditLog).values({
userId: actor.userId,
actor: actor.via,
action: "memory.patch",
entityType: "memory",
entityId: updated.id,
payload: {
fields: ["content"],
patch: {
offset: existing.content.indexOf(input.old_string),
removed: input.old_string.length,
added: input.new_string.length,
},
},
});
return succeed({
...updated,
contentLength: patch.content.length,
delta: patch.content.length - existing.content.length,
});
}
export async function softDeleteMemory(
actor: Actor,
input: MemoryDeleteInput,
): Promise<Outcome<{ id: string }>> {
const rows = await db
.select({
id: memories.id,
userId: memories.userId,
projectId: memories.projectId,
scope: memories.scope,
version: memories.version,
})
.from(memories)
.where(and(eq(memories.id, input.id), isNull(memories.deletedAt)))
.limit(1);
const existing = rows[0];
if (!existing) return fail("not found");
const authorized = await authorizeWrite(actor, existing);
if (!authorized.ok) return authorized;
const updated = await db
.update(memories)
.set({ deletedAt: new Date(), lastEditedBy: actor.userId })
.where(
and(
eq(memories.id, input.id),
eq(memories.version, input.version ?? existing.version),
isNull(memories.deletedAt),
),
)
.returning({ id: memories.id });
if (!updated[0]) return fail(CONCURRENT_EDIT_ERROR);
await db.insert(auditLog).values({
userId: actor.userId,
actor: actor.via,
action: "memory.delete",
entityType: "memory",
entityId: updated[0].id,
});
return succeed(updated[0]);
}
/**
* Compare-and-set on `version`. A zero-row result means a peer edited the
* row between our read and this write. Callers that omit an explicit
* version pass the one they just read, which still closes the read-
* modify-write window inside a single handler.
*/
async function casUpdate(
id: string,
update: Record<string, unknown>,
expectedVersion: number,
): Promise<MutatedMemory | null> {
const rows = await db
.update(memories)
.set(update)
.where(and(eq(memories.id, id), eq(memories.version, expectedVersion)))
.returning({
id: memories.id,
updatedAt: memories.updatedAt,
version: memories.version,
});
return rows[0] ?? null;
}
+77
View File
@@ -0,0 +1,77 @@
import { describe, expect, test } from "vitest";
import { applyPatch } from "@/lib/memory-patch";
describe("applyPatch", () => {
test("replaces an old_string that occurs exactly once", () => {
const result = applyPatch("alpha beta gamma", "beta", "BETA");
expect(result.ok).toBe(true);
if (result.ok) expect(result.content).toBe("alpha BETA gamma");
});
test("refuses when old_string is absent, rather than silently doing nothing", () => {
const result = applyPatch("alpha beta gamma", "delta", "DELTA");
expect(result.ok).toBe(false);
if (!result.ok) expect(result.error).toMatch(/not found/i);
});
test("refuses when old_string is ambiguous, and reports the match count", () => {
const result = applyPatch("x marks the spot, x marks it twice", "x", "y");
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error).toMatch(/2/);
expect(result.error).toMatch(/match/i);
}
});
test("treats old_string literally, not as a regular expression", () => {
// A naive RegExp implementation would match "axb" here.
const result = applyPatch("axb and a.b", "a.b", "REPLACED");
expect(result.ok).toBe(true);
if (result.ok) expect(result.content).toBe("axb and REPLACED");
});
test("replaces a multi-line old_string, preserving surrounding text", () => {
const content = "## HEADING\n- one\n- two\n\n## OTHER\n";
const result = applyPatch(content, "## HEADING\n- one", "## HEADING\n- zero\n- one");
expect(result.ok).toBe(true);
if (result.ok) {
expect(result.content).toBe("## HEADING\n- zero\n- one\n- two\n\n## OTHER\n");
}
});
test("rejects a patch whose result would exceed the 64,000-char content limit", () => {
// The anchor must be unique, or the ambiguity check fires first and
// this stops testing the length limit at all.
const content = "A".repeat(63_950) + "ANCHOR";
const result = applyPatch(content, "ANCHOR", "B".repeat(100));
expect(result.ok).toBe(false);
if (!result.ok) expect(result.error).toMatch(/64,?000|limit/i);
});
test("rejects a no-op patch where new_string equals old_string", () => {
const result = applyPatch("alpha beta", "beta", "beta");
expect(result.ok).toBe(false);
if (!result.ok) expect(result.error).toMatch(/identical|no-op|unchanged/i);
});
test("allows a patch that deletes text by replacing with an empty string", () => {
const result = applyPatch("keep this, drop this", ", drop this", "");
expect(result.ok).toBe(true);
if (result.ok) expect(result.content).toBe("keep this");
});
test("rejects a patch that would empty the memory entirely", () => {
const result = applyPatch("all of it", "all of it", "");
expect(result.ok).toBe(false);
if (!result.ok) expect(result.error).toMatch(/empty/i);
});
});
+76
View File
@@ -0,0 +1,76 @@
import { MEMORY_CONTENT_MAX } from "@shared-memory/schemas";
/**
* Pure string-level semantics for `memory.patch`.
*
* Kept free of any DB or auth dependency so both the MCP tool handler and
* the Web UI can share it, and so the refuse-rather-than-clobber rules
* below are directly testable.
*
* The contract mirrors the file-editing primitive coding agents already
* use: an `old_string` that is absent or ambiguous is an ERROR, never a
* silent no-op and never an arbitrary pick. That refusal is the property
* that makes the operation safe to hand to an agent editing a shared
* document it cannot afford to corrupt.
*/
export type PatchOutcome =
| { ok: true; content: string }
| { ok: false; error: string };
function countOccurrences(haystack: string, needle: string): number {
let count = 0;
let from = 0;
for (;;) {
const at = haystack.indexOf(needle, from);
if (at === -1) return count;
count += 1;
// Advance past this match so overlapping matches aren't double-counted.
from = at + needle.length;
}
}
export function applyPatch(
content: string,
oldString: string,
newString: string,
): PatchOutcome {
if (oldString === newString) {
return {
ok: false,
error: "old_string and new_string are identical; the patch would change nothing",
};
}
const first = content.indexOf(oldString);
if (first === -1) {
return {
ok: false,
error:
"old_string not found in the memory content; nothing was changed. Fetch the memory with memory.get and copy the exact text you mean to replace.",
};
}
// Only pay for a full count once we know there's more than one match.
if (content.indexOf(oldString, first + oldString.length) !== -1) {
const count = countOccurrences(content, oldString);
return {
ok: false,
error: `old_string matches ${count} times; it must match exactly once. Nothing was changed — include more surrounding context to identify the one you mean.`,
};
}
const patched =
content.slice(0, first) + newString + content.slice(first + oldString.length);
if (patched.length === 0) {
return { ok: false, error: "the patch would leave the memory empty" };
}
if (patched.length > MEMORY_CONTENT_MAX) {
return {
ok: false,
error: `the patched content would be ${patched.length.toLocaleString("en-US")} characters, over the ${MEMORY_CONTENT_MAX.toLocaleString("en-US")}-character limit`,
};
}
return { ok: true, content: patched };
}
+7 -3
View File
@@ -7,7 +7,9 @@
"dev": "next dev --port 3000",
"build": "next build",
"start": "next start --port 3000",
"lint": "next lint",
"lint": "eslint .",
"test": "vitest run",
"test:watch": "vitest",
"typecheck": "tsc --noEmit",
"db:generate": "drizzle-kit generate",
"db:migrate": "tsx ./scripts/migrate.ts",
@@ -26,16 +28,18 @@
"zod": "^3.23.8"
},
"devDependencies": {
"@eslint/eslintrc": "^3.3.6",
"@tailwindcss/postcss": "^4.0.0",
"@types/node": "^22.10.2",
"@types/react": "^19.0.2",
"@types/react-dom": "^19.0.2",
"@tailwindcss/postcss": "^4.0.0",
"drizzle-kit": "^0.30.1",
"esbuild": "^0.24.2",
"eslint": "^9.17.0",
"eslint-config-next": "^15.1.0",
"tailwindcss": "^4.0.0",
"tsx": "^4.19.2",
"typescript": "^5.7.2"
"typescript": "^5.7.2",
"vitest": "^2"
}
}
+3 -1
View File
@@ -1,5 +1,7 @@
export default {
const config = {
plugins: {
"@tailwindcss/postcss": {},
},
};
export default config;
+18
View File
@@ -0,0 +1,18 @@
import { defineConfig } from "vitest/config";
import { fileURLToPath } from "node:url";
export default defineConfig({
resolve: {
alias: {
"@": fileURLToPath(new URL("./", import.meta.url)),
},
},
test: {
environment: "node",
include: ["lib/**/*.test.ts", "app/**/*.test.ts"],
setupFiles: ["./vitest.setup.ts"],
// Integration tests share one Postgres database; running files in
// parallel would let them clobber each other's rows.
fileParallelism: false,
},
});
+25
View File
@@ -0,0 +1,25 @@
/**
* Test environment. `lib/env.ts` validates a full production config at
* import time, so integration tests that touch the DB need these set
* before any module under test is loaded.
*
* Only DATABASE_URL points at anything real a throwaway Postgres with
* pgvector. The OIDC/secret values exist purely to satisfy validation;
* tests construct a UserContext directly rather than going through auth.
*/
// NODE_ENV is set to "test" by vitest itself.
//
// DATABASE_URL points at a throwaway pgvector instance. The default assumes
// the published port is reachable on localhost; when the test runner is
// itself inside a container, export DATABASE_URL with the database
// container's address instead. See README → Running the tests.
process.env.DATABASE_URL ??= "postgres://test:test@127.0.0.1:55432/shared_memory_test";
process.env.PUBLIC_URL ??= "http://localhost:3000";
process.env.OIDC_ISSUER ??= "http://localhost:9000/application/o/test/";
process.env.OIDC_CLIENT_ID_WEB ??= "test-web";
process.env.OIDC_CLIENT_SECRET_WEB ??= "test-web-secret";
process.env.OIDC_CLIENT_ID_MCP ??= "test-mcp";
process.env.OIDC_AUDIENCE ??= "test-audience";
process.env.EMBEDDER_URL ??= "http://localhost:8080";
process.env.NEXTAUTH_SECRET ??= "test-nextauth-secret-at-least-32-chars-long";
process.env.CLI_TOKEN_SECRET ??= "test-cli-token-secret-at-least-32-chars-long";
+262
View File
@@ -0,0 +1,262 @@
# Decision record: memory read payloads and patch-style updates
**Status:** P1 shipped · P2 shipped · P3 declined
**Originated:** 2026-08-11, from live use of the deployed server
**Revised:** 2026-08-11, against source
**Closed:** 2026-08-11 — implemented in `feat/memory-patch-and-lean-get` (PR #19)
This started as a proposal written from black-box observation of the deployed
server. It is kept as a decision record because the reasoning behind what was
built — and behind what was deliberately *not* built — is not recoverable from
the diff.
---
## 0. Orientation
Tools are defined internally with dots (`memory.get`) and surfaced to clients
with underscores (`memory_get`). Don't let it confuse a grep.
### Data model
- Memories have `id` (uuid), `content`, `tags[]`, `scope` (`project` | `user`),
`projectId`, `visibility`, `version`, `lastEditedBy`, `createdAt`,
`updatedAt`, `deletedAt`, plus `userId`, `embedding`, and `contentTsv`
(`apps/web/lib/db/schema.ts`).
- `scope: project` requires `project`; `scope: user` requires it omitted.
- `version` is an optimistic-locking token, bumped on every successful update.
A stale `version` returns a concurrent-edit error.
- Content limit is 64,000 chars (`packages/schemas/src/index.ts`).
- Shared projects allow anyone with rw access to edit any memory — hence the
locking.
---
## 1. P1 — `memory_get` returned the embedding and the tsvector ✅ SHIPPED
**Problem.** `memory_get` used a bare `select()` and returned the raw DB row,
including the `embedding` vector and the `contentTsv` lexeme index. Both are
Postgres retrieval internals with zero value to a model consumer.
`memory_list` and `memory_search` already projected an explicit 9-field shape —
`memory_get` 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** | |
Two distinct failure shapes, which the original draft had flattened together:
- `embedding` is a **fixed** ~4,690-char tax on every read — 384 dims
regardless of content length. Nearly invisible on large memories (7.5%),
dominant on small ones (~58% of an ~8k response). Most memories are small.
- `contentTsv` scales **super-linearly** with content (frequent lexemes
accumulate long position lists, ~21 chars/entry) and was the largest single
component of the large payload — larger than the content itself.
**This was a correctness problem, not an efficiency nit.** Fetching that memory
exceeded the MCP tool-output cap and spilled to a file, even though `content`
alone is comfortably under the limit. `memory_get` was unusable on exactly the
large living documents P2 exists to serve. At the 64,000-char content ceiling a
response would land near 140,000 characters, under half of it content.
**Shipped:** `memory_get` returns the same 9 fields as `memory_list` /
`memory_search`. `userId` is still selected for the authorization check and
stripped before responding.
**Rejected: an `include: ("embedding" | "tsv")[]` opt-in.** The original draft
proposed gating the fields behind a flag in case some caller needed them. No
caller can: no MCP client can consume a 384-float vector or a lexeme index, and
the Web UI never goes through the MCP tools (it reads via `lib/memories.ts` and
writes via `lib/memory-actions.ts`). The parameter would have been dead on
arrival.
---
## 2. P2 — patch-style updates ✅ SHIPPED
**Problem.** `memory_update` accepted only full replacement. Adding four lines
to a 13,000-char living document meant reproducing the entire document.
**The evidence this was a real blocker.** The WHP roadmap mirror was three
weeks and two shipped releases out of date. The agent that noticed **declined
to fix it**, on the grounds that hand-reproducing 13k characters of shared team
history to add one entry risked silently dropping some of it — a worse outcome
than leaving it stale.
That is the failure mode this was designed against: **when the only safe way to
make a small edit is expensive, the edit doesn't happen.**
**Shipped:** `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 the property that makes
the operation safe to hand to an agent editing a shared document it cannot
afford to corrupt. Semantics live in `lib/memory-patch.ts` as a pure function,
free of DB and auth, so both surfaces share them.
**Locking came nearly free.** `memory.update` already computed
`expectedVersion = version ?? existing.version`, falling back to the version
read in the same handler. The CAS therefore already guarded the server-side
read-modify-write; patch copies that shape and is race-safe even when the
caller omits `version`. No explicit transaction was required.
**Rejected: `memory_append` with a `section` parameter.** Proposed as sugar for
heading-structured logs. Patch already covers that case exactly —
`memory_patch(id, "## RECENTLY SHIPPED", "## RECENTLY SHIPPED\n- entry")` — and
does so *with* the uniqueness guarantee: if the heading appears twice you get an
error instead of an arbitrary insert. Implementing `section` would have meant
defining heading-match semantics, insert position within a section, and
duplicate-heading behaviour, for something patch handles for free.
---
## 3. P3 — deriving mirrors rather than relying on convention ❌ DECLINED
**The problem as stated.** Some memories mirror local files. The sync is
enforced only by a note in the file's own header: *"MIRRORED to shared-memory
MCP … when you update this file, also `memory_update` that record."* That
depends on whoever edits the file noticing the note and performing a second
write. It went three weeks without one.
Proposed shapes were: a server-side `memory_sync_from_file`, a staleness signal
via `sourcePath` + content hash, or leaving it manual but cheap via P2.
**Declined, 2026-08-11.** Three reasons, in order of weight:
1. **The cause we have evidence for is now fixed.** The evidence was specific:
an agent *noticed* the drift and *declined* to fix it because the edit was
expensive and risky. That is a cost failure, not an attention failure. P2
makes that edit a single call. We have direct evidence for the cost cause
and none yet for any other.
2. **It would likely be a mechanism for N=1.** One mirror is known to exist.
A `sourcePath` column, hash computation, and staleness plumbing is real
schema-and-sync work; building it to police a single document is
disproportionate.
3. **If drift recurs, the better fix probably isn't sync machinery.** P3
assumes the mirror should exist and be kept honest. That assumption deserves
scrutiny first: the memory is a condensed prose rendition of a file that
lives in a container, existing separately only because the memory is
cross-machine and the file is not. Two hand-maintained sources of truth plus
a drift detector is strictly more machinery than one source of truth. The
cheaper answer would be to remove the duplication — make the memory
canonical and drop the file, or generate one from the other.
**What would reopen this:** the roadmap going stale *again* now that patching
is cheap. That is the clean experiment and it costs nothing to run. If it
drifts again, the cause was attention rather than cost, and option 2 above — a
staleness signal that makes drift *visible* rather than trying to fix it
automatically — becomes worth its weight.
Note for whoever picks this up: the mirror is **not** a byte copy of its local
file. It is a condensed prose rendition with different headings and no
wiki-links. A naive file-sync would destroy its established form. Any solution
has to preserve that distinction or deliberately abandon it.
---
## 4. Structural work this depended on ✅ SHIPPED
Neither of these was in the original proposal; both were found once the source
was available.
### 4.1 The write path was duplicated
`updateMemoryAction` (Web UI) and the `memory.update` MCP handler each
reimplemented authorize → mutate → re-embed → CAS → audit. Neither delegated to
a shared helper, and they had **drifted**: `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 entire Web UI always checked.
Both surfaces now route through `lib/memory-mutations.ts`. Authoring a row
grants no standing write privilege on any path — the project ACL is the
authority, not the byline. This was a behaviour change, shipped deliberately,
and is covered by a test that fails against the old code.
The one genuine difference between the surfaces is injected as a
`ProjectResolver`: MCP refuses an unknown project key (`call project.identify
first`) 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.
### 4.2 There was no test infrastructure
No vitest, no jest, no test files, no `test` script. Added vitest, with
integration tests running against a real Postgres rather than a mocked DB.
Setup is documented in the README.
---
## 5. Invariants — preserve these
- **Optimistic locking.** `version` must keep working, and every new mutating
primitive must accept it. Shared projects have concurrent editors.
- **Scope/project rules.** `scope: project``project` required and must
already exist on the MCP path; `scope: user``project` omitted.
- **Re-embedding on content change — and note what this does NOT cover.**
`content_tsv` is `GENERATED ALWAYS AS (to_tsvector('english',
coalesce(content, ''))) STORED` (`apps/web/drizzle/0000_init.sql`), so
Postgres maintains it and **full-text search cannot rot**. Only `embedding`
requires an explicit recompute. Any new mutation path must re-embed, or
*semantic* retrieval degrades silently while FTS keeps working — which is
exactly what makes the failure hard to notice.
- **Stable `id` across edits.** Never implement an edit as delete + recreate.
- **64,000-char content limit** enforced after a patch is applied, not just on
the incoming fragment.
- **Audit trail.** Partial edits record match offset and length delta, not just
the changed field names.
---
## 6. Acceptance checks
**P1** — verified in tests; the payload figures need a deploy to confirm.
- ✅ Response contains neither `embedding` nor `contentTsv`.
- ✅ Field set matches `memory_list` / `memory_search` exactly (9, down from 14).
- ⏳ Large specimen drops 62,695 → ~28,100 chars (55%); small specimen ~75%.
- ⏳ `memory_get` on `aaea192c-edce-4372-b011-5113a02dea16` returns inline
instead of spilling to a file. **This is the check that matters** — it is the
difference between the tool working and not working on large memories.
**P2** — all verified against a real Postgres.
- ✅ Unique `old_string` → applied; `version` incremented by exactly 1.
- ✅ Absent `old_string` → error; content byte-identical afterward.
- ✅ `old_string` occurring twice → error naming the count; content unchanged.
- ✅ Stale `version` → concurrent-edit error; content unchanged.
- ✅ Content exceeding 64,000 chars post-patch → rejected.
- ✅ Re-embedding — see the caveat below.
**A trap in the re-embedding check.** The original draft proposed "after a
patch, `memory_search` finds text introduced by that patch." *That test does not
work.* Search fuses three rankers via RRF, and because `content_tsv` is a
generated column the FTS ranker finds the literal inserted text **even if the
patch skipped re-embedding entirely** — it would pass on a broken
implementation. The shipped test asserts the **stored vector changed**, using a
deterministic per-text embedder stub. A separate test pins that `content_tsv`
updates itself, documenting why the naive check is misleading.
---
## 7. Test specimen
Memory `aaea192c-edce-4372-b011-5113a02dea16` (project
`cloud-hosting-platform/whp`, tags `roadmap` / `progress-tracker` / `planning`)
is a good real-world subject: ~13k chars, heading-structured, and its local
counterpart is
`/home/claude/.claude/projects/-workspace/memory/project_roadmap.md`.
**It is live team data.** Check its `updatedAt` and `version` before using it as
a fixture, and prefer a scratch memory for destructive tests.
+1
View File
@@ -12,6 +12,7 @@
"build": "pnpm -r build",
"dev": "pnpm --filter @shared-memory/web dev",
"lint": "pnpm -r lint",
"test": "pnpm -r test",
"typecheck": "pnpm -r typecheck",
"db:generate": "pnpm --filter @shared-memory/web db:generate",
"db:migrate": "pnpm --filter @shared-memory/web db:migrate"
+15 -1
View File
@@ -18,7 +18,8 @@ export const ProjectKey = z
.regex(/^[a-zA-Z0-9._\-/]+$/, "project key may only contain alphanumerics, ._-/");
export type ProjectKey = z.infer<typeof ProjectKey>;
export const MemoryContent = z.string().min(1).max(64_000);
export const MEMORY_CONTENT_MAX = 64_000;
export const MemoryContent = z.string().min(1).max(MEMORY_CONTENT_MAX);
export const Tags = z
.array(z.string().min(1).max(64).regex(/^[a-zA-Z0-9._\-]+$/, "tag must be alphanumeric ._-"))
@@ -84,6 +85,19 @@ export const MemoryUpdateInput = z.object({
});
export type MemoryUpdateInput = z.infer<typeof MemoryUpdateInput>;
// memory.patch replaces ONE exact occurrence of `old_string`. Absent or
// ambiguous matches are errors, never silent no-ops — see applyPatch.
// `new_string` may be empty (a deletion); the resulting content still has
// to satisfy MemoryContent, which is checked after the patch is applied.
export const MemoryPatchInput = z.object({
id: z.string().uuid(),
old_string: z.string().min(1).max(MEMORY_CONTENT_MAX),
new_string: z.string().max(MEMORY_CONTENT_MAX),
// Same optimistic-locking token as memory.update.
version: z.number().int().nonnegative().optional(),
});
export type MemoryPatchInput = z.infer<typeof MemoryPatchInput>;
export const MemorySearchInput = z.object({
query: z.string().min(1).max(2000),
project: ProjectKey.optional(),
+814 -8
View File
File diff suppressed because it is too large Load Diff