* ci: don't skip release jobs after the DB migration check on tag pushes
The validate-migrations job added in #5750 was gated at job level with
if: github.event_name == 'pull_request', so it concluded "skipped" on tag
pushes. GitHub Actions propagates a skipped job transitively through the
needs chain (actions/runner#491): even though Build overrode its own gate
with !cancelled() && !failure() and ran successfully, every job downstream
of Build (msi, release, push-manifest-*, PKG uploads) still failed the
implicit success() check and was skipped, which broke the v0.63.2 release.
Move the pull_request gate from the job to its steps. On non-PR events all
steps are skipped and the job concludes "success", so downstream jobs run
normally. This also restores the default success() gate on Build, keeping
the fail-fast behavior on PRs with a bad migration.
* ci: trim workflow comment
Condense the explanation of the step-level pull_request gate on the
validate-migrations job to the essential rationale.
* test(scanner): fix flaky Windows search_normalized rescan test
The 'repopulates a stale search_normalized on a full rescan' spec runs
two full scans back-to-back. Whether the second scan refreshes the
unchanged artist depends on folderEntry.isOutdated(), which compares
folder.updated_at (written during the first scan) against the second
scan's library.last_scan_started_at using a strict time.Before(). Both
are time.Now() values captured milliseconds apart.
On Linux's fine-grained clock they are always distinct, so the test
passes. On Windows the coarse wall-clock granularity frequently makes
the two timestamps land in the same tick and compare equal, so
Before() returns false, the folder is treated as up-to-date and
skipped, the artist is never re-persisted, and search_normalized stays
empty -- failing the assertion intermittently across unrelated PRs.
Backdate the folder's updated_at an hour before the second scan so the
comparison is unambiguous on every platform. This is a test-only
timing artifact (real rescans never run milliseconds apart on an
unchanged library), so no production code changes are needed.
* fix(smartplaylist): reject NSP mixing top-level 'any' and 'all'
A smart playlist (.nsp) that specified both a top-level "any" and a
top-level "all" group was imported by silently keeping only "any" and
discarding "all", regardless of key order. The Criteria model holds a
single top-level Expression, so it cannot represent both groups, and the
parser picked "any" without reporting the dropped rules.
Make Criteria.UnmarshalJSON return an error when both keys are present at
the top level, so the scanner fails loudly (logging the playlist as
invalid) instead of silently losing rules. Users should nest one group
inside the other, as shown in the documented examples.
Fixes#5757
* fix(smartplaylist): reject top-level any+all by key presence
Address code review feedback: the previous guard checked decoded slice
lengths, so it only rejected the mixed top-level any/all form when both
groups were non-empty. An input like {"any":[],"all":[...]} (or a
null group) slipped past and silently used just one group — the same
class of silent drop this change set out to prevent.
Decode the two keys as json.RawMessage and detect presence by key rather
than length, so any file that provides both top-level keys is rejected
regardless of whether one group is empty or null.
* refactor(smartplaylist): detect top-level any+all via presence type
Replace the json.RawMessage + manual double-unmarshal in
Criteria.UnmarshalJSON with a small optionalConjunction wrapper whose
UnmarshalJSON records that its key was present. Because encoding/json
invokes UnmarshalJSON even for a JSON null, this keeps the exact
behavior (a present-but-empty or null group still counts, so mixing
both top-level keys is rejected) while decoding in a single pass — no
raw-message capture, no re-decode, no shadow variables.
No behavior change; existing tests pass unchanged.
* fix(plugins): surface host service failures when loading plugins
When a host service failed to initialize during plugin load (e.g. the
taskqueue database could not be created because the data folder is not
writable), the error was logged and swallowed, and its host functions were
silently omitted. Instantiation then failed with a misleading error such as
'"task_createqueue" is not exported in module "extism:host/user"', which
reads as a plugin/host API mismatch and gets wrongly blamed on plugin
authors (see kgarner7/navidrome-listenbrainz-daily-playlist#26).
Host service factories now return an error, and loadPluginWithConfig fails
fast with the actual cause (e.g. 'creating Task service: creating plugin
data directory: ...'), which is also stored in the plugin's last_error.
Closers accumulated before a load failure are now closed (via a deferred
guard covering all failure paths), so partially-created services no longer
leak goroutines or database handles.
Also fix a panic in 'navidrome plugin enable': CLI commands use the plugin
Manager without calling Start, so manager.ctx was nil and
newTaskQueueService panicked in context.WithCancel. Long-lived host
services (taskqueue, kvstore, websocket) now receive their lifecycle
context explicitly in the constructor, sourced from serviceContext.baseCtx(),
which falls back to context.Background() for the unstarted-manager case.
* docs(plugins): correct websocket readLoop lifecycle comment
The comment claimed the read loop's context is always cancelled during
application shutdown, which is not true when the manager was never started
(one-shot CLI runs, where baseCtx falls back to context.Background()).
Clarify that connection closure via Close() on plugin unload is what ends
the read loop, with context cancellation as a server-shutdown backstop.
Addresses review feedback on #5756.
* test(plugins): exclude plugin-loading specs from Windows builds
The new loadPluginWithConfig specs reference test suite helpers
(testdataDir, noopMetricsRecorder) defined in plugins_suite_test.go, which
is excluded on Windows, breaking test compilation there. Move the specs to
their own file with the same build constraint, keeping the pure-function
specs in manager_loader_test.go running on Windows as before.
The 'repopulates a stale search_normalized on a full rescan' spec runs
two full scans back-to-back. Whether the second scan refreshes the
unchanged artist depends on folderEntry.isOutdated(), which compares
folder.updated_at (written during the first scan) against the second
scan's library.last_scan_started_at using a strict time.Before(). Both
are time.Now() values captured milliseconds apart.
On Linux's fine-grained clock they are always distinct, so the test
passes. On Windows the coarse wall-clock granularity frequently makes
the two timestamps land in the same tick and compare equal, so
Before() returns false, the folder is treated as up-to-date and
skipped, the artist is never re-persisted, and search_normalized stays
empty -- failing the assertion intermittently across unrelated PRs.
Backdate the folder's updated_at an hour before the second scan so the
comparison is unambiguous on every platform. This is a test-only
timing artifact (real rescans never run milliseconds apart on an
unchanged library), so no production code changes are needed.
* fix(scanner): resolve file symlinks with the production local storage FS
The symlink classification added for GHSA-r5qr-m328-qcf4 relied on
fs.ReadLink, but the local storage FS wraps os.DirFS behind the fs.FS
interface, hiding its ReadLinkFS implementation. Every resolution failed
at the first hop, so the scanner silently skipped ALL file symlinks,
regardless of target or the FollowSymlinks setting. Libraries made of
symlinks (e.g. shared-pool setups) lost all their tracks after
upgrading to 0.63.
The local storage now exposes full OS-level resolution (EvalSymlinks)
through a new optional storage.SymlinkResolverFS interface, which the
scanner prefers over the fs.ReadLink hop loop. This also classifies a
chain by its FINAL target even when it passes through an audio-named
intermediate outside the library, closing a bypass the hop loop had.
Regular (non-symlink) entries keep the same early-return path, so scan
performance is unaffected for normal libraries.
Fixes#5752
* fix(test): keep watcher specs off the real local storage
The watcher specs spawn watchLibrary goroutines that are not joined on
spec teardown. Now that the scanner test binary registers the file://
storage, those leaked goroutines reached newLocalStorage, which reads
conf.Server on construction, racing with the configtest cleanup that
restores the config snapshot (caught by CI's race detector). Point the
mock libraries at a fake storage scheme, which never touches the config
and does not support watching, so the goroutine exits immediately.
* fix(storage): reject invalid fs paths in ResolveSymlink
Defense-in-depth for the SymlinkResolverFS contract: names must be valid
fs.FS paths. A lexical ".." in the name would otherwise escape the
library root via filepath.Join cleaning. No current caller can produce
such a name (they come from ReadDir walks), but the guard enforces the
documented contract at the boundary.
* ci: add DB migration ordering/naming validation script
* ci: run DB migration validation on pull requests
* ci: don't reject non-migration .go files
* ci: fetch latest master before validating migration order
* ci: annotate the offending migration file on validation failure
* ci: gate Build on the migration check so a bad migration fails fast
* ci: reject migrations placed in a subdirectory of db/migrations
* ci: reword fetch-step comment to not hard-code the base branch name
* fix(build): force nodynamic webp tag on 32-bit standalone binaries
gen2brain/webp's native libwebp backend links ebitengine/purego, whose
reverse callbacks are unsupported on 32-bit ARM and x86. purego registers
its callback in package init(), so the binary crashes at startup (SIGSEGV
or SIGILL) before any Navidrome code runs.
The nodynamic build tag from #5606 forces the safe WASM path, but it was
only applied to the Docker-image build stage. The standalone build stage,
which produces the downloads-page tarballs and the deb/rpm packages, still
linked purego, so the armv7/v6/v5 and 386 downloads crashed on launch
(#5738, #5735).
Move the tag decision into release/build-tags.sh, shared by both build
stages so they can no longer drift, and add release/verify-binary.sh as a
build-time guard that fails if a 32-bit binary links purego.
* fix(build): harden webp build-tag scripts per review
- verify-binary.sh: fail loudly when the target binary is missing (e.g. an
unmatched glob) instead of letting `go version -m` fail inside a pipeline
and silently pass, which would bypass the guard.
- build-tags.sh / verify-binary.sh: fall back to `go env GOARCH` when xx-info
is unavailable, so the scripts stay correct outside the xx build image.
(Not `uname -m`, which reports the build host, not the cross target.)
- Dockerfile: use `set -e` in the standalone build block and drop the
redundant `|| exit 1` suffixes; keep the debug GOENV dump non-fatal.
* chore(build): quote -tags argument in both build stages
Defensive quoting per review; the value comes from release/build-tags.sh and
contains no whitespace today, but quoting prevents word-splitting if it ever does.
* fix(build): link 32-bit arm binaries with LLD to fix startup crash
The standalone armv7/v6/v5 binaries of 0.63.0 crash before main() with
SIGSEGV/SIGILL (issues #5738, #5735). Root cause, established from a core
dump of the crashing binary under qemu: GNU ld emits corrupt
R_ARM_IRELATIVE addends for libatomic's ifunc resolvers (wrong address and
missing Thumb bit) once .text outgrows the 16MB Thumb branch range. glibc's
static-init ifunc resolution then does `blx` into ARM-mode garbage and the
process dies before any log output. v0.62.0 was unaffected only because its
.text was still under 16MB (15.1MB); v0.63.0 crossed the line (17.5MB), so
every 0.63.0 32-bit arm build crashes regardless of Go or dependency
versions.
Link 32-bit arm with LLD (already installed in the build stage), which
emits correct IRELATIVE addends. Verified under qemu: the armv7 artifact
built by the unchanged pipeline now boots to "Navidrome server is ready"
with SQLite migrations working, where the previous binary segfaulted at
startup.
Also add a CI smoke test that runs each cross-compiled linux binary under
binfmt/qemu right after building it, so any future
crashes-at-startup-on-some-arch regression fails the pipeline instead of
shipping in a release.
kardianos/service v1.3.0 (shipped in Navidrome 0.63.0) replaced Go's
text/template with a small custom engine that uses bare keys ({{Description}})
instead of dotted fields ({{.Description}}). Our custom SystemdScript was
still written in text/template syntax, so 'navidrome service install' failed
with 'FATA service: unknown template key ".Description"', breaking the .deb
postinst and leaving an empty/masked systemd unit.
Rewrite the template in the new engine's syntax and add a test that validates
every key and pipeline function used in the template against the set the
library provides to systemd templates, so future engine/key drift is caught
at test time. Verified end-to-end on Linux: the previous binary reproduces
the reported fatal error and the fixed one generates a complete unit file.
Fixes#5742
Plugin scrobblers read the username from the request context via
getUsernameFromContext, but buffered scrobbles are persisted to the DB
scrobble buffer (which stores only the userId) and later drained by a
background worker running on context.Background(). That context carries
no authenticated user, so ScrobbleRequest.Username was always empty for
WASM scrobbler plugins. NowPlaying is unaffected because it is dispatched
synchronously on context.WithoutCancel(requestCtx), which retains the user.
Restore the user in the drain path: processUserQueue now looks up the
user by the buffered userId and injects it into the context via
request.WithUser before dispatching, mirroring the pattern already used
in play_tracker. Builtin scrobblers are unaffected as they resolve the
account from the userId argument rather than the context.
* fix(plugins): discard buffered scrobbles when a plugin is removed
Scrobbles are buffered in the DB per service, keyed by the plugin name.
When a plugin was removed (deleted from the plugins folder and detected by
the sync), its pending buffer entries were left behind forever: the drain
goroutine is stopped on the next scrobbler refresh, so the rows were never
retried nor discarded. Worse, if a plugin with the same name was installed
later, the stale entries would be drained into it - potentially a completely
unrelated plugin that just reuses the name.
Add a Discard(service) method to ScrobbleBufferRepository and call it from
removePluginFromDB, right after the plugin record is deleted. Disabling a
plugin intentionally keeps its buffered scrobbles, consistent with the
buffer's purpose of surviving temporary outages, and transient unload/reload
cycles during config updates are unaffected since they never delete the
plugin record.
* fix(plugins): don't wipe builtin scrobbler queues on plugin removal
Buffer entries are keyed by service name only, and removePluginFromDB runs
for any removed plugin file, so removing a plugin named e.g. lastfm.ndp -
regardless of its capability - would discard the builtin Last.fm retry
queue. Skip the discard when the plugin name is owned by a registered
builtin scrobbler, exposed via a new scrobbler.IsBuiltinScrobbler helper.
Reported by Codex review on the PR.
Also drop the testBroker usage from the new removePluginFromDB spec: it is
defined in manager_test.go which is excluded on Windows, breaking the
Windows test build. sendPluginRefreshEvent is nil-safe, so no broker is
needed.
* fix(build): derive version from reachable git tag
* fix(build): fall back gracefully when no tag is reachable
git describe --tags --abbrev=0 exits with an error when the checkout has
no reachable tag (e.g. tagless forks or pre-first-tag commits). In the
Makefile this printed a fatal message and produced a bare -SNAPSHOT
version, and in the CI git-version step the non-zero exit would abort
the job under bash -e before the empty-tag guard could run. Silence
stderr and fall back to v0.0.0 in the Makefile, and to an empty string
in the workflow so the existing guard keeps skipping the output as it
did before.
Flip the EnableSharing default from false to true so new installations have the sharing feature available out of the box. Users can still disable it via the EnableSharing config option.
The native API only registers the /share route when sharing is enabled, so the nativeapi tests that build the router without wiring a share service now explicitly disable sharing in their setup to avoid registering a route backed by a nil service.
* perf(db): add composite indexes for song list album/artist sorts
The media_file sort mappings for album, artist and albumArtist expand to
multi-column ORDER BY clauses that no existing index could satisfy, so SQLite
fell back to a full table scan plus a temp B-tree sort of every row (including
the large lyrics/tags/full_text columns) even for a single 15-item page. On a
96K-track library this made /api/song?_sort=album take 3.6s on a cold cache.
Add composite indexes matching the three sort mappings, allowing the query to
walk the index and stop at the page size, in both directions. Drop the now
redundant single-column order_album_name/order_artist_name indexes (strict
prefixes of the new composites) and three indexes with no query path:
birth_time is only read in Go code, and artist/album_artist text column
lookups go through the media_file_artists table instead.
* fix(ui): make composer and track number columns non-sortable in song list
Clicking the Composer header was a silent no-op: composer is not a media_file
column, so the native API's sanitizeSort drops the sort and returns rows in
table order. Track number sorting across the whole library is not meaningful
and cannot use an index (the existing index leads with disc_number). Mark both
columns sortable={false}, like quality and mood.
* test(persistence): add sort index coverage test for large tables
Guard against sort options silently losing index support: every sort mapping
on media_file, album and artist is now verified with EXPLAIN QUERY PLAN to be
satisfiable by an index (both directions), so adding a mapping or dropping an
index that reintroduces a full-table temp B-tree sort fails the test. Sorts
that genuinely cannot use an index (random, annotation-join columns, JSON
expressions) must be declared in an exceptions list with the reason, keeping
the trade-off visible in review.
To make the sort mappings the complete declared sort surface, add identity
mappings for the media_file columns the UI sorts by without a mapping (year,
genre, duration, channels, bpm, path, comment, play_count, play_date, rating).
These are behaviorally no-ops: the same ORDER BY was previously produced by
the field whitelist fallback.
* perf(db): drop PreferSortTags expression indexes from media_file
The media_file sort_title/sort_artist_name/sort_album_name expression indexes
are only usable when PreferSortTags is enabled - a config reported by ~0.1% of
installations (insights, week of 2026-06-22) - yet every install pays their
storage (~8.6MB on a 96K-track library) and scanner write overhead. Drop them:
PreferSortTags installs fall back to a full sort for title/artist/album orders,
everyone else gets smaller DBs and cheaper writes. The order_album_name and
order_artist_name collation checks remain valid, now satisfied by the composite
sort indexes.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(search): boost exact token matches over prefix matches
buildFTS5Query now emits (word OR word*) instead of word* for plain tokens. The match set is unchanged (exact is a subset of prefix), but bm25 gives the rare exact token a high-IDF contribution, so rows containing the literal query word rank above prefix-only matches. The degraded-query check keeps evaluating the plain prefix form, preserving the LIKE fallback for queries like "1+" and "C++".
* feat(search): weight artist search_normalized equal to name in bm25
For the artist table, search_normalized holds only the artist's name in alternate spelling (transliterated/punctuation-stripped), so a hit there is as meaningful as a name hit. Combined with exact-token boosting, artists like MØ now rank in the top results for the query "MO" instead of dead last. media_file and album keep weight 1.0 because their search_normalized mixes title, album, and artist variants.
* test(persistence): add exact-match ranking regression test
Seeds MØ, Morrissey, and Modest Mouse and asserts MØ ranks first for the queries "MO" and "MØ": the exact transliterated hit in search_normalized must outrank name-prefix matches. The corpus deliberately has no competing exact-word names, since exact-vs-exact ordering depends on corpus statistics rather than the guaranteed exact-over-prefix property. Rows are inserted per-test (with their library_artist associations) and cleaned up to avoid disturbing the shared seed fixtures and their count assertions.
* docs(search): document exact-token OR emission in buildFTS5Query
* test(persistence): harden exact-match ranking test fixtures
Register the corpus cleanup before the insert loop so a mid-loop assertion failure cannot leak fts-rank-% rows into the shared integration DB, and reuse the existing createArtistWithLibrary helper instead of hand-rolling Put+AddArtist (which also replaces the ad-hoc context.TODO with the helper's GinkgoT().Context).
* fix(search): flag multi-word degraded queries for the LIKE fallback
The degradation probe was joined with explicit " AND " like the real query, so ftsQueryDegraded counted the literal AND as a long token and never flagged queries where every term degrades to a short token (e.g. "1+ 2+"). This predates this branch (the old code passed the same AND-joined string), but the probe now exists separately, so join it with spaces — it only feeds ftsQueryDegraded, which needs no explicit operators.
* fix(scanner): update artist search_normalized when rescanning
The FTS5 migration back-fills artist.search_normalized with a SQL
punctuation-strip approximation, relying on the next scan to compute the
precise value in Go (normalizeForFTS transliterates atomic letters like
Ø/æ/ß that FTS5's remove_diacritics cannot fold). But the scanner
persisted artists with an explicit column list that omitted
search_normalized, so not even a full scan ever repaired it: an artist
migrated from a pre-FTS database (e.g. "GØGGS") stayed unfindable by
any ASCII search, while their albums and songs, which are saved with all
columns, were fixed by a full scan. Add search_normalized to the column
list so a full scan re-indexes the artist via the artist_fts trigger.
* refactor(persistence): move normalizeForFTS to utils/str
Export it as str.NormalizeForFTS so the upcoming migration can reuse the
exact index-time normalization. Migrations cannot import the persistence
package (persistence -> db -> db/migrations would be an import cycle).
* fix(persistence): backfill artist search_normalized via migration
Recompute artist.search_normalized with the precise Go normalization for
databases migrated from pre-FTS5 versions, where the SQL back-fill could
not transliterate atomic letters (Ø/æ/ß) and the scanner never rewrote
the column. Only changed rows are updated, so the artist_fts update
trigger re-indexes exactly the affected artists, making artists like
GØGGS or MØ findable again without requiring a full scan.
* refactor(persistence): share FTS punctuation-strip regex via utils/str
Index-time normalization (NormalizeForFTS) and query-time processing
(buildFTS5Query/ftsQueryDegraded) must produce matching tokens, so keep
the punctuation-strip pattern in a single exported symbol instead of two
identical private copies that could drift. Also document that derived
columns computed in dbArtist.PostMapArgs must be listed in the scanner's
artist Put, which is how search_normalized went stale in the first place.
* chore(migrations): announce artist search backfill in the log
Match the FTS5 migration's notice() pattern so startup isn't silent
while the backfill runs on large libraries.
* docs: tighten comments added in this branch
* docs: describe FTSPunctStrip by what it matches, not one replacement
* fix(scanner): stop logging expected lyrics sniff misses as warnings
During a scan, embedded lyrics are parsed with an empty suffix, which puts
ParseLyrics into content-sniffing mode: it tries the TTML, SRT and Lyricsfile
YAML parsers in turn before falling back to plain text. Every plain-text or LRC
lyric therefore fails the structured probes on its way to the fallback, and each
failure was logged at warning level with no indication of which file triggered
it, flooding the scan log with benign "Error parsing lyrics, falling back to
plain text" messages.
A probe rejecting content it does not own during sniffing is expected control
flow, so it is now logged at trace instead. A parse failure under an explicitly
requested suffix (e.g. a malformed .yaml/.srt/.ttml sidecar) still warns, since
the user declared that format. ParseLyrics gains ctx and path parameters so any
warning names the offending file and carries request context where available;
all call sites are updated accordingly.
Also fixes a test-isolation bug in the new logging spec: the BeforeEach swapped
the process-global default logger via SetDefaultLogger but only restored the log
level on cleanup, leaking the null logger and its hook into later specs in the
shared model suite.
* test: use spec-scoped contexts instead of context.Background in lyrics tests
Replace context.Background() with GinkgoT().Context() (and b.Context() in the
parse benchmarks) across the lyrics-related tests, so contexts are cancelled
when each spec ends. The embeddedLyrics fixture in core/lyrics is now a
hand-written literal like its sibling fixtures, removing the construction-time
ParseLyrics call that could not use a spec-scoped context.
* refactor(model): attach lyrics parse log attribution via context
Narrow ParseLyrics back to (ctx, suffix, lang, contents), dropping the path
parameter added by the previous commit. Attribution now uses the codebase's
existing idiom: callers that know the source attach it with log.NewContext
(e.g. "file" for the media file or sidecar), and the plugin adapter tags both
the plugin name and the track, fixing probe-miss logs that misattributed
plugin-returned content to the file's own tags. This removes three adjacent
string parameters that were easy to swap silently, and the "" placeholder most
call sites had to pass.
Also hardens the logging spec from the previous commit: the null test logger is
now swapped in before raising the level (SetLevel forces the current default
logger to trace, so the old order left the null logger at info and trace
entries never reached the hook), the sniff test now asserts probe misses are
observable at trace with file attribution instead of only asserting the absence
of warnings, and cleanup restores the actual previous logger — via a new return
value on log.SetDefaultLogger — instead of a bare logrus.New() that would
discard hooks configured on the process-wide logger.
* refactor(lyrics): hoist attributed log contexts out of loops
Address review feedback on #5702: build the log-attributed context once per
operation instead of per iteration, and reuse it on the surrounding log calls
so the error/trace lines around ParseLyrics carry the same attribution fields.
In fromExternalFile the sidecar path now rides the context for all log lines
in the function, replacing the repeated explicit "path" field.
* style(model): pass lyrics parse errors as final log arguments
Per the project logging convention, errors go as the last argument (the log
package normalizes them via its error case) instead of a keyed "error" pair,
which stores the raw error value and bypasses that handling. Flagged by review
on #5702; the keyed form was inherited from the original warning line.
* refactor(scanner): make tag value splitting position-based
Replaces the ZWSP substitution trick with index-based cutting, in
preparation for artist split exceptions, which need match positions.
* feat(scanner): protect whitelisted names in tag value splitting
Separator matches inside word-bounded exception matches no longer split.
Matching is case-insensitive and longest-first; boundaries are rune-aware.
* feat(scanner): add Scanner.ArtistSplitExceptions config option
* feat(scanner): honor artist split exceptions for participant tags
Applies Scanner.ArtistSplitExceptions to artist, albumartist and role tag
splitting. Generic tags (genre, mood, ...) are unaffected.
* fix(scanner): apply split exceptions when per-tag Split overrides participant tags
Per-tag Tags.<name>.Split makes the generic ingestion path split the tag
before participant mapping runs, bypassing the whitelist. Attach the
exceptions to participant tag mappings (including sort variants) in clean().
* feat(scanner): split performer names and honor split exceptions
Performer pair values were never split; multiple names in one PERFORMER
value stayed a single artist. Split them with the roles separators, using
the same whitelist protection as other participant tags.
* test(scanner): lock MBID ordering for split performer values
* refactor(scanner): consolidate split-exception wiring and drop hot-path lock
ArtistSplitExceptionsRx is called per tag mapping per scanned file across
concurrent goroutines; replace the mutex+joined-key cache with an atomic
pointer compared via slices.Equal. Route all participant call sites through
WithParticipantExceptions and a shared splitParticipantValues helper.
* refactor(scanner): unexport artistSplitExceptionsRx
All external callers go through WithParticipantExceptions, so the accessor
does not need to be part of the model package API.
* fix(ui): make self-service profile edits report their outcome
When a non-admin user saved their own profile (e.g. changing their
password via EnableUserEditing), the data provider followed the user
update with a call to the admin-only PUT /api/user/{id}/library
endpoint, which always failed with 403. The save error handler then
crashed reading error.body.errors on the plain-text response, so the
user got no notification at all - while the profile change had in fact
already been applied. This made password changes look like they were
silently ignored, and follow-up attempts failed with 'password does not
match' since the current password had already changed. Present since
the multi-library support introduced in v0.58.0 (#4181).
Only call the user-library association endpoint when the logged-in user
is an admin (the server manages assignments for self-edits), and make
the save error handler tolerate error bodies without field errors,
notifying a generic error instead of crashing.
* fix(ui): tolerate nullish rejection values in user save handler
Address review feedback: use optional chaining on the error itself in
the UserEdit save handler, so a nullish rejection value also results in
the generic error notification instead of a TypeError.
The 'sees all libraries' specs hard-coded the user's libraries as {1, 2}
and assumed the shared test DB held exactly two libraries. applyLibraryFilter
only skips the filter when granted count == total library count, so when
another spec left an extra library behind (Ginkgo randomizes spec order),
the count was 3, the filter was applied, and the SQL assertion failed. This
surfaced on the Windows CI run but reproduces on any platform.
Grant the user exactly the library IDs that actually exist in the DB at
runtime instead of hard-coding them.
* perf(db): skip library filter when a non-admin sees all libraries
applyLibraryFilter already short-circuits for admins and headless
contexts, but a non-admin who has been granted every library still paid
for the correlated user_library subquery, which filters out nothing yet
is the slow non-admin list/count path.
Reuse the visibility check search3 already had: skip the subquery when
the user's granted libraries cover the whole library table. The two
helpers (userSeesAllLibraries/visibleLibraryIDs) are promoted from
artist_repository to the base sqlRepository so all ~13 call sites
benefit and search3 shares the single implementation.
The skip is strictly gated on granted count >= total library count,
never on an empty/unknown library set, so access control is unchanged
for restricted users.
* refactor(db): make all-libraries skip fail closed; test cleanups
- userSeesAllLibraries: require len(visible) == total (not >=) so the
filter skip can never over-grant if the visible set is ever inflated.
- tests: assert ToSql() returns no error; restore r.db in AfterEach to
avoid leaking mutated state to other specs.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(plugins): add public Track and Artist DTOs for host services
* feat(plugins): add Matcher host-service interface and MatchSong DTO
* feat(plugins): generate Matcher host wrappers, PDK clients, and matcher permission
* feat(plugins): implement Matcher host service and MediaFile-to-Track converter
Also fixes an ndpgen bug where ParseDirectory parsed each host-service file
in isolation, so a service method referencing a struct defined in another
file of the same package (host.Track in track.go) could not be resolved.
ParseDirectory now collects package-wide structs in a first pass, mirroring
ParseCapabilities; PDK clients regenerated cleanly via make gen.
* feat(plugins): register Matcher host service in the manager
* test(plugins): add Matcher host service integration test plugin
* refactor(plugins): simplify matcher converter and parser file collection
- toTrack: use gg.V for nil-able field derefs and slice.Map for genres/
participants, removing the repeated nil-guard blocks and inner loop
- manager_loader: drop the redundant ds==nil guard (loadEnabledPlugins
already gates a nil DataStore), matching the other service entries
- ndpgen parser: extract collectGoFiles, shared by ParseDirectory and
ParseCapabilities instead of duplicating the file-filter loop
* fix(plugins): keep nullable Track numerics as pointers
ReplayGain values, BitDepth, and BPM are nullable in model.MediaFile, and 0
is a valid measured ReplayGain value. Flattening them to value types with
omitempty made a real 0 indistinguishable from absent. Model them as *float64
/*int32 so plugins can tell 'no data' from a measured 0. Regenerated PDK
clients; converter passes the model pointers through (RG) or maps *int->*int32
(BitDepth/BPM).
* refactor(plugins): trim redundant pass labels in ndpgen ParseDirectory
The function doc already explains the two-pass approach; the inline labels
restated it. Reduce to bare waypoints.
* fix(plugins): gate Track.Path on library filesystem permission
MatchSongs copied mf.Path into every result unconditionally, letting a plugin
with only the matcher permission enumerate on-disk file paths by matching known
songs. Gate Path behind library.filesystem, matching the Library host service.
toTrack is now a method carrying the permission flag.
* refactor(plugins): align MatchSong JSON casing and parse Go files once
- MatchSong: artistMBID/albumMBID JSON tags -> artistMbid/albumMbid so the Go
wire format matches the Rust SDK's camelCase serialization (cross-SDK fix)
- MatchSongs doc reworded to language-neutral 'empty (absent)' so generated
Rust/Python client docs no longer say Go-specific 'nil'
- ndpgen: parse each package file once (parseGoFiles) and reuse the ASTs across
both passes in ParseDirectory and ParseCapabilities, instead of re-parsing
* refactor(plugins): use shared types for Matcher host service
Move the Matcher host service onto the shared plugins/types package instead
of the host-local MatchSong and Track structs. MatchSongs now takes
[]types.SongRef and returns []*types.Track, dropping host.MatchSong and moving
host.Track (with its host.Artist dependency collapsed onto types.ArtistRef) into
plugins/types. ArtistRef gains SortName and SubRole so it can back a track's
Participants.
SongRef gains a millisecond-precision DurationMs field that supersedes the now
deprecated seconds-based Duration, with DurationInMs() resolving the effective
value and SetDurationMs() keeping both fields in sync when populating a SongRef
to send to a plugin.
The ndpgen host-wrapper template only ever imported context, json and extism, so
a host service referencing the shared types package produced uncompilable code.
Emit the plugins/types import when the service references shared types directly
(gated on the existing Service.ImportsSharedTypes), matching the client template,
and cover it with GenerateHost tests. This removes the need for host-local
re-export aliases. Regenerated the Go/Rust/Python PDK and capability schemas
accordingly.
* test(plugins): cover SongRef duration and artist conversion
Add unit coverage for the new SongRef behavior: SetDurationMs populating both
DurationMs and the deprecated seconds field, and the SongRef-to-agents.Song
conversion preferring DurationMs over Duration and the Artists list over the
scalar Artist/ArtistMBID.
Extract the inline SongRef-to-agents.Song closure in MatchSongs into a named
toAgentSong function so the conversion can be asserted directly rather than only
through the opaque matcher. The end-to-end wire shape of the moved types is
already validated by the existing MatcherService integration test, so no new
WASM-boundary test is needed.
* fix(plugins): harden and unify SongRef-to-agents.Song duration conversion
Address findings from a code review of the matcher host service:
- DurationInMs now clamps a negative deprecated-seconds value to 0 instead of
converting it through uint32, which previously wrapped a value like -1s into a
~49-day duration that corrupted the matcher's duration-proximity tiebreaker.
- Replace the unused SetDurationMs(uint32) with SetDuration(seconds float32),
which takes the unit callers actually hold (model.MediaFile.Duration is
float32 seconds) and centralizes the seconds-to-ms conversion. Wire it into
mediaFileToSongRef so outbound SongRefs carry both duration fields in sync.
- Make the metadata-agent path use DurationInMs() so every consumer of the
shared SongRef honors the DurationMs-over-Duration precedence contract; a
plugin sending only DurationMs no longer loses its duration on that path.
- Collapse the matcher's duplicate toAgentSong/agentArtists helpers into the
existing songRefToAgentSong converter, so there is a single SongRef-to-Song
mapping. Tests narrowed to the duration cases, with artist precedence still
covered in metadata_agent_test.go.
* feat(plugins): allow Matcher host service to scope a match to a user
Add an options struct to the Matcher host service so a plugin can run a match as
a specific user. When MatchOptions.Username is set, the match is run in that
user's context: their favourites and ratings inform the matcher's tiebreaker, and
the returned tracks carry that user's per-user annotations (Starred, StarredAt,
Rating, PlayCount, PlayDate, added to types.Track). An empty username preserves
the previous unscoped behaviour.
Cross-user access is gated by the same allowedUsers/allUsers permission the Users
and SubsonicAPI host services use: an unknown username, or one the plugin is not
permitted to act as, returns an error. User-library access applies automatically
once the user is in context (applyLibraryFilter). Independently, results are now
restricted to the libraries the plugin itself may access via the precomputed
libraryAccess set, dropping any matched track outside that set (the input index
stays unmatched) — this applies even without a username and even for an
admin-scoped user.
core/matcher is unchanged: it already loads and uses annotations and applies
user-library filtering from context, so the feature works by deriving the request
context and post-filtering by plugin library access in the host adapter. The new
opts parameter and the Track annotation fields are propagated to all PDK clients
(Go/Rust/Python) by make gen.
* fix(plugins): correct Matcher library scope and unify user-access checks
Address findings from a code review of the user-scoped Matcher host service:
- The plugin-library post-filter previously dropped every match for a plugin that
holds only the matcher permission, because library config is tied to the Library
permission and a matcher-only plugin has none (empty allowedLibraries,
AllLibraries=false). Gate the filter on whether the plugin actually declared the
Library permission: matcher-only plugins are no longer library-restricted, while
plugins that opt into a library scope are enforced as before. The per-user
library filter (applyLibraryFilter) still applies whenever a non-admin user is
scoped.
- resolveUser collapsed every FindByUsername error (including transient DB
failures) into a misleading "not found". Extract a shared userAccess type
(alongside libraryAccess) whose resolve() distinguishes model.ErrNotFound from a
real backend error and authorizes the user against the allowed set. The Matcher
service now uses it, and host_subsonicapi shares the same userAccess type for its
permission check (preserving its existing error messages), removing a third
divergent copy of the resolve-and-authorize logic.
- Document in the matcher tests that the mock MediaFileRepo returns annotations
unconditionally, so the unit tests cover the adapter's scoped-flag gating and
access checks but not the SQL per-user join. Add tests for the library-permission
gating and for surfacing a backend error instead of masking it as not-found.
* fix(plugins): require a library scope for Matcher, fail closed
Reverse the permissive default introduced when fixing the library post-filter: a
Matcher plugin now must be granted a library scope (all libraries, or at least one
specific library) and MatchSongs rejects the request with "no libraries
configured" when it has none, instead of either silently matching nothing or
defaulting to every library.
This mirrors how the SubsonicAPI host service requires a user scope
(checkPermissions errors with "no users configured" when none is set): the check
is a runtime guard via libraryAccess.configured(), needs no manifest changes, and
keeps the failure loud rather than silent. The per-match library post-filter then
always applies, and the restrictLibraries flag added in the previous commit is
removed.
* fix(plugins): require library permission for matcher; guard nil user
Close the gap where a plugin declaring only the matcher permission loaded
successfully but failed every MatchSongs call with "no libraries configured",
with no way for an admin to grant a library scope (the library-config UI is gated
on the library permission). Add a cross-field manifest rule, mirroring the
existing "subsonicapi requires users" rule, so the matcher permission requires
the library permission to be declared. A matcher plugin therefore also surfaces
the library-config panel and is subject to the existing load/enable-time library
configuration gate, making the fail-closed library check reachable and fixable
rather than a silent dead end. The test plugin manifest now declares the library
permission accordingly.
Also restore a defensive nil-user guard in userAccess.resolve: if a DataStore's
FindByUsername ever returns (nil, nil) instead of model.ErrNotFound, return a
clean "not found" error rather than dereferencing a nil *model.User.
* feat(plugins): expose track AverageRating in Matcher results
Add AverageRating to the Matcher's Track DTO. Unlike the per-user annotations
(Starred, Rating, PlayCount, ...), AverageRating is an aggregate stored on the
track itself and is loaded regardless of the request user, so it is populated
unconditionally rather than gated on a scoped username. Propagated to the PDK
types by make gen.
Signed-off-by: Deluan <deluan@navidrome.org>
* style(plugins): trim verbose comments in matcher host service
Condense the over-long explanatory comments added across the matcher host
service to one-liners that state the why, and simplify the ptrInt32/unixPtr
helpers to Go 1.26's new(value). No behavior change.
* refactor(plugins): pass userAccess into newSubsonicAPIService
Move newUserAccess construction to the loader call site so the SubsonicAPI service
constructor takes a userAccess value directly, matching newMatcherService. Pure
refactor: the service already stored a userAccess internally, so behavior and error
messages are unchanged.
* fix(plugins): regenerate PDK and drop omitempty from AverageRating
Re-run make gen so the generated PDK doc comments match the source comment
trimmed in an earlier commit (the source was simplified but the PDK was not
regenerated, leaving the committed files stale — a 'generated files up to date'
hazard).
Also drop omitempty from Track.AverageRating: it is always set (0 when unrated),
so it should be present in the payload like the other always-set fields
(BirthTime/CreatedAt/UpdatedAt), not dropped at zero. Tag change propagated to the
PDK by the same regeneration.
* fix(plugins): reject user-scoped match before lookup when plugin has no user scope
A matcher plugin requires the library permission but not the users permission, so
a matcher-only plugin always has an empty user scope (allUsers=false, no allowed
users). MatchSongs still ran FindByUsername for any opts.Username before checking
authorization and returned distinguishable errors ('user X not found' vs 'not
allowed to act as user X'), letting such a plugin enumerate account names from the
error text.
Guard userAccess.resolve to reject with a single fixed error before the lookup when
the plugin has no user scope, mirroring how the SubsonicAPI service short-circuits
with 'no users configured'. The unscoped match path (no username) is unaffected, so
matcher-only plugins still match normally.
* fix(plugins): run unscoped matcher as admin, not the inherited request user
A matcher host call can arrive on a context that already carries a request user
(e.g. a plugin capability invoked while serving that user's request — extism
propagates the call context into host functions). With no opts.Username, MatchSongs
passed that context straight through, so the media-file repository applied the
caller's library filter and per-user annotation ranking to an explicitly unscoped
match.
Set the user context explicitly: a username scopes to that user (overriding any
inherited one), and an unscoped match runs under adminContext so only the plugin's
own library scope constrains results. Adds tests using a context-capturing
DataStore to assert the user the matcher resolves in both cases.
* chore(plugins): drop the generated Python matcher PDK
The Python plugin PDK is no longer supported (ndpgen generates only Go and Rust
clients), so remove the stale generated nd_host_matcher.py rather than leave a
client that drifts from the host interface.
* docs(plugins): deprecate SongRef.Artist/ArtistMBID in favor of Artists
Mark the scalar single-artist fields deprecated; Artists (the ArtistRef list) is
the preferred way to supply artist data and already takes precedence for matching.
Propagated to the PDK and capability schemas by make gen.
* refactor(plugins): flatten Track.Participants and add Role to ArtistRef
Change Track.Participants from map[role][]ArtistRef to a flat []ArtistRef, and give
ArtistRef a Role field (the participation category: artist/composer/performer/...)
alongside SubRole (a specialization within a role, e.g. the instrument for a
performer). In the flat list each entry now self-describes its role rather than
relying on a map key, matching how SongRef.Artists is already a flat list; the
converter tags each entry with its role and emits them in a stable role order.
Propagated to the PDK and capability schemas by make gen.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* perf(persistence): skip annotation join in CountAll when unused
The Native API list endpoints (/api/song, /api/album, /api/artist) issue a
pagination count on every request via rest.GetAll. CountAll unconditionally
added a LEFT JOIN on the annotation table to a count(distinct id) query. The
join's columns are stripped by count(), but the distinct-over-join forced
SQLite to stream and dedup every row, making the count dominate the request
time on large libraries (e.g. ~190ms cold for 95k songs, seconds for
non-admin users behind the library subquery).
Gate the annotation join: only add it when a filter actually references an
annotation column. The need is detected by rendering the query to SQL and
matching annotation column names as whole words, which covers both named
filters (starred, has_rating) and raw squirrel filters. The column set is
derived from model.Annotations so it tracks schema changes; average_rating is
excluded because it lives on the base table, and word-boundary matching keeps
it from matching the annotation column rating.
Counts are unchanged; only the query plan changes. Unfiltered song counts drop
from ~37ms to ~4ms (warm) on a 95k-song library.
* test(persistence): make annotation-join detection case-insensitive
Address review feedback: SQLite column names are case-insensitive, so a raw
filter using e.g. "RATING" would previously evade the case-sensitive column
regex and wrongly drop the annotation join. Add the (?i) flag (average_rating
stays excluded — the underscore still prevents a word boundary before rating)
and cover it with mixed-case tests. Also make the starred-count assertion an
exact value instead of a range.
* refactor(plugins): remove Python PDK generation from ndpgen
* feat(plugins): parse Go type aliases distinctly in ndpgen
* feat(plugins): resolve shared-type aliases against a registry in ndpgen
* fix(plugins): resolve host-service shared aliases package-wide
Mirror the capability approach in ParseDirectoryWithShared: do a first
pass over all package files to build a package-wide alias map, then pass
it into parseServiceFile so that a shared-type alias declared in a sibling
file is visible when resolving types in the service interface file.
Add a focused test that writes the alias in one file and the hostservice
in another, confirming RED before the fix and GREEN after. Also
strengthens the existing Task 3 test with an ArtistRef.Target assertion.
* feat(plugins): add ndpgen -shared-types mode for the Go types package
* feat(plugins): generate the nd-pdk-types Rust crate from -shared-types
* feat(plugins): inject types import and emit deprecated aliases in Go output
* feat(plugins): emit deprecated Rust aliases to the shared types crate
* feat(plugins): inline shared-type shapes into XTP schemas
* feat(plugins): add nd-pdk-types crate and wire dependents
* feat(plugins): move shared capability types to plugins/types with deprecated aliases
* fix(plugins): point Rust deprecated-alias note at the replacement type
* fix(plugins): include shared aliases in KnownStructs so Rust fields keep their type
Capability.KnownStructs() and Service.KnownStructs() previously only
registered names from .Structs. After the shared-types migration, types
like ArtistRef/TrackInfo/SongRef live in .SharedAliases instead, so
ToRustTypeWithStructs could not find them and fell back to serde_json::Value
for every struct field referencing a shared type.
Add the shared-alias names to the knownStructs map in both methods.
Regenerate the Rust capability files; track/song/artist fields now render
as their named types (TrackInfo, SongRef, ArtistRef, etc.).
Add a regression test that verifies a struct field whose type is only in
SharedAliases renders as the named type and not serde_json::Value.
* docs(plugins): remove stale Python references from ndpgen and plugins READMEs
ndpgen no longer has a -python flag; remove it from the usage synopsis,
flags table, and defaults note in ndpgen/README.md. Delete the "Python
Client Library" section that described its output.
plugins/README.md referenced plugins/pdk/python/host/ (deleted) as the
source for Python host-service stubs. Remove that paragraph; Python plugins
still work via the XTP-schema / extism-py path (see examples/*-py).
* refactor(plugins): dedupe ndpgen helpers and tidy shared-type codegen
* docs(plugins): restore Python as a supported XTP schema target
The ndpgen-generated Python PDK was removed, but the XTP YAML schemas are
language-neutral and the XTP CLI still generates Python bindings from them
(as the extism-py examples demonstrate). Only the ndpgen Python output was
dropped, not Python support itself.
* test(plugins): use the shared types package in test plugins
The test fixtures referenced the now-deprecated capability aliases
(sonicsimilarity.SongRef, metadata.ArtistRef/SongRef). Point them at the
canonical types package so our own fixtures don't depend on symbols slated
for removal.
* refactor(plugins): use the shared types package in host adapters
Replace deprecated capabilities.TrackInfo, capabilities.ArtistRef, and
capabilities.SongRef aliases with the canonical types.TrackInfo,
types.ArtistRef, and types.SongRef from plugins/types.
* fix(plugins): reference shared types by canonical path in generated Rust
Previously the generator emitted `pub field: SongRef` (the local deprecated
alias) for struct fields whose type came from SharedAliases. Refactored
ToRustTypeWithStructs into a private toRustType that accepts a shared map,
and added ToRustTypeWithShared which resolves shared-alias names to their
canonical nd_pdk_types::X path before falling through to the knownStructs
check. Both rustCapabilityFuncMap and rustFuncMap now build the shared map
from SharedAliases and use it for fieldRustType, so the generated capability
files reference nd_pdk_types::SongRef / nd_pdk_types::TrackInfo directly.
The deprecated pub type aliases remain in place as the external back-compat
surface. Deprecation warning count from cargo build drops to 0.
* fix(examples): implement missing Scrobbler.playback_report in Rust examples
The Scrobbler trait gained a playback_report method but the two Rust example
plugins (webhook-rs and discord-rich-presence-rs) were not updated, causing
E0046 compile errors. Added the missing fn playback_report to both: webhook-rs
logs and returns Ok(()) mirroring its now_playing handler; discord-rich-presence-rs
is a no-op since Discord presence does not need playback reports. make all-rust
now exits 0.
* refactor(plugins): point Rust deprecation notes at the nd_pdk::types umbrella path
Plugin authors depend on the nd-pdk umbrella crate, which re-exports
nd_pdk_types as 'types', so the migration target they should type is
nd_pdk::types::X. The alias target stays nd_pdk_types::X (the real path
inside nd-pdk-capabilities).
* fix(plugins): error when a shared-type alias can't be resolved against the registry
* refactor(plugins): parse each Go source file once in ndpgen
* fix(plugins): correct ndpgen review nits (flag name, unused dep, docs)
* refactor(plugins): drop the now-unused path param from parseServiceFile
* refactor(plugins): use shared types directly, rename TrackInfo to Track
Capability interfaces now reference the shared `types` package by qualified
name (types.Track, types.SongRef, types.ArtistRef) instead of the package-local
deprecated aliases, and the shared TrackInfo type is renamed to Track to match
its role as the plugin-facing projection of a library media file.
The deprecated bare aliases (scrobbler.TrackInfo, metadata.ArtistRef,
sonicsimilarity.SongRef, etc.) are kept as re-exports so existing plugins keep
compiling, with a deprecation warning steering them to the canonical types.
To support this, ndpgen now resolves qualified types.X references: it collects
them during type discovery, maps each used canonical type back to its declared
deprecated alias for re-export, emits nd_pdk_types::X paths in Rust, and names
the XTP schema components by their canonical type. Regenerated the Go and Rust
PDK and the XTP schemas, and added generator tests covering the qualified-ref
path. Also adds clarifying doc comments to the shared types.
* refactor(plugins): extract shared types selector into a named const
Replace the "types." string literal that detects and strips the shared types
package selector with a single sharedTypesPrefix constant across the ndpgen
generator (parser, types, generator, xtp_schema), giving the package one source
of truth for the selector.
Also restore the single reused scratch map (cleared each iteration) in the
resolveSharedAliases BFS instead of allocating a fresh map per shared-struct
field, matching the prior implementation.
Pure cleanup from a /simplify pass: regeneration produces byte-for-byte
identical Go, Rust, and XTP output.
* refactor(plugins): keep TrackInfo in the capability package for now
Move the track type back out of the shared plugins/types package: it is again
defined inline as TrackInfo in plugins/capabilities/scrobbler.go and referenced
directly by the scrobbler and lyrics capabilities, reverting the rename to
types.Track. The host helper is renamed back to mediaFileToTrackInfo and now
returns capabilities.TrackInfo. SongRef and ArtistRef stay in the shared types
package; TrackInfo keeps using types.ArtistRef for its artist lists.
This type is expected to be reshaped in upcoming work, so leaving it in the
capability package avoids churning the shared types twice. Regenerated the Go
and Rust PDK and the XTP schemas accordingly.
* fix(plugins): emit the Go types import for direct shared-type refs
ndpgen's Capability/Service.ImportsSharedTypes only reported a shared-types
dependency when a deprecated re-export alias (type X = types.X) was declared. A
struct field referencing the canonical form directly (e.g. types.SongRef) with
no such alias produced an empty SharedAliases slice, so the Go templates skipped
the import while still emitting fields/signatures using types.SongRef — leaving
generated PDK code for new shared DTOs uncompilable unless an otherwise
unnecessary alias was added.
ImportsSharedTypes now also returns true when any struct field references the
types. package by qualified name, via a new structsReferenceSharedTypes helper
that reuses collectReferencedTypes (so []types.X and map[...]types.X are covered
too).
* fix(plugins): preserve base64 encoding for shared byte fields in Rust
The Rust shared-types crate template rendered a []byte field as a plain Vec<u8>
without the base64_bytes serde override used by the capability/client templates.
Go's encoding/json serializes []byte as a base64 string, so a Rust plugin using
nd_pdk::types would have serialized an array of numbers instead of the wire
format the Go/server side expects.
GenerateSharedTypesRust now registers the base64_bytes partial and passes a
HasByteFields flag (new anyFieldIsByteSlice helper); types.rs.tmpl emits the
base64_bytes module and a #[serde(with = "base64_bytes")] attribute on []byte
fields, mirroring the capability template.
* fix(plugins): include directly-referenced shared types in XTP schemas
buildSchemas registered shared types into the schema components only by iterating
cap.SharedAliases, which records deprecated re-export aliases. A capability that
referenced a shared DTO solely as types.Foo (no declared alias) therefore never
got Foo into the component set, so the self-contained XTP schema rendered the
field as a generic object (or emitted a dangling $ref), breaking the direct
shared-type use case enabled by -shared.
resolveSharedAliases now also returns the resolved shapes of every used shared
type (alias or not); these are carried on the new Capability.SharedTypes field
and registered by buildSchemas alongside SharedAliases. Validated end-to-end with
the xtp CLI: a direct types.Foo reference now produces a proper component plus a
$ref, so xtp generates a typed struct instead of an untyped serde_json::Map.
* fix(plugins): resolve renamed shared aliases to canonical schema refs
When a deprecated alias renames its canonical type (e.g. type TrackInfo =
types.Track) and a capability field is typed with the alias name (TrackInfo),
buildProperty emitted a $ref to #/components/schemas/TrackInfo. Components are
keyed by the canonical name (Track), so no TrackInfo component was emitted,
leaving a dangling reference that crashes the xtp code generator.
buildSchemas now builds an alias->canonical map; buildProperty (and the slice
item path) resolves $ref targets through it, and a used alias name marks its
canonical component used so it is emitted. Validated with the xtp CLI: the
renamed-alias schema previously crashed xtp and now generates cleanly.
* fix(plugins): detect shared types used directly in method signatures
ImportsSharedTypes only inspected struct fields, so a capability method using a
shared type directly in its signature (e.g. types.SongRef as input/output rather
than inside a local struct) was not detected. The generated Go templates still
rendered the provider/export signatures with types.SongRef, so the capability
package omitted the types import and failed to compile; the same gap applied to
service params/returns.
ImportsSharedTypes now also scans capability method input/output types and
service method params/returns, via a typeReferencesSharedTypes helper that reuses
collectReferencedTypes (covering pointer/slice/map wrappers).
* fix(plugins): add base64 dependency to the shared Rust types crate
When a shared DTO has a []byte field, ndpgen emits the base64_bytes serde helper
and use base64::... imports into nd-pdk-types/src/lib.rs, but the crate manifest
declared only serde. In that case make gen produced a crate that failed to
compile with 'unresolved module base64'.
Add base64 = "0.22" (matching nd-pdk-capabilities) so the generated shared types
crate compiles whenever a []byte field is present. Verified by generating a
shared crate with a []byte field and confirming cargo check fails before and
passes after.
* fix(plugins): translate shared method types in generated Rust
A capability method using a shared DTO directly as input/output (e.g.
types.SongRef) was passed through rustOutputType unchanged, so the Rust template
emitted invalid trait and extism_pdk::Json<$crate::pkg::types.SongRef> signatures
that do not compile.
Method input/output types now resolve through the shared registry: trait
signatures use rustTraitType (shared -> nd_pdk_types::X, locals stay bare) and the
export macros use rustMethodType (fully qualified: shared -> nd_pdk_types::X,
primitives -> Rust, locals -> $crate::<pkg>::X). Verified end-to-end by compiling
a generated capability that takes types.SongRef directly against the real
nd-pdk-types crate.
* fix(plugins): canonicalize XTP export refs for renamed shared aliases
buildSchemas canonicalized alias-to-canonical references for struct-field $ref
targets, but buildExport built export input/output $refs straight from
fieldBaseType. A capability whose export used a renamed deprecated alias
directly (e.g. type TrackInfo = types.Track with NowPlaying(TrackInfo)) emitted
$ref: #/components/schemas/TrackInfo, while the component is emitted under the
canonical name Track — a dangling export reference.
Lift the alias-to-canonical map into GenerateSchema (buildAliasToCanonical) and
apply it to export refs via canonicalRefName, the same resolution already used
for field properties.
* fix(plugins): route shared macro types through $crate for plugin builds
When a capability method used a shared type directly, the generated export macro
named the type as nd_pdk_types::SongRef. The macro expands in the downstream
plugin crate, which depends on the umbrella nd-pdk crate and not on nd-pdk-types
directly, so that path is unresolvable there and the plugin fails to build.
rustMethodType (macro-facing) now emits $crate::types::X, and the generated
nd-pdk-capabilities lib.rs re-exports nd_pdk_types as types so $crate resolves
it. Trait signatures keep nd_pdk_types::X since they live in nd-pdk-capabilities,
which has the direct dependency. Verified end-to-end: a plugin crate depending
only on the umbrella that uses a capability with a direct types.X method now
compiles via the macro.
* fix(plugins): add nd-pdk-types dependency to the Rust host crate
When a host service uses a shared type, ndpgen emits nd_pdk_types::X into the
generated nd-pdk-host client wrappers, but the host crate's manifest did not
depend on nd-pdk-types, so the crate failed to compile with 'unresolved module
nd_pdk_types'. Host client wrappers are plain functions resolved in the host
crate's own context (not macros expanded downstream), so a direct dependency is
the right fix.
Add nd-pdk-types = { path = "../nd-pdk-types" } to nd-pdk-host, mirroring
nd-pdk-capabilities. Found while auditing all Rust paths against the realistic
crate topology after the capability-side $crate fix; verified by generating a
host service with a shared-type return and confirming cargo check fails before
and passes after.
* fix(plugins): resolve shared aliases in Rust host signatures
The Rust host client rendered method params and returns through
RustTypeWithStructs, which only consults KnownStructs. A host service using a
shared alias in a signature (e.g. type Track = types.Track plus
MatchSongs(...) ([]Track, error)) therefore emitted a bare Vec<Track>, but the
client template emits no Track alias or import, so the generated nd-pdk-host
crate did not compile. Only struct fields went through the shared map.
rustType/rustParamType now use the shared map too (RustTypeWithShared /
RustParamTypeWithShared), so an aliased param/return resolves to its canonical
nd_pdk_types::X path, matching field handling. Verified by generating a host
service returning a shared alias and confirming cargo check fails before and
passes after.
* fix(subsonic): align album `created` with RecentlyAddedByModTime sort
The album `created` attribute returned by search3, getAlbumList2 and the
other album endpoints was always sourced from the album's CreatedAt (oldest
song birth time), while the "recently added" sort is governed by
RecentlyAddedByModTime: it orders by album.updated_at when that option is
enabled and album.created_at otherwise.
As a result, when RecentlyAddedByModTime was enabled, clients that cache
album results and sort locally by `created` (e.g. for a "Date added" view)
could not reproduce the order returned by getAlbumList2?type=newest, since
the exposed value did not match the column driving the sort.
Make albumCreatedAt config-aware so the primary timestamp it returns mirrors
recentlyAddedSort: UpdatedAt when RecentlyAddedByModTime is set, CreatedAt
otherwise. The existing zero-value fallback chain is preserved so this
required OpenSubsonic field is never emitted as zero on legacy rows.
Note: this is a behavior change for the contractual `created` attribute. With
RecentlyAddedByModTime enabled, an album's reported `created` now reflects the
newest song modification time and can change when files are modified.
Scope is limited to albums; the song-level `created` (BirthTime) is unchanged.
* fix(subsonic): order Recently Added by full-precision timestamp with tiebreak
The recently_added sort wrapped the timestamp in datetime(), truncating it
to whole seconds, and had no secondary sort key. Album timestamps carry
sub-second precision (aggregated from song file birth-times), so on a fresh
scan many albums tie at the second; SQLite then returns ties in query-plan
order, which changes when a library filter is applied. This made the web UI
"Recently Added" order invert between library selections and diverge from
getAlbumList2?type=newest, and clients receiving the full-precision created
value could never reproduce the server order.
Sort on the raw, full-precision column with an album.id / media_file.id
tiebreak instead. A new migration swaps the album datetime() expression
indexes (and the plain media_file indexes) for composite (col, id) indexes
that cover the new sort. Timestamps were already normalized to space-format
by 20260316000000_normalize_timestamps, so raw-string comparison is safe.
* fix(subsonic): make song created follow RecentlyAddedByModTime
The song Created field returned BirthTime (file ctime), but the recently_added
sort (shared by the Subsonic and native APIs, and the web UI) orders by
created_at, or updated_at when RecentlyAddedByModTime is set. A Subsonic client,
which can only sort by the created value it receives, could therefore never
reproduce the server's "recently added" order in either mode.
Add mediaFileCreatedAt mirroring albumCreatedAt and use it for child.Created:
CreatedAt by default, UpdatedAt under RecentlyAddedByModTime, with BirthTime as
a legacy fallback. This aligns song created with the sort column, matching how
album created already works and what the native API/web UI present.
Pulls in the lyric timing fixes: lyrics no longer stack from the previous
song on rapid track changes (#5661), stay in sync after seeking/scrubbing
(including while paused), and show a music-note placeholder during intros
and gaps instead of the 'no lyrics' message.
* feat(plugins): export ReadPackageManifest and ValidatePackage for off-disk inspection
* test(plugins): cover ValidatePackage cross-field validation branch
* feat(cmd): add 'plugin list' command
* refactor(cmd): drop premature pluginManager interface until first use
* feat(cmd): add 'plugin enable' and 'plugin disable' commands
* feat(cmd): add 'plugin edit' for config and permission updates
* test(cmd): cover plugin edit aborting on config validation failure
* feat(cmd): add 'plugin info' and 'plugin validate' (installed id or .ndp file)
* test(cmd): unit-test formatManifestInfo nil-guard and json branches
* feat(cmd): add 'plugin rescan' command
* feat(cmd): enrich plugin info text output and re-validate config on validate
* fix(cmd): omit zero-value timestamps in plugin info text output
* fix(cmd): give plugin list and info separate format flag vars
A shared package-global bound to both commands' --format flag caused the
later init() registration (info, default "text") to clobber the earlier
one (list, default "table"), so 'plugin list' with no -f failed with
'invalid output format "text"'. Split into pluginListFormat /
pluginInfoFormat and add a regression test on the registered defaults.
* fix(plugins): address code-review findings on plugin CLI
- edit: read-modify-write merge so flipping one permission flag no longer
wipes unspecified fields (matches the native API); reject non-JSON
--users/--libraries values.
- info: return an error on unknown --format (was silently text); include
sha256 in off-disk JSON output; align the text columns.
- move declaredPermissions onto plugins.Permissions.DeclaredNames() as the
single source of truth; drop ValidatePackage's redundant Validate call.
- readConfigFile: pass ctx to log.Fatal; copy flag globals before taking
their address; trim comments that restated the code.
* refactor(plugins): export ReadManifest/ComputeFileSHA256, drop thin CLI wrappers
Remove the ReadPackageManifest/ComputeFileSHA256/ValidatePackage wrappers in
favor of exporting the underlying readManifest->ReadManifest and
computeFileSHA256->ComputeFileSHA256 directly. ValidatePackage was identical to
ReadPackageManifest (readManifest already validates via ParseManifest), so the
CLI's validate path now calls ReadManifest too.
* test(plugins): consolidate ReadManifest specs and move ComputeFileSHA256 tests
Merge the two ReadManifest Describe blocks into one, and move the
ComputeFileSHA256 tests out of package_test.go into a new manager_sync_test.go
(where the function now lives), deduping the overlapping hash specs.
* test(plugins): use createTestPackage everywhere, drop writeNDP helper
The schema-invalid and cross-field ReadManifest specs now build packages with
createTestPackage (typed Manifest) instead of the raw-JSON writeNDP helper, so
there is a single package-building helper. Also trims the gratuitous 1MB wasm
buffer in the read-only spec to a few bytes.
* test(plugins): drop duplicate missing-manifest spec from ReadManifest
The openPackage block already covers the missing-manifest.json error path
(shared zip-walk code); ReadManifest's identical copy added no distinct
coverage.
* test(plugins): fix misleading ReadManifest spec name and drop unused wasm bytes
The spec was named 'should read only the manifest without loading wasm' but
ReadManifest returns only (*Manifest, error) — it cannot assert whether wasm was
loaded. Rename to what it actually verifies (manifest parses from a package that
also contains a wasm entry) and pass nil wasm bytes, since the contents are
never observed.
* fix(cmd): address PR review feedback on plugin CLI
- edit --users/--libraries now accept both comma-separated and JSON-array input
(CSV is converted to the JSON the manager stores); help text documents both.
- Permissions.DeclaredNames() derives names by reflecting over the generated
struct's json tags instead of a hand-maintained list, so new permission types
are picked up automatically.
- isPackagePath checks only the .ndp suffix, so a mistyped package path yields a
precise 'no such file' error instead of falling back to a misleading
'Plugin not found'.
- Restore a ReadManifest test for the missing-manifest.json error path (it has
its own branch, separate from openPackage).
* docs(cmd): trim verbose comments to the why
* fix(cmd): setting an explicit users/libraries list clears the all-* flag
Per Codex review: with allUsers/allLibraries true, 'plugin edit --users <list>'
preserved the all-* flag so the allow-list was silently ignored, and the
mutually-exclusive flags meant it couldn't be corrected in one command. An
explicit list now implies all-*=false.
Fixes a transient jump to the wrong song when switching the play queue.
When a new queue was loaded at a non-zero index (e.g. playing a different
album/playlist from a track other than the first, or playing a new album
after closing the player), the web player briefly loaded and played the
track that sat at the *previous* internal index in the new queue before
correcting to the chosen one — an audible "skip to a random song, then
back to the song I chose".
The root cause was in the player library: when loading a new audio list,
the initial track was picked using the stale internal play index instead
of the requested playIndex. Fixed in navidrome-music-player 4.25.3
(navidrome/react-music-player), which derives the initial track from the
requested playIndex.
* fix(lyrics): correct TTML background-vocal cue timing and whitespace
Two parsing defects surfaced by Apple Music TTML files that mix a main
vocal with an x-bg (background) span group within the same line:
- Cue end-time normalization ran over the whole line's cue list in
document order. Background cues are stored after the main cues but
interleave earlier on the timeline, so the next-cue clamp collapsed the
last main cue's end down to its own start (start == end). End times are
now normalized per agent group, matching how the Subsonic serializer
already groups cues, so parallel layers no longer corrupt each other.
- Whitespace between elements was treated as significant: pretty-printed
(indented) TTML injected spurious newlines into the line text, turning
one line into many. Per TTML2 default xml:space handling (linefeeds
treat-as-space, whitespace-collapse), formatting whitespace now collapses
to a single space and hard line breaks come only from <br/>.
The line-level value and per-agent cueLine.value remain the full line text,
as required by the OpenSubsonic songLyrics v2 contract; the per-agent text
is carried in each cueLine's cue[] array.
Two existing tests that encoded the buggy newline-as-break behavior are
corrected; new tests cover whitespace collapse, <br/> preservation, and
interleaved background cue timing.
* fix(lyrics): only collapse XML whitespace, preserve other Unicode spaces
Whitespace collapsing used unicode.IsSpace, which matches more than the XML
S production (space, tab, CR, LF): it also folds characters like NBSP and
U+3000 into a regular space, silently altering content. Restrict collapsing
to the four XML whitespace characters so other Unicode spaces pass through
unchanged, and add a regression test. Also clarify the doc comment that
collapsing is applied unconditionally (xml:space="preserve" is not supported).
