diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt index 4d0e130029..86361cadd4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -28,6 +28,7 @@ import com.vitorpamplona.quartz.marmot.appComponents.MarmotWebUrl import com.vitorpamplona.quartz.marmot.appComponents.MessageRetentionV1 import com.vitorpamplona.quartz.marmot.mip00KeyPackages.KeyPackageEvent import com.vitorpamplona.quartz.marmot.mip00KeyPackages.KeyPackageFetcher +import com.vitorpamplona.quartz.marmot.protocolCore.GroupLifecycleState import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl @@ -577,17 +578,28 @@ class AccountMarmotActions( * Deliberately NOT silent on failure the way the other actions here are: a * disband that did not happen must not look like one that did, so the * exception propagates to the caller's error path. + * + * It is no longer terminal the moment it is published, either: the Commit + * is admitted as a convergence candidate and only a SELECTED one moves the + * group to `Disbanded`, so between the two the request sits behind a + * durable `Disbanding` gate. Reporting that distinction is the whole point + * of the return value — announcing "group disbanded" for a request that is + * still pending is the one thing a terminal action must never do. + * + * @return true when the group is terminal now; false when the request is + * durable and unresolved, which is not a failure. */ suspend fun disbandMarmotGroup( nostrGroupId: HexKey, groupRelays: Set, - ) { - val manager = account.marmotManager ?: return - if (!account.isWriteable()) return + ): Boolean { + val manager = account.marmotManager ?: return false + if (!account.isWriteable()) return false manager.disbandGroup(nostrGroupId, groupRelays.toList()) val chatroom = account.marmotGroupList.getOrCreateGroup(nostrGroupId) manager.syncMetadataTo(nostrGroupId, chatroom) + return manager.lifecycle(nostrGroupId) == GroupLifecycleState.DISBANDED } /** diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/marmot/AndroidPublishObligationStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/marmot/AndroidPublishObligationStore.kt index 02baa30a3d..445e91ee91 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/marmot/AndroidPublishObligationStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/marmot/AndroidPublishObligationStore.kt @@ -95,6 +95,74 @@ class AndroidPublishObligationStore( } } + private fun gateDir(): File = File(rootDir, "marmot_gates") + + private fun gateFile(groupId: String) = File(gateDir(), "$groupId.gate") + + /** + * Outbound gates are durable for the same reason obligations are, and the + * `Disbanding` one more than any: the component requires it to survive + * "publication failure, restart, and a losing branch", and Android kills + * apps mid-work routinely. An in-memory gate loses an admin's irreversible + * request to the crash between raising it and a relay taking the Commit, + * and the next launch offers the group as ordinarily live. + * + * Not encrypted, unlike an obligation: the value is one enum name and the + * filename is a group id this device already stores in the clear + * everywhere else. There is no key material and no message content here. + */ + override suspend fun saveGate( + groupId: HexKey, + gate: String, + ) = withContext(Dispatchers.IO) { + mutex.withLock { + val target = gateFile(groupId) + try { + target.parentFile?.mkdirs() + val tmp = File(target.parentFile, "${target.name}.tmp") + tmp.writeBytes(gate.encodeToByteArray()) + if (!tmp.renameTo(target)) { + tmp.copyTo(target, overwrite = true) + if (!tmp.delete()) Log.w(TAG) { "could not delete temp file ${tmp.absolutePath}" } + } + } catch (e: Exception) { + // Same rule as an obligation: failing to record the gate is + // worse than failing to act, because the request is the part + // that cannot be reconstructed. + Log.e(TAG, "saveGate($groupId) FAILED", e) + throw e + } + } + } + + override suspend fun deleteGate(groupId: HexKey) = + withContext(Dispatchers.IO) { + mutex.withLock { + val target = gateFile(groupId) + if (target.exists() && !target.delete()) { + Log.w(TAG) { "could not delete cleared gate ${target.absolutePath}" } + } + Unit + } + } + + override suspend fun loadGates(): Map = + withContext(Dispatchers.IO) { + mutex.withLock { + gateDir() + .listFiles { f -> f.isFile && f.name.endsWith(".gate") } + ?.mapNotNull { file -> + try { + file.name.removeSuffix(".gate") to file.readText().trim() + } catch (e: Exception) { + Log.w(TAG, "could not read gate ${file.absolutePath}", e) + null + } + }?.toMap() + .orEmpty() + } + } + override suspend fun loadAll(): List = withContext(Dispatchers.IO) { mutex.withLock { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 1ae1996e6b..cda7e35f72 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -2533,10 +2533,13 @@ class AccountViewModel( /** * Disband the group for everyone. Irreversible — the caller is responsible * for confirming with the user before this is reached. + * + * @return true when the group is terminal now, false when the request is + * still pending convergence, which is not a failure. */ - suspend fun disbandMarmotGroup(nostrGroupId: String) { + suspend fun disbandMarmotGroup(nostrGroupId: String): Boolean { val relays = account.marmot.marmotGroupRelays(nostrGroupId) - account.marmot.disbandMarmotGroup(nostrGroupId, relays) + return account.marmot.disbandMarmotGroup(nostrGroupId, relays) } /** Set (or, with a blank string, clear) the group's plain-https avatar link. */ diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt index f5e234e722..bab18ae3dd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupInfoScreen.kt @@ -507,11 +507,27 @@ fun MarmotGroupInfoScreen( isDisbanding = true scope.launch(Dispatchers.IO) { try { - accountViewModel.disbandMarmotGroup(nostrGroupId) + // Ended, or ending. A disband terminalizes only once + // convergence SELECTS the Commit, so the request can + // still be pending here — and telling someone their + // conversation is over when it may not be is the one + // wrong answer. Either way the group takes no further + // outbound work, so leaving the screen is right. + val ended = accountViewModel.disbandMarmotGroup(nostrGroupId) launch(Dispatchers.Main) { Toast - .makeText(context, stringRes(context, R.string.marmot_group_disbanded_toast), Toast.LENGTH_SHORT) - .show() + .makeText( + context, + stringRes( + context, + if (ended) { + R.string.marmot_group_disbanded_toast + } else { + R.string.marmot_group_disbanding_toast + }, + ), + Toast.LENGTH_SHORT, + ).show() } nav.nav(Route.Message) } catch (e: Exception) { diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 3d6319a8f5..083066de6a 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -2700,6 +2700,7 @@ This group now uses encrypted attachments Could not switch this group to encrypted attachments: %1$s Group disbanded + Ending the group. It finishes once the group agrees. Could not disband the group: %1$s Adding %1$s… Failed to add %1$s: %2$s diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/FileStores.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/FileStores.kt index c8cfb68416..2898914350 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/FileStores.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/FileStores.kt @@ -312,6 +312,34 @@ class FilePublishObligationStore( ?.sortedBy { it.name } ?.mapNotNull { runCatching { it.readBytes() }.getOrNull() } .orEmpty() + + private fun gateFile(groupId: String) = File(dir, "$groupId.gate") + + /** + * Outbound gates live beside the obligations and are durable for the same + * reason: `Disbanding` must survive "publication failure, restart, and a + * losing branch", and every `amy` verb is its own process — so an + * in-memory gate would not survive even the next command, let alone a + * crash. + */ + override suspend fun saveGate( + groupId: HexKey, + gate: String, + ) { + SecureFileIO.writeBytesAtomic(gateFile(groupId), gate.encodeToByteArray()) + } + + override suspend fun deleteGate(groupId: HexKey) { + gateFile(groupId).deleteOrWarn("FilePublishObligationStore", "outbound gate") + } + + override suspend fun loadGates(): Map = + dir + .listFiles { f -> f.isFile && f.name.endsWith(".gate") } + ?.mapNotNull { file -> + runCatching { file.name.removeSuffix(".gate") to file.readText().trim() }.getOrNull() + }?.toMap() + .orEmpty() } /** diff --git a/cli/src/test/kotlin/com/vitorpamplona/amethyst/cli/FileStoresGateTest.kt b/cli/src/test/kotlin/com/vitorpamplona/amethyst/cli/FileStoresGateTest.kt new file mode 100644 index 0000000000..749e2024b6 --- /dev/null +++ b/cli/src/test/kotlin/com/vitorpamplona/amethyst/cli/FileStoresGateTest.kt @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.cli + +import com.vitorpamplona.amethyst.cli.stores.FilePublishObligationStore +import com.vitorpamplona.quartz.marmot.protocolCore.LocalOutboundGate +import kotlinx.coroutines.runBlocking +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Outbound gates have to reach the disk, not just the map. + * + * `LocalOutboundGate.DISBANDING` is required to survive "publication failure, + * restart, and a losing branch". The gate API is defaulted on the store + * interface so existing implementations keep compiling — which means a store + * that does not override it drops every gate silently, and the group comes back + * after a restart offering itself as ordinarily live with an admin's + * irreversible request forgotten. Every `amy` verb is its own process, so + * "survives a restart" is the ordinary case here rather than a crash scenario. + */ +class FileStoresGateTest { + @get:Rule val tmp = TemporaryFolder() + + private val groupA = "a".repeat(64) + private val groupB = "b".repeat(64) + + @Test + fun `a raised gate is readable by the next process`() = + runBlocking { + val dir = tmp.newFolder("obligations") + FilePublishObligationStore(dir).saveGate(groupA, LocalOutboundGate.DISBANDING.name) + + // A different instance over the same directory is what the next + // `amy` invocation actually does. + val reopened = FilePublishObligationStore(dir).loadGates() + assertEquals(mapOf(groupA to LocalOutboundGate.DISBANDING.name), reopened) + } + + @Test + fun `clearing a gate removes it for good`() = + runBlocking { + val dir = tmp.newFolder("obligations") + val store = FilePublishObligationStore(dir) + store.saveGate(groupA, LocalOutboundGate.DISBANDING.name) + store.deleteGate(groupA) + + assertTrue(FilePublishObligationStore(dir).loadGates().isEmpty()) + } + + @Test + fun `gates are per group and do not disturb obligations`() = + runBlocking { + // One file per group and per obligation, in one directory: a gate + // write must not be mistaken for an obligation on reload, or the + // publish gate would try to decode an enum name as a TLS record. + val dir = tmp.newFolder("obligations") + val store = FilePublishObligationStore(dir) + store.save("f".repeat(64), byteArrayOf(1, 2, 3)) + store.saveGate(groupA, LocalOutboundGate.DISBANDING.name) + store.saveGate(groupB, LocalOutboundGate.LEAVING.name) + + val reopened = FilePublishObligationStore(dir) + assertEquals( + mapOf(groupA to LocalOutboundGate.DISBANDING.name, groupB to LocalOutboundGate.LEAVING.name), + reopened.loadGates(), + ) + assertEquals(1, reopened.loadAll().size) + } +} diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 30c7dc3555..b3cb67e24e 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -2004,6 +2004,16 @@ class MarmotManager( "durable behind the Disbanding gate and retries with the obligation" } } + + // Start the carrier ourselves. Applying this Commit opened a bounded + // convergence pass, and terminalization now happens only when that pass + // SETTLES — but the settler is otherwise started by inbound traffic + // that detected a fork, and this pass has neither. Without this the + // group sits in `Recovering` behind its own `Disbanding` gate + // indefinitely: nothing may be sent to it and it never ends. A front + // end that drives settlement itself (the CLI does) would not notice; + // the app would. + startConvergenceSettler() return publication.event } @@ -2094,6 +2104,9 @@ class MarmotManager( groupRelays(nostrGroupId), ignoringGate = LocalOutboundGate.DISBANDING, ) { groupManager.stageDisband(nostrGroupId) } + // Same reason as in [disbandGroup]: the regenerated Commit opens a + // pass of its own that nothing else would carry to settlement. + startConvergenceSettler() DisbandResolution.PENDING } catch (e: Exception) { Log.w("MarmotManager", "could not regenerate the disband commit for $nostrGroupId", e)