Feature/dockerising jobs

[vector17002]

Notion

Summary by CodeRabbit

• New Features

  • Added logout functionality to the web interface
  • Implemented video transcoding with multiple resolution variants
  • Added HLS video segmentation and streaming asset generation

• Infrastructure

  • Introduced Docker containerization with separate API and worker services
  • Refactored worker processes to run independently with job queuing
  • Integrated logging system

[coderabbitai]

Warning

Rate limit exceeded

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

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 5 seconds.

⌛ 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.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ce2bc20d-5064-486b-9467-e262cd49ede7

📥 Commits

Reviewing files that changed from the base of the PR and between 13b85e0 and a2e1447.

📒 Files selected for processing (2)

• .gitignore
• server/Dockerfile

📝 Walkthrough

Walkthrough

Added Docker infrastructure with multi-stage builds and containerized services for transcoding and HLS processing. Introduced transcode and HLS service modules handling video download, processing via FFmpeg, and S3 uploads. Decoupled worker processes from main app startup, and added logout functionality to the frontend.

Changes

Cohort / File(s)	Summary
Docker & Containerization .gitignore, docker-compose.yml, server/.dockerignore, server/Dockerfile	Added Docker Compose configuration orchestrating redis, api, and worker services with health checks and dependency management. Introduced multi-stage Dockerfile with builder, prod-deps, api, and worker targets. Updated .gitignore patterns for downloads/hlsDownloads directories.
Transcoding Service server/src/services/transcode.service.ts, server/src/workers/transcode.queue.ts, server/src/workers/transcode.worker.ts	New service module for S3 download, FFmpeg transcoding to 1080p/720p/480p/360p resolutions, and S3 re-upload. Updated queue with retry/backoff policies. Enhanced worker to execute full transcoding pipeline, clean up downloads directory, and enqueue downstream HLS jobs.
HLS Service server/src/services/hls.service.ts, server/src/workers/hls.queue.ts, server/src/workers/hls.worker.ts	New service module for generating presigned URLs, downloading bitrate-specific MP4s, segmenting into HLS playlists, and uploading to S3. New queue with retry/backoff configuration. New worker processing per-bitrate HLS generation concurrently with cleanup.
Backend App Structure server/src/app.ts, server/src/index.ts, server/src/worker.ts	Decoupled worker startup from Express app initialization. Added dedicated worker entrypoint loading both transcode and HLS worker modules. Added logger import to app initialization.
Build & Configuration server/package.json, server/tsconfig.tsbuildinfo	Moved TypeScript @types to devDependencies. Added fluent-ffmpeg runtime dependency. Updated npm scripts with worker entry points and simplified build command. Updated build metadata with new service/worker files.
Logs & Frontend server/logs/log.txt, web/index.html	Added runtime log demonstrating server startup, transcoding/HLS job lifecycle, error scenarios, and retry mechanisms. Added logout button to frontend with token/localStorage cleanup and UI state reset.

Sequence Diagram(s)

sequenceDiagram
    participant User as Transcode Job
    participant TWorker as Transcode Worker
    participant S3 as S3 Storage
    participant FFmpeg as FFmpeg
    participant Redis as Redis Queue

    User->>Redis: Enqueue transcode job (fileId, userId)
    TWorker->>S3: Get presigned URL for original MP4
    S3-->>TWorker: Presigned URL
    TWorker->>S3: Download original MP4
    S3-->>TWorker: MP4 file
    TWorker->>FFmpeg: Transcode to 4 resolutions (concurrent)
    FFmpeg-->>TWorker: 1080p, 720p, 480p, 360p MP4s
    TWorker->>S3: Upload transcoded files (4 objects)
    S3-->>TWorker: Upload confirmed
    TWorker->>Redis: Enqueue HLS job with fileId
    TWorker->>TWorker: Cleanup: rm -rf downloads/

Loading

