Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6e9628b073 | ||
|
|
c3bbea5134 | ||
|
|
d9306884d3 | ||
|
|
d319f00227 | ||
|
|
fc0cb453d3 |
@@ -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
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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;
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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
@@ -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 (~50–150 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
@@ -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");
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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 (~50–150 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;
|
||||
}
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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,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"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
export default {
|
||||
const config = {
|
||||
plugins: {
|
||||
"@tailwindcss/postcss": {},
|
||||
},
|
||||
};
|
||||
|
||||
export default config;
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
});
|
||||
@@ -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";
|
||||
@@ -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.
|
||||
@@ -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"
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Generated
+814
-8
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user