2 commits

Author	SHA1	Message	Date
Deluan Quintão	c9385fbb6b	test(plugins): build test plugins in Go instead of shelling out to make (#6060) ... * 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.	2026-08-31 21:42:10 -04:00
Deluan Quintão	5b87a60b5d	fix(plugins): load plugin agents in CLI commands (#5959) ... * feat(plugins): load plugin agents in CLI commands A CLI that goes through core/agents saw only built-in agents: getEnabledAgentNames asks Manager.PluginNames, which reads a map populated solely by Manager.Start, and only the server calls that. On a plugin-using install the CLI's agent list was quietly short — artwork explain --live could name deezer for an artist whose stored source was external:apple-music, because apple-music was invisible to it. Adds Manager.LoadPlugins, the read-only counterpart to Start: extism/wazero init plus loadEnabledPlugins, without the folder sync, the error clearing, the cache purge or the watcher. It follows what the read-only plugin commands already do — list, info and validate read the DB and never start the manager — except that capabilities are detected from the WASM exports, not declared in the manifest, so instantiating is the only accurate source for what a plugin provides. loadEnabledPlugins disabled a plugin and recorded LastError when a load failed. That is right for the server and wrong for a diagnostic, so it is now gated on the read-only flag: inspecting a plugin must not disable it. No Subsonic router is required. Start log.Fatals without one, but the host function it feeds already nil-checks and reports 'SubsonicAPI router not available' at call time, so a plugin that reaches for it gets an error instead of the process dying. That Fatal's message also claimed the DataStore was missing; it checks the router. * fix(cli): correct the reprocess estimate's plugin caveat imageAgentCount now receives a manager with plugins loaded, so the external estimate already includes plugin image agents — but the disclaimer still said they were not counted, which told operators the opposite of what the number meant. Replaced rather than dropped: loadPluginAgents warns and continues when LoadPlugins fails, and LoadPlugins is a no-op when plugins are disabled or no folder is set, so there are still runs where plugin agents genuinely are not counted. The wording now covers all three cases, and the stale comment above it said the CLI never starts the plugin manager, which is what this branch changed. Found by Codex on 648cf38e9. * fix(plugins): load only the configured agents, and gate plugin init Loading a plugin is not free: the service constructors create a KVStore or TaskQueue database and a Storage directory for any plugin whose manifest declares those permissions, and the plugin's own init then runs arbitrary code. loadEnabledPlugins loads every enabled plugin, so inspecting artwork was starting scrobblers, schedulers and lyrics plugins that could never supply an image. Measured on a copy of a production library: 'artwork explain' created apple-music/kvstore.db, nd-lyrics/kvstore.db and listenbrainz-daily-playlist/taskqueue.db. The last one matters most — CreateQueue resets rows with status='running' to 'pending', which against a live server sets up its in-flight tasks to run twice. LoadPlugins now takes the names to load, and the artwork CLI passes the Agents list: a plugin that is not a configured agent can never win, so there is nothing to gain by instantiating it. The same two runs now create only apple-music, which is a configured agent and therefore the cost of answering the question. Init is gated separately on the caller's intent rather than on read-only. 'explain --live' already means 'reach the provider', so it runs init; plain 'explain' and 'reprocess' promise no external requests and must not. Documented in the --live flag help. Found by Codex on 28eb39ac4. * perf(cli): load plugin agents only when the selection can consult one Explaining disc or media file artwork loaded every configured metadata plugin, though neither resolver ever reaches an agent: resolveMediaFile is embedded-only and discArtworkReader.selectImage refuses external outright. With --live that also ran plugin init for a walk that provably cannot reach the network. The load now sits inside the artist/album branch that already exists, so it is a move rather than a new condition. reprocess did the same for a radio-only selection, whose estimate is unconditionally zero. It is gated on needsImageAgents, which asks exactly what ExternalLookupsPerItem asks, so the two cannot disagree. An artist/album test would look equivalent and would silently zero the playlist estimate, whose generated grid resolves album art through those agents; a test pins that, and reverting the predicate to a whitelist fails it. Found by Codex on b78e67b07. * docs(artwork): trim the comments this branch added to the project budget Six blocks ran past the one-to-two line limit. The LoadPlugins doc was eleven lines over three paragraphs, needsImageAgents spent two of its four explaining an alternative that was rejected, and inspectOpts restated what LoadPlugins already says. What went is reviewer-facing prose that belongs in a commit message: the enumeration of what Start does that this skips, and why an artist/album predicate would have been wrong. What stayed is the reasoning a future reader needs at that line, notably that instantiating a plugin creates its declared services, and that playlists consume the album agent count. * docs(artwork): correct the breaker comment after the recovery ramp It still said a success re-closes the breaker, which stopped being true when closing started requiring breakerRecoveries consecutive answers. * refactor(plugins): rename the scoped-load options to transientLoad inspect claimed the load was only looking, which is false when runInit is true: it instantiates the plugin and runs its init, which may open sockets. transient is accurate for every use of the field, and explains all three behaviours it gates. A load that will not outlive the command has no business persisting findings, instantiating plugins it will never consult, or starting background work it is about to tear down.	2026-08-15 11:01:36 -04:00