annotationBoolFilter serves both starred (boolean) and rating (integer);
the boolean form broke the Top Rated lists with 'COALESCE types integer
and boolean cannot be matched'.
The starred filter used COALESCE(starred, 0) > 0, which PostgreSQL
rejects for a boolean column ('COALESCE types boolean and integer cannot
be matched'); the Favourites lists returned 500.
The fts backend needs SQLite's FTS5 virtual tables; on this branch any
search returned 'syntax error at or near MATCH'. The LIKE backend works
unchanged on PostgreSQL.
A client that starts a handshake and disconnects before the password
step (a pool connection attempt timing out behind a long scanner
transaction) left the single backend in ClientAuthInProgress, and it
then answered other clients' queries without ReadyForQuery, or read
their query as an empty one, until the next handshake completed. Each
such reply broke a pooled connection and, during a scan, the scanner
transaction on it. The bridge now replies AuthenticationOk, the cached
ParameterStatus messages, BackendKeyData and ReadyForQuery itself for
every client after the first, answers SSLRequest with N, and drops
CancelRequest connections.
PGlite's interactive_one discards an incoming request when output from
the previous frame is still pending ('flush after frame'), which showed
up as replies without ReadyForQuery, first after handshakes and then in
bursts mid-scan, each one breaking a pooled connection and the scanner
transaction on it. After a complete reply the bridge now runs one empty
tick and appends anything the backend still had, and logs the message
tags of any reply that is still incomplete.
The whole pglite data dir was mounted as the guest's /tmp, which also
exposed the cluster twice. The guest only needs its password file and
the shared-memory stand-ins there, so <DataFolder>/pglite/tmp is mounted
instead. The cluster and /dev mounts are unchanged.
PGlite splices a fixed list of -c settings into every start and never
reads postgresql.conf or postgresql.auto.conf on restart (ALTER SYSTEM
writes to the wrong directory and pg_reload_conf is a no-op without a
postmaster), so the only way to lower max_wal_size is that list. With
128MB, a CHECKPOINT trims pg_wal from 405 MB to 134 MB locally instead
of letting it grow to the 1 GB default.
PGlite's single-user backend has no background checkpointer, so nothing
written during a scan is checkpointed until shutdown, and a crash would
replay the whole scan's WAL. Run CHECKPOINT right after the post-scan
ANALYZE. The log directory itself stays around max_wal_size because
PostgreSQL recycles segments rather than deleting them.
The three counters each joined media_file_artists to media_file again,
about 10 s per pass on the 97k-track library. A shared materialized
artist_rows set is joined once and aggregated three times: 35 s to 25 s
on roy for a full refresh, 240 ms to 138 ms per 1000-artist batch
locally.
database/sql keeps only 2 idle connections by default, so with 4 open
the pool closed and reopened connections constantly: about 225
Terminate+handshake cycles per minute during a scan on roy. Every
handshake goes through the single backend, and the first query after a
handshake is where the lost replies were seen.
Three times during a scan on a real library, a new pool connection's
first query got no bytes back at all, which the previous commit turned
from a hang into an error. A run always emits at least a completion, so
no output means the packet was not run; the bridge now hands it over
again (twice at most) and logs it.
The stats UPDATE now joins the materialized counters (with a LEFT JOIN
so artists without files still get '{}') instead of a correlated
subquery, and a full refresh runs as a single statement: 3.3 s for 30k
artists locally versus 30 batches. Batches of 1000 remain for partial
refreshes (0.24 s each).
library.total_size was integer; a 2 TB library overflowed it at the end
of the first scan. It is bigint now, matching the Go int64.
The bridge no longer synthesizes a reply for Terminate, which never gets
one by design.
When the backend produced no bytes for a request, or a reply without
ReadyForQuery, the bridge forwarded it as-is and the client waited for
ever while the session was free. On roy this froze the UI: a pooled
connection hung right after its handshake and every request queued in
the pool behind it. Incomplete replies now get a synthesized error plus
ReadyForQuery, and the bridge logs them, so a glitch becomes a visible
SQL error that the pool can retry.
PostgreSQL inlines single-use CTEs, so the correlated subquery in the
final UPDATE re-ran the whole aggregation for every library_artist row,
rescanning all media files of the library three times per artist. On
the 97k-track library one 1000-artist batch ran for more than five
minutes. MATERIALIZED computes each CTE once per batch: 0.3 s versus
1.1 s locally, and orders of magnitude on the real plan.
The CTE placed inside NOT EXISTS is correlated and re-evaluated for every
tag row, so it was as slow as NOT IN on the real library (still running
after 10 minutes). id IN (SELECT id FROM tag EXCEPT <used ids>) builds
the unused set once with a hash set-op: about 1 s at 1M tag references,
with or without planner statistics.
On a 97k-track library the tag purge in the scanner GC ran for many
minutes: PostgreSQL re-evaluates a NOT IN subquery of ~1M tag references
per tag row. A materialized set with NOT EXISTS takes 1.4 s locally
versus 55 s for NOT IN at that scale; work_mem made no difference.
PostgreSQL rejects SELECT DISTINCT with ORDER BY random(), which the
playlist cover picker uses to sample album ids. GROUP BY yields the same
distinct set and allows the volatile ordering.
The bridge gave every reply write a 5 second deadline. Scanner phase 3
streams all touched albums through a cursor and runs queries per row on
other pooled connections, which queue on the single session, so a large
library could not drain the reply in time and the bridge closed the pipe
mid-message ('unexpected EOF'). Replies now block as long as the client
needs, and client connections are closed on shutdown instead.
Playlist rules were bound as []byte, which pgx sends as bytea; the text
column then stored the hex form and the JSON parser failed on it.
Restore the go and coverage jobs on the spike branch, but test only
conf/ (which passes here) so a coverage artifact is still produced and
the Report coverage on PR workflow stops failing on every push.
The PGlite fork stubs MemoryContextResetAndDeleteChildren, which
PostgreSQL 17 removed, as an empty macro, so MessageContext was never
reset and each statement leaked its parse and plan state (about 10 KB).
On a real library the process grew to 15 GB during the first scan and
the host swapped itself to a crawl. The new build maps the macro to
MemoryContextReset (message_context_reset.diff); memory now stays flat
across thousands of statements. Adds PGlite.MemorySize for measuring.
In PostgreSQL, a bare column name in ON CONFLICT DO UPDATE is ambiguous
between the target row and excluded, and MAX() is an aggregate rather
than the two-argument function SQLite provides. Every artwork enqueue
failed with 'column reference "priority" is ambiguous' on a real
library.
pglite.wasi.gz is PostgreSQL 17.5 built for WASI at -O2 without the wizer
snapshot, gzipped from 14.6 MB to 4.9 MB. db/pglite/embed.go go:embed's
it, so the pipeline cannot build the binary without it in the tree. The
recipe that produces it is in db/pglite/build/README.md.
Comment out the go, go-plugins, go-windows and coverage jobs and the npm
test step, and drop the test jobs from the build job's needs, so the
pipeline still builds. The persistence and other suites that create an
in-memory SQLite database cannot run on this branch and would fail every
push. Commented rather than deleted so they are easy to restore.
Replace the 131 SQLite migrations with a single goose migration that creates
the whole schema in PostgreSQL (the user table becomes user_account, json
columns become jsonb, seed rows for the default library and transcodings).
Port the repositories' SQL just far enough for Navidrome to start, scan,
log in and browse artists and albums: json_agg/jsonb_* for the SQLite JSON
functions, lower() for COLLATE NOCASE, ON CONFLICT DO NOTHING, coalesce on
annotations, stricter GROUP BY, explicit casts under the simple protocol,
sequential queries in library.RefreshStats. db/ is now Postgres-only, with
backup/restore stubbed and DevExternalScanner forced off.
Embed the PGlite module like go-taglib: pglite.wasi.gz is go:embed'ed,
decompressed in memory and compiled by wazero with an on-disk compilation
cache, so the module itself is never written to disk. The cluster is
created by initdb at runtime on the first start; initdb runs in a
throwaway wasm instance and the backend in a fresh one, otherwise the next
start cannot find a valid checkpoint. Since initdb creates only template1,
a short-lived backend then runs CREATE DATABASE navidrome and the real
backend starts on it. The build recipe and patches for the -O2, no-wizer
module live in db/pglite/build.
Adds db/pglite, a bridge that runs the PGlite WASI build of PostgreSQL 17.5
inside the Navidrome process with wazero and exposes it to database/sql
through pgx over the real wire protocol. Selected with DbPath = "pglite://<dir>".
The bridge handles what the WASI build cannot do on its own: it emulates dup2
onto stdin (wazero refuses fd_renumber on preopens), replays ParameterStatus
for connections after the first, synthesizes a missing ReadyForQuery after an
error, recovers from the trap every PG ERROR raises, and passes wire bytes
through shared wasm memory instead of files (round-trip floor 377 µs to 39 µs).
Several connections are accepted and serialized onto the single backend,
holding the session across a whole transaction and a whole handshake; a client
that disconnects mid-transaction gets a ROLLBACK.
db.go opens the pglite:// scheme, skips the SQLite-only migrations, and can
apply a translated schema from ND_PGLITE_SCHEMA. The shared count() helper now
adds its ORDER BY only for SQLite, since Postgres rejects it next to
count(distinct ...).
The wasm archive is not committed. The README explains how to build it and
lists the limits found: one session, one process (DevExternalScanner must be
off), shared session state, simple protocol only.
`buildPlaylist` reported `evaluated_at` as `changed` for smart playlists, so a
rename or comment edit was invisible to clients until the next evaluation. A
never-evaluated smart playlist also reported the current time on every call,
which never settled.
Report `updated_at` for every playlist. `refreshCounters` now syncs the stamp it
writes back onto the model, and `refreshSmartPlaylist` reuses it for
`evaluated_at`. `changed` therefore still equals the evaluation time for a
just-evaluated playlist, and `validUntil` stays anchored to it.
Original Subsonic always bumps `changed` on any playlist update, so this also
aligns the behavior with upstream.
The Refresh Metadata action was only reachable from the Album and Artist context menus, so it could not be triggered from AlbumShow or ArtistShow. This adds an icon-only button, with a tooltip, to the action toolbar on both detail pages. Like the menu entry, it is only rendered for admins.
The button is built on react-admin's Button rather than a plain IconButton: the surrounding toolbars use the former, so the theme colour and the icon-only swap at the xs breakpoint are inherited instead of restated. The dataProvider call and its two notifications move into a new useRefreshMetadata hook, which ContextMenus now shares, keeping a single copy of that logic.
A playlistTrack id is a position in the playlist, not a stable key, so refetching
a row by id after the annotation is saved can return a different song: in a smart
playlist filtered on that annotation the track is gone and every later row has
shifted up. The stale-keyed record then renders as a duplicate of its neighbour.
Signed-off-by: Bjørn A. Andersen <polybjorn@users.noreply.github.com>
Co-authored-by: Bjørn A. Andersen <polybjorn@users.noreply.github.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
The exclusion only worked on master. Coverage profiles name files by import
path; octocov shortens those to repo-relative paths using the checked-out
source, but coverage-on-pr.yml sparse-checks-out only .octocov.yml, so the
paths stay as github.com/navidrome/navidrome/tests/mock_*.go and 'tests/**'
never matched. '**/*_gen.go' matched either way, which is why only the 30
tests/ files leaked.
Every pull request since b77fb45 therefore reported ~-3.7% against master:
447 files on the base side, 477 on the pull request side (#6002, #6069).
* chore: ensure that all durations are nonnegative
* make sure you actually include the test file
* test(conf): use non-zero durations in the valid_duration fixture
Zero is the boundary between the accepted and rejected ranges, so it
passes even if the guard is off by one. 1s exercises an ordinary value.
---------
Co-authored-by: Deluan <deluan@navidrome.org>
An absent artwork state was revisited by an hourly job, by viewing the entity, and
by the startup backfill on any artwork config change. On a large library the last
one queued tens of thousands of external lookups at once and got the provider to
rate-limit us for hours.
Nothing revisits an absent state now. Retrying is explicit: `artwork reprocess` on
the CLI, or the refresh button in the UI. The config fingerprint survives only as an
advisory, warning at startup and naming the command that clears it.
Since absent is terminal, `artwork status` splits it into two disjoint columns, and
`--source failed` targets only the ones that gave up rather than being answered.
Both read through the filter CountBySource and EnqueueBySource already share, so the
reported number is the set the command acts on.
Also fixes the last_failure default left by 20260819204637, which marked every
pre-existing absent row as failed, and removes the code the deleted retry paths
orphaned.
The coverage profile counted the tests/ package and the *_gen.go files, none of
which are code under test: tests/ is the mock and helper package, and generated
code is never hand-tested. Together they added 2926 uncounted statements at 0%,
pulling the reported number down by almost 6 points (70.13% -> 75.98% on the
current master profile).
octocov's coverage.exclude takes doublestar globs matched against git-root-relative
paths. All 26 mock_*.go files live under tests/, so the single 'tests/**' pattern
covers them.
* fix(deezer): treat an exhausted quota as a throttle, not as a missing artist
Deezer reports quota exhaustion in the response body, with HTTP 200 and no
rate-limit headers. The client only looked for errors when the status was
not 200, so a throttled reply was decoded into an empty result type, and an
empty search became ErrNotFound. The agent then compounded it: it tested
`errors.Is(err, ErrNotFound) || len(artists) == 0` before testing err, so
any failed search — which also returns no artists — reported not-found too.
The artwork worker settles an entity as "no image" on agents.ErrNotFound.
So being throttled did not make Navidrome back off; it made it record the
artist as having no artwork, and move on to do the same to the next one.
Errors are now parsed out of the body regardless of status, and the quota
code is joined with agents.RetryLaterError so the circuit breaker and the
artwork retry budget see a throttle for what it is. The agent checks err
before the empty-result case.
Last.fm already handles this exact shape (client.go errCodeRateLimit, with
a comment noting the 200-with-body-error pattern); this brings Deezer in
line with it, including the zero-delay RetryLaterError so both providers
share the default cooldown rather than a per-provider number.
Measured against the live API to pin the shape: a 120-request burst
returned 54 results and 66 quota replies, every one of them HTTP 200 with
{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}
and no Retry-After or rate-limit headers. A single request 5s later
succeeded, so the window is short and a cooldown fully clears it.
* refactor(deezer): fold the error envelope into one type
The envelope declared the code and message inline, parseBodyError copied
them field by field into a second struct with the same shape, and a zero
Code stood in for "no error reported". Making the envelope hold a pointer
to the error type removes all three: absent is nil, present is the error
itself, and the value returned needs no conversion.
searchArtist loses its empty-result branch. searchArtists converts an
empty result to errNotFound and returns early on any error, so it never
answers with no artists and no error, and the branch could not run. What
it left behind was a comment explaining an ordering that only mattered
while the branch existed.
ErrNotFound is unexported: nothing outside this package referenced it,
and it sat three lines from agents.ErrNotFound, which is a different
error with the opposite meaning for callers.
Throttling now joins agents.ErrRetryLater, the sentinel documented as the
zero-delay RetryLaterError, rather than allocating an equivalent value.
* refactor(deezer): return agents.ErrNotFound from the client
The client raised a package-local sentinel that the agent then translated
into agents.ErrNotFound, one call site each. Deezer was the only adapter
carrying its own: last.fm and listenbrainz have none.
The client already reports throttling with agents.ErrRetryLater, so it
already speaks the agent vocabulary; saying "not found" in the same words
costs nothing and lets searchArtist drop to plain error propagation.
* test(scrobbler): remove a race in the longest-server-delay test
newBufferedScrobbler starts its drain goroutine, and run() drains once
before it ever waits on the wake signal. The test enqueued user2, then
enqueued user1 via Scrobble, so that startup drain could land between
the two: it saw only user2, took its 45s delay, and set backingOff. The
wake from the second enqueue is then deliberately ignored — a wake
during a backoff window must not drain, which is the hammering the
window exists to prevent — so user1 was never attempted and the first
assertion read 1 instead of 2.
Buffering both users before the goroutine exists removes the window.
The test no longer goes through Scrobble, which the sibling tests
already cover; what this one is about is which delay wins.
Reproduced deterministically by forcing the interleaving with a
synctest.Wait between the two enqueues, which fails with the same
"expected both users drained, got 1 attempts" seen in CI. With both
enqueued first, that same forced drain passes.
An explicit RetryLaterError now opens the agent's breaker immediately for the
provider's own delay, instead of counting it as one generic failure that needs
five to open and then always probes after a fixed minute.
* ci: comment coverage on pull requests from forks
A pull_request run from a fork gets a read-only GITHUB_TOKEN, so octocov could not post its comment: it logged a 403 and exited 0, leaving the job green and the PR silent. The 'permissions:' block cannot grant what the token does not have.
The comment now comes from a workflow_run workflow, which runs on the base repository and does get a write token. The pipeline job keeps the job summary and the default-branch baseline, and hands the merged profile and the PR number to it as an artifact.
A workflow_run job otherwise looks like a push to the default branch, so octocov is pointed back at the pull request and at the run that produced the profile via its OCTOCOV_ environment overrides. Without the run id override the test execution time would be read from the wrong run; without the ref override a fork's coverage would be stored as the master baseline.
The job holds a write token, so it reads .octocov.yml from the base branch rather than from the fork.
* ci: stop checking out the fork in the coverage comment workflow
CodeQL flagged the pull request checkout as untrusted code in a privileged context (actions/untrusted-checkout/high): the job holds a write token. The checkout existed only so the code-to-test ratio would reflect the pull request, which does not justify the alert.
The workflow now checks out just .octocov.yml from the base branch, and the ratio is skipped when reporting from there. Coverage and its delta against master, the metrics that motivated the report, are unaffected: they come from the profile the pipeline uploads.
* ci: treat the coverage artifact as untrusted input
A pull_request run executes the fork's own copy of pipeline.yml, so every file in the octocov-pr artifact is attacker-controlled. The artifact was extracted into the workspace root, on top of the base-branch checkout, and download-artifact truncates existing files. A fork could therefore replace .octocov.yml before octocov loaded it.
That is not only a config swap. config.Load expands ${VAR} from the job environment and the action sets OCTOCOV_GITHUB_TOKEN, so a crafted comment.message posts the privileged job's token into a public comment; a body: section rewrites a pull request description, which pull-requests: write allows.
The artifact now lands in a subdirectory and only coverage.out is copied out, after pr_number is checked to be digits and the named pull request's head is confirmed to be the sha that triggered this run. Without that check the artifact could aim the comment at any open pull request, and unvalidated content reached GITHUB_OUTPUT.
* ci: report Go test coverage on pull requests
Adds octocov to the existing 'Test Go code' job. It reads the coverage
profile, posts a PR comment with the coverage percentage and the delta
against master, and writes the same report to the job summary.
The master-branch report is stored as a GitHub Actions artifact, so no
external service or secret is needed.
* ci: merge the plugins job coverage into the same report
The plugins suite runs in its own job, so its coverage was missing from
the report. Both jobs now upload their profile as an artifact and a new
'Report coverage' job merges them into a single PR comment.
* ci: update the coverage comment in place instead of reposting
octocov's default is to collapse the previous comment and create a new
one. updatePrevious edits the existing comment instead, so a PR keeps a
single coverage comment across pushes.
* ci: fix octocov timeout and step-time lookup
Storing the report hit the 30s default timeout: scanning this repo's
artifacts for the baseline consumed it first. Raise it to 5m.
The step-time lookup also matched the Windows job's 'Test' step and
waited for a job that was still running, so execution time was dropped
from the report. Rename the step to make it unique.
* ci: only store the coverage baseline from the default branch
* ci: report statement coverage instead of line coverage
octocov reports statement coverage for a single profile but switches to
line counting when it merges several itself, which made the number
disagree with 'go tool cover -func'. Merge the two job profiles into one
file first, so the reported number matches what developers see locally.
* ci: stop the download-link comment from clobbering the coverage report
Both comments are posted by github-actions[bot], and the download-link
job updated the first bot comment it found. On a new PR the coverage
comment is created first, so it would be overwritten. Match on the body
as well, and keep the coverage profiles out of the download list.
* test(plugins): build test plugins in Go instead of shelling out to make
The plugins suite built its .ndp test packages by running `make -C
plugins/testdata`, which needs make and zip on the PATH. That is the reason
the 26 WASM-dependent spec files are tagged //go:build !windows.
buildTestPlugins now does the same work in Go: the same mtime check make
performed, `GOOS=wasip1 GOARCH=wasm go build` per plugin, and archive/zip
for the package. TinyGo was already optional and unused in CI, so nothing is
lost there. The first plugin builds on its own so the shared wasip1 stdlib
and PDK objects land in the build cache before the rest fan out: on a cold
cache that is 2.2s against 3.4s for the sequential make and 7.3s for an
unrestrained fan-out.
Packaging moved into a writeNdp helper shared with createTestPackage, which
was already writing the same two-entry archive. Entries are written in a
fixed order, so the .ndp bytes are now reproducible; the loader hashes those
bytes, and `zip` also stored file mtimes, so the previous packages differed
on every rebuild.
The Makefile is unchanged and still works for building the plugins by hand.
Removing the !windows tags is a separate step, once CI is green here.
* test(plugins): run the WASM plugin specs on Windows
With the test plugins now built in Go, nothing in the suite needs a Unix
toolchain, so the //go:build !windows tags come off all 25 spec files. The
Windows CI job runs `go test ./...`, so it picks the suite up with no
workflow change.
plugins_suite_windows_test.go existed only to bootstrap the handful of specs
that compiled on Windows; plugins_suite_test.go now serves both.
* test(plugins): skip the planted-symlink spec where symlinks need privileges
os.Symlink needs an elevated token or Developer Mode on Windows, so the
unconditional Expect(...).To(Succeed()) would fail for contributors running
the suite on an ordinary Windows box. The elevated GitHub runner hides this.
The equivalent spec in sandbox_fs_internal_test.go already attempts the
symlink and skips on error; this does the same, keeping the pin live
everywhere it can run, including Windows CI.
* fix(ci): stop the Windows ndpgen test failing silently
The ndpgen suite builds its helper binary to %TEMP%\ndpgen-test, and Windows
will not exec a file without an executable extension, so the "supports
verbose mode" spec has been failing there. Nobody noticed because the
Test ndpgen step ran under pwsh, which carries on after a non-zero exit and
takes the step's status from the last command, so the job stayed green with
a FAIL line in its log.
Add the .exe suffix, and run the step under bash like the Linux job does, so
a failure in any of its three commands fails the job.
* fix(plugins): build public URLs on the caller's address instead of localhost
The artwork host service had no `*http.Request`, so it passed `nil` to
`publicurl.ImageURL`. With neither `ShareURL` nor `BaseURL` configured, that
produced `http://localhost/share/img/...`, which is useless to anything outside
the server. The Discord Rich Presence plugin explicitly drops localhost URLs, so
it fell back to the Navidrome logo instead of the real cover art.
`serverAddressMiddleware` already works out the client-facing scheme and host
from the `X-Forwarded-*` headers. It now also records them in the request
context, and `publicurl` takes a `context.Context` instead of an `*http.Request`
so any caller can reach them. Extism passes the caller's context through to host
functions, so plugins invoked during a request now get a reachable URL with no
configuration.
Switching the parameter also removes the need for a second, parallel entry
point: the package previously wanted only a scheme, a host, and a context, and
took a whole request to get them. `AbsoluteURL` no longer dereferences a
possibly-nil request on its parse-error path.
Plugin calls that start from `context.Background()` (scheduler and websocket
callbacks, the buffered scrobble drain) still fall back to localhost, since they
have no request to learn from. A debug log now points at `ShareURL` when that
happens.
* fix(publicurl): include the configured port in the localhost fallback
The last-resort fallback built `http://localhost/...`, which points at port 80
and so is unreachable for a server listening anywhere else — the default 4533
included. Use `conf.Server.Port` so a consumer on the same machine can actually
fetch the URL.
* fix(publicurl): use https in the localhost fallback when TLS is configured
The fallback hardcoded the http scheme, so a TLS-only server with no BaseURL
advertised a URL it does not answer on. Mirror the server's own switch, which
requires both a certificate and a key.
* refactor(publicurl): tidy the localhost fallback and its tests
Use gg.If for the fallback scheme so it reads as an expression, like the
BaseScheme branch above it, instead of assigning http and overwriting it.
Drop two tests the ctx refactor left redundant: one asserted PublicURL "works
without a request" but became a byte-identical copy of the ShareURL spec once
the *http.Request parameter went away, and the two port specs differed only in
the integer, where the non-default port is the stronger assertion.
* refactor(conf): add TLSEnabled and use it instead of repeating the predicate
Whether the server speaks HTTPS was decided inline in three unconnected
places. This PR added the third, in a URL-building package that has no
business inferring the transport config.
Move the rule to conf, next to the fields it derives from, and call it from
publicurl and the insights collector. server.Run keeps its own expression: it
takes the certificate and key as parameters, and its test passes values that
do not come from the config.
* fix(nativeapi): stop partial PUTs from clearing untouched columns
The REST layer parses the request body's top-level JSON keys and passes them
to Repository.Update as colsToUpdate. The radio and library repositories
discarded that list and issued a full-row UPDATE, so any field absent from
the body was written as its zero value.
For radio this wiped uploaded_image, deleting the station's cover on every
partial update (the Web UI is unaffected because its form submits the whole
record). For library it silently cleared remote_path and default_new_users.
Thread the column list through to Put in both repositories, and extract the
column-selection half of filterUpdateValues into selectUpdateColumns so
library, which hand-builds its update map, shares the same rule instead of
copying it.
Fixes#6057
* refactor(persistence): drop pluginRepository's dead rest.Persistable methods
Save and Update had no callers: PUT /api/plugin/{id} is served by the
hand-written updatePlugin handler over a typed request struct, and the
route only wires rest.GetAll and rest.Get. Both methods delegated to Put,
which upserts all twelve columns, so wiring rest.Put to this repository
would have reintroduced the partial-update clobbering fixed in the previous
commit. Removing them, along with the rest.Persistable assertion, makes
that a compile error instead of a silent data loss.
Put itself is unchanged and still backs plugin discovery.
* perf(persistence): avoid a full media_file scan when resolving playlist paths
FindByPaths built one OR-ed equality term per path. On the real media_file
schema SQLite abandons the path index at just two OR-ed terms and falls back to
SCAN media_file, re-testing every term against every row, so the cost grows with
(rows x terms).
Group the candidates by library and emit one IN list per library instead, which
plans as SEARCH media_file USING INDEX media_file_path_nocase. The NOCASE
collation is kept so ASCII case-insensitive matching still works.
This is the dominant cost of M3U playlist import, which resolves every track on
every scan. Measured with a 1000-track playlist against a migrated DB:
100k media_file rows: 397 -> 51,414 tracks/sec
500k media_file rows: 78.5 -> 47,174 tracks/sec
The rate no longer degrades as the table grows, which is the expected shape for
an index lookup. Reported in #6043, where an 8 hour scan of a 2M-song library
spent 7h52m in the playlist phase.
* docs(playlists): correct the stale reason for the M3U lookup chunk size
The expression-tree depth ceiling applied to the old OR-per-path query, which
capped a batch at roughly 500 terms. The IN form is bound by SQLite's 32766
variable limit instead, which the 400 candidates per chunk sit far below.
* fix(plugins): stop the cache janitor when a plugin cache is dropped
newCacheService started a ttlcache janitor goroutine that only stopped via the
explicit Close() path, so a cache service that was discarded without being closed
leaked its janitor for the process lifetime. It now registers the same
runtime.AddCleanup safety net that utils/cache.simpleCache already uses.
* fix(scanner): stop splitting multi-byte characters when truncating tags
sanitize() capped tag values with a byte slice, so a value whose limit falls in
the middle of a multi-byte character was stored as invalid UTF-8. defaultMaxTagLength
is 1024, which is not a multiple of 3, so any sufficiently long CJK title hit this.
Only trailing invalid bytes are trimmed, leaving bad bytes elsewhere in the value
untouched.
* fix(scanner): store MusicBrainz ids in their canonical form
uuid.Parse accepts a UUID wrapped in any two bytes, as well as braced and urn:
forms, but sanitize() returned the raw string. A tag like {<mbid>} or a quoted
value was therefore persisted with its wrapper into the mbz_* columns, where the
exact-match MBID search can never find it. The parsed value is now stored, which
also lowercases uppercase ids and adds the dashes to unhyphenated ones.
* fix(plugins): parse IPv6 hosts correctly in the websocket allowlist
isHostAllowed cut the host at the last colon, which mangles an IPv6 literal:
"[::1]:8080" became "[::1]" and "[::1]" became "[:". A plugin manifest could
therefore never allow an IPv6 host. It now uses net.SplitHostPort, falling back to
unwrapping the brackets when there is no port.
* fix(server): serve pprof profiles when a BaseURL is configured
net/http/pprof's Index resolves the profile name by trimming "/debug/pprof/" from
the raw request path, which never matches once MountRouter prepends the BasePath.
Requests for any profile without an explicit chi route fell through to the index
page, returning HTML with a 200 instead of the profile. The handler now strips the
BasePath first.
* test(scanner): run the goroutine leak check unconditionally
The scanner suite's goleak check only ran when the GOLEAK env var was set, so it
never ran in CI and could not catch a regression. It passes with the existing
ignore list, verified over repeated runs, so the gate is removed.
* fix(server): close the background image body on a non-200 response
serveImage returned early on an unexpected status code without closing the response
body, pinning the connection until the 5s client timeout. The nolint:bodyclose
above the request suppressed the linter that would have caught it, and its
justification only holds on the success path, where the body is handed to the
CachedStream wrapper.
* test(scanner): repair BenchmarkScan so it can actually run
The benchmark failed three ways before reaching its first iteration: it reused a
shared temp DB and tried to repoint the default library, it never loaded the config
defaults so the scanner got a concurrency of 0, and it lacked the notify ignore that
the suite already carries. tests.Init now takes a testing.TB so a benchmark can load
the test config the same way the suites do.
* refactor(artwork): drop the unused sourceFunc Stringer
sourceFunc.String derived a label from the closure's symbol name via reflection, but
nothing called it: the trace output builds its candidate labels from explicit strings.
Whole-program analysis confirms it is unreachable, and dropping it removes a
reflection-based dependency on compiler closure-naming details.
* refactor(plugins): reuse extractHostname in the websocket allowlist
The IPv6 host parsing added for isHostAllowed duplicated extractHostname, which
already lives in the same package and backs the HTTP client's identical allowlist
check. Two copies of a security-relevant parser can drift, so the websocket service
now calls the existing helper. The port-stripping specs move into the URL Validation
block that already covered them.
* perf(scanner): bound the tag truncation trim to a partial rune
The trim loop dropped every trailing byte that failed to decode, so a value ending
in a long run of invalid bytes was walked one byte at a time: a 1 MiB lyrics tag
measured 2.58ms against 45ns for a normal cut. A partial rune is at most 3 trailing
bytes, so the loop is capped there, which also stops it consuming a pre-existing
invalid run.
* test: tighten the tests added with the Go 1.27 bugfixes
Drop the testItem stub in favour of the package's own cacheKey, register the pprof
test profile once at package scope, and replace the hand-rolled goroutine settle
loop with Eventually. Also corrects a comment that credited a TestMain the scanner
suite does not have.
* test(scanner): ignore notify's nonrecursive-tree goroutines on Linux
The goroutine leak check only ignored the recursive tree (macOS/FSEvents).
Linux CI uses inotify, whose nonrecursive tree leaks dispatch and internal
goroutines after Stop(), failing the check.
* fix(scanner): avoid a truncation panic when MaxLength is 1 or 2
A value of only UTF-8 continuation bytes drained the partial-rune loop to
empty, then sliced value[:-1] and panicked. Break when DecodeLastRune returns
size 0 (empty string) by testing size != 1 instead of size > 1.
* fix: address Codex review on the pprof base path and scan benchmark
- profilerHandler: treat a root BasePath ("/") as no prefix, so http.StripPrefix
keeps the leading slash chi needs; without this the profiler 404s when BaseURL
is "/". Cover the root case in the test.
- BenchmarkScan: make it run regardless of test/benchmark ordering. Add
singleton.DeleteInstance so a fresh DB is opened after TestScanner closes the
shared one, guard driver registration with sync.Once so the rebuild does not
re-Register, and ignore the Ginkgo interrupt-handler and Linux notify
goroutines the preceding suite leaves behind.
* fix: address Codex round 2 on BasePath trailing slash and benchmark DB cleanup
- profilerHandler: trim all trailing slashes (TrimRight), not just a bare "/", so
a BaseURL like "/music/" strips correctly instead of 404ing. Cover it in the test.
- BenchmarkScan: keep and defer db.Init's closer so the DB is closed before
b.TempDir cleanup, which otherwise cannot delete the open SQLite/WAL files on Windows.
* test(plugins): make the suite safe to run in parallel processes
Two things broke when the suite ran across several Ginkgo processes.
buildTestPlugins ran in every process, so N copies of make raced in the same
directory. The packaging rule made that worse by staging every plugin through
one shared plugin.wasm, so concurrent targets clobbered each other and left
orphaned temp files behind. That also ruled out make -j.
Stage each package under its own per-target directory, and move the build into
SynchronizedBeforeSuite so process 1 does it once while the others wait.
* ci: run the plugins suite in parallel processes
With the compilation cache warm the suite is bound by spec execution, which
splits cleanly across processes. Run it as its own step with the ginkgo CLI,
already declared as a tool in go.mod, and drop the package from the main go
test invocation so it is not run twice.
Locally, with -race: 69s to 19s warm, and 256s to 96s cold.
* ci: give the plugins suite its own job so it runs concurrently
Running it as a second step in the go job serialised it against the other 90
packages, which cancelled out the parallel win: the job went from 5m41s to
only 5m29s even though the suite itself dropped from ~175s to 82s.
Move it to its own job so the two run at the same time. The WASM compilation
cache moves with it, since the go job no longer runs the suite.
* ci: cache the plugins test suite WASM compilation across runs
The 'Test Go code' job was dominated by a single package: 'plugins' took
541s of the 699s test step. The suite builds 25 test plugins as full-Go
wasip1 modules of ~4.5MB each, and wazero must compile every one to machine
code. Under -race that compiler work is instrumented, so each module costs
around 11 seconds.
The suite already shared a wazero compilation cache, but three things kept it
from paying off. It lived in a fresh temp dir, so nothing survived the run.
The default plugins.cachesize of 200MB was smaller than the 334MB the cache
actually needs, so the purge evicted entries mid-run. And the wasm binaries
embedded VCS stamps, so every commit produced different bytes and missed the
content-addressed cache anyway.
Point CacheFolder at plugins/testdata/.wazero-cache, raise the test cache
limit past what the suite needs, build the test plugins with -buildvcs=false,
and restore the directory in CI. Locally the package goes from 256s to 74s
with the cache warm and the wasm rebuilt from scratch.
* ci: key the WASM cache on what actually changes the modules
The test plugins are separate Go modules with their own go.mod and go.sum;
they reach the PDK through a replace directive and never read the root
module. So the root go.sum has no bearing on the wasm bytes, and the wazero
version it pins is already namespaced by wazero itself, which stores entries
under wazero-<version>-<goarch>-<goos>. Keying on it only rotated the cache
on every unrelated dependency bump.
Drop it, and add the go.mod files that were missing: the test plugins' own
and the PDK's. The root go.mod stays, since it selects the toolchain that
builds the modules.
* ci: key the WASM cache on the toolchain version, not go.mod
Only the Go toolchain in the root go.mod affects the built wasm, but the file
also changes on every direct dependency bump, which would rotate the cache for
no reason. Take setup-go's go-version output instead: it is the version that
actually built the modules.
Moves both the xx-build toolchain stage and the final runtime image from
Alpine 3.20 (past end of active support) to 3.22.
3.22 is the last release where ffmpeg is still 6.1.x — it jumps to 8.0 in
3.23 — so transcoding behavior is unchanged by this bump.
Alpine 3.21 repackaged mesa, and from that release on `mpv` requires
so:libEGL.so.1 and so:libgbm.so.1. Those pull mesa -> llvm20-libs (156MB)
plus the gallium drivers (62MB), which took the image from 231MB/62MB
compressed to 578MB/147MB. mesa-egl is the only provider of libEGL.so.1,
and newer Alpine releases do not improve on this.
Navidrome runs mpv headless for jukebox audio and never enters a video
path, so this replaces libEGL/libgbm with generated no-op stubs and drops
the mesa/LLVM stack. The stub symbol list is read from real mesa at build
time and cross-compiled with the existing xx toolchain, so it adapts per
architecture rather than being hardcoded.
Verified with logging stubs across mp3/flac/ogg/opus/m4a/wav driving the
default MPVCmdTemplate (pause, volume, time-pos seek, quit): zero calls
into the stubbed libraries. The final stage now also runs mpv once at
build time, so a broken stub fails the build instead of shipping.
Image size: 323MB -> 325MB (84MB -> 86MB compressed).
