PM_ASSESSMENT.md 47 KB
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:
- Auth model (items A1/A2) — touches every DB query if changed after build
- RLS policies (item A3) — foundational to all data security; retrofitting is painful
- 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 <Image> 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 <img> 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 <img> tags (or <Image unoptimized>). 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 <img> for posters
TECHFILE recommends bypassing next/image for TMDB poster images entirely and using plain <img> with TMDB's sized URLs. This avoids Docker's in-container sharp overhead and uses TMDB's CDN directly.
COMPLIANCE recommends using Next.js <Image> 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 <img> 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 <img> 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
w185for 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. PROJECTSCOPE.md has not been modified. Each item above should be reviewed individually before any changes are made to the scope document.