Skip to content

Comments

Server side video render#13

Merged
epavanello merged 7 commits intomainfrom
feat/ss-video-render
Feb 4, 2026
Merged

Server side video render#13
epavanello merged 7 commits intomainfrom
feat/ss-video-render

Conversation

@epavanello
Copy link
Owner

@epavanello epavanello commented Feb 4, 2026

Summary by CodeRabbit

  • New Features

    • Server-side export option with high‑quality FFmpeg encoding, real‑time progress updates, and downloadable MP4 streaming.
    • API-driven render page exposing a controllable render viewport for automated capture.
  • UI Improvements

    • Export method selector (Server HQ vs Browser) with mode‑specific status, tooltips, and progress messages.
    • New radio-group UI components for selection controls.
  • Chores

    • Icon package import updates, adjusted runtime container (now runs the Node server with FFmpeg & headless browser) and renamed example env vars.

@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

Warning

Rate limit exceeded

@epavanello has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 13 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Adds server-side high-quality video export using Playwright + FFmpeg with token-based auth and SSE progress; adds browser/server export modes in the UI, a render page exposing a Playwright-controlled window API, new radio-group UI components, many icon import/type adjustments, and a Node/Playwright production runtime in Docker.

Changes

Cohort / File(s) Summary
Deployment & Build
Dockerfile, docker-compose.yml, package.json
Switched production image to a Playwright Node runtime, removed nginx/pnpm prepare step, install runtime deps (pnpm, ffmpeg, playwright), expose port 3000 and start Node server; updated package deps (added fluent-ffmpeg dev deps, other dep tweaks); compose now runs app build context and env vars.
Server Render & Streaming
src/lib/server/render-token.ts, src/lib/server/video-renderer.ts, src/routes/api/export/[id]/+server.ts
New in-memory render token store (5min TTL); Playwright-based frame capture; FFmpeg image2pipe MP4 encoding; renderProjectToVideoStream + renderEmitter for progress; POST endpoint streams MP4, GET endpoint exposes SSE progress.
Render Page & Token Validation
src/routes/render/[id]/+page.server.ts, src/routes/render/[id]/+page.svelte
Added token-validated render page that loads project for headless capture and exposes window.__DEVMOTION__ (ready, seek, getConfig) for Playwright-controlled frame seeking and config reporting.
Export UI & Toolbar
src/lib/components/editor/export-dialog.svelte, src/lib/components/editor/toolbar.svelte
Introduced ExportMode (browser
Radio Group UI
src/lib/components/ui/radio-group/index.ts, src/lib/components/ui/radio-group/radio-group.svelte, src/lib/components/ui/radio-group/radio-group-item.svelte
New radio-group components and barrel exports (Root/Item with aliases RadioGroup/RadioGroupItem) used for export-mode selection.
Types & Global Declarations
src/app.d.ts
Added DevMotionAPI interface and augmented Window.__DEVMOTION__ typing.
Layered Integration & Typing Changes
src/lib/layers/registry.ts, src/lib/components/ui/button/button.svelte
Narrowed icon typing: removed ComponentType, LayerMeta.icon and ButtonProps.icon now use Component only.
Icon Import Changes (many files)
src/lib/components/**/*, src/lib/layers/components/*, src/routes/**/*
Replaced numerous lucide-svelte imports with @lucide/svelte; small UI class tweaks (properties panel) and minor type import updates across many components.
Env Example
.env.example
Renamed Google env vars to PRIVATE_GOOGLE_CLIENT_ID / PRIVATE_GOOGLE_CLIENT_SECRET and added trailing newline.

Sequence Diagram

sequenceDiagram
    participant Client
    participant ExportDialog
    participant API as /api/export/[id]
    participant DB as Database
    participant Renderer as VideoRenderer
    participant Playwright
    participant FFmpeg
    participant RenderPage as /render/[id]

    Client->>ExportDialog: Start export (server mode)
    ExportDialog->>API: POST ?renderId
    API->>DB: Load project by id
    DB-->>API: project config
    API->>Renderer: renderProjectToVideoStream(config, renderId)
    Renderer->>Renderer: generateRenderToken(projectId)
    Renderer->>Playwright: launch & navigate to /render/[id]?token=...
    Playwright->>RenderPage: load and validate token
    RenderPage-->>Playwright: expose __DEVMOTION__ (ready, seek, getConfig)
    Renderer->>Playwright: seek frames via __DEVMOTION__.seek()
    Playwright->>Renderer: capture PNG frames
    Renderer->>FFmpeg: pipe frames (image2pipe)
    FFmpeg-->>Renderer: encoded MP4 stream
    Renderer-->>API: Readable stream
    API-->>Client: stream response (video/mp4)
    par SSE progress
      Renderer->>Renderer: emit progress events (initializing, capturing, encoding, done)
      ExportDialog->>API: GET SSE ?renderId
      API-->>ExportDialog: stream progress events
      ExportDialog->>Client: update UI progress
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 I hopped through tokens, frames, and streams,
I nudged Playwright softly into dreams;
FFmpeg hummed while pixels took their place,
Progress beeps and downloads tied with grace;
A rabbit cheers — export lands with pace! 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Server side video render' clearly summarizes the main change: implementing server-side video rendering. The PR adds extensive server-side rendering infrastructure including render tokens, video rendering utilities, API endpoints, and related UI updates across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/ss-video-render

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 Fix all issues with AI agents
In `@Dockerfile`:
- Around line 31-36: The production stage is missing the pnpm-workspace.yaml
copied in the builder stage which can break pnpm install; update the Dockerfile
to also COPY the pnpm-workspace.yaml from the builder image into the production
image (alongside the existing COPY --from=builder /app/package.json and
/app/pnpm-lock.yaml lines) so that the RUN npm install -g pnpm && pnpm install
--prod --frozen-lockfile runs with the workspace config present.

In `@package.json`:
- Line 30: Remove the unused devDependency entry "@lucide/svelte": "^0.561.0"
from the package.json devDependencies section (the project already uses
"lucide-svelte" in dependencies), then run your package manager install to
update node_modules and the lockfile so the package is actually removed; ensure
no import references to "@lucide/svelte" remain and commit the updated
package.json and lockfile.

In `@src/lib/components/editor/export-dialog.svelte`:
- Line 50: The exportMode variable is currently created as a read-only $derived
(exportMode) but later reassigned, which is invalid in Svelte 5; change
exportMode to use $state instead and initialize it based on projectId
(exportMode = $state(projectId ? 'server' : 'browser')) so it can be mutated
later in the file (the places that assign to exportMode around the reassignment
points should keep their assignments but now mutate the state-backed
exportMode). Ensure you replace the $derived declaration with the $state
declaration and keep all existing references to exportMode unchanged.
- Around line 99-102: The SSE onerror handler currently only logs to console;
update eventSource.onerror (in export-dialog.svelte) to set the component's
user-visible error state (e.g., set an exportedError or exportErrorMessage
reactive variable and flip any exporting flag such as
isExporting/exportInProgress) so the UI shows a failure message and stops any
spinner, then close the EventSource; ensure you update any cleanup logic in the
same component (handlers around eventSource and the export flow functions) so
the error state is displayed to users and the export UI becomes actionable
again.

In `@src/lib/server/render-token.ts`:
- Around line 3-8: The in-memory tokenStore (const tokenStore = new Map<...>)
and TOKEN_TTL_MS should be migrated to a shared persistence backend (e.g.,
Redis) so tokens are available across multiple Node instances; replace uses of
tokenStore (get/set/delete) with Redis commands (GET/SET with TTL or
HSET/EXPIRE) while preserving TOKEN_TTL_MS as the TTL constant, update any
helper functions that read/write tokenStore to use the Redis client, and ensure
token expiration and cleanup rely on Redis TTLs rather than in-process timers.

In `@src/lib/server/video-renderer.ts`:
- Line 4: The PNG pipe write ignores backpressure: after calling the
stream.write(buffer) that writes frame PNG buffers to the PNG pipe, check its
boolean return value and when it returns false await the stream's 'drain' event
before continuing; implement this by wrapping the await in a Promise that uses
stream.once('drain', resolve) and also handles stream 'error'/'close' to reject,
and use once listeners to avoid leaks (apply to the write call that currently
drops the write() return value in this file; EventEmitter is already imported
and can be used for event handling).
- Around line 54-62: The chromium.launch invocation currently hardcodes insecure
flags (--no-sandbox and --disable-web-security); change it so those two flags
are only added when a development/env toggle is enabled (e.g.,
process.env.PLAYWRIGHT_ALLOW_INSECURE_FLAGS === 'true') and otherwise omit them;
update the code around the browser = await chromium.launch(...) call to build
the args array conditionally (include '--no-sandbox' and
'--disable-web-security' only when the env var is truthy) so production runs use
the safer defaults.
- Around line 123-126: The capture loop should stop when the HTTP client
disconnects because checking frameStream.destroyed is insufficient; modify the
loop that iterates over actualTotalFrames (the for loop using frameIndex and
page.evaluate) to bail out when the downstream videoStream (the PassThrough
piped to the response) is destroyed or when a disconnection flag is set by
listening to the response/stream close event; add a listener on videoStream or
the response to set a local boolean like clientDisconnected, break the loop when
clientDisconnected is true (and cleanly end/close FFmpeg input such as
ffmpeg.stdin/end), and ensure any resources (page, timers, streams) are cleaned
up after breaking.

In `@src/routes/api/export/`[id]/+server.ts:
- Around line 65-72: Sanitize projectData.name before embedding it in the
Content-Disposition header to prevent header injection: strip or replace CR/LF,
quotes, semicolons and other unsafe characters (e.g. replace with underscores),
enforce a length limit and a safe default (e.g. "video") and then use that
sanitizedName in the Content-Disposition value in the new Response call;
optionally provide an encoded filename* (UTF-8) variant for non-ASCII names.
Update the Response construction where Content-Disposition is set (the return
new Response(...) block) to reference the sanitizedName instead of
projectData.name directly.
- Around line 100-109: The cancel callback doesn't call the cleanup function so
onProgress remains registered; move the cleanup variable to a scope shared by
both start and cancel (or define cleanup before the start block) and call
cleanup() inside cancel(), ensuring
renderEmitter.removeListener(`progress:${renderId}`, onProgress) is executed;
update the start and cancel handlers (the code around start() and cancel() in
the +server.ts stream response) to reference the shared cleanup and remove the
listener when cancel() runs.
- Around line 12-27: The POST RequestHandler currently loads a project with
db.query.project.findFirst into dbProject without verifying access; add
authorization after retrieving dbProject in the POST handler: obtain the current
user (e.g., from request.locals or your auth helper), then check that
dbProject.ownerId === currentUser.id OR dbProject.isPublic (or equivalent public
flag) and if neither is true throw error(403, 'Forbidden'); ensure you reference
the POST handler, the dbProject variable, and the project.ownerId /
project.isPublic fields when implementing the check so unauthorized requests
cannot trigger renders.
🧹 Nitpick comments (4)
Dockerfile (2)

38-39: Redundant Playwright browser installation.

The mcr.microsoft.com/playwright:v1.50.0-jammy image already includes all browsers. Running npx playwright install chromium is likely redundant and adds to build time.

Proposed fix
-# Playwright images usually come with browsers, but let's ensure chromium is there
-RUN npx playwright install chromium
+# Note: Playwright official images already include browsers

22-51: Consider adding a non-root user for production security.

The container runs as root by default. For improved security posture, consider running the Node.js server as a non-root user.

Proposed security hardening
 # Playwright images usually come with browsers, but let's ensure chromium is there
 RUN npx playwright install chromium

+# Create non-root user
+RUN useradd -m -s /bin/bash appuser && chown -R appuser:appuser /app
+USER appuser

 # Environment variables
 ENV NODE_ENV=production
src/routes/api/export/[id]/+server.ts (1)

82-84: Unused variable id in GET handler.

The id parameter is destructured but never used. Either remove it or use it for validation if needed.

Proposed fix
-export const GET: RequestHandler = async ({ params, url }) => {
-  const { id } = params;
+export const GET: RequestHandler = async ({ url }) => {
   const renderId = url.searchParams.get('renderId');
src/lib/server/video-renderer.ts (1)

148-152: Emit an encoding phase between capture and completion.

RenderProgress includes encoding, but it's never emitted. The UI stays on capturing while FFmpeg finalizes the video. Emit an encoding update after frameStream.end() to provide accurate progress feedback during encoding.

➕ Suggested update
       frameStream.end();
+
+      emitProgress({
+        phase: 'encoding',
+        currentFrame: actualTotalFrames,
+        totalFrames: actualTotalFrames,
+        percent: 95
+      });

package.json Outdated
"@inlang/paraglide-js": "^2.9.1",
"@internationalized/date": "^3.10.1",
"@lucide/svelte": "^0.563.1",
"@lucide/svelte": "^0.561.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check which lucide package is actually imported in the codebase
echo "=== Imports from `@lucide/svelte` ==="
rg "from '@lucide/svelte'" --type svelte --type ts -c 2>/dev/null || rg "from '@lucide/svelte'" -c
echo ""
echo "=== Imports from lucide-svelte ==="
rg "from 'lucide-svelte'" --type svelte --type ts -c 2>/dev/null || rg "from 'lucide-svelte'" -c
echo ""
echo "=== Also check for import statements ==="
rg "import.*from.*lucide" --type svelte --type ts 2>/dev/null | head -20

Repository: epavanello/devmotion

Length of output: 1878


Remove the unused @lucide/svelte package from devDependencies.

@lucide/svelte (^0.561.0) in devDependencies is not imported anywhere in the codebase. The codebase only uses lucide-svelte (^0.563.0) from dependencies. Remove the unused package to reduce bundle bloat.

🤖 Prompt for AI Agents
In `@package.json` at line 30, Remove the unused devDependency entry
"@lucide/svelte": "^0.561.0" from the package.json devDependencies section (the
project already uses "lucide-svelte" in dependencies), then run your package
manager install to update node_modules and the lockfile so the package is
actually removed; ensure no import references to "@lucide/svelte" remain and
commit the updated package.json and lockfile.

Comment on lines 12 to 27
export const POST: RequestHandler = async ({ params, request, url }) => {
const { id } = params;
const renderId = url.searchParams.get('renderId');

if (!renderId) {
error(400, 'Missing renderId');
}

// Fetch project from DB
const dbProject = await db.query.project.findFirst({
where: eq(project.id, id)
});

if (!dbProject) {
error(404, 'Project not found');
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's session/auth middleware or if other similar endpoints have auth checks
echo "=== Checking for auth patterns in API routes ==="
rg -n "locals\.session|locals\.user|getUser" src/routes/api/ -A2 -B2

echo ""
echo "=== Check if project visibility is considered elsewhere ==="
rg -n "isPublic|userId|owner" src/routes/api/ -A2 -B2

Repository: epavanello/devmotion

Length of output: 173


🏁 Script executed:

#!/bin/bash
# List the API route structure
echo "=== API routes structure ==="
find src/routes/api -type f -name "*.ts" -o -name "*.js" | head -20

echo ""
echo "=== List all files in src/routes ==="
ls -la src/routes/ 2>/dev/null || echo "src/routes not found"

echo ""
echo "=== Check for hooks.ts or middleware files ==="
find src -name "hooks.ts" -o -name "hooks.server.ts" -o -name "middleware.*" | head -10

echo ""
echo "=== Read the export endpoint file ==="
cat src/routes/api/export/[id]/+server.ts

Repository: epavanello/devmotion

Length of output: 4553


🏁 Script executed:

#!/bin/bash
echo "=== Read hooks.server.ts ==="
cat src/hooks.server.ts

echo ""
echo "=== Read hooks.ts ==="
cat src/hooks.ts

echo ""
echo "=== Find render page mentioned in review ==="
find src -path "*render*" -name "*.ts" -o -path "*render*" -name "*.svelte" | head -10

echo ""
echo "=== Check for database schema to understand project ownership ==="
find src -path "*/db/schema*" -type f

echo ""
echo "=== Check if renderId relates to sessions or ownership ==="
rg -n "renderId" src/ -B2 -A2 | head -50

Repository: epavanello/devmotion

Length of output: 4947


🏁 Script executed:

#!/bin/bash
echo "=== Read render page server logic ==="
cat src/routes/render/[id]/+page.server.ts

echo ""
echo "=== Read projects schema ==="
cat src/lib/server/db/schema/projects.ts

echo ""
echo "=== Search for userId/owner checks in routes ==="
rg -n "userId|owner|session|locals\.user" src/routes/ -B2 -A2 | head -80

echo ""
echo "=== Check where export dialog is used ==="
rg -n "export-dialog" src/routes/ -B2 -A2

Repository: epavanello/devmotion

Length of output: 4774


Add authorization check to export endpoint.

The POST handler fetches any project by ID without verifying the requester's access. Unlike the render page (which validates a token), this endpoint allows any user to trigger expensive server-side video renders for any project. Add a check to ensure the project is owned by the current user or is public before rendering.

🤖 Prompt for AI Agents
In `@src/routes/api/export/`[id]/+server.ts around lines 12 - 27, The POST
RequestHandler currently loads a project with db.query.project.findFirst into
dbProject without verifying access; add authorization after retrieving dbProject
in the POST handler: obtain the current user (e.g., from request.locals or your
auth helper), then check that dbProject.ownerId === currentUser.id OR
dbProject.isPublic (or equivalent public flag) and if neither is true throw
error(403, 'Forbidden'); ensure you reference the POST handler, the dbProject
variable, and the project.ownerId / project.isPublic fields when implementing
the check so unauthorized requests cannot trigger renders.

@epavanello epavanello changed the title Feat/ss video render Server side video render Feb 4, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/components/editor/export-dialog.svelte (1)

288-302: ⚠️ Potential issue | 🟡 Minor

Cancel doesn't abort server-side export.

When exportMode === 'server', clicking Cancel sets isExporting = false and closes the dialog, but the server-side POST request (line 106) continues in the background. The server will still render the video, consuming resources unnecessarily.

Consider using AbortController to cancel the fetch request:

🛠️ Proposed fix using AbortController
+  let abortController: AbortController | null = null;
+
   async function handleServerExport() {
     // ... existing setup ...
+    abortController = new AbortController();
 
     try {
       const response = await fetch(`/api/export/${projectId}?renderId=${renderId}`, {
         method: 'POST',
         headers: { 'Content-Type': 'application/json' },
+        signal: abortController.signal,
         body: JSON.stringify({
           // ...
         })
       });
       // ...
     } finally {
       eventSource.close();
+      abortController = null;
       isExporting = false;
       // ...
     }
   }
 
   function handleCancel() {
     if (isExporting) {
+      if (abortController) {
+        abortController.abort();
+      }
       videoCapture.stopCapture();
       // ...
     }
   }
🤖 Fix all issues with AI agents
In `@src/routes/api/export/`[id]/+server.ts:
- Around line 45-52: Validate and sanitize incoming overrides from
request.json() before assigning to config.width, config.height, and config.fps:
parse values as numbers/integers, ensure they are within sane ranges (e.g.,
width/height > 0 and <= a reasonable max like 10000, fps between 1 and 240),
clamp or reject out-of-range values, and only assign when the parsed value is
valid; otherwise leave the existing config or return a 400 error. Use explicit
checks (Number.isFinite/Number.isInteger as appropriate) on the parsed
body.width/body.height/body.fps and reference the
config.width/config.height/config.fps fields when applying the sanitized values.
- Around line 93-98: The SSE GET handler (exported as GET: RequestHandler)
currently accepts any renderId and lacks authentication/authorization; update
the GET function to authenticate the requester (e.g., validate session/cookie,
bearer token, or SvelteKit locals.user) and then authorize access to the
requested renderId (e.g., check that the authenticated user owns or is permitted
to view the render) before opening the SSE stream; if authentication fails or
the user is not authorized for the renderId, return a 401/403 error instead of
proceeding.
🧹 Nitpick comments (5)
src/lib/server/video-renderer.ts (3)

187-187: Token invalidation is redundant.

invalidateRenderToken(token) is called both in the success path (line 187) and in the finally block (line 193). Since finally always executes, the call at line 187 is unnecessary.

Proposed fix
       frameStream.end();

       await page.close();
       await browser.close();
       browser = null;
-      invalidateRenderToken(token);
     } catch (err: unknown) {

Also applies to: 193-193


141-142: Consider documenting or making the frame delay configurable.

The 32ms delay is approximately two frames at 60fps. A brief comment explaining its purpose (allowing JS/canvas updates to settle) would improve maintainability.

Proposed fix
-        // Brief wait for any JS/canvas updates
-        await new Promise((resolve) => setTimeout(resolve, 32));
+        // Wait ~2 frames at 60fps for JS/canvas updates to settle after seek
+        const FRAME_SETTLE_DELAY_MS = 32;
+        await new Promise((resolve) => setTimeout(resolve, FRAME_SETTLE_DELAY_MS));

46-196: Consider adding a timeout for the entire render operation.

Long-running renders could consume resources indefinitely if something goes wrong. Consider adding a maximum render duration timeout to prevent resource exhaustion.

Example approach
const MAX_RENDER_DURATION_MS = 10 * 60 * 1000; // 10 minutes max

// Inside the async IIFE, add a timeout
const renderTimeout = setTimeout(() => {
  console.error('Render timeout exceeded');
  emitProgress({ phase: 'error', currentFrame: 0, totalFrames, percent: 0, error: 'Render timeout exceeded' });
  videoStream.destroy(new Error('Render timeout'));
}, MAX_RENDER_DURATION_MS);

// Clear in finally block
clearTimeout(renderTimeout);
src/routes/api/export/[id]/+server.ts (1)

71-72: Filename sanitization could be more comprehensive.

The current regex [;="\r\n] handles header injection but still allows characters that may cause issues on some filesystems (e.g., /, \, <, >, |, ?, *).

Proposed fix
     // Sanitize filename to prevent header injection and filesystem issues
-    const sanitizedName = (dbProject.name || 'video').replace(/[;="\r\n]/g, '_').substring(0, 100);
+    const sanitizedName = (dbProject.name || 'video')
+      .replace(/[;="\r\n\\/:<>|?*]/g, '_')
+      .replace(/_{2,}/g, '_')
+      .trim()
+      .substring(0, 100) || 'video';
Dockerfile (1)

39-40: Remove redundant Chromium installation.

The official Playwright image mcr.microsoft.com/playwright:v1.50.0-jammy comes with Chromium 133.0.6943.16 preinstalled in /ms-playwright/ along with all required OS dependencies. Running npx playwright install chromium is redundant and adds unnecessary build time.

Proposed fix
 # Install only production dependencies
 RUN npm install -g pnpm && pnpm install --prod --frozen-lockfile
 
-# Playwright images usually come with browsers, but let's ensure chromium is there
-RUN npx playwright install chromium

Comment on lines +45 to +52
try {
const body = await request.json();
if (body.width) config.width = body.width;
if (body.height) config.height = body.height;
if (body.fps) config.fps = body.fps;
} catch {
// No body or invalid JSON, use defaults
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Config overrides lack validation.

The body parsing silently accepts any values for width, height, and fps without validation. Negative or excessively large values could cause issues in the renderer.

Proposed fix
   try {
     const body = await request.json();
-    if (body.width) config.width = body.width;
-    if (body.height) config.height = body.height;
-    if (body.fps) config.fps = body.fps;
+    if (typeof body.width === 'number' && body.width > 0 && body.width <= 4096) {
+      config.width = body.width;
+    }
+    if (typeof body.height === 'number' && body.height > 0 && body.height <= 4096) {
+      config.height = body.height;
+    }
+    if (typeof body.fps === 'number' && body.fps > 0 && body.fps <= 120) {
+      config.fps = body.fps;
+    }
   } catch {
     // No body or invalid JSON, use defaults
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
const body = await request.json();
if (body.width) config.width = body.width;
if (body.height) config.height = body.height;
if (body.fps) config.fps = body.fps;
} catch {
// No body or invalid JSON, use defaults
}
try {
const body = await request.json();
if (typeof body.width === 'number' && body.width > 0 && body.width <= 4096) {
config.width = body.width;
}
if (typeof body.height === 'number' && body.height > 0 && body.height <= 4096) {
config.height = body.height;
}
if (typeof body.fps === 'number' && body.fps > 0 && body.fps <= 120) {
config.fps = body.fps;
}
} catch {
// No body or invalid JSON, use defaults
}
🤖 Prompt for AI Agents
In `@src/routes/api/export/`[id]/+server.ts around lines 45 - 52, Validate and
sanitize incoming overrides from request.json() before assigning to
config.width, config.height, and config.fps: parse values as numbers/integers,
ensure they are within sane ranges (e.g., width/height > 0 and <= a reasonable
max like 10000, fps between 1 and 240), clamp or reject out-of-range values, and
only assign when the parsed value is valid; otherwise leave the existing config
or return a 400 error. Use explicit checks (Number.isFinite/Number.isInteger as
appropriate) on the parsed body.width/body.height/body.fps and reference the
config.width/config.height/config.fps fields when applying the sanitized values.

Comment on lines 93 to 98
export const GET: RequestHandler = async ({ url }) => {
const renderId = url.searchParams.get('renderId');

if (!renderId) {
error(400, 'Missing renderId');
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

SSE endpoint lacks authentication.

The GET handler for progress streaming has no authentication. Anyone who knows or guesses a renderId can subscribe to progress updates. While this doesn't expose sensitive data, it could leak information about ongoing renders.

Proposed fix
-export const GET: RequestHandler = async ({ url }) => {
+export const GET: RequestHandler = async ({ url, locals }) => {
   const renderId = url.searchParams.get('renderId');

   if (!renderId) {
     error(400, 'Missing renderId');
   }

+  // Require authentication for progress tracking
+  if (!locals.user?.id) {
+    error(403, 'Forbidden');
+  }
+
   let onProgress: ((progress: RenderProgress) => void) | undefined;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const GET: RequestHandler = async ({ url }) => {
const renderId = url.searchParams.get('renderId');
if (!renderId) {
error(400, 'Missing renderId');
}
export const GET: RequestHandler = async ({ url, locals }) => {
const renderId = url.searchParams.get('renderId');
if (!renderId) {
error(400, 'Missing renderId');
}
// Require authentication for progress tracking
if (!locals.user?.id) {
error(403, 'Forbidden');
}
let onProgress: ((progress: RenderProgress) => void) | undefined;
🤖 Prompt for AI Agents
In `@src/routes/api/export/`[id]/+server.ts around lines 93 - 98, The SSE GET
handler (exported as GET: RequestHandler) currently accepts any renderId and
lacks authentication/authorization; update the GET function to authenticate the
requester (e.g., validate session/cookie, bearer token, or SvelteKit
locals.user) and then authorize access to the requested renderId (e.g., check
that the authenticated user owns or is permitted to view the render) before
opening the SSE stream; if authentication fails or the user is not authorized
for the renderId, return a 401/403 error instead of proceeding.

@epavanello epavanello merged commit d542c13 into main Feb 4, 2026
1 check passed
@epavanello epavanello deleted the feat/ss-video-render branch February 4, 2026 22:31
@coderabbitai coderabbitai bot mentioned this pull request Feb 16, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant