makiolaj/GearBox
1
0
Fork 0
Code Issues 8 Pull Requests Actions Packages Projects Releases 10 Wiki Activity

docs(16): create multi-user data model phase plan

Browse Source
This commit is contained in:
Jean-Luc Makiola
2026-04-05 10:27:30 +02:00
parent dbd265d18d
commit 64821f856c
5 changed files with 1213 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
.planning/ROADMAP.md
View File

@@ -145,7 +145,12 @@ Plans:
3. Existing data from the single-user era is assigned to the original user account after migration 3. Existing data from the single-user era is assigned to the original user account after migration
4. MCP tools return only data belonging to the authenticated API key's owner 4. MCP tools return only data belonging to the authenticated API key's owner
5. Each user has independent settings (weight unit, onboarding state) that do not affect other users 5. Each user has independent settings (weight unit, onboarding state) that do not affect other users
**Plans**: TBD **Plans:** 4 plans
Plans:
- [ ] 16-01-PLAN.md — Schema (pgTable + users table + userId columns), auth middleware userId resolution, test helper
- [ ] 16-02-PLAN.md — All services updated with userId parameter and per-user query scoping
- [ ] 16-03-PLAN.md — Routes + MCP tools wired to pass userId from context to services
- [ ] 16-04-PLAN.md — All tests updated with userId, cross-user isolation tests added
### Phase 17: Object Storage ### Phase 17: Object Storage
**Goal**: Images are stored in and served from MinIO instead of the local filesystem **Goal**: Images are stored in and served from MinIO instead of the local filesystem
@@ -190,6 +195,6 @@ Plans:
| 13. Setup Impact Preview | v1.3 | 0/2 | Not started | - | | 13. Setup Impact Preview | v1.3 | 0/2 | Not started | - |
| 14. PostgreSQL Migration | v2.0 | 0/? | Not started | - | | 14. PostgreSQL Migration | v2.0 | 0/? | Not started | - |
| 15. External Authentication | v2.0 | 1/1 | Complete | 2026-04-04 | | 15. External Authentication | v2.0 | 1/1 | Complete | 2026-04-04 |
| 16. Multi-User Data Model | v2.0 | 0/? | Not started | - | | 16. Multi-User Data Model | v2.0 | 0/4 | Not started | - |
| 17. Object Storage | v2.0 | 0/? | Not started | - | | 17. Object Storage | v2.0 | 0/? | Not started | - |
| 18. Global Items & Public Profiles | v2.0 | 0/? | Not started | - | | 18. Global Items & Public Profiles | v2.0 | 0/? | Not started | - |

355
.planning/phases/16-multi-user-data-model/16-01-PLAN.md Normal file
View File

