18 KiB
phase, verified, status, score, gaps
| phase | verified | status | score | gaps | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 16-multi-user-data-model | 2026-04-04T00:00:00Z | gaps_found | 5/8 must-haves verified |
|
Phase 16: Multi-User Data Model Verification Report
Phase Goal: Every piece of user-created data is owned by a specific user, with complete isolation between users Verified: 2026-04-04 Status: gaps_found Re-verification: No — initial verification
Goal Achievement
Observable Truths
| # | Truth | Status | Evidence |
|---|---|---|---|
| 1 | A users table exists with id (serial PK), logtoSub (text unique), createdAt (timestamp) | VERIFIED | src/db/schema.ts lines 14-18: export const users = pgTable("users", { id: serial("id").primaryKey(), logtoSub: text("logto_sub").notNull().unique(), createdAt: timestamp("created_at").defaultNow().notNull() }) |
| 2 | Every entity table (items, categories, threads, setups, settings, apiKeys, oauthTokens) has a userId integer FK column | VERIFIED | schema.ts has userId: integer("user_id").notNull().references(() => users.id) on items, categories, threads, setups, settings, apiKeys, oauthTokens — 9 userId occurrences confirmed |
| 3 | Categories have composite unique on (userId, name); settings have composite PK on (userId, key) | VERIFIED | (table) => [unique().on(table.userId, table.name)] in categories; (table) => [primaryKey({ columns: [table.userId, table.key] })] in settings |
| 4 | requireAuth middleware resolves userId and sets it on Hono context | VERIFIED | src/server/middleware/auth.ts calls c.set("userId", result.userId) in API-key path, Bearer path, and c.set("userId", user.id) in OIDC path |
| 5 | All service functions accept userId; all queries filter by userId using and(eq) | VERIFIED | All 7 service files accept userId: number; and(eq(table.id, id), eq(table.userId, userId)) confirmed in item, category, thread, setup services |
| 6 | Routes extract userId from context and pass to services; MCP tools receive userId | VERIFIED | 36 c.get("userId") calls across 7 route files; createMcpServer(db, userId) confirmed in src/server/mcp/index.ts line 20 |
| 7 | createTestDb returns { db, userId } with seeded user and per-user Uncategorized category | VERIFIED | tests/helpers/db.ts inserts user, inserts Uncategorized with userId, returns { db, userId: user.id } |
| 8 | All API routes require auth (no GET bypass) so userId is always available | VERIFIED | src/server/index.ts line 65-77: requireAuth applied to /api/*, /api/auth and /api/health bypassed, no GET method check present |
Score per observable truths: 8/8 truths structurally VERIFIED
Required Artifacts
| Artifact | Expected | Status | Details |
|---|---|---|---|
src/db/schema.ts |
Users table + userId columns on all entity tables + composite constraints | VERIFIED | pg-core imports, users table, 9 userId columns, composite unique + PK |
tests/helpers/db.ts |
Test DB with seeded user | VERIFIED | Returns { db, userId }, has createSecondTestUser helper |
src/server/middleware/auth.ts |
userId resolution middleware | VERIFIED | c.set("userId", ...) in all 3 auth paths |
src/server/services/item.service.ts |
User-scoped item CRUD | VERIFIED | userId: number on all 6 functions, and(eq) isolation |
src/server/services/category.service.ts |
User-scoped category CRUD | VERIFIED | userId: number on all functions, composite unique respected |
src/server/services/thread.service.ts |
User-scoped thread + candidate CRUD + resolution | VERIFIED | 10 userId: number occurrences, resolveThread inserts item with userId |
src/server/services/setup.service.ts |
User-scoped setup CRUD + item sync validation | VERIFIED | 8 userId: number occurrences, syncSetupItems validates item ownership |
src/server/services/totals.service.ts |
User-scoped aggregate queries | VERIFIED | userId: number parameter, queries filter by userId |
src/server/services/csv.service.ts |
User-scoped CSV import/export | VERIFIED | userId: number parameter on import/export |
src/server/routes/items.ts |
User-scoped item routes | VERIFIED | 8 c.get("userId") calls |
src/server/routes/settings.ts |
Per-user settings routes | VERIFIED | and(eq(settings.userId, userId), eq(settings.key, key)) and composite conflict target |
src/server/mcp/index.ts |
MCP server with userId threading | VERIFIED | createMcpServer(db, userId), MCP auth sets c.set("userId", ...) |
tests/services/item.service.test.ts |
User-scoped item service tests | VERIFIED | Destructures { db, userId }, all calls include userId, cross-user isolation tests pass |
tests/mcp/tools.test.ts |
User-scoped MCP tool tests | STUB/BROKEN | createTestDb() not awaited — all 18 tests fail at runtime |
Key Link Verification
| From | To | Via | Status | Details |
|---|---|---|---|---|
src/server/middleware/auth.ts |
src/server/services/auth.service.ts |
verifyApiKey returning { userId } |
VERIFIED | verifyApiKey returns Promise<{ userId: number } | null>, middleware calls c.set("userId", result.userId) |
src/server/middleware/auth.ts |
src/server/services/oauth.service.ts |
verifyAccessToken returning { userId } |
VERIFIED | verifyAccessToken returns Promise<{ userId: number } | null> |
src/server/services/thread.service.ts |
src/db/schema.ts |
userId on insert(items) during thread resolution | VERIFIED | resolveThread line 349: userId, in insert(items).values(...) |
src/server/services/setup.service.ts |
src/db/schema.ts |
validates item ownership before sync | VERIFIED | syncSetupItems uses inArray + eq(items.userId, userId) to validate item ownership |
src/server/services/category.service.ts |
src/db/schema.ts |
getOrCreateUncategorized uses composite unique |
VERIFIED | Function exists in category.service.ts, uses and(eq(categories.userId, userId), eq(categories.name, "Uncategorized")) |
src/server/routes/items.ts |
src/server/services/item.service.ts |
userId passed from context to service | VERIFIED | c.get("userId") in all 8 handlers, passed to getAllItems(db, userId) etc. |
src/server/mcp/index.ts |
src/server/mcp/tools/items.ts |
userId passed to registerItemTools | VERIFIED | registerItemTools(db, userId) called in createMcpServer |
tests/helpers/db.ts |
tests/services/*.test.ts |
createTestDb returns { db, userId } |
VERIFIED | All service tests correctly await createTestDb() and destructure { db, userId } |
tests/helpers/db.ts |
tests/routes/*.test.ts |
createTestDb returns { db, userId } |
BROKEN | 7 of 8 route test files call createTestDb() without await — db is a Promise |
tests/helpers/db.ts |
tests/mcp/tools.test.ts |
createTestDb returns { db, userId } |
BROKEN | createTestDb() not awaited — all MCP tests fail |
Data-Flow Trace (Level 4)
Applies to route and MCP layers that render dynamic user data.
| Artifact | Data Variable | Source | Produces Real Data | Status |
|---|---|---|---|---|
src/server/routes/items.ts |
items list | getAllItems(db, userId) → drizzle SELECT with eq(items.userId, userId) |
Yes | FLOWING |
src/server/routes/settings.ts |
setting value | eq(settings.userId, userId) in WHERE clause |
Yes | FLOWING |
src/server/mcp/tools/items.ts |
items list | getAllItems(db, userId) — userId from closure in createMcpServer |
Yes | FLOWING |
src/server/mcp/resources/collection.ts |
summary totals | getGlobalTotals(db, userId) |
Yes | FLOWING |
Behavioral Spot-Checks
Service-level tests run because createTestDb() is properly awaited in service tests.
| Behavior | Command | Result | Status |
|---|---|---|---|
| Item service — user isolation | bun test tests/services/item.service.test.ts |
13 pass, 0 fail | PASS |
| Category service — composite unique | bun test tests/services/category.service.test.ts |
9 pass, 0 fail | PASS |
| Thread service — resolve with userId | bun test tests/services/thread.service.test.ts |
34 pass, 0 fail | PASS |
| Setup service — item ownership validation | bun test tests/services/setup.service.test.ts |
20 pass, 0 fail | PASS |
| Totals service — user-scoped aggregates | bun test tests/services/totals.test.ts |
4 pass, 0 fail | PASS |
| CSV service — user-scoped import/export | bun test tests/services/csv.service.test.ts |
15 pass, 0 fail | PASS |
| Auth service — verifyApiKey returns { userId } | bun test tests/services/auth.service.test.ts |
5 pass, 0 fail | PASS |
| OAuth service — verifyAccessToken returns { userId } | bun test tests/services/oauth.service.test.ts |
12 pass, 0 fail | PASS |
| Item route tests | bun test tests/routes/items.test.ts |
2 pass, 9 fail | FAIL |
| Category route tests | bun test tests/routes/categories.test.ts |
0 pass, 4 fail | FAIL |
| Thread route tests | bun test tests/routes/threads.test.ts |
1 pass, 18 fail | FAIL |
| Setup route tests | bun test tests/routes/setups.test.ts |
1 pass, 12 fail | FAIL |
| Auth route tests | bun test tests/routes/auth.test.ts |
0 pass, 7 fail | FAIL |
| MCP tool tests (all) | bun test tests/mcp/tools.test.ts |
0 pass, 18 fail | FAIL |
Root cause of all route and MCP test failures: createTestDb() is an async function but is called without await in 7 route test files and tests/mcp/tools.test.ts. The destructuring const { db, userId } = createTestDb() assigns the raw Promise to db and undefined to userId, causing every DB operation to throw TypeError: undefined is not an object (evaluating 'db.select').
The same issue affects createSecondTestUser(db) and getCollectionSummary(db, userId) calls in tests/mcp/tools.test.ts.
tests/routes/oauth.test.ts is the only route test that correctly uses await createTestDb().
Requirements Coverage
All 6 requirement IDs from plan frontmatter (MULTI-01 through MULTI-06) are in scope for Phase 16.
| Requirement | Source Plan | Description | Status | Evidence |
|---|---|---|---|---|
| MULTI-01 | 16-01, 16-02 | Every item, category, thread, and setup is owned by a specific user | SATISFIED | userId FK on items, categories, threads, setups in schema; all service functions scope by userId |
| MULTI-02 | 16-02, 16-03, 16-04 | User can only see and modify their own data (cross-user isolation) | PARTIAL | Service layer isolation verified by passing tests; route and MCP layer tested but test suite fails due to missing await — isolation logic in source code is correct |
| MULTI-03 | 16-01, 16-02 | Categories use composite unique constraint (userId + name) | SATISFIED | unique().on(table.userId, table.name) in schema; getOrCreateUncategorized respects it; category composite unique test passes |
| MULTI-04 | 16-01, 16-04 | Existing data is assigned to the original user during migration | SATISFIED | Migration (drizzle-pg/0000_thankful_loners.sql) exists with users table and userId columns; test infrastructure seeds user; no old single-user data to migrate (greenfield for multi-user) |
| MULTI-05 | 16-03, 16-04 | MCP tools operate within the authenticated user's scope | PARTIAL | Source code: createMcpServer(db, userId) and all tool registrations pass userId — correct. Tests: all 18 MCP tests fail due to missing await; MCP isolation tests do not run |
| MULTI-06 | 16-01, 16-02, 16-03 | Settings are per-user rather than global | SATISFIED | Settings table has composite PK [userId, key]; settings routes use and(eq(settings.userId, userId), eq(settings.key, key)) and composite conflict target |
Note on REQUIREMENTS.md state: The file marks MULTI-01, MULTI-03, MULTI-06 as unchecked ([ ]) and MULTI-02, MULTI-04, MULTI-05 as checked ([x]). The implementation in the codebase satisfies all six at the source-code level. The checkbox state reflects that tests have not been fully verified green. This verification agrees — source-code satisfaction is confirmed, but test verification is blocked by the missing-await bug.
Anti-Patterns Found
| File | Line(s) | Pattern | Severity | Impact |
|---|---|---|---|---|
tests/mcp/tools.test.ts |
17, 25, 41, 53, 61, 79, 89, 103, 115, 135, 182, 192, 214, 226, 257, 288, 308, 332 | createTestDb() called without await — async function result not awaited |
BLOCKER | All 18 MCP tests fail |
tests/mcp/tools.test.ts |
258, 289, 309, 333 | createSecondTestUser(db) called without await |
BLOCKER | MCP isolation tests fail; userId2 is a Promise |
tests/mcp/tools.test.ts |
342, 343 | getCollectionSummary(db, userId) called without await |
BLOCKER | Collection summary isolation test fails |
tests/routes/items.test.ts |
8 | createTestDb() not awaited in createTestApp() |
BLOCKER | All 9 item route tests fail with 500 |
tests/routes/categories.test.ts |
8 | createTestDb() not awaited |
BLOCKER | All 4 category route tests fail |
tests/routes/threads.test.ts |
7 | createTestDb() not awaited |
BLOCKER | 18 of 19 thread route tests fail |
tests/routes/setups.test.ts |
8 | createTestDb() not awaited |
BLOCKER | 12 of 13 setup route tests fail |
tests/routes/auth.test.ts |
7 | createTestDb() not awaited |
BLOCKER | All 7 auth route tests fail |
tests/routes/images.test.ts |
6 | createTestDb() called at module top-level without await |
BLOCKER | Images test db is a Promise |
tests/routes/params.test.ts |
10 | createTestDb() not awaited |
BLOCKER | Route params tests fail |
Human Verification Required
None — all remaining gaps are code-level issues verifiable programmatically.
Gaps Summary
The entire source-code implementation is correct and complete. The multi-user data model foundation is structurally sound:
- Schema uses pg-core with a users table and userId FKs on all 6 entity tables
- Composite unique constraint on
categories(userId, name)and composite PK onsettings(userId, key)are correct - Auth middleware resolves userId for all three auth methods and removes the GET bypass
- All 7 service files accept userId and use
and(eq)isolation on every get/update/delete query - Thread resolution inserts new items with userId; setup sync validates item ownership
- All routes extract userId from context via
c.get("userId") - MCP server creation and tool registrations correctly thread userId
The single root cause blocking goal achievement: 7 route test files and tests/mcp/tools.test.ts call createTestDb() without await. Since createTestDb() is an async function that performs DB migration and seeding, omitting await means db receives the raw Promise object rather than the resolved Drizzle instance. Every database call then throws TypeError: undefined is not an object, causing 500 responses in route tests and TypeErrors in MCP tests.
Additionally, within tests/mcp/tools.test.ts, createSecondTestUser(db) and getCollectionSummary(db, userId) are also not awaited, breaking the 4 cross-user isolation tests in that suite.
Service-level tests (8 files) all pass because they correctly use await createTestDb(). The service-level cross-user isolation tests for items, categories, threads, and setups all pass, confirming the isolation logic works correctly at the service layer.
Fix required: In each affected test file, convert createTestDb() calls to use await. For files that use it inside synchronous helper functions (like createTestApp()), either make the helper async and await the call, or move the setup into a beforeEach(async () => {...}) block.
Verified: 2026-04-04 Verifier: Claude (gsd-verifier)