Files
GearBox/.planning/phases/16-multi-user-data-model/16-VERIFICATION.md
Jean-Luc Makiola 4ccbb2b070
Some checks failed
CI / ci (push) Failing after 1m44s
CI / e2e (push) Has been skipped
CI / deploy (push) Has been skipped
fix: wire catalog add buttons, fix Trans bold rendering, lint cleanup
- CatalogSearchOverlay: replace handleAddStub with real openAddToCollection/openAddToThread routing based on catalogSearchMode
- ConfirmDialog + __root.tsx: swap t() for Trans component on deleteItemMessage, deleteCandidateMessage, pickWinnerMessage — fixes <bold> rendering as literal text
- Biome format pass: fix 23 lint/format errors across scripts, services, tests
- Planning: mark all UAT and verification gaps resolved for phases 07, 11, 16, 20, 21, 22, 24, 32, 34; close debug sessions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-19 15:36:16 +02:00

18 KiB

phase, verified, status, score, gaps
phase verified status score gaps
16-multi-user-data-model 2026-04-04T00:00:00Z deferred 5/8 must-haves verified
truth status reason artifacts missing
All existing tests pass after updating to use { db, userId } from createTestDb failed 7 route test files and the MCP tools test file call createTestDb() without await, so db and userId are unresolved Promises — all route tests and all MCP tests return 500 or TypeError
path issue
tests/routes/items.test.ts createTestDb() not awaited in createTestApp() — db is a Promise, not a DB instance
path issue
tests/routes/categories.test.ts createTestDb() not awaited
path issue
tests/routes/threads.test.ts createTestDb() not awaited
path issue
tests/routes/setups.test.ts createTestDb() not awaited
path issue
tests/routes/auth.test.ts createTestDb() not awaited
path issue
tests/routes/images.test.ts createTestDb() not awaited (top-level call)
path issue
tests/routes/params.test.ts createTestDb() not awaited
path issue
tests/mcp/tools.test.ts createTestDb() not awaited AND createSecondTestUser() not awaited AND getCollectionSummary() not awaited — all 18 MCP tests fail
Add await to createTestDb() call in all 7 affected route test files (move to async function or beforeEach)
Add await to createSecondTestUser() calls in tests/mcp/tools.test.ts lines 258, 289, 309, 333
Add await to getCollectionSummary() calls in tests/mcp/tools.test.ts lines 342, 343
truth status reason artifacts missing
At least one cross-user isolation test exists proving User A cannot see User B's data partial Cross-user isolation tests exist in service tests (item, category, thread, setup) and all pass. However, the MCP isolation tests all fail due to missing await, so MCP-layer isolation is not verified by test suite
path issue
tests/mcp/tools.test.ts 4 cross-user isolation tests in MCP suite fail due to missing await on createTestDb/createSecondTestUser/getCollectionSummary
Fix await issue in MCP tools test — isolation logic is correct but async calls not awaited
truth status reason artifacts missing
MCP tests pass userId to MCP server creation failed MCP tools tests do not use createMcpServer; they call registerXTools(db, userId) directly. The underlying registration is correct but all 18 tests fail at runtime due to unresolved Promises from missing await on createTestDb()
path issue
tests/mcp/tools.test.ts createTestDb() not awaited — db resolves to Promise<{db, userId}>, not the actual DB object
Wrap test setup in async functions or beforeEach with await createTestDb()

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

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 on settings(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)