@@ -0,0 +1,355 @@
---
phase: 16-multi-user-data-model
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
- src/db/schema.ts
- src/db/seed.ts
- tests/helpers/db.ts
- src/server/middleware/auth.ts
- src/server/services/auth.service.ts
- src/server/services/oauth.service.ts
- src/server/index.ts
autonomous: true
requirements:
- MULTI-01
- MULTI-04
- MULTI-06
must_haves:
truths:
- "A users table exists with id (serial PK), logtoSub (text unique), createdAt (timestamp)"
- "Every entity table (items, categories, threads, setups, settings, apiKeys) has a userId integer FK column"
- "Categories have a composite unique constraint on (userId, name) instead of unique(name)"
- "Settings have a composite primary key on (userId, key) instead of just (key)"
- "requireAuth middleware resolves userId and sets it on Hono context"
- "verifyApiKey returns { userId } | null instead of boolean"
- "verifyAccessToken returns { userId } | null instead of boolean"
- "createTestDb returns { db, userId } with a seeded test user and per-user Uncategorized category"
- "All API routes require auth (no GET bypass) so userId is always available"
artifacts:
- path: "src/db/schema.ts"
provides: "Users table + userId columns on all entity tables + composite constraints"
contains: "export const users"
- path: "tests/helpers/db.ts"
provides: "Test DB with seeded user"
contains: "logtoSub"
- path: "src/server/middleware/auth.ts"
provides: "userId resolution middleware"
contains: "c.set(\"userId\""
key_links:
- from: "src/server/middleware/auth.ts"
to: "src/server/services/auth.service.ts"
via: "verifyApiKey returning userId"
pattern: "verifyApiKey.*userId"
- from: "src/server/middleware/auth.ts"
to: "src/server/services/oauth.service.ts"
via: "verifyAccessToken returning userId"
pattern: "verifyAccessToken.*userId"
---
<objective>
Create the multi-user data model foundation: users table, userId columns on all entity tables, updated auth middleware that resolves userId onto Hono context, and updated test infrastructure.
Purpose: This is the foundation that all subsequent plans depend on. Without the schema changes, userId columns, and middleware resolution, no service or route can be updated to scope data per-user.
Output: Updated schema.ts with pgTable imports and users table, migration generated, auth middleware resolving userId, test helper returning { db, userId }.
</objective>
<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/16-multi-user-data-model/16-CONTEXT.md
@.planning/phases/16-multi-user-data-model/16-RESEARCH.md
@src/db/schema.ts
@src/db/seed.ts
@src/server/middleware/auth.ts
@src/server/services/auth.service.ts
@src/server/services/oauth.service.ts
@src/server/index.ts
@tests/helpers/db.ts
<interfaces>
<!-- Key types and contracts the executor needs -->
From src/db/schema.ts (current - uses sqliteTable, must switch to pgTable):
- categories: id, name (unique), icon, createdAt
- items: id, name, weightGrams, priceCents, categoryId, notes, productUrl, imageFilename, imageSourceUrl, quantity, createdAt, updatedAt
- threads: id, name, status, resolvedCandidateId, categoryId, createdAt, updatedAt
- threadCandidates: id, threadId, name, weightGrams, priceCents, categoryId, notes, productUrl, imageFilename, imageSourceUrl, status, pros, cons, sortOrder, createdAt, updatedAt
- setups: id, name, createdAt, updatedAt
- setupItems: id, setupId, itemId, classification
- settings: key (PK), value
- apiKeys: id, name, keyHash, keyPrefix, createdAt
- oauthClients: id, clientId, clientName, redirectUris, createdAt
- oauthCodes: id, code, clientId, codeChallenge, codeChallengeMethod, redirectUri, expiresAt, used
- oauthTokens: id, accessTokenHash, refreshTokenHash, clientId, expiresAt, refreshExpiresAt, createdAt
From src/server/services/auth.service.ts:
```typescript
export async function verifyApiKey(db: Db, rawKey: string): Promise<boolean>
export async function createApiKey(db: Db, name: string)
export async function listApiKeys(db: Db)
export async function deleteApiKey(db: Db, id: number)
```
From src/server/services/oauth.service.ts:
```typescript
export async function verifyAccessToken(db: Db, token: string): Promise<boolean>
```
</interfaces>
</context>
<tasks>
<task type="auto">
<name>Task 1: Migrate schema.ts to pgTable and add users table + userId columns</name>
<files>src/db/schema.ts, src/db/seed.ts</files>
<read_first>src/db/schema.ts, src/db/seed.ts, .planning/phases/16-multi-user-data-model/16-RESEARCH.md</read_first>
<action>
1. Rewrite `src/db/schema.ts` to use `drizzle-orm/pg-core` imports instead of `drizzle-orm/sqlite-core`. Replace `sqliteTable` with `pgTable`, `integer("id").primaryKey({ autoIncrement: true })` with `serial("id").primaryKey()`, `integer` with `integer` from pg-core, `real` with `doublePrecision`, and `integer("...", { mode: "timestamp" })` with `timestamp("...").defaultNow()` (or equivalent).
2. Add the new `users` table per D-01:
```typescript
export const users = pgTable("users", {
id: serial("id").primaryKey(),
logtoSub: text("logto_sub").notNull().unique(),
createdAt: timestamp("created_at").defaultNow().notNull(),
});
```
3. Add `userId` column (integer, NOT NULL, FK to users.id) to these tables per D-04:
- `items`: `userId: integer("user_id").notNull().references(() => users.id)`
- `categories`: `userId: integer("user_id").notNull().references(() => users.id)`
- `threads`: `userId: integer("user_id").notNull().references(() => users.id)`
- `setups`: `userId: integer("user_id").notNull().references(() => users.id)`
- `apiKeys`: `userId: integer("user_id").notNull().references(() => users.id)` per D-07
- `oauthTokens`: `userId: integer("user_id").notNull().references(() => users.id)` (per Research open question 2)
4. Per D-05, change `categories` unique constraint from `name` alone to composite `(userId, name)`:
```typescript
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),
]);
```
5. Per D-06, change `settings` PK from `key` alone to composite `(userId, key)`:
```typescript
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] }),
]);
```
6. Per D-08, do NOT add userId to `threadCandidates` or `setupItems` (they inherit ownership via parent FK).
7. Update `src/db/seed.ts`: The `seedDefaults()` function currently seeds a global Uncategorized category. Since categories now require userId, this global seed no longer works. Change `seedDefaults()` to be a no-op or remove the category seeding entirely (per-user Uncategorized will be created lazily or on first login per D-12). The function can remain as an empty function for now:
```typescript
export async function seedDefaults() {
// Per-user default categories are created on first login (Phase 16)
}
```
8. Run `bun run db:generate` to generate the new Drizzle migration into `drizzle-pg/`. Then verify the generated SQL includes the users table, userId columns, composite constraints, and FK relationships.
IMPORTANT: The schema.ts file MUST use pg-core imports (`pgTable`, `serial`, `text`, `timestamp`, `integer`, `doublePrecision`, `unique`, `primaryKey`). The existing `drizzle-pg/` migration directory already has PostgreSQL DDL from Phase 14.
</action>
<verify>
<automated>grep -c "pgTable" src/db/schema.ts && grep -c "export const users" src/db/schema.ts && grep "userId" src/db/schema.ts | wc -l && grep -c "unique().on" src/db/schema.ts && grep -c "primaryKey" src/db/schema.ts</automated>
</verify>
<acceptance_criteria>
- `src/db/schema.ts` contains `import.*from "drizzle-orm/pg-core"` (no sqlite-core imports)
- `export const users = pgTable("users"` exists with `logtoSub`, `id`, `createdAt`
- `userId: integer("user_id").notNull().references(() => users.id)` appears in items, categories, threads, setups, apiKeys, oauthTokens
- `unique().on(table.userId, table.name)` exists in categories table definition
- `primaryKey({ columns: [table.userId, table.key] })` exists in settings table definition
- `threadCandidates` and `setupItems` do NOT have a userId column
- `src/db/seed.ts` no longer inserts a global Uncategorized category
- A new migration file exists in `drizzle-pg/` with the users table and userId column additions
</acceptance_criteria>
<done>Schema uses pg-core imports, users table exists, all 6 entity tables have userId FK, categories has composite unique, settings has composite PK, migration generated</done>
</task>
<task type="auto">
<name>Task 2: Update auth middleware and auth services to resolve userId</name>
<files>src/server/middleware/auth.ts, src/server/services/auth.service.ts, src/server/services/oauth.service.ts, src/server/index.ts</files>
<read_first>src/server/middleware/auth.ts, src/server/services/auth.service.ts, src/server/services/oauth.service.ts, src/server/index.ts, src/db/schema.ts</read_first>
<action>
1. **Update `verifyApiKey` in `src/server/services/auth.service.ts`** per D-03/D-07:
Change return type from `Promise<boolean>` to `Promise<{ userId: number } | null>`. The function queries apiKeys by keyPrefix, verifies the hash, and now returns `{ userId: candidate.userId }` on match or `null` on failure. Also update `createApiKey` to accept and store `userId`, `listApiKeys` to filter by `userId`, and `deleteApiKey` to filter by `userId` (using `and(eq(apiKeys.id, id), eq(apiKeys.userId, userId))`).
2. **Update `verifyAccessToken` in `src/server/services/oauth.service.ts`**:
Change return type from `Promise<boolean>` to `Promise<{ userId: number } | null>`. Select `userId` from the oauthTokens record and return `{ userId: record.userId }` on success, `null` on failure. Also update `createTokens` to accept and store `userId`.
3. **Create `getOrCreateUser` function** in `src/server/services/auth.service.ts` per D-01:
```typescript
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 },
})
.returning({ id: users.id });
return user;
}
```
4. **Create `getOrCreateUncategorized` helper** in `src/server/services/category.service.ts` (or auth.service.ts):
```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;
}
```
Place this in `category.service.ts` since it's category-related.
5. **Rewrite `requireAuth` in `src/server/middleware/auth.ts`** per D-03/D-10:
- For API key auth: call `verifyApiKey(db, apiKey)` which now returns `{ userId } | null`. On success, `c.set("userId", result.userId)` and call `next()`.
- For OAuth Bearer: call `verifyAccessToken(db, token)` which now returns `{ userId } | null`. On success, `c.set("userId", result.userId)`.
- For OIDC session: call `getAuth(c)` for the sub claim, then `getOrCreateUser(db, auth.sub)` to get the local userId. Then call `getOrCreateUncategorized(db, user.id)` to ensure the user has a default category. Set `c.set("userId", user.id)`.
- Import `getOrCreateUser` from auth.service and `getOrCreateUncategorized` from category.service.
6. **Update auth middleware configuration in `src/server/index.ts`** per Research pitfall 2:
Change the `/api/*` middleware from:
```typescript
if (c.req.method === "GET") return next();
```
to apply `requireAuth` to ALL methods on data routes (remove the GET bypass). This ensures userId is always available on context for read operations. Keep the `/api/auth` bypass and add a bypass for `/api/health`.
The new middleware block should be:
```typescript
app.use("/api/*", async (c, next) => {
if (c.req.path.startsWith("/api/auth")) return next();
if (c.req.path === "/api/health") return next();
return requireAuth(c, next);
});
```
</action>
<verify>
<automated>grep -c "c.set(\"userId\"" src/server/middleware/auth.ts && grep "Promise<{ userId: number } | null>" src/server/services/auth.service.ts | wc -l && grep "Promise<{ userId: number } | null>" src/server/services/oauth.service.ts | wc -l && grep -c "getOrCreateUser" src/server/services/auth.service.ts && ! grep "GET.*return next" src/server/index.ts</automated>
</verify>
<acceptance_criteria>
- `src/server/middleware/auth.ts` calls `c.set("userId", ...)` in all three auth paths (API key, Bearer, OIDC)
- `verifyApiKey` in auth.service.ts has return type `Promise<{ userId: number } | null>`
- `verifyAccessToken` in oauth.service.ts has return type `Promise<{ userId: number } | null>`
- `getOrCreateUser` function exists in auth.service.ts with `onConflictDoUpdate` pattern
- `getOrCreateUncategorized` function exists in category.service.ts
- `src/server/index.ts` does NOT contain `if (c.req.method === "GET") return next()`
- `src/server/index.ts` still bypasses auth for `/api/auth` and `/api/health` paths
</acceptance_criteria>
<done>Auth middleware resolves userId for all auth methods, verifyApiKey and verifyAccessToken return userId, GET routes require auth, getOrCreateUser and getOrCreateUncategorized helpers exist</done>
</task>
<task type="auto">
<name>Task 3: Update test helper to seed user and return { db, userId }</name>
<files>tests/helpers/db.ts</files>
<read_first>tests/helpers/db.ts, src/db/schema.ts</read_first>
<action>
Update `createTestDb()` in `tests/helpers/db.ts` to:
1. After running migrations, insert a test user: `await db.insert(schema.users).values({ logtoSub: "test-user-sub" }).returning()`
2. Insert the per-user Uncategorized category with the test user's ID: `await db.insert(schema.categories).values({ name: "Uncategorized", icon: "package", userId: user.id })`
3. Change the return type from just `db` to `{ db, userId: user.id }` so all tests can destructure it.
4. Also add a helper function `createSecondTestUser(db)` that creates a second user for cross-user isolation tests:
```typescript
export async function createSecondTestUser(db: Db) {
const [user] = await db
.insert(schema.users)
.values({ logtoSub: "test-user-2-sub" })
.returning();
await db
.insert(schema.categories)
.values({ name: "Uncategorized", icon: "package", userId: user.id });
return user.id;
}
```
The updated `createTestDb` should look like:
```typescript
export async function createTestDb() {
const db = drizzle({ schema });
await migrate(db, { migrationsFolder: "./drizzle-pg" });
const [user] = await db
.insert(schema.users)
.values({ logtoSub: "test-user-sub" })
.returning();
await db
.insert(schema.categories)
.values({ name: "Uncategorized", icon: "package", userId: user.id });
return { db, userId: user.id };
}
```
IMPORTANT: This changes the return type of createTestDb from `db` to `{ db, userId }`. All existing test files that call `createTestDb()` will need updating in Plan 04 to destructure the result.
</action>
<verify>
<automated>grep -c "logtoSub" tests/helpers/db.ts && grep -c "userId: user.id" tests/helpers/db.ts && grep "return { db, userId" tests/helpers/db.ts | wc -l && grep -c "createSecondTestUser" tests/helpers/db.ts</automated>
</verify>
<acceptance_criteria>
- `createTestDb()` inserts a user with `logtoSub: "test-user-sub"`
- `createTestDb()` returns `{ db, userId: user.id }` (not just `db`)
- Uncategorized category is created with the test user's ID
- `createSecondTestUser` function exists and is exported
- Import of `schema.users` is present
</acceptance_criteria>
<done>Test helper seeds a user, returns { db, userId }, has a createSecondTestUser helper for isolation tests</done>
</task>
</tasks>
<verification>
After all tasks complete:
1. `grep "pgTable" src/db/schema.ts` shows pg-core usage throughout
2. `grep "export const users" src/db/schema.ts` confirms users table
3. `grep "userId" src/db/schema.ts` shows userId on items, categories, threads, setups, settings, apiKeys, oauthTokens
4. `grep "c.set(\"userId\"" src/server/middleware/auth.ts` shows userId set in middleware
5. `grep "getOrCreateUser" src/server/services/auth.service.ts` confirms user upsert helper
6. `grep "return { db, userId" tests/helpers/db.ts` confirms new test helper return type
7. No `sqliteTable` imports remain in schema.ts
</verification>
<success_criteria>
- Schema uses pg-core imports exclusively (no sqlite-core)
- users table with id, logtoSub (unique), createdAt defined
- userId FK column present on items, categories, threads, setups, settings, apiKeys, oauthTokens
- categories: composite unique on (userId, name)
- settings: composite PK on (userId, key)
- requireAuth resolves userId for API key, Bearer token, and OIDC session
- verifyApiKey and verifyAccessToken return { userId } | null
- Test helper returns { db, userId } with seeded user
- All API routes require auth (no GET bypass)
- Drizzle migration generated in drizzle-pg/
</success_criteria>
<output>
After completion, create `.planning/phases/16-multi-user-data-model/16-01-SUMMARY.md`
</output>

