---
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>