From a4b83b86e77e97b8bd4f5b1cf42f3a6f50df8226 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 15:31:21 +0000 Subject: [PATCH 01/16] fix: clear compiler warnings in quartz, commons, marmotBench and quic-interop Fixes the Kotlin warnings the compiler reports for these modules across the jvm, android, linuxX64 and metadata compilations: - PartialTokensTest / marmotBench: drop redundant casts. kotlin.test's assertTrue carries a `returns() implies` contract, so the `is` check already smart-casts; the benchmark values were never nullable-typed. - MarmotPublish*Test, LastResortKeyPackageReuseTest: name overridden parameters as the supertype does (`retainedSecrets`, `snapshot`), so named-argument calls through the interface stay correct. - AuthOutcomeTest: PersistentMap.put is deprecated in favour of putting(), which is what the rest of the codebase already uses. - IndexableContentGoldenTest: drop an unnecessary !! on a non-null String. - Nip46Test: the generic encode/decode round trip cannot be checked at runtime, so suppress UNCHECKED_CAST with a note on why it is safe. - InternTradeoffBenchmark: hoist the liveness anchor from a local to a field. As a local its assignments were visible to data flow, which folded the trailing `check(sink != null)` into a constant. - Http3GetClient: an empty `else -> {}` branch instead of a bare `Unit` expression statement. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0123kXtseu4X18hL3GMDcdER --- .../amethyst/commons/search/PartialTokensTest.kt | 4 ++-- .../commons/marmot/MarmotPublishBeforeApplyTest.kt | 4 ++-- .../commons/marmot/MarmotPublishDurabilityTest.kt | 4 ++-- .../com/vitorpamplona/marmotbench/MarmotBenchmarks.kt | 6 ++---- .../quartz/nip46RemoteSigner/Nip46Test.kt | 4 ++++ .../nip01Core/relay/client/auth/AuthOutcomeTest.kt | 2 +- .../mip00KeyPackages/LastResortKeyPackageReuseTest.kt | 4 ++-- .../nip01Core/prodbench/InternTradeoffBenchmark.kt | 10 ++++++++-- .../quartz/nip50Search/IndexableContentGoldenTest.kt | 2 +- .../quic/interop/runner/Http3GetClient.kt | 4 +--- 10 files changed, 25 insertions(+), 19 deletions(-) diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/search/PartialTokensTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/search/PartialTokensTest.kt index acaca77d15..c631249285 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/search/PartialTokensTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/search/PartialTokensTest.kt @@ -38,7 +38,7 @@ class PartialTokensTest { fun aHalfWrittenFromOpensThePeoplePicker() { val picker = pickerAtEnd("zaps from:ali") assertTrue(picker is ActivePicker.People) - assertEquals(KeyField.FROM, (picker as ActivePicker.People).keyField) + assertEquals(KeyField.FROM, picker.keyField) assertEquals("ali", picker.token.partial) assertEquals(5, picker.token.start) } @@ -65,7 +65,7 @@ class PartialTokensTest { fun aHalfWrittenDateOpensTheCalendar() { val picker = pickerAtEnd("since:2026-0") assertTrue(picker is ActivePicker.Calendar) - assertEquals(DateField.SINCE, (picker as ActivePicker.Calendar).dateField) + assertEquals(DateField.SINCE, picker.dateField) } @Test diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishBeforeApplyTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishBeforeApplyTest.kt index d84c56ca69..c63f506200 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishBeforeApplyTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishBeforeApplyTest.kt @@ -461,9 +461,9 @@ class MarmotPublishBeforeApplyTest { override suspend fun saveRetainedEpochs( nostrGroupId: String, - epochs: List, + retainedSecrets: List, ) { - retained[nostrGroupId] = epochs + retained[nostrGroupId] = retainedSecrets } override suspend fun loadRetainedEpochs(nostrGroupId: String): List = retained[nostrGroupId] ?: emptyList() diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishDurabilityTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishDurabilityTest.kt index e85afc40ca..e06891794c 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishDurabilityTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotPublishDurabilityTest.kt @@ -105,9 +105,9 @@ class MarmotPublishDurabilityTest { override suspend fun saveRetainedEpochs( nostrGroupId: String, - epochs: List, + retainedSecrets: List, ) { - retained[nostrGroupId] = epochs + retained[nostrGroupId] = retainedSecrets } override suspend fun loadRetainedEpochs(nostrGroupId: String): List = retained[nostrGroupId].orEmpty() diff --git a/marmotBench/src/main/kotlin/com/vitorpamplona/marmotbench/MarmotBenchmarks.kt b/marmotBench/src/main/kotlin/com/vitorpamplona/marmotbench/MarmotBenchmarks.kt index bf5f68f7fe..7a030c58d4 100644 --- a/marmotBench/src/main/kotlin/com/vitorpamplona/marmotbench/MarmotBenchmarks.kt +++ b/marmotBench/src/main/kotlin/com/vitorpamplona/marmotbench/MarmotBenchmarks.kt @@ -24,12 +24,10 @@ import com.vitorpamplona.amethyst.commons.marmot.MarmotManager import com.vitorpamplona.amethyst.commons.marmot.ingest import com.vitorpamplona.quartz.marmot.appComponents.GroupProfileV1 import com.vitorpamplona.quartz.marmot.mip00KeyPackages.KeyPackageEvent -import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEvent import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal -import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent import com.vitorpamplona.quartz.utils.RandomInstance import kotlinx.coroutines.runBlocking @@ -170,7 +168,7 @@ fun benchJoinWelcome(): BenchResult = } }, ) { (bob, wrap) -> - runBlocking { bob.manager.ingest(wrap as GiftWrapEvent) } + runBlocking { bob.manager.ingest(wrap) } } /** @@ -223,7 +221,7 @@ fun benchIngestAppMessage(members: Int): BenchResult = } }, ) { (bob, event) -> - runBlocking { bob.manager.ingest(event as GroupEvent) } + runBlocking { bob.manager.ingest(event) } } private const val BENCH_RELAY = "wss://bench.invalid" diff --git a/quartz/src/androidDeviceTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/Nip46Test.kt b/quartz/src/androidDeviceTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/Nip46Test.kt index 33adaa8851..57c0aa4b8b 100644 --- a/quartz/src/androidDeviceTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/Nip46Test.kt +++ b/quartz/src/androidDeviceTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/Nip46Test.kt @@ -56,6 +56,10 @@ internal class Nip46Test { sig = "ec39e60722a083cccbd2d82d2827e13f5499fa7cbcedac5b76011a844c077473adb629d50d01fab147835ac6c8a3d5ba9aaddd87d6723f0c3c864b9119fc4356", ) + // The round trip is only type-safe by construction: the caller hands in a T, + // the message is encoded and decoded, and the decoder returns the same + // BunkerMessage subtype. The runtime has no way to check that. + @Suppress("UNCHECKED_CAST") suspend fun encodeDecodeEvent(req: T): T { val eventStr = NostrConnectEvent.create(req, remoteKey.pubKey, signer).toJson() diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/auth/AuthOutcomeTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/auth/AuthOutcomeTest.kt index 192b013387..af50790016 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/auth/AuthOutcomeTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/auth/AuthOutcomeTest.kt @@ -61,7 +61,7 @@ class AuthOutcomeTest { phase: RelayAuthSnapshot.Phase, successCount: Int = 0, ) { - state.value = state.value.put(relay, RelayAuthSnapshot(phase, null, successCount)) + state.value = state.value.putting(relay, RelayAuthSnapshot(phase, null, successCount)) } } diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/LastResortKeyPackageReuseTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/LastResortKeyPackageReuseTest.kt index b9feeeca5a..403aa6e217 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/LastResortKeyPackageReuseTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/LastResortKeyPackageReuseTest.kt @@ -245,8 +245,8 @@ class LastResortKeyPackageReuseTest { override suspend fun load(): ByteArray? = bytes - override suspend fun save(data: ByteArray) { - bytes = data + override suspend fun save(snapshot: ByteArray) { + bytes = snapshot } override suspend fun delete() { diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/prodbench/InternTradeoffBenchmark.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/prodbench/InternTradeoffBenchmark.kt index 9095a17bcf..7a2992d0ce 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/prodbench/InternTradeoffBenchmark.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/prodbench/InternTradeoffBenchmark.kt @@ -110,10 +110,16 @@ class InternTradeoffBenchmark { } } + /** + * Holds the last measured corpus so the JIT cannot drop the allocations we + * just paid for. A field rather than a local: a local's assignments are + * visible to the compiler's data flow, which then folds the `check` below + * into a constant. + */ + private var sink: Any? = null + @Test fun internCostAndBenefit() { - var sink: Any? = null - println("\n=== corpus: $EVENTS events, $AUTHORS authors, $RELAYS relays ===") // ---- memory ---- diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip50Search/IndexableContentGoldenTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip50Search/IndexableContentGoldenTest.kt index 8f44b686cb..8cfa6fef3c 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip50Search/IndexableContentGoldenTest.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip50Search/IndexableContentGoldenTest.kt @@ -131,7 +131,7 @@ class IndexableContentGoldenTest { } } val rejoined = visited.joinToString(event.indexableSeparator()) - if (rejoined == event.indexableContent()) null else "kind $kind: visitor=${rejoined.take(120)!!} content=${event.indexableContent().take(120)}" + if (rejoined == event.indexableContent()) null else "kind $kind: visitor=${rejoined.take(120)} content=${event.indexableContent().take(120)}" } assertEquals("visitor and indexed content disagree", emptyList(), disagreements) } diff --git a/quic/interop/src/main/kotlin/com/vitorpamplona/quic/interop/runner/Http3GetClient.kt b/quic/interop/src/main/kotlin/com/vitorpamplona/quic/interop/runner/Http3GetClient.kt index c1a20bed55..929b2ef6c4 100644 --- a/quic/interop/src/main/kotlin/com/vitorpamplona/quic/interop/runner/Http3GetClient.kt +++ b/quic/interop/src/main/kotlin/com/vitorpamplona/quic/interop/runner/Http3GetClient.kt @@ -203,9 +203,7 @@ class Http3GetClient( body += frame.body } - else -> { - Unit - } + else -> {} } } } From f6a49761f5e702df5cbb54bc84d39e7462b6986d Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 12 Sep 2026 13:54:15 +0200 Subject: [PATCH 02/16] Update import_eq_suggestions to true to avoid tedious updating in UI of identical strings. update cs,pt,de,sv --- .github/workflows/crowdin.yml | 8 ++ .../src/main/res/values-sv-rSE/strings.xml | 1 + .../composeResources/values-cs/strings.xml | 62 ++++++++++ .../values-de-rDE/strings.xml | 110 ++++++++++++++++++ .../values-pt-rBR/strings.xml | 78 +++++++++++++ .../values-sv-rSE/strings.xml | 76 ++++++++++++ 6 files changed, 335 insertions(+) diff --git a/.github/workflows/crowdin.yml b/.github/workflows/crowdin.yml index c56009804c..04304cf699 100644 --- a/.github/workflows/crowdin.yml +++ b/.github/workflows/crowdin.yml @@ -31,6 +31,14 @@ jobs: with: upload_sources: true upload_translations: true + # Upload translations that are identical to the English source (brand + # terms, loanwords like "Feed"/"Apps", bare formats like "v%1$s"). + # Without this they are SKIPPED on upload, so a locale that deliberately + # keeps English never reaches Crowdin's DB and the key keeps coming back + # as untranslated. They arrive as normal UNAPPROVED translations -- + # auto_approve_imported stays at its default false, so a translator still + # approves them in the Crowdin UI (bulk-select in the Editor). + import_eq_suggestions: true download_translations: true # Let the downloaded translations stay in the working tree; the single # create-pull-request step below opens the combined PR. diff --git a/amethyst/src/main/res/values-sv-rSE/strings.xml b/amethyst/src/main/res/values-sv-rSE/strings.xml index d6fab46b87..1a66e9ba1f 100644 --- a/amethyst/src/main/res/values-sv-rSE/strings.xml +++ b/amethyst/src/main/res/values-sv-rSE/strings.xml @@ -2387,4 +2387,5 @@ + Media diff --git a/commons/src/commonMain/composeResources/values-cs/strings.xml b/commons/src/commonMain/composeResources/values-cs/strings.xml index 5650182830..f208838430 100644 --- a/commons/src/commonMain/composeResources/values-cs/strings.xml +++ b/commons/src/commonMain/composeResources/values-cs/strings.xml @@ -2759,4 +2759,66 @@ Publikování… Publikovat personu Sleduje vás + Hashtag + + Emoji + NIP-%1$s + OK + Role + %1$s: %2$s + Workflow: %1$s + Workflow + Mint: %1$s + ✓ %1$s + iPhone 13 + +%1$d + Offline + OK + <%1$s + Gif + Commit + Text + 100000 + https://example.com/image.jpg + https://example.com + H.264 + %1$s → %2$s + OFFLINE + https://example.com/avatar.png + %1$s… + Ostrich McAwesome + Chat + %1$s + Ping + v%1$s + N/A + Nutzap + %1$s sat/vB · %2$s + %1$s sats + sats + Android + iOS + Web + Ep %1$d + Editor + S%1$d · E%2$d + %1$d sats/min + Video + Bot + Boost + Zap + limit %1$d + %1$d+ + %1$s sats + 😎 + ∞ + Cashu + On-chain + %1$d/%2$d + Tip + Auto + sats + nostr, tech, blog + URL + https://example.com + %1$s km diff --git a/commons/src/commonMain/composeResources/values-de-rDE/strings.xml b/commons/src/commonMain/composeResources/values-de-rDE/strings.xml index 0adbb765bd..b291e32aec 100644 --- a/commons/src/commonMain/composeResources/values-de-rDE/strings.xml +++ b/commons/src/commonMain/composeResources/values-de-rDE/strings.xml @@ -2697,4 +2697,114 @@ Wird veröffentlicht… Persona veröffentlichen Folgt dir + Hashtag + + Emoji + App + NIP-%1$s + Name + Canvas (Markdown) + Canvas + Workspace + OK + Workspace + Backlog + %1$s: %2$s + Name + Workflow: %1$s + Workflow + RSVPs (%1$d) + ✓ %1$s + Mints + Mints: %1$s + Thread + iPhone 13 + Budget + Name + Inline + +%1$d + Offline + OK + DMs + Inbox + <%1$s + Outbox + Relays: %1$d / %2$d + Feed + Fork + Gif + Branch + Commit + Branches + Commits + default + Text + Name + Branches + Tags + Code + Tags + 100000 + https://example.com/image.jpg + H.264 + Codec + %1$s → %2$s + LIVE + OFFLINE + Malware + https://example.com/avatar.png + Relays + %1$s… + Album (optional) + Ostrich McAwesome + LIVE + Moderator + Chat + Relay + %1$s + Ping + Live + v%1$s + N/A + Nutzap + %1$s sat/vB · %2$s + %1$s sats + sats + Android + iOS + Web + Podcasts + https://…/episode.mp3 + S%1$d · E%2$d + Trailer + Name (optional) + Video + 👀 + Apps · %1$d + Apps + Bot + Boost + Zap + LIVE + Name + %1$d+ + Moderator + Threads + Inline + Relays + Relays + 😎 + ∞ + Cashu + Lightning + %1$d/%2$d + # Tags + Version + Version %1$s + Auto + Zaps + nostr, tech, blog + URL + Website + Workout + %1$s km diff --git a/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml b/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml index ca6571f850..96ce9845e7 100644 --- a/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml +++ b/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml @@ -2729,4 +2729,82 @@ Publicando… Publicar persona Segue você + Hashtag + + Emoji + %1$s bits + NIP-%1$s + Canvas (Markdown) + Canvas + OK + Backlog + %1$s: %2$s + Mint: %1$s + ✓ %1$s + Mints + Mints: %1$s + iPhone 13 + +%1$d + Offline + %1$d emojis + OK + DMs + <%1$s + Feed + Gif + Branch + Commit + Branches + Commits + Branches + Tags + Tags + 100000 + H.264 + Codec + %1$d hashtag(s) + %1$s → %2$s + Malware + https://example.com/avatar.png + %1$s… + %1$s + Ping + Links + v%1$s + N/A + Nutzap + %1$s sat/vB · %2$s + %1$s sats + sats + original + Picture-in-Picture + Android + iOS + Web + Podcasts + Editor + Trailer + %1$d sats/min + 👀 + Apps · %1$d + Bot + Boost + Zap + %1$d+ + %1$s sats + Local + 😎 + ∞ + Cashu + Lightning + On-chain + %1$d/%2$d + # Tags + Picture-in-Picture + Auto + Zaps + sats + nostr, tech, blog + URL + %1$s km + Volume diff --git a/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml b/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml index dc9baa2062..f987afd543 100644 --- a/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml +++ b/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml @@ -2731,4 +2731,80 @@ Publicerar… Publicera persona Följer dig + Hashtag + + Emoji + App + NIP-%1$s + via %1$s + Banner URL + Canvas (Markdown) + Canvas + OK + %1$s: %2$s + Mint: %1$s + ✓ %1$s + Mints + Mints: %1$s + iPhone 13 + Budget + +%1$d + Offline + %1$d emojis + <%1$s + Gif + Commit + Commits + Text + 100000 + https://example.com/image.jpg + H.264 + Codec + %1$s → %2$s + LIVE + OFFLINE + https://example.com/avatar.png + %1$s… + Artist + LIVE + Moderator + %1$s + Ping + Live + v%1$s + N/A + Nutzap + %1$s sat/vB · %2$s + %1$s sats + sats + original + Android + iOS + Web + Explicit + Trailer + 02abc… (33-byte hex) + %1$d sats/min + Video + 👀 + Bot + Zap + LIVE + %1$d+ + Moderator + %1$s sats + 😎 + ∞ + Cashu + Lightning + On-chain + %1$d/%2$d + Version + Version %1$s + Auto + CLINK Debit + Zaps + sats + nostr, tech, blog + URL + %1$s km From d63e14bb36acd789f9ffe24a10b5e64a09bd797c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 16:17:00 +0000 Subject: [PATCH 03/16] fix: clear the remaining compiler warnings in amethyst and desktopApp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second half of the warning sweep, covering :amethyst (both flavors, all build types, unit + instrumented tests), :desktopApp and :benchmark: - GitReplyEvent (NIP-34 kind 1622) is deprecated in favour of NIP-22 comments, but events already on relays still arrive and still have to be routed, rendered and surfaced. @Suppress("DEPRECATION") with that reason at the five production sites and the coverage test that pins them. - The four instrumented Compose tests move to androidx.compose.ui.test.junit4.v2.createComposeRule. The v2 factory returns the same ComposeContentTestRule, so mainClock, setContent and the node assertions are unchanged; only the effect dispatcher differs. - RelayAuthPromptBusTest / RelayAuthSessionGrantsTest: @OptIn for the ExperimentalCoroutinesApi members (testScheduler.currentTime, runCurrent) they already use, matching the annotation the file's other tests carry. - MarmotFileUploader: drop a nullable alias of a non-null cipher, left behind when the v2 reference stopped being conditional. - LocalCacheSearchParityTest: hoist the Json format out of the loader. - LivesSection: FlowRowOverflow and FlowRow's overflow parameter are deprecated; the non-deprecated overload already clips beyond maxLines. - HexBenchmark: drop a bare `null` expression statement from the measured lambda. Also fixes :commons:compileCommonMainKotlinMetadata, which did not compile at all: shared code called BigDecimal.toLong(), which resolves in every platform compilation (every actual is a Number) but not in the common metadata one, where only the expect class's own members are visible. `expect class BigDecimal : Number` cannot work — java.math.BigDecimal leaves toByte()/toShort() abstract, so the JVM typealias fails the expect/actual modality check — so the conversion is a top-level expect/actual extension instead, with actuals next to each BigDecimal actual. Verified warning- and error-free across the jvm, android (play/fdroid × debug/release/benchmark), linuxX64, and the common/jvmAndroid/native/apple/ ios metadata compilations. The apple actuals are checked by compileAppleMainKotlinMetadata, which runs the frontend against the Apple klibs without needing a macOS host. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0123kXtseu4X18hL3GMDcdER --- .../composable/PlaybackErrorOverlayFitTest.kt | 5 ++-- .../components/AudioPlayerBoxOverflowTest.kt | 5 ++-- .../ui/insets/ComposeImeInsetWedgeTest.kt | 5 ++-- .../amethyst/ui/note/DeferredAnimationTest.kt | 2 +- .../EventNotificationConsumer.kt | 3 ++ .../notifications/NotificationDispatcher.kt | 3 ++ .../renderers/CodeNotification.kt | 3 ++ .../marmotGroup/send/MarmotFileUploader.kt | 29 +++++++++---------- .../dal/NotificationFeedFilter.kt | 6 ++++ .../model/LocalCacheSearchParityTest.kt | 5 +++- .../model/RelayAuthPromptBusTest.kt | 1 + .../model/RelayAuthSessionGrantsTest.kt | 3 ++ .../dal/Nip34NotificationCoverageTest.kt | 2 ++ .../quartz/benchmark/HexBenchmark.kt | 2 +- .../LiveStreamTopZappersViewModel.kt | 5 ++-- .../commons/viewmodels/RoomZapsState.kt | 3 +- .../amethyst/desktop/ui/live/LivesSection.kt | 2 -- .../quartz/utils/BigDecimalOps.apple.kt | 23 +++++++++++++++ .../quartz/marmot/MarmotInboundProcessor.kt | 2 +- .../protocolCore/MarmotConvergenceEngine.kt | 4 +-- .../quartz/utils/BigDecimalOps.kt | 12 ++++++++ .../quartz/utils/BigDecimalOps.jvmAndroid.kt | 23 +++++++++++++++ .../quartz/utils/BigDecimalOps.linux.kt | 23 +++++++++++++++ 23 files changed, 138 insertions(+), 33 deletions(-) create mode 100644 quartz/src/appleMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.apple.kt create mode 100644 quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.jvmAndroid.kt create mode 100644 quartz/src/linuxMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.linux.kt diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt index 5ea16ce0b6..6c2e86b123 100644 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt @@ -31,7 +31,7 @@ import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.test.assertHeightIsAtLeast import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.getUnclippedBoundsInRoot -import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.unit.Density import androidx.compose.ui.unit.Dp @@ -62,7 +62,8 @@ import org.junit.runner.RunWith */ @RunWith(AndroidJUnit4::class) class PlaybackErrorOverlayFitTest { - @get:Rule val rule = createComposeRule() + @get:Rule + val rule = createComposeRule() private val targetContext = InstrumentationRegistry.getInstrumentation().targetContext diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt index 564715b2f9..c9f738a88b 100644 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt @@ -27,7 +27,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.layout.onGloballyPositioned import androidx.compose.ui.layout.positionInRoot -import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.unit.dp import androidx.test.ext.junit.runners.AndroidJUnit4 import com.vitorpamplona.amethyst.service.playback.composable.audioSquare @@ -50,7 +50,8 @@ import org.junit.runner.RunWith */ @RunWith(AndroidJUnit4::class) class AudioPlayerBoxOverflowTest { - @get:Rule val rule = createComposeRule() + @get:Rule + val rule = createComposeRule() private class Bounds { var top = 0f diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/insets/ComposeImeInsetWedgeTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/insets/ComposeImeInsetWedgeTest.kt index 54a1cf4624..b6ef4068fd 100644 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/insets/ComposeImeInsetWedgeTest.kt +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/insets/ComposeImeInsetWedgeTest.kt @@ -31,7 +31,7 @@ import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.setValue import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalView -import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.core.graphics.Insets import androidx.core.view.OnApplyWindowInsetsListener import androidx.core.view.WindowInsetsAnimationCompat @@ -70,7 +70,8 @@ import org.junit.Test * fallback would silently start reading a dead value too. */ class ComposeImeInsetWedgeTest { - @get:Rule val rule = createComposeRule() + @get:Rule + val rule = createComposeRule() private val keyboardHeight = 957 diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/note/DeferredAnimationTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/note/DeferredAnimationTest.kt index 12b6892a0f..a2cbb273e7 100644 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/note/DeferredAnimationTest.kt +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/note/DeferredAnimationTest.kt @@ -27,7 +27,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.testTag import androidx.compose.ui.test.assertIsDisplayed -import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.test.ext.junit.runners.AndroidJUnit4 import com.vitorpamplona.amethyst.ui.actions.DeferredCrossfade diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt index a7ac9a4d4c..f28d53b3a8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/EventNotificationConsumer.kt @@ -201,6 +201,9 @@ class EventNotificationConsumer( .onFailure { Log.d(TAG) { "Skipping non-decodable npub $npub: ${it.message}" } } .getOrNull() + // GitReplyEvent (kind 1622) is deprecated in favour of NIP-22 comments, but + // events already on relays still arrive and still have to be routed. + @Suppress("DEPRECATION") private suspend fun dispatchForAccount( event: Event, account: Account, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt index 6c66b6dc6b..41c4619596 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationDispatcher.kt @@ -112,6 +112,9 @@ class NotificationDispatcher( // recipient account. // `internal` (was `private`) so the notification-kinds contract test // can pin the push-side kind set against the in-app feed's kind set. + // GitReplyEvent (kind 1622) is deprecated in favour of NIP-22 comments, but + // events already on relays still arrive and still have to be routed. + @Suppress("DEPRECATION") internal val NOTIFICATION_KINDS: Set = setOf( // Direct-arrival diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt index 1c8b8196b6..086f230ac7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/renderers/CodeNotification.kt @@ -81,6 +81,9 @@ object CodeNotification { event: GitPullRequestUpdateEvent, ) = post(context, account, event.id, event.createdAt, event.pubKey, R.string.app_notification_code_channel_message_pr_update, event.content) + // GitReplyEvent (kind 1622) is deprecated in favour of NIP-22 comments, but + // events already on relays still arrive and still have to be rendered. + @Suppress("DEPRECATION") suspend fun notify( context: Context, account: Account, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotFileUploader.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotFileUploader.kt index ad768a3be6..81ba7c19cc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotFileUploader.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/send/MarmotFileUploader.kt @@ -119,7 +119,6 @@ class MarmotFileUploader( // imprecisely. val canonicalMediaType = MarmotMediaType.canonicalize(mimeType) ?: GENERIC_MEDIA_TYPE val cipher = EncryptedMediaV2Cipher(exporterSecret, canonicalMediaType, filename) - val v2Cipher = cipher item.orchestrator.uploadEncrypted( uri = media.uri, @@ -142,21 +141,19 @@ class MarmotFileUploader( // compression and metadata stripping — because that is what the // key was derived from. val reference = - v2Cipher?.let { - EncryptedMediaReferenceV2( - locators = - listOf( - MediaLocatorV2(EncryptedMediaPolicyV2.INITIAL_LOCATOR_KIND, serverResult.url), - ), - ciphertextSha256 = it.ciphertextSha256, - plaintextSha256 = it.plaintextSha256, - nonce = it.nonce, - mediaType = it.mediaType, - filename = filename, - dim = serverResult.fileHeader.dim?.toString(), - thumbhash = serverResult.fileHeader.thumbHash?.thumbhash, - ) - } + EncryptedMediaReferenceV2( + locators = + listOf( + MediaLocatorV2(EncryptedMediaPolicyV2.INITIAL_LOCATOR_KIND, serverResult.url), + ), + ciphertextSha256 = cipher.ciphertextSha256, + plaintextSha256 = cipher.plaintextSha256, + nonce = cipher.nonce, + mediaType = cipher.mediaType, + filename = filename, + dim = serverResult.fileHeader.dim?.toString(), + thumbhash = serverResult.fileHeader.thumbHash?.thumbhash, + ) results.add( Mip04UploadResult( url = serverResult.url, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt index 750043a37c..cff84854d6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt @@ -139,6 +139,9 @@ class NotificationFeedFilter( AttestationRequestEvent.KIND, ) + // GitReplyEvent (kind 1622) is deprecated in favour of NIP-22 comments, but + // events already on relays still arrive and still have to be routed. + @Suppress("DEPRECATION") val NOTIFICATION_KINDS = // Kinds that RENDER as a row on the Notifications tab. This is a // display gate over whatever is already in LocalCache — it plays no @@ -268,6 +271,9 @@ class NotificationFeedFilter( // Shared with EventNotificationConsumer so push notifications and the // in-app feed apply the same per-kind "is this event for me" rule. + // GitReplyEvent (kind 1622) is deprecated in favour of NIP-22 comments, but + // events already on relays still arrive and still have to be routed. + @Suppress("DEPRECATION") fun tagsAnEventByUser( note: Note, authorHex: HexKey, diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt index c0bfc3a5c5..865f6c1457 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/LocalCacheSearchParityTest.kt @@ -47,6 +47,9 @@ import java.io.File */ class LocalCacheSearchParityTest { companion object { + /** Hoisted out of [loadCorpus]: building a Json format is expensive enough that the compiler warns on it. */ + private val json = Json { ignoreUnknownKeys = true } + private lateinit var corpus: List @BeforeClass @@ -57,7 +60,7 @@ class LocalCacheSearchParityTest { .firstOrNull { it.isFile } ?: error("tools/search-parity/fixture.json is missing; run tools/search-parity/fetch_fixtures.py") - val root = Json { ignoreUnknownKeys = true }.parseToJsonElement(file.readText()).jsonObject + val root = json.parseToJsonElement(file.readText()).jsonObject corpus = root["cases"]!! .jsonArray diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt index f022d7192f..12586ddf8a 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt @@ -153,6 +153,7 @@ class RelayAuthPromptBusTest { * answer already sitting in the deferred, so the relay it belongs to goes unauthenticated for that * long despite the user having answered. Marking it shown is what makes the answer land now. */ + @OptIn(ExperimentalCoroutinesApi::class) @Test fun anAnswerFannedOutToAQueuedPromptLandsWithoutWaitingOutTheQueueWindow() = runTest { diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt index 369f853b52..bbb2ea53a8 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt @@ -32,6 +32,7 @@ import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPermissionStore import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthVerdict import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest @@ -185,6 +186,7 @@ class RelayAuthSessionGrantsTest { } } + @OptIn(ExperimentalCoroutinesApi::class) @Test fun promotingAGrantToAlwaysNeverOpensAGapThatRePrompts() = runTest { @@ -205,6 +207,7 @@ class RelayAuthSessionGrantsTest { assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(relay))) } + @OptIn(ExperimentalCoroutinesApi::class) @Test fun neverAllowStopsAuthenticatingBeforeItsWriteLands() = runTest { diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt index 3a86f8de1b..05b567cabe 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/Nip34NotificationCoverageTest.kt @@ -70,6 +70,7 @@ class Nip34NotificationCoverageTest { * A NIP-22 [com.vitorpamplona.quartz.nip22Comments.CommentEvent] handles * modern comments through its own separate wiring. */ + @Suppress("DEPRECATION") private val nip34ParticipantKinds = setOf( GitPatchEvent.KIND, @@ -131,6 +132,7 @@ class Nip34NotificationCoverageTest { * uppercase `E`. Asserting the wrong half passes the kind list while matching * nothing on the wire. */ + @Suppress("DEPRECATION") @Test fun `status kinds are pulled by the lowercase-e engagement subscription`() { val eAnchoredActivityKinds = diff --git a/benchmark/src/androidTest/java/com/vitorpamplona/quartz/benchmark/HexBenchmark.kt b/benchmark/src/androidTest/java/com/vitorpamplona/quartz/benchmark/HexBenchmark.kt index 3b11aa84df..ee091ac91f 100644 --- a/benchmark/src/androidTest/java/com/vitorpamplona/quartz/benchmark/HexBenchmark.kt +++ b/benchmark/src/androidTest/java/com/vitorpamplona/quartz/benchmark/HexBenchmark.kt @@ -136,7 +136,7 @@ class HexBenchmark { /** The pre-existing two-pass way to safely decode an id, for comparison with [hexDecode64OrNull]. */ @Test fun hexIsHex64ThenDecode() { - r.measureRepeated { if (Hex.isHex64(hex)) Hex.decode(hex) else null } + r.measureRepeated { if (Hex.isHex64(hex)) Hex.decode(hex) } } @Test diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt index 5fe963a9cd..de16160bc4 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/LiveStreamTopZappersViewModel.kt @@ -33,6 +33,7 @@ import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent import com.vitorpamplona.quartz.nipB1Bolt12Zaps.zap.Bolt12ZapEvent +import com.vitorpamplona.quartz.utils.toLongValue import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.Job @@ -162,7 +163,7 @@ class LiveStreamTopZappersViewModel( when (val ev = note.event) { is LnZapEvent -> { val request = ev.zapRequest ?: return null - val sats = ev.amount()?.toLong() ?: return null + val sats = ev.amount()?.toLongValue() ?: return null ZapContribution(note.idHex, request.pubKey, request.isAnonTagged(), sats) } is Bolt12ZapEvent -> { @@ -178,7 +179,7 @@ class LiveStreamTopZappersViewModel( ): ZapContribution? { val receiptEv = receiptNote?.event as? LnZapEvent ?: return null val request = zapRequestNote.event as? LnZapRequestEvent ?: return null - val sats = receiptEv.amount()?.toLong() ?: return null + val sats = receiptEv.amount()?.toLongValue() ?: return null return ZapContribution(receiptNote.idHex, request.pubKey, request.isAnonTagged(), sats) } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomZapsState.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomZapsState.kt index 8568f793f4..ce0267fb9e 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomZapsState.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomZapsState.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.commons.viewmodels import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip57Zaps.LnZapEvent import com.vitorpamplona.quartz.nipB1Bolt12Zaps.zap.Bolt12ZapEvent +import com.vitorpamplona.quartz.utils.toLongValue /** * One in-flight kind-9735 zap to render as a floating overlay on the @@ -59,7 +60,7 @@ data class RoomZap( eventId = event.id, sourcePubkey = event.zapRequest?.pubKey ?: event.pubKey, targetPubkey = event.zappedAuthor().firstOrNull(), - amountSats = event.amount?.toLong(), + amountSats = event.amount?.toLongValue(), createdAtSec = event.createdAt, ) diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/live/LivesSection.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/live/LivesSection.kt index 2490177cfe..53013a900f 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/live/LivesSection.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/live/LivesSection.kt @@ -27,7 +27,6 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.FlowRow -import androidx.compose.foundation.layout.FlowRowOverflow import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.aspectRatio @@ -146,7 +145,6 @@ fun LivesSection( horizontalArrangement = Arrangement.spacedBy(12.dp), verticalArrangement = Arrangement.spacedBy(12.dp), maxLines = 2, - overflow = FlowRowOverflow.Clip, modifier = Modifier.fillMaxWidth(), ) { ranked.forEach { channel -> diff --git a/quartz/src/appleMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.apple.kt b/quartz/src/appleMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.apple.kt new file mode 100644 index 0000000000..4bf7fc3821 --- /dev/null +++ b/quartz/src/appleMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.apple.kt @@ -0,0 +1,23 @@ +/* + * 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.quartz.utils + +actual fun BigDecimal.toLongValue(): Long = toLong() diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt index 698be085ec..b1cb1f8f79 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt @@ -760,7 +760,7 @@ class MarmotInboundProcessor( val author = payloadAuthor(candidate.content.decodeToString()) val sender = candidate.senderAccount val valid = author != null && sender != null && author == sender - if (valid && sender != null) { + if (valid) { convergence.recordWitness(groupId, candidate.stateId, sender) } return GroupEventResult.AppMessageOnCandidateBranch( diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/protocolCore/MarmotConvergenceEngine.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/protocolCore/MarmotConvergenceEngine.kt index f1ed9c8d75..91b0760946 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/protocolCore/MarmotConvergenceEngine.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/protocolCore/MarmotConvergenceEngine.kt @@ -561,7 +561,7 @@ class MarmotConvergenceEngine( return mutex.withLock { val ctx = contexts[groupId] ?: return@withLock null - if (rewound && selectedTipId != null) { + if (rewound) { adoptBranch(ctx, graph, selectedTipId, inputs.baseId) } // Keep the states of branches that LOST but stay eligible: losing @@ -576,7 +576,7 @@ class MarmotConvergenceEngine( .forEach { tipId -> var cursor: String? = tipId while (cursor != null && cursor !in canonical) { - graph.statesById[cursor]?.let { ctx.candidateStates[cursor!!] = it } + graph.statesById[cursor]?.let { ctx.candidateStates[cursor] = it } cursor = graph.parentOf[cursor] } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.kt index 571f812db1..be65f1dac7 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.kt @@ -23,3 +23,15 @@ package com.vitorpamplona.quartz.utils operator fun BigDecimal.plus(other: BigDecimal): BigDecimal = add(other) operator fun BigDecimal.minus(other: BigDecimal): BigDecimal = subtract(other) + +/** + * Truncate to a Long, the way Number.toLong() does on every platform. + * + * It has to be an expect *function* rather than a member of `expect class + * BigDecimal`: every actual is already a Number and so already has toLong(), + * but java.math.BigDecimal leaves toByte()/toShort() abstract, which makes + * `expect class BigDecimal : Number` impossible to actualize with the JVM + * typealias. Without this, `amount.toLong()` in shared code resolves only in + * the platform compilations and breaks `compileCommonMainKotlinMetadata`. + */ +expect fun BigDecimal.toLongValue(): Long diff --git a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.jvmAndroid.kt b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.jvmAndroid.kt new file mode 100644 index 0000000000..4bf7fc3821 --- /dev/null +++ b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.jvmAndroid.kt @@ -0,0 +1,23 @@ +/* + * 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.quartz.utils + +actual fun BigDecimal.toLongValue(): Long = toLong() diff --git a/quartz/src/linuxMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.linux.kt b/quartz/src/linuxMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.linux.kt new file mode 100644 index 0000000000..4bf7fc3821 --- /dev/null +++ b/quartz/src/linuxMain/kotlin/com/vitorpamplona/quartz/utils/BigDecimalOps.linux.kt @@ -0,0 +1,23 @@ +/* + * 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.quartz.utils + +actual fun BigDecimal.toLongValue(): Long = toLong() From 32f0c462bb393149348a1509fe6c0b93e0be99c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 16:21:51 +0000 Subject: [PATCH 04/16] test: run relay-backed tests against geode instead of external relays Every test that used to open a socket to something outside the repo now talks to geode, the relay this project ships, either in-process or as the embedded `amy serve`. - amethyst: the Android instrumented EventSyncTest dialed vitor.nostr1.com, pyramid.fiatjaf.com and the nos.lol / nostr.mom defaults and asserted nothing. Replaced by a JVM unit test on geode's InProcessRelays that preloads a source relay and asserts what lands on the outbox, inbox and DM relays, plus a NIP-42 variant with the source behind FullAuthPolicy to cover the RelayAuthenticator wiring. Wires :geode, its testFixtures and the JVM SQLite driver into amethyst's unit-test classpath, the same way quartz's jvmAndroidTest already does. - cli/tests: the cache, dm and marmot headless harnesses cloned and cargo-built nostr-rs-relay on first run. They now boot `amy serve` (geode) from the amy binary they already build, via a shared start_local_relay / stop_local_relay in headless/helpers.sh. Rust is no longer needed for the cache and dm suites at all. - cli/tests/marmot/marmot-interop.sh: the interactive harness defaulted to relay.damus.io / nos.lol / primal / bitcoiner.social / nostr.mom, with `--local-relays` pointing at MDK's docker stack. It now boots the embedded relay on 0.0.0.0 by default (the phone reaches it over the LAN) and keeps the public set behind an explicit `--public-relays`. - docs: cli/tests/README.md, CONTRIBUTING.md, cli/DEVELOPMENT.md and cli/ROADMAP.md no longer describe a loopback nostr-rs-relay. The quartz prodbench probes (NegentropyStallRepro, CursorTerminationProbe, NegentropyMultiRelayLiveTest, ProductionReceiverBenchmark, ...) are left as they are: they are opt-in diagnostics of production relay behaviour, gated behind PROD_RELAY_BENCH / NEG_MULTI / NEG_STALL_REPRO, and would measure nothing against a local relay. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PguqnDbP2v11dtANs9xdxc --- CONTRIBUTING.md | 8 +- amethyst/build.gradle.kts | 9 + .../relays/eventsync/EventSyncTest.kt | 112 --------- .../relays/eventsync/EventSyncTest.kt | 221 ++++++++++++++++++ cli/DEVELOPMENT.md | 2 +- cli/ROADMAP.md | 8 +- cli/tests/.gitignore | 1 + cli/tests/README.md | 85 +++---- cli/tests/cache/cache-headless.sh | 20 +- cli/tests/dm/dm-interop-headless.sh | 22 +- cli/tests/dm/setup.sh | 36 +-- cli/tests/dm/tests-dm.sh | 2 +- cli/tests/headless/helpers.sh | 86 +++++++ cli/tests/marmot/marmot-interop-headless.sh | 21 +- cli/tests/marmot/marmot-interop.sh | 96 ++++++-- cli/tests/marmot/setup.sh | 100 +------- 16 files changed, 487 insertions(+), 342 deletions(-) delete mode 100644 amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 56a3348ba6..8006812c1a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -265,9 +265,11 @@ front: sequentially: `for peer in aioquic picoquic quic-go quinn; do quic/interop/run-matrix.sh -s $peer; done`. Plan at `quic/interop/plans/2026-05-06-interop-runner.md`. -- **CLI suites** ([`cli/tests/README.md`](cli/tests/README.md)): headless - variants need only `cargo` + a loopback `nostr-rs-relay`; the interactive - Marmot variant prompts a human to drive the Android UI. +- **CLI suites** ([`cli/tests/README.md`](cli/tests/README.md)): every + relay-backed suite boots the embedded `amy serve` relay (geode) — no + external relay binary; only the Marmot suites additionally need `cargo` + for MDK's `wn`/`wnd`. The interactive Marmot variant prompts a human to + drive the Android UI. If a change is documentation-only, UI-only, build-script-only, or otherwise cannot affect wire bytes / decoded audio / MLS state / DM envelopes, skip diff --git a/amethyst/build.gradle.kts b/amethyst/build.gradle.kts index 77010ea9fe..3089332b29 100644 --- a/amethyst/build.gradle.kts +++ b/amethyst/build.gradle.kts @@ -598,6 +598,15 @@ dependencies { testImplementation(libs.kotlinx.coroutines.test) testImplementation(libs.secp256k1.kmp.jni.jvm) + // In-process Nostr relay (geode) so unit tests that drive a real + // NostrClient talk to an embedded relay instead of a public one. Same + // wiring quartz uses for its jvmAndroidTest source set: the engine, its + // testFixtures (RelayClientTest base, preload/publish helpers) and the + // JVM SQLite driver the in-memory EventStore needs on a host JVM. + testImplementation(project(":geode")) + testImplementation(testFixtures(project(":geode"))) + testImplementation(libs.androidx.sqlite.bundled.jvm) + androidTestImplementation(platform(libs.androidx.compose.bom)) androidTestImplementation(libs.androidx.junit) androidTestImplementation(libs.androidx.junit.ktx) diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt deleted file mode 100644 index f4ed293a27..0000000000 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt +++ /dev/null @@ -1,112 +0,0 @@ -/* - * 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.ui.screen.loggedIn.relays.eventsync - -import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.vitorpamplona.amethyst.commons.defaults.Constants -import com.vitorpamplona.amethyst.commons.service.http.DefaultContentTypeInterceptor -import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair -import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient -import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.RelayLogger -import com.vitorpamplona.quartz.nip01Core.relay.client.auth.RelayAuthenticator -import com.vitorpamplona.quartz.nip01Core.relay.normalizer.normalizeRelayUrl -import com.vitorpamplona.quartz.nip01Core.relay.sockets.okhttp.BasicOkHttpWebSocket -import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.runBlocking -import okhttp3.OkHttpClient -import org.junit.Test -import org.junit.runner.RunWith - -@RunWith(AndroidJUnit4::class) -class EventSyncTest { - companion object { - val vitor = "wss://vitor.nostr1.com".normalizeRelayUrl() - val fiatjaf = "wss://pyramid.fiatjaf.com".normalizeRelayUrl() - val appScope = CoroutineScope(Dispatchers.Default + SupervisorJob()) - - val rootClient = - OkHttpClient - .Builder() - .followRedirects(true) - .followSslRedirects(true) - .addInterceptor(DefaultContentTypeInterceptor("Amethyst/v1.05")) - .build() - val socketBuilder = BasicOkHttpWebSocket.Builder { url -> rootClient } - } - - @Test - fun testSync() = - runBlocking { - val sync = - EventSync( - accountPubKey = "460c25e682fda7832b52d1f22d3d22b3176d972f60dcdc3212ed8c92ef85065c", - relayDb = { - listOf(Constants.mom, Constants.nos) - }, - outboxTargets = { setOf(vitor) }, - inboxTargets = { setOf(vitor) }, - dmTargets = { setOf(vitor) }, - clientBuilder = { - NostrClient(socketBuilder, appScope) - }, - scope = appScope, - ) - - sync.runSync() - } - - @Test - fun testFiatjafSync() = - runBlocking { - val sync = - EventSync( - accountPubKey = "460c25e682fda7832b52d1f22d3d22b3176d972f60dcdc3212ed8c92ef85065c", - relayDb = { listOf(fiatjaf) }, - outboxTargets = { setOf(vitor) }, - inboxTargets = { setOf(vitor) }, - dmTargets = { setOf(vitor) }, - clientBuilder = { - val newClient = NostrClient(socketBuilder, appScope) - val logger = RelayLogger(newClient, debugSending = true, debugReceiving = false) - - val signer = NostrSignerInternal(KeyPair()) - - // Authenticates with relays. - val auth = - RelayAuthenticator( - newClient, - appScope, - signWithAllLoggedInUsers = { _, authTemplate, _ -> - listOf(signer.sign(authTemplate)) - }, - ) - - newClient - }, - scope = appScope, - ) - - sync.runSync() - } -} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt new file mode 100644 index 0000000000..65931c1128 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt @@ -0,0 +1,221 @@ +/* + * 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.ui.screen.loggedIn.relays.eventsync + +import com.vitorpamplona.geode.InProcessRelays +import com.vitorpamplona.geode.RelayEngine +import com.vitorpamplona.geode.testing.RelayClientTest +import com.vitorpamplona.geode.testing.preload +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient +import com.vitorpamplona.quartz.nip01Core.relay.client.auth.RelayAuthenticator +import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer +import com.vitorpamplona.quartz.nip01Core.relay.server.policies.FullAuthPolicy +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocket +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebSocketListener +import com.vitorpamplona.quartz.nip01Core.relay.sockets.WebsocketBuilder +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync +import com.vitorpamplona.quartz.nip01Core.signers.eventTemplate +import com.vitorpamplona.quartz.nip01Core.tags.people.pTag +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import kotlinx.coroutines.withTimeoutOrNull +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Drives [EventSync] end to end against geode's in-process relays: one + * "source" relay that already holds the account's history and three empty + * destination relays (outbox / inbox / DM). No network, no public relay — + * every relay is a [RelayEngine] inside this JVM, so the assertions are on + * what actually landed in each destination store, not on "it didn't crash". + * + * The second scenario gates the source relay behind NIP-42 ([FullAuthPolicy]) + * to cover the [RelayAuthenticator] wiring the sync screen relies on when a + * user's relay demands AUTH before serving REQs. + */ +class EventSyncTest : RelayClientTest() { + private val account = NostrSignerSync(KeyPair()) + private val other = NostrSignerSync(KeyPair()) + + private val source: NormalizedRelayUrl = RelayUrlNormalizer.normalize("ws://source.relay/") + private val outbox: NormalizedRelayUrl = RelayUrlNormalizer.normalize("ws://outbox.relay/") + private val inbox: NormalizedRelayUrl = RelayUrlNormalizer.normalize("ws://inbox.relay/") + private val dm: NormalizedRelayUrl = RelayUrlNormalizer.normalize("ws://dm.relay/") + + /** Separate hub so only the source relay demands AUTH; destinations stay open. */ + private val authHub = InProcessRelays(defaultPolicy = { FullAuthPolicy(source) }) + + @After + fun tearDownAuthHub() { + authHub.close() + } + + private fun note( + author: NostrSignerSync, + content: String, + tagged: HexKey? = null, + ): Event = author.sign(eventTemplate(1, content) { tagged?.let { pTag(it) } }) + + private fun legacyDm( + author: NostrSignerSync, + recipient: HexKey, + ): Event = author.sign(eventTemplate(4, "ciphertext") { pTag(recipient) }) + + private val mine = List(3) { note(account, "mine $it") } + private val mentions = List(2) { note(other, "hey $it", tagged = account.pubKey) } + private val dmToMe = legacyDm(other, account.pubKey) + private val noise = note(other, "unrelated") + + private fun corpus(): List = mine + mentions + dmToMe + noise + + private fun eventSync(builder: WebsocketBuilder): EventSync = + EventSync( + accountPubKey = account.pubKey, + relayDb = { listOf(source) }, + outboxTargets = { setOf(outbox) }, + inboxTargets = { setOf(inbox) }, + dmTargets = { setOf(dm) }, + clientBuilder = { NostrClient(builder, scope) }, + scope = scope, + ) + + /** + * Publishes are fire-and-forget on the client side, so the destination + * store can lag `runSync` returning by a few ticks. Poll instead of + * asserting a snapshot. + */ + private suspend fun RelayEngine.awaitCount( + filter: Filter, + expected: Int, + ): Int = + withTimeoutOrNull(10_000) { + while (store.count(filter) < expected) delay(25) + store.count(filter) + } ?: store.count(filter) + + private suspend fun assertRouted(hubOfTargets: InProcessRelays) { + val outboxRelay = hubOfTargets.getOrCreate(outbox) + val inboxRelay = hubOfTargets.getOrCreate(inbox) + val dmRelay = hubOfTargets.getOrCreate(dm) + + assertEquals( + "every event authored by the account lands on the outbox relay", + mine.size, + outboxRelay.awaitCount(Filter(authors = listOf(account.pubKey)), mine.size), + ) + assertEquals( + "non-DM mentions land on the inbox relay", + mentions.size, + inboxRelay.awaitCount(Filter(tags = mapOf("p" to listOf(account.pubKey))), mentions.size), + ) + assertEquals( + "the kind-4 DM lands on the DM relay", + 1, + dmRelay.awaitCount(Filter(kinds = listOf(4)), 1), + ) + + // Routing is exclusive per rule: nothing leaks across destinations and the + // unrelated note never leaves the source. + assertEquals("outbox holds only the account's events", mine.size, outboxRelay.store.count(Filter())) + assertEquals("inbox holds only the mentions", mentions.size, inboxRelay.store.count(Filter())) + assertEquals("dm relay holds only the DM", 1, dmRelay.store.count(Filter())) + assertEquals("noise stays on the source", 0, outboxRelay.store.count(Filter(ids = listOf(noise.id)))) + } + + @Test + fun syncRoutesEventsFromSourceToOutboxInboxAndDmRelays() = + runBlocking { + hub.getOrCreate(source).preload(corpus()) + + val sync = eventSync(hub) + withTimeout(30_000) { sync.runSync() } + + val done = sync.syncState.value + assertTrue("sync should finish in Done, got $done", done is EventSync.SyncState.Done) + done as EventSync.SyncState.Done + assertEquals( + "mine + mentions + dm match a routing rule; noise does not", + mine.size + mentions.size + 1, + done.totalEventsReceived, + ) + + assertRouted(hub) + } + + @Test + fun syncReadsFromAuthRequiredSourceOnceAuthenticated() = + runBlocking { + authHub.getOrCreate(source).preload(corpus()) + + // Source demands NIP-42 before serving REQs; destinations are the open hub. + val router = + object : WebsocketBuilder { + override fun build( + url: NormalizedRelayUrl, + out: WebSocketListener, + ): WebSocket = if (url == source) authHub.build(url, out) else hub.build(url, out) + } + + val authSigner = NostrSignerSync(KeyPair()) + var authenticator: RelayAuthenticator? = null + val sync = + EventSync( + accountPubKey = account.pubKey, + relayDb = { listOf(source) }, + outboxTargets = { setOf(outbox) }, + inboxTargets = { setOf(inbox) }, + dmTargets = { setOf(dm) }, + clientBuilder = { + val client = NostrClient(router, scope) + authenticator = + RelayAuthenticator(client = client, scope = scope) { _, template, _ -> + listOf(authSigner.sign(template)) + } + client + }, + scope = scope, + ) + + try { + withTimeout(30_000) { sync.runSync() } + } finally { + authenticator?.destroy() + } + + val done = sync.syncState.value + assertTrue("sync should finish in Done, got $done", done is EventSync.SyncState.Done) + assertEquals( + "the auth-gated source still yields every routed event", + mine.size + mentions.size + 1, + (done as EventSync.SyncState.Done).totalEventsReceived, + ) + + assertRouted(hub) + } +} diff --git a/cli/DEVELOPMENT.md b/cli/DEVELOPMENT.md index d343eb4fb8..7e96d3f501 100644 --- a/cli/DEVELOPMENT.md +++ b/cli/DEVELOPMENT.md @@ -348,7 +348,7 @@ Amy-specific layer still needs its own coverage: | Error / exit-code contract (bad args → 2, timeout → 124, `rejected` → 1) | `ExitCodeContractTest` — table-driven tests invoking `runCli(argv)` with captured stdout/stderr. | | JSON output shape (keys and types under `--json`) | `JsonContractTest` — runs commands under `--json` and asserts on the parsed object. The default text render has no shape contract and isn't asserted on. | | File layout on disk (`identity.json`, `shared/events.db`, `marmot/groups/*.mls`, …) | Structural assertions after a command sequence. | -| Round-trip between two accounts on a local relay | End-to-end shell harnesses under `cli/tests/`: each spins up a local `nostr-rs-relay` and a fresh `$HOME=$STATE_DIR` so amy sees a virgin `~/.amy/`, then bootstraps multiple accounts sharing one store and drives a scenario through them. Nine suites today — see [`cli/tests/README.md`](./tests/README.md). | +| Round-trip between two accounts on a local relay | End-to-end shell harnesses under `cli/tests/`: each spins up the embedded `amy serve` relay (geode) and a fresh `$HOME=$STATE_DIR` so amy sees a virgin `~/.amy/`, then bootstraps multiple accounts sharing one store and drives a scenario through them. Nine suites today — see [`cli/tests/README.md`](./tests/README.md). | The JVM suite drives `runCli` **in-process** through the shared `amy(vararg argv)` harness in `CliResult.kt`: it captures stdout/stderr, diff --git a/cli/ROADMAP.md b/cli/ROADMAP.md index ede7f2c648..74645af784 100644 --- a/cli/ROADMAP.md +++ b/cli/ROADMAP.md @@ -180,11 +180,11 @@ move anything, re-audit — you're probably duplicating logic. 9. **Test suite** — largely in place, two layers: - **Shell harnesses** under `cli/tests/` — ten suites: `blossom` (live servers), `cache`, `clink`, `dm`, `git` (NIP-34 vs `amy serve`), - `marmot` (vs whitenoise-rs), `nests` (manual audio-rooms matrix), `pow`, + `marmot` (vs MDK), `nests` (manual audio-rooms matrix), `pow`, `relaygroup`, `sync`, plus the shared `headless/` helpers. See - `cli/tests/README.md`. - None run in CI yet (the relay-backed ones need Rust + a ~3 min - cold `nostr-rs-relay` build). + `cli/tests/README.md`. Every relay-backed suite runs against the + embedded `amy serve` relay (geode) — no external relay binary. + None run in CI yet (the Marmot ones need Rust for MDK's `wn`). - **JVM unit suite** at `cli/src/test/kotlin/` — `Args` parsing, exit-code contract, and `--json` shape tests driving `runCli` in-process via the `amy.home` isolation seam. diff --git a/cli/tests/.gitignore b/cli/tests/.gitignore index 363484739c..53f48067cc 100644 --- a/cli/tests/.gitignore +++ b/cli/tests/.gitignore @@ -1,6 +1,7 @@ marmot/state/ marmot/state-headless/ dm/state-dm-headless/ +cache/state-cache-headless/ nests/state/ clink/state-clink-headless/ relaygroup/state-relaygroup-headless/ diff --git a/cli/tests/README.md b/cli/tests/README.md index cf2b314a64..caa8f88122 100644 --- a/cli/tests/README.md +++ b/cli/tests/README.md @@ -1,21 +1,24 @@ # amy CLI test harnesses -Shell-based end-to-end harnesses that drive the `amy` CLI binary — against a -loopback `nostr-rs-relay`, an embedded `amy serve` relay, live public servers, -or no relay at all, depending on the suite. Eleven directories: +Shell-based end-to-end harnesses that drive the `amy` CLI binary — against an +embedded relay (`amy serve`, i.e. **geode**, the relay this repo ships), live +public servers, or no relay at all, depending on the suite. No suite depends on +an external relay binary or a Rust toolchain for its relay: every relay-backed +harness boots geode from the `amy` binary it already built, so the relay under +test is the same server code that runs in production. Eleven directories: ``` cli/tests/ ├── lib.sh # shared logging, results, assertions ├── headless/ # shared bits used by every harness -│ └── helpers.sh +│ └── helpers.sh # amy wrappers, assertions, embedded relay boot ├── blossom/ # Blossom blob lifecycle vs LIVE public servers │ └── blossom-live.sh ├── cache/ # local-store-as-cache semantics (profile show -│ └── cache-headless.sh # cache/refresh, store stat) vs nostr-rs-relay +│ └── cache-headless.sh # cache/refresh, store stat) vs embedded `amy serve` ├── clink/ # CLINK pointer decode — local-only, no relay │ └── clink-headless.sh -├── dm/ # NIP-17 DM interop (amy ↔ amy) +├── dm/ # NIP-17 DM interop (amy ↔ amy) vs embedded `amy serve` │ ├── dm-interop-headless.sh │ ├── setup.sh # preflight + identities │ └── tests-dm.sh @@ -24,7 +27,7 @@ cli/tests/ ├── marmot/ # Marmot / MLS group-messaging interop │ ├── marmot-interop.sh # interactive — prompts Amethyst Android UI │ ├── marmot-interop-headless.sh # zero-prompt -│ ├── setup.sh # preflight + wn + relay + identities +│ ├── setup.sh # preflight + wn + identities │ ├── tests-create.sh # tests 01–05 │ ├── tests-manage.sh # tests 06–08, 11 │ ├── tests-extras.sh # tests 09, 10, 12, 13 @@ -60,7 +63,7 @@ Suite notes: and mined-nonce round-trips through `pow check`. - **`cache/cache-headless.sh`** proves the local store is the source of truth for reads: `profile show` served from cache vs `--refresh`, and - `store stat` reporting the right histogram, vs a loopback nostr-rs-relay. + `store stat` reporting the right histogram, vs the embedded `amy serve` relay. - **`relaygroup/relaygroup-headless.sh`** runs NIP-29 create/message/join/ list/browse against an embedded relay (`amy serve`, which boots geode) — no external relay binary. geode doesn't sign 39000-39003, so browse/info @@ -124,10 +127,8 @@ The Marmot harnesses come in two flavours, same scenarios: A third, slimmer harness covers the NIP-17 DM surface: - **`dm/dm-interop-headless.sh`** — two `amy` processes (Identity A and - Identity D) exchange NIP-17 DMs through the loopback nostr-rs-relay. - No MDK required — only `amy` and the relay binary (which - is shared with the Marmot harness's checkout at - `marmot/state-headless/nostr-rs-relay/`). + Identity D) exchange NIP-17 DMs through the embedded `amy serve` relay. + No MDK, no Rust — only `amy`. A harness covers Blossom blob storage (BUD-01/02/04/09) against **live** public servers rather than a loopback relay: @@ -212,15 +213,18 @@ at `desktopApp/src/jvmTest/kotlin/.../service/upload/`. On the machine that runs the harness: -- **Rust 1.90+** — install via https://rustup.rs +- **Rust 1.90+** — install via https://rustup.rs (for MDK's `wn`/`wnd` only; + the relay is `amy serve`, no Rust needed for it) - **git**, **curl**, **jq** — package manager - **~5 GB disk** for the first-run build of `wn` + `wnd` -- Public internet access (for the default relay set and fetching crates) +- Internet access for fetching crates on the first build. Test traffic + stays on the machine unless you pass `--public-relays`. On the Android side: - Amethyst installed on an **emulator** or a **physical device** -- The device must reach the same relays the harness uses (see below) +- The device must reach the harness's embedded relay over the network + (see below), or the public relays when running with `--public-relays` ## Quick start @@ -240,9 +244,11 @@ The script will, in order: 4. Create Nostr identities for B and C, persist their npubs in `state/run.env`. 5. Ask you to paste **your Amethyst account npub** (Identity A). This is cached for subsequent runs. -6. Add the default public relays to both daemons and run a sanity check - (publish a KP from B, fetch it from C). -7. Print an **Amethyst setup checklist** — add the same relays to Amethyst, +6. Boot the embedded relay (`amy serve`, i.e. geode, on `0.0.0.0:8080`), + add it to both daemons and run a sanity check (publish a KP from B, + fetch it from C). With `--public-relays` the default public set is used + instead and the relay is not started. +7. Print an **Amethyst setup checklist** — add the same relay to Amethyst, publish a KP, verify you are logged in with A. 8. Run all 13 tests sequentially. Each test either: - runs `wn` commands fully automatically and asserts on JSON output, **or** @@ -253,9 +259,10 @@ The script will, in order: ## Command-line flags ``` ---local-relays Use ws://localhost:8080 instead of the default public relays. - Required if the public relays reject kinds 444/445/30443. - Run 'just docker-up' inside the mdk checkout first. +--public-relays Use the public relay set below instead of the embedded relay. + The only mode whose test traffic leaves the machine; the + public relays may reject kinds 444/445/30443. +--port N Port for the embedded relay (default 8080). --transponder Run Test 14 (push notifications via the transponder service). --no-build Fail instead of rebuilding wn/wnd. Useful when iterating. -h, --help Show help. @@ -267,31 +274,31 @@ Environment overrides: WN_REPO=/some/path/mdk # use an existing checkout ``` -## Default relays +## Relays + +By default the harness owns the only relay: `amy serve` (geode) bound to +`0.0.0.0:8080`. The `wn` daemons reach it on loopback; Amethyst reaches it +over the network: + +- **Android emulator:** add `ws://10.0.2.2:8080` to Settings → Relays, + Settings → Key Package Relays and Settings → DM Inbox Relays. +- **Physical device on same Wi-Fi:** add `ws://:8080`. + +With `--public-relays` the daemons are bootstrapped on ``` wss://relay.damus.io wss://nos.lol wss://relay.primal.net +wss://nostr.bitcoiner.social +wss://nostr.mom ``` -These are known to accept kind 1059 (gift wraps) and kind 30000+ (addressable -events). If the **sanity check fails** — meaning C cannot read the KeyPackage -that B just published — the harness warns you and continues. In that case -re-run with `--local-relays` after starting the Docker stack: - -```bash -cd state/mdk -just docker-up -cd ../.. -./marmot-interop.sh --local-relays -``` - -For Amethyst with `--local-relays`: - -- **Android emulator:** add `ws://10.0.2.2:8080` to Settings → Relays and - Settings → Key Package Relays. -- **Physical device on same Wi-Fi:** add `ws://:8080`. +instead and Amethyst is left on its own relay set, so the run surfaces +real-world discovery failures (A's inbox behind NIP-42, whitelists, kinds the +public relays drop). If the **sanity check fails** in that mode — meaning C +cannot read the KeyPackage that B just published — the harness warns you and +continues; re-run without `--public-relays` to rule the relays out. ## How human interaction works diff --git a/cli/tests/cache/cache-headless.sh b/cli/tests/cache/cache-headless.sh index b1944678ce..8d2d3df51d 100755 --- a/cli/tests/cache/cache-headless.sh +++ b/cli/tests/cache/cache-headless.sh @@ -3,8 +3,8 @@ # cache-headless.sh — verifies the file-backed event store is the # source of truth for `amy` reads. # -# Two amy identities (A and B) talk to a local nostr-rs-relay. We -# assert that: +# Two amy identities (A and B) talk to a local embedded relay +# (`amy serve`, i.e. geode). We assert that: # # 1. After A runs `amy create`, A's local store contains the bootstrap # events (kind:0 / 3 / 10002 / 10050 / 10051 …). @@ -39,10 +39,11 @@ RESULTS_FILE="$STATE_DIR/results-$RUN_TS.tsv" AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" -# Reuse the relay binary the marmot harness builds. +# Loopback relay = `amy serve` (geode), booted from $AMY_BIN by +# start_local_relay in headless/helpers.sh. 127.0.0.2 rather than +# 127.0.0.1 so Quartz's isLocalHost() filter doesn't strip it out of the +# published relay lists (see the DM harness for the full note). RELAY_HOST="${RELAY_HOST:-127.0.0.2}" -RELAY_REPO="${RELAY_REPO:-$TESTS_DIR/marmot/state-headless/nostr-rs-relay}" -RELAY_BIN="$RELAY_REPO/target/release/nostr-rs-relay" RELAY_DATA="$STATE_DIR/relay" RELAY_PORT="${RELAY_PORT:-8092}" RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT" @@ -72,14 +73,11 @@ mkdir -p "$STATE_DIR" "$LOG_DIR" # shellcheck source=../lib.sh source "$TESTS_DIR/lib.sh" -# shellcheck source=../marmot/setup.sh — provides start_local_relay / stop_local_relay -source "$TESTS_DIR/marmot/setup.sh" -# shellcheck source=../headless/helpers.sh +# shellcheck source=../headless/helpers.sh — amy wrappers + start_local_relay / stop_local_relay source "$TESTS_DIR/headless/helpers.sh" -# Keep the dm setup's preflight (just checks for amy + the relay) but -# define our own identity bootstrap so we don't pull in DM-specific -# wiring. +# Keep the dm setup's preflight (just checks for amy) but define our own +# identity bootstrap so we don't pull in DM-specific wiring. # shellcheck source=../dm/setup.sh source "$TESTS_DIR/dm/setup.sh" diff --git a/cli/tests/dm/dm-interop-headless.sh b/cli/tests/dm/dm-interop-headless.sh index 687b9349cc..fa7fbaf84f 100755 --- a/cli/tests/dm/dm-interop-headless.sh +++ b/cli/tests/dm/dm-interop-headless.sh @@ -3,8 +3,9 @@ # dm-interop-headless.sh — zero-prompt NIP-17 DM interop harness. # # Two `amy` processes (Identity A and Identity D) talk to each other -# through a local nostr-rs-relay on ws://127.0.0.1:$RELAY_PORT. No -# whitenoise-rs, no Marmot, no public internet traffic. +# through a local embedded relay (`amy serve`, i.e. geode) on +# ws://127.0.0.2:$RELAY_PORT. No MDK, no Marmot, no Rust toolchain, no +# public internet traffic. # # Usage: ./dm-interop-headless.sh [--port N] [--no-build] # @@ -26,16 +27,14 @@ RESULTS_FILE="$STATE_DIR/results-$RUN_TS.tsv" AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" -# Share the nostr-rs-relay checkout with the Marmot harness to avoid -# rebuilding it twice. Override RELAY_REPO / RELAY_DATA if you want full -# isolation between runs. +# Loopback relay = `amy serve` (geode), booted from $AMY_BIN by +# start_local_relay in headless/helpers.sh. Override RELAY_DATA if you +# want full isolation between runs. # Bind the loopback relay to 127.0.0.2 rather than 127.0.0.1 so Quartz's # `isLocalHost()` filter doesn't silently strip it out of the kind:10050 # inbox events during recipient-relay resolution. 127.0.0.2 is still pure # loopback — no network traffic, no config needed. RELAY_HOST="${RELAY_HOST:-127.0.0.2}" -RELAY_REPO="${RELAY_REPO:-$TESTS_DIR/marmot/state-headless/nostr-rs-relay}" -RELAY_BIN="$RELAY_REPO/target/release/nostr-rs-relay" RELAY_DATA="$STATE_DIR/relay" RELAY_PORT="${RELAY_PORT:-8090}" RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT" @@ -68,12 +67,9 @@ mkdir -p "$STATE_DIR" "$LOG_DIR" # shellcheck source=../lib.sh source "$TESTS_DIR/lib.sh" -# Reuse start_local_relay / stop_local_relay from the Marmot harness's -# setup.sh — the relay lifecycle is identical. preflight() there also -# builds whitenoise-rs, which we don't need; setup.sh in this dir -# defines a slimmer preflight_dm(). -# shellcheck source=../marmot/setup.sh -source "$TESTS_DIR/marmot/setup.sh" +# setup.sh in this dir defines the slim preflight_dm() (amy only, no +# MDK); the relay lifecycle (start_local_relay / stop_local_relay, the +# embedded `amy serve`) comes from the shared headless helpers. # shellcheck source=setup.sh source "$SCRIPT_DIR/setup.sh" # shellcheck source=../headless/helpers.sh diff --git a/cli/tests/dm/setup.sh b/cli/tests/dm/setup.sh index 5af3b5605d..5fdd65fba5 100644 --- a/cli/tests/dm/setup.sh +++ b/cli/tests/dm/setup.sh @@ -3,20 +3,21 @@ # setup.sh — amy-only preflight + identity bootstrap for the # NIP-17 DM interop harness. Much slimmer than the Marmot setup: # -# - Builds `amy` (same retry-on-503 logic as setup.sh). -# - Builds nostr-rs-relay if missing. +# - Builds `amy` (same retry-on-503 logic as setup.sh). The loopback +# relay is `amy serve` (geode), so amy is the only binary needed. # - Bootstraps two fresh amy identities (A and D), each with its own # `--data-dir`, both pointed at the loopback relay. # - Publishes kind:10050 (plus NIP-65) for both so NIP-17's strict # recipient-inbox routing has something to resolve to. # -# The heavy `start_local_relay` / `stop_local_relay` helpers live in -# the Marmot harness's setup.sh and are sourced by the top-level harness. +# The `start_local_relay` / `stop_local_relay` helpers (embedded +# `amy serve`) live in ../headless/helpers.sh and are sourced by the +# top-level harness. -# --- preflight (amy + relay only, no wn / Marmot patches) ------------------- +# --- preflight (amy only, no wn / Marmot patches, no Rust) ------------------- preflight_dm() { banner "Preflight (DM harness)" - for cmd in jq git cargo; do + for cmd in jq git curl; do if ! command -v "$cmd" >/dev/null 2>&1; then fail_msg "missing required tool: $cmd" exit 1 @@ -43,29 +44,6 @@ preflight_dm() { [[ -x "$AMY_BIN" ]] || { fail_msg "amy still missing after build"; exit 1; } info "amy: $AMY_BIN" - # nostr-rs-relay (same build path as the Marmot harness). - if [[ ! -x "$RELAY_BIN" ]]; then - if [[ "$NO_BUILD" -eq 1 ]]; then - fail_msg "nostr-rs-relay not found at $RELAY_BIN and --no-build set"; exit 1 - fi - if [[ ! -d "$RELAY_REPO/.git" ]]; then - step "cloning nostr-rs-relay into $RELAY_REPO" - git clone --depth 1 https://github.com/scsibug/nostr-rs-relay "$RELAY_REPO" \ - 2>&1 | tee -a "$LOG_FILE" - fi - local attempt max=4 - for attempt in $(seq 1 $max); do - step "building nostr-rs-relay (attempt $attempt/$max, ~3 min first run)" - ( cd "$RELAY_REPO" && cargo build --release --bin nostr-rs-relay ) \ - 2>&1 | tee -a "$LOG_FILE" - [[ -x "$RELAY_BIN" ]] && break - [[ "$attempt" -lt "$max" ]] && warn "nostr-rs-relay build failed — retrying" - done - [[ -x "$RELAY_BIN" ]] || { - fail_msg "nostr-rs-relay still missing after $max attempts"; exit 1 - } - fi - info "relay bin: $RELAY_BIN" } # --- amy identity wrappers --------------------------------------------------- diff --git a/cli/tests/dm/tests-dm.sh b/cli/tests/dm/tests-dm.sh index 4fff53fbe8..fe0f2bb393 100644 --- a/cli/tests/dm/tests-dm.sh +++ b/cli/tests/dm/tests-dm.sh @@ -3,7 +3,7 @@ # tests-dm.sh — NIP-17 DM interop tests for two `amy` clients. # # Identity A (sender) and Identity D (recipient) each live in their own -# --data-dir and share one loopback nostr-rs-relay. Tests cover: +# --data-dir and share one loopback embedded relay (`amy serve` / geode). Tests cover: # # dm-01 text round-trip (both directions) # dm-02 dm list surfaces prior exchange with type:text discriminator diff --git a/cli/tests/headless/helpers.sh b/cli/tests/headless/helpers.sh index e01069bb3f..c2c504854e 100644 --- a/cli/tests/headless/helpers.sh +++ b/cli/tests/headless/helpers.sh @@ -66,5 +66,91 @@ assert_eq() { return 1 } +# --- embedded relay (amy serve → geode) -------------------------------------- +# Every relay-backed harness talks to ONE loopback relay, and that relay is +# `amy serve` — i.e. geode, the relay this repo ships — booted from the amy +# binary the harness already built. No Rust toolchain, no clone, no cargo +# build, no external relay binary: the relay under test is part of the +# product, so a harness run exercises the same server code `amy serve` +# and the standalone geode distribution run in production. +# +# Callers set (before sourcing or at least before calling): +# AMY_BIN amy launcher (built via `./gradlew :cli:installDist`) +# RELAY_HOST host clients connect to (most harnesses use 127.0.0.2 — +# see the isLocalHost() note at the top of each script) +# RELAY_BIND optional bind address; defaults to $RELAY_HOST. Set to +# 0.0.0.0 when a device on the LAN must reach the relay. +# RELAY_PORT listen port +# RELAY_URL ws://$RELAY_HOST:$RELAY_PORT +# RELAY_DATA scratch dir for the relay's own $HOME, pid file and logs +# +# The relay process runs as its own amy account ("relay") inside its own +# $HOME under $RELAY_DATA, so its identity and store never mix with the +# test identities. The store is in-memory (amy serve's default): every run +# starts from an empty relay, matching the state wipe the harnesses do. +start_local_relay() { + banner "Starting embedded relay (amy serve / geode) on $RELAY_URL" + local relay_home="$RELAY_DATA/home" + local bind="${RELAY_BIND:-$RELAY_HOST}" + mkdir -p "$relay_home" "$RELAY_DATA/logs" + + [[ -x "$AMY_BIN" ]] || { fail_msg "amy not found at $AMY_BIN — build it with ./gradlew :cli:installDist"; exit 1; } + + # Abort early if something else is already bound to the port — failing + # with a clear error beats a mysterious-looking daemon stall later. + # bash's /dev/tcp probe needs no `ss`/`lsof`; a refused connect on + # loopback returns immediately. + if (exec 3<>"/dev/tcp/$RELAY_HOST/$RELAY_PORT") 2>/dev/null; then + fail_msg "port $RELAY_PORT already in use on $RELAY_HOST — pass --port N or free it" + exit 1 + fi + + # `amy serve` resolves its admin pubkey from the account, so the relay + # needs an identity of its own. Idempotent across --reuse-state runs. + if [[ ! -d "$relay_home/.amy/relay" ]]; then + HOME="$relay_home" "$AMY_BIN" --account relay --secret-backend plaintext --json init \ + >"$RELAY_DATA/logs/init.log" 2>&1 \ + || { fail_msg "amy init failed for the relay account (see $RELAY_DATA/logs/init.log)"; exit 1; } + fi + + nohup env HOME="$relay_home" "$AMY_BIN" --account relay --secret-backend plaintext \ + serve --host "$bind" --port "$RELAY_PORT" \ + >"$RELAY_DATA/logs/stdout.log" 2>"$RELAY_DATA/logs/stderr.log" & + echo "$!" > "$RELAY_DATA/pid" + step "relay pid $(cat "$RELAY_DATA/pid"); waiting for $RELAY_URL …" + + # Readiness = the NIP-11 document answers on the same port. geode serves + # it on a plain GET with `Accept: application/nostr+json` (anything else + # gets a 426 hint, which curl -f would treat as failure). + local deadline=$(( $(date +%s) + 60 )) + while [[ $(date +%s) -lt $deadline ]]; do + if curl -sSf -m 1 -H 'Accept: application/nostr+json' \ + "http://$RELAY_HOST:$RELAY_PORT/" >/dev/null 2>&1; then + info "relay up" + return 0 + fi + if ! kill -0 "$(cat "$RELAY_DATA/pid")" 2>/dev/null; then + break + fi + sleep 0.5 + done + fail_msg "relay never came up (see $RELAY_DATA/logs/stderr.log)" + tail -n 40 "$RELAY_DATA/logs/stderr.log" 2>/dev/null | sed 's/^/ /' >&2 || true + exit 1 +} + +stop_local_relay() { + local pid_file="$RELAY_DATA/pid" + [[ -f "$pid_file" ]] || return 0 + local pid; pid=$(cat "$pid_file" 2>/dev/null || echo "") + if [[ -n "$pid" ]] && kill -0 "$pid" 2>/dev/null; then + info "stopping relay pid $pid" + kill "$pid" 2>/dev/null || true + sleep 1 + kill -9 "$pid" 2>/dev/null || true + fi + rm -f "$pid_file" +} + # --- wn-side pollers (delegates to lib.sh) ----------------------------------- # Both exist in lib.sh already; this file only adds headless-specific niceties. diff --git a/cli/tests/marmot/marmot-interop-headless.sh b/cli/tests/marmot/marmot-interop-headless.sh index 5aa8a3786d..e1d7e8d2e8 100755 --- a/cli/tests/marmot/marmot-interop-headless.sh +++ b/cli/tests/marmot/marmot-interop-headless.sh @@ -3,9 +3,9 @@ # marmot-interop-headless.sh — zero-prompt, zero-internet interop harness. # # Drives Identity A via the `amy` CLI (./gradlew :cli:installDist) and -# Identities B/C via MDK's `wn`/`wnd`. Spins up a local -# nostr-rs-relay on ws://127.0.0.1:$RELAY_PORT so nothing ever leaves the -# machine. Matches the 13 test scenarios in marmot-interop.sh but without +# Identities B/C via MDK's `wn`/`wnd`. Spins up a local embedded relay +# (`amy serve`, i.e. geode) on ws://127.0.0.2:$RELAY_PORT so nothing ever +# leaves the machine. Matches the 13 test scenarios in marmot-interop.sh but without # any human prompts — all checks run to completion and the exit code # reflects pass/fail totals. # @@ -37,9 +37,10 @@ WN_BIN="$WN_REPO/target/release/wn" WND_BIN="$WN_REPO/target/release/wnd" AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" -# Local relay wiring — cloned + built during preflight, started on -# $RELAY_PORT. The harness never touches the public internet for test -# traffic; wn/wnd/amy all point at this one loopback endpoint. +# Local relay wiring — the embedded `amy serve` (geode), started on +# $RELAY_PORT by start_local_relay (../headless/helpers.sh). The harness +# never touches the public internet for test traffic; wn/wnd/amy all +# point at this one loopback endpoint. # # Bind to 127.0.0.2 rather than 127.0.0.1: Quartz's RelayUrlNormalizer # strips literal 127.0.0.1 / localhost / 192.168.* out of NIP-17 inbox @@ -48,8 +49,6 @@ AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" # Amethyst's public defaults instead of the loopback. 127.0.0.2 is # still pure loopback (no network traffic) but isn't on the strip list. RELAY_HOST="${RELAY_HOST:-127.0.0.2}" -RELAY_REPO="${RELAY_REPO:-$STATE_DIR/nostr-rs-relay}" -RELAY_BIN="$RELAY_REPO/target/release/nostr-rs-relay" RELAY_DATA="$STATE_DIR/relay" RELAY_PORT="${RELAY_PORT:-8080}" RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT" @@ -73,7 +72,7 @@ BLOSSOM_PID="" NO_BUILD=0 # Every run starts from empty stores. wnd already wipes B's and C's data dirs -# on each start, but A's amy home and the relay's SQLite file used to survive, +# on each start, but A's amy home and the relay's state used to survive, # and the leftovers are not inert: a KeyPackage A published in an earlier run # is still on the relay for B to invite with, an old group's kind:445 events # still arrive and fail to decrypt, and A's cursors still say it has seen them. @@ -119,8 +118,8 @@ while [[ $# -gt 0 ]]; do done if [[ $RESET_STATE -eq 1 && -d "$STATE_DIR" ]]; then - # Keep the relay checkout + its build (minutes to rebuild) and the log and - # results history; drop everything that holds protocol state. + # Keep the log and results history; drop everything that holds protocol + # state (the relay is in-memory, so wiping its dir just drops its identity). # # run.env counts as protocol state: it is where tests hand each other group # ids. Leaving it behind a wipe leaves ids naming groups nobody is in any diff --git a/cli/tests/marmot/marmot-interop.sh b/cli/tests/marmot/marmot-interop.sh index b7b9a44d25..dc5bf401eb 100755 --- a/cli/tests/marmot/marmot-interop.sh +++ b/cli/tests/marmot/marmot-interop.sh @@ -5,12 +5,19 @@ # Sequential, all-or-nothing. Script drives the `wn` side automatically and # prompts the human operator at each step that requires Amethyst UI action. # -# Usage: ./marmot-interop.sh [--local-relays] [--transponder] [--no-build] +# Usage: ./marmot-interop.sh [--public-relays] [--port N] [--transponder] [--no-build] +# +# By default the harness boots its own relay — `amy serve`, i.e. geode — bound +# to 0.0.0.0:$RELAY_PORT so the wn daemons reach it on loopback and the phone +# reaches it over the LAN (ws://:PORT, or ws://10.0.2.2:PORT from an +# emulator). Pass --public-relays to run the old real-world path against the +# public relay set instead; that is the only mode that touches the internet. # set -uo pipefail SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" TESTS_DIR="$(cd -- "$SCRIPT_DIR/.." && pwd)" +REPO_ROOT="$(cd -- "$SCRIPT_DIR/../../.." && pwd)" STATE_DIR="$SCRIPT_DIR/state" LOG_DIR="$STATE_DIR/logs" B_DIR="$STATE_DIR/B" @@ -27,6 +34,17 @@ RESULTS_FILE="$STATE_DIR/results-$RUN_TS.tsv" WN_REPO="${WN_REPO:-$STATE_DIR/mdk}" WN_BIN="" WND_BIN="" +AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" + +# Embedded relay (default mode). Bound on every interface so a device on the +# same network can reach it; the daemons connect over loopback. Loopback +# `ws://` relays are only accepted by MDK behind this explicit opt-in. +RELAY_HOST="127.0.0.1" +RELAY_BIND="0.0.0.0" +RELAY_PORT="${RELAY_PORT:-8080}" +RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT" +RELAY_DATA="$STATE_DIR/relay" +export WN_ALLOW_LOOPBACK_RELAYS=1 B_NPUB="" B_HEX="" C_NPUB="" @@ -34,6 +52,7 @@ C_HEX="" A_NPUB="" A_HEX="" +# Only used with --public-relays. DEFAULT_RELAYS=( "wss://relay.damus.io" "wss://nos.lol" @@ -41,7 +60,7 @@ DEFAULT_RELAYS=( "wss://nostr.bitcoiner.social" "wss://nostr.mom" ) -USE_LOCAL_RELAYS=0 +USE_PUBLIC_RELAYS=0 ENABLE_TRANSPONDER=0 NO_BUILD=0 @@ -50,7 +69,9 @@ usage() { marmot-interop.sh — Amethyst <-> MDK interop harness Options: - --local-relays Use ws://localhost:8080 instead of public relays (requires 'just docker-up') + --public-relays Use the public relay set instead of the embedded relay + (amy serve / geode, the default). Only mode that leaves the machine. + --port N Port for the embedded relay (default 8080) --transponder Run Test 14 (MIP-05 push notifications) --no-build Don't rebuild wn/wnd if binaries are missing -h, --help Show this help @@ -62,8 +83,10 @@ EOF while [[ $# -gt 0 ]]; do case "$1" in - --local-relays) USE_LOCAL_RELAYS=1 ;; - --transponder) ENABLE_TRANSPONDER=1 ;; + --public-relays) USE_PUBLIC_RELAYS=1 ;; + --local-relays) printf '%s\n' "note: --local-relays is now the default (embedded amy serve relay); flag ignored" >&2 ;; + --port) RELAY_PORT="$2"; RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT"; shift ;; + --transponder) ENABLE_TRANSPONDER=1 ;; --no-build) NO_BUILD=1 ;; -h|--help) usage; exit 0 ;; *) printf 'unknown flag: %s\n' "$1" >&2; usage; exit 2 ;; @@ -77,6 +100,8 @@ mkdir -p "$STATE_DIR" "$LOG_DIR" "$B_DIR/logs" "$C_DIR/logs" # shellcheck source=../lib.sh source "$TESTS_DIR/lib.sh" +# shellcheck source=../headless/helpers.sh — start_local_relay / stop_local_relay (embedded amy serve) +source "$TESTS_DIR/headless/helpers.sh" # --- preflight --------------------------------------------------------------- preflight() { @@ -88,6 +113,26 @@ preflight() { printf ' %s: %s\n' "$cmd" "$(command -v "$cmd")" >>"$LOG_FILE" done + # The embedded relay is `amy serve`, so amy has to exist unless the run + # goes to the public relays. Same transient-503 retry as the headless + # harness: one bad jitpack/dl.google.com roll must not abort the run. + if [[ "$USE_PUBLIC_RELAYS" -ne 1 && ! -x "$AMY_BIN" ]]; then + if [[ "$NO_BUILD" -eq 1 ]]; then + fail_msg "amy not found at $AMY_BIN and --no-build set"; exit 1 + fi + local attempt max_attempts=4 + for attempt in $(seq 1 $max_attempts); do + step "building :cli:installDist (attempt $attempt/$max_attempts)" + if ( cd "$REPO_ROOT" && ./gradlew :cli:installDist ) 2>&1 | tee -a "$LOG_FILE" \ + && [[ -x "$AMY_BIN" ]]; then + break + fi + [[ "$attempt" -lt "$max_attempts" ]] && warn "gradle build failed (likely transient jitpack/Google 503) — retrying" + done + [[ -x "$AMY_BIN" ]] || { fail_msg "amy still missing after build"; exit 1; } + printf ' amy: %s\n' "$AMY_BIN" >>"$LOG_FILE" + fi + WN_BIN="$WN_REPO/target/release/wn" WND_BIN="$WN_REPO/target/release/wnd" @@ -285,7 +330,7 @@ discover_a_relays() { if [[ -n "$kp_event_id" && "$kp_event_id" != "null" ]]; then info "wn_b found A's KeyPackage (kind:30443) — discovery plane is working" else - warn "wn_b could NOT find A's KeyPackage. wn is bootstrapped on ${DEFAULT_RELAYS[*]}." + warn "wn_b could NOT find A's KeyPackage. wn is bootstrapped on ${RELAY_LIST[*]}." warn "Either Amethyst never published a KeyPackage, or it's only on relays wn can't reach." warn "All later tests will fail. Fix this before continuing (tap KP publish in Amethyst settings)." fi @@ -300,7 +345,7 @@ discover_a_relays() { if ! command -v sqlite3 >/dev/null 2>&1; then warn "sqlite3 not installed — skipping wn user_relays cache probe." - warn "If Test 03 fails with 'no invite arrived', install sqlite3 or rerun with --local-relays." + warn "If Test 03 fails with 'no invite arrived', install sqlite3 or rerun without --public-relays." return fi @@ -383,12 +428,7 @@ discover_a_relays() { # --- relays ------------------------------------------------------------------ configure_relays() { banner "Configuring relays" - local relays=() - if [[ "$USE_LOCAL_RELAYS" -eq 1 ]]; then - relays=( "ws://localhost:8080" ) - else - relays=( "${DEFAULT_RELAYS[@]}" ) - fi + local relays=( "${RELAY_LIST[@]}" ) # Each relay × 3 types × 2 daemons produces a lot of repetitive "ok" # lines — the happy path doesn't need any of it on screen. Quiet the # per-add logging into $LOG_FILE and only surface real failures as @@ -527,27 +567,28 @@ configure_relays() { info "sanity kinds 10050/1059/445 ok (B->C welcome + message round-trip)" else warn "kind:445 failed — C never decrypted sanity-ping (relays may be dropping group messages)" - warn "Consider rerunning with --local-relays." + warn "Consider rerunning without --public-relays (the embedded relay accepts every kind)." fi # best-effort cleanup so re-runs don't accumulate dead sanity groups wn_c groups leave "$sanity_c_gid" >/dev/null 2>&1 || true wn_b groups leave "$sanity_gid" >/dev/null 2>&1 || true else warn "kind:10050/1059 failed — C never received welcome; relays likely dropping gift wraps or inbox lists" - warn "Consider rerunning with --local-relays (requires 'just docker-up' in the mdk checkout)." + warn "Consider rerunning without --public-relays (the embedded relay accepts every kind)." fi fi } instruct_amethyst_setup() { - if [[ "$USE_LOCAL_RELAYS" -eq 1 ]]; then - # Offline/sandbox path: we own the only relay, so the harness DOES - # need to dictate Amethyst's relay config — nothing is discoverable - # via the public network. - prompt_human "Configure Amethyst to match this --local-relays harness: + if [[ "$USE_PUBLIC_RELAYS" -ne 1 ]]; then + # Offline/sandbox path (default): we own the only relay — the embedded + # `amy serve` (geode) on 0.0.0.0:$RELAY_PORT — so the harness DOES need + # to dictate Amethyst's relay config; nothing is discoverable via the + # public network. + prompt_human "Configure Amethyst to use this harness's embedded relay (amy serve / geode): 1. Settings -> Relays: add as READ+WRITE - ws://10.0.2.2:8080 (Android emulator) - ws://:8080 (physical device on same Wi-Fi) + ws://10.0.2.2:$RELAY_PORT (Android emulator) + ws://:$RELAY_PORT (physical device on same Wi-Fi) 2. Settings -> Key Package Relays: add the SAME URL 3. Settings -> DM Inbox Relays (NIP-17/kind:10050): add the SAME URL 4. Trigger key-package publish (toggle KP relay on/off if needed) @@ -555,7 +596,7 @@ instruct_amethyst_setup() { return fi - # Public-relay path: the harness should behave like any real Nostr + # --public-relays path: the harness should behave like any real Nostr # client — discover A's advertised relays via kind:10002 / 10050 / # 10051 and publish there, rather than forcing A to adopt the # harness's own relay set. That lets the tests surface real-world @@ -1301,6 +1342,7 @@ main() { local rc=$? trap - EXIT INT TERM HUP stop_daemons + stop_local_relay print_summary exit "$rc" } @@ -1311,6 +1353,12 @@ main() { banner "Amethyst <-> MDK interop harness ($RUN_TS)" preflight + if [[ "$USE_PUBLIC_RELAYS" -eq 1 ]]; then + RELAY_LIST=( "${DEFAULT_RELAYS[@]}" ) + else + RELAY_LIST=( "$RELAY_URL" ) + start_local_relay + fi start_daemon B "$B_DIR" "$B_SOCKET" start_daemon C "$C_DIR" "$C_SOCKET" ensure_identity B @@ -1327,7 +1375,7 @@ main() { # plane, then summarise what wn sees. Surfaces up front the kind of # failure (A's 10050 unreachable from wn, missing KP list, etc.) that # would otherwise bite as a silent Test 03 timeout. - if [[ "$USE_LOCAL_RELAYS" -ne 1 ]]; then + if [[ "$USE_PUBLIC_RELAYS" -eq 1 ]]; then discover_a_relays fi diff --git a/cli/tests/marmot/setup.sh b/cli/tests/marmot/setup.sh index 1bf5e32db4..3b1ceabafb 100644 --- a/cli/tests/marmot/setup.sh +++ b/cli/tests/marmot/setup.sh @@ -147,29 +147,8 @@ preflight() { info "wn: $WN_BIN ($(git -C "$WN_REPO" rev-parse --short HEAD 2>/dev/null || echo unknown))" info "wnd: $WND_BIN" - # Clone/build nostr-rs-relay — the harness's single loopback relay. - if [[ ! -x "$RELAY_BIN" ]]; then - if [[ "$NO_BUILD" -eq 1 ]]; then - fail_msg "nostr-rs-relay not found at $RELAY_BIN and --no-build set"; exit 1 - fi - if [[ ! -d "$RELAY_REPO/.git" ]]; then - step "cloning nostr-rs-relay into $RELAY_REPO" - git clone --depth 1 https://github.com/scsibug/nostr-rs-relay "$RELAY_REPO" \ - 2>&1 | tee -a "$LOG_FILE" - fi - local attempt max=4 - for attempt in $(seq 1 $max); do - step "building nostr-rs-relay (attempt $attempt/$max, ~3 min first run)" - ( cd "$RELAY_REPO" && cargo build --release --bin nostr-rs-relay ) \ - 2>&1 | tee -a "$LOG_FILE" - [[ -x "$RELAY_BIN" ]] && break - [[ "$attempt" -lt "$max" ]] && warn "nostr-rs-relay build failed (likely transient 503 from crates.io) — retrying" - done - [[ -x "$RELAY_BIN" ]] || { - fail_msg "nostr-rs-relay still missing after $max build attempts"; exit 1 - } - fi - info "relay bin: $RELAY_BIN" + # The loopback relay is `amy serve` (geode) — see start_local_relay in + # ../headless/helpers.sh. Nothing to clone or build beyond amy itself. } # --- local QUIC broker ------------------------------------------------------- @@ -262,75 +241,8 @@ stop_quic_broker() { } # --- local relay ------------------------------------------------------------- -# Start nostr-rs-relay on $RELAY_PORT with a minimal config. Every test -# runs against this one loopback endpoint — no external network traffic. -start_local_relay() { - banner "Starting local nostr-rs-relay on $RELAY_URL" - mkdir -p "$RELAY_DATA" "$RELAY_DATA/logs" - - # Render a minimal config file each run so port/limits come from the - # harness rather than whatever was left on disk from a previous session. - cat >"$RELAY_DATA/config.toml" </dev/null | awk '{print $4}' | grep -qE "[:.]$RELAY_PORT\$"; then - fail_msg "port $RELAY_PORT already in use — pass --port N or free it" - exit 1 - fi - - nohup "$RELAY_BIN" --db "$RELAY_DATA" --config "$RELAY_DATA/config.toml" \ - >"$RELAY_DATA/logs/stdout.log" 2>"$RELAY_DATA/logs/stderr.log" & - echo "$!" > "$RELAY_DATA/pid" - step "relay pid $(cat "$RELAY_DATA/pid"); waiting for $RELAY_URL …" - - local deadline=$(( $(date +%s) + 20 )) - while [[ $(date +%s) -lt $deadline ]]; do - if curl -sSf -m 1 "http://${RELAY_HOST:-127.0.0.1}:$RELAY_PORT/" >/dev/null 2>&1; then - info "relay up" - return 0 - fi - sleep 0.5 - done - fail_msg "relay never came up (see $RELAY_DATA/logs/stderr.log)" - tail -n 40 "$RELAY_DATA/logs/stderr.log" 2>/dev/null | sed 's/^/ /' >&2 || true - exit 1 -} - -stop_local_relay() { - local pid_file="$RELAY_DATA/pid" - [[ -f "$pid_file" ]] || return 0 - local pid; pid=$(cat "$pid_file" 2>/dev/null || echo "") - if [[ -n "$pid" ]] && kill -0 "$pid" 2>/dev/null; then - info "stopping relay pid $pid" - kill "$pid" 2>/dev/null || true - sleep 1 - kill -9 "$pid" 2>/dev/null || true - fi - rm -f "$pid_file" -} +# start_local_relay / stop_local_relay live in ../headless/helpers.sh: the +# relay is the embedded `amy serve` (geode), shared by every harness. # --- daemons ----------------------------------------------------------------- start_daemon() { @@ -480,9 +392,9 @@ configure_relays() { step "publishing A's KeyPackage" amy_a marmot key-package publish >>"$LOG_FILE" 2>&1 || warn "amy marmot key-package publish failed" - # Give nostr-rs-relay a breath to fsync the kind:10002 / 10050 / 30443 + # Give the relay a breath to ingest the kind:10002 / 10050 / 30443 # writes and push them out on the discovery subscription so that the # first `wn keys check` that follows actually sees them instead of - # racing the relay's WAL flush. + # racing the relay's ingest queue. sleep 2 } From cfa6f7276052884badf9a872777242d7b39eef9d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 16:43:59 +0000 Subject: [PATCH 05/16] fix(blossom): take a third cache look once the in-flight slot is held BlossomReadAuthTokenProviderTest.aFastSignerStillSharesOneSignature still fails about one run in three (round N: expected 1 signature, was 2), which the pre-push hook turns into a hard block on every push. The second cache look added in 6dc631e8 closes the leader-finished-early window for callers that reach it after the leader retired its entry, but not for a caller parked between that look and its own putIfAbsent: a leader that starts after the caller's miss can sign, cache and retire in that gap (a local key does it in microseconds), so the parked caller's putIfAbsent then succeeds against an empty map and mints a second token. Once the caller holds the in-flight slot, any earlier leader has already cached, because a leader caches before it retires. So a cache hit taken at that point is definitive: hand the cached token to ourselves and to every follower already parked on our deferred, and retire the slot. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PguqnDbP2v11dtANs9xdxc --- .../service/http/BlossomReadAuthTokenProvider.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt index 7c635ce442..618dd57934 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt @@ -129,6 +129,20 @@ class BlossomReadAuthTokenProvider( val fresh = CompletableDeferred() inFlight.putIfAbsent(host, fresh)?.let { return it } + // Third look, taken while holding the [inFlight] slot. The second look above + // still leaves a window: a caller that missed the cache there can be parked + // before its `putIfAbsent` while a leader that started after it signs, caches + // and retires its own entry — a local key does all of that in microseconds — + // so the parked caller's `putIfAbsent` then succeeds against an empty map and + // mints a second signature. With the slot held, any earlier leader has already + // cached (it caches *before* retiring), so a hit here is definitive: hand the + // cached token to ourselves and to every follower that already joined [fresh]. + cachedHeader(host)?.let { cached -> + inFlight.remove(host, fresh) + fresh.complete(cached) + return fresh + } + scope .launch { val header = From 6e3af61e6a321347d62246e2b9aaaa70f3badeab Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 17:50:14 +0000 Subject: [PATCH 06/16] fix: clear the Gradle 10 deprecation warnings in the build scripts Every build printed "Deprecated Gradle features were used in this build, making it incompatible with Gradle 10". With --warning-mode all that was five distinct Kotlin DSL delegated-property deprecations, all in our own scripts: - `val x by extra(...)` / `val x: T by extra` in the root script, for the opt-in Sonar gate that buildscript {} publishes and the body reads. Now extra.set("x", v) and extra["x"] as T. - `val x by getting { }` for eight of quartz's KMP source sets. Now getByName("x") { }, which is what commons already used. None of those vals were referenced, so the local binding goes away with them. - `val x by tasks.registering { }` and the typed `by tasks.registering(T::class) { }`, thirteen tasks across quartz, commons, cli, geode, nestsClient and desktopApp. Now tasks.register("x") { } and tasks.register("x") { }, which return the same TaskProvider, so the dependsOn / finalizedBy references to them are unchanged. `./gradlew --warning-mode all help` is now silent, and all nineteen converted tasks still register and run. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0123kXtseu4X18hL3GMDcdER --- build.gradle.kts | 18 +++++++++--------- cli/build.gradle.kts | 2 +- commons/build.gradle.kts | 2 +- desktopApp/build.gradle.kts | 6 +++--- geode/build.gradle.kts | 2 +- nestsClient/build.gradle.kts | 12 ++++++------ quartz/build.gradle.kts | 18 +++++++++--------- 7 files changed, 30 insertions(+), 30 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 6b87fbd049..d3ea321ee8 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -9,18 +9,18 @@ import java.util.Properties // compiles this buildscript {} section in an earlier stage that can't see the // file's imports (hence the qualified Properties) or share code with the body, // but it can publish values — the gate is computed once here and read below -// via `by extra`. +// via the project's extra properties. buildscript { val localProperties = File(rootDir, "local.properties") - val sonarProperties by extra( + val sonarProperties = java.util.Properties().apply { if (localProperties.exists()) localProperties.inputStream().use { load(it) } - }, - ) - val sonarEnabled by extra( + } + extra.set("sonarProperties", sonarProperties) + val sonarEnabled = sonarProperties.getProperty("sonar.host.url") != null && - gradle.startParameter.taskNames.any { it.substringAfterLast(":") in setOf("sonar", "sonarqube") }, - ) + gradle.startParameter.taskNames.any { it.substringAfterLast(":") in setOf("sonar", "sonarqube") } + extra.set("sonarEnabled", sonarEnabled) if (sonarEnabled) { repositories { gradlePluginPortal() @@ -110,9 +110,9 @@ subprojects { // `./gradlew sonar` behaves exactly like passing them via -Dsonar.xxx=... on the // command line. sonar.projectKey/projectName default to the root project name // ("Amethyst") and only need overriding in local.properties if desired. -val sonarEnabled: Boolean by extra +val sonarEnabled = extra["sonarEnabled"] as Boolean if (sonarEnabled) { - val sonarProperties: Properties by extra + val sonarProperties = extra["sonarProperties"] as Properties apply(plugin = "org.sonarqube") sonarProperties diff --git a/cli/build.gradle.kts b/cli/build.gradle.kts index cd33bc9080..52ed74887e 100644 --- a/cli/build.gradle.kts +++ b/cli/build.gradle.kts @@ -110,7 +110,7 @@ application { // JVM decodes each one as ASCII (every byte > 0x7F → U+FFFD), and amy // then signs a kind:7 whose `content` is four replacement characters. // Whitenoise rejects it with "Invalid reaction content". -val patchAmyLauncherCharset by tasks.registering { +val patchAmyLauncherCharset = tasks.register("patchAmyLauncherCharset") { val appName = application.applicationName val startScriptsTask = tasks.named("startScripts") dependsOn(startScriptsTask) diff --git a/commons/build.gradle.kts b/commons/build.gradle.kts index 50437bf088..5e2b80f12b 100644 --- a/commons/build.gradle.kts +++ b/commons/build.gradle.kts @@ -278,7 +278,7 @@ tasks.withType().configureEach { // there. Commons gains this gate once FeedDefinitionSerializer.kt has been // migrated off Jackson; future commonMain code must not reintroduce JVM-only // JSON / HTTP deps. -val verifyKmpPurity by tasks.registering { +val verifyKmpPurity = tasks.register("verifyKmpPurity") { group = "verification" description = "Fails if iOS-targeted source sets import JVM-only deps." val checkedDirs = diff --git a/desktopApp/build.gradle.kts b/desktopApp/build.gradle.kts index 7e3fea51fb..82b40e25b2 100644 --- a/desktopApp/build.gradle.kts +++ b/desktopApp/build.gradle.kts @@ -254,7 +254,7 @@ compose.desktop { // The arch is selected at task-execution time from the host JVM's os.arch, so // the same task builds the correct AppImage on both x86_64 and aarch64 hosts. // BUILDING.md documents local-dev fetch. -val createReleaseAppImage by tasks.registering(Exec::class) { +val createReleaseAppImage = tasks.register("createReleaseAppImage") { group = "compose desktop" description = "Package createReleaseDistributable output into a Linux AppImage via appimagetool." dependsOn("createReleaseDistributable") @@ -330,7 +330,7 @@ val createReleaseAppImage by tasks.registering(Exec::class) { // proguarded jkeychain-1.1.0-*.jar with all 117 KB intact), so this task is // a regression guard, not a workaround. It's wired onto every release task so // it fails the build immediately if the .so disappears. -val verifyJkeychainNativeSurvivesProguard by tasks.registering { +val verifyJkeychainNativeSurvivesProguard = tasks.register("verifyJkeychainNativeSurvivesProguard") { description = "Fail the release build if osxkeychain.so is stripped from proguarded output (would break macOS Keychain at runtime)" group = "verification" dependsOn("proguardReleaseJars") @@ -395,7 +395,7 @@ listOf( // into the .app) so the subsequent bundle signing seals already-signed code. // Runs only on macOS with the Developer ID identity exported — a no-op on every // other leg and on unsigned local/PR builds. -val signMacJarNatives by tasks.registering { +val signMacJarNatives = tasks.register("signMacJarNatives") { description = "Codesign macOS Mach-O natives embedded in bundled jars before the .app is sealed + notarized" group = "build" dependsOn("proguardReleaseJars") diff --git a/geode/build.gradle.kts b/geode/build.gradle.kts index 2c8efa600f..6870826151 100644 --- a/geode/build.gradle.kts +++ b/geode/build.gradle.kts @@ -22,7 +22,7 @@ kotlin { // Generate a BuildConfig.kt carrying the app version from the catalog so // RelayInfo.VERSION (reported over NIP-11) tracks releases automatically // instead of being a hand-bumped literal. -val generateVersionFile by tasks.registering { +val generateVersionFile = tasks.register("generateVersionFile") { val versionValue = libs.versions.app.get() val outDir = layout.buildDirectory.dir("generated/version/kotlin") inputs.property("version", versionValue) diff --git a/nestsClient/build.gradle.kts b/nestsClient/build.gradle.kts index 15e463a301..f14d312308 100644 --- a/nestsClient/build.gradle.kts +++ b/nestsClient/build.gradle.kts @@ -154,7 +154,7 @@ val hangInteropCacheDir = val moqRelayVersion = "0.10.25" val moqTokenCliVersion = "0.5.23" -val interopInstallMoqRelay by tasks.registering(Exec::class) { +val interopInstallMoqRelay = tasks.register("interopInstallMoqRelay") { description = "cargo install moq-relay $moqRelayVersion (interop)" group = "interop" commandLine( @@ -178,7 +178,7 @@ val interopInstallMoqRelay by tasks.registering(Exec::class) { doFirst { hangInteropCacheDir.asFile.mkdirs() } } -val interopInstallMoqTokenCli by tasks.registering(Exec::class) { +val interopInstallMoqTokenCli = tasks.register("interopInstallMoqTokenCli") { description = "cargo install moq-token-cli $moqTokenCliVersion (interop)" group = "interop" commandLine( @@ -201,7 +201,7 @@ val interopInstallMoqTokenCli by tasks.registering(Exec::class) { doFirst { hangInteropCacheDir.asFile.mkdirs() } } -val interopBuildSidecars by tasks.registering(Exec::class) { +val interopBuildSidecars = tasks.register("interopBuildSidecars") { description = "cargo build --release for nestsClient/tests/hang-interop sidecars" group = "interop" workingDir = hangInteropDir.asFile @@ -226,7 +226,7 @@ val interopBuildSidecars by tasks.registering(Exec::class) { outputs.dir(hangInteropDir.dir("target/release")) } -val interopBuildHangSidecars by tasks.registering { +val interopBuildHangSidecars = tasks.register("interopBuildHangSidecars") { description = "Build all hang-interop binaries (sidecars + moq-relay + moq-token)." group = "interop" dependsOn(interopBuildSidecars, interopInstallMoqRelay, interopInstallMoqTokenCli) @@ -305,7 +305,7 @@ fun resolveBunBinary(): String { fun resolveNpxBinary(): String = System.getenv("NPX_BIN") ?: System.getProperty("npxBin") ?: "npx" -val interopBuildBrowserHarness by tasks.registering(Exec::class) { +val interopBuildBrowserHarness = tasks.register("interopBuildBrowserHarness") { description = "bun install && bun build for the browser interop harness" group = "interop" workingDir = browserInteropDir.asFile @@ -325,7 +325,7 @@ val interopBuildBrowserHarness by tasks.registering(Exec::class) { outputs.dir(browserInteropDir.dir("dist")) } -val interopInstallPlaywrightChromium by tasks.registering(Exec::class) { +val interopInstallPlaywrightChromium = tasks.register("interopInstallPlaywrightChromium") { description = "Install Playwright Chromium + dependencies for the browser interop harness" group = "interop" workingDir = browserInteropDir.asFile diff --git a/quartz/build.gradle.kts b/quartz/build.gradle.kts index 9253f2c607..e3f46e609b 100644 --- a/quartz/build.gradle.kts +++ b/quartz/build.gradle.kts @@ -304,11 +304,11 @@ kotlin { dependsOn(appleMain) } - val iosArm64Main by getting { + getByName("iosArm64Main") { dependsOn(iosMain.get()) } - val iosSimulatorArm64Main by getting { + getByName("iosSimulatorArm64Main") { dependsOn(iosMain.get()) } @@ -316,11 +316,11 @@ kotlin { dependsOn(appleTest) } - val iosArm64Test by getting { + getByName("iosArm64Test") { dependsOn(iosTest.get()) } - val iosSimulatorArm64Test by getting { + getByName("iosSimulatorArm64Test") { dependsOn(iosTest.get()) } @@ -334,11 +334,11 @@ kotlin { dependsOn(appleTest) } - val macosArm64Main by getting { + getByName("macosArm64Main") { dependsOn(macosMain) } - val macosArm64Test by getting { + getByName("macosArm64Test") { dependsOn(macosTest) } @@ -355,11 +355,11 @@ kotlin { dependsOn(nativeTest) } - val linuxX64Main by getting { + getByName("linuxX64Main") { dependsOn(linuxMain) } - val linuxX64Test by getting { + getByName("linuxX64Test") { dependsOn(linuxTest) } } @@ -383,7 +383,7 @@ dependencies { // Scope: source sets whose code is compiled for at least one non-JVM // target. Excludes jvmAndroid, jvmMain, androidMain (and their tests), // where Jackson and OkHttp are legitimately used. -val verifyKmpPurity by tasks.registering { +val verifyKmpPurity = tasks.register("verifyKmpPurity") { group = "verification" description = "Fails if iOS-targeted source sets import JVM-only deps." val checkedDirs = From 3fefb62f1acbc14aba2c9fbd463bc67d8f1f3a29 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 12 Sep 2026 20:24:11 +0200 Subject: [PATCH 07/16] update cs,pt,de,sv --- amethyst/src/main/res/values-cs/strings.xml | 1 + amethyst/src/main/res/values-de-rDE/strings.xml | 1 + amethyst/src/main/res/values-pt-rBR/strings.xml | 5 +++++ amethyst/src/main/res/values-sv-rSE/strings.xml | 1 + .../composeResources/values-cs/strings.xml | 17 +++++++++++++++++ .../composeResources/values-de-rDE/strings.xml | 17 +++++++++++++++++ .../composeResources/values-pt-rBR/strings.xml | 17 +++++++++++++++++ .../composeResources/values-sv-rSE/strings.xml | 17 +++++++++++++++++ 8 files changed, 76 insertions(+) diff --git a/amethyst/src/main/res/values-cs/strings.xml b/amethyst/src/main/res/values-cs/strings.xml index e11986bf5a..71f7fb1c3e 100644 --- a/amethyst/src/main/res/values-cs/strings.xml +++ b/amethyst/src/main/res/values-cs/strings.xml @@ -2608,4 +2608,5 @@ + Health Connect a Amethyst diff --git a/amethyst/src/main/res/values-de-rDE/strings.xml b/amethyst/src/main/res/values-de-rDE/strings.xml index 56ac560096..077c3c79f3 100644 --- a/amethyst/src/main/res/values-de-rDE/strings.xml +++ b/amethyst/src/main/res/values-de-rDE/strings.xml @@ -2390,4 +2390,5 @@ + Health Connect und Amethyst diff --git a/amethyst/src/main/res/values-pt-rBR/strings.xml b/amethyst/src/main/res/values-pt-rBR/strings.xml index bed8c1770d..3f794a806b 100644 --- a/amethyst/src/main/res/values-pt-rBR/strings.xml +++ b/amethyst/src/main/res/values-pt-rBR/strings.xml @@ -2384,4 +2384,9 @@ + Health Connect e Amethyst + + %1$d item + %1$d itens + diff --git a/amethyst/src/main/res/values-sv-rSE/strings.xml b/amethyst/src/main/res/values-sv-rSE/strings.xml index 1a66e9ba1f..4467f58617 100644 --- a/amethyst/src/main/res/values-sv-rSE/strings.xml +++ b/amethyst/src/main/res/values-sv-rSE/strings.xml @@ -2388,4 +2388,5 @@ rationale screen. Needs to be an Android resource (not a commons Compose resource) because android:label on the manifest entry can only reference @string/. --> Media + Health Connect och Amethyst diff --git a/commons/src/commonMain/composeResources/values-cs/strings.xml b/commons/src/commonMain/composeResources/values-cs/strings.xml index f208838430..b98e5d40ed 100644 --- a/commons/src/commonMain/composeResources/values-cs/strings.xml +++ b/commons/src/commonMain/composeResources/values-cs/strings.xml @@ -2821,4 +2821,21 @@ URL https://example.com %1$s km + Amethyst čte dokončené tréninky, aby za vás mohl předvyplnit příspěvek o tréninku. + Health Connect a Amethyst + Amethyst je sociální klient sítě Nostr. Jeho sekce Tréninky vám umožňuje zveřejnit souhrn dokončeného tréninku na relaye Nostr, které si zvolíte, aby lidé, kteří vás sledují, viděli, co jste dělali. Místo ručního vypisování každého čísla může Amethyst načíst trénink, který vaše hodinky nebo fitness aplikace už uložily do Health Connect, a příspěvek předvyplnit. Předvyplněný příspěvek vždy uvidíte a sami rozhodnete, zda ho zveřejníte. + Co Amethyst čte a proč + Cvičení · jakou aktivitu jste dělali, kdy začala a jak dlouho trvala — to je samotný trénink a zároveň název a doba trvání příspěvku. + Kroky · počet kroků při běhu, chůzi nebo turistice. + Vzdálenost · jak daleko jste se dostali, zobrazená jako vzdálenost běhu, jízdy, chůze nebo plavání. + Aktivní a celkové kalorie · energie spálená při tréninku. Aktivní kalorie se použijí, pokud je váš zdroj zaznamenává; celkové kalorie jsou náhradou pro zdroje, které zaznamenávají pouze celkovou energii. + Převýšení · kolik jste nastoupali, což odlišuje rovinatou jízdu od kopcovité. + Tepová frekvence · průměrná a maximální tepová frekvence během tréninku, běžné měřítko náročnosti. + Co Amethyst nedělá + Čte pouze tréninky dokončené za posledních 7 dní, a to jen když je otevřený editor Tréninků. Na pozadí nečte nikdy. + Nic neopustí váš telefon, dokud sami neklepnete na návrh a příspěvek nezveřejníte. Amethyst nemá žádný server: příspěvek jde na relaye Nostr, které jste nastavili. + Nikdy nic nezapisuje do Health Connect a nikdy nežádá o trasu vašeho cvičení, polohu ani jiný typ zdravotních údajů. + Celá funkce je volitelná. Vypnete ji v Nastavení → Nastavení editoru, nebo kdykoli odeberete oprávnění v Health Connect — zbytek Amethystu funguje dál. + Přečíst si celé zásady ochrany soukromí + Co Amethyst čte diff --git a/commons/src/commonMain/composeResources/values-de-rDE/strings.xml b/commons/src/commonMain/composeResources/values-de-rDE/strings.xml index b291e32aec..aa6ea620b0 100644 --- a/commons/src/commonMain/composeResources/values-de-rDE/strings.xml +++ b/commons/src/commonMain/composeResources/values-de-rDE/strings.xml @@ -2807,4 +2807,21 @@ Website Workout %1$s km + Amethyst liest abgeschlossene Workouts, um einen Workout-Beitrag für dich vorauszufüllen. + Health Connect und Amethyst + Amethyst ist ein sozialer Nostr-Client. Im Bereich Workouts kannst du eine Zusammenfassung eines abgeschlossenen Workouts an die Nostr-Relays deiner Wahl veröffentlichen, damit die Leute, die dir folgen, sehen können, was du gemacht hast. Statt jede Zahl von Hand einzutippen, kann Amethyst das Workout lesen, das deine Uhr oder Fitness-App bereits in Health Connect gespeichert hat, und den Beitrag vorausfüllen. Du siehst den vorausgefüllten Beitrag immer und entscheidest, ob du ihn veröffentlichst. + Was Amethyst liest und warum + Übung · welche Aktivität du gemacht hast, wann sie begann und wie lange sie dauerte — das ist das Workout selbst sowie Titel und Dauer des Beitrags. + Schritte · die Schrittzahl eines Laufs, Spaziergangs oder einer Wanderung. + Distanz · wie weit du gekommen bist, angezeigt als Distanz des Laufs, der Fahrt, des Spaziergangs oder des Schwimmens. + Aktive und gesamte Kalorien · die beim Workout verbrannte Energie. Aktive Kalorien werden verwendet, wenn deine Quelle sie aufzeichnet; gesamte Kalorien sind der Ersatz für Quellen, die nur die Gesamtenergie aufzeichnen. + Höhenmeter · wie viel du gestiegen bist, was eine flache Fahrt von einer hügeligen unterscheidet. + Herzfrequenz · die durchschnittliche und maximale Herzfrequenz während des Workouts, das übliche Maß für die Anstrengung. + Was Amethyst nicht tut + Liest nur Workouts, die in den letzten 7 Tagen beendet wurden, und nur während der Workout-Editor geöffnet ist. Im Hintergrund wird nie gelesen. + Nichts verlässt dein Telefon, bis du auf einen Vorschlag tippst und den Beitrag selbst veröffentlichst. Amethyst hat keinen Server: Der Beitrag geht an die Nostr-Relays, die du eingerichtet hast. + Schreibt nie etwas in Health Connect und fragt nie nach deiner Trainingsroute, deinem Standort oder anderen Gesundheitsdaten. + Die gesamte Funktion ist optional. Schalte sie unter Einstellungen → Editor-Einstellungen aus oder entziehe die Berechtigungen jederzeit in Health Connect — der Rest von Amethyst funktioniert weiter. + Vollständige Datenschutzerklärung lesen + Was Amethyst liest diff --git a/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml b/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml index 96ce9845e7..597082e5bb 100644 --- a/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml +++ b/commons/src/commonMain/composeResources/values-pt-rBR/strings.xml @@ -2807,4 +2807,21 @@ URL %1$s km Volume + O Amethyst lê treinos concluídos para preencher previamente uma publicação de treino para você. + Health Connect e Amethyst + O Amethyst é um cliente social do Nostr. A seção Treinos permite publicar um resumo de um treino concluído nos relays do Nostr que você escolher, para que quem segue você veja o que você fez. Em vez de digitar cada número à mão, o Amethyst pode ler o treino que seu relógio ou aplicativo de fitness já salvou no Health Connect e preencher a publicação previamente. Você sempre vê a publicação preenchida e decide se quer publicá-la. + O que o Amethyst lê e por quê + Exercício · qual atividade você fez, quando começou e quanto tempo durou — é o treino em si, além do título e da duração da publicação. + Passos · a contagem de passos de uma corrida, caminhada ou trilha. + Distância · o quanto você percorreu, exibido como a distância da corrida, pedalada, caminhada ou natação. + Calorias ativas e totais · a energia gasta no treino. As calorias ativas são usadas quando sua fonte as registra; as calorias totais são a alternativa para fontes que registram apenas a energia total. + Elevação · o quanto você subiu, que é o que distingue um percurso plano de um acidentado. + Frequência cardíaca · a frequência média e máxima durante o treino, a medida padrão do esforço. + O que o Amethyst não faz + Lê apenas treinos concluídos nos últimos 7 dias e somente enquanto o editor de Treinos está aberto. Nunca lê em segundo plano. + Nada sai do seu telefone até você tocar em uma sugestão e publicar por conta própria. O Amethyst não tem servidor: a publicação vai para os relays do Nostr que você configurou. + Nunca grava nada no Health Connect e nunca solicita seu trajeto de exercício, localização ou qualquer outro tipo de dado de saúde. + Todo o recurso é opcional. Desative-o em Configurações → Configurações de composição, ou revogue as permissões no Health Connect a qualquer momento — o restante do Amethyst continua funcionando. + Ler a política de privacidade completa + O que o Amethyst lê diff --git a/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml b/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml index f987afd543..10685379ad 100644 --- a/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml +++ b/commons/src/commonMain/composeResources/values-sv-rSE/strings.xml @@ -2807,4 +2807,21 @@ nostr, tech, blog URL %1$s km + Amethyst läser avslutade träningspass för att kunna förifylla ett träningsinlägg åt dig. + Health Connect och Amethyst + Amethyst är en social Nostr-klient. I avsnittet Träningspass kan du publicera en sammanfattning av ett avslutat träningspass till de Nostr-reläer du väljer, så att de som följer dig kan se vad du har gjort. I stället för att skriva in varje siffra för hand kan Amethyst läsa det träningspass som din klocka eller träningsapp redan har sparat i Health Connect och förifylla inlägget. Du ser alltid det förifyllda inlägget och avgör själv om det ska publiceras. + Vad Amethyst läser och varför + Övning · vilken aktivitet du gjorde, när den började och hur länge den varade — det är själva träningspasset och även inläggets titel och varaktighet. + Steg · antalet steg under en löprunda, promenad eller vandring. + Distans · hur långt du tog dig, som visas som distansen för löprundan, cykelturen, promenaden eller simningen. + Aktiva och totala kalorier · energin som träningspasset förbrände. Aktiva kalorier används när din källa registrerar dem; totala kalorier är reserven för källor som bara registrerar total energi. + Höjdmeter · hur mycket du klättrade, vilket skiljer en platt tur från en kuperad. + Puls · genomsnittlig och maximal puls under träningspasset, det vanliga måttet på hur ansträngande det var. + Vad Amethyst inte gör + Läser bara träningspass som avslutades de senaste 7 dagarna, och bara medan träningsredigeraren är öppen. Den läser aldrig i bakgrunden. + Ingenting lämnar din telefon förrän du trycker på ett förslag och publicerar inlägget själv. Amethyst har ingen server: inlägget går till de Nostr-reläer du har ställt in. + Skriver aldrig något till Health Connect och begär aldrig din träningsrutt, plats eller någon annan typ av hälsodata. + Hela funktionen är valfri. Stäng av den under Inställningar → Skrivinställningar, eller återkalla behörigheterna i Health Connect när som helst — resten av Amethyst fortsätter att fungera. + Läs hela integritetspolicyn + Vad Amethyst läser From 54252b0fb95209da1f9132d76c7d5483b609d8aa Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 12 Sep 2026 20:43:27 +0200 Subject: [PATCH 08/16] docs(skill): teach find-missing-translations to seed source-identical values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Crowdin workflow now passes import_eq_suggestions: true, so a value equal to the English source is no longer skipped on upload — the repo's locale files are the seed for Crowdin's database. The skill said the opposite ("Don't add source-identical fallbacks"), which now leaves keys untranslated in the UI forever. - Background rewritten for the flag, with the confirming evidence: run 34706537802 -> PR #4107 round-tripped 330 identical values with zero net key changes. - Items 1-3: missing keys are actionable in the repo; copy English verbatim, except (trips MissingQuantity) and words a locale would genuinely translate. - Item 4 reconciled with item 3: seeding a key Crowdin holds nothing for sticks; overwriting a value it holds differently still loses. - Records the diff-reading trap: compare key sets per file, never -/+ lines separately, or a reorder reads as a mass strip. --- .../skills/find-missing-translations/SKILL.md | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/.claude/skills/find-missing-translations/SKILL.md b/.claude/skills/find-missing-translations/SKILL.md index 50699978e5..548dc3b635 100644 --- a/.claude/skills/find-missing-translations/SKILL.md +++ b/.claude/skills/find-missing-translations/SKILL.md @@ -7,7 +7,7 @@ description: Use when comparing Android strings.xml locale files to find untrans ## Overview -Extract string resource keys from a default `values/strings.xml` that are absent in a target locale's `strings.xml`, excluding non-translatable entries. Outputs missing keys and offers to translate them. +Extract string resource keys from a default `values/strings.xml` that are absent in a target locale's `strings.xml`, excluding non-translatable entries. Outputs the missing keys, then offers the two things that close them: **translate** the ones needing translation, and **copy the English value verbatim** for the ones a locale deliberately keeps in English (since 2026-09-12 that copy is what seeds Crowdin — see Background). The repo now has **two independent Crowdin-managed resource trees** — you must scan **both** (see "Resource trees" below). @@ -67,15 +67,23 @@ grep -nE '"' commons/src/commonMain/composeResources/values **Do not** treat the value-overlap as something to deduplicate during a translation pass. Migrating amethyst's own screens onto the shared `action_*` strings is a *separate, optional* refactor and a maintainer call — out of scope for this skill. Just translate each tree correctly and independently. -## Background: Crowdin strip-identical behavior +## Background: source-identical translations and the `import_eq_suggestions` flag -This repo syncs translations via Crowdin (branch `l10n_crowdin_translations`). Crowdin's default export behavior **omits any translation that exactly equals the source**, so a key that the translator deliberately kept as English (common for brand terms like `"Nowhere Drop"`, single-word loanwords like `"Apps"` / `"Feed"` / `"Issues"`, or version prefixes like `"v%1$s"`) will not appear in the locale's `strings.xml` even though the Crowdin UI shows it as 100% translated. +This repo syncs translations via Crowdin (branch `l10n_crowdin_translations`). Crowdin does not *store* a translation that exactly equals the source unless it is told to, so historically a key a translator deliberately kept as English (brand terms like `"Nowhere Drop"`, single-word loanwords like `"Apps"` / `"Feed"` / `"Issues"`, version prefixes like `"v%1$s"`) never appeared in the locale's `strings.xml`, even though the Crowdin UI showed it as 100% translated. + +**That changed on 2026-09-12.** `.github/workflows/crowdin.yml` now passes `import_eq_suggestions: true` to `crowdin/github-action`, so `upload_translations` no longer skips values equal to the source — whatever sits in the repo's locale files is seeded into Crowdin's database, identical values included. `auto_approve_imported` stays at its default `false`, so they arrive as **pending** translations for a translator to approve. + +Confirmed end-to-end the same day: the first sync after the flag landed (workflow run `34706537802` → PR #4107) rewrote all five touched locale files in Crowdin's own key order with **zero net key changes** — 323 additions and 323 removals that pair up exactly. All 330 identical values pushed that morning came back down intact, unapproved included. Since Crowdin's download *replaces* file content with its export, a value it did not hold would have vanished; none did. + +**Reading such a sync diff: compare key *sets* per file, never `-`/`+` lines separately.** A reorder looks identical to a mass strip under `grep '^-'`, and it will convince you the mechanism failed when nothing changed at all. What this means for this skill: -1. **The raw on-disk diff is the candidate set.** A key missing from a locale file is either genuinely untranslated *or* a source-identical entry Crowdin stripped. Both are reported; the human decides which to skip. The Crowdin web UI ("N untranslated") is the ground truth for what genuinely needs work. -2. **Source-identical entries are a small, recognizable minority.** Brand terms (`Nowhere X`), single-word loanwords (`Apps` / `Feed` / `Issues`), and bare version/format strings (`v%1$s`) are the usual cases. Skip these by inspection rather than translating them to something identical. -3. **Don't add source-identical fallbacks.** Android falls back to `values/strings.xml` at runtime, so a key intentionally kept as English already renders correctly, and Crowdin's next sync would strip a local duplicate anyway. +1. **The raw on-disk diff is the candidate set.** A key missing from a locale file is genuinely untranslated, *or* a source-identical entry stripped before 2026-09-12 that no sync has re-seeded yet. Both are reported, and both are now actionable in the repo — translate the first, copy English into the second. The Crowdin web UI ("N untranslated") remains the ground truth for what needs human work. +2. **Source-identical entries are still recognizable, but no longer skipped.** Brand terms (`Nowhere X`), loanwords (`Apps` / `Feed` / `Issues`), symbol- or format-only values (`v%1$s`, `+%1$d`, `%1$d/%2$d`, `∞`, 👀) and example placeholders (`iPhone 13`, `https://example.com`) are the usual cases. Copy the English value into the locale file verbatim so the upload can seed it. +3. **DO add source-identical values — that is now the mechanism, not churn.** A key absent from a locale file is invisible to `upload_translations`; writing the English value in is what gets it into Crowdin, so a translator approves it once in bulk instead of typing it into the UI ~70 times per locale. (Runtime behaviour is unchanged either way: Android still falls back to `values/strings.xml`.) Two exclusions: + - **Never for ``.** Copying English `one`/`other` into cs/pl trips `MissingQuantity`, which is a CI error (cs needs `one`/`few`/`many`/`other`). Plurals stay a Crowdin-UI job. + - **Not for words a locale would genuinely translate.** German `buzz_dm_workspace` ("Arbeitsbereich"), `workout` ("Training"), `relay_group_threads_title` ("Themen"), `calendar_rsvp_section` ("Zusagen") are *gaps*, not deliberate English keeps. Copying English there seeds a wrong pending suggestion — list those for the human to translate rather than approve. 4. **A repo-side edit to a translated value only sticks where Crowdin's database doesn't contradict it.** Download replaces file content with Crowdin's current @@ -96,6 +104,13 @@ What this means for this skill: from `values/strings.xml` removes it project-wide, and attributes declared there propagate into every export. + **This does not contradict item 3 — the two cases differ.** Seeding a key + Crowdin holds *nothing* for (the identical-value copy) sticks, because there is + no stored value to contradict it; that is exactly why the copy pass works. + *Overwriting* a value Crowdin already holds differently — including an empty + one — still loses on the next sync. Add missing entries in the repo; change + existing translations in the UI. + > **Historical note:** an earlier version of this skill tried to auto-filter the > candidate list with a git "sync-timestamp" heuristic (skip any key added before > the last `New Crowdin translations` commit). It was **dropped** because it @@ -172,7 +187,7 @@ comm -23 \ This gives two lists of missing key names — keep them separate; `` translations need the per-locale CLDR category set (see Step 5 → "Plurals: handle with care"). -Crowdin can asymmetrically strip keys across locales (each translator independently chose source-identical for different keys), so **cs is not a reliable upper bound**. Diff **every** target locale and union the results — don't assume the cs set covers the others. A quick per-locale count is a useful sanity check against the Crowdin UI's "N untranslated": +Locale files are asymmetric — legacy pre-2026-09-12 strips and uneven translator progress both leave different keys missing in different locales — so **cs is not a reliable upper bound**. Diff **every** target locale and union the results — don't assume the cs set covers the others. A quick per-locale count is a useful sanity check against the Crowdin UI's "N untranslated": ```bash for locale in cs de-rDE sv-rSE pt-rBR; do @@ -190,7 +205,7 @@ for locale in cs de-rDE sv-rSE pt-rBR; do done ``` -The combined `strings + plurals` total should line up with the Crowdin web UI's untranslated count for that locale. If it does, the raw diff is your actionable set (minus any source-identical entries you skip by inspection — see Background). +The combined `strings + plurals` total should line up with the Crowdin web UI's untranslated count for that locale. If it does, the raw diff is your actionable set: translate what needs translating, and copy the English value verbatim for the entries a locale keeps in English (see Background). ### 3. Get English values for missing keys @@ -458,7 +473,7 @@ When adding translated strings to locale files: - **Append new strings at the bottom** of the file, just before the closing `` tag. - Do NOT try to insert them in alphabetical or matching order — a separate process handles ordering. -- **Insert into each locale ONLY the keys missing from *that* locale — never a shared "union" block.** Because Crowdin strips keys asymmetrically (Step 2), a key you translate may already exist in some target locales. If you compute one union set of missing keys, translate it, and paste the *same* block into every locale, you will create **duplicate keys** in whichever locales already had them. Drive the insertion off the **per-locale** diff, not the union: +- **Insert into each locale ONLY the keys missing from *that* locale — never a shared "union" block.** Because locale files are asymmetric (Step 2), a key you translate may already exist in some target locales. If you compute one union set of missing keys, translate it, and paste the *same* block into every locale, you will create **duplicate keys** in whichever locales already had them. Drive the insertion off the **per-locale** diff, not the union: ```bash # For each locale, insert only the keys comm -23 reports missing FOR THAT LOCALE. @@ -535,8 +550,8 @@ When adding translated strings to locale files: - **Forgetting `translatable="false"`** — these should never appear in locale files - **Diffing only `` is a separate resource type; a source `` missing from a locale will never show up in a `` diff. Always run the diff twice (once per resource type) as shown in Step 2. The same goes for `` if the project uses it. - **Trusting a git "sync-timestamp" heuristic to pre-filter the list** — this skill used to skip keys added before the last `New Crowdin translations` commit, on the theory that Crowdin had already "decided" them. It was dropped: a key added shortly before an export that translators hadn't reached yet is genuinely missing, so the heuristic silently dropped real work. Use the raw on-disk diff and reconcile against the Crowdin web UI's untranslated count instead. -- **Adding source-identical fallbacks locally** — they get overwritten on the next Crowdin sync. Android falls back to `values/strings.xml` at runtime anyway, so a key intentionally kept as English already renders correctly. Skip these by inspection (brand terms, loanwords, `v%1$s`-style strings); don't translate them to an identical value. -- **Skipping per-locale diffs when only diffing cs** — Crowdin can strip different keys in different locales (each translator's choice), so cs is not a reliable upper bound. Diff each target locale and union the results. +- **Skipping source-identical entries instead of copying them in** — correct before 2026-09-12, wrong now. With `import_eq_suggestions: true` the repo file is the *seed* for Crowdin's database, so a key you leave out stays untranslated in the UI forever and reappears in every future scan. Copy the English value verbatim, except for `` (trips `MissingQuantity`) and words the locale would really translate. (Confirmed by PR #4107: 330 identical values survived the next sync with zero net changes.) +- **Skipping per-locale diffs when only diffing cs** — different keys are missing in different locales (legacy strips plus uneven translator progress), so cs is not a reliable upper bound. Diff each target locale and union the results. - **Pasting the union set of missing keys into every locale → duplicate keys** — the union is the right set to *translate*, but the wrong set to *insert*. A key missing in only some locales, inserted into all of them, duplicates in the ones that already had it. Drive each file's insertion off its own per-locale diff (see Step 6). In `commons`, a duplicate key is build-breaking: `convertXmlValueResourcesForCommonMain` fails with `Duplicated key '…'`. **Always run the post-insertion duplicate + XML-wellformedness gate in Step 6 before declaring done.** (Happened 2026-07-21 with `ps1_save_block` / `podcast_value_for_value` / `chats_history_relays`.) - **Declaring the pass done without running `:amethyst:lintPlayBenchmark`** — the duplicate-key + XML + `convertXmlValueResourcesForCommonMain` gate is necessary but nowhere near sufficient. `MissingQuantity` and `ImpliedQuantity` are errors, there is no lint baseline, and `abortOnError` is on, so a change that compiles and passes every check in Step 6's first half can still take CI red. Compiling is not evidence. (Happened 2026-08-13: 3 lint errors after a clean duplicate/XML gate and a green `compileFdroidDebugKotlin`.) - **Converting a `` to `` with `other` only** — "Crowdin fills the rest" is false; `MissingQuantity` errors immediately and CI fails before any sync. Supply every category the locale uses at conversion time, and re-check the declension rather than reusing the old text for `one`. From 1f4d2bf9b5be9fa1e5e489ff3b22018ca33b08c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 18:47:55 +0000 Subject: [PATCH 09/16] fix: clear the mechanical Android Lint warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lint reports 874 warnings / 0 errors per amethyst variant. 512 of those are in Crowdin-managed values-*/strings.xml (mostly MissingQuantity — a translator's plural missing a quantity) and are not ours to hand-edit; this takes the ones that are mechanical and behaviour-preserving: - EmptySuperCall (24 in amethyst, plus 2 in commons the amethyst run cannot see): ViewModel.onCleared is documented empty, so drop the super calls. - UseKtx (2): Canvas.withTranslation for the LaTeX drawable, and Bitmap.toDrawable for the map pin — the KTX form CLAUDE.md asks for, and both compile to the same calls. - ConstantLocale (1): CalendarEventListCard held its "MMM" formatter in a file-level val, which captures Locale.getDefault() once — month abbreviations stayed in whatever language was active at class init. The formatter is now cached per locale, which keeps the property the original comment was protecting (a formatter per recompose was 500 allocations while scrolling), and the locale comes from LocalLocale.current.platformLocale so the read is observable: Locale.getDefault() inside a composable is not, and Compose's own NonObservableLocale check rates that an error. - UnusedResources (6): the Android Studio new-project wizard's leftover colors (purple_200, teal_200, teal_700, black, white, transparent), each verified unreferenced from Kotlin and XML. purple_500/700 are in use and stay. - UseTomlInstead (3): the debug-only Compose/Perfetto tracing dependencies move into the version catalog. Same coordinates and versions; the catalog already carries BOM-managed versionless entries. playDebug goes from 874 warnings to 838, still 0 errors. Deliberately left, because each is a decision rather than a cleanup: AppLinkWarning (autoVerify only works if the domains serve a matching assetlinks.json), the 126 unused source strings and 10 PluralsCandidate (both churn the translation surface), GradleDependency / NewerVersionAvailable (dependency bumps need the license check), VectorRaster / VectorPath / IconDensities / IconXmlAndPng (redrawing assets), BatteryLife (the battery-optimization helper working as designed), and InlinedApi / ClickableViewAccessibility / DiscouragedApi / InsecureBaseConfiguration (each needs its surrounding intent read first). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0123kXtseu4X18hL3GMDcdER --- amethyst/build.gradle.kts | 6 +++--- .../amethyst/ui/components/LatexEquation.kt | 10 +++++----- .../note/creators/location/LocationPreviewMap.kt | 4 ++-- .../amethyst/ui/screen/UserFeedViewModel.kt | 1 - .../ui/screen/loggedIn/AccountViewModel.kt | 1 - .../screen/loggedIn/buzz/AgentConsoleViewModel.kt | 1 - .../loggedIn/buzz/AgentWorkBoardViewModel.kt | 1 - .../ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt | 1 - .../ui/screen/loggedIn/buzz/JobBoardViewModel.kt | 1 - .../loggedIn/buzz/WorkflowRunBoardViewModel.kt | 1 - .../loggedIn/calendars/CalendarEventListCard.kt | 14 +++++++++++--- .../create/NewCalendarCollectionViewModel.kt | 1 - .../privateDM/send/ChatNewMessageViewModel.kt | 1 - .../send/ChannelNewMessageViewModel.kt | 1 - .../ui/screen/loggedIn/chess/ChessViewModelNew.kt | 1 - .../nip23LongForm/LongFormPostViewModel.kt | 1 - .../nip99Classifieds/NewProductViewModel.kt | 1 - .../screen/loggedIn/home/ShortNotePostViewModel.kt | 1 - .../ui/screen/loggedIn/home/VoiceReplyViewModel.kt | 1 - .../loggedIn/music/AddToMusicPlaylistViewModel.kt | 1 - .../nests/room/chat/NestNewMessageViewModel.kt | 1 - .../publicMessages/NewPublicMessageViewModel.kt | 1 - .../loggedIn/profile/relays/RelayFeedViewModel.kt | 1 - .../loggedIn/settings/StringFeedViewModel.kt | 1 - .../loggedIn/video/hls/NewHlsVideoViewModel.kt | 1 - .../screen/loggedIn/wallet/ReloadMintViewModel.kt | 1 - .../screen/loggedIn/wallet/TopUpMintViewModel.kt | 1 - .../wallet/wizard/CashuWalletWizardViewModel.kt | 1 - amethyst/src/main/res/values/colors.xml | 6 ------ .../amethyst/commons/viewmodels/FeedViewModel.kt | 1 - .../amethyst/commons/viewmodels/NestViewModel.kt | 1 - gradle/libs.versions.toml | 4 ++++ 32 files changed, 25 insertions(+), 45 deletions(-) diff --git a/amethyst/build.gradle.kts b/amethyst/build.gradle.kts index 77010ea9fe..38e3f1d217 100644 --- a/amethyst/build.gradle.kts +++ b/amethyst/build.gradle.kts @@ -399,9 +399,9 @@ dependencies { // Usage: runtime-enable, then capture a Perfetto trace with the `track_event` data source: // adb shell am broadcast -a androidx.tracing.perfetto.action.ENABLE_TRACING \ // -n com.vitorpamplona.amethyst.debug/androidx.tracing.perfetto.TracingReceiver - debugImplementation("androidx.compose.runtime:runtime-tracing") - debugImplementation("androidx.tracing:tracing-perfetto:1.0.1") - debugImplementation("androidx.tracing:tracing-perfetto-binary:1.0.1") + debugImplementation(libs.androidx.compose.runtime.tracing) + debugImplementation(libs.androidx.tracing.perfetto) + debugImplementation(libs.androidx.tracing.perfetto.binary) implementation(project(":quartz")) implementation(project(":commons")) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/LatexEquation.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/LatexEquation.kt index ea0f43dffc..f8b68bce78 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/LatexEquation.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/LatexEquation.kt @@ -40,6 +40,7 @@ import androidx.compose.ui.text.rememberTextMeasurer import androidx.compose.ui.unit.TextUnit import androidx.compose.ui.unit.TextUnitType import androidx.compose.ui.unit.sp +import androidx.core.graphics.withTranslation import com.vitorpamplona.amethyst.commons.richtext.MathParser import ru.noties.jlatexmath.JLatexMathDrawable @@ -130,12 +131,11 @@ fun LatexEquation( Canvas(modifier = equationModifier) { drawIntoCanvas { canvas -> val native = canvas.nativeCanvas - val checkpoint = native.save() // Position the icon's baseline on the text baseline within the padded box. - native.translate(0f, drawTopPx) - drawable.setBounds(0, 0, drawable.intrinsicWidth, drawable.intrinsicHeight) - drawable.draw(native) - native.restoreToCount(checkpoint) + native.withTranslation(y = drawTopPx) { + drawable.setBounds(0, 0, drawable.intrinsicWidth, drawable.intrinsicHeight) + drawable.draw(this) + } } } if (trailing.isNotEmpty()) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/location/LocationPreviewMap.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/location/LocationPreviewMap.kt index 5d9161d1e6..28a5d99dfd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/location/LocationPreviewMap.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/creators/location/LocationPreviewMap.kt @@ -23,7 +23,6 @@ package com.vitorpamplona.amethyst.ui.note.creators.location import android.graphics.ColorFilter import android.graphics.ColorMatrix import android.graphics.ColorMatrixColorFilter -import android.graphics.drawable.BitmapDrawable import android.view.MotionEvent import androidx.compose.foundation.layout.aspectRatio import androidx.compose.foundation.layout.fillMaxWidth @@ -36,6 +35,7 @@ import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.toArgb import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.viewinterop.AndroidView +import androidx.core.graphics.drawable.toDrawable import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleEventObserver import androidx.lifecycle.compose.LocalLifecycleOwner @@ -159,7 +159,7 @@ fun LocationPreviewMap( remember(pinColor, pinEmoji) { if (pinColor != null && pinEmoji != null) { val bitmap = roadEventPinBitmap(pinEmoji, pinColor.toArgb(), context.resources.displayMetrics.density) - BitmapDrawable(context.resources, bitmap) + bitmap.toDrawable(context.resources) } else { null } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt index d2e039deb8..fc7ba0b466 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UserFeedViewModel.kt @@ -118,6 +118,5 @@ open class UserFeedViewModel( override fun onCleared() { Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } bundler.cancel() - super.onCleared() } } 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 cda7e35f72..a1b4069ec7 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 @@ -2647,7 +2647,6 @@ class AccountViewModel( com.vitorpamplona.amethyst.ui.screen.loggedIn.nests.room.activity.NestBridge .clear() feedStates.destroy() - super.onCleared() } fun loadMentions( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt index ad6c0e879b..771037b0ce 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt @@ -310,7 +310,6 @@ class AgentConsoleViewModel : ViewModel() { override fun onCleared() { stopObserving() stopWatching() - super.onCleared() } /** One decrypted observer telemetry frame rendered on the Observer tab. */ diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentWorkBoardViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentWorkBoardViewModel.kt index e3f106f595..fe0d4e9ebf 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentWorkBoardViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentWorkBoardViewModel.kt @@ -218,7 +218,6 @@ class AgentWorkBoardViewModel : ViewModel() { override fun onCleared() { stopWatching() - super.onCleared() } companion object { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt index dd2284163e..4fd4f3509f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt @@ -360,6 +360,5 @@ class BuzzDmListViewModel : ViewModel() { override fun onCleared() { liveJob?.cancel() liveJob = null - super.onCleared() } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/JobBoardViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/JobBoardViewModel.kt index 62ead90720..1f60812ea3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/JobBoardViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/JobBoardViewModel.kt @@ -137,7 +137,6 @@ class JobBoardViewModel : ViewModel() { override fun onCleared() { stopWatching() - super.onCleared() } companion object { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/WorkflowRunBoardViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/WorkflowRunBoardViewModel.kt index 4dc32ff52c..10982c18d5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/WorkflowRunBoardViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/WorkflowRunBoardViewModel.kt @@ -258,7 +258,6 @@ class WorkflowRunBoardViewModel : ViewModel() { override fun onCleared() { stopWatching() - super.onCleared() } companion object { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/CalendarEventListCard.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/CalendarEventListCard.kt index 66ef72ab54..782330a58a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/CalendarEventListCard.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/CalendarEventListCard.kt @@ -41,6 +41,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalLocale import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp @@ -58,11 +59,15 @@ import java.time.Instant import java.time.ZoneId import java.time.format.DateTimeFormatter import java.util.Locale +import java.util.concurrent.ConcurrentHashMap // Thread-safe and hoisted: previously each CalendarDateBadge recompose allocated a new // SimpleDateFormat, which (a) is not thread-safe and (b) created 500 allocations while scrolling. -private val MonthShortFormatter: DateTimeFormatter = - DateTimeFormatter.ofPattern("MMM", Locale.getDefault()) +// Cached per locale rather than in a single val that captures the locale once: the month +// names have to follow a language the user changes while the app is running. +private val monthShortFormatters = ConcurrentHashMap() + +private fun monthShortFormatter(locale: Locale): DateTimeFormatter = monthShortFormatters.getOrPut(locale) { DateTimeFormatter.ofPattern("MMM", locale) } @Composable fun CalendarEventListCard( @@ -214,7 +219,10 @@ private fun CalendarDateBadge(startSeconds: Long?) { Instant.ofEpochSecond(startSeconds).atZone(ZoneId.systemDefault()).toLocalDate() } val day = localDate.dayOfMonth.toString() - val month = remember(localDate) { MonthShortFormatter.format(localDate).uppercase() } + // LocalLocale rather than Locale.getDefault(): the latter is not observable, so a + // locale change while the app runs would leave the month name in the old language. + val locale = LocalLocale.current.platformLocale + val month = remember(localDate, locale) { monthShortFormatter(locale).format(localDate).uppercase() } Column( modifier = Modifier.size(width = 52.dp, height = 60.dp), diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/create/NewCalendarCollectionViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/create/NewCalendarCollectionViewModel.kt index 84e811a228..be0fdf4c00 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/create/NewCalendarCollectionViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/calendars/create/NewCalendarCollectionViewModel.kt @@ -109,7 +109,6 @@ class NewCalendarCollectionViewModel : ViewModel() { override fun onCleared() { liveScanJob?.cancel() - super.onCleared() } fun toggle(address: Address) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/send/ChatNewMessageViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/send/ChatNewMessageViewModel.kt index 4c7e45f109..863e3ff8ef 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/send/ChatNewMessageViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/send/ChatNewMessageViewModel.kt @@ -801,7 +801,6 @@ class ChatNewMessageViewModel : } override fun onCleared() { - super.onCleared() Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/send/ChannelNewMessageViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/send/ChannelNewMessageViewModel.kt index 4b51c95c6e..baab2e6279 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/send/ChannelNewMessageViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/send/ChannelNewMessageViewModel.kt @@ -1027,7 +1027,6 @@ open class ChannelNewMessageViewModel : } override fun onCleared() { - super.onCleared() Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chess/ChessViewModelNew.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chess/ChessViewModelNew.kt index 8966460217..893c23fb17 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chess/ChessViewModelNew.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chess/ChessViewModelNew.kt @@ -132,7 +132,6 @@ class ChessViewModelNew( fun clearFocusedGame() = logic.clearFocusedGame() override fun onCleared() { - super.onCleared() logic.stopPolling() } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt index 0488d076df..ed5ff94326 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt @@ -739,7 +739,6 @@ class LongFormPostViewModel : } override fun onCleared() { - super.onCleared() Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip99Classifieds/NewProductViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip99Classifieds/NewProductViewModel.kt index 3075d4a6a7..32a137507f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip99Classifieds/NewProductViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip99Classifieds/NewProductViewModel.kt @@ -612,7 +612,6 @@ open class NewProductViewModel : } override fun onCleared() { - super.onCleared() Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt index af9a45edd5..cf79d6b90d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt @@ -1977,7 +1977,6 @@ open class ShortNotePostViewModel : } override fun onCleared() { - super.onCleared() writingAssistant?.close() writingAssistant = null Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt index 3ba074d1ca..0bf353eea1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/VoiceReplyViewModel.kt @@ -320,7 +320,6 @@ class VoiceReplyViewModel : ViewModel() { override fun onCleared() { cancel() - super.onCleared() Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/AddToMusicPlaylistViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/AddToMusicPlaylistViewModel.kt index 3707b774fa..23078cb368 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/AddToMusicPlaylistViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/AddToMusicPlaylistViewModel.kt @@ -102,7 +102,6 @@ class AddToMusicPlaylistViewModel : ViewModel() { override fun onCleared() { liveScanJob?.cancel() - super.onCleared() } private suspend fun rescan() { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/chat/NestNewMessageViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/chat/NestNewMessageViewModel.kt index 86ebb445bb..4eb9691cfe 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/chat/NestNewMessageViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/chat/NestNewMessageViewModel.kt @@ -597,7 +597,6 @@ open class NestNewMessageViewModel : } override fun onCleared() { - super.onCleared() Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/publicMessages/NewPublicMessageViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/publicMessages/NewPublicMessageViewModel.kt index 1fbfff94a6..d573313ee9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/publicMessages/NewPublicMessageViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/publicMessages/NewPublicMessageViewModel.kt @@ -660,7 +660,6 @@ class NewPublicMessageViewModel : } override fun onCleared() { - super.onCleared() Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/relays/RelayFeedViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/relays/RelayFeedViewModel.kt index 4f4beac566..bc191f6643 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/relays/RelayFeedViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/relays/RelayFeedViewModel.kt @@ -196,6 +196,5 @@ class RelayFeedViewModel : override fun onCleared() { Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } - super.onCleared() } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/StringFeedViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/StringFeedViewModel.kt index 53d18baf2e..1b0e00f245 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/StringFeedViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/StringFeedViewModel.kt @@ -116,6 +116,5 @@ open class StringFeedViewModel( override fun onCleared() { Log.d("Init") { "OnCleared: ${this.javaClass.simpleName}" } bundler.cancel() - super.onCleared() } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/video/hls/NewHlsVideoViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/video/hls/NewHlsVideoViewModel.kt index f5115a3feb..aed382fcf0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/video/hls/NewHlsVideoViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/video/hls/NewHlsVideoViewModel.kt @@ -180,7 +180,6 @@ open class NewHlsVideoViewModel : ViewModel() { } override fun onCleared() { - super.onCleared() currentJob?.cancel() } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt index 532b8c925a..8a30b81dc5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/ReloadMintViewModel.kt @@ -459,7 +459,6 @@ class ReloadMintViewModel : ViewModel() { // The pipeline runs on the AccountViewModel scope, not this VM's, so it would // outlive the screen — cancel it when the screen goes away. job?.cancel() - super.onCleared() } companion object { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/TopUpMintViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/TopUpMintViewModel.kt index 6fde099208..f6dc022003 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/TopUpMintViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/TopUpMintViewModel.kt @@ -267,7 +267,6 @@ class TopUpMintViewModel : ViewModel() { // The pipeline runs on the AccountViewModel scope, not this VM's, so it would // outlive the screen — cancel it when the screen goes away. job?.cancel() - super.onCleared() } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/wizard/CashuWalletWizardViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/wizard/CashuWalletWizardViewModel.kt index 2b3724c09b..9051c9a277 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/wizard/CashuWalletWizardViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/wizard/CashuWalletWizardViewModel.kt @@ -293,7 +293,6 @@ class CashuWalletWizardViewModel : ViewModel() { } override fun onCleared() { - super.onCleared() discovery?.cancel() } } diff --git a/amethyst/src/main/res/values/colors.xml b/amethyst/src/main/res/values/colors.xml index 611066da6b..1f20970d04 100644 --- a/amethyst/src/main/res/values/colors.xml +++ b/amethyst/src/main/res/values/colors.xml @@ -1,13 +1,7 @@ - #FFBB86FC #FF6200EE #FF3700B3 - #FF03DAC5 - #FF018786 - #FF000000 - #FFFFFFFF - #00FFFFFF diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/FeedViewModel.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/FeedViewModel.kt index 172614a573..5465931342 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/FeedViewModel.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/FeedViewModel.kt @@ -68,6 +68,5 @@ abstract class FeedViewModel( override fun onCleared() { Log.d("Init") { "OnCleared: ${this::class.simpleName}" } - super.onCleared() } } diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt index 6fb1a8088a..80ad79160f 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt @@ -838,7 +838,6 @@ class NestViewModel( closed = true teardownBroadcast(BroadcastUiState.Idle, finalCleanup = true) teardown(targetState = ConnectionUiState.Closed, finalCleanup = true) - super.onCleared() } private fun observeSpeakerState(s: NestsSpeaker) { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0b02920047..266e447dd3 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -28,6 +28,7 @@ uiautomator = "2.4.0" biometricKtx = "1.4.0-alpha02" coil = "3.6.2" composeBom = "2026.09.00" +tracingPerfetto = "1.0.1" composeRuntimeAnnotation = "1.12.1" coreKtx = "1.19.0" datastore = "1.2.1" @@ -136,6 +137,9 @@ androidx-camera-extensions = { module = "androidx.camera:camera-extensions", ver androidx-camera-view = { module = "androidx.camera:camera-view", version.ref = "androidxCamera" } androidx-camera-lifecycle = { module = "androidx.camera:camera-lifecycle", version.ref = "androidxCamera" } androidx-compose-bom = { group = "androidx.compose", name = "compose-bom", version.ref = "composeBom" } +androidx-compose-runtime-tracing = { group = "androidx.compose.runtime", name = "runtime-tracing" } +androidx-tracing-perfetto = { group = "androidx.tracing", name = "tracing-perfetto", version.ref = "tracingPerfetto" } +androidx-tracing-perfetto-binary = { group = "androidx.tracing", name = "tracing-perfetto-binary", version.ref = "tracingPerfetto" } androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation" } androidx-compose-runtime-annotation = { group = "androidx.compose.runtime", name = "runtime-annotation", version.ref = "composeRuntimeAnnotation" } androidx-collection = { group = "androidx.collection", name = "collection", version.ref = "androidxCollection" } From ae2e971dd9f12df69156a915be702fefb9745b84 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 12 Sep 2026 22:29:04 +0200 Subject: [PATCH 10/16] fix(blossom): re-check the token cache after winning the in-flight slot The pre-insert cache look added in 6dc631e85d narrowed the single-flight gap but didn't close it: a fast leader can insert, sign, cache and retire its entry entirely between a straggler's cache read and its putIfAbsent, so the straggler wins an empty map and signs a second time. That is the intermittent `expected:<1> but was:<2>` in aFastSignerStillSharesOneSignature failing main CI. Check the cache again once this caller owns the slot. A leader always caches before retiring its entry, so any token minted before the insert is visible there; take it and give the slot back instead of re-signing. Reproduced locally at round 2431 of 5000; two 5000-round runs pass with the fix. --- .../service/http/BlossomReadAuthTokenProvider.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt index 7c635ce442..2f75de6084 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt @@ -129,6 +129,17 @@ class BlossomReadAuthTokenProvider( val fresh = CompletableDeferred() inFlight.putIfAbsent(host, fresh)?.let { return it } + // Third look, now that this caller owns the slot. The look above still leaves a + // gap: a leader can insert, sign, cache and retire its entry entirely between + // that read and the putIfAbsent, so the map is empty again and this caller wins + // it. Any leader that retired before this insert cached first, so a token + // present now is theirs — take it and give the slot back instead of re-signing. + cachedHeader(host)?.let { + inFlight.remove(host, fresh) + fresh.complete(it) + return fresh + } + scope .launch { val header = From 2ab934b2d217bd64a2c732f0963cebd21c5a865b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 20:49:24 +0000 Subject: [PATCH 11/16] perf: stop debugState walking the whole cache when its output is dropped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MainActivity.onPause() calls debugState() unconditionally, and every line it emits is Log.d built through the *eager* overload — so the arguments are evaluated before the level check. Those arguments are the expensive part: nine materialising LargeCache.filter scans over notes/addressables/users and the three channel maps, nested sums over every channel's and chatroom's notes, two sorted passes, and three passes calling Event.countMemory(), which itself walks every tag and tag element of every cached event. Release builds sit at WARN, so all of that ran on every backgrounding and the result was discarded. Gated on `Log.minLevel > LogLevel.DEBUG` rather than `isDebug`, because that is exactly the condition under which the lines are dropped: benchmark builds are `isDebug` but sit at INFO, so an isDebug gate would have left the one variant whose numbers are meant to be trustworthy still paying the cost. The function only reads and logs — no mutation — so the early return cannot skip a side effect. The same shape is already gated elsewhere: AppModules builds relayReqStats and bootDiagnostics as `if (isDebug) ... else null`, and BootRelayDiagnostics does comparable work. debugState was the one that was not. fix: rethrow CancellationException in six catch blocks All six sat in suspend functions whose try body contains a suspension point, so a cancelled scope surfaced as CancellationException and was swallowed, against the `if (e is CancellationException) throw e` convention this codebase follows in 192 files. What cancellation used to mean: - VanishRequestsState: published ComplianceStatus.ERROR, showing a relay as having answered badly when it was never asked. - NappletResourceFetcher: reported ERROR_NETWORK to the sandboxed page, so a cancelled fetch was indistinguishable from an upstream failure. - MarmotAgentStreamWatcher (two sites): the per-candidate handler does `continue`, so a cancelled watcher kept dialling the remaining broker candidates. - CallSession: logged as a PeerConnection creation failure. - NamecoinSharedPreferences: returned emptyList(), i.e. "no pinned certs". Uses kotlin.coroutines.cancellation.CancellationException, the import that also works from commons/commonMain. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0123kXtseu4X18hL3GMDcdER --- .../java/com/vitorpamplona/amethyst/DebugUtils.kt | 11 +++++++++++ .../amethyst/model/nip62Vanish/VanishRequestsState.kt | 6 +++++- .../model/preferences/NamecoinSharedPreferences.kt | 3 ++- .../napplet/gateways/NappletResourceFetcher.kt | 6 +++++- .../amethyst/ui/call/session/CallSession.kt | 2 ++ .../commons/marmot/MarmotAgentStreamWatcher.kt | 5 +++++ 6 files changed, 30 insertions(+), 3 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/DebugUtils.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/DebugUtils.kt index dcabff9b11..9735216a02 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/DebugUtils.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/DebugUtils.kt @@ -29,6 +29,7 @@ import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.relay.normalizer.normalizedUrls import com.vitorpamplona.quartz.utils.Log +import com.vitorpamplona.quartz.utils.LogLevel import com.vitorpamplona.quartz.utils.bytesUsedInMemory import com.vitorpamplona.quartz.utils.pointerSizeInBytes import kotlin.time.DurationUnit @@ -92,6 +93,16 @@ fun collectMemorySnapshot(context: Context): MemorySnapshot { private const val STATE_DUMP_TAG = "STATE DUMP" fun debugState(context: Context) { + // Everything below is logged at DEBUG, and every argument is built eagerly (the + // eager Log.d overload, not the lambda one). Gate on the level that would drop + // those lines, because the arguments are the expensive part: nine materialising + // LargeCache.filter scans over notes/addressables/users/channels, plus three + // passes calling Event.countMemory() — which walks every tag of every cached + // event. MainActivity.onPause() calls this unconditionally, so without the gate + // a release build (minLevel WARN) did all of that on every backgrounding and + // threw the result away. Benchmark builds sit at INFO and paid it too. + if (Log.minLevel > LogLevel.DEBUG) return + val totalMemoryMb = Runtime.getRuntime().totalMemory() / (1024 * 1024) val freeMemoryMb = Runtime.getRuntime().freeMemory() / (1024 * 1024) val maxMemoryMb = Runtime.getRuntime().maxMemory() / (1024 * 1024) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip62Vanish/VanishRequestsState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip62Vanish/VanishRequestsState.kt index f8123a60fd..63ccacf2a8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip62Vanish/VanishRequestsState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip62Vanish/VanishRequestsState.kt @@ -36,6 +36,7 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.withContext +import kotlin.coroutines.cancellation.CancellationException @Stable data class VanishEventItem( @@ -124,7 +125,10 @@ class VanishRequestsState( } ) } - } catch (_: Exception) { + } catch (e: Exception) { + // A cancelled check has no result. Reporting ERROR would show the relay as + // having answered badly when it was never asked. + if (e is CancellationException) throw e item.complianceResults.update { it + (relay to ComplianceStatus.ERROR) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/NamecoinSharedPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/NamecoinSharedPreferences.kt index 3f37ce3066..0844905ab4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/NamecoinSharedPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/NamecoinSharedPreferences.kt @@ -166,7 +166,8 @@ class NamecoinSharedPreferences( } else { emptyList() } - } catch (_: Exception) { + } catch (e: Exception) { + if (e is CancellationException) throw e emptyList() } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/gateways/NappletResourceFetcher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/gateways/NappletResourceFetcher.kt index 162cc22c9c..32ca910f6f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/gateways/NappletResourceFetcher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/gateways/NappletResourceFetcher.kt @@ -60,6 +60,7 @@ import java.net.URLDecoder import java.nio.ByteBuffer import java.nio.charset.CodingErrorAction import java.util.concurrent.TimeUnit +import kotlin.coroutines.cancellation.CancellationException /** * Fetches a resource URL on an applet's behalf — the applet has no direct network @@ -162,7 +163,10 @@ class NappletResourceFetcher( return failure(ERROR_BLOCKED, e.message) } catch (_: InterruptedIOException) { return failure(ERROR_TIMEOUT) - } catch (_: Exception) { + } catch (e: Exception) { + // Cancellation is not an upstream failure — do not report it to the + // napplet as one, and do not keep the request alive past it. + if (e is CancellationException) throw e return failure(ERROR_NETWORK) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt index 37858696cb..e198c246f5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/session/CallSession.kt @@ -65,6 +65,7 @@ import org.webrtc.RtpSender import org.webrtc.VideoTrack import java.util.UUID import java.util.concurrent.ConcurrentHashMap +import kotlin.coroutines.cancellation.CancellationException private const val TAG = "CallSession" private const val VIDEO_MAX_BITRATE_BPS_DEFAULT = 1_500_000 @@ -429,6 +430,7 @@ class CallSession( try { withContext(Dispatchers.IO) { createWebRtcSession(peerPubKey) } } catch (e: Exception) { + if (e is CancellationException) throw e Log.e(TAG, "Failed to create PeerConnection for ${peerPubKey.take(8)}", e) return } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotAgentStreamWatcher.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotAgentStreamWatcher.kt index 3b4a1e1381..0444058aad 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotAgentStreamWatcher.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotAgentStreamWatcher.kt @@ -37,6 +37,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlin.coroutines.cancellation.CancellationException /** * What a front end renders for one live agent text stream. @@ -203,6 +204,9 @@ class MarmotAgentStreamWatcher( try { quic.subscribe(candidate, start.streamId.hexToByteArray(), startEvent.id.hexToByteArray()) } catch (e: Exception) { + // Without this a cancelled watcher keeps dialling the remaining + // candidates instead of stopping. + if (e is CancellationException) throw e Log.d("MarmotAgentStreamWatcher") { "candidate $candidate unusable: ${e.message}" } continue } @@ -222,6 +226,7 @@ class MarmotAgentStreamWatcher( ) } } catch (e: Exception) { + if (e is CancellationException) throw e Log.d("MarmotAgentStreamWatcher") { "stream from $candidate ended: ${e.message}" } } finally { runCatching { stream.close() } From 42698ed59c7389c3207e2c615f4a6326b89ac889 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 20:57:11 +0000 Subject: [PATCH 12/16] fix(eventsync): drain the outbox before closing and count each send once Audit of the sync path the new geode-backed EventSyncTest exercises found two bugs in EventSync itself, plus review nits on the harness changes. - runSync closed its client (`use {}`) the moment the last page arrived, while `publish` is fire-and-forget through the client's outbox. Events forwarded from the final page of the last relay were still waiting for a socket or an OK when the outbox was destroyed, so the sync reported Done and silently never delivered them. Wait, bounded by the existing per-relay timeout, until no forwarded event has a relay left pending. - The "events sent" counters incremented on every onSent, including the failed write to a destination still connecting and the outbox's at-least-once resend of an unacknowledged event after the connection syncs. Every cold destination therefore reported at least one extra event sent. Count only successful writes, once per (event, relay). The test now asserts the sent total equals the routed total. - Harness: the 127.0.0.2 rationale claimed it survives Quartz's isLocalHost() strip; that filter now covers all of 127.0.0.0/8, so say so and note what it means for the strict-inbox DM cases. The interactive Marmot harness gets an overridable RELAY_HOST/RELAY_BIND and documents the loopback/RFC1918 stripping limit it inherits, and its new --port guards a missing value instead of dying on set -u. - EventSyncTest builds both scenarios through one helper. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PguqnDbP2v11dtANs9xdxc --- .../loggedIn/relays/eventsync/EventSync.kt | 44 ++++++++++++++++++- .../relays/eventsync/EventSyncTest.kt | 38 ++++++++-------- cli/tests/cache/cache-headless.sh | 8 ++-- cli/tests/dm/dm-interop-headless.sh | 10 +++-- cli/tests/headless/helpers.sh | 7 ++- cli/tests/marmot/marmot-interop.sh | 21 ++++++--- 6 files changed, 94 insertions(+), 34 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSync.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSync.kt index 0918f1a13b..0a9066af70 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSync.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSync.kt @@ -43,10 +43,12 @@ import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull import java.util.concurrent.ConcurrentHashMap import kotlin.coroutines.cancellation.CancellationException @@ -90,6 +92,9 @@ class EventSync( /** Maximum number of completed-relay entries kept in the activity log. */ const val MAX_ACTIVITY_LOG = 5000 + + /** Poll interval while waiting for the last forwarded events to be acknowledged. */ + const val OUTBOX_DRAIN_POLL_MS = 100L } // ------------------------------------------------------------------------- @@ -392,6 +397,13 @@ class EventSync( val sourceRelayOfEvent = ConcurrentHashMap() + // (event id, destination) pairs already counted as sent. The outbox is + // at-least-once: it writes an event as soon as the socket is ready and + // resends everything still unacknowledged when the connection finishes + // syncing, so one event can hit the same relay twice before its OK lands. + // The relay dedups the second copy; the counters must too. + val sentPairs = ConcurrentHashMap.newKeySet() + val runningState = SyncState.Running( relaysCompleted = 0, @@ -423,7 +435,12 @@ class EventSync( success: Boolean, ) { super.onSent(relay, cmdStr, cmd, success) - if (cmd is EventCmd) { + // `success` is "written to the socket", not "OK received". A write to a + // destination that is still connecting fails and the outbox resends it + // once the socket opens; counting the failed attempt too made every + // cold destination report one extra event sent. Likewise a successful + // resend of an unacknowledged event is the same send, not a second one. + if (cmd is EventCmd && success && sentPairs.add(cmd.event.id + relay.url.url)) { var hasSent = false if (outboxDedup.contains(cmd.event.id)) { @@ -586,6 +603,13 @@ class EventSync( }, ) + // `publish` is fire-and-forget through the client's outbox, and `use` closes + // the client as soon as this block returns. Without a drain, the events + // forwarded from the last page of the last relay are still waiting for a + // socket or an OK when the outbox is destroyed — the sync reports Done and + // silently never delivers them. Bounded by the same per-relay timeout. + awaitOutboxDrain(client, outboxDedup + inboxDedup + dmDedup) + _syncState.value = SyncState.Done( totalEventsReceived = runningState.eventsReceived.value, @@ -613,4 +637,22 @@ class EventSync( } } } + + /** + * Waits until no forwarded event in [ids] has a relay left in the client's outbox, or + * until [RELAY_TIMEOUT_MS] passes. Ids that drain are dropped from the working set so + * each poll only revisits what is still pending. + */ + private suspend fun awaitOutboxDrain( + client: INostrClient, + ids: Set, + ) { + val pending = ids.toMutableSet() + withTimeoutOrNull(RELAY_TIMEOUT_MS) { + while (pending.isNotEmpty()) { + pending.removeAll { client.pendingPublishRelaysFor(it).isNullOrEmpty() } + if (pending.isNotEmpty()) delay(OUTBOX_DRAIN_POLL_MS) + } + } + } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt index 65931c1128..f729de7e9e 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relays/eventsync/EventSyncTest.kt @@ -94,14 +94,18 @@ class EventSyncTest : RelayClientTest() { private fun corpus(): List = mine + mentions + dmToMe + noise - private fun eventSync(builder: WebsocketBuilder): EventSync = + /** [decorate] runs on every client the sync builds, e.g. to attach an authenticator. */ + private fun eventSync( + builder: WebsocketBuilder, + decorate: (NostrClient) -> Unit = {}, + ): EventSync = EventSync( accountPubKey = account.pubKey, relayDb = { listOf(source) }, outboxTargets = { setOf(outbox) }, inboxTargets = { setOf(inbox) }, dmTargets = { setOf(dm) }, - clientBuilder = { NostrClient(builder, scope) }, + clientBuilder = { NostrClient(builder, scope).also(decorate) }, scope = scope, ) @@ -164,6 +168,14 @@ class EventSyncTest : RelayClientTest() { mine.size + mentions.size + 1, done.totalEventsReceived, ) + // runSync drains the outbox before closing its client, so by the time + // Done is published every forwarded event has been written to its + // destination socket — not merely queued. + assertEquals( + "every routed event was sent before the client closed", + mine.size + mentions.size + 1, + done.totalEventsSent, + ) assertRouted(hub) } @@ -185,22 +197,12 @@ class EventSyncTest : RelayClientTest() { val authSigner = NostrSignerSync(KeyPair()) var authenticator: RelayAuthenticator? = null val sync = - EventSync( - accountPubKey = account.pubKey, - relayDb = { listOf(source) }, - outboxTargets = { setOf(outbox) }, - inboxTargets = { setOf(inbox) }, - dmTargets = { setOf(dm) }, - clientBuilder = { - val client = NostrClient(router, scope) - authenticator = - RelayAuthenticator(client = client, scope = scope) { _, template, _ -> - listOf(authSigner.sign(template)) - } - client - }, - scope = scope, - ) + eventSync(router) { client -> + authenticator = + RelayAuthenticator(client = client, scope = scope) { _, template, _ -> + listOf(authSigner.sign(template)) + } + } try { withTimeout(30_000) { sync.runSync() } diff --git a/cli/tests/cache/cache-headless.sh b/cli/tests/cache/cache-headless.sh index 8d2d3df51d..454c457a42 100755 --- a/cli/tests/cache/cache-headless.sh +++ b/cli/tests/cache/cache-headless.sh @@ -40,9 +40,11 @@ RESULTS_FILE="$STATE_DIR/results-$RUN_TS.tsv" AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" # Loopback relay = `amy serve` (geode), booted from $AMY_BIN by -# start_local_relay in headless/helpers.sh. 127.0.0.2 rather than -# 127.0.0.1 so Quartz's isLocalHost() filter doesn't strip it out of the -# published relay lists (see the DM harness for the full note). +# start_local_relay in headless/helpers.sh. 127.0.0.2 only for parity +# with the DM and Marmot harnesses: Quartz's isLocalHost() now covers all +# of 127.0.0.0/8, so it is stripped from parsed relay lists exactly like +# 127.0.0.1. Nothing here depends on that parse — amy publishes to and +# reads from the relay it was told about. RELAY_HOST="${RELAY_HOST:-127.0.0.2}" RELAY_DATA="$STATE_DIR/relay" RELAY_PORT="${RELAY_PORT:-8092}" diff --git a/cli/tests/dm/dm-interop-headless.sh b/cli/tests/dm/dm-interop-headless.sh index fa7fbaf84f..5d3ed45ab6 100755 --- a/cli/tests/dm/dm-interop-headless.sh +++ b/cli/tests/dm/dm-interop-headless.sh @@ -30,10 +30,12 @@ AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" # Loopback relay = `amy serve` (geode), booted from $AMY_BIN by # start_local_relay in headless/helpers.sh. Override RELAY_DATA if you # want full isolation between runs. -# Bind the loopback relay to 127.0.0.2 rather than 127.0.0.1 so Quartz's -# `isLocalHost()` filter doesn't silently strip it out of the kind:10050 -# inbox events during recipient-relay resolution. 127.0.0.2 is still pure -# loopback — no network traffic, no config needed. +# 127.0.0.2 used to dodge Quartz's `isLocalHost()` strip of loopback +# relays in kind:10050 inbox lists. That filter now covers all of +# 127.0.0.0/8, so the strict-inbox sends (dm-01/02/05/06) fail with +# no_dm_relays regardless of which loopback address the relay binds; +# only the fallback-chain tests (dm-03/04) are unaffected. Kept for +# parity with the other harnesses until that routing rule is revisited. RELAY_HOST="${RELAY_HOST:-127.0.0.2}" RELAY_DATA="$STATE_DIR/relay" RELAY_PORT="${RELAY_PORT:-8090}" diff --git a/cli/tests/headless/helpers.sh b/cli/tests/headless/helpers.sh index c2c504854e..ea45199223 100644 --- a/cli/tests/headless/helpers.sh +++ b/cli/tests/headless/helpers.sh @@ -76,8 +76,11 @@ assert_eq() { # # Callers set (before sourcing or at least before calling): # AMY_BIN amy launcher (built via `./gradlew :cli:installDist`) -# RELAY_HOST host clients connect to (most harnesses use 127.0.0.2 — -# see the isLocalHost() note at the top of each script) +# RELAY_HOST host clients connect to. The harnesses use 127.0.0.2 for +# parity with each other; note that Quartz's isLocalHost() +# treats all of 127.0.0.0/8 as loopback, so it does NOT +# survive the NIP-17 / NIP-65 relay-list parsers any better +# than 127.0.0.1 does. # RELAY_BIND optional bind address; defaults to $RELAY_HOST. Set to # 0.0.0.0 when a device on the LAN must reach the relay. # RELAY_PORT listen port diff --git a/cli/tests/marmot/marmot-interop.sh b/cli/tests/marmot/marmot-interop.sh index dc5bf401eb..d39481c7c3 100755 --- a/cli/tests/marmot/marmot-interop.sh +++ b/cli/tests/marmot/marmot-interop.sh @@ -37,10 +37,17 @@ WND_BIN="" AMY_BIN="$REPO_ROOT/cli/build/install/amy/bin/amy" # Embedded relay (default mode). Bound on every interface so a device on the -# same network can reach it; the daemons connect over loopback. Loopback +# same network can reach it; the daemons connect over $RELAY_HOST. Loopback # `ws://` relays are only accepted by MDK behind this explicit opt-in. -RELAY_HOST="127.0.0.1" -RELAY_BIND="0.0.0.0" +# +# Known limit, inherited from the old --local-relays mode: the URL wn +# advertises in its kind:10050/10051 lists is $RELAY_URL, and Amethyst's +# parsers drop loopback and RFC1918 relays from those lists, so A→B welcome +# delivery leans on Amethyst's fallback relays. Override RELAY_HOST with an +# address the device can dial (e.g. the laptop's LAN IP) to have wn +# advertise that instead; the daemons then connect to it too. +RELAY_HOST="${RELAY_HOST:-127.0.0.1}" +RELAY_BIND="${RELAY_BIND:-0.0.0.0}" RELAY_PORT="${RELAY_PORT:-8080}" RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT" RELAY_DATA="$STATE_DIR/relay" @@ -85,7 +92,9 @@ while [[ $# -gt 0 ]]; do case "$1" in --public-relays) USE_PUBLIC_RELAYS=1 ;; --local-relays) printf '%s\n' "note: --local-relays is now the default (embedded amy serve relay); flag ignored" >&2 ;; - --port) RELAY_PORT="$2"; RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT"; shift ;; + --port) + [[ $# -ge 2 && "$2" != --* ]] || { printf 'missing value for --port\n' >&2; usage; exit 2; } + RELAY_PORT="$2"; RELAY_URL="ws://$RELAY_HOST:$RELAY_PORT"; shift ;; --transponder) ENABLE_TRANSPONDER=1 ;; --no-build) NO_BUILD=1 ;; -h|--help) usage; exit 0 ;; @@ -567,14 +576,14 @@ configure_relays() { info "sanity kinds 10050/1059/445 ok (B->C welcome + message round-trip)" else warn "kind:445 failed — C never decrypted sanity-ping (relays may be dropping group messages)" - warn "Consider rerunning without --public-relays (the embedded relay accepts every kind)." + warn "Consider rerunning without --public-relays (the embedded relay stores every kind)." fi # best-effort cleanup so re-runs don't accumulate dead sanity groups wn_c groups leave "$sanity_c_gid" >/dev/null 2>&1 || true wn_b groups leave "$sanity_gid" >/dev/null 2>&1 || true else warn "kind:10050/1059 failed — C never received welcome; relays likely dropping gift wraps or inbox lists" - warn "Consider rerunning without --public-relays (the embedded relay accepts every kind)." + warn "Consider rerunning without --public-relays (the embedded relay stores every kind)." fi fi } From 285a51e98fe11f1fa8a6c7f21a8fbed2d1375f24 Mon Sep 17 00:00:00 2001 From: mstrofnone Date: Sat, 12 Sep 2026 14:38:38 +1000 Subject: [PATCH 13/16] fix(desktop): stop silent account wipe on keychain errors and upgrade races Three independent bugs in DesktopAccountStorage / SecureKeyStorage could turn one ambiguous macOS Keychain reply, one transient read error, or one Homebrew upgrade race into permanent account-metadata loss on ~/.amethyst/accounts.json.enc. 1. Silent AES metadata-key rotation on ambiguous keychain miss. getOrCreateKey() treated null from getPrivateKey("account-metadata- key") as "no key exists" and generated a fresh AES key. On macOS javakeyring collapses errSecItemNotFound (-25300), errSecAuthFailed (-25293), errSecUserCanceled (-128), and errSecInteractionNotAllowed (-25308) into the same PasswordAccessException; getFromKeyring turns them all into null. A single Deny click on the OS Keychain dialog silently rotated the AES key and destroyed the ability to decrypt the existing accounts.json.enc. Fix: add a new strict SecureKeyStorage.getPrivateKeyOrThrow(npub) on the common expect. On JVM/macOS it wraps /usr/bin/security find-generic-password whose exit codes (0 = found, 44 = not found, others = ambiguous) are documented and unambiguous. On JVM Windows/Linux it uses javakeyring but throws on any PasswordAccessException from the strict path. On Android it uses EncryptedSharedPreferences.contains(). On iOS it mirrors the existing "pending (iOS Phase 4)" stub. getOrCreateKey now calls getPrivateKeyOrThrow and propagates SecureStorageException without ever rotating the key. The permissive getPrivateKey(npub) is unchanged; its callers (per-account nsec, ephemeral bunker keys) still tolerate null on any error. 2. Any read failure resets the file. readMetadataFromDisk() used to rename to accounts.json.enc.corrupt. and return empty AccountMetadata() on any exception, including transient IO and the newly-throwing keychain path from bug 1. Fix: distinguish exception types. - AEADBadTagException / BadPaddingException: back up to .corrupt., reset, fire StorageCorruption.FileCorrupted (unchanged). - JacksonException: back up but to .jsonerror. so it is distinguishable from ciphertext corruption; fire StorageCorruption.JsonMalformed. - Anything else (IO error, OOM, thrown keychain path): do NOT rename; rethrow to caller and fire a new StorageCorruption.TransientError(cause) subtype. The file stays untouched. AccountManager.loadSavedAccount already wraps in try/catch and turns the throw into Result.failure. - Truncated file (size < GCM IV size): still backup + reset, genuinely unusable. 3. No cross-process advisory lock. Homebrew replacing the .app while the old process is mid-save, or an accidental double-launch of Compose Desktop (no built-in single-instance guard), could produce a truncated file that trips bug 2. Fix: withAccountsFileLock helper (mirrors SecureKeyStorage. withFileLock) wraps read + write in a RandomAccessFile(lockFile, "rw").channel.lock() on ~/.amethyst/accounts.json.enc.lock (0600). Because FileChannel.lock is per-JVM, an in-process Mutex is held before acquiring the channel lock. A separate stateMutex guards the read-modify-write cycle in saveAccount / deleteAccount / setCurrentAccount so two concurrent writers cannot each read the same base metadata and each rewrite it. Backward compatibility: existing keychain items are read unchanged; no schema migration for accounts.json.enc; the file lock adds a .lock sidecar older builds ignore. Tests: new SecureKeyStorageOrThrowTest (pure exit-code parser, mac lookup Found/NotFound/Ambiguous, non-mac keyring hit and throw-on-PasswordAccessException). DesktopAccountStorageTest gains five cases: getOrCreateKey ambiguous-error preserves file and does not rotate; getOrCreateKey definitive-not-found happy path; readMetadataFromDisk transient-IO preserves file with no backup sibling; GCM tag mismatch keeps .corrupt. backup; JSON malformed uses new .jsonerror. suffix; eight concurrent saveAccount calls serialize under the file lock with no lost updates. All existing AccountManager* MockK setups extended to also stub getPrivateKeyOrThrow. Local verify: :desktopApp:test + :commons:jvmTest, 2490 tests, all pass. Spotless clean. --- .../commons/keystorage/SecureKeyStorage.kt | 20 ++ .../commons/keystorage/SecureKeyStorage.kt | 22 ++ .../keystorage/SecureKeyStorage.ios.kt | 2 + .../commons/keystorage/SecureKeyStorage.kt | 161 +++++++++++++ .../keystorage/SecureKeyStorageOrThrowTest.kt | 212 +++++++++++++++++ .../desktop/account/DesktopAccountStorage.kt | 176 ++++++++++---- .../account/AccountManagerKeyLoginTest.kt | 1 + .../account/AccountManagerLoadAccountTest.kt | 1 + .../AccountManagerLoadStateTransitionsTest.kt | 1 + .../account/AccountManagerLogoutTest.kt | 1 + .../AccountManagerNip46IsolationTest.kt | 1 + .../AccountManagerStateTransitionTest.kt | 1 + .../account/DesktopAccountStorageTest.kt | 217 ++++++++++++++++++ .../desktop/benchmark/LaunchScenario.kt | 1 + .../desktop/ui/AppStateMachineTest.kt | 1 + 15 files changed, 777 insertions(+), 41 deletions(-) create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt diff --git a/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index 74b279d621..dbb4efb401 100644 --- a/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -107,6 +107,26 @@ actual class SecureKeyStorage private actual constructor() { } } + /** + * Android backend: EncryptedSharedPreferences.contains + getString has no + * ambiguous-error state comparable to macOS Keychain user-cancel/deny, so + * "key not present" and "key present" are the only two null outcomes. + * Any thrown exception is a genuine failure and propagates. + */ + actual suspend fun getPrivateKeyOrThrow(npub: String): String? = + withContext(Dispatchers.IO) { + try { + val key = KEY_PREFIX + npub + if (!encryptedPrefs.contains(key)) { + null + } else { + encryptedPrefs.getString(key, null) + } + } catch (e: Exception) { + throw SecureStorageException("Failed to retrieve private key", e) + } + } + actual suspend fun deletePrivateKey(npub: String): Boolean = withContext(Dispatchers.IO) { try { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index efb9d7342e..2ee7e124cf 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -76,12 +76,34 @@ expect class SecureKeyStorage private constructor() { * **Security Warning:** The returned String cannot be securely zeroed from memory (JVM limitation). * Dereference the returned value immediately after use to minimize exposure time. * + * Callers that MUST distinguish "key does not exist" from "backend refused/locked/failed" + * (for example, before generating a replacement key on disk) should use + * [getPrivateKeyOrThrow] instead. This method returns null on any error and cannot + * safely be used as an "is this the first launch?" probe. + * * @param npub The public key in npub (Bech32) format * @return The private key in hexadecimal format, or null if not found * @throws SecureStorageException if retrieval operation fails */ suspend fun getPrivateKey(npub: String): String? + /** + * Retrieves a private key for the given npub, distinguishing "definitively absent" + * from any other failure mode. + * + * On success, returns the key. When the backend confirms the item does not exist, + * returns null. Any other outcome (backend unavailable, user denied the OS prompt, + * keychain locked, I/O error) throws [SecureStorageException]. This is the safe + * primitive for compare-and-swap style flows where a null must not be interpreted + * as permission to generate and persist a replacement. + * + * @param npub The public key in npub (Bech32) format + * @return The private key in hexadecimal format, or null only when the backend + * confirms the item does not exist + * @throws SecureStorageException on any ambiguous or transient failure + */ + suspend fun getPrivateKeyOrThrow(npub: String): String? + /** * Deletes a private key for the given npub. * diff --git a/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt b/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt index d67627da12..d48ac93cfd 100644 --- a/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt +++ b/commons/src/iosMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.ios.kt @@ -34,6 +34,8 @@ actual class SecureKeyStorage private actual constructor() { actual suspend fun getPrivateKey(npub: String): String? = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") + actual suspend fun getPrivateKeyOrThrow(npub: String): String? = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") + actual suspend fun deletePrivateKey(npub: String): Boolean = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") actual suspend fun hasPrivateKey(npub: String): Boolean = throw SecureStorageException("Keychain Services binding pending (iOS Phase 4)") diff --git a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index 85c8b9c95e..31a27b98fb 100644 --- a/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/jvmMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -149,6 +149,75 @@ actual class SecureKeyStorage private actual constructor() { } } + /** + * Strict variant that distinguishes "backend confirms item not found" from every + * other outcome. This matters on macOS: `javakeyring` collapses `errSecItemNotFound` + * (-25300), `errSecAuthFailed` (-25293), `errSecUserCanceled` (-128), and + * `errSecInteractionNotAllowed` (-25308) into the same `PasswordAccessException`. + * A caller that mistook "user clicked Deny" for "first launch, generate a fresh + * key" would silently rotate the metadata AES key and permanently destroy the + * accounts.json.enc it was supposed to unlock. + * + * On macOS this shells out to `/usr/bin/security find-generic-password`, whose + * exit codes are documented and unambiguous (44 = not found, 128 = user cancel / + * dialog dismissed, others = backend failure). On Windows / Linux, javakeyring + * has no such ambiguity for the equivalent flows in practice, but we still treat + * any `PasswordAccessException` here as ambiguous (throw) to keep the contract + * strict on the getOrCreate path. + */ + actual suspend fun getPrivateKeyOrThrow(npub: String): String? = + withContext(Dispatchers.IO) { + try { + if (!keyringAvailable) { + return@withContext getFromFallback(npub) + } + if (isMacOs()) { + return@withContext getFromMacSecurityCli(SERVICE_NAME, npub) + } + try { + keyring().getPassword(SERVICE_NAME, npub) + } catch (e: PasswordAccessException) { + // Non-mac backends: keep the strict contract by refusing to + // treat this as "definitively absent". A caller that needs a + // permissive lookup should use getPrivateKey() instead. + throw SecureStorageException( + "Keyring backend refused access or returned ambiguous not-found", + e, + ) + } + } catch (e: SecureStorageException) { + throw e + } catch (e: BackendNotSupportedException) { + keyringAvailable = false + println("OS keyring not available, using fallback encrypted storage") + getFromFallback(npub) + } catch (e: Exception) { + throw SecureStorageException("Failed to retrieve private key (strict)", e) + } + } + + /** + * Test seam: overridable strategy for the strict macOS lookup. Production wires + * to [defaultMacSecurityLookup] which spawns `/usr/bin/security`. Tests replace + * this with a stub so unit tests run hermetically on any OS. + */ + internal var macSecurityLookup: (String, String) -> MacSecurityResult = + ::defaultMacSecurityLookup + + private fun getFromMacSecurityCli( + service: String, + account: String, + ): String? { + val result = macSecurityLookup(service, account) + return when (result) { + is MacSecurityResult.Found -> result.password + is MacSecurityResult.NotFound -> null + is MacSecurityResult.Ambiguous -> throw SecureStorageException( + "macOS Keychain access failed (${result.reason}, exit=${result.exitCode})", + ) + } + } + actual suspend fun deletePrivateKey(npub: String): Boolean = withContext(Dispatchers.IO) { try { @@ -466,6 +535,98 @@ internal interface KeyringHandle { ) } +/** + * Outcome of a strict macOS `/usr/bin/security find-generic-password` lookup. + * Kept as a sealed hierarchy so [SecureKeyStorage.getPrivateKeyOrThrow] can + * cleanly translate to `null` versus `SecureStorageException`. + */ +internal sealed class MacSecurityResult { + data class Found( + val password: String, + ) : MacSecurityResult() + + object NotFound : MacSecurityResult() + + /** + * Any exit code other than 0 (found) or 44 (item not found). Reason is a short + * human string derived from stderr / documented codes: + * 128 = user cancelled or dismissed the Keychain Access dialog + * -25293 (errSecAuthFailed) surfaces as exit 51 in practice + * -25308 (errSecInteractionNotAllowed) surfaces when Keychain is locked + */ + data class Ambiguous( + val exitCode: Int, + val reason: String, + ) : MacSecurityResult() +} + +/** + * Pure parser split out for testability on non-macOS CI runners. Maps the + * documented exit code contract of `/usr/bin/security find-generic-password` + * to a [MacSecurityResult]. `stdout` is the raw password body (`-w` prints it + * followed by a newline; strip the trailing newline only). `stderr` is used + * as a hint for the ambiguous [MacSecurityResult.Ambiguous.reason] string. + */ +internal fun parseMacSecurityFindResult( + exitCode: Int, + stdout: String, + stderr: String, +): MacSecurityResult = + when (exitCode) { + 0 -> MacSecurityResult.Found(stdout.trimEnd('\n', '\r')) + 44 -> MacSecurityResult.NotFound + else -> { + val reason = + when { + exitCode == 128 -> "user cancelled Keychain dialog" + stderr.contains("-25293") -> "errSecAuthFailed" + stderr.contains("-25308") -> "errSecInteractionNotAllowed" + stderr.contains("-128") -> "user cancelled Keychain dialog" + stderr.isNotBlank() -> + stderr + .lineSequence() + .first() + .trim() + .take(120) + else -> "unknown" + } + MacSecurityResult.Ambiguous(exitCode, reason) + } + } + +private fun isMacOs(): Boolean = System.getProperty("os.name").orEmpty().startsWith("Mac") + +/** + * Production implementation: spawn `/usr/bin/security` and read exit code + streams. + * Kept package-private so tests can also reach it if they want to run the real path + * on a mac host, but production always goes through the [SecureKeyStorage.macSecurityLookup] + * indirection. + */ +internal fun defaultMacSecurityLookup( + service: String, + account: String, +): MacSecurityResult { + val process = + try { + ProcessBuilder( + "/usr/bin/security", + "find-generic-password", + "-s", + service, + "-a", + account, + "-w", + ).redirectErrorStream(false).start() + } catch (e: Exception) { + return MacSecurityResult.Ambiguous(-1, "failed to spawn /usr/bin/security: ${e.message ?: e::class.simpleName ?: "unknown"}") + } + process.outputStream.close() + val stdout = process.inputStream.bufferedReader().use { it.readText() } + val stderr = process.errorStream.bufferedReader().use { it.readText() } + val exitCode = process.waitFor() + return parseMacSecurityFindResult(exitCode, stdout, stderr) +} + internal class RealKeyringHandle( private val keyring: Keyring, ) : KeyringHandle { diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt new file mode 100644 index 0000000000..980d6c718f --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorageOrThrowTest.kt @@ -0,0 +1,212 @@ +/* + * 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.commons.keystorage + +import com.github.javakeyring.PasswordAccessException +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test + +/** + * Unit tests for the strict `getPrivateKeyOrThrow` lookup and its pure macOS + * `/usr/bin/security` exit-code parser. + * + * These tests must never touch the OS keychain and must run on any host, so: + * - the macOS integration paths are gated behind [MacSecurityResult] stubs + * injected via `SecureKeyStorage.macSecurityLookup`; + * - the parser test operates on captured stdout/stderr/exitCode triples; + * - non-mac backends are exercised via the `KeyringHandle` test seam already + * used by [SecureKeyStorageKeyringCacheTest]. + */ +class SecureKeyStorageOrThrowTest { + private class ExplodingKeyring( + private val onGet: () -> Nothing, + ) : KeyringHandle { + override fun getPassword( + service: String, + account: String, + ): String = onGet() + + override fun setPassword( + service: String, + account: String, + password: String, + ) { + // unused in these tests + } + + override fun deletePassword( + service: String, + account: String, + ) { + // unused in these tests + } + } + + private class StaticKeyring( + private val map: Map, String>, + ) : KeyringHandle { + override fun getPassword( + service: String, + account: String, + ): String = map[service to account] ?: throw PasswordAccessException("no entry") + + override fun setPassword( + service: String, + account: String, + password: String, + ) {} + + override fun deletePassword( + service: String, + account: String, + ) {} + } + + // --- macOS security(1) parser --- + + @Test + fun `parser exit 0 returns Found with trimmed password`() { + val result = parseMacSecurityFindResult(0, "hunter2\n", "") + assertTrue(result is MacSecurityResult.Found) + assertEquals("hunter2", (result as MacSecurityResult.Found).password) + } + + @Test + fun `parser exit 0 preserves internal newlines and only strips trailing`() { + val result = parseMacSecurityFindResult(0, "line1\nline2\n", "") + assertEquals("line1\nline2", (result as MacSecurityResult.Found).password) + } + + @Test + fun `parser exit 44 returns NotFound`() { + val result = + parseMacSecurityFindResult( + 44, + "", + "security: SecKeychainSearchCopyNext: The specified item could not be found in the keychain.\n", + ) + assertTrue(result is MacSecurityResult.NotFound) + } + + @Test + fun `parser exit 128 flagged as user-cancelled`() { + val result = parseMacSecurityFindResult(128, "", "security: dismissed\n") + assertTrue(result is MacSecurityResult.Ambiguous) + val ambig = result as MacSecurityResult.Ambiguous + assertEquals(128, ambig.exitCode) + assertTrue(ambig.reason.contains("cancel")) + } + + @Test + fun `parser stderr -25293 mapped to errSecAuthFailed`() { + val result = parseMacSecurityFindResult(51, "", "security: SecKeychainItemCopyContent (-25293)\n") + val ambig = result as MacSecurityResult.Ambiguous + assertEquals("errSecAuthFailed", ambig.reason) + } + + @Test + fun `parser unknown exit falls back to first stderr line`() { + val result = parseMacSecurityFindResult(9999, "", "security: mystery: line 1\nline 2\n") + val ambig = result as MacSecurityResult.Ambiguous + assertEquals("security: mystery: line 1", ambig.reason) + } + + // --- getPrivateKeyOrThrow: strict semantics via injected macOS lookup --- + // (Enabled unconditionally: the macSecurityLookup indirection is exercised + // via a stub, so no `security` binary is invoked. The `isMacOs()` check + // means this test only takes the mac path on macOS runners; on Linux it + // takes the javakeyring path, which we validate separately below.) + + private fun newStorage(): SecureKeyStorage = SecureKeyStorage.create() + + @Test + fun `mac lookup Found returns password without ambiguity`() = + runBlocking { + if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.macSecurityLookup = { _, _ -> MacSecurityResult.Found("secretval") } + assertEquals("secretval", storage.getPrivateKeyOrThrow("account-metadata-key")) + } + + @Test + fun `mac lookup NotFound returns null`() = + runBlocking { + if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.macSecurityLookup = { _, _ -> MacSecurityResult.NotFound } + assertNull(storage.getPrivateKeyOrThrow("account-metadata-key")) + } + + @Test + fun `mac lookup Ambiguous throws SecureStorageException with reason`() = + runBlocking { + if (!System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.macSecurityLookup = { _, _ -> + MacSecurityResult.Ambiguous(128, "user cancelled Keychain dialog") + } + try { + storage.getPrivateKeyOrThrow("account-metadata-key") + fail("Expected SecureStorageException") + } catch (e: SecureStorageException) { + assertTrue(e.message?.contains("user cancelled") == true) + assertTrue(e.message?.contains("128") == true) + } + } + + // --- non-mac backend: PasswordAccessException must throw, never null --- + + @Test + fun `non-mac keyring PasswordAccessException throws not returns null`() = + runBlocking { + if (System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.keyringFactory = { ExplodingKeyring { throw PasswordAccessException("locked") } } + try { + storage.getPrivateKeyOrThrow("account-metadata-key") + fail("Expected SecureStorageException") + } catch (e: SecureStorageException) { + assertTrue( + e.message?.contains("ambiguous", ignoreCase = true) == true || + e.message?.contains("refused", ignoreCase = true) == true, + ) + } + } + + @Test + fun `non-mac keyring hit returns password`() = + runBlocking { + if (System.getProperty("os.name").orEmpty().startsWith("Mac")) return@runBlocking + val storage = newStorage() + storage.keyringFactory = { + StaticKeyring( + mapOf( + ("amethyst-desktop" to "account-metadata-key") to "abc123", + ), + ) + } + assertEquals("abc123", storage.getPrivateKeyOrThrow("account-metadata-key")) + } +} diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt index 3f6ff53ec2..9ef274a2ad 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt @@ -28,7 +28,10 @@ import com.vitorpamplona.amethyst.commons.model.account.AccountStorage import com.vitorpamplona.amethyst.commons.model.account.SignerType import com.vitorpamplona.amethyst.commons.util.deleteOrWarn import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import java.io.File +import java.io.RandomAccessFile import java.nio.file.Files import java.nio.file.StandardCopyOption import java.nio.file.attribute.PosixFilePermission @@ -58,6 +61,16 @@ sealed class StorageCorruption( class JsonMalformed( backupPath: String?, ) : StorageCorruption(backupPath) + + /** + * A transient failure surfaced from the read path (I/O error, keychain refused + * or otherwise ambiguous access, OOM, etc). No backup was written and the + * on-disk file is untouched. Callers should retry or surface an error UI rather + * than treating this as data loss. See [DesktopAccountStorage.readMetadataFromDisk]. + */ + class TransientError( + val cause: Throwable, + ) : StorageCorruption(backupPath = null) } class DesktopAccountStorage( @@ -68,6 +81,7 @@ class DesktopAccountStorage( companion object { private const val METADATA_KEY_ALIAS = "account-metadata-key" private const val ACCOUNTS_FILE = "accounts.json.enc" + private const val ACCOUNTS_LOCK_FILE = "accounts.json.enc.lock" private const val AES_KEY_SIZE = 32 // 256 bits private const val GCM_IV_SIZE = 12 private const val GCM_TAG_BITS = 128 @@ -76,38 +90,51 @@ class DesktopAccountStorage( private val mapper = jacksonObjectMapper() private val amethystDir by lazy { File(homeDir, ".amethyst") } - // In-memory cache — read from disk once, then serve from memory + // In-memory cache: read from disk once, then serve from memory private var cachedMetadata: AccountMetadata? = null + // In-process mutex around the cross-process file lock. Two callers inside + // the same JVM would otherwise fail with OverlappingFileLockException from + // FileChannel.lock(), since JVM file locks are per-JVM not per-thread. + private val fileLockMutex = Mutex() + + // Guards read-modify-write cycles on [cachedMetadata]. Distinct from + // [fileLockMutex] so we can hold it across a full read + mutate + write + // sequence (the file lock is taken and released inside each disk op). + private val stateMutex = Mutex() + // --- AccountStorage interface --- override suspend fun loadAccounts(): List = getCachedMetadata().accounts.map { it.toAccountInfo() } - override suspend fun saveAccount(info: AccountInfo) { - val metadata = getCachedMetadata() - val dto = AccountInfoDto.from(info) - val updated = metadata.accounts.filter { it.npub != info.npub } + dto - writeCachedMetadata(metadata.copy(accounts = updated)) - } + override suspend fun saveAccount(info: AccountInfo) = + stateMutex.withLock { + val metadata = getCachedMetadata() + val dto = AccountInfoDto.from(info) + val updated = metadata.accounts.filter { it.npub != info.npub } + dto + writeCachedMetadata(metadata.copy(accounts = updated)) + } - override suspend fun deleteAccount(npub: String) { - val metadata = getCachedMetadata() - val updated = metadata.accounts.filter { it.npub != npub } - val newActive = - if (metadata.activeNpub == npub) { - updated.firstOrNull()?.npub - } else { - metadata.activeNpub - } - writeCachedMetadata(metadata.copy(accounts = updated, activeNpub = newActive)) - } + override suspend fun deleteAccount(npub: String) = + stateMutex.withLock { + val metadata = getCachedMetadata() + val updated = metadata.accounts.filter { it.npub != npub } + val newActive = + if (metadata.activeNpub == npub) { + updated.firstOrNull()?.npub + } else { + metadata.activeNpub + } + writeCachedMetadata(metadata.copy(accounts = updated, activeNpub = newActive)) + } override suspend fun currentAccount(): String? = getCachedMetadata().activeNpub - override suspend fun setCurrentAccount(npub: String) { - val metadata = getCachedMetadata() - writeCachedMetadata(metadata.copy(activeNpub = npub)) - } + override suspend fun setCurrentAccount(npub: String) = + stateMutex.withLock { + val metadata = getCachedMetadata() + writeCachedMetadata(metadata.copy(activeNpub = npub)) + } // --- Cached I/O --- @@ -129,9 +156,17 @@ class DesktopAccountStorage( val file = getAccountsFile() if (!file.exists()) return AccountMetadata() + ensureDir() + return withAccountsFileLock { + readMetadataFromDiskLocked(file) + } + } + + private suspend fun readMetadataFromDiskLocked(file: File): AccountMetadata { val encrypted = file.readBytes() if (encrypted.size < GCM_IV_SIZE) { - val backup = backupCorruptFile(file) + // Genuinely unusable: not enough bytes for the IV. Back up and reset. + val backup = backupCorruptFile(file, ".corrupt") onCorruption(StorageCorruption.FileCorrupted(backup)) return AccountMetadata() } @@ -140,31 +175,43 @@ class DesktopAccountStorage( val decrypted = decrypt(encrypted) mapper.readValue(decrypted) } catch (e: javax.crypto.AEADBadTagException) { - Log.e("DesktopAccountStorage", "GCM auth tag mismatch — file corrupted or key lost", e) - val backup = backupCorruptFile(file) + // Genuine ciphertext corruption or lost/rotated AES key. + Log.e("DesktopAccountStorage", "GCM auth tag mismatch, file corrupted or key lost", e) + val backup = backupCorruptFile(file, ".corrupt") onCorruption(StorageCorruption.FileCorrupted(backup)) AccountMetadata() } catch (e: javax.crypto.BadPaddingException) { - Log.e("DesktopAccountStorage", "Decryption failed — file corrupted", e) - val backup = backupCorruptFile(file) + // Genuine ciphertext corruption. + Log.e("DesktopAccountStorage", "Decryption failed, file corrupted", e) + val backup = backupCorruptFile(file, ".corrupt") onCorruption(StorageCorruption.FileCorrupted(backup)) AccountMetadata() } catch (e: com.fasterxml.jackson.core.JacksonException) { + // Schema mismatch: decrypted cleanly but the JSON does not fit our shape. + // Distinct suffix so operators can tell it apart from ciphertext corruption. Log.e("DesktopAccountStorage", "JSON malformed after decryption", e) - val backup = backupCorruptFile(file) + val backup = backupCorruptFile(file, ".jsonerror") onCorruption(StorageCorruption.JsonMalformed(backup)) AccountMetadata() + } catch (e: kotlin.coroutines.cancellation.CancellationException) { + throw e } catch (e: Exception) { - Log.e("DesktopAccountStorage", "Failed to read accounts metadata", e) - val backup = backupCorruptFile(file) - onCorruption(StorageCorruption.FileCorrupted(backup)) - AccountMetadata() + // Transient failure: I/O error, keychain refused / ambiguous, OOM, etc. + // DO NOT rename the on-disk file; the ciphertext is intact and the next + // launch may succeed (for example after the user re-approves the + // Keychain Access prompt). Surface up for the caller to decide. + Log.e("DesktopAccountStorage", "Transient error reading accounts metadata; file preserved", e) + onCorruption(StorageCorruption.TransientError(e)) + throw e } } - private fun backupCorruptFile(file: File): String? = + private fun backupCorruptFile( + file: File, + suffix: String, + ): String? = try { - val backup = File(file.parent, "accounts.json.enc.corrupt.${System.currentTimeMillis()}") + val backup = File(file.parent, "${file.name}$suffix.${System.currentTimeMillis()}") java.nio.file.Files .copy(file.toPath(), backup.toPath()) file.deleteOrWarn("DesktopAccountStorage", "corrupt accounts file") @@ -178,31 +225,78 @@ class DesktopAccountStorage( val json = mapper.writeValueAsBytes(metadata) val encrypted = encrypt(json) - // Atomic write via temp file val file = getAccountsFile() - val temp = File(amethystDir, "${ACCOUNTS_FILE}.tmp") - temp.writeBytes(encrypted) - Files.move(temp.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING) - - setFilePermissions(file) + withAccountsFileLock { + // Atomic write via temp file, under the cross-process lock so two + // Amethyst instances (Homebrew upgrade race, accidental double-launch) + // cannot interleave writes and truncate the file. + val temp = File(amethystDir, "$ACCOUNTS_FILE.tmp") + temp.writeBytes(encrypted) + Files.move( + temp.toPath(), + file.toPath(), + StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE, + ) + setFilePermissions(file) + } } + /** + * Cross-process advisory lock + in-process mutex around the accounts.json.enc + * read/write critical section. The mutex is required because JVM + * `FileChannel.lock()` is a per-JVM lock and would throw + * `OverlappingFileLockException` on the second acquire from the same JVM. + * The channel lock is required to keep two Amethyst processes serial (upgrade + * race, accidental double-launch, cron-style relaunch). + * + * Mirrors the pattern used in SecureKeyStorage.withFileLock; kept private + * to this class so the two lock lifecycles stay independent. + */ + private suspend inline fun withAccountsFileLock(crossinline block: suspend () -> T): T = + fileLockMutex.withLock { + val lockFile = File(amethystDir, ACCOUNTS_LOCK_FILE) + if (!lockFile.exists()) { + lockFile.createNewFile() + setFilePermissions(lockFile) + } + RandomAccessFile(lockFile, "rw").use { raf -> + raf.channel.lock().use { _ -> + block() + } + } + } + private fun getAccountsFile() = File(amethystDir, ACCOUNTS_FILE) // --- AES-256-GCM encryption --- private var cachedKey: ByteArray? = null + /** + * Reads (or creates on first launch) the metadata AES key. + * + * Distinguishes: + * - key exists in keychain: use it + * - keychain confirms definitively absent: generate + persist a fresh key + * - any other outcome (user cancelled/denied prompt, keychain locked, + * backend transient error): propagate the exception, do NOT rotate. + * + * Rotating the AES key on an ambiguous miss silently destroys the ability + * to decrypt the existing accounts.json.enc, wiping the logged-in accounts + * on next launch. That is the bug this method exists to prevent. + */ private suspend fun getOrCreateKey(): ByteArray { cachedKey?.let { return it } - val existing = secureStorage.getPrivateKey(METADATA_KEY_ALIAS) + val existing = secureStorage.getPrivateKeyOrThrow(METADATA_KEY_ALIAS) if (existing != null) { val key = Base64.getDecoder().decode(existing) cachedKey = key return key } + // Definitively absent: safe to create and persist a fresh key. val key = ByteArray(AES_KEY_SIZE).also { SecureRandom().nextBytes(it) } secureStorage.savePrivateKey(METADATA_KEY_ALIAS, Base64.getEncoder().encodeToString(key)) cachedKey = key diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt index 4d511c57f7..6059f961fe 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt @@ -58,6 +58,7 @@ class AccountManagerKeyLoginTest { val keySlot = slot() val valueSlot = slot() coEvery { storage.getPrivateKey(capture(keySlot)) } answers { keyStore[keySlot.captured] } + coEvery { storage.getPrivateKeyOrThrow(capture(keySlot)) } answers { keyStore[keySlot.captured] } coEvery { storage.savePrivateKey(capture(keySlot), capture(valueSlot)) } answers { keyStore[keySlot.captured] = valueSlot.captured } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt index 8b01b71677..5aad247a29 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadAccountTest.kt @@ -49,6 +49,7 @@ class AccountManagerLoadAccountTest { storage = mockk(relaxed = true) // Return null so DesktopAccountStorage generates a fresh AES key coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-load-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt index f98a4c3407..602016ed5f 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLoadStateTransitionsTest.kt @@ -63,6 +63,7 @@ class AccountManagerLoadStateTransitionsTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-load-state").toFile() File(tempDir, ".amethyst").mkdirs() manager = AccountManager(storage, tempDir) diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt index 2f74a42390..741b79044d 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerLogoutTest.kt @@ -46,6 +46,7 @@ class AccountManagerLogoutTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-logout-test").toFile() manager = AccountManager(storage, tempDir) } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt index f2688a2819..6cb2a59e15 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerNip46IsolationTest.kt @@ -56,6 +56,7 @@ class AccountManagerNip46IsolationTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-nip46-iso-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt index 6734d49793..62e5eb08c0 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerStateTransitionTest.kt @@ -56,6 +56,7 @@ class AccountManagerStateTransitionTest { fun setup() { storage = mockk(relaxed = true) coEvery { storage.getPrivateKey("account-metadata-key") } returns null + coEvery { storage.getPrivateKeyOrThrow("account-metadata-key") } returns null tempDir = createTempDirectory("acctmgr-state-test").toFile() amethystDir = File(tempDir, ".amethyst") amethystDir.mkdirs() diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt index 9ffd9ac1b3..c1c4a88993 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt @@ -21,18 +21,28 @@ package com.vitorpamplona.amethyst.desktop.account import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage +import com.vitorpamplona.amethyst.commons.keystorage.SecureStorageException import com.vitorpamplona.amethyst.commons.model.account.AccountInfo import com.vitorpamplona.amethyst.commons.model.account.SignerType import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk import io.mockk.slot +import kotlinx.coroutines.asCoroutineDispatcher +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext import java.io.File +import java.util.concurrent.Executors import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFails +import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -56,6 +66,9 @@ class DesktopAccountStorageTest { coEvery { secureStorage.getPrivateKey(capture(keySlot)) } answers { keyStore[keySlot.captured] } + coEvery { secureStorage.getPrivateKeyOrThrow(capture(keySlot)) } answers { + keyStore[keySlot.captured] + } coEvery { secureStorage.savePrivateKey(capture(keySlot), capture(valueSlot)) } answers { keyStore[keySlot.captured] = valueSlot.captured } @@ -197,4 +210,208 @@ class DesktopAccountStorageTest { // Encrypted content should NOT contain the npub in plaintext assertTrue(!content.contains("npub1secret")) } + + // --- Bug 1: silent AES key rotation --- + + @Test + fun `getOrCreateKey keyring throws ambiguous error does not rotate key or touch file`() = + runTest { + // First launch: seed a real metadata key + an existing accounts.json.enc + storage.saveAccount(AccountInfo("npub1existing", SignerType.Internal)) + val file = File(File(tempDir, ".amethyst"), "accounts.json.enc") + val originalBytes = file.readBytes() + val originalMetadataKey = keyStore["account-metadata-key"] + assertNotNull(originalMetadataKey) + + // Fresh storage instance simulating a relaunch: the keychain now + // returns an ambiguous error (user cancelled the Keychain dialog). + val throwingStorage: SecureKeyStorage = mockk() + coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws + SecureStorageException("user cancelled Keychain dialog") + // Legacy permissive read should still return the key; production + // must not fall back to it on the getOrCreate path. + coEvery { throwingStorage.getPrivateKey(any()) } answers { keyStore[firstArg()] } + coEvery { throwingStorage.hasPrivateKey(any()) } answers { keyStore.containsKey(firstArg()) } + + val relaunched = DesktopAccountStorage(throwingStorage, tempDir) + + // Any operation that needs the metadata key must fail loudly, not + // rotate the key or write a fresh empty file. + assertFails { runBlocking { relaunched.loadAccounts() } } + + // Bug 1 invariant: no new savePrivateKey call for the metadata key. + coVerify(exactly = 0) { + throwingStorage.savePrivateKey("account-metadata-key", any()) + } + // Bug 1 + Bug 2 invariant: on-disk ciphertext untouched. + assertTrue(file.exists()) + assertContentEquals(originalBytes, file.readBytes()) + // Bug 1 invariant: keyStore metadata key unchanged. + assertEquals(originalMetadataKey, keyStore["account-metadata-key"]) + } + + @Test + fun `getOrCreateKey keyring returns definitive not-found creates and persists new key`() = + runTest { + // Happy path first launch: getPrivateKeyOrThrow returns null, + // storage generates + persists a fresh AES key exactly once. + assertNull(keyStore["account-metadata-key"]) + + storage.saveAccount(AccountInfo("npub1first", SignerType.Internal)) + + assertNotNull(keyStore["account-metadata-key"]) + coVerify(exactly = 1) { + secureStorage.savePrivateKey("account-metadata-key", any()) + } + } + + // --- Bug 2: read failure must not silently reset the file --- + + @Test + fun `readMetadataFromDisk transient IO error does not backup file`() = + runTest { + // Seed a real file we can inspect. + storage.saveAccount(AccountInfo("npub1existing", SignerType.Internal)) + val amethystDir = File(tempDir, ".amethyst") + val file = File(amethystDir, "accounts.json.enc") + val originalBytes = file.readBytes() + val originalName = file.name + + // Fresh storage that surfaces a transient error from the keychain. + val throwingStorage: SecureKeyStorage = mockk() + coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws + SecureStorageException("transient keychain error") + coEvery { throwingStorage.getPrivateKey(any()) } returns null + coEvery { throwingStorage.hasPrivateKey(any()) } returns false + + val corruptions = mutableListOf() + val relaunched = + DesktopAccountStorage(throwingStorage, tempDir, onCorruption = { corruptions += it }) + + assertFails { runBlocking { relaunched.loadAccounts() } } + + // File preserved, no .corrupt.* or .jsonerror.* sibling created. + assertTrue(file.exists()) + assertContentEquals(originalBytes, file.readBytes()) + val siblings = amethystDir.listFiles().orEmpty().map { it.name } + assertFalse(siblings.any { it != originalName && it.startsWith("accounts.json.enc") && (it.contains(".corrupt.") || it.contains(".jsonerror.")) }) + + // The callback fired with the transient subtype so the app can retry. + assertTrue(corruptions.any { it is StorageCorruption.TransientError }) + } + + @Test + fun `readMetadataFromDisk gcm tag mismatch backs up and resets`() = + runTest { + // Seed a valid file so we have a real metadata key in the mock keystore. + storage.saveAccount(AccountInfo("npub1a", SignerType.Internal)) + val amethystDir = File(tempDir, ".amethyst") + val file = File(amethystDir, "accounts.json.enc") + assertTrue(file.exists()) + + // Overwrite with random bytes that pass the length check but fail + // GCM auth tag verification. Prefix with a fresh IV, then garbage. + val garbage = ByteArray(64) { it.toByte() } + file.writeBytes(garbage) + + val corruptions = mutableListOf() + val relaunched = + DesktopAccountStorage(secureStorage, tempDir, onCorruption = { corruptions += it }) + + val loaded = relaunched.loadAccounts() + assertTrue(loaded.isEmpty()) + + // Backup exists with the .corrupt. suffix; original file was + // removed (and will be re-created on next save). + val siblings = amethystDir.listFiles().orEmpty().map { it.name } + assertTrue(siblings.any { it.startsWith("accounts.json.enc.corrupt.") }) + assertTrue(corruptions.any { it is StorageCorruption.FileCorrupted }) + } + + @Test + fun `readMetadataFromDisk json malformed uses jsonerror suffix`() = + runTest { + // Build an accounts.json.enc whose plaintext decrypts fine but is + // not the expected AccountMetadata shape. Easiest path: reuse the + // production encrypt via a lightweight helper storage that lets us + // control the plaintext. + storage.saveAccount(AccountInfo("npub1a", SignerType.Internal)) + val amethystDir = File(tempDir, ".amethyst") + val file = File(amethystDir, "accounts.json.enc") + + // Encrypt an unrelated JSON payload with the same AES key the mock + // keystore holds so decryption succeeds but Jackson rejects the shape. + val key = + java.util.Base64 + .getDecoder() + .decode(keyStore["account-metadata-key"]!!) + val iv = ByteArray(12) { 7 } + val cipher = javax.crypto.Cipher.getInstance("AES/GCM/NoPadding") + cipher.init( + javax.crypto.Cipher.ENCRYPT_MODE, + javax.crypto.spec.SecretKeySpec(key, "AES"), + javax.crypto.spec.GCMParameterSpec(128, iv), + ) + val badPayload = cipher.doFinal("\"not an object\"".toByteArray()) + file.writeBytes(iv + badPayload) + + val corruptions = mutableListOf() + val relaunched = + DesktopAccountStorage(secureStorage, tempDir, onCorruption = { corruptions += it }) + + val loaded = relaunched.loadAccounts() + assertTrue(loaded.isEmpty()) + + val siblings = amethystDir.listFiles().orEmpty().map { it.name } + assertTrue(siblings.any { it.startsWith("accounts.json.enc.jsonerror.") }) + assertTrue(corruptions.any { it is StorageCorruption.JsonMalformed }) + } + + // --- Bug 3: cross-process file lock --- + + @Test + fun `writeMetadataToDisk concurrent saves serialize under file lock`() { + val executor = Executors.newFixedThreadPool(4) + try { + runBlocking { + withContext(executor.asCoroutineDispatcher()) { + val jobs = + (1..8).map { idx -> + async { + storage.saveAccount( + AccountInfo( + npub = "npub1parallel$idx", + signerType = SignerType.Internal, + ), + ) + } + } + jobs.awaitAll() + } + } + // All eight accounts present, file not truncated. + val loaded = runBlocking { storage.loadAccounts() } + assertEquals(8, loaded.size) + val npubs = loaded.map { it.npub }.toSet() + assertEquals((1..8).map { "npub1parallel$it" }.toSet(), npubs) + + // Lock sidecar exists and is respected. + val lockFile = File(File(tempDir, ".amethyst"), "accounts.json.enc.lock") + assertTrue(lockFile.exists()) + } finally { + executor.shutdownNow() + } + } + + private fun assertContentEquals( + expected: ByteArray, + actual: ByteArray, + ) { + assertEquals(expected.size, actual.size, "byte size mismatch") + for (i in expected.indices) { + if (expected[i] != actual[i]) { + throw AssertionError("byte differs at index $i: expected=${expected[i]} actual=${actual[i]}") + } + } + } } diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt index 7edddfa185..3b017fff94 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/benchmark/LaunchScenario.kt @@ -95,6 +95,7 @@ object LaunchScenario { val tempHome = createTempDirectory("launch-scenario").toFile() val storage = mockk(relaxed = true) coEvery { storage.getPrivateKey(any()) } returns null + coEvery { storage.getPrivateKeyOrThrow(any()) } returns null File(tempHome, ".amethyst").mkdirs() val account = AccountManager(storage, tempHome) diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt index e645ca4cfc..cd98c7ea9c 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/ui/AppStateMachineTest.kt @@ -89,6 +89,7 @@ class AppStateMachineTest { File(tempDir, ".amethyst").mkdirs() storage = mockk(relaxed = true) coEvery { storage.getPrivateKey(any()) } returns null + coEvery { storage.getPrivateKeyOrThrow(any()) } returns null harnessScope = CoroutineScope(Dispatchers.Default + SupervisorJob()) relay = LaunchFixtureRelay.open(LaunchFixture.build(noteCount = 0).events) } From 1ea820e699543db083c44c97432ddb3d10c22db7 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Sat, 12 Sep 2026 17:43:12 -0400 Subject: [PATCH 14/16] fix(desktop): allow first-launch key bootstrap and stop caching unwritten state Two defects found reviewing the strict-keychain fix. 1. Fresh Linux/Windows installs could never persist an account. getPrivateKeyOrThrow turns any PasswordAccessException into a SecureStorageException on non-macOS backends, but every backend java-keyring ships throws that exact exception for a *genuinely absent* credential: - WinCredentialStoreBackend: CredReadA false (ERROR_NOT_FOUND) -> throw - FreedesktopKeyringBackend: empty object paths -> throwNoExistingCredentialException - KWalletBackend: hasEntry false -> "Password is not in wallet" So the strict lookup structurally cannot report "definitively absent" there, the create branch in getOrCreateKey was unreachable, and nothing between it and AccountManager.addAccountToStorage catches the throw. getOrCreateKey now bootstraps a fresh key when the strict lookup fails *and* accounts.json.enc does not exist. With no ciphertext on disk there is nothing a new key can orphan, so the invariant the strict contract protects is untouched: once the file exists the exception propagates exactly as before. 2. writeCachedMetadata updated the in-memory cache before the disk write, so a failed write (keychain refusal, I/O error, disk full) left the session serving accounts that were never persisted -- a save that reported success and vanished on the next launch. Persist first, cache second. Both are pinned by new tests, and both were mutation-checked: reverting either fix fails exactly one of them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VgVDQQXAg4cmzsWHoJj61k --- .../desktop/account/DesktopAccountStorage.kt | 42 +++++++++++-- .../account/DesktopAccountStorageTest.kt | 61 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt index 9ef274a2ad..e1813ff197 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorage.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.desktop.account import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.fasterxml.jackson.module.kotlin.readValue import com.vitorpamplona.amethyst.commons.keystorage.SecureKeyStorage +import com.vitorpamplona.amethyst.commons.keystorage.SecureStorageException import com.vitorpamplona.amethyst.commons.model.account.AccountInfo import com.vitorpamplona.amethyst.commons.model.account.AccountStorage import com.vitorpamplona.amethyst.commons.model.account.SignerType @@ -145,9 +146,17 @@ class DesktopAccountStorage( return loaded } + /** + * Persists first, caches second. + * + * If the disk write fails (keychain refused, I/O error, disk full) the in-memory + * cache must NOT be left claiming a state that was never written: the rest of the + * session would serve accounts that vanish on the next launch, and the user would + * see a successful save that silently did nothing. + */ private suspend fun writeCachedMetadata(metadata: AccountMetadata) { - cachedMetadata = metadata writeMetadataToDisk(metadata) + cachedMetadata = metadata } // --- Encrypted file I/O --- @@ -280,7 +289,9 @@ class DesktopAccountStorage( * - key exists in keychain: use it * - keychain confirms definitively absent: generate + persist a fresh key * - any other outcome (user cancelled/denied prompt, keychain locked, - * backend transient error): propagate the exception, do NOT rotate. + * backend transient error): propagate the exception, do NOT rotate -- + * unless there is no accounts.json.enc yet, in which case there is no + * ciphertext to orphan and we bootstrap a fresh key (see below). * * Rotating the AES key on an ambiguous miss silently destroys the ability * to decrypt the existing accounts.json.enc, wiping the logged-in accounts @@ -289,14 +300,37 @@ class DesktopAccountStorage( private suspend fun getOrCreateKey(): ByteArray { cachedKey?.let { return it } - val existing = secureStorage.getPrivateKeyOrThrow(METADATA_KEY_ALIAS) + val existing = + try { + secureStorage.getPrivateKeyOrThrow(METADATA_KEY_ALIAS) + } catch (e: SecureStorageException) { + // Bootstrap escape. Every non-macOS backend java-keyring ships + // (Windows Credential Store, Freedesktop Secret Service, KWallet) + // throws PasswordAccessException for a *genuinely absent* credential, + // so the strict lookup structurally cannot report "definitively + // absent" there. Without this branch a fresh Linux/Windows install + // could never mint the key and could never persist an account. + // + // Minting is only safe while there is no accounts.json.enc: with no + // ciphertext on disk there is nothing a new key can orphan. Once the + // file exists the strict contract applies and we propagate. + if (getAccountsFile().exists()) throw e + Log.w( + "DesktopAccountStorage", + "Keychain lookup failed and no accounts file exists; bootstrapping a fresh metadata key", + e, + ) + null + } + if (existing != null) { val key = Base64.getDecoder().decode(existing) cachedKey = key return key } - // Definitively absent: safe to create and persist a fresh key. + // Definitively absent (or bootstrapping with nothing on disk): safe to + // create and persist a fresh key. val key = ByteArray(AES_KEY_SIZE).also { SecureRandom().nextBytes(it) } secureStorage.savePrivateKey(METADATA_KEY_ALIAS, Base64.getEncoder().encodeToString(key)) cachedKey = key diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt index c1c4a88993..a4d2e9a260 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/DesktopAccountStorageTest.kt @@ -265,6 +265,67 @@ class DesktopAccountStorageTest { } } + @Test + fun `getOrCreateKey ambiguous error with no accounts file bootstraps a fresh key`() = + runTest { + // Every non-macOS backend java-keyring ships (Windows Credential Store, + // Freedesktop Secret Service, KWallet) throws PasswordAccessException for a + // *genuinely absent* credential, which the strict lookup surfaces as + // SecureStorageException. With no accounts.json.enc there is no ciphertext + // a new key could orphan, so a fresh install must still be able to mint one + // -- otherwise Linux/Windows can never persist an account at all. + val saved = mutableMapOf() + val throwingStorage: SecureKeyStorage = mockk() + coEvery { throwingStorage.getPrivateKeyOrThrow("account-metadata-key") } throws + SecureStorageException("Keyring backend refused access or returned ambiguous not-found") + coEvery { throwingStorage.savePrivateKey(any(), any()) } answers { + saved[firstArg()] = secondArg() + } + coEvery { throwingStorage.getPrivateKey(any()) } answers { saved[firstArg()] } + coEvery { throwingStorage.hasPrivateKey(any()) } answers { saved.containsKey(firstArg()) } + + val file = File(File(tempDir, ".amethyst"), "accounts.json.enc") + assertFalse(file.exists()) + + val fresh = DesktopAccountStorage(throwingStorage, tempDir) + fresh.saveAccount(AccountInfo("npub1freshinstall", SignerType.Internal)) + + assertNotNull(saved["account-metadata-key"]) + assertTrue(file.exists()) + assertEquals(listOf("npub1freshinstall"), fresh.loadAccounts().map { it.npub }) + // The escape is bootstrap-only: once the file exists the strict contract + // applies again -- pinned by `getOrCreateKey keyring throws ambiguous error + // does not rotate key or touch file` above. + } + + // --- Cache must never claim a state that was not persisted --- + + @Test + fun `failed disk write does not poison the in-memory cache`() = + runTest { + storage.saveAccount(AccountInfo("npub1persisted", SignerType.Internal)) + assertEquals(listOf("npub1persisted"), storage.loadAccounts().map { it.npub }) + + // Block the atomic-write temp path so writeMetadataToDisk fails. + val temp = File(File(tempDir, ".amethyst"), "accounts.json.enc.tmp") + assertTrue(temp.mkdirs()) + + assertFails { + runBlocking { + storage.saveAccount(AccountInfo("npub1phantom", SignerType.Internal)) + } + } + + // Same instance: the cache must still reflect only what reached the disk, + // not the account the failed save handed it. + assertEquals(listOf("npub1persisted"), storage.loadAccounts().map { it.npub }) + + // And the on-disk file agrees. + temp.delete() + val relaunched = DesktopAccountStorage(secureStorage, tempDir) + assertEquals(listOf("npub1persisted"), relaunched.loadAccounts().map { it.npub }) + } + // --- Bug 2: read failure must not silently reset the file --- @Test From 5b846d64d60055de16aee4dceea55aca8388411e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 22:11:23 +0000 Subject: [PATCH 15/16] fix(relay-server): answer a duplicate EVENT with OK true, per NIP-01 Running the Marmot headless harness against the embedded geode relay failed 10 of 29 scenarios, every one on the same reply: the relay answered a resent EVENT with ["OK", , false, "Error code: 2067, message: UNIQUE constraint failed: event_headers.id"] NIP-01 says a relay that already holds the event answers ["OK", , true, "duplicate: already have this event"], and every client here depends on that: amethyst's outbox writes an event as soon as the socket is ready and resends it when the connection finishes syncing, so one of the two copies is always a duplicate; MDK's wn counts a `duplicate:` prefix as idempotent success but files an unclassified OK false as "publish acknowledgement unknown" and keeps retrying. Both amy's group commits and wn's KeyPackage publish were failing on it, while nostr-rs-relay had answered the resend correctly. SQLiteEventStore now recognises the unique-index violation on event_headers.id and reports RejectionReason.DUPLICATE, the constant that already carried NIP-01's exact wording but was never produced; RelaySession sends `OK true` for a `duplicate:` reason and keeps `OK false` for every other rejection. The store outcome stays Rejected, so a duplicate is still not fanned out to live subscriptions or counted as a new write by the mirror worker and importer. The filesystem store already treated a duplicate insert as a no-op. Two tests pinned the old OK false behaviour (NostrServerTest, KtorRelayTest) and now assert the NIP-01 reply. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PguqnDbP2v11dtANs9xdxc --- .../kotlin/com/vitorpamplona/geode/KtorRelayTest.kt | 6 +++--- .../quartz/nip01Core/relay/server/RelaySession.kt | 9 ++++++++- .../quartz/nip01Core/store/RejectionReason.kt | 3 +++ .../quartz/nip01Core/store/sqlite/SQLiteEventStore.kt | 10 ++++++++++ .../quartz/nip01Core/relay/server/NostrServerTest.kt | 10 ++++++++-- 5 files changed, 32 insertions(+), 6 deletions(-) diff --git a/geode/src/test/kotlin/com/vitorpamplona/geode/KtorRelayTest.kt b/geode/src/test/kotlin/com/vitorpamplona/geode/KtorRelayTest.kt index b79f81bf51..ea1a1352bd 100644 --- a/geode/src/test/kotlin/com/vitorpamplona/geode/KtorRelayTest.kt +++ b/geode/src/test/kotlin/com/vitorpamplona/geode/KtorRelayTest.kt @@ -311,14 +311,14 @@ class KtorRelayTest { ) assertEquals(true, ok, "successful insert must round-trip OK true on the wire") - // Duplicate insert returns OK false; this also exercises the - // "non-empty message" branch of the serializer. + // Duplicate insert returns OK true with a `duplicate:` message (NIP-01); + // this also exercises the "non-empty message" branch of the serializer. val ok2 = client.publishAndConfirm( event = event, relayList = setOf(server.url.normalizeRelayUrl()), ) - assertEquals(false, ok2, "duplicate insert must round-trip OK false") + assertEquals(true, ok2, "duplicate insert must round-trip OK true (NIP-01 duplicate:)") } /** diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt index 9520dd9848..33a01e0e92 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt @@ -41,6 +41,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.server.policies.IRelayPolicy import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PolicyResult import com.vitorpamplona.quartz.nip01Core.store.IEventStore import com.vitorpamplona.quartz.nip01Core.store.RawEvent +import com.vitorpamplona.quartz.nip01Core.store.RejectionReason import com.vitorpamplona.quartz.nip77Negentropy.NegCloseCmd import com.vitorpamplona.quartz.nip77Negentropy.NegMsgCmd import com.vitorpamplona.quartz.nip77Negentropy.NegOpenCmd @@ -212,7 +213,13 @@ class RelaySession( } is IEventStore.InsertOutcome.Rejected -> { - send(OkMessage(cmd.event.id, false, outcome.reason)) + // NIP-01: an event the relay already holds is acknowledged with + // `OK true` and the `duplicate:` prefix. Every real client + // (amethyst's outbox included) resends an event whose OK has not + // landed yet, and treats OK false as a rejection to surface — + // so answering false here turns a routine resend into an error. + val duplicate = outcome.reason.startsWith(RejectionReason.PREFIX_DUPLICATE) + send(OkMessage(cmd.event.id, duplicate, outcome.reason)) } is IEventStore.InsertOutcome.Failed -> { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt index 4e14cb2550..90e416f230 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt @@ -44,6 +44,9 @@ object RejectionReason { */ const val PREFIX_REPLACED = "replaced:" + /** NIP-01 prefix for "already have this event" — answered with `OK true`, not false. */ + const val PREFIX_DUPLICATE = "duplicate:" + // The standard store reasons. const val DUPLICATE = "duplicate: already have this event" const val EXPIRED = "blocked: Cannot insert an expired event" diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt index 1b3643543c..080d26f4c4 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt @@ -62,6 +62,8 @@ class SQLiteEventStore( val extraPragmas: List = emptyList(), ) { companion object { + /** SQLite's message for the unique index on `event_headers (id)`. */ + private const val DUPLICATE_ID_CONSTRAINT = "UNIQUE constraint failed: event_headers.id" const val DATABASE_VERSION = 5 } @@ -526,6 +528,14 @@ class SQLiteEventStore( */ private fun classifyRowError(e: Throwable): IEventStore.InsertOutcome { val message = e.message ?: e::class.simpleName ?: RejectionReason.INSERT_FAILED + // A second copy of an event the store already holds trips the unique index on + // event_headers.id. That is not a refusal of the event but a statement that it + // is already here, and NIP-01 has a dedicated answer for it (`OK true` with the + // `duplicate:` prefix) — so name it, instead of leaking SQLite's constraint text + // for the session to turn into a rejection the client then retries or reports. + if (message.contains(DUPLICATE_ID_CONSTRAINT)) { + return IEventStore.InsertOutcome.Rejected(RejectionReason.DUPLICATE) + } val refusal = message.contains("blocked:") || message.contains("duplicate:") || diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt index 7171a9e4a0..39d27b15fc 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt @@ -120,8 +120,13 @@ class NostrServerTest { server.close() } + /** + * NIP-01: `["OK", , true, "duplicate: already have this event"]`. A client + * resends any event whose OK has not landed, so a duplicate must read as + * success — OK false would make every such resend look like a rejection. + */ @Test - fun duplicateEventReturnsOkFalse() = + fun duplicateEventReturnsOkTrueWithDuplicatePrefix() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val store = EventStore(null) @@ -138,7 +143,8 @@ class NostrServerTest { val okMessages = collector.rawMessagesContaining("OK") assertEquals(2, okMessages.size) assertTrue(okMessages[0].contains(",true,")) - assertTrue(okMessages[1].contains(",false,")) + assertTrue(okMessages[1].contains(",true,")) + assertTrue(okMessages[1].contains("duplicate:")) server.close() } From 18c576a068e6793b6d62018a0d1ace2315f32b4f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 22:18:04 +0000 Subject: [PATCH 16/16] fix(relay-server): acknowledge a superseded replaceable with OK true duplicate: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second finding from the Marmot headless harness on geode. wn's `keys publish` mints a KeyPackage (kind 30443, same `d` tag) in the same second as the one its bootstrap already published; NIP-01's lowest-id-wins tie keeps the stored one, and the insert of the loser trips the addressable unique index. The store classified that as a rejection carrying SQLite's text — "UNIQUE constraint failed: event_headers.kind, event_headers.pubkey, event_headers.d_tag" — so the relay answered OK false with a reason no client can classify, and MDK filed it as "publish acknowledgement unknown" and retried forever. nostr-rs-relay, which this harness was validated against, does not even attempt the insert when a newer version exists and acknowledges the event as `OK true "duplicate: ..."` (its Duplicate status maps to true). Match that: the replaceable and addressable unique-index failures now classify as RejectionReason.SUPERSEDED, a `duplicate:`-prefixed reason the session already answers with OK true. The stored version is untouched, nothing is fanned out, and the STORE-W01/W02 contract in the event-store-semantics skill is updated to say so. Tests: NostrServerTest covers an older kind-0 re-insert and the same-second kind-30443 tie, asserting the OK true duplicate: reply and that the winner remains the only stored version. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PguqnDbP2v11dtANs9xdxc --- .claude/skills/event-store-semantics/SKILL.md | 7 +- .../quartz/nip01Core/store/RejectionReason.kt | 10 +++ .../store/sqlite/SQLiteEventStore.kt | 12 ++++ .../nip01Core/relay/server/NostrServerTest.kt | 64 +++++++++++++++++++ 4 files changed, 91 insertions(+), 2 deletions(-) diff --git a/.claude/skills/event-store-semantics/SKILL.md b/.claude/skills/event-store-semantics/SKILL.md index 1c99e335a9..57a9e9a7d0 100644 --- a/.claude/skills/event-store-semantics/SKILL.md +++ b/.claude/skills/event-store-semantics/SKILL.md @@ -143,8 +143,11 @@ messages quoted below (they surface as the NIP-01 `OK false` reason). kinds. A `BEFORE INSERT` trigger deletes any stored version that is *older* — meaning `created_at` smaller, **or equal `created_at` with lexicographically larger id** (NIP-01 lowest-id-wins). Inserting a version that is *not* newer under that ordering leaves the stored -row in place and fails the unique index → rejected (`UNIQUE constraint failed`). Net contract: -exactly one version stored; newest wins; ties broken by lowest id; older re-inserts blocked. +row in place and fails the unique index → rejected with `RejectionReason.SUPERSEDED` +(`duplicate: a newer version of this replaceable event is already stored`), which the relay +session answers with `OK true` exactly like an id duplicate (NIP-01 `duplicate:` prefix; same +reply nostr-rs-relay gives). Net contract: exactly one version stored; newest wins; ties broken +by lowest id; older re-inserts blocked but acknowledged as already covered. **STORE-W02 — addressable supersession.** Same as W01 with unique index `(kind, pubkey, d_tag)` over `30000 ≤ kind < 40000`. Nuance: `d_tag` is populated from the diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt index 90e416f230..00d6ccc7dc 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/RejectionReason.kt @@ -49,6 +49,16 @@ object RejectionReason { // The standard store reasons. const val DUPLICATE = "duplicate: already have this event" + + /** + * A replaceable or addressable event that a stored version already supersedes + * (newer `created_at`, or the same `created_at` and a lower id). Nothing is + * written, and — like [DUPLICATE] — the relay answers `OK true`: NIP-01 keeps + * `duplicate:` as the machine-readable prefix for "already covered", and that + * is what nostr-rs-relay sends here too, so clients that retry on anything + * else (MDK's `wn`) settle instead of re-offering the same event forever. + */ + const val SUPERSEDED = "duplicate: a newer version of this replaceable event is already stored" const val EXPIRED = "blocked: Cannot insert an expired event" const val DELETED = "blocked: a deletion event exists" const val VANISHED = "blocked: a request to vanish event exists" diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt index 080d26f4c4..a56fb390f4 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt @@ -64,6 +64,12 @@ class SQLiteEventStore( companion object { /** SQLite's message for the unique index on `event_headers (id)`. */ private const val DUPLICATE_ID_CONSTRAINT = "UNIQUE constraint failed: event_headers.id" + + /** + * Common prefix of SQLite's messages for `replaceable_idx` (`kind, pubkey`) and + * `addressable_idx` (`kind, pubkey, d_tag`) — both start with these two columns. + */ + private const val SUPERSEDED_CONSTRAINT = "UNIQUE constraint failed: event_headers.kind, event_headers.pubkey" const val DATABASE_VERSION = 5 } @@ -536,6 +542,12 @@ class SQLiteEventStore( if (message.contains(DUPLICATE_ID_CONSTRAINT)) { return IEventStore.InsertOutcome.Rejected(RejectionReason.DUPLICATE) } + // The replaceable / addressable unique indexes fire only when the supersession + // trigger found nothing older to delete, i.e. the stored version already wins + // (STORE-W01/W02). Same shape as a duplicate: nothing to write, `OK true`. + if (message.contains(SUPERSEDED_CONSTRAINT)) { + return IEventStore.InsertOutcome.Rejected(RejectionReason.SUPERSEDED) + } val refusal = message.contains("blocked:") || message.contains("duplicate:") || diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt index 39d27b15fc..bde070947f 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerTest.kt @@ -149,6 +149,70 @@ class NostrServerTest { server.close() } + /** + * STORE-W01: a replaceable event older than the stored version is not written, + * and the relay acknowledges it the way nostr-rs-relay does — `OK true` with the + * NIP-01 `duplicate:` prefix — rather than leaking the unique-index text as a + * rejection the client would keep retrying. + */ + @Test + fun olderReplaceableIsAcknowledgedAsDuplicateNotRejected() = + runTest { + val dispatcher = UnconfinedTestDispatcher(testScheduler) + val store = EventStore(null) + val server = createServer(dispatcher, store) + val collector = MessageCollector() + val c1 = server.connect(collector.sendCallback) + + val newer = testEvent(id = hexId(2), kind = 0, createdAt = 2000L) + val older = testEvent(id = hexId(3), kind = 0, createdAt = 1000L) + c1.insert(newer) + c1.insert(older) + + val okMessages = collector.rawMessagesContaining("OK") + assertEquals(2, okMessages.size) + assertTrue(okMessages[0].contains(",true,")) + assertTrue(okMessages[1].contains(",true,"), "older version must be acked, got ${okMessages[1]}") + assertTrue(okMessages[1].contains("duplicate:"), "older version must carry the duplicate: prefix") + + val stored = store.query(Filter(kinds = listOf(0))) + assertEquals(listOf(newer.id), stored.map { it.id }, "the newer version stays the only stored one") + + server.close() + } + + /** + * STORE-W02 tie: two addressable events with the same `d` tag and the same + * `created_at` — the lower id wins, the other is acknowledged as superseded. + * This is the exact shape MDK's `wn keys publish` produces when it mints a + * second KeyPackage within the same second as the first. + */ + @Test + fun sameSecondAddressableTieLoserIsAcknowledgedAsDuplicate() = + runTest { + val dispatcher = UnconfinedTestDispatcher(testScheduler) + val store = EventStore(null) + val server = createServer(dispatcher, store) + val collector = MessageCollector() + val c1 = server.connect(collector.sendCallback) + + val dTag = arrayOf(arrayOf("d", "kp")) + val lowerId = testEvent(id = hexId(4), kind = 30443, createdAt = 5000L, tags = dTag) + val higherId = testEvent(id = hexId(5), kind = 30443, createdAt = 5000L, tags = dTag) + c1.insert(lowerId) + c1.insert(higherId) + + val okMessages = collector.rawMessagesContaining("OK") + assertEquals(2, okMessages.size) + assertTrue(okMessages[1].contains(",true,"), "tie loser must be acked, got ${okMessages[1]}") + assertTrue(okMessages[1].contains("duplicate:")) + + val stored = store.query(Filter(kinds = listOf(30443))) + assertEquals(listOf(lowerId.id), stored.map { it.id }, "lowest id wins the tie") + + server.close() + } + // -- REQ command ----------------------------------------------------------- @Test