From 20f06f9b9601ea3a2ead0f1d5a2c81d88e227391 Mon Sep 17 00:00:00 2001 From: NexzaDev <235230475+NexzaDev@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:14:09 +0200 Subject: [PATCH 1/2] fix(plugins): keep the previous plugin until its replacement is committed Move the installed plugin to a backup before committing the staged one, restore it on failure, and recover interrupted updates at startup. --- .../xposed/tweaks/plugins/PluginMethods.kt | 1 + .../tweaks/plugins/external/Discovery.kt | 4 + .../app/valenced/plugins/PluginPaths.kt | 3 + .../valenced/plugins/external/Artifacts.kt | 8 +- .../plugins/external/PluginReplacement.kt | 152 +++++++++++ .../plugins/external/ApplyStagedPluginTest.kt | 240 ++++++++++++++++++ 6 files changed, 402 insertions(+), 6 deletions(-) create mode 100644 core/src/commonMain/kotlin/app/valenced/plugins/external/PluginReplacement.kt create mode 100644 core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt diff --git a/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/PluginMethods.kt b/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/PluginMethods.kt index 50a50531..88899019 100644 --- a/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/PluginMethods.kt +++ b/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/PluginMethods.kt @@ -86,6 +86,7 @@ val pluginMethods by tweak { requireValidPluginId(pluginId) // Delete data from all slots. + File(pluginPaths(appInfo.dataDir).backups.toFile(), pluginId).deleteRecursively() File(externalPluginsRoot(appInfo.dataDir), pluginId).deleteRecursively() for (dir in pluginStorageDirsOfAllSlots(appInfo.dataDir, pluginId)) dir.deleteRecursively() PluginStatesStore.removePluginFromAllSlots(pluginPaths(appInfo.dataDir), pluginId) diff --git a/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/external/Discovery.kt b/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/external/Discovery.kt index 9aa00274..f838ba24 100644 --- a/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/external/Discovery.kt +++ b/android/src/main/kotlin/app/valenced/xposed/tweaks/plugins/external/Discovery.kt @@ -8,6 +8,7 @@ import app.valenced.plugins.external.DiscoveryFailure import app.valenced.plugins.external.ExternalManifest import app.valenced.plugins.external.MANIFEST_FILE import app.valenced.plugins.external.ParsedPlugin +import app.valenced.plugins.external.restoreInterruptedReplacements import app.valenced.plugins.external.sessionSkip import app.valenced.plugins.pluginLog import app.valenced.xposed.tweaks.plugins.* @@ -33,6 +34,9 @@ internal fun discoverExternalPlugins( internalManifests: Map, ): ExternalDiscovery { val failures = mutableMapOf() + runCatching { restoreInterruptedReplacements(pluginPaths(dataDir)) } + .onFailure { pluginLog.e("Failed to restore interrupted plugin updates", it) } + val root = externalPluginsRoot(dataDir) if (!root.isDirectory) return ExternalDiscovery(emptyList(), failures) diff --git a/core/src/commonMain/kotlin/app/valenced/plugins/PluginPaths.kt b/core/src/commonMain/kotlin/app/valenced/plugins/PluginPaths.kt index 9b15c9f8..731cc17f 100644 --- a/core/src/commonMain/kotlin/app/valenced/plugins/PluginPaths.kt +++ b/core/src/commonMain/kotlin/app/valenced/plugins/PluginPaths.kt @@ -12,6 +12,7 @@ import okio.Path * ``` * files/valenced/plugins/ * dist// installed plugin files + * backups// previous version while an update is committed * storage// per-slot plugin storage * states/ slot pointers and slot files * repos/ repos.json, sources.json and the index caches @@ -30,6 +31,8 @@ class PluginPaths( /** Installed plugin directories, one per plugin ID. */ val dist: Path = root / "dist" + val backups: Path = root / "backups" + /** Parent of the per-slot storage roots. */ val storage: Path = root / "storage" diff --git a/core/src/commonMain/kotlin/app/valenced/plugins/external/Artifacts.kt b/core/src/commonMain/kotlin/app/valenced/plugins/external/Artifacts.kt index 2302df09..417d4227 100644 --- a/core/src/commonMain/kotlin/app/valenced/plugins/external/Artifacts.kt +++ b/core/src/commonMain/kotlin/app/valenced/plugins/external/Artifacts.kt @@ -141,12 +141,8 @@ private fun BufferedSink.writeCapped(source: Source, budget: Long): Long { * * The running session keeps running the code it already loaded. The new files take effect next boot. */ -fun applyStagedPlugin(paths: PluginPaths, staged: StagedPlugin): Path { - val dest = paths.dist / staged.manifest.id - paths.fs.deleteRecursively(dest, mustExist = false) - paths.fs.atomicMove(staged.dir, dest) - return dest -} +fun applyStagedPlugin(paths: PluginPaths, staged: StagedPlugin): Path = + replaceInstalledPlugin(paths, staged.dir, staged.manifest.id) /** Deletes a staged directory that will never be applied. */ fun discardStagedPlugin(paths: PluginPaths, staged: StagedPlugin) { diff --git a/core/src/commonMain/kotlin/app/valenced/plugins/external/PluginReplacement.kt b/core/src/commonMain/kotlin/app/valenced/plugins/external/PluginReplacement.kt new file mode 100644 index 00000000..bcd09750 --- /dev/null +++ b/core/src/commonMain/kotlin/app/valenced/plugins/external/PluginReplacement.kt @@ -0,0 +1,152 @@ +package app.valenced.plugins.external + +import app.valenced.core.Lock +import app.valenced.plugins.PluginErrorCodes +import app.valenced.plugins.PluginPaths +import app.valenced.plugins.PluginSystemError +import app.valenced.plugins.pluginLog +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import okio.Path + +private val replacementLock = Lock() + +internal fun replaceInstalledPlugin(paths: PluginPaths, stagedDir: Path, id: String): Path = + replacementLock.withLock { PluginReplacement(paths, stagedDir, id).commit() } + +fun restoreInterruptedReplacements(paths: PluginPaths): List = replacementLock.withLock { + val fs = paths.fs + fs.listOrNull(paths.backups).orEmpty().mapNotNull { backup -> + val dest = paths.dist / backup.name + if (fs.exists(dest)) return@mapNotNull null + + runCatching { + fs.createDirectories(paths.dist) + fs.atomicMove(backup, dest) + }.onFailure { + pluginLog.e("Failed to restore '${backup.name}' from $backup", it) + }.map { backup.name }.getOrNull() + } +} + +private class PluginReplacement( + private val paths: PluginPaths, + private val staged: Path, + private val id: String, +) { + private val fs = paths.fs + private val dest = paths.dist / id + private val backup = paths.backups / id + + fun commit(): Path { + fs.createDirectories(paths.dist) + fs.createDirectories(paths.backups) + + recoverInterruptedReplacement() + discardStaleBackup() + + if (fs.exists(dest)) replaceInstalled() else installFresh() + return dest + } + + private fun installFresh() { + try { + fs.atomicMove(staged, dest) + } catch (cause: Throwable) { + throw PluginSystemError(PluginErrorCodes.INSTALL_FAILED, "Could not install '$id'", cause) + } + } + + private fun replaceInstalled() { + setInstalledAside() + moveStagedIntoPlace() + dropBackup() + } + + private fun setInstalledAside() { + try { + fs.atomicMove(dest, backup) + } catch (cause: Throwable) { + if (fs.exists(dest)) throw PluginSystemError( + PluginErrorCodes.INSTALL_FAILED, + "Could not back up the installed '$id'; it was left untouched", + cause, + ) + throw rollback(cause) + } + } + + private fun moveStagedIntoPlace() { + try { + fs.atomicMove(staged, dest) + } catch (cause: Throwable) { + throw rollback(cause) + } + } + + private fun rollback(cause: Throwable): PluginSystemError { + val failure = runCatching { restoreBackup() }.exceptionOrNull() + ?: return PluginSystemError( + PluginErrorCodes.INSTALL_FAILED, + "Could not install '$id'; the previous version was restored", + cause, + ) + + cause.addSuppressed(failure) + return PluginSystemError( + PluginErrorCodes.INSTALL_FAILED, + "Could not install '$id' and the previous version could not be restored automatically. " + + "It is preserved at $backup; move it back to $dest to recover it", + cause, + buildJsonObject { + put("backupPath", backup.toString()) + put("pluginPath", dest.toString()) + }, + ) + } + + private fun restoreBackup() { + check(fs.exists(backup)) { "The backup at $backup is missing" } + fs.deleteRecursively(dest, mustExist = false) + fs.atomicMove(backup, dest) + } + + private fun recoverInterruptedReplacement() { + if (fs.exists(dest) || !fs.exists(backup)) return + + try { + fs.atomicMove(backup, dest) + pluginLog.w("Restored '$id' from an interrupted update") + } catch (cause: Throwable) { + throw PluginSystemError( + PluginErrorCodes.INSTALL_FAILED, + "An earlier update of '$id' was interrupted and its previous version could not be restored. " + + "It is preserved at $backup; move it back to $dest to recover it", + cause, + buildJsonObject { + put("backupPath", backup.toString()) + put("pluginPath", dest.toString()) + }, + ) + } + } + + private fun discardStaleBackup() { + if (!fs.exists(backup)) return + + try { + fs.deleteRecursively(backup) + } catch (cause: Throwable) { + throw PluginSystemError( + PluginErrorCodes.INSTALL_FAILED, + "Could not remove the leftover backup of '$id' at $backup; the installed version was left untouched", + cause, + ) + } + } + + private fun dropBackup() { + runCatching { fs.deleteRecursively(backup, mustExist = false) } + .onFailure { pluginLog.w("Could not remove the backup of '$id' at $backup", it) } + } +} diff --git a/core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt b/core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt new file mode 100644 index 00000000..c01b32fc --- /dev/null +++ b/core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt @@ -0,0 +1,240 @@ +package app.valenced.plugins.external + +import app.valenced.core.readUtf8OrNull +import app.valenced.plugins.PluginErrorCodes +import app.valenced.plugins.PluginPaths +import app.valenced.plugins.PluginSystemError +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import kotlinx.serialization.json.jsonPrimitive +import okio.FileSystem +import okio.ForwardingFileSystem +import okio.IOException +import okio.Path +import okio.Path.Companion.toPath +import okio.fakefilesystem.FakeFileSystem + +private class FaultyFileSystem(delegate: FileSystem) : ForwardingFileSystem(delegate) { + var failMove: (Path, Path) -> Boolean = { _, _ -> false } + var failDelete: (Path) -> Boolean = { false } + + override fun atomicMove(source: Path, target: Path) { + if (failMove(source, target)) throw IOException("injected move failure: $source -> $target") + super.atomicMove(source, target) + } + + override fun deleteRecursively(fileOrDirectory: Path, mustExist: Boolean) { + if (failDelete(fileOrDirectory)) throw IOException("injected delete failure: $fileOrDirectory") + super.deleteRecursively(fileOrDirectory, mustExist) + } +} + +class ApplyStagedPluginTest { + private val fs = FaultyFileSystem(FakeFileSystem()) + private val paths = PluginPaths(fs, "/data".toPath()) + + private fun stage(id: String, content: String): StagedPlugin { + val dir = paths.dist / ".stage-$id" + fs.deleteRecursively(dir, mustExist = false) + fs.createDirectories(dir / "lib") + fs.write(dir / "index.js") { writeUtf8(content) } + fs.write(dir / "lib" / "util.js") { writeUtf8("util-$content") } + return StagedPlugin(dir, ExternalManifest(MANIFEST_FORMAT, id, name = id, version = "1.0.0")) + } + + private fun install(id: String, content: String) = applyStagedPlugin(paths, stage(id, content)) + + private fun installed(id: String) = fs.readUtf8OrNull(paths.dist / id / "index.js") + + private fun backupExists(id: String) = fs.exists(paths.backups / id) + + private fun names(dir: Path) = fs.listOrNull(dir).orEmpty().map { it.name }.sorted() + + private fun installFailure(block: () -> Unit) = assertFailsWith { block() }.also { + assertEquals(PluginErrorCodes.INSTALL_FAILED, it.code) + } + + @Test + fun `a first install places the staged files at the plugin directory`() { + val dest = install("com.example.a", "one") + + assertEquals(paths.dist / "com.example.a", dest) + assertEquals("one", installed("com.example.a")) + assertEquals("util-one", fs.readUtf8OrNull(dest / "lib" / "util.js")) + assertEquals(listOf("com.example.a"), names(paths.dist)) + assertFalse(backupExists("com.example.a")) + } + + @Test + fun `an update replaces every file of the old version`() { + install("com.example.a", "one") + val old = paths.dist / "com.example.a" / "obsolete.js" + fs.write(old) { writeUtf8("old") } + + install("com.example.a", "two") + + assertEquals("two", installed("com.example.a")) + assertFalse(fs.exists(old)) + assertEquals("util-two", fs.readUtf8OrNull(paths.dist / "com.example.a" / "lib" / "util.js")) + assertFalse(backupExists("com.example.a")) + } + + @Test + fun `updating one plugin leaves the others untouched`() { + install("com.example.a", "a1") + install("com.example.b", "b1") + + install("com.example.a", "a2") + + assertEquals("a2", installed("com.example.a")) + assertEquals("b1", installed("com.example.b")) + assertEquals(listOf("com.example.a", "com.example.b"), names(paths.dist)) + } + + @Test + fun `repeated updates leave no temporary directories behind`() { + repeat(5) { install("com.example.a", "v$it") } + + assertEquals("v4", installed("com.example.a")) + assertEquals(listOf("com.example.a"), names(paths.dist)) + assertTrue(names(paths.backups).isEmpty()) + } + + @Test + fun `a failed backup move leaves the old version in place`() { + install("com.example.a", "old") + val staged = stage("com.example.a", "new") + fs.failMove = { _, target -> target == paths.backups / "com.example.a" } + + val error = installFailure { applyStagedPlugin(paths, staged) } + + assertContains(error.message.orEmpty(), "left untouched") + assertEquals("old", installed("com.example.a")) + assertTrue(fs.exists(staged.dir)) + assertFalse(backupExists("com.example.a")) + } + + @Test + fun `a failed commit restores the old version`() { + install("com.example.a", "old") + val staged = stage("com.example.a", "new") + fs.failMove = { source, _ -> source == staged.dir } + + val error = installFailure { applyStagedPlugin(paths, staged) } + + assertContains(error.message.orEmpty(), "previous version was restored") + assertEquals("old", installed("com.example.a")) + assertEquals("util-old", fs.readUtf8OrNull(paths.dist / "com.example.a" / "lib" / "util.js")) + assertFalse(backupExists("com.example.a")) + assertTrue(fs.exists(staged.dir)) + } + + @Test + fun `a failed rollback preserves the backup and reports where it is`() { + install("com.example.a", "old") + val staged = stage("com.example.a", "new") + val backup = paths.backups / "com.example.a" + fs.failMove = { source, target -> source == staged.dir || source == backup && target == paths.dist / "com.example.a" } + + val error = installFailure { applyStagedPlugin(paths, staged) } + + assertContains(error.message.orEmpty(), backup.toString()) + assertEquals(backup.toString(), error.details?.get("backupPath")?.jsonPrimitive?.content) + assertEquals((paths.dist / "com.example.a").toString(), error.details?.get("pluginPath")?.jsonPrimitive?.content) + assertEquals("old", fs.readUtf8OrNull(backup / "index.js")) + assertTrue(error.cause?.suppressed?.isNotEmpty() == true) + } + + @Test + fun `a failed cleanup keeps the new version active`() { + install("com.example.a", "old") + fs.failDelete = { it == paths.backups / "com.example.a" } + + install("com.example.a", "new") + + assertEquals("new", installed("com.example.a")) + assertTrue(backupExists("com.example.a")) + } + + @Test + fun `a leftover backup is cleared by the next update`() { + install("com.example.a", "one") + fs.failDelete = { it == paths.backups / "com.example.a" } + install("com.example.a", "two") + fs.failDelete = { false } + + install("com.example.a", "three") + + assertEquals("three", installed("com.example.a")) + assertFalse(backupExists("com.example.a")) + } + + @Test + fun `a leftover backup that cannot be removed blocks the update without touching the installed version`() { + install("com.example.a", "one") + fs.failDelete = { it == paths.backups / "com.example.a" } + install("com.example.a", "two") + + val error = installFailure { install("com.example.a", "three") } + + assertContains(error.message.orEmpty(), "leftover backup") + assertEquals("two", installed("com.example.a")) + } + + @Test + fun `an interrupted update is recovered before the next one is applied`() { + install("com.example.a", "old") + fs.atomicMove(paths.dist / "com.example.a", paths.backups / "com.example.a") + + install("com.example.a", "new") + + assertEquals("new", installed("com.example.a")) + assertFalse(backupExists("com.example.a")) + } + + @Test + fun `an interrupted update is recovered at startup`() { + install("com.example.a", "old") + install("com.example.b", "b") + fs.atomicMove(paths.dist / "com.example.a", paths.backups / "com.example.a") + + val restored = restoreInterruptedReplacements(paths) + + assertEquals(listOf("com.example.a"), restored) + assertEquals("old", installed("com.example.a")) + assertEquals("b", installed("com.example.b")) + assertFalse(backupExists("com.example.a")) + } + + @Test + fun `startup recovery never overwrites an installed plugin with its backup`() { + install("com.example.a", "one") + fs.failDelete = { it == paths.backups / "com.example.a" } + install("com.example.a", "two") + + val restored = restoreInterruptedReplacements(paths) + + assertTrue(restored.isEmpty()) + assertEquals("two", installed("com.example.a")) + } + + @Test + fun `startup recovery without any backups does nothing`() { + assertTrue(restoreInterruptedReplacements(paths).isEmpty()) + } + + @Test + fun `a failed first install reports an error and leaves no plugin behind`() { + val staged = stage("com.example.a", "new") + fs.failMove = { source, _ -> source == staged.dir } + + installFailure { applyStagedPlugin(paths, staged) } + + assertFalse(fs.exists(paths.dist / "com.example.a")) + assertTrue(fs.exists(staged.dir)) + } +} From ea4f0a2d5b60bc103bfa309ae6982fbfafd0844e Mon Sep 17 00:00:00 2001 From: NexzaDev <235230475+NexzaDev@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:23:26 +0200 Subject: [PATCH 2/2] test: use common suppressedExceptions --- .../app/valenced/plugins/external/ApplyStagedPluginTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt b/core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt index c01b32fc..e275ef90 100644 --- a/core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt +++ b/core/src/commonTest/kotlin/app/valenced/plugins/external/ApplyStagedPluginTest.kt @@ -146,7 +146,7 @@ class ApplyStagedPluginTest { assertEquals(backup.toString(), error.details?.get("backupPath")?.jsonPrimitive?.content) assertEquals((paths.dist / "com.example.a").toString(), error.details?.get("pluginPath")?.jsonPrimitive?.content) assertEquals("old", fs.readUtf8OrNull(backup / "index.js")) - assertTrue(error.cause?.suppressed?.isNotEmpty() == true) + assertTrue(error.cause?.suppressedExceptions?.isNotEmpty() == true) } @Test