mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(marmot): wire the disband fix through to Android
The convergence-based disband landed in `commons` and `quartz` but three things it depends on only exist per front end, and the app had none of them. **Nothing would have carried the pass to settlement.** Terminalization now happens when the convergence pass SETTLES, and the settler is started by inbound traffic that detected a fork — but a disband opens its pass from a local outbound Commit with no fork, so no carrier ever started. On Android the group would have sat in `Recovering` behind its own `Disbanding` gate forever: nothing sendable, never ending. The CLI never showed it because the harness drives settlement explicitly. `disbandGroup` and the regeneration path now start the carrier themselves. **The gate was not durable anywhere real.** Gate storage is defaulted on `MarmotPublishObligationStore` so existing stores keep compiling, and neither `AndroidPublishObligationStore` nor `FilePublishObligationStore` overrode it — so the previous commit's durability claim held only for the in-memory store the test used. Both now persist gates: one file per group beside the obligations. Android writes them unencrypted, unlike an obligation, because the value is one enum name and the filename is a group id the device already stores in the clear — no key material, no message content. `FileStoresGateTest` pins the round trip on the real file store, including that a gate write is not mistaken for an obligation on reload. **The UI announced an ending that may not have happened.** The toast said "Group disbanded" as soon as the call returned, which used to be true because an unacknowledged publish threw. It no longer throws — the request stays durable and pending — so `disbandMarmotGroup` now returns whether the group is terminal, and the screen says "Ending the group" when it is not. Leaving the screen is right either way: the group takes no further outbound work. Verified: quartz 4836, commons 1886, cli 53 green, and the full MDK interop harness is 29/29 at the new 0.9.21 pin with all of this built in — including test 05, whose earlier failure was the loopback relay dropping a websocket before OK rather than anything in 0.9.21. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kCuA6tc4JQzHPCDd39GHq
This commit is contained in:
@@ -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<NormalizedRelayUrl>,
|
||||
) {
|
||||
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
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+68
@@ -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<HexKey, String> =
|
||||
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<ByteArray> =
|
||||
withContext(Dispatchers.IO) {
|
||||
mutex.withLock {
|
||||
|
||||
+5
-2
@@ -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. */
|
||||
|
||||
+19
-3
@@ -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) {
|
||||
|
||||
@@ -2700,6 +2700,7 @@
|
||||
<string name="marmot_encrypted_media_enabled_toast">This group now uses encrypted attachments</string>
|
||||
<string name="marmot_failed_to_enable_encrypted_media">Could not switch this group to encrypted attachments: %1$s</string>
|
||||
<string name="marmot_group_disbanded_toast">Group disbanded</string>
|
||||
<string name="marmot_group_disbanding_toast">Ending the group. It finishes once the group agrees.</string>
|
||||
<string name="marmot_failed_to_disband">Could not disband the group: %1$s</string>
|
||||
<string name="marmot_adding_user">Adding %1$s…</string>
|
||||
<string name="marmot_failed_to_add_user">Failed to add %1$s: %2$s</string>
|
||||
|
||||
@@ -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<HexKey, String> =
|
||||
dir
|
||||
.listFiles { f -> f.isFile && f.name.endsWith(".gate") }
|
||||
?.mapNotNull { file ->
|
||||
runCatching { file.name.removeSuffix(".gate") to file.readText().trim() }.getOrNull()
|
||||
}?.toMap()
|
||||
.orEmpty()
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
+13
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user