# PM Assessment — MovieDice Audit Consolidation Produced: 2026-04-05 | Assessor: Claude (PM Agent) Sources reviewed: TECHFILE.md (tech stack), SECFILE.md (security), COMPLIANCE.md (compliance) Deployment context: Docker container (primary), bare server (potential later). NOT Vercel. Supabase context: Self-hosted via Docker (NOT Supabase managed hosting). Full Supabase Docker stack (Postgres, GoTrue, Realtime, PostgREST, Kong, Studio). Constraint: User reviews each suggestion before any changes are made to PROJECT_SCOPE.md. --- ## Summary | Recommendation Level | Count | | -------------------------- | ------ | | MUST DO (before code) | 7 | | MUST DO (during Phase 1) | 5 | | MUST DO (during Phase 3+) | 3 | | SHOULD DO | 11 | | NICE TO HAVE | 5 | | SKIP/DEFER | 4 | | NEEDS DISCUSSION | 4 | | **Total actionable items** | **39** | **Conflicts between reports:** 3 flagged (see Section at bottom) **Top 3 issues that would be expensive to fix post-launch:** 1. Auth model (items A1/A2) — touches every DB query if changed after build 2. RLS policies (item A3) — foundational to all data security; retrofitting is painful 3. Vercel Cron references (item A4) — baked into Phase 5 MVP work; breaks on day one --- ## MUST DO (before code) _These are foundational decisions. Starting any Phase 1 work without resolving them means expensive rework._ --- ### A1. Replace custom UUID auth with Supabase Anonymous Sign-In **Source:** TECHFILE (Finding 3, 11), SECFILE (Finding 4, 19) **Original Finding:** The scope proposes generating a UUID client-side, storing it in localStorage, and using it as the user's identity. This bypasses Supabase Auth entirely. Without a real JWT, Supabase's Row Level Security cannot use `auth.uid()` in policies — meaning the publicly-exposed anon key becomes a master key to the entire database. Additionally, a bare UUID in localStorage is vulnerable to XSS theft and trivial impersonation (anyone who knows a UUID can write it to their own localStorage and act as that user). **PM Assessment:** Valid and critical. This is the single highest-impact pre-code decision. The custom auth approach would require building session management, JWT issuance, and token refresh from scratch — all security-critical primitives that Supabase already implements correctly. Adopting `supabase.auth.signInAnonymously()` resolves the RLS gap, the localStorage spoofing risk, and the JWT management problem simultaneously. The user experience is identical: no email, no password, instant account. The recovery code flow still works as a layer on top. TECHFILE estimates this saves 2-3 days of Phase 1 work. The only reason not to do this is unfamiliarity with the Supabase Auth SDK — budget a half-day to read the anonymous sign-in docs before starting. **Recommendation:** MUST DO (before code) **Scope Impact:** Revise Phase 1 tasks 1.3–1.6 to reference Supabase Anonymous Sign-In instead of custom UUID generation. Update the Data Model section: `users.id` becomes the Supabase Auth UID. Update the Technical Considerations section to reflect Supabase Auth as the auth mechanism. --- ### A2. Specify recovery code entropy and hashing algorithm **Source:** SECFILE (Finding 5), COMPLIANCE (Finding 4) **Original Finding:** The scope says recovery codes are "hashed before storage" but does not specify: code length/entropy, the hashing algorithm, whether a salt is used, rate limiting on the claim endpoint, or single-use enforcement. A weak recovery code (e.g., 8 characters) or a fast hash (SHA-256 without salt) can be brute-forced. **PM Assessment:** Valid and must be decided before implementation. This is a security design gap — the scope makes a promise ("hashed") without specifying how. The implementation choices here (bcrypt vs. Argon2id, 16 chars vs. 24 chars) need to be locked in before Phase 1.5 is built, not discovered mid-implementation. The fix is low-complexity: pick Argon2id (it is the modern recommendation over bcrypt), generate at least 128 bits of entropy (24 alphanumeric characters), add rate limiting on the claim endpoint, and invalidate the code after a successful claim. Both reports agree on these specifics. **Recommendation:** MUST DO (before code) **Scope Impact:** Add entropy and algorithm specification to the Data Model section under `recovery_code`. Add rate limiting requirement to Phase 1.6. --- ### A3. Define and enable Supabase RLS policies on every table **Source:** TECHFILE (Finding 3), SECFILE (Findings 1, 2, 6), COMPLIANCE (Finding 14) **Original Finding:** All three reports flag this independently. No RLS policies are specified anywhere in the scope. Without them, the publicly-exposed Supabase anon key grants full read/write access to every row in every table. This affects: user data isolation, group data isolation, movie list privacy, admin session protection, and real-time subscription authorization. Supabase Realtime also respects RLS — without it, any client can subscribe to any group's movie changes. **PM Assessment:** Valid and critical. Flagged by all three reports — this is the most unanimously agreed-upon finding. RLS is not optional on Supabase; it is the fundamental authorization mechanism. The anon key is deliberately public. Every data access rule the scope describes (only group members can see a group's movies, only admins can remove members, etc.) must be enforced at the RLS layer. This cannot be retrofitted after data access code is written without touching every query. Define policies at the same time as the schema in Phase 1.2. **Recommendation:** MUST DO (before code) **Scope Impact:** Add RLS policy definitions to Phase 1.2 as a required subtask alongside schema creation. Add a note to the Technical Considerations section specifying that RLS is enabled on all tables with explicit policies (no permissive catch-all). --- ### A4. Replace all Vercel Cron references with Supabase pg_cron **Source:** TECHFILE (Finding 1) **Original Finding:** The scope explicitly plans two background jobs using Vercel Cron: the bi-weekly landing reel poster refresh (Phase 5.2) and the trailer URL refresh (Phase 6.2). In a Docker deployment, `vercel.json` cron entries are ignored entirely — these jobs will never run. The landing page reel refresh is an MVP deliverable (Phase 5); it will be broken on the first Docker deployment. **PM Assessment:** Valid and critical for this project's deployment target. The scope was written with a Vercel-first assumption that conflicts with the stated deployment target. Since Supabase is self-hosted, pg_cron is directly available on the Postgres instance — no free tier limitations apply. Both jobs are fundamentally database operations (fetch from TMDB, write to Postgres), so running them from a pg_cron job that calls a Supabase Edge Function (running via the local Deno runtime in Docker) is architecturally cleaner and deployment-agnostic. This decision affects Phase 5.2 (MVP) and Phase 6.2 (post-MVP). Must be decided before Phase 5 is built; if a Vercel Cron implementation is coded as a placeholder it will need full rewriting. **Recommendation:** MUST DO (before code) **Scope Impact:** Update Phase 5.2, Phase 6.2, and Phase 6.2 task descriptions to reference Supabase pg_cron + Edge Functions (self-hosted Deno runtime) instead of Vercel Cron. Update the Deployment section to remove Vercel Cron as a mechanism. --- ### A5. Proxy all TMDB API calls through Next.js server routes **Source:** TECHFILE (Finding 7), SECFILE (Finding 8), COMPLIANCE (Finding 1) **Original Finding:** All three reports flag this. The TMDB API key must not appear in client-side code or browser network requests. If calls are made from React components, the key is visible in the JS bundle and in DevTools network panel. TMDB's Terms of Use prohibit publicly exposing the API key. A leaked key can be used to exhaust your rate limit and get your key revoked. The landing page's unauthenticated TMDB calls further suggest the current design may intend client-side usage. **PM Assessment:** Valid, mandatory, and doubles as a compliance requirement (TMDB ToS). Routing all TMDB calls through a `/api/tmdb/*` Next.js Route Handler is the correct pattern. It keeps the key server-side, enables server-side caching with `Cache-Control` headers, and allows rate limiting the proxy endpoint. The slight added latency (one extra server hop) is fully offset by TanStack Query's client-side caching (which the scope already plans for). This is a Phase 1 architectural decision — all TMDB-calling code in Phases 3-5 should be written assuming a server-side proxy from the start. **Recommendation:** MUST DO (before code) **Scope Impact:** Add API proxy route creation to Phase 1 (or early Phase 3). Add a note to Technical Considerations that `TMDB_API_KEY` must never use the `NEXT_PUBLIC_` prefix. --- ### A6. Add TMDB attribution to every page **Source:** COMPLIANCE (Finding 2) **Original Finding:** TMDB's Terms of Use require visible attribution on any app using their API: the TMDB logo, a link to themoviedb.org, and the disclaimer text "This product uses the TMDB API but is not endorsed or certified by TMDB." The scope makes no mention of this anywhere. **PM Assessment:** Valid and non-negotiable. This is a Terms of Service requirement, not a best practice. Violating it risks API key revocation, which would break the entire app. It is a low-effort fix: add the logo and text to a site footer that appears on all pages. Should be in Phase 1 or Phase 5 at the latest. Notably, this is the only finding from the compliance report that is both critical and low-effort — no architectural changes required, just a UI element. **Recommendation:** MUST DO (before code) **Scope Impact:** Add a footer component requirement to Phase 1 (or Phase 5.1). Add a note to Technical Considerations referencing TMDB attribution requirements. --- ### A7. Configure `output: 'standalone'` and Docker base image before writing code **Source:** TECHFILE (Findings 5, 6) **Original Finding:** Two related infrastructure decisions that must be made before the first Docker build. (1) Without `output: 'standalone'` in `next.config.ts`, the Docker image will include the full `node_modules` tree and balloon to 500MB–1GB+. (2) Node.js 18 is EOL; Node.js 20 enters maintenance mode now (April 2026). A new project should start on Node.js 22 LTS. **PM Assessment:** Valid. Both are trivial configuration decisions that have zero cost to implement now and significant cost to change later (a Node version upgrade mid-project can break native dependencies; removing `output: 'standalone'` from a Dockerfile structure requires restructuring the entire multi-stage build). Pin Node 22 LTS (`node:22-slim`) and set `output: 'standalone'` in `next.config.ts` before Phase 1.1 is committed. TECHFILE notes one gotcha: `public/` and `.next/static/` must be copied separately in the Dockerfile since they are not included in standalone output. With self-hosted Supabase, the docker-compose.yml will reference both the Next.js app image and the Supabase service images — the Node version and standalone output only apply to the Next.js app container. **Recommendation:** MUST DO (before code) **Scope Impact:** Update Deployment section to specify Node.js 22 LTS, `output: 'standalone'`, and self-hosted Supabase Docker stack. Update Phase 1.7 to include Docker configuration validation for both the app and Supabase services. --- ## MUST DO (during Phase 1) _These need to be built early to avoid blocking later phases, but do not require a decision before the first commit._ --- ### B1. Add Docker infrastructure tasks to Phase 1 **Source:** TECHFILE (Finding 8), SECFILE (Finding 12), COMPLIANCE (Finding 22) **Original Finding:** All three reports flag the Docker deployment gap. The scope has no Dockerfile, no docker-compose.yml, no health check endpoint, no reverse proxy specification, and no non-root user configuration. Running the container as root, baking secrets into the image, and having no health check are the specific risks called out. **PM Assessment:** Valid. Docker infrastructure is not optional infrastructure for a Docker-deployed app — it is the deployment plan. Since Supabase is also self-hosted, the Docker setup must orchestrate the full Supabase stack (Postgres, GoTrue, Realtime, PostgREST, Kong, Studio, etc.) alongside the Next.js app via docker-compose. The specific items needed before Phase 2 work begins: multi-stage Dockerfile for the Next.js app with `node:22-slim`, non-root user, `.dockerignore`, health check endpoint (`/api/health`), Supabase's official docker-compose.yml adapted for this project, and a Caddy or Traefik reverse proxy for HTTPS (required for service workers, Supabase Realtime's `wss://`, and secure cookies). The compliance report also notes that `tini` should be used for PID 1 signal handling. This work is currently only in Phase 5.7 ("Deploy skeleton to Vercel") and Phase 10.1 ("Final Vercel production deployment") — both wrong deployment target and too late. **Recommendation:** MUST DO (during Phase 1) **Scope Impact:** Add Docker infrastructure subtasks to Phase 1.7 (or a new 1.8). Update Deployment section to describe Docker + self-hosted Supabase + Caddy/Traefik architecture. Remove Vercel-specific deployment language from Phases 1 and 10. Add Supabase self-hosted docker-compose setup to Phase 1.2. --- ### B2. Invite code entropy increase and rate limiting **Source:** TECHFILE (Finding 9), SECFILE (Finding 3), COMPLIANCE (Finding 5) **Original Finding:** All three reports flag this independently. The `WOLF-42` format (word + 2-digit number) has ~100,000–200,000 combinations, which is trivially enumerable. Since the invite code is the sole access control for group membership, a brute-force attack lets an attacker join any group. All three reports also call for rate limiting on the join endpoint (5–10 failed attempts per IP per time window). **PM Assessment:** Valid. The entropy issue and rate limiting issue are separable: rate limiting is the higher-priority fix (it makes brute force impractical even with low entropy), and the format change is a secondary improvement. For rate limiting: implement server-side middleware on the join-code endpoint before Phase 2 ships. For code format: the scope can keep human-readable codes but should expand to at least `WORD-WORD` or `WORD-4DIGITS` to reach millions of combinations. This is a Phase 2 decision (group creation), so it needs to be locked in before Phase 2.1 is coded. **Recommendation:** MUST DO (during Phase 1) **Scope Impact:** Update Phase 2.1 to specify the expanded invite code format. Add a rate limiting requirement to Phase 2.2 (join flow). Optionally update the Data Model section's invite code description. --- ### B3. Admin session: HttpOnly cookie with iron-session (or equivalent) **Source:** TECHFILE (Finding 10), SECFILE (Finding 7) **Original Finding:** The scope notes `admin_sessions` as a "secure server-side token store" but does not specify the mechanism. A naive implementation (plain cookie, localStorage state) would be insecure for a panel that controls global deletion. The TOTP secret also has no rotation mechanism and no session expiry specified. **PM Assessment:** Valid. The admin session mechanism needs to be decided before Phase 7 is built — but Phase 7 is post-MVP (May 4-17). The recommendation to use `iron-session` (encrypted cookie, no DB required) is solid for a single-admin use case. The admin session should expire after 8 hours (or shorter with activity extension) and be bound to an HttpOnly, Secure, SameSite=Strict cookie. The TOTP rotation gap (requires redeployment to change the secret) is a documented operational constraint that is acceptable for MVP — document it explicitly rather than building a rotation UI. Also: remove `is_master_admin` from the `users` table (see item D1 below) since admin status is determined by the session, not a DB flag. **Recommendation:** MUST DO (during Phase 1) — decide the session mechanism now; implement in Phase 7 **Scope Impact:** Update Phase 7 tasks to specify `iron-session` (or equivalent). Add session expiry requirement. Add TOTP rotation procedure documentation to Phase 7.1. Consider removing `is_master_admin` from the Data Model. --- ### B4. Input validation for display names and group names **Source:** SECFILE (Findings 9, 15), COMPLIANCE (implied) **Original Finding:** User-entered display names and group names have no validation rules. Both are rendered throughout the app in other users' browsers. Without server-side length limits and character restrictions, these fields are potential stored XSS vectors and layout-breaking inputs. **PM Assessment:** Valid. React's default JSX escaping provides strong baseline XSS protection (it escapes `<`, `>`, `&`, etc. in rendered text), so stored XSS via display names is low-risk as long as developers consistently avoid `dangerouslySetInnerHTML`. However, the SECFILE recommendation to add `CHECK` constraints in Supabase and server-side validation is still correct: it's a one-time setup that prevents garbage data and gives a clear contract. Recommended limits: display name 1-30 characters, group name 1-50 characters, basic character set that allows Unicode letters (don't restrict to Latin-only — international users exist) but rejects HTML angle brackets and control characters. Add these to the schema in Phase 1.2. **Recommendation:** MUST DO (during Phase 1) **Scope Impact:** Add CHECK constraints to Phase 1.2 schema creation. Minimal scope document change needed — could be a note under Data Model. --- ### B5. Environment variable validation at startup **Source:** COMPLIANCE (Finding 24) **Original Finding:** Five required environment variables are listed in the scope but nothing validates them at startup. Missing or malformed variables will produce cryptic runtime errors rather than a clear startup failure. **PM Assessment:** Valid and trivially easy. Using `t3-env` or `zod` to validate env vars at build/startup time is a 30-minute task with zero downside. Catches misconfigured deployments immediately. Should be in Phase 1.3 (when Supabase env vars are first configured). This is one of the lowest-effort, highest-reliability items in the entire audit. **Recommendation:** MUST DO (during Phase 1) **Scope Impact:** Add env validation library to Phase 1.3 task. No scope document structural changes needed. --- ## MUST DO (during Phase 3 or later) _These are valid and important but can be deferred until the relevant feature phase._ --- ### C1. HTTP security headers (CSP, HSTS, X-Frame-Options, etc.) **Source:** SECFILE (Finding 13) **Original Finding:** No security headers are specified. Without them: clickjacking via iframes is possible, XSS is easier to exploit, MIME sniffing can be weaponized, and HSTS is absent. The recommended header set includes CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy, and HSTS. **PM Assessment:** Valid. Security headers are configured once in `next.config.js` (or at the reverse proxy level for Docker) and apply globally. The main complexity is getting the CSP right: Tailwind may need `style-src 'unsafe-inline'`, TMDB images need `img-src https://image.tmdb.org`, and Supabase Realtime needs `connect-src wss://*.supabase.co`. The SECFILE provides a reasonable starting CSP — use `Content-Security-Policy-Report-Only` mode during development to catch violations without blocking. This should be done during Phase 5 (Landing Page and MVP Polish) since the CSP must account for all third-party resources once the app is complete. **Recommendation:** MUST DO (during Phase 5) **Scope Impact:** Add security headers task to Phase 5 (MVP Polish) or Phase 1 initial config. No structural scope changes needed. --- ### C2. Define the TMDB image strategy before Phase 3 **Source:** TECHFILE (Findings 2, 13), COMPLIANCE (Findings 16, 26) **Original Finding:** Multiple related findings across two reports converge on the same decision. In Docker, Next.js `` optimization runs `sharp` in-container (CPU/memory pressure under load). TMDB already serves posters at discrete sizes (`w92`, `w154`, `w185`, `w342`, `w500`). Using TMDB's native sized URLs with a plain `` tag bypasses the optimization pipeline entirely and uses TMDB's own CDN — better for performance, simpler for Docker, and avoids any ToS gray area around re-serving images through the Next.js proxy. COMPLIANCE flags a ToS concern: if Next.js image optimization proxies TMDB images through the app's own domain, this may violate TMDB's requirement that images be served from their CDN. **PM Assessment:** Valid, and the two reports converge on the same solution. Decision: use TMDB native sized URLs directly (`w342` for grid on mobile, `w185` for reel animation, `w500` for expanded panel) with plain `` tags (or ``). Install `sharp` as an explicit production dependency anyway (for any locally-served assets where optimization is valuable, like the logo). Add `image.tmdb.org` to `remotePatterns` in `next.config.js` regardless. This decision must be made before Phase 3 (Movie List Core) since the poster grid is the first TMDB image-heavy component. **Recommendation:** MUST DO (during Phase 3) **Scope Impact:** Add an image strategy note to Technical Considerations. Add `sharp` installation to Phase 1.1 setup. No major structural changes. --- ### C3. Real-time subscription lifecycle management **Source:** SECFILE (Finding 6), COMPLIANCE (Finding 17) **Original Finding:** Two reports flag the real-time subscriptions issue from different angles. SECFILE: without RLS, subscriptions are unauthorized. COMPLIANCE: subscription count limits, reconnection logic, and cleanup in `useEffect` return functions are unaddressed. Users in multiple groups should only subscribe to the currently-viewed list, not all lists simultaneously. **PM Assessment:** Valid. The RLS piece is already covered by A3. The lifecycle management piece (subscribe on mount, unsubscribe on unmount, one subscription per viewed list, exponential backoff reconnection) is a Phase 3.9 implementation detail that should be specified before that task is built. The compliance report's suggestion to use polling for home page movie counts (rather than full subscriptions) instead of maintaining subscriptions for all groups is pragmatic and correct. Not a scope document change — more of an implementation constraint for Phase 3.9. **Recommendation:** MUST DO (during Phase 3) **Scope Impact:** Add subscription lifecycle requirements as a note to Phase 3.9. Minimal structural scope change. --- ## SHOULD DO _Valid findings that meaningfully improve quality, security, or compliance but are not blockers._ --- ### D1. Remove `is_master_admin` from the users table **Source:** SECFILE (Finding 20) **Original Finding:** The `is_master_admin (boolean, default false)` column on the users table is redundant — admin status is determined by a valid TOTP-authenticated session, not a DB flag. Worse, if RLS policies are misconfigured, this flag could be set by a client to self-escalate privileges. **PM Assessment:** Valid and simple fix. Remove the column from the Data Model. Admin status checked via the session only. Zero implementation cost since no code exists yet. This is a pre-code decision with no downside. **Recommendation:** SHOULD DO **Scope Impact:** Remove `is_master_admin` from the users table definition in Section 7. --- ### D2. Add serwist instead of next-pwa for Phase 8 PWA support **Source:** TECHFILE (Finding 4) **Original Finding:** `next-pwa` (the most commonly referenced PWA package) is effectively unmaintained since 2022 — no App Router support, open security issues, outdated Workbox. The community-maintained successor `@serwist/next` supports App Router, Workbox 7, TypeScript, and is actively maintained. **PM Assessment:** Valid and a simple substitution. Phase 8 (PWA) is post-MVP and well ahead, so this is not urgent. When Phase 8 begins, use `@serwist/next` from the start rather than `next-pwa`. The API is similar; the main difference is writing the service worker in `app/sw.ts`. **Recommendation:** SHOULD DO **Scope Impact:** Update Phase 8.1 to reference `@serwist/next` instead of `next-pwa`. --- ### D3. Implement structured logging from Phase 1 (not Phase 10) **Source:** SECFILE (Finding 18), COMPLIANCE (Finding 23) **Original Finding:** The scope defers error monitoring to Phase 10.3 ("basic error monitoring — Vercel logs + Sentry free tier"). For a Docker-deployed app, structured logging (JSON to stdout, captured by Docker logging driver) must be in place from Phase 1 to debug issues in production. Sentry error tracking is also more valuable when added early (it catches issues across the entire development period, not just post-launch). **PM Assessment:** Mostly valid. The COMPLIANCE report makes the stronger case: logging is harder to retrofit and production issues during the soft launch (Phase 10.4) will be impossible to investigate without it. However, full structured logging with `pino` in Phase 1 is overkill for a solo developer or small team. A pragmatic middle ground: add Sentry in Phase 3 (when real features are being built), use `console.error` with structured JSON early on, and do a proper logging pass in Phase 9 or 10 before the soft launch. The Phase 10.3 placement for monitoring is too late for a Docker deployment. **Recommendation:** SHOULD DO **Scope Impact:** Move Sentry/monitoring setup from Phase 10.3 to Phase 3 or Phase 5. Update Phase 10.3 to be a verification step rather than initial setup. --- ### D4. Establish linting, TypeScript strict mode, and Prettier before first commit **Source:** COMPLIANCE (Finding 7) **Original Finding:** No code quality tooling is specified. ESLint, Prettier, TypeScript strict mode, and a pre-commit hook should be established before any code is written. **PM Assessment:** Valid. This is a half-day setup that prevents significant debt accumulation. TypeScript strict mode in particular catches a class of bugs that are expensive to fix later if the codebase grows without it. `next/core-web-vitals` + `next/typescript` ESLint presets cover most of the important rules. `husky` + `lint-staged` for pre-commit enforcement is standard. Should be in Phase 1.1 alongside project initialization. **Recommendation:** SHOULD DO **Scope Impact:** Add linting/TypeScript setup subtask to Phase 1.1. --- ### D5. Define a minimum automated testing strategy **Source:** COMPLIANCE (Finding 8) **Original Finding:** Phase 9 is entirely manual QA. No automated tests are specified at any level. For a real-time collaborative app with complex state (group membership, watched state, cross-list rolls), manual testing alone will produce regressions. **PM Assessment:** Partially valid. Full test coverage from Phase 1 is overkill for a solo developer on an MVP deadline of April 26. However, the scope has complex business logic (emotion-to-genre mapping, ownership transfer flows, RLS policy enforcement) that is excellent candidate for unit tests. Recommended pragmatic approach: add Vitest for unit tests of pure logic (invite code generation, emotion mapping, recovery code hashing) in Phase 1. Add Playwright E2E for the three critical paths (onboarding, add movie + real-time sync, roll the dice) in Phase 4 or 5. Skip component-level tests for MVP. This is less than the COMPLIANCE report recommends but more than the current scope has. **Recommendation:** SHOULD DO **Scope Impact:** Add Vitest setup to Phase 1.1. Add Playwright E2E for critical paths to Phase 5 (MVP Polish) or Phase 9. No full CI gate needed for MVP deadline. --- ### D6. `prefers-reduced-motion` support for both animations **Source:** COMPLIANCE (Finding 30) **Original Finding:** Both animations (slot-machine reel, scatter/eliminate) have no motion preference handling. WCAG 2.3.3 and general accessibility best practice require respecting the `prefers-reduced-motion` media query. Users with vestibular disorders can be physically affected by spinning/scattering animations. **PM Assessment:** Valid and achievable. This is a CSS media query check plus a fallback state for both animations. For the landing page reel: instant reveal or simple fade-in when reduced motion is preferred. For the in-app roll: fade-in on winner rather than scatter animation. The full animation is still the default. This is genuinely WCAG-relevant (not theoretical) given that the animations are the visual centerpiece. Should be implemented in Phase 4 (Randomizer) and Phase 5 (Landing Page) alongside the animation builds. Does not require a separate implementation phase. **Recommendation:** SHOULD DO **Scope Impact:** Add `prefers-reduced-motion` note to both animation implementation tasks (Phase 4.3, Phase 5.3) and to the Usability Concerns section. --- ### D7. Inline panel keyboard navigation and focus management **Source:** COMPLIANCE (Finding 31) **Original Finding:** The inline panel expansion has no specified keyboard navigation: how a keyboard user opens/closes the panel, focus management on open/close, Escape key to close, `aria-expanded` on trigger, `role="region"` on panel. **PM Assessment:** Valid accessibility requirement. The inline panel is the primary interaction for viewing movie details. Without focus management, screen reader users and keyboard users cannot use the core app feature. Phase 8.3 already has an "accessibility pass" task — this work belongs there. The scope's Usability Concerns section mentions 44x44px tap targets and screen reader labels on icon buttons, so accessibility is already on the radar; the inline panel focus management is an extension of that. **Recommendation:** SHOULD DO **Scope Impact:** Add inline panel keyboard/focus management requirements to Phase 8.3. Add a note to Section 6 (Usability Concerns) about keyboard navigation for the inline panel. --- ### D8. Implement a CI pipeline before the MVP cutoff **Source:** COMPLIANCE (Finding 9) **Original Finding:** No CI/CD pipeline is specified. For Docker deployment: a GitHub Actions workflow running lint, type-check, and build on every PR; Docker image build verification; dependency vulnerability scanning. **PM Assessment:** Valid but scoped to what's realistic for the April 26 MVP deadline. A full CI pipeline with E2E tests and staging environments is post-MVP. The minimum viable CI for this project is: lint + type-check on every push (5-minute setup via GitHub Actions), and a Docker build test before the Phase 5.7 deployment. Dependabot/Renovate for dependency scanning can be enabled in one click and runs automatically. This is the practical floor, not the ceiling. **Recommendation:** SHOULD DO **Scope Impact:** Add GitHub Actions setup to Phase 1 or Phase 5. Minimal — a note in the Deployment section. --- ### D9. Add privacy policy page **Source:** COMPLIANCE (Finding 3, 27, 29) **Original Finding:** Three related compliance findings: (1) No privacy policy exists. (2) The ePrivacy Directive requires disclosure of any persistent identifiers stored on the user's device. (3) No data retention periods are defined. The compliance report also notes the privacy section's claim that "no personal data beyond display name is stored" is incorrect under GDPR — UUIDs, group membership data, movie preferences, and IP addresses in server logs are all personal data. **PM Assessment:** Partially valid. MovieDice's data footprint is genuinely minimal (anonymous UUID, display name, movie preferences — no email, no location, no payment info), which simplifies the privacy policy. The ePrivacy Directive concern about storing user IDs is satisfied if the storage is "strictly necessary" for the service to function (it is). The GDPR right-to-erasure issue is covered by item D10 below. A short, honest privacy policy page that accurately describes what is stored is sufficient and should be part of the Phase 5 landing page build. No legal review is required for a simple hobbyist/small-scale app, but the document should be factually accurate. **Recommendation:** SHOULD DO **Scope Impact:** Add a privacy policy page to Phase 5.1 (landing page build). Add footer with privacy policy link to the landing page layout. --- ### D10. Self-service account deletion for users **Source:** SECFILE (Finding 16), COMPLIANCE (Finding 28) **Original Finding:** No self-service account deletion flow exists. GDPR Article 17 (Right to Erasure) requires users can request deletion of their data. The scope only allows Master Admin deletion of users — there is no user-facing delete option. **PM Assessment:** Valid. Both reports flag this. The implementation is complex (cascading deletes, ownership transfer for groups the user admins, anonymizing `added_by` references in movies) but the feature itself belongs in the post-MVP plan, not the MVP. The compliance gap is partially mitigated by the Master Admin's ability to delete users on request — acceptable for MVP where real user volume is minimal. This should be added to the Extra Features table now so it is not forgotten, with a target of Phase 8 or 9. **Recommendation:** SHOULD DO **Scope Impact:** Add to Extra Features table or post-MVP roadmap. Add cascade delete behavior notes to the Data Model section for `users`. --- ### D11. Database migration strategy using Supabase migrations **Source:** COMPLIANCE (Finding 13) **Original Finding:** No schema migration strategy is specified. Ad-hoc schema changes during development are untrackable and irreproducible across environments. **PM Assessment:** Valid. Using `supabase migration new` and storing migrations in version control is the correct approach. This is a Phase 1 setup decision with no ongoing overhead once established. Supabase's CLI makes this straightforward. Must be in place before the Phase 1.2 schema is first created. **Recommendation:** SHOULD DO **Scope Impact:** Add migration workflow note to Phase 1.2. --- ## NICE TO HAVE _Real improvements, but low priority relative to everything above. Post-MVP or opportunistic._ --- ### E1. Virtualized scrolling for the movie grid **Source:** COMPLIANCE (Finding 18) **Original Finding:** Infinite scroll appends cards to the DOM indefinitely. For large lists (50-200+ movies with full-bleed poster images), all loaded cards remain in the DOM simultaneously, causing memory bloat on low-end mobile devices. `@tanstack/react-virtual` or `react-window` would keep DOM node count constant. **PM Assessment:** Real concern but a post-MVP optimization. The complicating factor is the inline panel expansion: it inserts a full-width element between grid rows, which breaks simple list virtualization. A virtualized grid with inline expansion is a non-trivial implementation. For MVP, 12 items initially + infinite scroll is reasonable; most groups won't hit 50+ movies in early usage. Revisit if performance testing in Phase 8.4 shows actual issues. **Recommendation:** NICE TO HAVE **Scope Impact:** None for MVP. Could be added to Phase 8.4 (Performance Tuning) as a conditional task. --- ### E2. Add a `/api/health` endpoint **Source:** SECFILE (Finding 12), COMPLIANCE (Findings 22, 23) **Original Finding:** Multiple reports call for a health check endpoint for Docker `HEALTHCHECK` and uptime monitoring. Should check Supabase connectivity and return a structured response. **PM Assessment:** Valid and genuinely low effort. A health endpoint is one file, ~20 lines. Valuable for Docker `HEALTHCHECK` (prevents traffic routing to an unhealthy container) and for uptime monitoring tools. Should be in Phase 1.7 alongside the Docker skeleton deployment. **Recommendation:** NICE TO HAVE (but very easy — could be SHOULD DO) **Scope Impact:** Add to Phase 1.7. --- ### E3. Trailer URL validation before storage **Source:** SECFILE (Finding 17) **Original Finding:** Trailer URLs are fetched from TMDB and stored as-is. If a stored URL is corrupted or manipulated, users could be redirected to a malicious site. Validate URLs match expected YouTube/TMDB patterns before storage. **PM Assessment:** Low risk in practice. TMDB is the data source and is a trusted third party. The realistic attack vector here is extremely narrow. However, adding `rel="noopener noreferrer"` to the trailer link (already standard practice in React for `target="_blank"`) and a domain allowlist check (must be youtube.com or themoviedb.org) before storage is a 10-line addition that is worth including in Phase 3.3 (add-movie flow). **Recommendation:** NICE TO HAVE **Scope Impact:** Add URL validation note to Phase 3.3. --- ### E4. Add a `--nnnn` or `WORD-WORD` format for CORS on API routes **Source:** SECFILE (Finding 14) **Original Finding:** No CORS configuration is specified on API routes. For a same-origin app served from a Docker container, CORS misconfiguration on API routes (e.g., `Access-Control-Allow-Origin: *`) would allow any website to make authenticated requests. **PM Assessment:** Lower priority than the other security findings. For a Docker deployment where frontend and API are on the same origin, CORS is not an active concern — the browser's same-origin policy handles it. The risk is only if someone writes a new API route and adds a permissive CORS header. The fix is: add a linting rule or code review note. Not worth a scope document change. **Recommendation:** NICE TO HAVE **Scope Impact:** None — a developer convention note. --- ### E5. Invite code expiry **Source:** SECFILE (Finding 3) **Original Finding:** The SECFILE suggests invite codes should have an optional expiry (e.g., 7 days) so old codes cannot be used indefinitely. **PM Assessment:** Nice-to-have. The scope already includes invite code regeneration (the admin's tool for revoking access), which addresses the main use case. Automatic expiry adds complexity to the join flow ("this code has expired" error state) for limited benefit given that regeneration is available. Post-MVP consideration. **Recommendation:** NICE TO HAVE **Scope Impact:** None for MVP. --- ## SKIP / DEFER _Either overkill for this project's scale and stage, or the concern is not actionable._ --- ### F1. Full CI/CD with staging environment **Source:** COMPLIANCE (Finding 9) **PM Assessment:** Defer. A full staging environment with pre-production validation and Docker image smoke tests is reasonable for a team project but overkill for an MVP solo build. The minimum CI (lint + type-check + build) is captured in D8 above. A staging environment can be added post-launch. **Recommendation:** SKIP/DEFER --- ### F2. API route documentation with OpenAPI spec **Source:** COMPLIANCE (Finding 11) **PM Assessment:** Skip for MVP. This is a documentation standard appropriate for a team with external API consumers. For a single-developer project with internal-only routes, JSDoc headers on Route Handlers are sufficient. OpenAPI generation can be added if the project grows a team. **Recommendation:** SKIP/DEFER --- ### F3. CAPTCHA or proof-of-work on public endpoints **Source:** SECFILE (Finding 11) **PM Assessment:** Skip for MVP. IP-based rate limiting on public endpoints (covered in item A5 via server-side middleware) is sufficient to deter automated abuse at this scale. CAPTCHA adds meaningful UX friction for users who have "low tolerance for signup friction" (per the scope's own user trait). Revisit if abuse is observed post-launch. **Recommendation:** SKIP/DEFER --- ### F4. Password alongside TOTP for Master Admin **Source:** SECFILE (Finding 7) **PM Assessment:** Skip. The scope explicitly says "no password-only fallback" and uses username + TOTP. Adding a password would require storing and managing a password credential, adding a password reset flow, and contradicting the established design decision. TOTP alone (when implemented correctly with IP binding and session expiry per item B3) is sufficient for a single-admin panel used by the site owner. **Recommendation:** SKIP/DEFER --- ## NEEDS DISCUSSION _These findings involve real tradeoffs or design tensions that the user should weigh in on._ --- ### G1. Emotion-to-genre mapping: static TypeScript constant vs. database table **Source:** TECHFILE (Finding 14), COMPLIANCE (Finding 10) **PM Assessment:** Both reports agree the mapping should not be hardcoded as if-else logic, but diverge on the right home for it. TECHFILE recommends a static TypeScript `const` object in a config file (never changes without a deploy). COMPLIANCE recommends a database table (allows runtime updates without a deploy). For MVP, the TypeScript constant is clearly correct — the mapping is fixed, known at build time, and never changes without a deliberate decision. The "move to DB post-MVP for runtime updates" suggestion from COMPLIANCE is a valid future option but adds a query per Genre Roll with no immediate benefit. The scope's Section 10 table is the right source of truth; it just needs to be translated to TypeScript numeric TMDB genre IDs during Phase 4.6 implementation. **Recommendation:** NEEDS DISCUSSION (recommend static TypeScript constant for MVP) **Scope Impact:** None — implementation detail for Phase 4.6. --- ### G2. Offline caching strategy for the PWA **Source:** COMPLIANCE (Finding 33) **PM Assessment:** The scope says "show cached list, disable write actions." The compliance report asks for more detail: what is precached (app shell, fonts, CSS/JS), how is list data cached (TanStack Query persistence plugin or service worker), and what happens to in-flight writes when connection drops. The "do not queue offline writes" recommendation in the compliance report is the right call for MVP (conflict resolution is a v2 problem). But the specific caching library approach (TanStack Query `persistQueryClient` vs. service worker cache) needs a decision before Phase 8.2 is built. **Recommendation:** NEEDS DISCUSSION (decide caching mechanism before Phase 8) **Scope Impact:** Phase 8.2 description could be more specific about the caching strategy once decided. --- ### G3. GDPR data retention periods **Source:** COMPLIANCE (Finding 29) **PM Assessment:** The compliance report recommends auto-deleting inactive accounts after 12 months. For an app without email, notifying users before deletion is impossible — they could lose their account silently. This is a genuine design tension: GDPR compliance favors bounded retention, but the user experience favors persistence. The pragmatic MVP answer: store data indefinitely (the data footprint is minimal and the compliance risk is low at small scale), add account deletion on request via Master Admin, and revisit retention policy before any public marketing or scale-up. Add retention policy to the privacy policy page (item D9) as a deferred decision note. **Recommendation:** NEEDS DISCUSSION **Scope Impact:** Add a retention policy note to the Privacy section. --- ### G4. Pre-render the landing page as a static page with ISR **Source:** TECHFILE (Finding 12) **PM Assessment:** The TECHFILE recommends making the landing page root layout a Server Component with ISR-fetched reel posters (revalidate every ~12 hours instead of fetching from DB on every request) and a thin Client Component wrapper for the localStorage redirect check. This is architecturally sound and directly addresses the user's stated priority of fast page loads. However, it requires careful Server/Client Component boundary design: the redirect check must be in a Client Component (localStorage is browser-only), while the static shell can be a Server Component. The risk is a hydration mismatch if the boundaries are drawn incorrectly. This is worth doing but requires developer familiarity with Next.js App Router's Server/Client Component model. **Recommendation:** NEEDS DISCUSSION (recommend doing it; assess team familiarity with App Router patterns first) **Scope Impact:** Phase 5.1 could be updated to specify Server Component + Client Component boundary design for the landing page. --- ## Conflicts Between Reports ### Conflict 1: next/image vs. plain `` for posters **TECHFILE** recommends bypassing `next/image` for TMDB poster images entirely and using plain `` with TMDB's sized URLs. This avoids Docker's in-container `sharp` overhead and uses TMDB's CDN directly. **COMPLIANCE** recommends _using_ Next.js `` component with a custom TMDB loader for all poster images, citing lazy loading, blur placeholders, and WebP conversion. **PM Resolution:** TECHFILE's recommendation is correct for this deployment target. The COMPLIANCE report was not written with Docker deployment context in mind. In Docker, image optimization runs in-container and creates CPU/memory pressure under load — the opposite of the user's fast page load goal. TMDB already serves optimized JPEGs from their own CDN at discrete sizes. Using `` with TMDB's native sizes (`w342` for grid, `w500` for expanded panel) is both simpler and faster. Reserve `next/image` for locally-served assets only. Add lazy loading via the native `loading="lazy"` attribute on `` tags (no `next/image` required). --- ### Conflict 2: Severity rating of missing RLS **SECFILE** rates missing RLS as Critical (2 findings at Critical level). **COMPLIANCE** rates it as borderline-critical under Documentation Gaps. **TECHFILE** treats it as Critical (Finding 3). **PM Resolution:** No practical conflict — all three reports agree it must be done. The severity label disagreement is a classification difference, not a substantive disagreement. Treat as Critical (item A3 above). --- ### Conflict 3: When to add monitoring/Sentry **COMPLIANCE** argues Sentry should be added in Phase 1 to catch issues throughout development. **PROJECT_SCOPE.md** places basic error monitoring in Phase 10.3 (final phase). **PM Resolution:** Partially resolved as item D3 above. The recommendation is Phase 3 as a compromise — early enough to be useful during active feature development, without the overhead of setting it up during Phase 1 when the project structure is still being established. The current Phase 10.3 placement is too late for a Docker deployment where server logs are the primary debugging tool. --- ## Notes on Items Not Requiring Scope Changes The following findings from the reports are valid implementation notes but do not require changes to PROJECT_SCOPE.md — they are implementation details: - TECHFILE Finding 14 (emotion map as TypeScript constant): Phase 4.6 implementation note - TECHFILE Finding 7 (TanStack Query `staleTime`): Phase 3.1/4 implementation note - SECFILE Finding 22 (invite code rotation clarification): Documentation note, not a functional gap - COMPLIANCE Finding 12 (error response standard): A development convention to establish in Phase 1; minimal scope impact - COMPLIANCE Finding 15 (reel animation poster size): Phase 5.3 implementation note; use `w185` for reel - COMPLIANCE Finding 19 (cross-list roll query with index): Phase 4.2 implementation note; add composite index - COMPLIANCE Finding 20 (trailer refresh rate limiting): Phase 6.1 implementation note - COMPLIANCE Finding 21 (code splitting / `next/dynamic`): Phase 5 implementation practice; App Router does route-based splitting automatically --- _This assessment is for review only. PROJECT_SCOPE.md has not been modified. Each item above should be reviewed individually before any changes are made to the scope document._