feat: add Plex Watchlist to Overseerr sync

[mchestr]

Summary

Add automatic syncing of users' Plex Watchlists to Overseerr requests. This feature allows users to enable watchlist sync from their dashboard, which will automatically create requests in Overseerr for items on their Plex watchlist.

Features

• Per-user sync settings - Each user can enable/disable sync from their dashboard
• Background polling job - Configurable sync interval (default 60 minutes)
• Distributed lock - Multi-pod safe using database locks
• Cumulative tracking - Total items synced and requests created
• Recent sync history - Shows last 5 synced items in user card
• Admin controls - Global enable/disable and sync interval configuration
• Admin statistics - Users with sync enabled, total synced items, recent activity

Technical Details

• New models: WatchlistSyncSettings, WatchlistSyncHistory, WatchlistSyncLock
• Plex token storage: Auth token stored during login for watchlist API access
• Plex API: Uses discover.provider.plex.tv to fetch user watchlist
• Overseerr integration: Submits requests via existing API
• Edge Runtime isolation: Node.js instrumentation isolated to avoid bundling issues

Files Changed

• prisma/schema.prisma - New models and User.plexAuthToken field
• lib/auth.ts - Store Plex auth token on login
• lib/watchlist/ - Plex watchlist API, sync service, distributed lock
• lib/instrumentation/node.ts - Isolated Node.js background jobs
• actions/watchlist.ts - User actions for settings and manual sync
• actions/admin/watchlist.ts - Admin actions for global settings
• components/watchlist/ - User sync card and history table
• components/admin/settings/watchlist-sync-settings.tsx - Admin settings UI

Test plan

• Enable watchlist sync globally in admin settings
• Enable sync for a user account
• Verify items from Plex watchlist appear in Overseerr requests
• Check cumulative stats increment correctly
• Verify recent history shows synced items
• Test manual "Sync Now" button
• Verify background job runs at configured interval

🤖 Generated with Claude Code

[claude]

Claude finished @mchestr's task in 3m 25s —— View job

---

Review Complete ✅

I've posted a comprehensive code review as a PR comment. Here's a summary:

Overall Assessment

This is a well-architected feature with excellent design patterns. The distributed lock mechanism is particularly impressive and production-ready.

Key Findings

Strengths:

• ✅ Excellent distributed lock implementation with multi-pod safety
• ✅ Clean separation of concerns across API clients, sync logic, and UI
• ✅ Proper security (auth checks, Zod validation, Prisma parameterized queries)
• ✅ Good error handling and centralized logging
• ✅ Follows project conventions (CLAUDE.md)

Critical Issue:

• 🔴 Missing test coverage - No unit, integration, or E2E tests found

Medium Priority:

• 🟡 Rate limiting could be configurable
• 🟡 Token storage in plain text (consider encryption for production)
• 🟡 Hardcoded batch sizes and delays
• 🟡 Error truncation (only stores first 3 errors)

Minor Issues:

• 🟢 Using console.error instead of logger in 2 places
• 🟢 Missing JSDoc for complex functions
• 🟢 Component file size approaching guideline limit (363 lines)
• 🟢 Could benefit from more data-testid attributes

Recommendation

⚠️ Conditional Approval - Strongly recommend adding test coverage before merging to production. The code quality is excellent, but tests are essential for a feature touching authentication, external APIs, and background jobs.

Next Steps

1. Add comprehensive test coverage (critical)
2. Add data-testid attributes for E2E tests
3. Replace console.error with centralized logger
4. Add basic documentation to README

View detailed review |

[claude]

Code Review: Plex Watchlist to Overseerr Sync

This is a well-structured implementation with good architecture decisions. Below is my comprehensive review organized by area.

---

✅ Strengths

Architecture & Design

• Excellent distributed lock implementation - Uses database-based locking with automatic renewal and expiration handling (lib/watchlist/lock.ts:65-168)
• Proper separation of concerns - Clean separation between API clients, sync logic, and UI components
• Multi-pod safe design - Lock mechanism ensures only one instance runs sync at a time
• Background job isolation - Correctly uses Node.js instrumentation to avoid Edge Runtime bundling issues (instrumentation.ts:1-13)

Security

• Auth token storage is justified - Storing plexAuthToken is necessary for watchlist API access (requires user's token, not server token)
• Token validation before sync - Validates tokens before attempting sync (lib/watchlist/sync-service.ts:54-63)
• Admin-only global controls - Proper admin checks for global settings (actions/admin/watchlist.ts:14-16)
• Proper auth checks in Server Actions - All actions verify session/admin status

Code Quality

• Comprehensive error handling - Good error messages and fallback behavior throughout
• Zod validation - All inputs validated with proper schemas (lib/validations/watchlist.ts)
• Logging strategy - Uses centralized logger with appropriate log levels
• Type safety - Proper TypeScript types throughout

---

⚠️ Issues & Recommendations

🔴 Critical: Missing Test Coverage

Issue: No tests found for this feature (checked __tests__/**watchlist** and e2e/**watchlist**)

Impact: High-risk feature touching authentication, external APIs, and database

Recommendation: Add test coverage for:

1. Unit tests for core logic:

   • lib/watchlist/sync-service.ts - sync logic, error handling
   • lib/watchlist/plex-watchlist.ts - API response parsing, GUID extraction
   • lib/watchlist/lock.ts - lock acquisition, renewal, race conditions
   • parseExternalIds() function - various GUID formats

2. Integration tests for:

   • Server Actions with mock Prisma calls
   • Token validation flow
   • Overseerr request submission

3. E2E tests for:

   • User enabling/disabling sync from dashboard
   • Admin configuring global settings
   • Manual sync trigger

Files to create:

• lib/watchlist/__tests__/sync-service.test.ts
• lib/watchlist/__tests__/plex-watchlist.test.ts
• lib/watchlist/__tests__/lock.test.ts
• actions/__tests__/watchlist.test.ts
• e2e/watchlist-sync.spec.ts

---

🟡 Medium: Potential Issues

1. Rate Limiting Concerns (lib/watchlist/sync-service.ts:324-340)

Issue: 1-second delay between users may not be sufficient for Plex/Overseerr APIs

// Small delay between users to be respectful of APIs
await new Promise((resolve) => setTimeout(resolve, 1000))

Recommendation:

• Consider making delay configurable via env var
• Add exponential backoff on API errors
• Monitor for 429 (rate limit) responses

2. Token Security in Database (lib/auth.ts:161, 185)

Issue: Plex auth tokens stored in plain text in database

plexAuthToken: authToken, // Store for watchlist sync

Recommendation:

• Consider encrypting tokens at rest using AES-256
• Add token rotation mechanism
• Document security implications in README
• Consider adding SELECT restrictions to prevent accidental token exposure

3. Batch Size Hardcoded (lib/watchlist/sync-service.ts:313)

Issue:

take: 50, // Limit batch size

Recommendation: Make batch size configurable via environment variable or admin settings

4. Error Array Truncation (lib/watchlist/sync-service.ts:207)

Issue: Only stores first 3 errors

lastSyncError: errors.length > 0 ? errors.slice(0, 3).join("; ") : null,

Recommendation: Store all errors in a separate field or increase limit with better formatting

---

🟢 Minor: Code Quality Improvements

1. Console.error Usage (actions/watchlist.ts:89, actions/admin/watchlist.ts:36)

Issue: Using console.error instead of centralized logger

console.error("Error fetching watchlist sync settings:", error)

Recommendation: Use createLogger('watchlist-actions') for consistency

2. Magic Numbers (lib/watchlist/lock.ts:40-43)

Issue: Lock timing constants could benefit from better documentation

const LOCK_LEASE_DURATION_MS = 30 * 1000
const LOCK_RENEWAL_INTERVAL_MS = 10 * 1000

Recommendation: Add comments explaining why these values were chosen (e.g., "30s lease allows for network delays")

3. Type Assertions (lib/watchlist/sync-service.ts:140)

Issue: Manual type mapping instead of using Prisma enums

const prismaMediaType = item.type === "movie" ? MediaType.MOVIE : MediaType.TV_SERIES

Recommendation: Create a helper function or use a map object for clarity

4. Missing JSDoc (lib/watchlist/sync-service.ts)

Issue: Complex sync logic lacks JSDoc documentation per CLAUDE.md guidelines

Recommendation: Add comprehensive JSDoc following the project's JSDoc strategy (see CLAUDE.md:291-362), especially for:

• syncUserWatchlist() - document sync algorithm
• syncAllEnabledUsers() - document batch processing behavior

5. Component File Size (components/watchlist/sync-settings-card.tsx)

Issue: Component is 362 lines, approaching the 200-300 line guideline (CLAUDE.md:196-199)

Recommendation: Consider extracting:

• Status formatting logic to a hook (useWatchlistStatus)
• Recent history table to separate component
• Statistics display to separate component

---

📋 Database & Schema

Excellent Migration Design

• Proper indexes on frequently queried fields
• Appropriate foreign key constraints with CASCADE
• Enum for sync status (type-safe)

Suggestions

1. Add database comment to User.plexAuthToken explaining security implications
2. Consider retention policy for WatchlistSyncHistory (auto-cleanup after N days)

---

🔒 Security Assessment

Potential Vulnerabilities Checked

• ✅ SQL Injection: Protected (Prisma parameterized queries)
• ✅ XSS: Protected (React escaping)
• ✅ CSRF: Protected (Server Actions)
• ✅ Auth Bypass: Protected (proper session checks)
• ⚠️ Token Storage: Plain text (see recommendation above)
• ✅ Rate Limiting: Partial (has delays, could be improved)

Auth Token Flow

The decision to store plexAuthToken is architecturally sound because:

1. Plex Watchlist API requires user's personal token (not server admin token)
2. Token is updated on each login (lib/auth.ts:185)
3. Token is validated before use (lib/watchlist/plex-watchlist.ts:177-189)

However, consider adding encryption for defense-in-depth.

---

📊 Performance Considerations

Potential Bottlenecks

1. Sequential user processing - Could benefit from controlled parallelism
2. N+1 query potential - Check if Prisma includes are optimized
3. No caching - Recent history is queried on every dashboard load

Recommendations

1. Add database query logging in development
2. Monitor sync duration metrics
3. Consider caching recent history with revalidation

---

🎨 UI/UX Review

Strengths

• Loading states implemented
• Error messages user-friendly
• Proper use of existing UI components (follows CLAUDE.md:222-244)
• Accessible (uses semantic HTML)

Suggestions

1. Add data-testid attributes per CLAUDE.md:278-284 (only found one: watchlist-sync-card-loading)
2. Consider optimistic UI updates for toggle switch
3. Add confirmation dialog for disabling sync (prevents accidental clicks)

---

📝 Documentation Needs

1. README.md: Document the watchlist sync feature
2. Environment Variables: Document ENABLE_WATCHLIST_SYNC, WATCHLIST_SYNC_POLL_INTERVAL_MS
3. User Guide: How to enable/use watchlist sync
4. Admin Guide: Global configuration and troubleshooting

---

🚀 Deployment Considerations

Pre-Deploy Checklist

• Run database migration: npm run db:migrate
• Verify Overseerr API access from production
• Test Plex Discover API access (firewall rules)
• Configure monitoring for sync failures
• Set appropriate WATCHLIST_SYNC_POLL_INTERVAL_MS

Monitoring Recommendations

1. Track sync success/failure rates
2. Alert on repeated failures for same user
3. Monitor Plex/Overseerr API response times
4. Track lock acquisition conflicts (multiple pods)

---

📦 Summary

This PR implements a solid watchlist sync feature with good architecture and security practices. The main concern is lack of test coverage for a feature that touches authentication, external APIs, and background jobs.

Before Merging

1. Add test coverage (critical)
2. Consider token encryption (recommended)
3. Add data-testid attributes to components
4. Replace console.error with logger
5. Add documentation

Nice to Have

• Rate limiting improvements
• Configurable batch size
• Better error aggregation
• Performance optimizations

Approval Status

⚠️ Conditional Approval - Recommend adding tests before merging to production. The code quality is excellent, but test coverage is essential for this type of feature.

---

Great work on this implementation! The distributed lock mechanism is particularly well done. Let me know if you'd like me to help implement any of the suggested improvements.

[claude]

Code Review: Plex Watchlist to Overseerr Sync

This is a well-architected feature implementation with strong design patterns. I've identified both strengths and areas for improvement.

---

✅ Strengths

Architecture & Design

• Excellent distributed lock implementation - Database-based locking with automatic renewal and expiration (lib/watchlist/lock.ts:65-168)
• Proper separation of concerns - Clean separation between API clients, sync logic, and UI components
• Multi-pod safe design - Lock mechanism ensures only one instance runs sync at a time
• Background job isolation - Correctly uses Node.js instrumentation to avoid Edge Runtime bundling issues

Security

• Auth token storage is justified - Storing plexAuthToken is necessary for watchlist API access (requires user's token, not server token)
• Token validation before sync - Validates tokens before attempting sync (lib/watchlist/sync-service.ts:54-63)
• Admin-only global controls - Proper admin checks for global settings (actions/admin/watchlist.ts:14-16)
• Proper auth checks in Server Actions - All actions verify session/admin status

Code Quality

• Comprehensive error handling - Good error messages and fallback behavior
• Zod validation - All inputs validated with proper schemas
• Logging strategy - Uses centralized logger with appropriate log levels
• Type safety - Proper TypeScript types throughout

---

⚠️ Issues & Recommendations

🔴 Critical: Missing Test Coverage

Issue: No tests found for this feature

Impact: High-risk feature touching authentication, external APIs, and database operations

Recommendation: Add test coverage for:

1. Unit tests for core logic:

lib/watchlist/__tests__/sync-service.test.ts
lib/watchlist/__tests__/plex-watchlist.test.ts  
lib/watchlist/__tests__/lock.test.ts

• Test sync logic, error handling
• Test API response parsing, GUID extraction
• Test lock acquisition, renewal, race conditions
• Test parseExternalIds() with various GUID formats

2. Integration tests:

actions/__tests__/watchlist.test.ts
actions/__tests__/admin/watchlist.test.ts

• Server Actions with mock Prisma calls
• Token validation flow
• Overseerr request submission

3. E2E tests:

e2e/watchlist-sync.spec.ts

• User enabling/disabling sync from dashboard
• Admin configuring global settings
• Manual sync trigger

---

🟡 Medium Priority Issues

1. Rate Limiting Concerns (lib/watchlist/sync-service.ts:334)

Issue: 1-second delay between users may not be sufficient

await new Promise((resolve) => setTimeout(resolve, 1000))

Recommendation:

• Make delay configurable via env var
• Add exponential backoff on API errors
• Monitor for 429 (rate limit) responses

2. Token Security in Database (lib/auth.ts:161, 185)

Issue: Plex auth tokens stored in plain text

Recommendation:

• Consider encrypting tokens at rest using AES-256
• Add token rotation mechanism
• Document security implications in README
• Consider adding SELECT restrictions to prevent accidental exposure

3. Batch Size Hardcoded (lib/watchlist/sync-service.ts:313)

Issue: take: 50 is hardcoded

Recommendation: Make configurable via environment variable or admin settings

4. Error Array Truncation (lib/watchlist/sync-service.ts:207)

Issue: Only stores first 3 errors

lastSyncError: errors.length > 0 ? errors.slice(0, 3).join("; ") : null,

Recommendation: Store all errors in a separate field or increase limit with better formatting

---

🟢 Minor: Code Quality Improvements

1. Console.error Usage (actions/watchlist.ts:92, actions/admin/watchlist.ts:39)

Issue: Using console.error instead of centralized logger

Recommendation: Use createLogger('watchlist-actions') for consistency

2. Magic Numbers (lib/watchlist/lock.ts:40-43)

Issue: Lock timing constants need better documentation

const LOCK_LEASE_DURATION_MS = 30 * 1000
const LOCK_RENEWAL_INTERVAL_MS = 10 * 1000

Recommendation: Add comments explaining why these values were chosen

3. Missing JSDoc (lib/watchlist/sync-service.ts)

Issue: Complex sync logic lacks JSDoc documentation per CLAUDE.md guidelines

Recommendation: Add comprehensive JSDoc for:

• syncUserWatchlist() - document sync algorithm
• syncAllEnabledUsers() - document batch processing behavior

4. Component File Size (components/watchlist/sync-settings-card.tsx)

Issue: Component is 362 lines, approaching the 200-300 line guideline

Recommendation: Extract:

• Status formatting logic to a hook (useWatchlistStatus)
• Recent history table to separate component
• Statistics display to separate component

5. Missing data-testid Attributes

Issue: Per CLAUDE.md:278-284, E2E tests should use data-testid selectors. Only found one: watchlist-sync-card-loading

Recommendation: Add more test IDs to components for E2E testing

---

📋 Database & Schema

Excellent Migration Design

• Proper indexes on frequently queried fields
• Appropriate foreign key constraints with CASCADE
• Enum for sync status (type-safe)

Suggestions

1. Add database comment to User.plexAuthToken explaining security implications
2. Consider retention policy for WatchlistSyncHistory (auto-cleanup after N days)

---

🔒 Security Assessment

Vulnerabilities Checked

• ✅ SQL Injection: Protected (Prisma parameterized queries)
• ✅ XSS: Protected (React escaping)
• ✅ CSRF: Protected (Server Actions)
• ✅ Auth Bypass: Protected (proper session checks)
• ⚠️ Token Storage: Plain text (see recommendation above)
• ✅ Rate Limiting: Partial (has delays, could be improved)

Auth Token Flow

Storing plexAuthToken is architecturally sound because:

1. Plex Watchlist API requires user's personal token (not server admin token)
2. Token is updated on each login (lib/auth.ts:185)
3. Token is validated before use

Consider adding encryption for defense-in-depth.

---

📊 Performance Considerations

Potential Bottlenecks

1. Sequential user processing - Could benefit from controlled parallelism
2. N+1 query potential - Check if Prisma includes are optimized
3. No caching - Recent history queried on every dashboard load

Recommendations

1. Add database query logging in development
2. Monitor sync duration metrics
3. Consider caching recent history with revalidation

---

🎨 UI/UX Review

Strengths

• Loading states implemented
• Error messages user-friendly
• Proper use of existing UI components
• Accessible (semantic HTML)

Suggestions

1. Add more data-testid attributes per CLAUDE.md:278-284
2. Consider optimistic UI updates for toggle switch
3. Add confirmation dialog for disabling sync (prevents accidental clicks)

---

📝 Documentation Needs

1. README.md: Document the watchlist sync feature
2. Environment Variables: Document ENABLE_WATCHLIST_SYNC, WATCHLIST_SYNC_POLL_INTERVAL_MS
3. User Guide: How to enable/use watchlist sync
4. Admin Guide: Global configuration and troubleshooting

---

🚀 Deployment Considerations

Pre-Deploy Checklist

• Run database migration: npm run db:migrate
• Verify Overseerr API access from production
• Test Plex Discover API access (firewall rules)
• Configure monitoring for sync failures
• Set appropriate WATCHLIST_SYNC_POLL_INTERVAL_MS

Monitoring Recommendations

1. Track sync success/failure rates
2. Alert on repeated failures for same user
3. Monitor Plex/Overseerr API response times
4. Track lock acquisition conflicts (multiple pods)

---

📦 Summary

This PR implements a solid watchlist sync feature with excellent architecture. The distributed lock mechanism is particularly well done.

Before Merging

1. Add test coverage (critical - this is the blocker)
2. Consider token encryption (recommended)
3. Add data-testid attributes to components
4. Replace console.error with logger
5. Add documentation

Nice to Have

• Rate limiting improvements
• Configurable batch size
• Better error aggregation
• Performance optimizations

Approval Status

⚠️ Conditional Approval - Strongly recommend adding tests before merging. The code quality is excellent, but test coverage is essential for a feature touching authentication, external APIs, and background jobs.

---

Great work on this implementation! Let me know if you'd like help implementing any of the suggested improvements.

[claude]

Code Review: Plex Watchlist to Overseerr Sync

This is a well-architected feature implementation with excellent design patterns and strong separation of concerns. Below is my comprehensive review.

---

✅ Strengths

Architecture & Design

• Excellent distributed lock implementation - Database-based locking with automatic renewal and expiration (lib/watchlist/lock.ts:65-168). The lock mechanism properly handles multi-pod scenarios with lease duration and renewal intervals.
• Proper separation of concerns - Clean separation between API clients (plex-watchlist.ts), sync orchestration (sync-service.ts), and UI components
• Multi-pod safe design - Lock mechanism ensures only one instance runs sync at a time across distributed deployments
• Background job isolation - Correctly uses Node.js instrumentation (lib/instrumentation/node.ts) to avoid Edge Runtime bundling issues

Security

• Auth token storage is architecturally sound - Storing plexAuthToken is necessary because the Plex Watchlist API requires the user's personal token (not the server admin token). Token is:
  • Updated on each login (lib/auth.ts:185)
  • Validated before use (lib/watchlist/plex-watchlist.ts:177-189)
  • Only accessible via authenticated Server Actions
• Proper admin checks - All admin actions verify authorization (actions/admin/watchlist.ts:14-16)
• Comprehensive auth checks - All Server Actions verify session status before proceeding
• SQL injection protection - Uses Prisma parameterized queries throughout
• XSS protection - React handles escaping by default
• CSRF protection - Built into Server Actions

Code Quality

• Comprehensive error handling - Good error messages and graceful fallback behavior throughout
• Zod validation - All inputs validated with proper schemas (lib/validations/watchlist.ts)
• Centralized logging - Uses createLogger with appropriate log levels
• Type safety - Proper TypeScript types throughout, good use of Prisma generated types

Database Schema

• Excellent migration design - Proper indexes on frequently queried fields
• Appropriate foreign key constraints - CASCADE on delete for cleanup
• Type-safe enums - WatchlistSyncStatus enum for status tracking
• Unique constraints - Prevents duplicate tracking (userId, plexRatingKey)

---

⚠️ Issues & Recommendations

🔴 Critical: Missing Test Coverage

Issue: No tests found for this feature (checked for *watchlist*.test.ts and *watchlist*.spec.ts)

Impact: High-risk feature touching authentication, external APIs, database operations, and background jobs

Recommendation: Add comprehensive test coverage before merging:

1. Unit tests for core logic:

lib/watchlist/__tests__/sync-service.test.ts
lib/watchlist/__tests__/plex-watchlist.test.ts  
lib/watchlist/__tests__/lock.test.ts

Test cases should cover:

• Sync logic with various states (success, partial, failed)
• Error handling (invalid tokens, API failures, rate limits)
• API response parsing and GUID extraction
• Lock acquisition, renewal, and race conditions
• parseExternalIds() with various GUID formats (tmdb://, tvdb://, imdb://)

2. Integration tests:

actions/__tests__/watchlist.test.ts
actions/__tests__/admin/watchlist.test.ts

Test cases:

• Server Actions with mock Prisma calls
• Token validation flow
• Overseerr request submission
• Auth checks for both user and admin actions

3. E2E tests:

e2e/watchlist-sync.spec.ts

Test cases:

• User enabling/disabling sync from dashboard
• Admin configuring global settings
• Manual sync trigger
• Prerequisites validation (Plex token, Overseerr config)

---

🟡 Medium Priority Issues

1. Rate Limiting Concerns (lib/watchlist/sync-service.ts:334)

Issue: 1-second delay between users may not be sufficient for Plex/Overseerr APIs

await new Promise((resolve) => setTimeout(resolve, 1000))

Recommendation:

• Make delay configurable via environment variable
• Implement exponential backoff on API errors
• Monitor for 429 (rate limit) responses and handle gracefully
• Consider adaptive rate limiting based on API response times

2. Token Security (lib/auth.ts:161, 185)

Issue: Plex auth tokens stored in plain text in database

Recommendation:

• Consider encrypting tokens at rest using AES-256
• Implement token rotation mechanism
• Add database comment to User.plexAuthToken explaining security implications
• Document token storage decision in security documentation
• Consider adding Prisma SELECT restrictions to prevent accidental exposure

3. Hardcoded Batch Size (lib/watchlist/sync-service.ts:313)

Issue:

take: 50, // Limit batch size

Recommendation: Make configurable via environment variable or admin settings to allow tuning for different deployment sizes

4. Error Truncation (lib/watchlist/sync-service.ts:207)

Issue: Only stores first 3 errors, potentially losing important debugging information

lastSyncError: errors.length > 0 ? errors.slice(0, 3).join("; ") : null,

Recommendation:

• Store all errors in a separate field
• Increase limit with better formatting (e.g., JSON array)
• Consider logging full error details separately

---

🟢 Minor: Code Quality Improvements

1. Console Usage (actions/watchlist.ts:92, actions/admin/watchlist.ts:39)

Issue: Using console.error instead of centralized logger

console.error("Error fetching watchlist sync settings:", error)

Recommendation: Use createLogger('watchlist-actions') for consistency with project standards

2. Magic Numbers (lib/watchlist/lock.ts:40-43)

Issue: Lock timing constants lack documentation

const LOCK_LEASE_DURATION_MS = 30 * 1000
const LOCK_RENEWAL_INTERVAL_MS = 10 * 1000

Recommendation: Add comments explaining the rationale (e.g., "30s lease allows for network delays and processing time")

3. Missing JSDoc (lib/watchlist/sync-service.ts)

Issue: Complex sync logic lacks JSDoc documentation per CLAUDE.md guidelines (lines 291-362)

Recommendation: Add comprehensive JSDoc following the project's strategy for:

• syncUserWatchlist() - document sync algorithm, error handling, state transitions
• syncAllEnabledUsers() - document batch processing behavior, rate limiting approach

4. Component File Size (components/watchlist/sync-settings-card.tsx)

Issue: Component is 362 lines, approaching the 200-300 line guideline (CLAUDE.md:196-199)

Recommendation: Extract into smaller components:

• Status formatting logic → useWatchlistStatus hook
• Recent history table → <RecentSyncHistory> component
• Statistics display → <SyncStatistics> component

5. Missing data-testid Attributes

Issue: Per CLAUDE.md:278-284, E2E tests must use data-testid selectors. Currently only found:

• watchlist-sync-card-loading
• watchlist-sync-card
• watchlist-sync-toggle
• watchlist-sync-now-button

Recommendation: Add more test IDs for E2E testing:

• Error message containers
• Prerequisites warning sections
• Recent history items
• Admin settings UI elements

---

📊 Performance Considerations

Potential Bottlenecks

1. Sequential user processing - Could benefit from controlled parallelism (e.g., process 3-5 users concurrently)
2. N+1 query potential - Verify Prisma includes are optimized, especially in sync history queries
3. No caching - Recent history is queried on every dashboard load

Recommendations

1. Add database query logging in development mode
2. Monitor sync duration metrics and add alerts for slow syncs
3. Consider caching recent history with 1-minute revalidation
4. Add database connection pooling configuration for background jobs

---

🎨 UI/UX Review

Strengths

• Loading states properly implemented
• Error messages are user-friendly and actionable
• Proper use of existing UI components (follows CLAUDE.md:222-244)
• Accessible - uses semantic HTML and ARIA attributes

Suggestions

1. Add confirmation dialog for disabling sync to prevent accidental toggles
2. Consider optimistic UI updates for toggle switch to improve perceived performance
3. Add success animation when sync completes successfully
4. Show sync progress indicator when background job is running (requires WebSocket or polling)

---

🔒 Security Assessment

Vulnerabilities Checked

• ✅ SQL Injection: Protected (Prisma parameterized queries)
• ✅ XSS: Protected (React escaping, no dangerouslySetInnerHTML)
• ✅ CSRF: Protected (Server Actions)
• ✅ Auth Bypass: Protected (proper session checks in all actions)
• ⚠️ Token Storage: Plain text (see Medium Priority recommendation above)
• ✅ Rate Limiting: Partial (has delays, could be improved)
• ✅ Authorization: Proper admin checks for global settings

---

📝 Documentation Needs

1. README.md: Document the watchlist sync feature, prerequisites, and usage
2. Environment Variables: Document:
   • ENABLE_WATCHLIST_SYNC
   • WATCHLIST_SYNC_POLL_INTERVAL_MS
3. User Guide: How to enable/use watchlist sync, troubleshooting common issues
4. Admin Guide: Global configuration, monitoring, and troubleshooting
5. Security Documentation: Document token storage decision and security considerations

---

🚀 Deployment Considerations

Pre-Deploy Checklist

• Run database migration: npm run db:migrate
• Verify Overseerr API access from production environment
• Test Plex Discover API access (check firewall rules)
• Configure monitoring for sync failures
• Set appropriate WATCHLIST_SYNC_POLL_INTERVAL_MS (recommend 300000 for 5 minutes in production)
• Verify database has sufficient connections for background jobs
• Test lock mechanism with multiple pods (if using multi-pod deployment)

Monitoring Recommendations

1. Metrics to track:

   • Sync success/failure rates
   • Average sync duration per user
   • Plex/Overseerr API response times
   • Lock acquisition conflicts
   • Database connection pool usage

2. Alerts to configure:

   • Repeated failures for same user (3+ consecutive failures)
   • Sync duration > 5 minutes
   • Lock acquisition failures
   • Database connection pool exhaustion

---

📦 Summary

This PR implements a solid watchlist sync feature with excellent architecture and design patterns. The distributed lock mechanism is particularly well-implemented and production-ready.

Before Merging (Required)

1. Add comprehensive test coverage - This is the primary blocker
2. Add data-testid attributes to components for E2E tests
3. Replace console.error with logger for consistency
4. Add basic documentation (at minimum, update README)

Recommended Improvements (Nice to Have)

• Token encryption at rest
• Configurable rate limiting and batch sizes
• Better error aggregation (store all errors, not just first 3)
• Performance optimizations (caching, parallelism)
• Enhanced monitoring and alerting

Approval Status

⚠️ Conditional Approval - Strongly recommend adding tests before merging to production. The code quality is excellent, but test coverage is essential for a feature that:

• Touches authentication and user tokens
• Integrates with external APIs
• Runs as a background job
• Manages distributed state with locks

---

Excellent work on this implementation! The distributed lock mechanism and clean architecture are particularly impressive. The main concern is test coverage - once that's addressed, this will be production-ready.

Let me know if you'd like help implementing any of the suggested improvements or writing tests for this feature.

[claude]

Code Review: Plex Watchlist to Overseerr Sync

This is a well-architected feature implementation with excellent design patterns and strong separation of concerns. The distributed lock mechanism is particularly impressive and production-ready.

---

✅ Strengths

Architecture & Design

• Excellent distributed lock implementation - Database-based locking with automatic renewal and expiration (lib/watchlist/lock.ts:65-168). Properly handles multi-pod scenarios with lease duration (30s) and renewal intervals (10s)
• Clean separation of concerns - Clear boundaries between API clients (plex-watchlist.ts), sync orchestration (sync-service.ts), database operations, and UI components
• Multi-pod safe design - Lock mechanism with instance ID tracking ensures only one pod runs sync at a time across distributed deployments
• Background job isolation - Correctly uses Node.js instrumentation (lib/instrumentation/node.ts) to avoid Edge Runtime bundling issues

Security

• Auth token storage is architecturally sound - Storing plexAuthToken is necessary because the Plex Watchlist API requires the user's personal token (not the server admin token). Token is:
  • Updated on each login (lib/auth.ts:185)
  • Validated before use (lib/watchlist/plex-watchlist.ts:177-189)
  • Only accessible via authenticated Server Actions
• Proper authorization checks - All admin actions verify authorization (actions/admin/watchlist.ts:17-19)
• Comprehensive auth checks - All Server Actions verify session status before proceeding
• SQL injection protection - Uses Prisma parameterized queries throughout
• XSS protection - React handles escaping by default, no dangerouslySetInnerHTML
• CSRF protection - Built into Server Actions

Code Quality

• Comprehensive error handling - Good error messages and graceful fallback behavior throughout
• Zod validation - All inputs validated with proper schemas (lib/validations/watchlist.ts)
• Centralized logging - Uses createLogger with appropriate log levels
• Type safety - Proper TypeScript types throughout, good use of Prisma generated types
• Follows project conventions - Adheres to CLAUDE.md guidelines for Server Actions, error handling, and file organization

Database Schema

• Excellent migration design - Proper indexes on frequently queried fields
• Appropriate foreign key constraints - CASCADE on delete for proper cleanup
• Type-safe enums - WatchlistSyncStatus enum for status tracking
• Unique constraints - Prevents duplicate tracking (userId, plexRatingKey)
• Cumulative counters - Tracks both per-sync and total stats

---

⚠️ Issues & Recommendations

🔴 Critical: Missing Test Coverage

Issue: No tests found for this feature (verified with glob searches for *watchlist*.test.ts and *watchlist*.spec.ts)

Impact: High-risk feature touching authentication, external APIs, database operations, and background jobs

Recommendation: Add comprehensive test coverage before merging:

1. Unit tests for core logic:

lib/watchlist/__tests__/sync-service.test.ts
lib/watchlist/__tests__/plex-watchlist.test.ts  
lib/watchlist/__tests__/lock.test.ts

Test cases should cover:

• Sync logic with various states (success, partial, failed)
• Error handling (invalid tokens, API failures, rate limits)
• API response parsing and GUID extraction (parseExternalIds with tmdb://, tvdb://, imdb://)
• Lock acquisition, renewal, race conditions, and expired lock cleanup
• Token validation flow

2. Integration tests:

actions/__tests__/watchlist.test.ts
actions/__tests__/admin/watchlist.test.ts

Test cases:

• Server Actions with mock Prisma calls
• Prerequisites validation (token, Overseerr config, global settings)
• Overseerr request submission with various response codes
• Auth checks for both user and admin actions

3. E2E tests:

e2e/watchlist-sync.spec.ts

Test cases (following CLAUDE.md:278-284, use data-testid selectors):

• User enabling/disabling sync from dashboard
• Admin configuring global settings
• Manual sync trigger
• Prerequisites validation UI

---

🟡 Medium Priority Issues

1. Rate Limiting Concerns (lib/watchlist/sync-service.ts:335)

Issue: 1-second delay between users may not be sufficient for Plex/Overseerr APIs

await new Promise((resolve) => setTimeout(resolve, 1000))

Recommendation:

• Make delay configurable via environment variable (WATCHLIST_SYNC_USER_DELAY_MS)
• Implement exponential backoff on API errors
• Monitor for 429 (rate limit) responses and handle gracefully
• Consider adaptive rate limiting based on API response times

2. Token Security (lib/auth.ts:161, 185)

Issue: Plex auth tokens stored in plain text in database

Recommendation:

• Consider encrypting tokens at rest using AES-256 (especially for production deployments)
• Implement token rotation mechanism
• Add database comment to User.plexAuthToken field explaining security implications and design decision
• Document token storage decision in security documentation
• Consider adding Prisma SELECT restrictions to prevent accidental exposure in queries

3. Hardcoded Batch Size (lib/watchlist/sync-service.ts:313)

Issue:

take: 50, // Limit batch size

Recommendation: Make configurable via environment variable or admin settings (WATCHLIST_SYNC_BATCH_SIZE) to allow tuning for different deployment sizes

4. Error Truncation (lib/watchlist/sync-service.ts:207)

Issue: Only stores first 3 errors, potentially losing important debugging information

lastSyncError: errors.length > 0 ? errors.slice(0, 3).join("; ") : null,

Recommendation:

• Store all errors in a separate JSON field or increase limit with better formatting
• Consider logging full error details separately for admin review
• Add error count to sync status (totalErrorCount)

---

🟢 Minor: Code Quality Improvements

1. Console Usage (actions/watchlist.ts:92, actions/admin/watchlist.ts:39)

Issue: Using console.error instead of centralized logger (violates CLAUDE.md:365-377)

console.error("Error fetching watchlist sync settings:", error)

Recommendation: Use createLogger('watchlist-actions') and createLogger('admin-watchlist-actions') for consistency with project standards

2. Magic Numbers (lib/watchlist/lock.ts:40-43)

Issue: Lock timing constants lack documentation

const LOCK_LEASE_DURATION_MS = 30 * 1000
const LOCK_RENEWAL_INTERVAL_MS = 10 * 1000

Recommendation: Add JSDoc comments explaining the rationale:

// 30s lease allows for network delays, API calls, and processing time
// while still detecting crashed pods quickly enough
const LOCK_LEASE_DURATION_MS = 30 * 1000

// Renew every 10s (3x per lease) for robust lock retention
const LOCK_RENEWAL_INTERVAL_MS = 10 * 1000

3. Missing JSDoc (lib/watchlist/sync-service.ts)

Issue: Complex sync logic lacks JSDoc documentation per CLAUDE.md guidelines (lines 291-362)

Recommendation: Add comprehensive JSDoc following the project's strategy for:

• syncUserWatchlist() - document sync algorithm, error handling, state transitions
• syncAllEnabledUsers() - document batch processing behavior, rate limiting approach
• Include @example blocks with working code examples

4. Component File Size (components/watchlist/sync-settings-card.tsx)

Issue: Component is 363 lines, approaching the 200-300 line guideline (CLAUDE.md:196-199)

Recommendation: Consider extracting into smaller components:

• Status formatting logic → useWatchlistStatus custom hook
• Recent history display (lines 271-312) → <RecentSyncHistory> component
• Statistics display (lines 252-261) → <SyncStatistics> component

5. Missing data-testid Attributes

Issue: Per CLAUDE.md:278-284, E2E tests must use data-testid selectors. Currently only found:

• watchlist-sync-card-loading
• watchlist-sync-card
• watchlist-sync-toggle
• watchlist-sync-now-button
• watchlist-sync-settings
• watchlist-sync-settings-loading

Recommendation: Add more test IDs for E2E testing:

• Error message containers
• Prerequisites warning sections
• Recent history items
• Admin settings form fields
• Statistics displays

---

📊 Performance Considerations

Potential Bottlenecks

1. Sequential user processing - Could benefit from controlled parallelism (e.g., process 3-5 users concurrently with Promise.all batching)
2. N+1 query potential - Verify Prisma includes are optimized, especially in getWatchlistSyncHistory and recent history queries
3. No caching - Recent history is queried on every dashboard load

Recommendations

1. Add database query logging in development mode to identify slow queries
2. Monitor sync duration metrics and add alerts for slow syncs (>5 minutes)
3. Consider caching recent history with 1-minute revalidation using React Query
4. Add database connection pooling configuration documentation for background jobs

---

🎨 UI/UX Review

Strengths

• Loading states properly implemented with skeleton UI
• Error messages are user-friendly and actionable
• Proper use of existing UI components (follows CLAUDE.md:222-244)
• Accessible - uses semantic HTML, ARIA attributes, and role="switch"
• Good visual hierarchy and information density

Suggestions

1. Add confirmation dialog for disabling sync to prevent accidental toggles (especially when user has active sync history)
2. Consider optimistic UI updates for toggle switch to improve perceived performance
3. Add success animation when sync completes successfully (e.g., checkmark fade-in)
4. Show sync progress indicator when background job is running (would require WebSocket or polling mechanism)

---

🔒 Security Assessment

Vulnerabilities Checked

• ✅ SQL Injection: Protected (Prisma parameterized queries)
• ✅ XSS: Protected (React escaping, no dangerouslySetInnerHTML)
• ✅ CSRF: Protected (Server Actions built-in protection)
• ✅ Auth Bypass: Protected (proper session checks in all actions)
• ⚠️ Token Storage: Plain text (see Medium Priority recommendation above)
• ✅ Rate Limiting: Partial (has delays, could be improved)
• ✅ Authorization: Proper admin checks for global settings
• ✅ Input Validation: Comprehensive Zod schemas

Auth Token Flow

Storing plexAuthToken is architecturally sound because:

1. Plex Watchlist API requires user's personal token (not server admin token)
2. Token is updated on each login (lib/auth.ts:185)
3. Token is validated before use (lib/watchlist/plex-watchlist.ts:177-189)
4. Only accessible via authenticated Server Actions

Consider adding encryption for defense-in-depth in production environments.

---

📝 Documentation Needs

1. README.md: Document the watchlist sync feature, prerequisites, and usage
2. Environment Variables: Document:
   • ENABLE_WATCHLIST_SYNC (if applicable)
   • WATCHLIST_SYNC_POLL_INTERVAL_MS (if applicable)
   • Consider adding: WATCHLIST_SYNC_USER_DELAY_MS, WATCHLIST_SYNC_BATCH_SIZE
3. User Guide: How to enable/use watchlist sync, troubleshooting common issues (token expiry, Overseerr errors)
4. Admin Guide: Global configuration, monitoring, and troubleshooting
5. Security Documentation: Document token storage decision and security considerations

---

🚀 Deployment Considerations

Pre-Deploy Checklist

• Run database migration: npm run db:migrate
• Verify Overseerr API access from production environment
• Test Plex Discover API access (check firewall rules for discover.provider.plex.tv)
• Configure monitoring for sync failures
• Set appropriate sync interval (recommend 300000 for 5 minutes in production vs 60 minutes default)
• Verify database has sufficient connections for background jobs
• Test lock mechanism with multiple pods if using multi-pod deployment

Monitoring Recommendations

1. Metrics to track:

   • Sync success/failure rates
   • Average sync duration per user
   • Plex/Overseerr API response times and error rates
   • Lock acquisition conflicts (indicates contention)
   • Database connection pool usage

2. Alerts to configure:

   • Repeated failures for same user (3+ consecutive failures)
   • Sync duration > 5 minutes (indicates API issues or large watchlists)
   • Lock acquisition failures (indicates pod issues)
   • Database connection pool exhaustion

---

📦 Summary

This PR implements a solid watchlist sync feature with excellent architecture and design patterns. The distributed lock mechanism is particularly well-implemented and production-ready.

Before Merging (Required)

1. ✅ Add comprehensive test coverage - This is the primary blocker (unit, integration, E2E tests)
2. ✅ Add data-testid attributes to components for E2E tests
3. ✅ Replace console.error with logger for consistency
4. ✅ Add basic documentation (at minimum, update README with feature overview)

Recommended Improvements (Nice to Have)

• Token encryption at rest for production deployments
• Configurable rate limiting and batch sizes via env vars
• Better error aggregation (store all errors, not just first 3)
• Performance optimizations (caching, controlled parallelism)
• Enhanced monitoring and alerting setup documentation

Approval Status

⚠️ Conditional Approval - Strongly recommend adding tests before merging to production. The code quality is excellent and the architecture is sound, but test coverage is essential for a feature that:

• Touches authentication and stores user tokens
• Integrates with two external APIs (Plex, Overseerr)
• Runs as a background job with distributed locking
• Manages state across multiple database tables

---

Excellent work on this implementation! The distributed lock mechanism, clean separation of concerns, and proper error handling are particularly impressive. Once test coverage is added, this will be production-ready.

Let me know if you'd like help implementing any of the suggested improvements or writing tests for this feature.