From 5b2bf7d19dc118ddd399913f6b9289b49abc0a7c Mon Sep 17 00:00:00 2001
From: jknapp <jknapp85@gmail.com>
Date: Sun, 17 May 2026 06:53:32 -0700
Subject: [PATCH] fix: address code review on merged feature set
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Five high-confidence findings from the post-merge reviewer pass:

1. memory.update MCP tool description claimed "Project is upserted if it
   doesn't exist", but the handler used resolveProjectId and would error.
   Matched the description to the actual behavior (call project.identify
   first) — keeps parity with memory.write.

2. updateMemoryAction's UPDATE statement was missing the userId guard.
   The preceding scoped SELECT made it not exploitable in practice, but
   it diverged from deleteMemoryAction's pattern. Added the guard for
   defense in depth.

3. putSnippet's UPDATE statement had the same missing userId guard —
   fixed the same way.

4. MemoryUpdateInput's refine for scope='user' accepted both
   project=undefined AND project=""; the snippets refine only accepted
   undefined. Tightened MemoryUpdateInput to require undefined, matching
   the snippets rule. Web actions already coerce "" → undefined before
   parsing, so no caller is affected.

5. 0002_snippets_scope.sql created two indexes unconditionally —
   replaced with CREATE INDEX IF NOT EXISTS so re-runs after a
   drizzle-kit push won't trip.

Also adds .claude/ to .gitignore so worktree directories from
multi-agent builds aren't accidentally committed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .gitignore                               | 1 +
 apps/web/drizzle/0002_snippets_scope.sql | 4 ++--
 apps/web/lib/mcp/tools.ts                | 2 +-
 apps/web/lib/memory-actions.ts           | 5 ++++-
 apps/web/lib/snippets.ts                 | 5 ++++-
 packages/schemas/src/index.ts            | 7 +++----
 6 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/.gitignore b/.gitignore
index 818d5df..d764d4f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -42,3 +42,4 @@ coverage/
 # Local data volumes (if anyone bind-mounts under repo)
 data/
 postgres-data/
+.claude/
diff --git a/apps/web/drizzle/0002_snippets_scope.sql b/apps/web/drizzle/0002_snippets_scope.sql
index 8cf04a1..6762e41 100644
--- a/apps/web/drizzle/0002_snippets_scope.sql
+++ b/apps/web/drizzle/0002_snippets_scope.sql
@@ -35,5 +35,5 @@ CREATE UNIQUE INDEX "snippets_user_project_name_uq"
   ON "snippets" ("user_id", "project_id", "name")
   WHERE scope = 'project' AND deleted_at IS NULL;
 
-CREATE INDEX "snippets_user_idx"    ON "snippets" ("user_id");
-CREATE INDEX "snippets_project_idx" ON "snippets" ("project_id");
+CREATE INDEX IF NOT EXISTS "snippets_user_idx"    ON "snippets" ("user_id");
+CREATE INDEX IF NOT EXISTS "snippets_project_idx" ON "snippets" ("project_id");
diff --git a/apps/web/lib/mcp/tools.ts b/apps/web/lib/mcp/tools.ts
index 0620899..927b3d2 100644
--- a/apps/web/lib/mcp/tools.ts
+++ b/apps/web/lib/mcp/tools.ts
@@ -337,7 +337,7 @@ const memoryUpdate: ToolDef = {
       project: {
         type: "string",
         description:
-          "Project key the memory should attach to (required and only valid when scope='project'). Project is upserted if it doesn't exist.",
+          "Project key the memory should attach to (required and only valid when scope='project'). The project must already exist — call `project.identify` first if it doesn't.",
       },
     },
     required: ["id"],
diff --git a/apps/web/lib/memory-actions.ts b/apps/web/lib/memory-actions.ts
index 6c3ca2e..0f83dc4 100644
--- a/apps/web/lib/memory-actions.ts
+++ b/apps/web/lib/memory-actions.ts
@@ -185,7 +185,10 @@ export async function updateMemoryAction(formData: FormData) {
     }
   }
 
-  await db.update(memories).set(update).where(eq(memories.id, parsed.data.id));
+  await db
+    .update(memories)
+    .set(update)
+    .where(and(eq(memories.id, parsed.data.id), eq(memories.userId, userId)));
 
   const auditFields = Object.keys(update).filter((k) => k !== "updatedAt");
   const auditPayload: Record<string, unknown> = { fields: auditFields };
diff --git a/apps/web/lib/snippets.ts b/apps/web/lib/snippets.ts
index 74126d2..bf31d10 100644
--- a/apps/web/lib/snippets.ts
+++ b/apps/web/lib/snippets.ts
@@ -156,7 +156,10 @@ export async function putSnippet(
       updatedAt: new Date(),
     };
     if (description !== undefined) updateValues.description = description;
-    await db.update(snippets).set(updateValues).where(eq(snippets.id, existing.id));
+    await db
+      .update(snippets)
+      .set(updateValues)
+      .where(and(eq(snippets.id, existing.id), eq(snippets.userId, userId)));
     const refreshed = await findSnippet(userId, name, scope, projectId);
     return { snippet: refreshed!, inserted: false };
   }
diff --git a/packages/schemas/src/index.ts b/packages/schemas/src/index.ts
index 3b15aa7..b980fb6 100644
--- a/packages/schemas/src/index.ts
+++ b/packages/schemas/src/index.ts
@@ -61,10 +61,9 @@ export const MemoryUpdateInput = z.object({
     (v) => v.scope !== "project" || (v.project !== undefined && v.project !== ""),
     { message: "scope='project' requires a non-empty project key" },
   )
-  .refine(
-    (v) => v.scope !== "user" || v.project === undefined || v.project === "",
-    { message: "scope='user' cannot have a project key" },
-  );
+  .refine((v) => v.scope !== "user" || v.project === undefined, {
+    message: "scope='user' cannot have a project key",
+  });
 export type MemoryUpdateInput = z.infer<typeof MemoryUpdateInput>;
 
 export const MemorySearchInput = z.object({