sequenceDiagram
    participant HLSJob as HLS Job
    participant HWorker as HLS Worker
    participant S3 as S3 Storage
    participant FFmpeg as FFmpeg
    participant Cleanup as Local FS

    HLSJob->>S3: Get presigned URLs (4 bitrates)
    S3-->>HWorker: URLs for 1080p, 720p, 480p, 360p
    par Per-Bitrate Processing
        HWorker->>S3: Download 1080p MP4
        S3-->>HWorker: 1080p file
        HWorker->>FFmpeg: Segment to HLS playlist
        FFmpeg-->>HWorker: playlist.m3u8 + segments
        HWorker->>S3: Upload .m3u8 and .ts files
        S3-->>HWorker: Upload confirmed
    and
        HWorker->>S3: Download 720p MP4
        S3-->>HWorker: 720p file
        HWorker->>FFmpeg: Segment to HLS playlist
        FFmpeg-->>HWorker: playlist.m3u8 + segments
        HWorker->>S3: Upload .m3u8 and .ts files
        S3-->>HWorker: Upload confirmed
    and
        HWorker->>S3: Download 480p MP4
        S3-->>HWorker: 480p file
        HWorker->>FFmpeg: Segment to HLS playlist
        FFmpeg-->>HWorker: playlist.m3u8 + segments
        HWorker->>S3: Upload .m3u8 and .ts files
        S3-->>HWorker: Upload confirmed
    and
        HWorker->>S3: Download 360p MP4
        S3-->>HWorker: 360p file
        HWorker->>FFmpeg: Segment to HLS playlist
        FFmpeg-->>HWorker: playlist.m3u8 + segments
        HWorker->>S3: Upload .m3u8 and .ts files
        S3-->>HWorker: Upload confirmed
    end
    HWorker->>Cleanup: Cleanup: rm -rf hlsDownloads/
    Cleanup-->>HWorker: Directories removed

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hop! Hop! The Docker's alive,
With workers transcoding, HLS thrives!
FFmpeg's dancing, S3 sings,
Redis queues all the video things,
And logout buttons—what joy they bring! 🎉

🚥 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 refers to a real and significant aspect of the changeset (dockerization of the job system), but does not capture the full scope of the PR, which also introduces substantial new transcoding and HLS processing services, updated worker architecture, and frontend logout functionality.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/dockerising-jobs

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 17

Caution

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

⚠️ Outside diff range comments (1)

web/index.html (1)

312-324: ⚠️ Potential issue | 🟡 Minor

File input lacks an associated <label> element.

The static analysis tool correctly identifies that the file input at line 313 has no associated label, which impacts accessibility for screen reader users. The surrounding text divs are not semantically linked to the input.

Proposed fix to add proper labeling

         <div class="file-upload-wrapper" id="drop-zone">
-            <input type="file" id="file-input" required>
+            <input type="file" id="file-input" required aria-label="Choose file to upload">
             <div class="file-upload-icon">

Or use a proper label element:

         <div class="file-upload-wrapper" id="drop-zone">
+            <label for="file-input" class="visually-hidden">Choose file to upload</label>
             <input type="file" id="file-input" required>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/index.html` around lines 312 - 324, The file input with id "file-input"
is missing an associated label, harming accessibility; add a proper <label>
element linked via the input's id (or use aria-labelledby referencing an element
like the "file-upload-text" or "selected-file-name") so screen readers can
announce the control; update the DOM around the "file-input" (or the wrapper
with id "drop-zone") to include a descriptive label text (e.g., "Upload file")
and ensure the label uses for="file-input" or the input has aria-labelledby
pointing to the descriptive element, and verify required/disabled state remains
intact.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/.dockerignore`:
- Around line 1-12: The .dockerignore currently contains an entry for
".Dockerfile" which appears to be a typo; update the .dockerignore by either
removing the ".Dockerfile" entry entirely or replacing it with "Dockerfile" if
you intend to exclude the actual Dockerfile, ensuring the change targets the
".Dockerfile" line in the diff so only the intended file is excluded from the
build context.

In `@server/Dockerfile`:
- Around line 31-54: The runtime stages ("api" and "worker") run Node/FFmpeg as
root; create a non-root user and group (e.g., adduser/appuser), chown the /app
workspace and node_modules/dist to that user, and switch to that user with USER
before the CMD in both the api and worker stages; ensure the FFmpeg installation
step (apk add ffmpeg) and any files created earlier remain accessible to the
non-root account so that "dist/index.js" and "dist/worker.js" can be executed
without root privileges.

