* fix(docker): boot when the container runs as a non-default UID
Unraid runs the container as 99:100, which can't write to the image's
/romm (owned by 1000, mode 755), only to the mounted subfolders. The
PSP pending dir at /romm/cache/retroarch_sync was created at import
time, so every gunicorn worker failed to boot.
- Buffer pending PSP save files under the user's assets folder
(assets/users/<user>/retroarch/psp_pending/), beside the other
RetroArch Cloud Sync files and outside the client-reachable blob
categories.
- Move the zip and rom-converto download caches to /tmp/romm/cache,
and point nginx's internal /cache/ alias there.
- Put gunicorn's control socket in /tmp/gunicorn instead of
$HOME/.gunicorn, since HOME is / for that UID.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(docker): honor ROMM_TMP_PATH for caches, explain an unwritable sync folder
- The download caches live under $ROMM_TMP_PATH/cache when it's set,
so installs with a small Docker disk (Unraid's docker.img) can point
them at a share. The entrypoint exports the same path for nginx's
/cache/ alias.
- With ENABLE_SYNC_FOLDER_WATCHER on and /romm/sync not writable, /init
warns and skips the watcher instead of dying on mkdir under errexit.
- The compose example lists the /romm/sync mount the sync features need.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* test(sync): cover reading back and clearing buffered PSP files
Addresses Greptile's review on #5189.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(sync): apply code-review findings to PSP pending buffer
Remove the emptied psp_pending folder after a merge, define the sync
folder path once in init, update the architecture doc's PSP and blob
locations, and drop a stale .gitignore entry.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* refactor(sync): apply simplify findings to PSP pending cleanup
Clear the buffered folder with remove_directory, let the startup check
own creating the sync folder, and reword the test fixture docstring.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(sync): cap buffered PSP files and drop them on delete
Buffered files for an unresolved save folder now count against the
bundle limits, so a folder can't fill the assets share and then fail
every merge once it resolves. A DELETE also removes the buffered copy,
so a deleted file isn't merged back when the folder resolves.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(docker): check the sync folder for push-pull sync too
ENABLE_SYNC_PUSH_PULL keeps SSH keys and device saves under /romm/sync
as well, so an unwritable folder now disables it with the watcher and
warns at boot instead of failing when the task runs.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(docker): strip whitespace from env vars like the backend does
The backend strips every variable it reads, while the init script and
nginx templates used raw values, so a padded ROMM_TMP_PATH sent nginx's
/cache/ alias somewhere the backend never writes.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* chore(config): drop the CACHE_BASE_PATH comment
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* docs(docker): say which backend config strips env whitespace
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* revert(docker): drop the entrypoint env whitespace trimming
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(sync): apply pr-ready code-review findings
Gate push-pull on its SSH keys path rather than a writable sync folder,
accept the same truthy values as the backend, and clear only the merged
PSP pending files so another worker's buffered file survives.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* refactor: apply pr-ready simplify findings
Share one truthy pattern across init's flag checks (the watcher gates
now accept the same values as the backend), disable gunicorn's unused
control socket instead of placing it, derive the converts redirect from
its own cache path, and factor the unknown PSP folder in tests.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* docs(docker): apply pr-ready humanizer pass to the sync mount note
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(fs): create handler base folders on first write, skip leftover temp files
FSHandler no longer mkdirs its base path when constructed, so a module-level
handler rooted at an unmounted or unwritable folder can't take down boot;
every write path already creates its parent folders. Blob listings skip
.romm_tmp_ files a crashed write left behind, so they are never merged into
a PSP bundle or advertised in the sync manifest. The prefix now lives in one
TEMP_FILE_PREFIX constant.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
report_task_failure can run in the worker parent when a work horse is
killed, so it now enqueues notify_task_failure on the default queue
instead of loading the notification stack there.
enqueue_task defaults to the queue its spec names. The streaming worker
runs as PreloadingWorker, which imports the enabled scheduled tasks of
its queues, so the reaper's per-minute horse inherits its stack. Spec
tests now read config in a fresh interpreter instead of comparing a
value with itself.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
- A failed login keeps the attempted username only when it names an
account, so a password typed into the username box is never stored.
- Gunicorn trusts X-Forwarded-For only from loopback and private ranges
(FORWARDED_ALLOW_IPS, documented in env.template), so a client can no
longer choose the address the audit log and rate limits see.
- A player's fetch (purpose=play) is recorded as rom.player_load instead
of being skipped, so the marker can't hide a download.
- Play-session ingest treats a rom hidden from the caller as unknown, so
its session and rom.play event don't name it.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The test checks the one line the watchdog relies on, next to the keepalive
pair instead of between it, and the comment states RQ's late write once.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Adds a test that start_rq_worker records $! instead of passing --pid, and
moves the comment next to the line it explains.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The watchdog restarts any process whose PID file is missing, and RQ's --pid
writes that file only after importing the worker class. Since RomMWorker
imports tasks.tasks (and with it config, SQLAlchemy and the auth and socket
handlers), reaching that point takes seconds on a cold, bytecode-less image,
so on a slower host the watchdog started a fresh pair of workers every 5 s
and each one lived on as a duplicate. The init script now writes $! like it
does for the cron scheduler and the watchers.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
open_file_cache keyed descriptors by path, so a cover replaced via
os.replace() kept serving the old inode until the validity window expired,
and the ?ts= buster does not change the filesystem key. The comment beside
it claimed the opposite. The benefit was never measured either: the cache
saves an open() and an fstat() per request, which is microseconds on a
local file, and covers are served immutable so repeat hits are rare.
Removing it is simpler than scoping it away from the one tree it was
supposed to help.
The keepalive test asserted gunicorn outlives nginx's idle timeout without
checking that nginx pools upstream connections at all, which it only does
by default from 1.29.7, so a downgrade would have left the test green and
the pooling silently gone. It now checks the pinned version too.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HDXxq3vAxJVGhm1pQuK8V7
The comment and test docstring said a short keep-alive makes nginx reuse a
connection gunicorn is closing and return 502. Reproducing the race says
otherwise: across 320 requests timed against the close boundary, GET and
POST alike, nothing failed and nginx logged nothing. It notices the closed
idle connection and opens a fresh one.
What the short value does cost is the connection pool. Measured with a
counting upstream behind nginx, eight requests three seconds apart opened
eight upstream connections at a 2s idle close and one at 65s, so below
nginx's idle timeout the keepalive it now enables by default never pays off.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HDXxq3vAxJVGhm1pQuK8V7
nginx 1.29.7 is a security release (CVE-2026-27654, CVE-2026-27784,
CVE-2026-32647, CVE-2026-27651, CVE-2026-28753, CVE-2026-28755) and also
enables upstream keepalive by default, defaulting proxy_http_version to
1.1. That removes a TCP connect and teardown per /api request, which the
config never opted into.
It also makes the gunicorn keep-alive too short: nginx now holds idle
upstream connections for up to 60s while gunicorn closed them after 2,
so nginx could send a request on a connection gunicorn was closing and
return 502. Raise the default past nginx's idle timeout.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HDXxq3vAxJVGhm1pQuK8V7
A single worker was the reason a lone user loading a gallery measured
6.474 s worst-case TTFB against 1.939 s at 4 workers. The multi-worker
path is already supported: the log forwarder takes a Redis lock so
exactly one worker relays.
Only installs that never set WEB_SERVER_CONCURRENCY move; any explicit
value is preserved.
Co-Authored-By: Claude Opus 5 <[email protected]>
The dispatch job was hidden from the task list by a meta flag the endpoint
had to look for on every job in every queue and registry. RQ already has
the rule the flag was implementing: `result_ttl=0` deletes a job the moment
it succeeds, while a failed one still lands in the failed registry under
its own TTL. Registering the scan entry with it drops the marker, the
predicate, its four filter sites and the two Redis reads each one cost.
The concurrency guard is now one call. `get_blocking_library_scans()` says
which scans a second one has to wait for, so the socket handler no longer
assembles that rule out of two helpers in two shapes, and `ALL_QUEUES` sits
with the queues it lists rather than in the API layer.
Also merges the finished and failed registry sweeps, which were the same
loop, trims comments that explained the design rather than the code, and
drops two socket tests that duplicated their neighbours.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Master landed #4274 with the review changes that came out of it, and those
moved the code this branch was built on. Scan job bookkeeping now lives in
`handler/scan_jobs.py`, every enqueue goes through `enqueue_task`, cron
entries carry their own name and result TTL, and the watcher builds its two
rescans from one helper.
Resolved by putting this branch's changes where that code now lives:
- The scan queue, the scoped-scan check and the second scheduled registry
moved into `handler/scan_jobs.py`, keeping master's single-round-trip
reads (`get_job_ids` plus one `fetch_many`, and `get_jobs_to_schedule`
for the stale sweep) across both registries.
- `enqueue_scheduled_scan` sits next to `enqueue_task` in the registry and
is built from it, rather than hand-rolling the same enqueue in
`tasks/tasks.py`. Its tests moved with it.
- Cron keeps master's per-entry name and result TTL, and adds the scan
dispatch on top: `TASK_TIMEOUT` for the dispatch, the task's own timeout
(now `SCAN_TIMEOUT`) for the scan it enqueues.
- The watcher's `schedule_rescan` helper enqueues on the scan queue with
the abandoned-job callback.
- `PeriodicTask.func` is gone from master, so the scheduled rescan declares
only its timeout. `tasks.registry.enqueue_scheduled_scan` joins the func
paths an RQ worker has to be able to import.
- The job stubs shared by the discovery tests grew the scan queue and a
scoped-scan job; the duplicated stale-sweep tests are master's.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Scheduling moved onto RQ, which left the task catalog with two identity
schemes: the registry key a job payload now carries, and the dotted func
path each PeriodicTask still declared. Nothing read the latter, so it is
gone, along with the test that guarded paths no payload names.
Four sites built the same enqueue by hand and had already drifted: the
startup backfills passed the global timeout and no result_ttl, so an
auto-enqueued conversion fell out of the task list after RQ's default
500s while the same task triggered by an admin lived for TASK_RESULT_TTL.
They now go through enqueue_task, which derives timeout, ttl and meta
from the task itself.
Scan discovery paid a Redis round trip per queued job to re-read a status
the fetch already carried, and the stale sweep read every delayed job to
find the few past due. Both now ask Redis once.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A library scan runs for hours on the same worker as everything else, so
every cleanup, conversion and metadata refresh waited behind it. Scans go
on a queue of their own, consumed by a second worker, and the queues that
serve the rest of the app keep moving while a scan runs.
One worker on one queue also makes "one scan at a time" structural rather
than something the socket handler has to enforce, and it leaves another
worker running to notice when the scan worker dies.
Which is what the second half of this does. A worker killed mid-scan
never reaches the handler that reports the scan ended, so clients kept
showing a scan that no longer existed. RQ hands an abandoned job to its
failure callback when a surviving worker reaps it, so scans carry one
that emits `scan:done_ko`. Only for AbandonedJobError: every other
failure is already reported by the scan as it unwinds, and emitting here
too would report it twice. The scheduled rescan is not covered, because
rq cron cannot attach callbacks.
Scan discovery still reads the other queues, so a scan enqueued before
this change is still found and still runs.
Co-Authored-By: Claude Opus 5 <[email protected]>
The shell built the connection URL with `redis${REDIS_SSL:+s}`, which
only asks whether the variable is set. `env.template` ships
`REDIS_SSL=false`, so following the documented value pointed the worker
at `rediss://` while the app parsed the same variable through
`safe_str_to_bool` and connected in plaintext. Adding the cron process
put a second process behind it.
Parse the value in both entrypoints, matching the truthy set the app
accepts.
Also clear the old scheduler's lock and instance keys even when its
registry held no jobs: a scheduler that never queued anything still
registered itself, and only the orphan-job work needs the registry.
Co-Authored-By: Claude Opus 5 <[email protected]>
rq-scheduler has one release since 2023, 109 open issues, and RomM ran a
fork of it for Redis username and SSL options that upstream never took.
Everything it was here for now ships with RQ.
Its two failure modes were the reason for the workarounds in 9e9f59c04.
It reads a job's function name before taking the job out of its registry,
so one unreadable job stalls it for good; RQ's scheduler removes every id
it looked at whether or not the job could be read. And cancelling a
delayed job needed both `Scheduler.cancel` and `Job.cancel`, because the
first left the status alone and the second left the registry entry, which
the scheduler then queued anyway; `Job.cancel` on its own now clears the
scheduled registry.
Delayed jobs, which is how the watcher defers a rescan, go onto the queue
with `Queue.enqueue_in` and are released by the worker running with
`--with-scheduler`. Periodic tasks are declared in `tasks/cron_config.py`
and loaded by an `rq cron` process, replacing `rqscheduler`. A task is
registered only when it is enabled and has a cron string, so the
schedule-then-unschedule dance around the env toggles is gone, and with
it `PeriodicTask.init/schedule/unschedule`.
Jobs are enqueued by task name through `run_task_by_name`, resolved
against a catalog in `tasks/registry.py`. Nothing pickles a task instance
into Redis any more, which is what made a payload unreadable across
upgrades in the first place. The catalog also gives the tasks endpoint
one home for what a task is, rather than a list of its own.
Scan discovery keys off the type a job carries rather than the scheduled
rescan's function name, since every task now shares one entry point. That
also drops SCAN_LIBRARY_TASK_FUNC, which only existed to name a function
across an import cycle.
The watcher's dedupe worked on `job.args[0]` while scans are enqueued with
keyword arguments, so it never matched and every filesystem change
scheduled another scan. It reads `platform_ids` from the payload now, and
deliberately ignores a scan already running: that one may have walked past
the folder that just changed.
Startup clears what the old scheduler left in Redis, once.
Co-Authored-By: Claude Opus 5 <[email protected]>
The image bundles valkey but not its CLI, which Alpine packages
separately, so the internal cache cannot be inspected from inside a
running container. The watchdog also restarted dead processes silently,
hiding both the crash and whatever state died with it.
Co-Authored-By: Claude Opus 5 <[email protected]>
The worker received its connection string through `rq worker --url`, which
put the password in argv for the life of the process, readable via
/proc/<pid>/cmdline, ps, and any monitoring or diagnostic tooling that
collects command lines (CWE-214).
RQ's `--url` option already reads from RQ_REDIS_URL, so passing it through
the environment instead is behavior-preserving. This matches how the RQ
scheduler alongside it already takes its credentials.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The scheduler only started when one of four ENABLE_SCHEDULED_* flags was
true, but nothing else respects that condition. startup.py registers the
netplay, upload-tmp and zip-cache cleanups unconditionally, three more
flags gate periodic tasks that are absent from the list, and the
filesystem watcher defers its rescans through the scheduler as well.
Any of those leaves jobs sitting in the scheduler registry with no
process to run them. On a default install the three cleanups never fire
at all. A watcher running with none of the four listed tasks enabled
leaves a delayed scan_platforms entry that nothing will execute, and the
concurrent scan guard reads it as a scan already queued, so every manual
scan is refused with "A scan is already in progress". Restarting does not
help, because the entry is persisted state rather than a running process.
Start the scheduler unconditionally, matching entrypoint.sh, which never
gated it. A stuck entry then clears on its own, since the scheduler
enqueues past-due jobs as soon as it comes up.
Co-Authored-By: Claude Opus 5 <[email protected]>
The keyspace holds the Switch TitleDB and LaunchBox caches, so dump.rdb runs
to hundreds of MB (424MB on a dev instance). Valkey's stock save policy
rewrites all of it every ~5 minutes once 100 keys change, which idle RQ
bookkeeping alone triggers, for gigabytes of disk writes per hour.
Fixes#3980
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Orphaned covers and screenshots left behind by deleted ROMs accumulate
indefinitely, because the only way to reclaim them is to trigger the
cleanup task by hand. On a long-lived instance the resources directory
grows without bound, which is a problem when it shares a filesystem with
the database.
Move the task to `tasks/scheduled/` and turn it into a `PeriodicTask`
with an opt-in cron schedule, defaulting to 5:00 AM so it runs after the
nightly scan and metadata tasks have settled.
`enabled` stays `True` rather than tracking the new env var, because the
run-task endpoint requires both `enabled` and `manual_run`. Gating it
would have silently removed the ability to run the cleanup by hand,
which is how it works today. The cron string is what is opt-in, and
`init()` unschedules when it is unset.
Add the new flag to the init script's scheduler condition, so enabling
only this schedule still starts `rqscheduler`.
Co-Authored-By: Claude Opus 5 <[email protected]>
The init script runs with `set -o nounset`, so testing `${ROMM_AUTH_SECRET_KEY}`
aborted with "unbound variable" before reaching the generation branch, leaving
the container in a restart loop. This only triggered when the variable was
omitted entirely; shipping it with an empty value took the working path.
Use `${ROMM_AUTH_SECRET_KEY-}` so the unset case is testable, and warn instead
of info since a generated key changes on every restart.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The nginx upstream was hardcoded to 127.0.0.1:5000, while gunicorn and the
readiness check bind/probe ${DEV_PORT}. With a non-default DEV_PORT the
readiness check passes but every proxied request returns 502.
nginx.conf is not run through envsubst, so move the wsgi_server upstream into
the templated default.conf.template and export DEV_PORT in the entrypoint so
the upstream port tracks the gunicorn bind.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
nginx workers are dropped to the `romm` user while gunicorn creates
/tmp/gunicorn.sock as root. This only worked because gunicorn's default
umask leaves the socket world-writable; on some hosts (e.g. OMV) the
worker cannot connect() and every /api request 502s with
"connect() to unix:/tmp/gunicorn.sock failed (13: Permission denied)".
Proxy over loopback TCP (127.0.0.1:5000), which gunicorn already binds,
so there is no socket file owner/mode to mismatch. Readiness now polls
the TCP port instead of waiting for the socket file.
Fixes#3809
Generated-By: PostHog Code
Task-Id: 3cb72cda-eb77-4bc0-97ab-cde365a031c9
The registry-cleanup log noise is already suppressed by the RomMWorker log
filter, so the hourly maintenance interval was unnecessary and delayed pruning
of orphaned STARTED jobs and stale workers. The watcher's Worker.all() scan
dedupe relies on that pruning, so restore the default (~10 min) cadence.
Generated-By: PostHog Code
Task-Id: 78c8b2f8-2b03-443f-aa4c-8867c1259f2a
The default Docker image symlinked /romm/assets into the nginx static web
root (/assets/romm/assets), where it was served by an unauthenticated
`location /assets { try_files ... }` block. /romm/assets holds private user
data (save files, save states, screenshots, avatars) that is meant to be
accessible only through the authenticated /api/raw/assets/{path} route
(Scope.ASSETS_READ). The static symlink bypassed that protection, letting any
unauthenticated caller read another user's files given a (guessable) path.
Avatar URLs leaked the hex user ID through the same static route, making path
construction straightforward.
Fix:
- Drop the /romm/assets symlink from the Docker image build and both
entrypoint scripts; only /romm/resources (public cover art, screenshots,
manuals) remains statically served.
- Point the frontend avatar URLs at the authenticated /api/raw/assets/ route
instead of /assets/romm/assets/. Browser <img> loads authenticate via the
existing session cookie.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
All callers declare a fresh `local -a wrap=()` before invoking, so the
in-function reset is unnecessary.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Collapse `otel_prefix` and `otel_prefix_str` into a single nameref-based
helper. Watchfiles call sites embed the array as a shell-quoted prefix
via `${wrap[*]@Q}`, which also fixes a quoting bug where an
`OTEL_SERVICE_NAME_PREFIX` containing a single quote would produce an
invalid command string and break the watcher.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Collapse the duplicated OTEL_SDK_DISABLED / opentelemetry-instrument
branches in run_startup, start_bin_gunicorn, start_bin_watcher, and
start_bin_sync_watcher into two small helpers:
- otel_prefix: emits the wrapper as NUL-delimited argv tokens (for
direct process invocation).
- otel_prefix_str: emits the wrapper as a shell-string prefix (for
embedding inside `watchfiles --target-type command`).
Each call site becomes a single command instead of a 2- or 3-way
branch with a fully duplicated command body. As a side effect, the
watcher functions now also gain the `command -v opentelemetry-instrument`
fallback that the gunicorn/startup paths added.
The shell fallback was assigned locally but never exported, so
sync_watcher.py and the Python config layer never saw the resolved
value. They happened to land on the same /var/lib/romm/sync default by
coincidence; export it so the shell and Python defaults stay linked
through a single source of truth.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The prod Dockerfile creates /var/lib/romm/sync at build time, but if a
user overrides SYNC_BASE_PATH to a path that doesn't exist (or runs the
dev entrypoint, which never created the default), watchfiles fails to
start because its target directory is missing. Have both entrypoints
mkdir -p the resolved path before handing it to watchfiles.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Move the sync staging folder out of ROMM_BASE_PATH so it lives on a
dedicated writable mount. This lets the container run with a read-only
root filesystem without losing in-flight save uploads, and keeps
app-owned state separate from the user-curated library volume.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The init script ran `sed -i` against /etc/gunicorn/logging.conf, which
fails both on read-only root filesystems and when the container runs
as a non-root UID (since /etc/gunicorn is not chmod'd writable). Copy
the config to /tmp/gunicorn/logging.conf at startup and edit/use that
copy instead, leaving the image file untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The gunicorn logging config hard-coded level=INFO, ignoring LOGLEVEL.
Patch it at startup via sed. Also pass --logging_level to rq worker
and rqscheduler so their framework logging respects LOGLEVEL.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
