docs(16): research multi-user data model phase

2026-04-05 10:17:56 +02:00
parent 632e4d3a1a
commit b87551694f
1 changed files with 534 additions and 0 deletions
--- a/.planning/phases/16-multi-user-data-model/16-RESEARCH.md
+++ b/.planning/phases/16-multi-user-data-model/16-RESEARCH.md
@@ -0,0 +1,534 @@
+# Phase 16: Multi-User Data Model - Research
+
+**Researched:** 2026-04-04
+**Domain:** Multi-tenant data isolation with Drizzle ORM on PostgreSQL
+**Confidence:** HIGH
+
+## Summary
+
+Phase 16 adds user ownership to all user-created entities (items, categories, threads, setups, settings, apiKeys) and enforces complete cross-user data isolation. The current codebase has no `users` table (Phase 15 removed the old one) and no `userId` columns on any entity table. Every service function signature is `(db, ...)` and needs to become `(db, userId, ...)`.
+
+The scope is well-defined and mechanical: create a new `users` table (with `logtoSub` for OIDC mapping), add `userId` FK columns to 6 tables, update the `requireAuth` middleware to resolve and set `userId` on context, update all 7 service files to accept and filter by `userId`, update all route handlers to extract `userId` from context, update all MCP tool registrations to pass `userId`, and update all tests.
+
+**Primary recommendation:** Use a multi-step Drizzle migration (add column nullable, backfill existing data to user 1, set NOT NULL + FK constraint) and a systematic service-by-service approach with `and(eq(table.userId, userId), ...)` filtering on every query.
+
+<user_constraints>
+## User Constraints (from CONTEXT.md)
+
+### Locked Decisions
+- **D-01:** Create a thin local `users` table: `id` (serial integer PK), `logtoSub` (text, unique, not null), `createdAt` (timestamp). Auto-created on first OIDC login via upsert.
+- **D-02:** All entity tables reference `users.id` (integer FK) -- not the Logto sub string directly.
+- **D-03:** The `requireAuth` middleware resolves the authenticated identity to a local `users.id` and sets it on the Hono context (e.g., `c.set("userId", userId)`).
+- **D-04:** Add `userId` (integer, NOT NULL, FK -> users.id) column to: `items`, `categories`, `threads`, `setups`, `settings`, `apiKeys`.
+- **D-05:** `categories`: Drop global unique constraint on `name`. Add composite unique constraint on `(userId, name)`. Each user gets their own "Uncategorized" default category.
+- **D-06:** `settings`: Change primary key from `key` alone to composite `(userId, key)`. Each user has their own settings.
+- **D-07:** `apiKeys`: Add `userId` column so middleware can resolve which user's data an API key grants access to.
+- **D-08:** `threadCandidates` and `setupItems`: No userId needed -- they inherit ownership through their parent thread/setup FK.
+- **D-09:** Every service function that reads or writes user-owned data gains a `userId` parameter. All queries include `where(eq(table.userId, userId))`.
+- **D-10:** `requireAuth` middleware sets `userId` on context. Routes extract `userId` from context and pass to services.
+- **D-11:** Migration script adds `userId` column with a temporary default, then updates all existing rows to user ID 1, then removes the default and sets NOT NULL.
+- **D-12:** Create "Uncategorized" category per-user on first login (or lazily when needed).
+- **D-13:** MCP tools resolve userId from the authenticated token (API key -> userId lookup, or Bearer token -> userId). All tool operations are scoped to that user.
+
+### Claude's Discretion
+- Exact migration SQL approach (single migration vs multi-step)
+- Whether to use Drizzle's `.where()` chaining or a helper function for userId scoping
+- Default category creation strategy (eager on first login vs lazy on first item creation)
+- Whether thread resolution should check that the target category belongs to the same user
+- Order of service file changes (all at once vs table-by-table)
+
+### Deferred Ideas (OUT OF SCOPE)
+None -- discussion stayed within phase scope.
+</user_constraints>
+
+<phase_requirements>
+## Phase Requirements
+
+| ID | Description | Research Support |
+|----|-------------|------------------|
+| MULTI-01 | Every item, category, thread, and setup is owned by a specific user | D-04 adds userId FK to all entity tables; D-01 creates users table |
+| MULTI-02 | User can only see and modify their own data (cross-user isolation) | D-09 adds userId filtering to all service queries; D-10 threads userId through routes |
+| MULTI-03 | Categories use composite unique constraint (userId + name) | D-05 replaces global unique(name) with unique(userId, name) |
+| MULTI-04 | Existing data is assigned to the original user during migration | D-11 migration backfills all rows to user ID 1 |
+| MULTI-05 | MCP tools operate within the authenticated user's scope | D-13 resolves userId from API key or Bearer token in MCP auth middleware |
+| MULTI-06 | Settings are per-user rather than global | D-06 changes settings PK from (key) to composite (userId, key) |
+</phase_requirements>
+
+## Project Constraints (from CLAUDE.md)
+
+- **Stack**: React 19 + Hono + Drizzle ORM + PostgreSQL (migrated from SQLite in Phase 14), running on Bun
+- **Services pattern**: Pure business logic functions that take a db instance. No HTTP awareness.
+- **Prices stored as cents** (integer). Timestamps as integers with `{ mode: "timestamp" }`.
+- **Testing**: Bun test runner. `createTestDb()` uses PGlite with Drizzle migrations. Tests at service level and route level.
+- **Auth model**: Public-read, authenticated-write. Cookie sessions for web UI, API keys for programmatic access.
+- **Schema file**: `src/db/schema.ts` -- currently uses `sqliteTable` imports but targets PostgreSQL via Drizzle abstraction.
+- **Migrations**: Generated via `bun run db:generate`, applied via `bun run db:push`. Migration directory: `drizzle-pg/`.
+
+## Standard Stack
+
+### Core
+| Library | Version | Purpose | Why Standard |
+|---------|---------|---------|--------------|
+| drizzle-orm | (current in project) | Schema definition, query building, migrations | Already in use; provides `and()`, `eq()`, composite constraints |
+| drizzle-kit | (current in project) | Migration generation from schema changes | Already in use; `bun run db:generate` |
+| @hono/oidc-auth | (current in project) | OIDC session for browser users (getAuth -> sub claim) | Already in use from Phase 15 |
+| hono | (current in project) | HTTP framework with typed context | Already in use; `c.set("userId", ...)` / `c.get("userId")` |
+
+### Supporting
+| Library | Version | Purpose | When to Use |
+|---------|---------|---------|-------------|
+| @electric-sql/pglite | (current in project) | In-memory PG for tests | Already in use in `createTestDb()` |
+
+No new dependencies are needed for this phase.
+
+## Architecture Patterns
+
+### Current Service Signature Pattern
+```typescript
+// BEFORE (current)
+export async function getAllItems(db: Db = prodDb) {
+  return db.select().from(items);
+}
+
+// AFTER (Phase 16)
+export async function getAllItems(db: Db, userId: number) {
+  return db.select().from(items)
+    .where(eq(items.userId, userId));
+}
+```
+
+### userId Filtering Pattern (Drizzle `and()`)
+```typescript
+import { and, eq } from "drizzle-orm";
+
+// Single condition (list queries)
+.where(eq(items.userId, userId))
+
+// Multiple conditions (get by ID queries -- CRITICAL for isolation)
+.where(and(eq(items.id, id), eq(items.userId, userId)))
+```
+
+Every query that reads or writes user-owned data MUST include the userId filter. For get-by-id, update, and delete operations, this means using `and()` to combine the id condition with the userId condition. This prevents user A from accessing user B's data by guessing IDs.
+
+### Middleware userId Resolution Pattern
+```typescript
+// src/server/middleware/auth.ts
+export async function requireAuth(c: Context, next: Next) {
+  const db = c.get("db");
+
+  // 1. API key -> resolve userId from apiKeys table
+  const apiKey = c.req.header("X-API-Key");
+  if (apiKey) {
+    const result = await verifyApiKeyWithUser(db, apiKey);
+    if (result) {
+      c.set("userId", result.userId);
+      return next();
+    }
+    return c.json({ error: "Invalid API key" }, 401);
+  }
+
+  // 2. OAuth Bearer -> resolve userId from token -> user mapping
+  const authHeader = c.req.header("Authorization");
+  if (authHeader?.startsWith("Bearer ")) {
+    const token = authHeader.slice(7);
+    const result = await verifyAccessTokenWithUser(db, token);
+    if (result) {
+      c.set("userId", result.userId);
+      return next();
+    }
+    return c.json({ error: "invalid_token" }, 401);
+  }
+
+  // 3. OIDC session -> resolve logtoSub to local userId via upsert
+  const auth = await getAuth(c);
+  if (auth) {
+    const user = await getOrCreateUser(db, auth.sub);
+    c.set("userId", user.id);
+    return next();
+  }
+
+  return c.json({ error: "Authentication required" }, 401);
+}
+```
+
+### Route userId Extraction Pattern
+```typescript
+// In route handlers
+app.get("/", async (c) => {
+  const db = c.get("db");
+  const userId = c.get("userId");  // Set by requireAuth middleware
+  const items = await getAllItems(db, userId);
+  return c.json(items);
+});
+```
+
+**Important nuance**: Currently GET routes are public (no auth required). With multi-user data, GET routes ALSO need auth to know whose data to return. The middleware configuration in `src/server/index.ts` currently skips auth for GET requests:
+```typescript
+if (c.req.method === "GET") return next();
+```
+This must change -- all data routes need userId resolution. Options:
+1. Apply `requireAuth` to all methods on data routes (recommended -- simplest)
+2. Create a separate `resolveUser` middleware that runs on GET but returns 401 only on writes
+
+**Recommendation:** Apply `requireAuth` to all API routes (not just writes). The "public read" model no longer makes sense in a multi-user context where you need to know whose data to show.
+
+### MCP Tool Registration Pattern
+```typescript
+// MCP tools need userId passed through
+export function registerItemTools(db: Db, userId: number) {
+  return {
+    list_items: async (args: { categoryId?: number }) => {
+      const items = await getAllItems(db, userId);
+      // ...
+    },
+  };
+}
+```
+
+The MCP server creation must receive userId, which means the MCP auth middleware resolves userId and passes it to `createMcpServer(db, userId)`.
+
+### Schema Changes Pattern
+
+**New users table:**
+```typescript
+import { pgTable, serial, text, timestamp, unique } from "drizzle-orm/pg-core";
+
+export const users = pgTable("users", {
+  id: serial("id").primaryKey(),
+  logtoSub: text("logto_sub").notNull().unique(),
+  createdAt: timestamp("created_at").defaultNow().notNull(),
+});
+```
+
+**Adding userId to entity tables (example: items):**
+```typescript
+export const items = pgTable("items", {
+  // ... existing columns ...
+  userId: integer("user_id").notNull().references(() => users.id),
+});
+```
+
+**Composite unique on categories:**
+```typescript
+import { unique } from "drizzle-orm/pg-core";
+
+export const categories = pgTable("categories", {
+  id: serial("id").primaryKey(),
+  name: text("name").notNull(),
+  icon: text("icon").notNull().default("package"),
+  userId: integer("user_id").notNull().references(() => users.id),
+  createdAt: timestamp("created_at").defaultNow().notNull(),
+}, (table) => [
+  unique().on(table.userId, table.name),
+]);
+```
+
+**Composite PK on settings:**
+```typescript
+import { primaryKey } from "drizzle-orm/pg-core";
+
+export const settings = pgTable("settings", {
+  userId: integer("user_id").notNull().references(() => users.id),
+  key: text("key").notNull(),
+  value: text("value").notNull(),
+}, (table) => [
+  primaryKey({ columns: [table.userId, table.key] }),
+]);
+```
+
+### Recommended Project Structure (unchanged)
+```
+src/
+  db/
+    schema.ts           # Add users table, userId columns, composite constraints
+  server/
+    middleware/auth.ts   # Extend to resolve and set userId
+    services/            # All service functions gain userId parameter
+    routes/              # All routes extract userId from context
+    mcp/
+      index.ts           # MCP auth resolves userId, passes to createMcpServer
+      tools/             # Tool registrations accept userId
+tests/
+  helpers/db.ts          # createTestDb() seeds a test user
+  services/              # All tests pass userId to service calls
+  routes/                # Route tests set userId in context
+  mcp/                   # MCP tests pass userId
+```
+
+### Anti-Patterns to Avoid
+- **Filtering in application code instead of SQL:** Never fetch all records then filter by userId in JS. Always use `.where(eq(table.userId, userId))` in the query.
+- **Missing userId on get-by-id queries:** `getItemById(db, id)` without userId allows cross-user access by ID guessing. MUST be `and(eq(items.id, id), eq(items.userId, userId))`.
+- **Trusting child entity ownership via parent lookup:** When deleting a candidate, verify the parent thread belongs to the user, not just that the candidate exists.
+- **Hardcoding Uncategorized category id=1:** With per-user categories, there is no global ID 1. Each user's Uncategorized category has its own ID.
+
+## Don't Hand-Roll
+
+| Problem | Don't Build | Use Instead | Why |
+|---------|-------------|-------------|-----|
+| Composite unique constraints | Custom application-level uniqueness checks | Drizzle's `unique().on()` + PostgreSQL UNIQUE constraint | DB-level enforcement is atomic and race-condition-free |
+| Composite primary keys | Surrogate key + application uniqueness check | Drizzle's `primaryKey({ columns: [...] })` | Cleaner for settings table |
+| User upsert on first login | SELECT then conditional INSERT | PostgreSQL `ON CONFLICT DO NOTHING` / `ON CONFLICT DO UPDATE` via Drizzle | Race-condition-free, single query |
+| Migration data backfill | Manual SQL scripts outside Drizzle | Drizzle migration with raw SQL for backfill step | Keeps migration history consistent |
+
+## Common Pitfalls
+
+### Pitfall 1: Uncategorized Category Hardcoded to ID 1
+**What goes wrong:** Current code hardcodes `categoryId: 1` as the Uncategorized fallback (in `deleteCategory`, `resolveThread`, `createTestDb`). With per-user categories, each user's Uncategorized has a different ID.
+**Why it happens:** Single-user era guaranteed ID 1 was always Uncategorized.
+**How to avoid:** Create a helper function `getOrCreateUncategorized(db, userId)` that looks up the user's Uncategorized category by name+userId, creating it if missing. Replace all hardcoded `1` references.
+**Warning signs:** Tests failing with FK constraint violations, or items silently assigned to another user's category.
+
+### Pitfall 2: GET Routes Still Public (No userId Available)
+**What goes wrong:** GET /api/items returns all items from all users because no userId is available in context.
+**Why it happens:** Current middleware skips auth for GET requests. Multi-user data requires knowing the user for reads too.
+**How to avoid:** Apply `requireAuth` to all data API routes, not just write operations. Update the middleware configuration in `src/server/index.ts`.
+**Warning signs:** API returning data from other users, or 500 errors from `userId` being undefined.
+
+### Pitfall 3: Settings Table PK Change Requires Migration Care
+**What goes wrong:** Changing a primary key on an existing table with data requires dropping and recreating the constraint.
+**Why it happens:** PostgreSQL doesn't support ALTER PRIMARY KEY directly.
+**How to avoid:** Migration should: (1) add userId column, (2) drop old PK constraint, (3) add composite PK. All in a transaction.
+**Warning signs:** Migration fails with "cannot add constraint" errors.
+
+### Pitfall 4: verifyApiKey Must Return userId, Not Just Boolean
+**What goes wrong:** Current `verifyApiKey` returns `boolean`. But the middleware needs the userId associated with that API key to set on context.
+**Why it happens:** In single-user mode, knowing "auth is valid" was sufficient. Multi-user needs to know WHICH user.
+**How to avoid:** Change `verifyApiKey` to return `{ userId: number } | null` instead of `boolean`. Same for `verifyAccessToken`.
+**Warning signs:** userId is undefined in routes when using API key auth.
+
+### Pitfall 5: MCP Server Per-Session Architecture vs Per-Request userId
+**What goes wrong:** The current MCP architecture creates one `McpServer` per session and reuses it. But userId needs to be available for every tool call.
+**Why it happens:** `createMcpServer(db)` is called once at session init. userId is resolved per-request in the MCP auth middleware.
+**How to avoid:** Either (a) create the MCP server with the userId at session init (requires storing userId alongside transport), or (b) pass userId through the tool call context. Option (a) is simpler since the session is already per-authenticated-user.
+**Warning signs:** MCP tools returning data from wrong user, or userId undefined in tool handlers.
+
+### Pitfall 6: Thread Resolution Must Scope New Item to Same User
+**What goes wrong:** `resolveThread` creates a new item from a candidate. The new item must have the same userId as the thread.
+**Why it happens:** Current code doesn't set userId on the new item because the field doesn't exist yet.
+**How to avoid:** Pass userId to `resolveThread` and include it in the `insert(items).values({ ..., userId })` call. Also verify the thread belongs to the user before resolving.
+**Warning signs:** Resolved items appearing under wrong user or FK constraint violations.
+
+### Pitfall 7: CSV Import Creates Categories Without userId
+**What goes wrong:** `importItemsCsv` creates new categories on-the-fly when importing. These must be scoped to the importing user.
+**Why it happens:** Current code inserts categories without userId.
+**How to avoid:** Pass userId to `importItemsCsv`. Category creation and lookup must filter by userId.
+**Warning signs:** Categories from CSV import visible to all users, or unique constraint violations.
+
+### Pitfall 8: Setup Items Cross-User Boundary
+**What goes wrong:** `syncSetupItems` takes arbitrary itemIds. A user could add another user's items to their setup.
+**Why it happens:** No validation that the items belong to the same user as the setup.
+**How to avoid:** In `syncSetupItems`, verify each itemId belongs to the same userId before inserting. Or filter the item list to only user-owned items.
+**Warning signs:** Setup showing items owned by other users.
+
+## Code Examples
+
+### User Upsert on First Login
+```typescript
+// Source: Drizzle ORM PostgreSQL onConflict pattern
+export async function getOrCreateUser(db: Db, logtoSub: string): Promise<{ id: number }> {
+  const [user] = await db
+    .insert(users)
+    .values({ logtoSub })
+    .onConflictDoUpdate({
+      target: users.logtoSub,
+      set: { logtoSub },  // no-op update to return existing row
+    })
+    .returning({ id: users.id });
+  return user;
+}
+```
+
+### Get or Create Uncategorized Category
+```typescript
+export async function getOrCreateUncategorized(db: Db, userId: number): Promise<number> {
+  const [existing] = await db
+    .select({ id: categories.id })
+    .from(categories)
+    .where(and(eq(categories.userId, userId), eq(categories.name, "Uncategorized")));
+  
+  if (existing) return existing.id;
+  
+  const [created] = await db
+    .insert(categories)
+    .values({ name: "Uncategorized", icon: "package", userId })
+    .returning({ id: categories.id });
+  
+  return created.id;
+}
+```
+
+### Migration SQL (multi-step in single migration file)
+```sql
+-- Step 1: Create users table
+CREATE TABLE "users" (
+  "id" serial PRIMARY KEY NOT NULL,
+  "logto_sub" text NOT NULL,
+  "created_at" timestamp DEFAULT now() NOT NULL,
+  CONSTRAINT "users_logto_sub_unique" UNIQUE("logto_sub")
+);
+
+-- Step 2: Insert placeholder user for existing data
+INSERT INTO "users" ("logto_sub") VALUES ('migration-placeholder');
+
+-- Step 3: Add userId columns (nullable first)
+ALTER TABLE "items" ADD COLUMN "user_id" integer;
+ALTER TABLE "categories" ADD COLUMN "user_id" integer;
+ALTER TABLE "threads" ADD COLUMN "user_id" integer;
+ALTER TABLE "setups" ADD COLUMN "user_id" integer;
+ALTER TABLE "api_keys" ADD COLUMN "user_id" integer;
+
+-- Step 4: Backfill all rows to user 1
+UPDATE "items" SET "user_id" = 1;
+UPDATE "categories" SET "user_id" = 1;
+UPDATE "threads" SET "user_id" = 1;
+UPDATE "setups" SET "user_id" = 1;
+UPDATE "api_keys" SET "user_id" = 1;
+
+-- Step 5: Set NOT NULL and add FK constraints
+ALTER TABLE "items" ALTER COLUMN "user_id" SET NOT NULL;
+ALTER TABLE "items" ADD CONSTRAINT "items_user_id_users_id_fk" 
+  FOREIGN KEY ("user_id") REFERENCES "users"("id");
+-- (repeat for all tables)
+
+-- Step 6: Settings table - add userId, change PK
+ALTER TABLE "settings" ADD COLUMN "user_id" integer;
+UPDATE "settings" SET "user_id" = 1;
+ALTER TABLE "settings" ALTER COLUMN "user_id" SET NOT NULL;
+ALTER TABLE "settings" DROP CONSTRAINT "settings_pkey";
+ALTER TABLE "settings" ADD PRIMARY KEY ("user_id", "key");
+ALTER TABLE "settings" ADD CONSTRAINT "settings_user_id_users_id_fk"
+  FOREIGN KEY ("user_id") REFERENCES "users"("id");
+
+-- Step 7: Categories - drop old unique, add composite unique
+ALTER TABLE "categories" DROP CONSTRAINT "categories_name_unique";
+ALTER TABLE "categories" ADD CONSTRAINT "categories_user_id_name_unique" 
+  UNIQUE("user_id", "name");
+```
+
+### Test Helper Update
+```typescript
+export async function createTestDb() {
+  const db = drizzle({ schema });
+  await migrate(db, { migrationsFolder: "./drizzle-pg" });
+
+  // Seed test user
+  const [user] = await db
+    .insert(schema.users)
+    .values({ logtoSub: "test-user-sub" })
+    .returning();
+
+  // Seed per-user Uncategorized category
+  await db
+    .insert(schema.categories)
+    .values({ name: "Uncategorized", icon: "package", userId: user.id });
+
+  return { db, userId: user.id };
+}
+```
+
+Note: this changes the return type of `createTestDb()` from just `db` to `{ db, userId }`. All test files need updating.
+
+### API Key Verification Returning userId
+```typescript
+export async function verifyApiKey(
+  db: Db,
+  rawKey: string,
+): Promise<{ userId: number } | null> {
+  const prefix = rawKey.slice(0, 8);
+  const candidates = await db
+    .select({ keyHash: apiKeys.keyHash, userId: apiKeys.userId })
+    .from(apiKeys)
+    .where(eq(apiKeys.keyPrefix, prefix));
+
+  for (const candidate of candidates) {
+    if (await Bun.password.verify(rawKey, candidate.keyHash)) {
+      return { userId: candidate.userId };
+    }
+  }
+  return null;
+}
+```
+
+## State of the Art
+
+| Old Approach | Current Approach | When Changed | Impact |
+|--------------|------------------|--------------|--------|
+| Single-user, no userId columns | Multi-user with userId FK on all entities | Phase 16 | Every query must be scoped |
+| Global Uncategorized category (id=1) | Per-user Uncategorized (dynamic lookup) | Phase 16 | No more hardcoded category IDs |
+| `verifyApiKey` returns boolean | Returns `{ userId } | null` | Phase 16 | Middleware can set userId on context |
+| Public GET endpoints | All data endpoints require auth | Phase 16 | Must know user to scope data |
+| `settings` table PK = `key` | PK = `(userId, key)` | Phase 16 | Per-user settings |
+
+## Validation Architecture
+
+### Test Framework
+| Property | Value |
+|----------|-------|
+| Framework | Bun test runner (built-in) |
+| Config file | none (Bun built-in) |
+| Quick run command | `bun test tests/services/item.service.test.ts` |
+| Full suite command | `bun test` |
+
+### Phase Requirements -> Test Map
+| Req ID | Behavior | Test Type | Automated Command | File Exists? |
+|--------|----------|-----------|-------------------|-------------|
+| MULTI-01 | Items/categories/threads/setups have userId FK | unit | `bun test tests/services/item.service.test.ts` | Exists (needs update) |
+| MULTI-02 | User A cannot see User B's data | unit | `bun test tests/services/item.service.test.ts` (new isolation test) | Needs new test |
+| MULTI-03 | Categories composite unique (userId, name) | unit | `bun test tests/services/category.service.test.ts` | Exists (needs update) |
+| MULTI-04 | Migration backfills existing data to user 1 | integration | Migration test (manual verification) | Needs new test |
+| MULTI-05 | MCP tools scoped to authenticated user | unit | `bun test tests/mcp/tools.test.ts` | Exists (needs update) |
+| MULTI-06 | Settings per-user | unit | `bun test tests/routes/settings.test.ts` (new) | Needs update |
+
+### Sampling Rate
+- **Per task commit:** `bun test` (full suite, fast with PGlite)
+- **Per wave merge:** `bun test` + `bun run lint`
+- **Phase gate:** Full suite green before `/gsd:verify-work`
+
+### Wave 0 Gaps
+- [ ] Update `createTestDb()` to return `{ db, userId }` with seeded user
+- [ ] Add cross-user isolation tests (create data as user A, verify user B cannot see it)
+- [ ] Update all existing service tests to pass userId parameter
+- [ ] Update all existing route tests to set userId in context middleware
+- [ ] Update MCP tool tests to pass userId to register functions
+
+## Open Questions
+
+1. **Schema file still uses `sqliteTable` imports**
+   - What we know: `src/db/schema.ts` imports from `drizzle-orm/sqlite-core` but `src/db/index.ts` uses `drizzle-orm/postgres-js`. The PG migration in `drizzle-pg/` has the correct PostgreSQL DDL. Tests use PGlite successfully.
+   - What's unclear: Whether Phase 14 intended to switch schema.ts to `pgTable` imports or if Drizzle's abstraction handles this. The migration SQL was generated from this schema and works.
+   - Recommendation: This phase should switch schema.ts to use `drizzle-orm/pg-core` imports (`pgTable`, `serial`, `text`, `timestamp`, `integer`, `doublePrecision`) as part of adding the new columns. This ensures composite constraints and PG-specific features work correctly. Verify by running `bun run db:generate` after changes.
+
+2. **OAuth token -> userId mapping**
+   - What we know: `oauth_tokens` table stores access/refresh token hashes but has no userId column.
+   - What's unclear: How to resolve an OAuth Bearer token to a userId. Currently `verifyAccessToken` just returns boolean.
+   - Recommendation: Add `userId` column to `oauth_tokens` table, set during token creation (the `/oauth/authorize` flow knows the OIDC user). Then `verifyAccessToken` can return userId.
+
+3. **MCP session architecture and userId**
+   - What we know: MCP creates one server per session. Auth middleware runs per-request.
+   - Recommendation: Store userId alongside the transport in the session map: `Map<string, { transport, userId }>`. Pass userId when creating the MCP server. Since a session is always for one authenticated user, this is safe.
+
+## Sources
+
+### Primary (HIGH confidence)
+- `src/db/schema.ts` -- Current table definitions (no users table, no userId columns)
+- `src/server/middleware/auth.ts` -- Current auth middleware (returns boolean, no userId resolution)
+- `src/server/services/*.ts` -- All 7 service files (all use `(db, ...)` signature)
+- `src/server/routes/*.ts` -- All route handlers (no userId extraction from context)
+- `src/server/mcp/index.ts` -- MCP server creation (no userId threading)
+- `tests/helpers/db.ts` -- Test helper (seeds Uncategorized with id=1, no user)
+- `drizzle-pg/0000_fuzzy_shiva.sql` -- Current PostgreSQL migration (includes old users table from pre-Phase 15)
+- `.planning/phases/15-external-authentication/15-VERIFICATION.md` -- Confirms users/sessions tables dropped
+
+### Secondary (MEDIUM confidence)
+- Drizzle ORM composite constraints -- `unique().on()` and `primaryKey({ columns: [...] })` patterns verified from existing codebase (`oauth.service.ts` uses `and()`)
+
+## Metadata
+
+**Confidence breakdown:**
+- Standard stack: HIGH -- no new libraries needed, all patterns exist in codebase
+- Architecture: HIGH -- mechanical transformation of existing service/route/test patterns
+- Pitfalls: HIGH -- identified from direct code inspection of all affected files
+- Migration: MEDIUM -- composite PK change on settings table needs careful SQL ordering
+
+**Research date:** 2026-04-04
+**Valid until:** 2026-05-04 (stable -- no external dependency changes expected)