In `@server/logs/log.txt`:
- Around line 1-684: Summary: Committed runtime log server/logs/log.txt contains
sensitive runtime data and must be removed from the repo and ignored going
forward; update .gitignore to include logs/ (or server/logs/) so similar files
aren't committed. Fix: open .gitignore and add a line for logs/ (or
server/logs/) in addition to the existing *.log entry, then remove the committed
file from git tracking with git rm --cached server/logs/log.txt and commit the
changes (include a short commit message like "chore: remove committed logs and
ignore logs/"). Refer to server/logs/log.txt and the repository .gitignore entry
(*.log) when applying the change.

In `@server/package.json`:
- Line 9: The package.json contains a "build" script that runs "tsc -b" but
TypeScript is not declared in devDependencies; add "typescript" to
devDependencies (e.g., add a "typescript": "<appropriate-version>" entry) so tsc
is installed when running npm ci && npm run build outside Docker, ensuring the
build script works consistently; update package.json's devDependencies
accordingly (and regenerate lockfile if needed) so the "build" script and tsc
usage are properly declared.
- Around line 10-11: The dev scripts ("dev" and "dev:worker") only build once
then nodemon watches dist/*.js; change them to watch the TypeScript source and
trigger builds on source changes—e.g., update "dev" to run nodemon with --watch
src --ext ts --exec "npm run build && node dist/index.js" (and similarly update
"dev:worker" to exec the worker output), or replace with a tool like ts-node-dev
that restarts on src changes; ensure the modified scripts reference "dev" and
"dev:worker" so editing files under src triggers rebuild + restart
automatically.

In `@server/src/services/hls.service.ts`:
- Around line 120-137: The current use of Promise.all over files in the upload
block (files.map async ... where PutObjectCommand is sent via s3Client and
streams opened with fs.createReadStream from segmentDir into uploadedKeys)
launches all uploads concurrently and can exhaust file descriptors/sockets;
replace Promise.all with a bounded concurrency queue (e.g., use p-limit or an
async pool) to limit concurrent executions (suggest 10–20), and inside the
queued task keep the same logic that builds s3Key from s3BaseKey, creates the
PutObjectCommand, sends via s3Client.send, logs and pushes to uploadedKeys so
the rest of the flow is unchanged but uploads are rate-limited.
- Around line 47-65: The downloadObjectFromPreSignedUrlWithBitrate function
currently buffers the entire response (responseType: 'arraybuffer') causing high
memory use; change the axios request to stream the response (responseType:
'stream'), create the downloadsDir as before, open a writable stream with
fs.createWriteStream(localFilePath) and pipe the axios response.data stream into
it, then await stream completion (use stream.finished or promisify to wait) and
handle non-200 responses by rejecting/throwing so callers see failures; keep
existing logs (use job.id) and ensure proper error handling and stream cleanup
on error.

In `@server/src/services/transcode.service.ts`:
- Around line 27-45: The function downloadObjectFromPreSignedUrl currently uses
axios with responseType: 'arraybuffer' which buffers the entire video in memory;
change it to stream the response by calling axios with responseType: 'stream'
(or using a streaming GET) on videoDownloadSignedUrl, create the downloadsDir if
missing, pipe response.data into a fs.createWriteStream(localFilePath), await
the stream 'finish'/'error' events (or promisify stream pipeline) and handle
non-200 responses or request errors by rejecting/throwing with a clear log
referencing job.id and fileId; keep symbols: downloadObjectFromPreSignedUrl,
videoDownloadSignedUrl, fileId, job, downloadsDir, localFilePath.
- Around line 95-115: The current upload loop reads entire files into memory via
fs.readFileSync(filePath) causing high memory use; replace that with a streaming
upload by using fs.createReadStream(filePath) as the PutObjectCommand Body (in
the uploadPromises map) or, for better bounded concurrency and multipart
handling, swap to `@aws-sdk/lib-storage`'s Upload class with the same s3Client and
file stream; update the code that builds the command (references:
uploadPromises, filePath, fileBuffer, PutObjectCommand, s3Client.send,
uploadedKeys) to pass the stream as Body (or instantiate Upload) and ensure you
await the Upload.done() (or s3Client.send) and still push s3Key into
uploadedKeys and handle/propagate errors appropriately.

In `@server/src/workers/hls.queue.ts`:
- Around line 4-15: The code casts the ioredis client to any when constructing
new Queue (hlsQueue) because `@types/ioredis` is out-of-sync with runtime ioredis;
update the type package to a version compatible with ioredis v5 (or remove the
outdated `@types` package if ioredis ships its own types) and then remove the "as
any" cast from the Queue connection option (the new Queue("hlsQueue", {
connection: redis, ... }) in hls.queue.ts); apply the same change to the other
instances mentioned (hls.worker.ts, transcode.queue.ts, transcode.worker.ts) so
the Queue constructor accepts the typed ioredis client directly.

In `@server/src/workers/hls.worker.ts`:
- Around line 29-45: The current fire-and-forget exec("rm -rf hlsDownloads") in
hlsWorker.on("completed") causes races and leaks because artifacts are stored
under a shared hlsDownloads tree; instead, change the code to remove only the
job-specific directory (e.g., hlsDownloads/${job.id} or the exact path used by
server/src/services/hls.service.ts) and perform the removal with an awaited,
error-handled call (use fs.promises.rm or fs.rmSync wrapped in try/catch) inside
both hlsWorker.on("completed", (job) => {...}) and hlsWorker.on("failed", (job,
err) => {...}) so cleanup is deterministic, per-job, and any errors are logged
rather than fire-and-forget.

In `@server/src/workers/transcode.worker.ts`:
- Around line 89-100: The current cleanup handler in
transcodeWorker.on("completed", ...) races by removing the shared downloads/
root and misses failed jobs; change to per-job temp directories (e.g., derive
jobTempDir from the job id or fileId used in
server/src/services/transcode.service.ts) and ensure removal is done per-job in
an async, awaited finally block so it runs for both success and failure; replace
the fire-and-forget exec("rm -rf downloads") in the transcodeWorker completed
handler (and similar code at the 104-114 region) with an awaited
fs.promises.rm(jobTempDir, { recursive: true, force: true }) (or equivalent safe
removal) and add removal in the job failure handler so each job only deletes its
own temp folder and large artifacts are always cleaned up.
- Around line 72-73: The transcode worker currently calls hlsQueue.add("hls", {
fileId, userId }) which creates duplicate HLS jobs on transcode retries; make
the enqueue idempotent by supplying a deterministic jobId (e.g. using fileId and
userId or a stable prefix like `hls:${fileId}:${userId}`) as the third argument
to hlsQueue.add so retries reuse the same downstream job instead of creating
duplicates; update the call site where hlsQueue.add is invoked in
transcode.worker.ts and ensure the chosen jobId format is stable across retries
(and unique enough to avoid collisions).

In `@server/tsconfig.tsbuildinfo`:
- Line 1: The tsconfig.tsbuildinfo file is a generated TypeScript incremental
build artifact and should be ignored by VCS; add "tsconfig.tsbuildinfo" to the
repository .gitignore so it is not committed (or update .gitignore entries to
cover *.tsbuildinfo if you prefer), then remove the tracked file from the index
if already committed (e.g., git rm --cached tsconfig.tsbuildinfo) and commit the
updated .gitignore; ensure no code changes are required in functions or
classes—this is purely a VCS ignore/cleanup change.

In `@web/index.html`:
- Around line 436-437: The page currently writes the JWT to both document.cookie
and localStorage (document.cookie = `token=${data.token}...` and
localStorage.setItem('token', data.token)); stop exposing the token client-side
by removing localStorage.setItem('token', ...) and stop setting a cookie from
JavaScript without HttpOnly. Instead send the token to the server (or include it
in the API response) and have the server set the cookie via Set-Cookie with
HttpOnly; Secure; SameSite=Strict; if you must keep a client-side fix, at
minimum change the document.cookie assignment to include Secure and
SameSite=Strict, but prefer server-side Set-Cookie with HttpOnly and remove
localStorage usage.
- Around line 338-340: The logout button uses inline styles instead of a CSS
class; create a .btn-secondary CSS class (matching the inline rules: background:
var(--surface-color); color: var(--text-muted); border: 1px solid
var(--border-color); margin-top: 1rem;) in the page's <style> block and replace
the inline style on the element with that class (keep the existing id
"logout-btn" and base "btn" class) so the button uses ".btn btn-secondary" for
consistent styling and maintainability.
- Around line 596-610: Update the logoutBtn click handler to call the server
logout endpoint before clearing client state: read the JWT from cookie or
localStorage, POST to /auth/logout with the Authorization: Bearer <token> header
(handle network errors and non-2xx responses), then on success (or after
best-effort attempt) clear cookie/localStorage and toggle uploadView/authView
and inputs (emailInput, passwordInput) like today; additionally ensure the
server implements token revocation (e.g., Redis blacklist) and that
authMiddleware checks that blacklist so the JWT is invalidated server-side.

---

Outside diff comments:
In `@web/index.html`:
- Around line 312-324: The file input with id "file-input" is missing an
associated label, harming accessibility; add a proper <label> element linked via
the input's id (or use aria-labelledby referencing an element like the
"file-upload-text" or "selected-file-name") so screen readers can announce the
control; update the DOM around the "file-input" (or the wrapper with id
"drop-zone") to include a descriptive label text (e.g., "Upload file") and
ensure the label uses for="file-input" or the input has aria-labelledby pointing
to the descriptive element, and verify required/disabled state remains intact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2fdb37c4-0179-4484-acff-401d77819734

📥 Commits

Reviewing files that changed from the base of the PR and between 385aa03 and 13b85e0.

⛔ Files ignored due to path filters (1)

• server/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (17)

• .gitignore
• docker-compose.yml
• server/.dockerignore
• server/Dockerfile
• server/logs/log.txt
• server/package.json
• server/src/app.ts
• server/src/index.ts
• server/src/services/hls.service.ts
• server/src/services/transcode.service.ts
• server/src/worker.ts
• server/src/workers/hls.queue.ts
• server/src/workers/hls.worker.ts
• server/src/workers/transcode.queue.ts
• server/src/workers/transcode.worker.ts
• server/tsconfig.tsbuildinfo
• web/index.html

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's actually a .Dockerfile file or if it should be Dockerfile
fd -H '\.?Dockerfile' server/

Repository: vector17002/video-transcoding

Length of output: 90

---

Remove or correct the .Dockerfile entry on line 12.

The .dockerignore file excludes a file named .Dockerfile (with leading dot), but the actual Dockerfile in the repository is named Dockerfile (without the dot). This entry is either a typo or unnecessary. If it was meant to exclude the Dockerfile from the build context, change it to Dockerfile; otherwise, remove it entirely.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/.dockerignore` around lines 1 - 12, The .dockerignore currently
contains an entry for ".Dockerfile" which appears to be a typo; update the
.dockerignore by either removing the ".Dockerfile" entry entirely or replacing
it with "Dockerfile" if you intend to exclude the actual Dockerfile, ensuring
the change targets the ".Dockerfile" line in the diff so only the intended file
is excluded from the build context.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Run the final images as a non-root user.

Both runtime stages execute Node/FFmpeg as root today, which unnecessarily widens the blast radius if either service is compromised.

Harden the runtime stages

 FROM node:22-alpine AS api

 WORKDIR /app

-COPY --from=prod-deps /app/node_modules ./node_modules
-COPY --from=builder /app/dist ./dist
-COPY package.json ./
+COPY --chown=node:node --from=prod-deps /app/node_modules ./node_modules
+COPY --chown=node:node --from=builder /app/dist ./dist
+COPY --chown=node:node package.json ./
+USER node

 FROM node:22-alpine AS worker

 # Only workers need FFmpeg
 RUN apk add --no-cache ffmpeg

 WORKDIR /app

-COPY --from=prod-deps /app/node_modules ./node_modules
-COPY --from=builder /app/dist ./dist
-COPY package.json ./
+COPY --chown=node:node --from=prod-deps /app/node_modules ./node_modules
+COPY --chown=node:node --from=builder /app/dist ./dist
+COPY --chown=node:node package.json ./
+USER node

📝 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

	FROM node:22-alpine AS api
	WORKDIR /app
	COPY --from=prod-deps /app/node_modules ./node_modules
	COPY --from=builder /app/dist ./dist
	COPY package.json ./
	CMD ["node", "dist/index.js"]
	# ── Stage 3b: Worker (with FFmpeg for transcoding) ─────
	FROM node:22-alpine AS worker
	# Only workers need FFmpeg
	RUN apk add --no-cache ffmpeg
	WORKDIR /app
	COPY --from=prod-deps /app/node_modules ./node_modules
	COPY --from=builder /app/dist ./dist
	COPY package.json ./
	CMD ["node", "dist/worker.js"]
	FROM node:22-alpine AS api
	WORKDIR /app
	COPY --chown=node:node --from=prod-deps /app/node_modules ./node_modules
	COPY --chown=node:node --from=builder /app/dist ./dist
	COPY --chown=node:node package.json ./
	USER node
	CMD ["node", "dist/index.js"]
	# ── Stage 3b: Worker (with FFmpeg for transcoding) ─────
	FROM node:22-alpine AS worker
	# Only workers need FFmpeg
	RUN apk add --no-cache ffmpeg
	WORKDIR /app
	COPY --chown=node:node --from=prod-deps /app/node_modules ./node_modules
	COPY --chown=node:node --from=builder /app/dist ./dist
	COPY --chown=node:node package.json ./
	USER node
	CMD ["node", "dist/worker.js"]

🧰 Tools

🪛 Checkov (3.2.510)

[low] 1-54: Ensure that HEALTHCHECK instructions have been added to container images

(CKV_DOCKER_2)

---

[low] 1-54: Ensure that a user for the container has been created

(CKV_DOCKER_3)

🪛 Hadolint (2.14.0)

[warning] 46-46: Pin versions in apk add. Instead of apk add <package> use apk add <package>=<version>

(DL3018)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/Dockerfile` around lines 31 - 54, The runtime stages ("api" and
"worker") run Node/FFmpeg as root; create a non-root user and group (e.g.,
adduser/appuser), chown the /app workspace and node_modules/dist to that user,
and switch to that user with USER before the CMD in both the api and worker
stages; ensure the FFmpeg installation step (apk add ffmpeg) and any files
created earlier remain accessible to the non-root account so that
"dist/index.js" and "dist/worker.js" can be executed without root privileges.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Do not commit log files to version control.

This log file contains runtime output with potentially sensitive information including:

• Local filesystem paths exposing developer usernames
• User IDs and file IDs from S3 paths
• Stack traces revealing internal implementation details

The .gitignore has *.log but this file is log.txt. Add logs/ directory to .gitignore and remove this file from the repository.

📝 Proposed fix for .gitignore

 *.log
+logs/
 downloads/
 hlsDownloads/

Then remove the committed file:

git rm --cached server/logs/log.txt

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/logs/log.txt` around lines 1 - 684, Summary: Committed runtime log
server/logs/log.txt contains sensitive runtime data and must be removed from the
repo and ignored going forward; update .gitignore to include logs/ (or
server/logs/) so similar files aren't committed. Fix: open .gitignore and add a
line for logs/ (or server/logs/) in addition to the existing *.log entry, then
remove the committed file from git tracking with git rm --cached
server/logs/log.txt and commit the changes (include a short commit message like
"chore: remove committed logs and ignore logs/"). Refer to server/logs/log.txt
and the repository .gitignore entry (*.log) when applying the change.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n server/package.json | head -30

Repository: vector17002/video-transcoding

Length of output: 1108

---

🏁 Script executed:

cat server/Dockerfile | grep -n typescript -i

Repository: vector17002/video-transcoding

Length of output: 134

---

🏁 Script executed:

cat server/Dockerfile

Repository: vector17002/video-transcoding

Length of output: 1305

---

🏁 Script executed:

cat server/package.json | grep -A 20 '"devDependencies"'

Repository: vector17002/video-transcoding

Length of output: 429

---

Add typescript to devDependencies before using tsc in the build script.

The "build": "tsc -b" script requires the TypeScript compiler, but typescript is not declared in devDependencies. The ad-hoc RUN npm i typescript in the Dockerfile masks this dependency gap instead of fixing it, causing npm ci && npm run build to fail outside Docker.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/package.json` at line 9, The package.json contains a "build" script
that runs "tsc -b" but TypeScript is not declared in devDependencies; add
"typescript" to devDependencies (e.g., add a "typescript":
"<appropriate-version>" entry) so tsc is installed when running npm ci && npm
run build outside Docker, ensuring the build script works consistently; update
package.json's devDependencies accordingly (and regenerate lockfile if needed)
so the "build" script and tsc usage are properly declared.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd server && cat -n package.json | head -30

Repository: vector17002/video-transcoding

Length of output: 1108

---

🏁 Script executed:

cd server && ls -la | grep -E "tsconfig|build" && cat -n tsconfig.json

Repository: vector17002/video-transcoding

Length of output: 1632

---

🏁 Script executed:

cd server && cat -n package.json | grep -A 2 -B 2 '"build"'

Repository: vector17002/video-transcoding

Length of output: 321

---

Dev scripts don't rebuild on TypeScript source changes.

The current setup compiles once with npm run build, then nodemon only watches the compiled dist/*.js files. Any edits to src/ require manually restarting the command.

Rebuild on source changes

-    "dev": "npm run build && nodemon dist/index.js",
-    "dev:worker": "npm run build && nodemon dist/worker.js"
+    "dev": "nodemon --watch src --watch tsconfig.json --ext ts --exec \"npm run build && node dist/index.js\"",
+    "dev:worker": "nodemon --watch src --watch tsconfig.json --ext ts --exec \"npm run build && node dist/worker.js\""

📝 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

	"dev": "npm run build && nodemon dist/index.js",
	"dev:worker": "npm run build && nodemon dist/worker.js"
	"dev": "nodemon --watch src --watch tsconfig.json --ext ts --exec \"npm run build && node dist/index.js\"",
	"dev:worker": "nodemon --watch src --watch tsconfig.json --ext ts --exec \"npm run build && node dist/worker.js\""

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/package.json` around lines 10 - 11, The dev scripts ("dev" and
"dev:worker") only build once then nodemon watches dist/*.js; change them to
watch the TypeScript source and trigger builds on source changes—e.g., update
"dev" to run nodemon with --watch src --ext ts --exec "npm run build && node
dist/index.js" (and similarly update "dev:worker" to exec the worker output), or
replace with a tool like ts-node-dev that restarts on src changes; ensure the
modified scripts reference "dev" and "dev:worker" so editing files under src
triggers rebuild + restart automatically.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

The temp cleanup is racy and misses failed jobs.

server/src/services/transcode.service.ts writes every job into the shared downloads/ root. This fire-and-forget rm -rf downloads can overlap the next job, and nothing removes large artifacts when the job fails.

Safer cleanup pattern

const jobTempDir = path.join(downloadsRoot, fileId);

try {
  // download, transcode, upload, enqueue HLS
} finally {
  await fs.promises.rm(jobTempDir, { recursive: true, force: true });
}

Also applies to: 104-114

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/src/workers/transcode.worker.ts` around lines 89 - 100, The current
cleanup handler in transcodeWorker.on("completed", ...) races by removing the
shared downloads/ root and misses failed jobs; change to per-job temp
directories (e.g., derive jobTempDir from the job id or fileId used in
server/src/services/transcode.service.ts) and ensure removal is done per-job in
an async, awaited finally block so it runs for both success and failure; replace
the fire-and-forget exec("rm -rf downloads") in the transcodeWorker completed
handler (and similar code at the 104-114 region) with an awaited
fs.promises.rm(jobTempDir, { recursive: true, force: true }) (or equivalent safe
removal) and add removal in the job failure handler so each job only deletes its
own temp folder and large artifacts are always cleaned up.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider adding tsconfig.tsbuildinfo to .gitignore.

This file is auto-generated by the TypeScript compiler during incremental builds. Committing it can cause unnecessary merge conflicts and churn. It's typically excluded from version control.

📝 Add to .gitignore

 downloads/
 hlsDownloads/
+*.tsbuildinfo

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/tsconfig.tsbuildinfo` at line 1, The tsconfig.tsbuildinfo file is a
generated TypeScript incremental build artifact and should be ignored by VCS;
add "tsconfig.tsbuildinfo" to the repository .gitignore so it is not committed
(or update .gitignore entries to cover *.tsbuildinfo if you prefer), then remove
the tracked file from the index if already committed (e.g., git rm --cached
tsconfig.tsbuildinfo) and commit the updated .gitignore; ensure no code changes
are required in functions or classes—this is purely a VCS ignore/cleanup change.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Logout button styling uses inline styles.

The button uses inline style attribute while the rest of the page uses CSS classes. Consider adding a .btn-secondary class for consistency and maintainability.

Proposed refactor to use CSS class

Add to the <style> block:

.btn-secondary {
    background: var(--surface-color);
    color: var(--text-muted);
    border: 1px solid var(--border-color);
    margin-top: 1rem;
}

Then update the button:

-        <button type="button" class="btn" id="logout-btn" style="background: var(--surface-color); color: var(--text-muted); border: 1px solid var(--border-color); margin-top: 1rem;">
+        <button type="button" class="btn btn-secondary" id="logout-btn">
             Logout
         </button>

📝 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

	<button type="button" class="btn" id="logout-btn" style="background: var(--surface-color); color: var(--text-muted); border: 1px solid var(--border-color); margin-top: 1rem;">
	Logout
	</button>
	<button type="button" class="btn btn-secondary" id="logout-btn">
	Logout
	</button>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/index.html` around lines 338 - 340, The logout button uses inline styles
instead of a CSS class; create a .btn-secondary CSS class (matching the inline
rules: background: var(--surface-color); color: var(--text-muted); border: 1px
solid var(--border-color); margin-top: 1rem;) in the page's <style> block and
replace the inline style on the element with that class (keep the existing id
"logout-btn" and base "btn" class) so the button uses ".btn btn-secondary" for
consistent styling and maintainability.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Token stored in both localStorage and a non-HttpOnly cookie exposes it to XSS.

Storing the JWT in localStorage and a client-set cookie (without HttpOnly, Secure, or SameSite attributes) means any XSS vulnerability can exfiltrate the token. Consider:

1. Having the server set the cookie with HttpOnly; Secure; SameSite=Strict via Set-Cookie header
2. Removing localStorage storage entirely, relying solely on the secure cookie
3. Or at minimum, add Secure and SameSite=Strict to the client-set cookie

Minimal improvement: Add Secure and SameSite attributes

-                document.cookie = `token=${data.token}; path=/; max-age=3600`;
-                localStorage.setItem('token', data.token);
+                // Note: For production, the server should set HttpOnly cookie instead
+                document.cookie = `token=${data.token}; path=/; max-age=3600; SameSite=Strict; Secure`;
+                localStorage.setItem('token', data.token);

📝 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

	document.cookie = `token=${data.token}; path=/; max-age=3600`;
	localStorage.setItem('token', data.token);
	// Note: For production, the server should set HttpOnly cookie instead
	document.cookie = `token=${data.token}; path=/; max-age=3600; SameSite=Strict; Secure`;
	localStorage.setItem('token', data.token);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/index.html` around lines 436 - 437, The page currently writes the JWT to
both document.cookie and localStorage (document.cookie =
`token=${data.token}...` and localStorage.setItem('token', data.token)); stop
exposing the token client-side by removing localStorage.setItem('token', ...)
and stop setting a cookie from JavaScript without HttpOnly. Instead send the
token to the server (or include it in the API response) and have the server set
the cookie via Set-Cookie with HttpOnly; Secure; SameSite=Strict; if you must
keep a client-side fix, at minimum change the document.cookie assignment to
include Secure and SameSite=Strict, but prefer server-side Set-Cookie with
HttpOnly and remove localStorage usage.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Client-side logout doesn't invalidate the JWT server-side.

The logout handler only clears local storage and cookies on the client. The JWT remains valid on the server until its natural 1-hour expiration. An attacker who captured the token (e.g., via XSS) can continue using it after the user "logs out."

Consider calling the server's existing /auth/logout endpoint and implementing server-side token revocation (e.g., a Redis-backed blacklist checked by authMiddleware).

Proposed fix to call server logout endpoint

         if (logoutBtn) {
-            logoutBtn.addEventListener('click', () => {
+            logoutBtn.addEventListener('click', async () => {
+                try {
+                    await fetch(`${API_BASE}/auth/logout`, {
+                        method: 'POST',
+                        headers: {
+                            'Authorization': `Bearer ${localStorage.getItem('token') || ''}`
+                        },
+                        credentials: 'include'
+                    });
+                } catch (e) {
+                    console.error('Logout request failed', e);
+                }
                 document.cookie = 'token=; Max-Age=0; path=/;';
                 localStorage.removeItem('token');

Note: Even with this fix, true token invalidation requires the server to maintain a revocation list and check it in authMiddleware.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/index.html` around lines 596 - 610, Update the logoutBtn click handler to
call the server logout endpoint before clearing client state: read the JWT from
cookie or localStorage, POST to /auth/logout with the Authorization: Bearer
<token> header (handle network errors and non-2xx responses), then on success
(or after best-effort attempt) clear cookie/localStorage and toggle
uploadView/authView and inputs (emailInput, passwordInput) like today;
additionally ensure the server implements token revocation (e.g., Redis
blacklist) and that authMiddleware checks that blacklist so the JWT is
invalidated server-side.