254
.planning/phases/16-multi-user-data-model/16-02-PLAN.md Normal file
View File

@@ -0,0 +1,254 @@
---
phase: 16-multi-user-data-model
plan: 02
type: execute
wave: 2
depends_on: ["16-01"]
files_modified:
- src/server/services/item.service.ts
- src/server/services/category.service.ts
- src/server/services/thread.service.ts
- src/server/services/setup.service.ts
- src/server/services/totals.service.ts
- src/server/services/csv.service.ts
- src/server/services/auth.service.ts
autonomous: true
requirements:
- MULTI-01
- MULTI-02
- MULTI-03
- MULTI-06
must_haves:
truths:
- "Every service function that reads or writes user data accepts a userId parameter"
- "All queries filter by userId using eq(table.userId, userId)"
- "Get-by-id queries use and(eq(table.id, id), eq(table.userId, userId)) to prevent cross-user access"
- "Category operations respect composite unique (userId, name)"
- "Settings operations use composite PK (userId, key)"
- "Thread resolution creates the new item with the same userId as the thread"
- "CSV import scopes category creation and lookup to the importing user"
- "API key CRUD is scoped to the owning user"
artifacts:
- path: "src/server/services/item.service.ts"
provides: "User-scoped item CRUD"
contains: "userId: number"
- path: "src/server/services/category.service.ts"
provides: "User-scoped category CRUD with composite unique"
contains: "userId: number"
- path: "src/server/services/thread.service.ts"
provides: "User-scoped thread + candidate CRUD + resolution"
contains: "userId: number"
- path: "src/server/services/setup.service.ts"
provides: "User-scoped setup CRUD + item sync validation"
contains: "userId: number"
- path: "src/server/services/totals.service.ts"
provides: "User-scoped aggregate queries"
contains: "userId: number"
- path: "src/server/services/csv.service.ts"
provides: "User-scoped CSV import/export"
contains: "userId: number"
key_links:
- from: "src/server/services/thread.service.ts"
to: "src/db/schema.ts"
via: "userId on insert(items) during thread resolution"
pattern: "userId.*resolv"
- from: "src/server/services/setup.service.ts"
to: "src/db/schema.ts"
via: "validates item ownership before sync"
pattern: "eq.*items.userId.*userId"
- from: "src/server/services/category.service.ts"
to: "src/db/schema.ts"
via: "getOrCreateUncategorized uses composite unique"
pattern: "getOrCreateUncategorized"
---
<objective>
Add userId parameter to every service function and scope all database queries to the authenticated user.
Purpose: Services are the data access layer. Scoping them to userId is the core of multi-user data isolation (MULTI-02). Without this, routes and MCP tools have no way to enforce per-user boundaries.
Output: All 7 service files updated with userId parameter on every function, all queries filtered by userId.
</objective>
<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/phases/16-multi-user-data-model/16-CONTEXT.md
@.planning/phases/16-multi-user-data-model/16-RESEARCH.md
@.planning/phases/16-multi-user-data-model/16-01-SUMMARY.md
@src/db/schema.ts
@src/server/services/item.service.ts
@src/server/services/category.service.ts
@src/server/services/thread.service.ts
@src/server/services/setup.service.ts
@src/server/services/totals.service.ts
@src/server/services/csv.service.ts
@src/server/services/auth.service.ts
<interfaces>
<!-- Service function signatures that must change from (db, ...) to (db, userId, ...) -->
<!-- From Plan 01 SUMMARY: schema.ts now has userId on items, categories, threads, setups, settings, apiKeys -->
<!-- getOrCreateUncategorized(db, userId) already created in Plan 01 in category.service.ts -->
Expected new signatures (all gain userId as second param):
- item.service.ts: getAllItems(db, userId), getItemById(db, userId, id), createItem(db, userId, data), updateItem(db, userId, id, data), deleteItem(db, userId, id)
- category.service.ts: getAllCategories(db, userId), getCategoryById(db, userId, id), createCategory(db, userId, data), updateCategory(db, userId, id, data), deleteCategory(db, userId, id)
- getOrCreateUncategorized(db, userId) already exists from Plan 01
- thread.service.ts: getAllThreads(db, userId), getThreadById(db, userId, id), createThread(db, userId, data), updateThread(db, userId, id, data), deleteThread(db, userId, id), resolveThread(db, userId, id, candidateId), addCandidate(db, userId, ...), updateCandidate(db, userId, ...), removeCandidate(db, userId, ...)
- setup.service.ts: getAllSetups(db, userId), getSetupById(db, userId, id), createSetup(db, userId, data), updateSetup(db, userId, id, data), deleteSetup(db, userId, id), syncSetupItems(db, userId, setupId, items)
- totals.service.ts: getTotals(db, userId)
- csv.service.ts: importItemsCsv(db, userId, data), exportItemsCsv(db, userId)
- auth.service.ts: createApiKey(db, userId, name), listApiKeys(db, userId), deleteApiKey(db, userId, id) — verifyApiKey already updated in Plan 01
</interfaces>
</context>
<tasks>
<task type="auto">
<name>Task 1: Update item, category, totals, and CSV services with userId scoping</name>
<files>src/server/services/item.service.ts, src/server/services/category.service.ts, src/server/services/totals.service.ts, src/server/services/csv.service.ts</files>
<read_first>src/server/services/item.service.ts, src/server/services/category.service.ts, src/server/services/totals.service.ts, src/server/services/csv.service.ts, src/db/schema.ts</read_first>
<action>
**item.service.ts** per D-09:
- Add `userId: number` as second parameter to ALL exported functions
- Remove default `db` parameter values (no more `db: Db = prodDb`) -- db is always injected
- `getAllItems`: add `.where(eq(items.userId, userId))`
- `getItemById`: change `.where(eq(items.id, id))` to `.where(and(eq(items.id, id), eq(items.userId, userId)))` -- CRITICAL for isolation per Research anti-pattern
- `createItem`: include `userId` in the `.values({...})` insert
- `updateItem`: add `eq(items.userId, userId)` to the `.where()` clause using `and()`
- `deleteItem`: add `eq(items.userId, userId)` to the `.where()` clause using `and()`
- Import `and` from `drizzle-orm` if not already imported
**category.service.ts** per D-05/D-09:
- Add `userId: number` as second parameter to ALL exported functions
- Remove default `db` parameter values
- `getAllCategories`: add `.where(eq(categories.userId, userId))`
- `getCategoryById`: use `and(eq(categories.id, id), eq(categories.userId, userId))`
- `createCategory`: include `userId` in the insert values
- `updateCategory`: add userId filter with `and()`
- `deleteCategory`: add userId filter with `and()`. When reassigning items to Uncategorized on delete, use `getOrCreateUncategorized(db, userId)` instead of hardcoded category ID 1. Also scope the item reassignment to only items belonging to this user.
- The `getOrCreateUncategorized` function was already created in Plan 01
**totals.service.ts** per D-09:
- Add `userId: number` parameter
- Filter all aggregate queries by userId
- This file computes weight/cost totals across the collection -- must only sum the user's items
**csv.service.ts** per D-09 and Research pitfall 7:
- Add `userId: number` parameter to `importItemsCsv` and `exportItemsCsv`
- `exportItemsCsv`: filter items query by userId
- `importItemsCsv`:
- Category lookup/creation must filter by userId (use `getOrCreateUncategorized` for fallback)
- When creating new categories from CSV data, include userId
- When creating items, include userId
- Category name matching must be scoped to user's categories
</action>
<verify>
<automated>grep -c "userId: number" src/server/services/item.service.ts && grep -c "userId: number" src/server/services/category.service.ts && grep -c "userId: number" src/server/services/totals.service.ts && grep -c "userId: number" src/server/services/csv.service.ts && grep "and(" src/server/services/item.service.ts | wc -l</automated>
</verify>
<acceptance_criteria>
- Every exported function in item.service.ts has `userId: number` parameter
- Every exported function in category.service.ts has `userId: number` parameter
- totals.service.ts functions have `userId: number` parameter
- csv.service.ts import/export functions have `userId: number` parameter
- `getItemById` uses `and(eq(items.id, id), eq(items.userId, userId))` (not just eq on id)
- `deleteCategory` uses `getOrCreateUncategorized(db, userId)` not hardcoded ID
- `importItemsCsv` scopes category operations to userId
- No `= prodDb` default parameter values remain
- `and` imported from `drizzle-orm` in all files that use it
</acceptance_criteria>
<done>Item, category, totals, and CSV services accept userId and scope all queries to the authenticated user. Get-by-id uses and() for isolation.</done>
</task>
<task type="auto">
<name>Task 2: Update thread, setup, settings, and auth services with userId scoping</name>
<files>src/server/services/thread.service.ts, src/server/services/setup.service.ts, src/server/services/auth.service.ts</files>
<read_first>src/server/services/thread.service.ts, src/server/services/setup.service.ts, src/server/services/auth.service.ts, src/db/schema.ts</read_first>
<action>
**thread.service.ts** per D-09 and Research pitfall 6:
- Add `userId: number` as second parameter to ALL exported functions
- Remove default `db` parameter values
- `getAllThreads`: add `.where(eq(threads.userId, userId))`
- `getThreadById`: use `and(eq(threads.id, id), eq(threads.userId, userId))`
- `createThread`: include `userId` in the insert values
- `updateThread`: add userId filter
- `deleteThread`: add userId filter
- `resolveThread`: CRITICAL -- verify the thread belongs to the user before resolving. When creating the new item from the winning candidate, include `userId` in the `insert(items).values({...})`. Also verify the target category belongs to the user. Use `getOrCreateUncategorized(db, userId)` if category fallback is needed.
- `addCandidate`: verify the parent thread belongs to the user before inserting candidate
- `updateCandidate`: verify the parent thread belongs to the user (join or subquery)
- `removeCandidate`: verify the parent thread belongs to the user
For candidate operations, the pattern should be:
1. Look up the thread with userId filter: `and(eq(threads.id, threadId), eq(threads.userId, userId))`
2. If thread not found, return null/throw (the thread doesn't exist for this user)
3. Proceed with candidate operation on the verified thread
**setup.service.ts** per D-09 and Research pitfall 8:
- Add `userId: number` as second parameter to ALL exported functions
- `getAllSetups`: add `.where(eq(setups.userId, userId))`
- `getSetupById`: use `and(eq(setups.id, id), eq(setups.userId, userId))`
- `createSetup`: include `userId` in insert values
- `updateSetup`: add userId filter
- `deleteSetup`: add userId filter
- `syncSetupItems`: CRITICAL -- verify the setup belongs to the user AND verify each itemId belongs to the user before inserting into setupItems. Filter the incoming item list against user-owned items:
```typescript
const userItemIds = await db.select({ id: items.id }).from(items)
.where(and(eq(items.userId, userId), inArray(items.id, itemIds)));
// Only insert items that belong to this user
```
**auth.service.ts** per D-07:
- `createApiKey`: add `userId: number` parameter, include userId in insert values
- `listApiKeys`: add `userId: number` parameter, filter by `.where(eq(apiKeys.userId, userId))`
- `deleteApiKey`: add `userId: number` parameter, filter by `and(eq(apiKeys.id, id), eq(apiKeys.userId, userId))` to prevent deleting another user's API key
- `verifyApiKey` was already updated in Plan 01 to return `{ userId } | null`
- `getOrCreateUser` was already created in Plan 01
</action>
<verify>
<automated>grep -c "userId: number" src/server/services/thread.service.ts && grep -c "userId: number" src/server/services/setup.service.ts && grep -c "userId: number" src/server/services/auth.service.ts && grep "and(" src/server/services/thread.service.ts | wc -l && grep "and(" src/server/services/setup.service.ts | wc -l</automated>
</verify>
<acceptance_criteria>
- Every exported function in thread.service.ts has `userId: number` parameter
- Every exported function in setup.service.ts has `userId: number` parameter
- `createApiKey`, `listApiKeys`, `deleteApiKey` in auth.service.ts have `userId: number` parameter
- `resolveThread` includes `userId` in the `insert(items).values({...})` call
- `resolveThread` verifies thread ownership before resolving
- Candidate operations (add, update, remove) verify parent thread ownership
- `syncSetupItems` verifies both setup and item ownership
- `getThreadById` uses `and(eq(threads.id, id), eq(threads.userId, userId))`
- `getSetupById` uses `and(eq(setups.id, id), eq(setups.userId, userId))`
- No `= prodDb` default parameter values remain
</acceptance_criteria>
<done>Thread, setup, and auth services accept userId and scope all queries. Thread resolution and setup sync validate ownership. Candidate operations verify parent thread belongs to user.</done>
</task>
</tasks>
<verification>
After all tasks complete:
1. `grep -r "userId: number" src/server/services/ | wc -l` shows userId parameter across all service files
2. `grep -r "= prodDb" src/server/services/` returns no matches (no default db params)
3. `grep -r "and(eq" src/server/services/` shows isolation on get-by-id queries
4. No service function reads or writes user-owned data without userId filtering
</verification>
<success_criteria>
- All 7 service files accept userId as a parameter
- All queries filter by userId (no unscoped reads or writes)
- Get-by-id, update, and delete operations use and() to combine id + userId conditions
- Thread resolution includes userId on new item creation
- Setup item sync validates item ownership
- Category deletion uses dynamic Uncategorized lookup (not hardcoded ID)
- CSV import scopes all operations to the importing user
- API key CRUD is user-scoped
</success_criteria>
<output>
After completion, create `.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md`
</output>

277
.planning/phases/16-multi-user-data-model/16-03-PLAN.md Normal file
View File

@@ -0,0 +1,277 @@
---
phase: 16-multi-user-data-model
plan: 03
type: execute
wave: 3
depends_on: ["16-01", "16-02"]
files_modified:
- src/server/routes/items.ts
- src/server/routes/categories.ts
- src/server/routes/threads.ts
- src/server/routes/setups.ts
- src/server/routes/settings.ts
- src/server/routes/totals.ts
- src/server/routes/auth.ts
- src/server/routes/images.ts
- src/server/mcp/index.ts
- src/server/mcp/tools/items.ts
- src/server/mcp/tools/categories.ts
- src/server/mcp/tools/threads.ts
- src/server/mcp/tools/setups.ts
- src/server/mcp/resources/collection.ts
autonomous: true
requirements:
- MULTI-02
- MULTI-05
- MULTI-06
must_haves:
truths:
- "Every route handler extracts userId from context and passes it to service functions"
- "Settings routes use userId for per-user settings"
- "MCP tools receive userId and pass it to service functions"
- "MCP server is created with userId from the authenticated session"
- "No route calls a service function without passing userId"
artifacts:
- path: "src/server/routes/items.ts"
provides: "User-scoped item routes"
contains: "c.get(\"userId\")"
- path: "src/server/routes/settings.ts"
provides: "Per-user settings routes"
contains: "c.get(\"userId\")"
- path: "src/server/mcp/index.ts"
provides: "MCP server with userId threading"
contains: "createMcpServer(db, userId)"
key_links:
- from: "src/server/routes/items.ts"
to: "src/server/services/item.service.ts"
via: "userId passed from context to service"
pattern: "getAllItems.*db.*userId"
- from: "src/server/mcp/index.ts"
to: "src/server/mcp/tools/items.ts"
via: "userId passed to registerItemTools"
pattern: "registerItemTools.*db.*userId"
---
<objective>
Wire userId from Hono context into all route handlers and MCP tool registrations, completing the multi-user data isolation chain.
Purpose: Routes are the HTTP boundary. They extract userId (set by requireAuth middleware in Plan 01) and pass it to services (updated in Plan 02). MCP tools are the programmatic boundary. Together they ensure every data operation is scoped to the authenticated user.
Output: All route files and MCP tools pass userId to service calls. Settings use per-user composite key.
</objective>
<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/phases/16-multi-user-data-model/16-CONTEXT.md
@.planning/phases/16-multi-user-data-model/16-RESEARCH.md
@.planning/phases/16-multi-user-data-model/16-01-SUMMARY.md
@.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md
@src/server/routes/items.ts
@src/server/routes/categories.ts
@src/server/routes/threads.ts
@src/server/routes/setups.ts
@src/server/routes/settings.ts
@src/server/routes/totals.ts
@src/server/routes/auth.ts
@src/server/routes/images.ts
@src/server/mcp/index.ts
@src/server/mcp/tools/items.ts
@src/server/mcp/tools/categories.ts
@src/server/mcp/tools/threads.ts
@src/server/mcp/tools/setups.ts
<interfaces>
<!-- From Plan 01 SUMMARY: requireAuth sets c.set("userId", userId) on context -->
<!-- From Plan 02 SUMMARY: all service functions now accept (db, userId, ...) -->
Route pattern (what every handler must do):
```typescript
app.get("/", async (c) => {
const db = c.get("db");
const userId = c.get("userId");
const result = await serviceFunction(db, userId, ...);
return c.json(result);
});
```
MCP pattern (what must change):
```typescript
// Before: createMcpServer(db: Db)
// After: createMcpServer(db: Db, userId: number)
// Before: registerItemTools(db)
// After: registerItemTools(db, userId)
```
Settings pattern (composite key):
```typescript
// Before: eq(settings.key, key)
// After: and(eq(settings.userId, userId), eq(settings.key, key))
// Insert with onConflict must target [settings.userId, settings.key]
```
</interfaces>
</context>
<tasks>
<task type="auto">
<name>Task 1: Update all route handlers to extract and pass userId</name>
<files>src/server/routes/items.ts, src/server/routes/categories.ts, src/server/routes/threads.ts, src/server/routes/setups.ts, src/server/routes/settings.ts, src/server/routes/totals.ts, src/server/routes/auth.ts, src/server/routes/images.ts</files>
<read_first>src/server/routes/items.ts, src/server/routes/categories.ts, src/server/routes/threads.ts, src/server/routes/setups.ts, src/server/routes/settings.ts, src/server/routes/totals.ts, src/server/routes/auth.ts, src/server/routes/images.ts</read_first>
<action>
For EVERY route handler in EVERY route file, add `const userId = c.get("userId");` after the `const db = c.get("db");` line, then pass `userId` to every service function call.
**items.ts**: Extract userId, pass to getAllItems(db, userId), getItemById(db, userId, id), createItem(db, userId, data), updateItem(db, userId, id, data), deleteItem(db, userId, id).
**categories.ts**: Extract userId, pass to getAllCategories(db, userId), getCategoryById(db, userId, id), createCategory(db, userId, data), updateCategory(db, userId, id, data), deleteCategory(db, userId, id).
**threads.ts**: Extract userId, pass to all thread service calls including addCandidate, updateCandidate, removeCandidate, resolveThread.
**setups.ts**: Extract userId, pass to all setup service calls including syncSetupItems.
**totals.ts**: Extract userId, pass to getTotals(db, userId) or equivalent.
**settings.ts** per D-06: This route does inline DB queries (no service file). Update to:
- GET `/:key`: Add userId to the where clause: `and(eq(settings.userId, userId), eq(settings.key, key))`
- PUT `/:key`: Update the upsert to use composite conflict target: `.onConflictDoUpdate({ target: [settings.userId, settings.key], set: { value: body.value } })` and include userId in the insert values: `.values({ userId, key, value: body.value })`
- Import `and` from `drizzle-orm` and `settings` from schema
**auth.ts**: Extract userId, pass to createApiKey(db, userId, name), listApiKeys(db, userId), deleteApiKey(db, userId, id). Auth routes that don't need userId (login, me, setup) can skip it.
**images.ts**: This route handles image uploads which don't directly involve userId scoping on the images table (images are stored by filename, not in a user-scoped table). However, if the route calls any service that now requires userId, pass it. Read the file first to determine what changes are needed.
IMPORTANT: The `Env` type annotation on each Hono app may need updating to include `userId` in the Variables type:
```typescript
type Env = { Variables: { db?: any; userId?: number } };
```
</action>
<verify>
<automated>for f in src/server/routes/items.ts src/server/routes/categories.ts src/server/routes/threads.ts src/server/routes/setups.ts src/server/routes/settings.ts src/server/routes/totals.ts src/server/routes/auth.ts; do echo "$f: $(grep -c 'c.get("userId")' $f)"; done</automated>
</verify>
<acceptance_criteria>
- Every route handler in items.ts, categories.ts, threads.ts, setups.ts, totals.ts, auth.ts contains `c.get("userId")`
- Every service function call includes userId as the second argument
- settings.ts uses `and(eq(settings.userId, userId), eq(settings.key, key))` for reads
- settings.ts upsert targets `[settings.userId, settings.key]` for composite conflict
- settings.ts insert includes userId in values
- Env type includes `userId` in Variables
- No service call is missing the userId parameter
</acceptance_criteria>
<done>All route handlers extract userId from context and pass to every service call. Settings routes use composite key.</done>
</task>
<task type="auto">
<name>Task 2: Update MCP server and tool registrations with userId</name>
<files>src/server/mcp/index.ts, src/server/mcp/tools/items.ts, src/server/mcp/tools/categories.ts, src/server/mcp/tools/threads.ts, src/server/mcp/tools/setups.ts, src/server/mcp/resources/collection.ts</files>
<read_first>src/server/mcp/index.ts, src/server/mcp/tools/items.ts, src/server/mcp/tools/categories.ts, src/server/mcp/tools/threads.ts, src/server/mcp/tools/setups.ts, src/server/mcp/resources/collection.ts</read_first>
<action>
Per D-13 and Research pitfall 5:
1. **Update `createMcpServer` signature** in `src/server/mcp/index.ts`:
Change from `createMcpServer(db: Db)` to `createMcpServer(db: Db, userId: number)`.
Pass userId to all `register*Tools` calls:
- `registerItemTools(db, userId)`
- `registerCategoryTools(db, userId)`
- `registerThreadTools(db, userId)`
- `registerSetupTools(db, userId)`
- `getCollectionSummary(db, userId)`
(registerImageTools has no db/userId dependency so leave unchanged)
2. **Update MCP auth middleware** to resolve userId:
The MCP auth middleware in `mcpRoutes.use("/*", ...)` currently calls `verifyAccessToken` and `verifyApiKey` which now return `{ userId } | null`. Store the userId and make it available to the POST handler.
Use the Hono context to pass userId, similar to the main API middleware:
```typescript
mcpRoutes.use("/*", async (c, next) => {
const db = c.get("db") ?? prodDb;
// Try Bearer token first (OAuth)
const authHeader = c.req.header("Authorization");
if (authHeader?.startsWith("Bearer ")) {
const token = authHeader.slice(7);
const result = await verifyAccessToken(db, token);
if (result) {
c.set("userId", result.userId);
return next();
}
return c.json({ error: "invalid_token" }, 401);
}
// Try API key
const apiKey = c.req.header("X-API-Key");
if (apiKey) {
const result = await verifyApiKey(db, apiKey);
if (result) {
c.set("userId", result.userId);
return next();
}
return c.json({ error: "Invalid API key" }, 401);
}
// ... rest of auth handling
});
```
3. **Update MCP POST handler** to pass userId when creating MCP server:
In the `mcpRoutes.post("/", ...)` handler, extract userId from context and pass to createMcpServer:
```typescript
const userId = c.get("userId");
const server = createMcpServer(db, userId);
```
4. **Store userId alongside transport** in the session map per Research pitfall 5:
Change `transports` map type from `Map<string, Transport>` to `Map<string, { transport: Transport, userId: number }>`.
When reusing an existing session, extract userId from the stored session data (no need to recreate MCP server -- the session was already initialized with the correct userId).
5. **Update each tool registration file** to accept and use userId:
- `src/server/mcp/tools/items.ts`: `registerItemTools(db: Db, userId: number)` -- pass userId to all item service calls
- `src/server/mcp/tools/categories.ts`: `registerCategoryTools(db: Db, userId: number)` -- pass userId to all category service calls
- `src/server/mcp/tools/threads.ts`: `registerThreadTools(db: Db, userId: number)` -- pass userId to all thread service calls
- `src/server/mcp/tools/setups.ts`: `registerSetupTools(db: Db, userId: number)` -- pass userId to all setup service calls
6. **Update `getCollectionSummary`** in `src/server/mcp/resources/collection.ts`:
Add userId parameter, scope the summary queries to the user's data only.
</action>
<verify>
<automated>grep -c "userId: number" src/server/mcp/index.ts && grep "createMcpServer(db, userId)" src/server/mcp/index.ts | wc -l && grep -c "userId: number" src/server/mcp/tools/items.ts && grep -c "userId: number" src/server/mcp/tools/categories.ts && grep -c "userId: number" src/server/mcp/tools/threads.ts && grep -c "userId: number" src/server/mcp/tools/setups.ts && grep -c "c.set(\"userId\"" src/server/mcp/index.ts</automated>
</verify>
<acceptance_criteria>
- `createMcpServer` accepts `(db: Db, userId: number)` signature
- MCP auth middleware sets `c.set("userId", result.userId)` for both API key and Bearer token auth
- MCP POST handler passes userId to `createMcpServer(db, userId)`
- Transport map stores userId alongside transport
- All 4 tool registration functions accept `(db: Db, userId: number)`
- All tool handlers pass userId to service function calls
- `getCollectionSummary` accepts and uses userId
- No MCP tool calls a service function without userId
</acceptance_criteria>
<done>MCP server creation receives userId, all tool registrations pass userId to service calls, MCP auth middleware resolves userId from API key or Bearer token</done>
</task>
</tasks>
<verification>
After all tasks complete:
1. `grep -r 'c.get("userId")' src/server/routes/ | wc -l` shows userId extraction in all route files
2. `grep -r 'c.get("userId")' src/server/mcp/ | wc -l` shows userId in MCP middleware
3. `grep "createMcpServer(db, userId)" src/server/mcp/index.ts` confirms MCP userId threading
4. No service call anywhere in routes/ or mcp/ is missing the userId argument
</verification>
<success_criteria>
- All route handlers extract userId from context and pass to services
- Settings routes use composite PK for per-user settings
- MCP server creation includes userId
- MCP tool registrations pass userId to all service calls
- MCP auth middleware resolves userId from API key and Bearer token
- Complete chain: middleware sets userId -> routes/MCP extract it -> services filter by it
</success_criteria>
<output>
After completion, create `.planning/phases/16-multi-user-data-model/16-03-SUMMARY.md`
</output>

320
.planning/phases/16-multi-user-data-model/16-04-PLAN.md Normal file
View File

@@ -0,0 +1,320 @@
---
phase: 16-multi-user-data-model
plan: 04
type: execute
wave: 3
depends_on: ["16-01", "16-02"]
files_modified:
- tests/services/item.service.test.ts
- tests/services/category.service.test.ts
- tests/services/thread.service.test.ts
- tests/services/setup.service.test.ts
- tests/services/totals.test.ts
- tests/services/csv.service.test.ts
- tests/services/auth.service.test.ts
- tests/services/oauth.service.test.ts
- tests/routes/items.test.ts
- tests/routes/categories.test.ts
- tests/routes/threads.test.ts
- tests/routes/setups.test.ts
- tests/routes/auth.test.ts
- tests/routes/images.test.ts
- tests/routes/oauth.test.ts
- tests/routes/params.test.ts
- tests/mcp/tools.test.ts
autonomous: true
requirements:
- MULTI-02
- MULTI-04
- MULTI-05
must_haves:
truths:
- "All existing tests pass after updating to use { db, userId } from createTestDb"
- "Service tests pass userId to every service function call"
- "Route tests set userId on the test app context"
- "MCP tests pass userId to MCP server creation"
- "At least one cross-user isolation test exists proving User A cannot see User B's data"
artifacts:
- path: "tests/services/item.service.test.ts"
provides: "User-scoped item service tests"
contains: "userId"
- path: "tests/mcp/tools.test.ts"
provides: "User-scoped MCP tool tests"
contains: "userId"
key_links:
- from: "tests/helpers/db.ts"
to: "tests/services/*.test.ts"
via: "createTestDb returns { db, userId }"
pattern: "const \\{ db, userId \\}"
- from: "tests/services/item.service.test.ts"
to: "src/server/services/item.service.ts"
via: "passes userId to all service calls"
pattern: "getAllItems.*db.*userId"
---
<objective>
Update all test files to work with the new multi-user data model: destructure { db, userId } from createTestDb(), pass userId to all service calls, set userId in route test contexts, and add cross-user isolation tests.
Purpose: Tests validate that multi-user isolation works correctly. Without updated tests, we cannot verify that MULTI-02 (cross-user isolation) is enforced. The test suite must pass green before the phase is complete.
Output: All 17 test files updated, cross-user isolation tests added, full test suite passes.
</objective>
<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/phases/16-multi-user-data-model/16-CONTEXT.md
@.planning/phases/16-multi-user-data-model/16-RESEARCH.md
@.planning/phases/16-multi-user-data-model/16-01-SUMMARY.md
@.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md
@tests/helpers/db.ts
@tests/services/item.service.test.ts
@tests/services/category.service.test.ts
@tests/routes/items.test.ts
@tests/mcp/tools.test.ts
<interfaces>
<!-- From Plan 01: createTestDb() now returns { db, userId } -->
<!-- From Plan 01: createSecondTestUser(db) creates user 2 for isolation tests -->
<!-- From Plan 02: all service functions now accept (db, userId, ...) -->
<!-- From Plan 03: routes expect userId on context, MCP expects userId -->
Test update pattern for service tests:
```typescript
// Before:
let db: any;
beforeEach(async () => { db = await createTestDb(); });
// ...
const items = await getAllItems(db);
// After:
let db: any;
let userId: number;
beforeEach(async () => { ({ db, userId } = await createTestDb()); });
// ...
const items = await getAllItems(db, userId);
```
Test update pattern for route tests:
```typescript
// Before:
const testApp = new Hono();
testApp.use("*", async (c, next) => { c.set("db", db); await next(); });
// After:
const testApp = new Hono();
testApp.use("*", async (c, next) => { c.set("db", db); c.set("userId", userId); await next(); });
```
Cross-user isolation test pattern:
```typescript
import { createSecondTestUser } from "../helpers/db";
test("user cannot see other user's items", async () => {
const userId2 = await createSecondTestUser(db);
await createItem(db, userId, { name: "User 1 Item", ... });
await createItem(db, userId2, { name: "User 2 Item", ... });
const user1Items = await getAllItems(db, userId);
const user2Items = await getAllItems(db, userId2);
expect(user1Items).toHaveLength(1);
expect(user1Items[0].name).toBe("User 1 Item");
expect(user2Items).toHaveLength(1);
expect(user2Items[0].name).toBe("User 2 Item");
});
```
</interfaces>
</context>
<tasks>
<task type="auto">
<name>Task 1: Update all service test files to pass userId</name>
<files>tests/services/item.service.test.ts, tests/services/category.service.test.ts, tests/services/thread.service.test.ts, tests/services/setup.service.test.ts, tests/services/totals.test.ts, tests/services/csv.service.test.ts, tests/services/auth.service.test.ts, tests/services/oauth.service.test.ts</files>
<read_first>tests/services/item.service.test.ts, tests/services/category.service.test.ts, tests/services/thread.service.test.ts, tests/services/setup.service.test.ts, tests/services/totals.test.ts, tests/services/csv.service.test.ts, tests/services/auth.service.test.ts, tests/services/oauth.service.test.ts, tests/helpers/db.ts</read_first>
<action>
For EACH of the 8 service test files, apply this systematic transformation:
1. **Change createTestDb destructuring** from `db = await createTestDb()` to `({ db, userId } = await createTestDb())`. Add `userId` variable declaration alongside `db`.
2. **Add userId to every service function call** as the second argument after `db`. Go through every call to any service function in each test and add `userId`.
3. **Fix category references**: Tests that reference "Uncategorized" by hardcoded ID 1 must instead look up the category by name+userId or use the ID returned from the seeded data. The test helper already seeds Uncategorized with the test user's ID.
4. **Specific file notes:**
**item.service.test.ts** (~160 lines):
- Destructure `{ db, userId }` from createTestDb
- Every `getAllItems(db)` becomes `getAllItems(db, userId)`
- Every `getItemById(db, id)` becomes `getItemById(db, userId, id)`
- Every `createItem(db, data)` becomes `createItem(db, userId, data)`
- Every `updateItem(db, id, data)` becomes `updateItem(db, userId, id, data)`
- Every `deleteItem(db, id)` becomes `deleteItem(db, userId, id)`
- ADD a cross-user isolation test: create a second user with `createSecondTestUser(db)`, create items for each user, verify each user only sees their own items, and verify getItemById returns null for another user's item ID
**category.service.test.ts** (~97 lines):
- Same destructuring pattern
- Add userId to all category service calls
- ADD a test for composite unique constraint: two users can have categories with the same name
- Fix any hardcoded "Uncategorized" ID references
**thread.service.test.ts** (~523 lines):
- Same destructuring pattern
- Add userId to ALL thread, candidate, and resolve calls
- Candidate operations need userId (for parent thread verification)
- Thread resolution test: verify the created item has the correct userId
- ADD a cross-user isolation test for threads
**setup.service.test.ts** (~293 lines):
- Same destructuring pattern
- Add userId to all setup service calls
- syncSetupItems calls need userId
- ADD a test that verifies a user cannot add another user's items to their setup
**totals.test.ts** (~79 lines):
- Same destructuring pattern
- Add userId to getTotals calls
**csv.service.test.ts** (~196 lines):
- Same destructuring pattern
- Add userId to import/export calls
- CSV import must create categories with userId
**auth.service.test.ts** (~68 lines):
- Same destructuring pattern
- Add userId to createApiKey, listApiKeys, deleteApiKey calls
- Update verifyApiKey assertions to check for `{ userId }` return instead of boolean `true`
**oauth.service.test.ts** (~290 lines):
- Same destructuring pattern
- Add userId where needed (createTokens, verifyAccessToken)
- Update verifyAccessToken assertions to check for `{ userId }` return instead of boolean
5. **Import `createSecondTestUser`** from helpers/db.ts in files that add isolation tests (at minimum: item, category, thread, setup test files).
After all files are updated, run `bun test` to verify the full suite passes.
</action>
<verify>
<automated>bun test tests/services/ 2>&1 | tail -5</automated>
</verify>
<acceptance_criteria>
- All 8 service test files use `{ db, userId } = await createTestDb()`
- Every service function call in tests includes userId argument
- `item.service.test.ts` has a cross-user isolation test using `createSecondTestUser`
- `category.service.test.ts` has a composite unique constraint test (same name, different users)
- `auth.service.test.ts` checks `verifyApiKey` returns `{ userId }` not boolean
- `oauth.service.test.ts` checks `verifyAccessToken` returns `{ userId }` not boolean
- `bun test tests/services/` passes all tests
</acceptance_criteria>
<done>All 8 service test files updated with userId, cross-user isolation tests added, all service tests pass</done>
</task>
<task type="auto">
<name>Task 2: Update route tests, MCP tests, and run full suite</name>
<files>tests/routes/items.test.ts, tests/routes/categories.test.ts, tests/routes/threads.test.ts, tests/routes/setups.test.ts, tests/routes/auth.test.ts, tests/routes/images.test.ts, tests/routes/oauth.test.ts, tests/routes/params.test.ts, tests/mcp/tools.test.ts</files>
<read_first>tests/routes/items.test.ts, tests/routes/categories.test.ts, tests/routes/threads.test.ts, tests/routes/setups.test.ts, tests/routes/auth.test.ts, tests/routes/images.test.ts, tests/routes/oauth.test.ts, tests/routes/params.test.ts, tests/mcp/tools.test.ts</read_first>
<action>
**Route tests** (7 files):
Route tests create a test Hono app with middleware that sets `db` on context. They need TWO changes:
1. **Destructure `{ db, userId }`** from createTestDb
2. **Set userId on the test app context** in the middleware setup:
```typescript
testApp.use("*", async (c, next) => {
c.set("db", db);
c.set("userId", userId);
await next();
});
```
For each route test file, read the file first to understand its middleware setup pattern, then add `c.set("userId", userId)` to the middleware. The route handlers will then pick it up via `c.get("userId")`.
**Specific notes per file:**
**items.test.ts** (~207 lines): Set userId in context middleware. Tests should work as-is since routes now pass userId to services.
**categories.test.ts** (~91 lines): Set userId in context middleware.
**threads.test.ts** (~413 lines): Set userId in context middleware. Thread tests may reference hardcoded Uncategorized category ID -- fix these.
**setups.test.ts** (~305 lines): Set userId in context middleware.
**auth.test.ts** (~139 lines): Set userId in context middleware for API key management routes. Auth-specific routes (login, setup) may not need userId.
**images.test.ts** (~26 lines): Set userId in context middleware if needed.
**oauth.test.ts** (~443 lines): May not need userId for OAuth flow tests, but set it for consistency. OAuth token creation may need userId.
**params.test.ts** (~81 lines): Set userId in context middleware.
**MCP tests** (1 file):
**tools.test.ts** (~253 lines):
- Destructure `{ db, userId }` from createTestDb
- Update `createMcpServer(db)` calls to `createMcpServer(db, userId)`
- MCP tool calls should then work since tools now receive userId from the server
- ADD an isolation test: create data as user 1, verify MCP tools as user 2 don't return user 1's data
After ALL test files are updated, run the FULL test suite:
```bash
bun test
```
Fix any remaining failures. Common issues to watch for:
- Missing userId argument (TypeScript will flag this)
- Hardcoded category ID 1 (use the seeded category from createTestDb)
- Boolean vs object return type from verifyApiKey/verifyAccessToken
- Settings tests using single key lookup instead of composite
- Route tests not setting userId in context
Also run lint to ensure no formatting issues:
```bash
bun run lint
```
</action>
<verify>
<automated>bun test 2>&1 | tail -10</automated>
</verify>
<acceptance_criteria>
- All route test files set `c.set("userId", userId)` in test middleware
- All route test files use `{ db, userId } = await createTestDb()`
- MCP tools.test.ts passes userId to createMcpServer
- MCP tools.test.ts has a cross-user isolation test
- `bun test` passes ALL tests (0 failures)
- `bun run lint` passes
</acceptance_criteria>
<done>All 17 test files updated, full test suite passes green, lint passes, cross-user isolation verified by tests</done>
</task>
</tasks>
<verification>
After all tasks complete:
1. `bun test` exits with 0 failures
2. `bun run lint` passes
3. `grep -r "createSecondTestUser" tests/` shows isolation tests exist
4. `grep -r 'c.set("userId"' tests/routes/` shows userId set in all route test middleware
5. `grep "createMcpServer(db, userId)" tests/mcp/tools.test.ts` confirms MCP test update
</verification>
<success_criteria>
- Full test suite passes (`bun test` returns 0 failures)
- All service tests pass userId to every service call
- All route tests set userId in test app context
- MCP tests pass userId to createMcpServer
- Cross-user isolation tests exist for items, categories, threads, setups
- Composite unique constraint test exists for categories
- Lint passes
</success_criteria>
<output>
After completion, create `.planning/phases/16-multi-user-data-model/16-04-SUMMARY.md`
</output>