From c8cbd936f1609fa6e0628acfa5660219098bb585 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sat, 5 Sep 2026 23:32:53 +0200 Subject: [PATCH] docs: record where the sync's safety net is, and where it is not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two updates after the test pass. long-running-sync.md gains a section naming what each test file pins and, more usefully, the three things they cannot reach: NostrSocketClientImpl's reconnect loop and ordered inbound (exercised only through their extracted arithmetic — covering them wants a fake WebSocketSession), everything downstream of saveNostrEvent (Room-backed, and there is no sqlite driver on the JVM test classpath), and the app on a device. The manual checks stay the manual checks. It also records that the tests were verified by mutation rather than by passing, so the next person knows the assertions were confirmed to bite. dead-code.md's line references are refreshed — the testability seams shifted most of them — and it now says which commit they were correct at and to confirm with the grep rather than trusting them. One entry added: the DefaultNostrSocketClientFactory overload taking an explicit HttpClient has no caller now that everything goes through the interface method. Co-Authored-By: Claude Opus 5 --- docs/dead-code.md | 26 +++++++++++++++----------- docs/long-running-sync.md | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/docs/dead-code.md b/docs/dead-code.md index e8daa3c3..828838c2 100644 --- a/docs/dead-code.md +++ b/docs/dead-code.md @@ -19,6 +19,9 @@ every `.kt` file outside `build/` and counts occurrences: grep -rn "\bidentifierName\b" --include=*.kt composeApp/src | grep -v '/build/' ``` +Line numbers below were correct at `f5eb744` and drift with every edit to those +files; treat them as a hint and confirm with the grep. + Two things that method cannot see, so each item was also read in context: - **Room DAOs** are called through generated code as `database.xDao().method()`, @@ -54,16 +57,17 @@ Each of these predates the sync work. | what | where | note | |---|---|---| -| `RelayPool.removeRelays` | [RelayPool.kt:119](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:119) | never called; `changeRelays` and `closePool` cover every path that removes a relay | -| `RelayPool.hasRelays` | [RelayPool.kt:167](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:167) | never called | -| `RelayPool.transformWhileEventsAreIncoming` | [RelayPool.kt:482](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:482) | private, never called. Superseded by `completeOnSubscriptionEnd`, which ends a flow on EOSE/CLOSED/NEG-ERR rather than on "the last message was not an event" | -| the commented-out publish gate | [RelayPool.kt:543-556](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:543) | the only thing keeping `import kotlinx.coroutines.flow.transform` ([:30](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:30)) alive | +| `RelayPool.removeRelays` | [RelayPool.kt:123](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:123) | never called; `changeRelays` and `closePool` cover every path that removes a relay | +| `RelayPool.hasRelays` | [RelayPool.kt:171](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:171) | never called | +| `RelayPool.transformWhileEventsAreIncoming` | [RelayPool.kt:486](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:486) | private, never called. Superseded by `completeOnSubscriptionEnd`, which ends a flow on EOSE/CLOSED/NEG-ERR rather than on "the last message was not an event" | +| the commented-out publish gate | [RelayPool.kt:547-560](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:547) | the only thing keeping `import kotlinx.coroutines.flow.transform` ([:30](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelayPool.kt:30)) alive | | `RelaysSocketManager.clearRelayPools` | [RelaysSocketManager.kt:100](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:100) | private, never called. Nothing tears the pool down on sign-out | -| `RelaysSocketManager.tryConnectingToAllRelays` | [RelaysSocketManager.kt:132](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:132) | never called; the only caller of `RelayPool.tryConnectingToRelay`, which is otherwise dead too | -| the commented `tryConnectingToUserRelay` | [RelaysSocketManager.kt:140](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:140) | | +| `RelaysSocketManager.tryConnectingToAllRelays` | [RelaysSocketManager.kt:131](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:131) | never called; the only caller of `RelayPool.tryConnectingToRelay`, which is otherwise dead too | +| the commented `tryConnectingToUserRelay` | [RelaysSocketManager.kt:139](composeApp/src/commonMain/kotlin/press/mantra/compose/network/relays/RelaysSocketManager.kt:139) | | | `NostrIncomingMessage?.verifyOrThrow` | [NostrIncomingMessage.kt:59](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrIncomingMessage.kt:59) | never called. It also treats any NOTICE as a failure, which is wrong for the reason `filterByEventId` documents — a NOTICE has no subscription id and reaches every collector | | `String?.decodeFromJsonStringOrNull` | [CommonJson.kt:37](composeApp/src/commonMain/kotlin/press/mantra/compose/network/serialization/CommonJson.kt:37) | never called | -| `NostrSocketClientImpl.compressMessage` | [NostrSocketClientImpl.kt:384](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:384) | already carries `@Suppress("unused")`. Outgoing compression is not a thing any relay here asks for | +| `NostrSocketClientImpl.compressMessage` | [NostrSocketClientImpl.kt:368](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:368) | already carries `@Suppress("unused")`. Outgoing compression is not a thing any relay here asks for | +| `DefaultNostrSocketClientFactory.create(httpClient = …)` | [NostrSocketClientFactory.kt](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientFactory.kt) | the overload taking an explicit `HttpClient` has no caller; everything goes through the interface method and `defaultSocketsHttpClient` | ### `RelaysSocketManager.userRelays` is a field that is never written @@ -98,10 +102,10 @@ command with `OptimizedJsonMapper` and goes through `sendMESSAGE`. | declaration | impl | |---|---| -| `sendREQ` [:37](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:37) | [:349](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:349) | -| `sendCLOSE` [:31](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:31) | [:361](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:361) | -| `sendCOUNT` [:33](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:33) | [:354](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:354) | -| `sendAUTH` [:29](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:29) | [:365](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:365) | +| `sendREQ` [:37](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:37) | [:349](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:333) | +| `sendCLOSE` [:31](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:31) | [:361](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:345) | +| `sendCOUNT` [:33](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:33) | [:354](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:338) | +| `sendAUTH` [:29](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClient.kt:29) | [:365](composeApp/src/commonMain/kotlin/press/mantra/compose/network/sockets/NostrSocketClientImpl.kt:349) | Removing them also orphans `buildNostrREQMessage`, `buildNostrCOUNTMessage`, `buildNostrCLOSEMessage` and `buildNostrAUTHMessage` in diff --git a/docs/long-running-sync.md b/docs/long-running-sync.md index 9ed04e0e..56f4e5da 100644 --- a/docs/long-running-sync.md +++ b/docs/long-running-sync.md @@ -199,6 +199,41 @@ and its indexes, so two of those interleaving is a lost update — survivable wh a single queue was the only writer, not survivable with a live subscription writing alongside a backfill. +## What is tested, and what is not + +Every failure mode in this subsystem is silent — a filter that drops messages, a +subscription that stops being replayed, a group id that never reaches an `#h` +tag. None of them throws, and all of them present as "some messages didn't +arrive", later, on someone else's phone. So the tests are aimed at the decisions +rather than at the plumbing: + +| file | pins | +|---|---| +| `RelayPoolSubscriptionTest` | retain on query, replay on reconnect, release on close, never replay negentropy, per-relay scoping — and that a live subscription keeps delivering after EOSE while a one-shot query still ends at it | +| `LiveSubscriptionReconcileTest` | the group filter following membership: widening in place rather than reopening, dropping a left group, closing when the last one goes, collapsing churn inside the debounce, and the CLOSED/back-pressure reopen behaviour | +| `LiveSubscriptionPlanTest` | the filter shapes, led by `since` being absent from the gift wrap filter | +| `RelayBackPressureTest`, `ReconnectBackoffTest` | the two pure decisions: which CLOSED reasons mean "ease off", and the backoff arithmetic including the exponent clamp | + +They were checked by mutation rather than by passing: reintroducing a `since`, +making EOSE terminal, dropping the `leftGroupAt` filter, removing retention from +`query()`, reconciling by close-and-reopen, removing the debounce and removing +the back-pressure branch each produce failures. + +Three things this does **not** cover, and could not without a real relay or a +database: + +- **`NostrSocketClientImpl` itself.** The reconnect loop, the session-identity + check on teardown, and the ordered inbound emission are exercised only through + their extracted arithmetic. Testing them wants a fake `WebSocketSession`. +- **Anything downstream of `saveNostrEvent`.** Indexing, gift wrap unwrapping, + Welcome handling and MLS decryption are Room-backed, and there is no sqlite + driver on the JVM test classpath (see the note in `build-verification-commands`). + The tests assert the event reaches the repository with the right relay and + level, and stop there. +- **The app on a device.** Nothing here proves a DM lands with the chat list + closed; that still wants the two manual checks — send yourself a message from + another client, and have a second device add you to a group. + ## Not done - **Connectivity changes.** A network switch mid-foreground is only noticed by