From 0f849da77717c3454b4e9442c1d3daec3996d2f8 Mon Sep 17 00:00:00 2001 From: sudacode Date: Tue, 12 May 2026 20:03:02 -0700 Subject: [PATCH] harden bootstrap version load and clean plugin on uninstall - Use pcall for version.lua in bootstrap.lua so missing version module does not crash plugin startup - Remove plugin/subminer from app-data dirs in uninstall-linux and uninstall-macos targets - Add Lua compat test asserting bootstrap uses defensive pcall for version load - Add release-workflow test asserting uninstall targets clean bundled plugin dirs - Delete completed planning document --- Makefile | 6 +- app-managed-mpv-runtime-plugin-plan.md | 381 ------------------------- plugin/subminer/bootstrap.lua | 5 +- scripts/test-plugin-lua-compat.lua | 8 + src/release-workflow.test.ts | 7 + 5 files changed, 23 insertions(+), 384 deletions(-) delete mode 100644 app-managed-mpv-runtime-plugin-plan.md diff --git a/Makefile b/Makefile index 76a85a4a..822a1939 100644 --- a/Makefile +++ b/Makefile @@ -247,13 +247,15 @@ uninstall: uninstall-linux: @rm -f "$(BINDIR)/subminer" "$(BINDIR)/SubMiner.AppImage" @rm -f "$(LINUX_DATA_DIR)/themes/$(THEME_FILE)" - @printf '%s\n' "Removed:" " $(BINDIR)/subminer" " $(BINDIR)/SubMiner.AppImage" " $(LINUX_DATA_DIR)/themes/$(THEME_FILE)" + @rm -rf "$(LINUX_DATA_DIR)/plugin/subminer" + @printf '%s\n' "Removed:" " $(BINDIR)/subminer" " $(BINDIR)/SubMiner.AppImage" " $(LINUX_DATA_DIR)/themes/$(THEME_FILE)" " $(LINUX_DATA_DIR)/plugin/subminer" uninstall-macos: @rm -f "$(BINDIR)/subminer" @rm -f "$(MACOS_DATA_DIR)/themes/$(THEME_FILE)" + @rm -rf "$(MACOS_DATA_DIR)/plugin/subminer" @rm -rf "$(MACOS_APP_DEST)" - @printf '%s\n' "Removed:" " $(BINDIR)/subminer" " $(MACOS_DATA_DIR)/themes/$(THEME_FILE)" " $(MACOS_APP_DEST)" + @printf '%s\n' "Removed:" " $(BINDIR)/subminer" " $(MACOS_DATA_DIR)/themes/$(THEME_FILE)" " $(MACOS_DATA_DIR)/plugin/subminer" " $(MACOS_APP_DEST)" uninstall-windows: @rm -rf "$(MPV_SCRIPTS_DIR)/subminer" diff --git a/app-managed-mpv-runtime-plugin-plan.md b/app-managed-mpv-runtime-plugin-plan.md deleted file mode 100644 index a2d2e1a2..00000000 --- a/app-managed-mpv-runtime-plugin-plan.md +++ /dev/null @@ -1,381 +0,0 @@ -# App-Managed mpv Runtime Plugin Plan - -## Summary - -Shift SubMiner’s default mpv integration from “install and maintain a plugin in the user’s mpv scripts directory” to “inject the bundled plugin at launch time when SubMiner controls mpv.” Existing user-installed plugins remain supported and take precedence. If SubMiner detects an installed plugin, it will not inject the bundled runtime plugin and will show/log a helpful message with the detected path. - -This keeps existing users working, avoids double-loading, and removes the need to update user plugin files for new installs. - -## Goals - -- New users should not need to install the mpv plugin manually for SubMiner-managed mpv launches. -- Existing users with an installed mpv plugin should keep working without behavior changes. -- SubMiner should avoid loading both the installed plugin and bundled runtime plugin in the same mpv session. -- SubMiner should tell users exactly where an installed plugin was detected so they can remove it if they want app-managed runtime loading. -- Windows app-managed mpv launch should support this. -- macOS/Linux launcher-managed mpv launch should support this. - -## Non-Goals - -- Do not remove the current first-run plugin installer yet. -- Do not auto-delete, rename, or overwrite user-installed mpv plugin files. -- Do not force `--load-scripts=no` by default. -- Do not make the runtime plugin override an installed user plugin. -- Do not solve arbitrary already-running mpv sessions without IPC or prior SubMiner launch control. - -## Runtime Policy - -When SubMiner is about to launch mpv: - -1. Detect whether a SubMiner plugin is already installed in mpv’s user scripts directory. -2. If an installed plugin is found: - - Do not pass `--script=`. - - Continue passing SubMiner script options needed by the installed plugin. - - Show/log a warning with the detected installed plugin path and detected version if available. - - Defer to the installed plugin for that mpv session. -3. If no installed plugin is found: - - Pass `--script=`. - - Pass runtime script options for binary path, socket path, log level, and any existing launch metadata. - - Use the plugin bundled with the currently running SubMiner app. - -## Plugin Detection - -Add a shared detector module, likely under `src/shared/` or `src/core/services/`, usable by both Electron main runtime and launcher code. - -Suggested type: - -```ts -export type InstalledMpvPluginSource = - | 'default-config' - | 'xdg-config' - | 'portable-config' - | 'legacy-file'; - -export interface InstalledMpvPluginDetection { - installed: boolean; - path: string | null; - version: string | null; - source: InstalledMpvPluginSource | null; - message: string | null; -} -``` - -Detection candidates: - -Windows: - -```text -%APPDATA%\mpv\scripts\subminer\main.lua -%APPDATA%\mpv\scripts\subminer.lua -%APPDATA%\mpv\scripts\subminer-loader.lua -``` - -If an mpv executable path is known, also check: - -```text -\portable_config\scripts\subminer\main.lua -\portable_config\scripts\subminer.lua -\portable_config\scripts\subminer-loader.lua -``` - -macOS/Linux: - -```text -$XDG_CONFIG_HOME/mpv/scripts/subminer/main.lua -$XDG_CONFIG_HOME/mpv/scripts/subminer.lua -$XDG_CONFIG_HOME/mpv/scripts/subminer-loader.lua -~/.config/mpv/scripts/subminer/main.lua -~/.config/mpv/scripts/subminer.lua -~/.config/mpv/scripts/subminer-loader.lua -``` - -Rules: - -- Return the first existing candidate using the same precedence mpv would most likely use. -- Prefer `subminer/main.lua` over legacy single-file loaders within the same config root. -- Include the absolute detected path in the warning message. -- If multiple plugin candidates are found, log all candidates at debug level, but use the first one as the active detection result. - -## Plugin Versioning - -Add version metadata to the bundled Lua plugin. - -New file: - -```text -plugin/subminer/version.lua -``` - -Contents shape: - -```lua -return { - name = "SubMiner mpv plugin", - version = "0.12.0", -} -``` - -Update `plugin/subminer/init.lua` or `bootstrap.lua` to load this version and expose it in logs. - -Version detection should try: - -1. Read sibling `version.lua` next to detected `main.lua`. -2. Parse a simple `version = "..."` value. -3. If unavailable, return `version: null`. - -For legacy single-file installs, return `version: null` unless a parseable marker is present. - -Important: old installed plugins will not have version metadata, so `null` is expected and should be messaged as “unknown or legacy version,” not treated as an error. - -## Bundled Runtime Plugin Path Resolution - -Add a helper for resolving the packaged plugin entrypoint/directory. - -For Electron main process: - -- Reuse `resolvePackagedFirstRunPluginAssets()` or extract a narrower helper from `src/main/runtime/first-run-setup-plugin.ts`. -- Preferred runtime script path should be the plugin directory if mpv supports directory script loading: - ```text - /plugin/subminer - ``` -- Fallback: - ```text - /plugin/subminer/main.lua - ``` - -For launcher: - -- Accept a resolved runtime plugin path where possible. -- If only `appPath` is available, resolve relative packaged resources using existing app path conventions. -- If runtime plugin path cannot be resolved and no installed plugin exists, fail with a clear message telling the user the packaged mpv plugin assets were not found. - -## Windows Implementation - -Update `src/main/runtime/windows-mpv-launch.ts`. - -Current behavior already supports `pluginEntrypointPath`. - -Change launch preparation so the caller passes either: - -- `pluginEntrypointPath` when no installed plugin is detected. -- `undefined` when an installed plugin is detected. - -Add or update a dependency boundary so detection is testable: - -```ts -detectInstalledMpvPlugin?: () => InstalledMpvPluginDetection; -notifyInstalledPluginDetected?: (detection: InstalledMpvPluginDetection) => void; -``` - -Behavior: - -- Resolve `mpv.exe` first. -- Run installed plugin detection, including portable config checks when `mpv.exe` path is known. -- If installed plugin exists: - - show a non-fatal notification or warning dialog/log entry: - ```text - SubMiner detected an installed mpv plugin at: - - - This mpv session will use the installed plugin. Remove it to use SubMiner's bundled runtime plugin automatically. - ``` - - launch without `--script=`. -- If no installed plugin exists: - - launch with `--script=`. - -Keep current script options: - -```text -subminer-binary_path= -subminer-socket_path= -``` - -## macOS/Linux Launcher Implementation - -Update `launcher/mpv.ts`. - -Affected functions: - -- `startMpv()` -- `launchMpvIdleDetached()` - -Current launcher passes `--script-opts=...` but does not explicitly pass `--script=...`. - -New behavior: - -- Detect installed plugin before building mpv args. -- If installed plugin exists: - - do not add `--script=`. - - keep passing SubMiner script opts. - - log warning with detected path/version. -- If no installed plugin exists: - - add `--script=`. - - keep passing SubMiner script opts. - -Prefer repeated `--script-opt=key=value` in new code where practical, but do not require a full conversion if it risks broad parser churn. If keeping the existing `--script-opts=...` helper, preserve current escaping behavior. - -## User Messaging - -Add user-facing copy in two places: - -1. Runtime warning/log when launching mpv and installed plugin is detected. -2. First-run/setup UI copy update, if applicable, to explain that plugin installation is now optional for normal SubMiner-managed launch. - -Suggested warning: - -```text -SubMiner detected an installed mpv plugin at: - - -This session will use the installed plugin. Remove that plugin to use the bundled runtime plugin that ships with this SubMiner version. -``` - -If version is known: - -```text -Detected plugin version: -Bundled plugin version: -``` - -If version is unknown: - -```text -Detected plugin version: unknown or legacy -``` - -## Compatibility Behavior - -Existing installed plugin: - -- Used as-is. -- Receives script options from the launch command. -- Not modified. - -No installed plugin: - -- Bundled runtime plugin is injected. -- No files are written to mpv config. - -Old installed plugin with no version: - -- Used as-is. -- Warning says version is unknown or legacy. -- User can remove it to switch to runtime injection. - -Portable Windows mpv: - -- If `mpv.exe` path is known and `portable_config` exists beside it, detect installed plugin there before checking `%APPDATA%`. -- If `mpv.exe` path is unknown, only `%APPDATA%` detection is possible. - -## Public Interface / Type Changes - -Add shared detection exports: - -```ts -export interface InstalledMpvPluginDetection { - installed: boolean; - path: string | null; - version: string | null; - source: InstalledMpvPluginSource | null; - message: string | null; -} - -export function detectInstalledMpvPlugin(options: { - platform: NodeJS.Platform; - homeDir: string; - xdgConfigHome?: string; - appDataDir?: string; - mpvExecutablePath?: string; - existsSync?: (path: string) => boolean; - readFileSync?: (path: string, encoding: BufferEncoding) => string; -}): InstalledMpvPluginDetection; -``` - -Add runtime plugin path helper: - -```ts -export function resolvePackagedRuntimePluginPath(options: { - dirname: string; - appPath: string; - resourcesPath: string; - existsSync?: (path: string) => boolean; - joinPath?: (...parts: string[]) => string; -}): string | null; -``` - -Lua plugin version module: - -```text -plugin/subminer/version.lua -``` - -## Tests - -Add detector unit tests covering: - -- Windows `%APPDATA%\mpv\scripts\subminer\main.lua`. -- Windows legacy `subminer.lua`. -- Windows `portable_config` next to known `mpv.exe`. -- Windows portable config precedence over `%APPDATA%`. -- Linux `$XDG_CONFIG_HOME/mpv/scripts/subminer/main.lua`. -- Linux fallback to `~/.config/mpv/scripts/subminer/main.lua`. -- macOS `~/.config/mpv/scripts/subminer/main.lua`. -- No installed plugin. -- Version parsed from `version.lua`. -- Missing version returns `null`. - -Update Windows launch tests: - -- Installed plugin detected means no `--script=`. -- Installed plugin detected still includes `--script-opts` or equivalent script options. -- No installed plugin means `--script=`. -- Warning callback receives detected path. - -Update launcher tests: - -- `startMpv()` includes `--script=` when no installed plugin exists. -- `startMpv()` omits `--script=` when installed plugin exists. -- `launchMpvIdleDetached()` follows the same policy. -- Warning/log path includes detected installed plugin path. - -Lua/plugin tests: - -- Version module loads successfully. -- Bootstrap logs or exposes version without breaking existing plugin startup. - -## Verification - -Minimum targeted verification: - -```text -bun test src/shared/.test.ts -bun test src/main/runtime/windows-mpv-launch.test.ts -bun test launcher/mpv.test.ts launcher/main.test.ts -lua scripts/test-plugin-lua-compat.lua -bun run typecheck -``` - -Broader handoff verification if implementation touches setup or launch wiring substantially: - -```text -bun run test:launcher -bun run test:fast -``` - -## Rollout Notes - -- Keep first-run plugin install available for one release as an optional compatibility path. -- Update docs/release notes to explain: - - SubMiner-managed mpv launch no longer requires plugin installation. - - Existing installed plugin takes precedence. - - Remove the installed plugin to use the bundled runtime plugin. -- Add a `changes/*.md` fragment because this is user-visible behavior. - -## Assumptions and Defaults - -- Default behavior preserves user-installed plugins and does not override them. -- Runtime injection is only guaranteed when SubMiner launches mpv or has IPC access to an existing mpv. -- No automatic deletion or migration of installed plugin files. -- Windows portable mpv detection is implemented only when SubMiner knows the `mpv.exe` path. -- macOS/Linux use default mpv config paths unless future work adds custom config-dir discovery. -- Use the bundled plugin from `process.resourcesPath/plugin/subminer` as the runtime source of truth. diff --git a/plugin/subminer/bootstrap.lua b/plugin/subminer/bootstrap.lua index 6cab605a..992c8b5d 100644 --- a/plugin/subminer/bootstrap.lua +++ b/plugin/subminer/bootstrap.lua @@ -14,7 +14,10 @@ function M.init() local utils = require("mp.utils") local options_helper = require("options") - local version = require("version") + local ok_version, version = pcall(require, "version") + if not ok_version or type(version) ~= "table" then + version = { version = "unknown" } + end local environment = require("environment").create({ mp = mp, utils = utils }) local opts = options_helper.load(options_lib, environment.default_socket_path()) local state = require("state").new() diff --git a/scripts/test-plugin-lua-compat.lua b/scripts/test-plugin-lua-compat.lua index 88cd1d33..e0549176 100644 --- a/scripts/test-plugin-lua-compat.lua +++ b/scripts/test-plugin-lua-compat.lua @@ -1,4 +1,5 @@ local MODULE_PATHS = { + "plugin/subminer/bootstrap.lua", "plugin/subminer/hover.lua", "plugin/subminer/environment.lua", "plugin/subminer/version.lua", @@ -49,6 +50,12 @@ local function assert_loadfile_ok(path) assert_true(chunk ~= nil, "loadfile failed for " .. path .. ": " .. tostring(err)) end +local function assert_bootstrap_uses_defensive_version_load() + local source = read_file("plugin/subminer/bootstrap.lua") + assert_true(not source:find('require%("version"%)'), "bootstrap.lua must not hard-require version.lua") + assert_true(source:find('pcall%(require, "version"%)') ~= nil, "bootstrap.lua must load version.lua with pcall") +end + local function normalize_execute_result(ok, why, code) if type(ok) == "number" then return ok == 0, ok @@ -129,6 +136,7 @@ for _, path in ipairs(MODULE_PATHS) do assert_no_legacy_incompatible_continue(path) assert_loadfile_ok(path) end +assert_bootstrap_uses_defensive_version_load() local parser = find_legacy_parser() if parser then diff --git a/src/release-workflow.test.ts b/src/release-workflow.test.ts index 0b8e9141..3645cc00 100644 --- a/src/release-workflow.test.ts +++ b/src/release-workflow.test.ts @@ -187,3 +187,10 @@ test('Makefile does not expose the legacy global mpv plugin installer', () => { assert.doesNotMatch(makefile, /\binstall-plugin\b/); assert.doesNotMatch(makefile, /configure-plugin-binary-path\.mjs/); }); + +test('Makefile uninstall targets remove bundled runtime plugin app-data copies', () => { + assert.match(makefile, /uninstall-linux:[\s\S]*@rm -rf "\$\(LINUX_DATA_DIR\)\/plugin\/subminer"/); + assert.match(makefile, /uninstall-macos:[\s\S]*@rm -rf "\$\(MACOS_DATA_DIR\)\/plugin\/subminer"/); + assert.match(makefile, /Removed:[\s\S]*\$\(LINUX_DATA_DIR\)\/plugin\/subminer/); + assert.match(makefile, /Removed:[\s\S]*\$\(MACOS_DATA_DIR\)\/plugin\/subminer/); +});