fix: bug fixes and authenticated BullBoard queue UI#139
Conversation
- Dockerfile.background-jobs: set COREPACK_HOME=/opt/corepack so pnpm is stored in a shared location; mkdir /pnpm before chmod so the directory exists at build time; create the node user explicitly in the CUDA base (nvidia image has no node user); add USER node in production - Dockerfile.web: same COREPACK_HOME + mkdir pattern; add USER node in production - Dockerfile.ml: create appuser (UID/GID 1001) in both base-cpu and base-gpu; replace chmod -R 777 /ml-models with chown+chmod 755; chown /app to appuser and add USER appuser in production and production-gpu Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extend non-root enforcement to dev stages: chown /app to the runtime user before switching so tsx watch (bg-jobs), Vite cache (web), and Python bytecache (ml) can write at runtime. All six image targets (prod + dev for each service) now run as node/appuser. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bug fixes:
- env: ENABLE_QUEUE_UI=false now correctly disables the UI (Boolean('false')
was true; fixed to val === 'true')
- faceLabelling: outer catch now rethrows so BullMQ retries on failure;
rename inner 'job' variable to 'dbJob' to remove shadowing
- transcoding: overallProgress 1000 → 10 (progress bar no longer jumps
to 1000%)
Queue UI auth:
- BullBoard is now protected by requireQueueAuth middleware; unauthenticated
requests are redirected to the web app login with ?next= so the browser
returns to the queue page after sign-in
- Uses the same __session/__session_dev cookie the web app already sets,
verified with HMAC-SHA256 matching React Router's createCookieSessionStorage
signing format — no separate login or session store needed
- WEB_APP_URL docker hostname replaced with localhost for browser-facing
redirects (http://web:3746 → http://localhost:3746)
- Web app login now accepts a ?next query param and redirects there after
successful authentication (validated to http/https only)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds authentication to the Bull Board UI using signed session cookies with login redirect support, fixes background job error handling and progress metrics, standardizes Corepack/pnpm setup and non-root container execution, and converts media-utils FFmpeg helpers to synchronous spawns with event-driven process handling. ChangesAuthentication & Queue Access Control
Background Jobs, Container Setup & Media Utils
Sequence DiagramsequenceDiagram
participant Client as Client
participant QueueUI as Queue UI Route
participant AuthMiddleware as requireQueueAuth
participant SessionVerify as verifyReactRouterSession
participant LoginPage as Web Login Page
Client->>QueueUI: GET /
QueueUI->>AuthMiddleware: request with cookies
AuthMiddleware->>SessionVerify: verify session cookie
alt Session Valid
SessionVerify-->>AuthMiddleware: userId extracted
AuthMiddleware-->>QueueUI: next()
QueueUI-->>Client: Queue UI page
else Session Invalid
SessionVerify-->>AuthMiddleware: verification failed
AuthMiddleware-->>LoginPage: redirect with next param
LoginPage-->>Client: login form
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/background-jobs/src/index.ts`:
- Line 111: The middleware requireQueueAuth is mounted at the app root
(app.use('/', requireQueueAuth, serverAdapter.getRouter())) which causes it to
run for all routes; change this so requireQueueAuth only applies to the Bull
Board router: attach the auth middleware to the Bull Board route (e.g.,
app.use('/queues', requireQueueAuth, serverAdapter.getRouter()) or call
serverAdapter.getRouter().use(requireQueueAuth) and then mount that router at a
dedicated path) so /internal/*, /api/v0 and /health are not intercepted.
In `@apps/background-jobs/src/middleware/queueAuth.ts`:
- Around line 54-56: The hostname replacement currently uses string replace on
env.WEB_APP_URL; change it to parse the URL (new URL(...)) inside the queueAuth
middleware so you only swap the hostname when url.hostname === 'web', set
url.hostname = 'localhost' in that case, and then use the reconstructed URL
(e.g. url.toString() or url.origin + url.pathname) for publicWebUrl; keep the
rest of the redirect logic that builds returnTo and calls res.redirect with the
encoded next param unchanged.
In `@apps/web/app/services/auth.server.ts`:
- Around line 21-22: The current redirect logic builds redirectTo from the query
param `next` and only checks the scheme, allowing open redirects; change the
logic around the `redirectTo` calculation so that when `next` is an absolute URL
you parse it with the URL constructor (in a try/catch) and verify its
origin/host is in a maintained allowlist (e.g., an array of trusted origins or
hostnames) before using it; if the origin is not allowlisted or parsing fails,
fall back to the safe local path '/app/home'. Update the code that sets
`redirectTo` (the variable named `redirectTo` used in the `redirect(redirectTo,
{ headers })` call) to implement this allowlist check.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae64a7b9-62a3-4fe4-baa0-cd1abd2c6a88
📒 Files selected for processing (10)
apps/background-jobs/src/index.tsapps/background-jobs/src/jobs/faceLabelling.tsapps/background-jobs/src/jobs/transcoding.tsapps/background-jobs/src/middleware/queueAuth.tsapps/background-jobs/src/utils/env.tsapps/web/app/routes/auth.login.tsxapps/web/app/services/auth.server.tsdocker/Dockerfile.background-jobsdocker/Dockerfile.mldocker/Dockerfile.web
| }) | ||
|
|
||
| app.use('/', serverAdapter.getRouter()) | ||
| app.use('/', requireQueueAuth, serverAdapter.getRouter()) |
There was a problem hiding this comment.
Scope the queue auth middleware to the Bull Board path only.
Line 111 mounts requireQueueAuth at '/', so every request hits it before /internal/*, /api/v0, and /health. When the queue UI is enabled, existing JWT/header-based callers will start getting redirected to the login page instead of reaching those handlers.
Suggested fix
- serverAdapter.setBasePath('/')
+ serverAdapter.setBasePath('/queues')
…
- app.use('/', requireQueueAuth, serverAdapter.getRouter())
+ app.use('/queues', requireQueueAuth, serverAdapter.getRouter())🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 111-111: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/background-jobs/src/index.ts` at line 111, The middleware
requireQueueAuth is mounted at the app root (app.use('/', requireQueueAuth,
serverAdapter.getRouter())) which causes it to run for all routes; change this
so requireQueueAuth only applies to the Bull Board router: attach the auth
middleware to the Bull Board route (e.g., app.use('/queues', requireQueueAuth,
serverAdapter.getRouter()) or call
serverAdapter.getRouter().use(requireQueueAuth) and then mount that router at a
dedicated path) so /internal/*, /api/v0 and /health are not intercepted.
| const publicWebUrl = env.WEB_APP_URL.replace('web', 'localhost') | ||
| const returnTo = `${req.protocol}://${req.get('host')}${req.originalUrl}` | ||
| res.redirect(`${publicWebUrl}/auth/login?next=${encodeURIComponent(returnTo)}`) |
There was a problem hiding this comment.
Rewrite only the Docker hostname.
Line 54 uses replace('web', 'localhost'), which also rewrites legitimate hosts like https://web.example.com into https://localhost.example.com. Parse the URL and swap the hostname only when it is exactly web.
Suggested fix
- const publicWebUrl = env.WEB_APP_URL.replace('web', 'localhost')
+ const publicWebUrl = new URL(env.WEB_APP_URL)
+ if (publicWebUrl.hostname === 'web') {
+ publicWebUrl.hostname = 'localhost'
+ }
const returnTo = `${req.protocol}://${req.get('host')}${req.originalUrl}`
- res.redirect(`${publicWebUrl}/auth/login?next=${encodeURIComponent(returnTo)}`)
+ const loginUrl = new URL('/auth/login', publicWebUrl)
+ loginUrl.searchParams.set('next', returnTo)
+ res.redirect(loginUrl.toString())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/background-jobs/src/middleware/queueAuth.ts` around lines 54 - 56, The
hostname replacement currently uses string replace on env.WEB_APP_URL; change it
to parse the URL (new URL(...)) inside the queueAuth middleware so you only swap
the hostname when url.hostname === 'web', set url.hostname = 'localhost' in that
case, and then use the reconstructed URL (e.g. url.toString() or url.origin +
url.pathname) for publicWebUrl; keep the rest of the redirect logic that builds
returnTo and calls res.redirect with the encoded next param unchanged.
| const redirectTo = next && /^https?:\/\//.test(next) ? next : '/app/home' | ||
| return redirect(redirectTo, { headers }) |
There was a problem hiding this comment.
next validation still permits open redirects.
Line 21 accepts any absolute http(s) URL, so a request like /auth/login?next=https://evil.example will send a successful login there. Restrict next to an explicit allowlist of trusted origins instead of only checking the scheme; same-origin-only would be too strict here because the queue UI lives on a different origin.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/services/auth.server.ts` around lines 21 - 22, The current
redirect logic builds redirectTo from the query param `next` and only checks the
scheme, allowing open redirects; change the logic around the `redirectTo`
calculation so that when `next` is an absolute URL you parse it with the URL
constructor (in a try/catch) and verify its origin/host is in a maintained
allowlist (e.g., an array of trusted origins or hostnames) before using it; if
the origin is not allowlisted or parsing fails, fall back to the safe local path
'/app/home'. Update the code that sets `redirectTo` (the variable named
`redirectTo` used in the `redirect(redirectTo, { headers })` call) to implement
this allowlist check.
All three service containers were running as root. This PR fixes each one for both production and development targets. **Dockerfile.background-jobs** - Set COREPACK_HOME=/opt/corepack so the pnpm binary is stored in a shared, world-readable path - Create the node user explicitly in the CUDA base image - USER node in production and development stages - Pre-create /ml-models/embedding-models owned by node (chmod 777) so the embedding-core module can write its model cache at startup **Dockerfile.web** - Same COREPACK_HOME + mkdir pattern in the base stage - USER node in production and development stages - Pre-create /ml-models/embedding-models owned by node (chmod 777) **Dockerfile.ml** - Create appuser (UID/GID 1001) in both base-cpu and base-gpu stages - Replace chmod -R 777 /ml-models with chown -R appuser:appgroup + chmod 777 so the shared ml-models volume is also writable by the node user - USER appuser in all four stages Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Non-root containers cannot chmod system binaries installed via apt. The chmod was unnecessary since apt-installed binaries are already executable. Drop ensureBinaryPermissions entirely and make the validate/spawn functions sync. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove .then() wrappers that assumed the spawn functions returned Promises — they now return ChildProcess directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/Dockerfile.ml`:
- Line 20: The Dockerfile line that currently runs "chown -R appuser:appgroup
/ml-models && chmod 777 /ml-models" is too permissive; update that command so
only the owner (and optionally the group) get full access — e.g., replace chmod
777 with chmod 755 (or chmod 750 if group access is not needed) while keeping
the chown to appuser:appgroup unchanged; ensure the corrected command appears
wherever the same "chown -R appuser:appgroup /ml-models && chmod 777 /ml-models"
occurs.
In `@docker/Dockerfile.web`:
- Line 98: The Dockerfile RUN line currently creates
/ml-models/embedding-models, sets ownership to node:node and uses chmod 777
which is too permissive; update the RUN command(s) that contain "mkdir -p
/ml-models/embedding-models && chown -R node:node /ml-models && chmod 777
/ml-models" to use a more restrictive permission such as chmod 755 (or chmod 750
if group access isn't needed) so only the node user (and optionally group) has
full access; apply the same replacement to the other occurrence noted in the
file.
In `@packages/media-utils/src/utils/audio.ts`:
- Around line 185-213: The readAudio function spawns FFmpeg via spawnFFmpeg but
never drains ffmpeg.stderr, which can cause the child to block; add a listener
on ffmpeg.stderr (e.g., ffmpeg.stderr.on('data', chunk => { /* collect or
discard */ })) to consume stderr and prevent backpressure, collect stderr into a
string/Buffer and include it in any reject/new Error messages inside the ffmpeg
'close' and 'error' handlers so failures from readAudio surface the FFmpeg
stderr output (mirroring the pattern used in extractAudio).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db60a960-2385-4b07-9791-b0d625fd54e8
📒 Files selected for processing (6)
docker/Dockerfile.background-jobsdocker/Dockerfile.mldocker/Dockerfile.webpackages/media-utils/src/lib/ffmpeg.tspackages/media-utils/src/utils/audio.tspackages/media-utils/src/utils/frame.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docker/Dockerfile.background-jobs
|
|
||
| RUN mkdir -p /ml-models/ultralytics && mkdir -p /ml-models/whisper && \ | ||
| chmod -R 777 /ml-models | ||
| chown -R appuser:appgroup /ml-models && chmod 777 /ml-models |
There was a problem hiding this comment.
Replace chmod 777 with more restrictive permissions.
The chmod 777 grants read, write, and execute permissions to owner, group, and others, which is overly permissive. Since ownership is already set to appuser:appgroup, only the owner needs full access. Use chmod 755 (or 750 if group access isn't required) to follow the principle of least privilege.
🔐 Proposed fix
- chown -R appuser:appgroup /ml-models && chmod 777 /ml-models
+ chown -R appuser:appgroup /ml-models && chmod 755 /ml-modelsAlso applies to: 40-40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/Dockerfile.ml` at line 20, The Dockerfile line that currently runs
"chown -R appuser:appgroup /ml-models && chmod 777 /ml-models" is too
permissive; update that command so only the owner (and optionally the group) get
full access — e.g., replace chmod 777 with chmod 755 (or chmod 750 if group
access is not needed) while keeping the chown to appuser:appgroup unchanged;
ensure the corrected command appears wherever the same "chown -R
appuser:appgroup /ml-models && chmod 777 /ml-models" occurs.
|
|
||
| RUN pnpm --filter prisma generate | ||
|
|
||
| RUN mkdir -p /ml-models/embedding-models && chown -R node:node /ml-models && chmod 777 /ml-models |
There was a problem hiding this comment.
Replace chmod 777 with more restrictive permissions.
The chmod 777 grants read, write, and execute permissions to owner, group, and others, which is overly permissive. Since ownership is already set to node:node, only the node user needs full access. Use chmod 755 (or 750 if group access isn't required) to follow the principle of least privilege.
🔐 Proposed fix
-RUN mkdir -p /ml-models/embedding-models && chown -R node:node /ml-models && chmod 777 /ml-models
+RUN mkdir -p /ml-models/embedding-models && chown -R node:node /ml-models && chmod 755 /ml-modelsAlso applies to: 121-121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/Dockerfile.web` at line 98, The Dockerfile RUN line currently creates
/ml-models/embedding-models, sets ownership to node:node and uses chmod 777
which is too permissive; update the RUN command(s) that contain "mkdir -p
/ml-models/embedding-models && chown -R node:node /ml-models && chmod 777
/ml-models" to use a more restrictive permission such as chmod 755 (or chmod 750
if group access isn't needed) so only the node user (and optionally group) has
full access; apply the same replacement to the other occurrence noted in the
file.
| const ffmpeg = spawnFFmpeg(args) | ||
| const chunks: Buffer[] = [] | ||
|
|
||
| ffmpeg.stdout?.on('data', (chunk) => { | ||
| chunks.push(chunk) | ||
| }) | ||
|
|
||
| ffmpeg.on('close', (code) => { | ||
| if (code !== 0) { | ||
| reject(new Error(`FFmpeg process exited with code ${code}`)) | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| const buffer = Buffer.concat(chunks) | ||
| const audioData = new Float32Array( | ||
| buffer.buffer, | ||
| buffer.byteOffset, | ||
| buffer.length / Float32Array.BYTES_PER_ELEMENT | ||
| ) | ||
| resolve(audioData) | ||
| } catch (error) { | ||
| reject(error) | ||
| } | ||
| }) | ||
|
|
||
| ffmpeg.on('error', (error) => { | ||
| reject(new Error(`Failed to spawn ffmpeg: ${error.message}`)) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does FFmpeg write logs/progress to stderr by default, and can an unread stderr pipe block a child process?
💡 Result:
Yes, FFmpeg writes logs and progress information to stderr by default [1][2][3]. Regarding whether an unread stderr pipe can block a child process: Yes, it can. When you capture FFmpeg's stderr using a pipe (e.g., via subprocess.PIPE in Python or similar mechanisms in other languages), the operating system provides a finite buffer for that pipe [4]. If FFmpeg generates log output faster than the parent process reads it, the buffer will eventually fill up [5][4]. Once the buffer is full, any further attempts by FFmpeg to write to stderr will block the process until the parent process reads from the pipe and clears space [5][6][7]. To prevent this, you must ensure that your parent process actively reads from the stderr pipe (often by using a separate thread or non-blocking I/O) while FFmpeg is running [5][6][4]. Alternatively, if you do not need the log output, you can redirect stderr to /dev/null (or the equivalent on your platform) to avoid the need for reading it [4].
Citations:
- 1: https://ffmpeg.org/ffmpeg.html
- 2: https://stackoverflow.com/questions/747982/can-ffmpeg-show-a-progress-bar
- 3: https://www.ffmpeg.org/doxygen/8.0/group__lavu__log.html
- 4: quiet mode for run_async method might cause ffmpeg process to stick. kkroening/ffmpeg-python#195
- 5: https://stackoverflow.com/questions/40964071/piping-to-ffmpeg-with-python-subprocess-freezes
- 6: https://stackoverflow.com/questions/29028619/ffmpeg-blocking-pipe-until-done
- 7: https://superuser.com/questions/1708271/ffmpeg-waits-to-close-the-pipe-in-order-to-start-processing-data
🏁 Script executed:
# First, locate and examine the audio.ts file
fd -t f "audio.ts" | head -5Repository: IliasHad/edit-mind
Length of output: 102
🏁 Script executed:
# Get the file size first to ensure we can read it
wc -l packages/media-utils/src/utils/audio.tsRepository: IliasHad/edit-mind
Length of output: 106
🏁 Script executed:
# Read the relevant section around lines 185-213
sed -n '180,220p' packages/media-utils/src/utils/audio.tsRepository: IliasHad/edit-mind
Length of output: 852
🏁 Script executed:
# Find the spawnFFmpeg function definition
grep -n "spawnFFmpeg" packages/media-utils/src/utils/audio.ts | head -20Repository: IliasHad/edit-mind
Length of output: 206
🏁 Script executed:
# Search for spawnFFmpeg implementation across the codebase
rg "function spawnFFmpeg|const spawnFFmpeg|export.*spawnFFmpeg" -A 10Repository: IliasHad/edit-mind
Length of output: 920
🏁 Script executed:
# Check the context around line 123 to see how spawnFFmpeg is used there
sed -n '110,145p' packages/media-utils/src/utils/audio.tsRepository: IliasHad/edit-mind
Length of output: 888
🏁 Script executed:
# Read the full handleFFmpegProcess function
sed -n '1,50p' packages/media-utils/src/lib/ffmpeg.tsRepository: IliasHad/edit-mind
Length of output: 1621
🏁 Script executed:
# Get the full ffmpeg.ts file to see all code
cat packages/media-utils/src/lib/ffmpeg.tsRepository: IliasHad/edit-mind
Length of output: 1623
Add stderr handling to readAudio to prevent FFmpeg backpressure stalls.
The readAudio function at line 185 spawns FFmpeg but never drains the stderr stream. FFmpeg writes logs and progress data to stderr by default; when this output isn't consumed, the pipe buffer fills and blocks the child process, hanging the Promise. This pattern is already correctly handled in the extractAudio function at line 123.
Add a listener for ffmpeg.stderr and optionally include the captured stderr in error messages for better debugging:
Suggested fix
const ffmpeg = spawnFFmpeg(args)
const chunks: Buffer[] = []
+ let stderr = ''
ffmpeg.stdout?.on('data', (chunk) => {
chunks.push(chunk)
})
+ ffmpeg.stderr?.on('data', (data) => {
+ stderr += data.toString()
+ })
ffmpeg.on('close', (code) => {
if (code !== 0) {
- reject(new Error(`FFmpeg process exited with code ${code}`))
+ reject(new Error(`FFmpeg process exited with code ${code}: ${stderr || 'Unknown error'}`))
return
}📝 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.
| const ffmpeg = spawnFFmpeg(args) | |
| const chunks: Buffer[] = [] | |
| ffmpeg.stdout?.on('data', (chunk) => { | |
| chunks.push(chunk) | |
| }) | |
| ffmpeg.on('close', (code) => { | |
| if (code !== 0) { | |
| reject(new Error(`FFmpeg process exited with code ${code}`)) | |
| return | |
| } | |
| try { | |
| const buffer = Buffer.concat(chunks) | |
| const audioData = new Float32Array( | |
| buffer.buffer, | |
| buffer.byteOffset, | |
| buffer.length / Float32Array.BYTES_PER_ELEMENT | |
| ) | |
| resolve(audioData) | |
| } catch (error) { | |
| reject(error) | |
| } | |
| }) | |
| ffmpeg.on('error', (error) => { | |
| reject(new Error(`Failed to spawn ffmpeg: ${error.message}`)) | |
| }) | |
| const ffmpeg = spawnFFmpeg(args) | |
| const chunks: Buffer[] = [] | |
| let stderr = '' | |
| ffmpeg.stdout?.on('data', (chunk) => { | |
| chunks.push(chunk) | |
| }) | |
| ffmpeg.stderr?.on('data', (data) => { | |
| stderr += data.toString() | |
| }) | |
| ffmpeg.on('close', (code) => { | |
| if (code !== 0) { | |
| reject(new Error(`FFmpeg process exited with code ${code}: ${stderr || 'Unknown error'}`)) | |
| return | |
| } | |
| try { | |
| const buffer = Buffer.concat(chunks) | |
| const audioData = new Float32Array( | |
| buffer.buffer, | |
| buffer.byteOffset, | |
| buffer.length / Float32Array.BYTES_PER_ELEMENT | |
| ) | |
| resolve(audioData) | |
| } catch (error) { | |
| reject(error) | |
| } | |
| }) | |
| ffmpeg.on('error', (error) => { | |
| reject(new Error(`Failed to spawn ffmpeg: ${error.message}`)) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/media-utils/src/utils/audio.ts` around lines 185 - 213, The
readAudio function spawns FFmpeg via spawnFFmpeg but never drains ffmpeg.stderr,
which can cause the child to block; add a listener on ffmpeg.stderr (e.g.,
ffmpeg.stderr.on('data', chunk => { /* collect or discard */ })) to consume
stderr and prevent backpressure, collect stderr into a string/Buffer and include
it in any reject/new Error messages inside the ffmpeg 'close' and 'error'
handlers so failures from readAudio surface the FFmpeg stderr output (mirroring
the pattern used in extractAudio).
2 participants
Summary
Bug fixes
ENABLE_QUEUE_UIenv var —Boolean('false') === truemeant settingENABLE_QUEUE_UI=falseactually enabled the UI. Fixed toval === 'true'.faceLabellingsilent failures — outercatchlogged the error but didn't rethrow, so BullMQ treated every failure as success and never retried. Addedthrow error. Also renamed the innerjobvariable todbJobto remove shadowing of the BullMQJobparameter.overallProgress: 1000— transcoding job reported 1000% progress on completion. Fixed to10.Authenticated BullBoard queue UI
The queue dashboard at
http://localhost:${BACKGROUND_JOBS_PORT}is now protected byrequireQueueAuthmiddleware:__session/__session_devcookie the web app already sets. Verified with HMAC-SHA256 matching React Router'screateCookieSessionStoragesigning format exactly.http://localhost:<PORT>/auth/login?next=<queue-url>. After login, the web app bounces back to the queue page.WEB_APP_URL(http://web:3746) is rewritten tohttp://localhost:3746for browser-facing redirects.?nextsupport — the login route now reads?nextfrom the URL, threads it through a hidden form field, and redirects there on success (validated tohttp://orhttps://only to prevent open-redirect attacks).Test plan
ENABLE_QUEUE_UI=falsein env and confirm BullBoard does not mountENABLE_QUEUE_UI=true, visithttp://localhost:${BACKGROUND_JOBS_PORT}without being logged into the web app → should redirect to web login with?next=param🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores