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