From 884bec0b35ed11d9cc487ab7b5e0ec22f26ef7b3 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Sun, 5 Apr 2026 10:45:30 +0200 Subject: [PATCH] docs(16-02): complete service layer userId scoping plan - SUMMARY.md documents 7 service files updated with userId parameter - STATE.md advanced to plan 2 of 4 in phase 16 - ROADMAP.md updated with plan progress - Requirements MULTI-01, MULTI-02, MULTI-03, MULTI-06 marked complete --- .planning/REQUIREMENTS.md | 8 +- .planning/ROADMAP.md | 4 +- .planning/STATE.md | 33 ++-- .../16-multi-user-data-model/16-02-SUMMARY.md | 146 ++++++++++++++++++ 4 files changed, 171 insertions(+), 20 deletions(-) create mode 100644 .planning/phases/16-multi-user-data-model/16-02-SUMMARY.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 7843ceb..06481ee 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -26,8 +26,8 @@ Requirements for this milestone. Each maps to roadmap phases. ### Multi-User Data Model - [x] **MULTI-01**: Every item, category, thread, and setup is owned by a specific user -- [ ] **MULTI-02**: User can only see and modify their own data (cross-user isolation) -- [ ] **MULTI-03**: Categories use composite unique constraint (userId + name) +- [x] **MULTI-02**: User can only see and modify their own data (cross-user isolation) +- [x] **MULTI-03**: Categories use composite unique constraint (userId + name) - [x] **MULTI-04**: Existing data is assigned to the original user during migration - [ ] **MULTI-05**: MCP tools operate within the authenticated user's scope - [x] **MULTI-06**: Settings are per-user rather than global @@ -127,8 +127,8 @@ Which phases cover which requirements. Updated during roadmap creation. | AUTH-04 | Phase 15 | Pending | | AUTH-05 | Phase 15 | Pending | | MULTI-01 | Phase 16 | Complete | -| MULTI-02 | Phase 16 | Pending | -| MULTI-03 | Phase 16 | Pending | +| MULTI-02 | Phase 16 | Complete | +| MULTI-03 | Phase 16 | Complete | | MULTI-04 | Phase 16 | Complete | | MULTI-05 | Phase 16 | Pending | | MULTI-06 | Phase 16 | Complete | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index c801ddd..e79cbeb 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -148,7 +148,7 @@ Plans: **Plans**: 4 plans Plans: - [x] 16-01-PLAN.md — Schema foundation: users table, userId columns, auth middleware, test helper -- [ ] 16-02-PLAN.md — Service layer userId scoping +- [x] 16-02-PLAN.md — Service layer userId scoping - [ ] 16-03-PLAN.md — Route handlers userId extraction - [ ] 16-04-PLAN.md — Test suite updates @@ -195,6 +195,6 @@ Plans: | 13. Setup Impact Preview | v1.3 | 0/2 | Not started | - | | 14. PostgreSQL Migration | v2.0 | 0/? | Not started | - | | 15. External Authentication | v2.0 | 0/? | Not started | - | -| 16. Multi-User Data Model | v2.0 | 1/4 | In progress | - | +| 16. Multi-User Data Model | v2.0 | 2/4 | In Progress| | | 17. Object Storage | v2.0 | 0/? | Not started | - | | 18. Global Items & Public Profiles | v2.0 | 0/? | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 9280f35..7cf3eaa 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -1,16 +1,16 @@ --- gsd_state_version: 1.0 -milestone: v2.0 -milestone_name: Platform Foundation +milestone: v1.3 +milestone_name: Research & Decision Tools status: executing -stopped_at: null -last_updated: "2026-04-05" -last_activity: 2026-04-05 — Completed 16-01-PLAN.md (multi-user data model foundation) +stopped_at: Completed 16-02-PLAN.md +last_updated: "2026-04-05T08:45:22.411Z" +last_activity: 2026-04-05 progress: - total_phases: 5 - completed_phases: 0 - total_plans: 4 - completed_plans: 1 + total_phases: 10 + completed_phases: 8 + total_plans: 25 + completed_plans: 21 percent: 5 --- @@ -26,15 +26,16 @@ See: .planning/PROJECT.md (updated 2026-04-03) ## Current Position Phase: 16 of 18 (Multi-User Data Model) -Plan: 1 of 4 in current phase -Status: Plan 16-01 complete, executing remaining plans -Last activity: 2026-04-05 — Completed 16-01 (multi-user data model foundation) +Plan: 2 of 4 in current phase +Status: Ready to execute +Last activity: 2026-04-05 Progress: [#---------] 5% (v2.0 milestone) ## Performance Metrics **Velocity:** + - Total plans completed: 1 (v2.0 milestone) - Average duration: 8min - Total execution time: 8min @@ -46,6 +47,7 @@ Progress: [#---------] 5% (v2.0 milestone) ### Decisions Key decisions made during v2.0 execution: + - Platform pivot: single-user to multi-user with discovery-first approach - External auth provider (self-hosted, open-source) — Logto vs Authentik OPEN decision - SQLite to Postgres migration — required by auth provider and multi-user concurrency @@ -55,6 +57,9 @@ Key decisions made during v2.0 execution: - All API routes require auth (no GET bypass) for per-user data scoping (16-01) - OAuth service converted from sync to async for pg compatibility (16-01) - getOrCreateUncategorized placed in category.service.ts (16-01) +- [Phase 16]: Category deletion uses dynamic getOrCreateUncategorized per user instead of hardcoded ID +- [Phase 16]: Candidate operations verify parent thread ownership for cross-user isolation +- [Phase 16]: syncSetupItems validates both setup and item ownership via inArray ### Pending Todos @@ -67,6 +72,6 @@ None active. ## Session Continuity -Last session: 2026-04-05 -Stopped at: Completed 16-01-PLAN.md (multi-user data model foundation) +Last session: 2026-04-05T08:45:22.408Z +Stopped at: Completed 16-02-PLAN.md Resume file: None diff --git a/.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md b/.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md new file mode 100644 index 0000000..ee5332f --- /dev/null +++ b/.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md @@ -0,0 +1,146 @@ +--- +phase: 16-multi-user-data-model +plan: 02 +subsystem: api +tags: [drizzle, postgres, multi-user, data-isolation, services] + +requires: + - phase: 16-multi-user-data-model (plan 01) + provides: schema with userId columns, users table, auth middleware resolving userId +provides: + - User-scoped service layer — all 7 service files accept userId and filter queries + - Cross-user data isolation via and(eq(id), eq(userId)) on all get/update/delete + - Ownership validation on thread resolution, setup item sync, candidate operations +affects: [16-03 route handlers, 16-04 MCP tools, tests] + +tech-stack: + added: [] + patterns: [userId-as-second-param, and(eq) isolation, ownership-validation-before-mutation] + +key-files: + created: [] + 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 + +key-decisions: + - "Category deletion uses dynamic getOrCreateUncategorized(db, userId) instead of hardcoded ID 1" + - "Candidate operations verify parent thread ownership before proceeding (not just candidate existence)" + - "syncSetupItems validates both setup ownership and item ownership via inArray" + - "resolveThread verifies category belongs to user with fallback to getOrCreateUncategorized" + - "createApiKey param order changed to (db, userId, name) for consistency with other services" + +patterns-established: + - "userId-second-param: all service functions use signature (db, userId, ...rest)" + - "composite-where: get/update/delete by ID always use and(eq(table.id, id), eq(table.userId, userId))" + - "ownership-chain: candidate ops verify parent thread ownership, setup sync verifies item ownership" + +requirements-completed: [MULTI-01, MULTI-02, MULTI-03, MULTI-06] + +duration: 4min +completed: 2026-04-05 +--- + +# Phase 16 Plan 02: Service Layer userId Scoping Summary + +**All 7 service files accept userId parameter with and(eq) isolation on every query — no unscoped reads or writes remain** + +## Performance + +- **Duration:** 4 min +- **Started:** 2026-04-05T08:40:21Z +- **Completed:** 2026-04-05T08:43:55Z +- **Tasks:** 2 +- **Files modified:** 7 + +## Accomplishments +- Every exported service function now accepts userId as second parameter with no default db values +- All get-by-id, update, and delete operations use and(eq(id), eq(userId)) for cross-user isolation +- Thread resolution includes userId on new item creation and verifies category ownership +- Setup item sync validates both setup and item ownership before inserting +- Candidate operations (add, update, remove) verify parent thread belongs to user +- CSV import scopes category lookup/creation and item insertion to the importing user +- Category deletion uses dynamic Uncategorized lookup per user instead of hardcoded ID + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Update item, category, totals, and CSV services** - `8d85d28` (feat) +2. **Task 2: Update thread, setup, and auth services** - `242cace` (feat) + +## Files Created/Modified +- `src/server/services/item.service.ts` - userId on all 6 functions, and() isolation on get/update/delete/duplicate +- `src/server/services/category.service.ts` - userId on all functions, async Postgres patterns, dynamic Uncategorized lookup on delete +- `src/server/services/totals.service.ts` - userId filtering on category and global totals +- `src/server/services/csv.service.ts` - userId on import/export, user-scoped category cache, userId on item inserts +- `src/server/services/thread.service.ts` - userId on all 9 functions, ownership verification on candidate ops, userId on resolveThread item insert +- `src/server/services/setup.service.ts` - userId on all 7 functions, inArray item ownership validation in syncSetupItems +- `src/server/services/auth.service.ts` - removed prodDb defaults, reordered createApiKey params + +## Decisions Made +- Category deletion now uses `getOrCreateUncategorized(db, userId)` instead of hardcoded category ID 1, supporting multi-user where each user has their own Uncategorized category +- Candidate operations verify parent thread ownership (not just candidate existence) to prevent cross-user manipulation via candidate ID guessing +- `syncSetupItems` validates item ownership via `inArray` query before inserting, silently filtering out items that don't belong to the user +- `resolveThread` verifies the candidate's category belongs to the user, falling back to `getOrCreateUncategorized` if not +- `createApiKey` parameter order changed from `(db, name, userId)` to `(db, userId, name)` for consistency with the userId-second-param pattern + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Category service sync SQLite to async Postgres patterns** +- **Found during:** Task 1 (category.service.ts) +- **Issue:** Category service still used synchronous SQLite patterns (.get(), .all(), .run()) that don't work with Postgres driver +- **Fix:** Converted all functions to async with await and array destructuring, consistent with other services +- **Files modified:** src/server/services/category.service.ts +- **Verification:** All functions now use async/await patterns +- **Committed in:** 8d85d28 (Task 1 commit) + +**2. [Rule 2 - Missing Critical] Added getCategoryById function** +- **Found during:** Task 1 (category.service.ts) +- **Issue:** Category service had no getCategoryById function — needed for userId-scoped lookups +- **Fix:** Added getCategoryById(db, userId, id) with and(eq) isolation +- **Files modified:** src/server/services/category.service.ts +- **Verification:** Function exists with proper userId scoping +- **Committed in:** 8d85d28 (Task 1 commit) + +**3. [Rule 2 - Missing Critical] Setup operations verify ownership before mutations** +- **Found during:** Task 2 (setup.service.ts) +- **Issue:** updateItemClassification and removeSetupItem had no ownership checks — raw SQL conditions without setup ownership validation +- **Fix:** Added setup ownership verification before both operations +- **Files modified:** src/server/services/setup.service.ts +- **Verification:** Both functions check setup belongs to user before proceeding +- **Committed in:** 242cace (Task 2 commit) + +--- + +**Total deviations:** 3 auto-fixed (1 bug, 2 missing critical) +**Impact on plan:** All auto-fixes necessary for correctness and security. No scope creep. + +## Issues Encountered +None + +## User Setup Required +None - no external service configuration required. + +## Known Stubs +None - all service functions are fully implemented with proper userId scoping. + +## Next Phase Readiness +- All services now accept userId — route handlers (Plan 03) can pass c.get("userId") from auth middleware +- MCP tools (Plan 04) can pass userId from MCP auth context +- Tests will need updating to pass userId to all service calls + +## Self-Check: PASSED + +All 7 modified service files exist. Both task commits (8d85d28, 242cace) verified in git log. + +--- +*Phase: 16-multi-user-data-model* +*Completed: 2026-04-05*