From c31746b2dc6e560640e03ad6de351b434c766a4c Mon Sep 17 00:00:00 2001 From: kipavy Date: Wed, 19 Aug 2026 13:37:16 +0000 Subject: [PATCH] fix(plugins): refuse a concurrent install instead of reporting success installPlugin returned silently when the same id was already installing, so every caller read the resolve as a completed install. Accepting a plugin-install deep-link sheet while the Settings tab was installing the same plugin closed the sheet reporting success having written nothing. The second request now throws PluginInstallInProgressError, which pluginInstallErrorMessage names for the user. It is refused rather than joined to the running install: the two callers may have reviewed different manifests for the same id, and the running install cannot stand in for the one this caller consented to. --- src/i18n/locales/en/settings.json | 1 + src/i18n/locales/fr/settings.json | 1 + src/i18n/locales/ru/settings.json | 1 + src/i18n/locales/zh/settings.json | 1 + src/plugins/installErrors.ts | 12 ++++ .../marketplaceStore.installUpdate.test.ts | 57 +++++++++++++++++++ src/stores/marketplaceStore.ts | 6 +- 7 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/i18n/locales/en/settings.json b/src/i18n/locales/en/settings.json index 7f1aeaaf2..f31e9d511 100644 --- a/src/i18n/locales/en/settings.json +++ b/src/i18n/locales/en/settings.json @@ -794,6 +794,7 @@ }, "install": { "integrityFailed": "Plugin bundle failed its integrity check — install blocked.", + "alreadyInstalling": "This plugin is already being installed — check Settings › Plugins.", "failed": "Failed to install plugin.", "versionUnsupported": "This plugin requires app version {{version}} or later." }, diff --git a/src/i18n/locales/fr/settings.json b/src/i18n/locales/fr/settings.json index bfae89cfb..ec1a66fbc 100644 --- a/src/i18n/locales/fr/settings.json +++ b/src/i18n/locales/fr/settings.json @@ -794,6 +794,7 @@ }, "install": { "integrityFailed": "L'intégrité du module a échoué — installation bloquée.", + "alreadyInstalling": "Ce module est déjà en cours d'installation — voir Paramètres › Modules.", "failed": "Échec de l'installation du module.", "versionUnsupported": "Ce module nécessite la version {{version}} ou supérieure de l'application." }, diff --git a/src/i18n/locales/ru/settings.json b/src/i18n/locales/ru/settings.json index b9d961d76..22acb5808 100644 --- a/src/i18n/locales/ru/settings.json +++ b/src/i18n/locales/ru/settings.json @@ -895,6 +895,7 @@ }, "install": { "integrityFailed": "Пакет плагина не прошёл проверку целостности — установка заблокирована.", + "alreadyInstalling": "Этот плагин уже устанавливается — откройте «Настройки › Плагины».", "failed": "Не удалось установить плагин.", "versionUnsupported": "Этот плагин требует версию приложения {{version}} или новее." }, diff --git a/src/i18n/locales/zh/settings.json b/src/i18n/locales/zh/settings.json index e06947402..4e5217cec 100644 --- a/src/i18n/locales/zh/settings.json +++ b/src/i18n/locales/zh/settings.json @@ -614,6 +614,7 @@ }, "install": { "integrityFailed": "插件包未通过完整性校验——已阻止安装。", + "alreadyInstalling": "该插件正在安装中——请查看“设置 › 插件”。", "failed": "安装插件失败。", "versionUnsupported": "此插件需要应用版本 {{version}} 或更高版本。" }, diff --git a/src/plugins/installErrors.ts b/src/plugins/installErrors.ts index 3f54b82f3..052b9f54d 100644 --- a/src/plugins/installErrors.ts +++ b/src/plugins/installErrors.ts @@ -7,6 +7,17 @@ export interface TranslatableMessage { params?: Record; } +/** Raised when an install is requested for an id whose install is still running. + * The second request is refused rather than joined: the two callers may have + * reviewed different manifests for the same id, so the running install cannot + * stand in for the one this caller consented to. */ +export class PluginInstallInProgressError extends Error { + constructor(public readonly id: string) { + super(`An install of "${id}" is already in progress.`); + this.name = "PluginInstallInProgressError"; + } +} + /** * The user-facing message for a failed plugin install, matched on the error's * type rather than its text. A hash mismatch is the user's only tamper signal, @@ -15,6 +26,7 @@ export interface TranslatableMessage { */ export function pluginInstallErrorMessage(e: unknown, fallbackKey: string): TranslatableMessage { if (e instanceof PluginHashMismatchError) return { key: "settings.plugins.install.integrityFailed" }; + if (e instanceof PluginInstallInProgressError) return { key: "settings.plugins.install.alreadyInstalling" }; if (e instanceof MinAppVersionError) return { key: "settings.plugins.install.versionUnsupported", params: { version: e.required } }; return { key: fallbackKey }; diff --git a/src/stores/marketplaceStore.installUpdate.test.ts b/src/stores/marketplaceStore.installUpdate.test.ts index 31b1fe73c..ccb692f81 100644 --- a/src/stores/marketplaceStore.installUpdate.test.ts +++ b/src/stores/marketplaceStore.installUpdate.test.ts @@ -31,6 +31,7 @@ import { getExposedApi, getLoadedPlugins, unloadPlugin } from "@/plugins/runtime import * as runtimeModule from "@/plugins/runtime"; import { injectPluginStyle } from "@/plugins/importPluginModule"; import { PluginHashMismatchError } from "@/plugins/integrity"; +import { PluginInstallInProgressError } from "@/plugins/installErrors"; import { sha256Hex } from "@/plugins/integrity"; import type { PluginRegisterFn } from "@/plugins/api"; @@ -246,3 +247,59 @@ test("installing a plugin that was never loaded does not call unloadPlugin", asy spy.mockRestore(); } }); + +/** Parks installPlugin inside its first plugin_fetch_url until the returned + * release() runs, so a second install can race a genuinely in-flight one. */ +function gatedFetch(): () => void { + let release!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + h.invoke.mockImplementation(async (cmd: string, args: { url?: string }) => { + if (cmd === "plugin_fetch_url") { + await gate; + return args.url!.endsWith("manifest.json") ? manifestFor("1.0.0") : "v1-js"; + } + return undefined; + }); + return release; +} + +test("a second install of an id already installing is refused rather than resolving as a no-op", async () => { + mockBundle("v1"); + const release = gatedFetch(); + + const first = useMarketplaceStore.getState().installPlugin(basePlugin()); + + // A caller that resolves here — the deep-link confirm sheet — would report a + // success for an install it never performed. + await expect( + useMarketplaceStore.getState().installPlugin(basePlugin()), + ).rejects.toBeInstanceOf(PluginInstallInProgressError); + + release(); + await first; + expect(getExposedApi("p1")).toBe("v1"); +}); + +test("a refused concurrent install does not clear the running install's busy state", async () => { + mockBundle("v1"); + const release = gatedFetch(); + + const first = useMarketplaceStore.getState().installPlugin(basePlugin()); + await expect(useMarketplaceStore.getState().installPlugin(basePlugin())).rejects.toThrow(); + + expect(useMarketplaceStore.getState().installing.has("p1")).toBe(true); + release(); + await first; + expect(useMarketplaceStore.getState().installing.has("p1")).toBe(false); +}); + +test("an id is installable again once its previous install has settled", async () => { + mockBundle("v1"); + h.invoke.mockImplementation(async (cmd: string, args: { url?: string }) => { + if (cmd === "plugin_fetch_url") return args.url!.endsWith("manifest.json") ? manifestFor("1.0.0") : "v1-js"; + return undefined; + }); + await useMarketplaceStore.getState().installPlugin(basePlugin()); + + await expect(useMarketplaceStore.getState().installPlugin(basePlugin())).resolves.toBeUndefined(); +}); diff --git a/src/stores/marketplaceStore.ts b/src/stores/marketplaceStore.ts index b51cff45e..599b0dc2e 100644 --- a/src/stores/marketplaceStore.ts +++ b/src/stores/marketplaceStore.ts @@ -9,6 +9,7 @@ import { usePluginRegistryStore } from "@/stores/pluginRegistryStore"; import { appFetch } from "@/services/http"; import { resolveVerifiedHash } from "@/plugins/integrity"; import { assertValidPluginId } from "@/plugins/pluginId"; +import { PluginInstallInProgressError } from "@/plugins/installErrors"; import { satisfiesMinAppVersion, MinAppVersionError, beatsSeededVersion, isParsableVersion } from "@/plugins/version"; import { useSeededTombstoneStore, loadSeededEntries } from "@/stores/seededTombstoneStore"; @@ -349,7 +350,10 @@ export const useMarketplaceStore = create((set, get) => ({ // checked separately by loadPlugin, since the two are not required to match. assertValidPluginId(plugin.id); const { installing, installedMeta } = get(); - if (installing.has(plugin.id)) return; + // Refused, not silently skipped: resolving here told every caller the install + // succeeded — a deep-link confirm sheet accepted while the Settings tab was + // installing the same id closed reporting success having written nothing. + if (installing.has(plugin.id)) throw new PluginInstallInProgressError(plugin.id); set((s) => ({ installing: new Set([...s.installing, plugin.id]) })); try {