Land on the conversation list, not in the last DM

#1064 asked that the app stop dropping people into a plaintext broadcast
they never chose. Re-opening the last private chat answered that, but it
paid for it with a worse disclosure: launch put a contact's nickname and
message history on screen before anyone chose to show them, to whoever
is holding an unlocked phone or standing behind it. Nothing gates that
today - the app-lock work is #1664. And the restorability rule selected
favorites, so the conversations it surfaced were the ones worth most to
an observer.

Present the conversation list instead. It gets #1064's actual ask, and
since #1598 the list carries a recent-chats section deduped by
fingerprint, so the last conversation is one tap away rather than lost -
which is what made the auto-reopen worth its cost in the first place.

A persisted DM and a stale one now take the same branch, so the
machinery that told them apart has nothing left to decide:
isDirectChatRestorable, shouldPresentConversationList and the
suppressSystemMessages plumbing are deleted, and
restoreLastActiveConversation drops its resolvability predicate.
ChatPeerIdentityCoordinator returns to main's signature exactly.

Tests: ConversationStoreLastActiveTests goes 32 -> 7 (610 -> 180 lines);
the ~25 that covered the deleted predicate went with it. The survivors
pin persistence and the launch decision, and one new test pins the
security property directly - a persisted DM presents the list and never
the chat, whoever the peer is.

Verified by running, not compiling: 7 XCTest + 31 swift-testing across
ConversationStoreLastActiveTests, AppArchitectureTests and
ChatPeerIdentityCoordinatorContextTests, 0 failures, exit 0 (macOS).

Raised by Chessing234 in review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ecgang 2026-08-26 10:07:18 -06:00
parent be2275ec71
commit c1baf8392b
6 changed files with 98 additions and 778 deletions

View File

@ -159,217 +159,23 @@ final class AppRuntime: ObservableObject {
record(.startupCompleted)
}
/// #1064: restore the last-active conversation at launch. A persisted DM
/// re-opens via the normal private-chat path (which never writes
/// `activeChannel`); a first-ever launch or a stale DM peer presents the
/// conversation list; a public channel defers to the existing mesh /
/// `GeoChannelCoordinator` restore (the sole launch-time writer of
/// `activeChannel`), so there is no race.
/// #1064: decide what launch presents. A persisted public channel defers
/// to the existing mesh / `GeoChannelCoordinator` restore (the sole
/// launch-time writer of `activeChannel`), so there is no race; everything
/// else raises the conversation list.
///
/// Launch deliberately does not re-open a persisted DM. Opening it would
/// put a contact's name and message history on screen before anyone chose
/// to show them, and nothing gates that today (the app-lock work is #1664).
/// The list is what #1064 actually asked for, and since #1598 it carries a
/// recent-chats section, so the last conversation is one tap away rather
/// than lost.
private func restoreLastActiveConversationOnLaunch() {
let presentation = conversations.restoreLastActiveConversation(
isPeerResolvable: {
Self.isDirectChatRestorable(
$0,
favorites: .shared,
hasStoredCryptographicIdentity: {
!chatViewModel.identityManager
.getCryptoIdentitiesByPeerIDPrefix($0.toShort())
.isEmpty
},
isPeerBlocked: { chatViewModel.isPeerBlocked($0) }
)
}
)
var didOpenDirectChat = false
if case .restoredDirectChat(let peerID) = presentation {
// `startPrivateChat`'s gate (ChatPeerIdentityCoordinator) rejects a
// now-blocked peer by emitting a system message and returning
// WITHOUT opening the chat. At launch that message would land in
// the current (public mesh) timeline, so pass
// `suppressSystemMessages: true` — the reject stays silent and we
// detect it via `selectedPrivateChatPeer`, which is only set on the
// success path.
//
// Not a second line of defence any more. Post-#1415 that gate
// screens only self, group and blocked, so it catches nothing
// `isDirectChatRestorable` has not already caught. It is kept for
// the narrow race where the peer is blocked between the predicate
// and this call, and for the silent-failure detection above.
chatViewModel.startPrivateChat(with: peerID, suppressSystemMessages: true)
didOpenDirectChat = chatViewModel.selectedPrivateChatPeer == peerID
}
// Fall back to the conversation list rather than silently landing on
// the public mesh timeline when a restore target existed but could not
// be opened.
if Self.shouldPresentConversationList(for: presentation, didOpenDirectChat: didOpenDirectChat) {
if conversations.restoreLastActiveConversation() == .conversationList {
appChromeModel.showSidebar = true
}
}
/// Whether a persisted last-active DM peer is genuinely restorable at
/// launch — validated against *durable* relationship state, never live
/// presence (mesh discovery is async, so no peer is connected yet). A
/// syntactically valid `PeerID` is NOT sufficient: an unknown peer would
/// otherwise fall straight through `startPrivateChat` into an empty phantom
/// DM.
///
/// Restorable iff the peer is NOT blocked and we hold durable evidence
/// *locally* that the conversation is addressable: we favorited them, or we
/// have a stored cryptographic identity for them.
///
/// `theyFavoritedUs` is deliberately NOT a term. It is remote state, and on
/// its own it proves nothing about our ability to address the peer — a peer
/// who favorited us, whom we never favorited and hold no identity for, is
/// exactly the unaddressable phantom this predicate exists to refuse. It
/// would also override a deliberate local unfavorite on the strength of the
/// other side's opinion.
///
/// This tracks `ChatPeerIdentityCoordinator.startPrivateChat`, which #1415
/// relaxed — it no longer requires a mutual favorite, on the grounds that
/// store-and-forward (couriers, bridge drops, retained outbox) needs only
/// the recipient's noise key, so "the router decides what delivery looks
/// like, not chat entry". Keeping the old mutual-favorite rule here would
/// have made launch-restore stricter than chat entry: a one-way-favorite or
/// merely-known peer whose DM is perfectly sendable would fail to restore
/// and drop the user on the conversation list instead.
///
/// It cannot simply defer to that gate, though. Post-#1415 the open path
/// screens only self, group and blocked, so this predicate is the sole
/// defence against restoring into a phantom DM, and it has to hold the line
/// on its own.
///
/// The terms are chosen for durability at launch. Favorites are
/// keychain-backed; stored cryptographic identities are on disk. The outbox
/// and live Noise session state are deliberately NOT consulted — the outbox
/// defers loading until protected data is available and session state is
/// in-memory, so both read empty at launch regardless of the truth.
///
/// A private group is a virtual conversation, so none of the peer terms
/// apply to it and none of them would pass: a group id is `group_` plus 32
/// hex, and both lookups guard on `isShort` (a 16-hex bare), so they return
/// empty for every group without ever consulting the group. Left to the
/// peer terms a group would therefore never restore — silently, every
/// time. `startPrivateChat` gates group re-entry on nothing at all ("no
/// peer identity, favorites, handshake … just select the chat"), so
/// admitting groups here restores what re-entry would have opened. (Not
/// quite an identity: the block veto above has no counterpart in
/// `startPrivateChat`'s group branch, so restore is the narrower of the
/// two. Moot today — `isBlocked` needs a resolved fingerprint and a group
/// id never has one — but stated rather than leaned on.)
///
/// Note the bound on that admission: `isGroup` tests the `group_` prefix
/// only — `PeerID(str:)` assigns a prefix by `hasPrefix` and never
/// validates the bare — so this branch admits any persisted id *claiming*
/// to be a group, not a group proven to exist.
///
/// That is acceptable, but for a narrower reason than "the id is trusted".
/// A group id can certainly originate remotely: an invite carries one, and
/// `GroupProtocol` derives the conversation id from it. Every such id is
/// built by `PeerID(groupID:)`, which hex-encodes the raw bytes, so a
/// group learned from the network still has a structurally valid bare. A
/// malformed `group_` id therefore implies corrupted local persistence
/// rather than hostile input. And the failure it produces is an empty
/// group, not a DM compose box aimed at an unreachable peer — which is the
/// specific hazard this predicate exists to prevent. Admitting only groups
/// that still exist locally would need a membership lookup this predicate
/// does not take.
///
/// Geohash/Nostr ids are screened before the peer terms, so nothing can
/// bypass the check by matching earlier. A geoChat id is not a direct chat
/// at all. A geoDM's full Nostr key is rebuilt only from inbound ephemeral
/// events, so at launch it cannot resolve, and `startPrivateChat` skips the
/// handshake for geoDMs — a phantom would open with no error at all. Its
/// only conceivable durable anchor is our own favorite record.
///
/// In practice that anchor does not exist yet, so **a geoDM never restores
/// today**. `FavoritesPersistenceService` is keyed by Noise public key
/// alone, and `getFavoriteStatus(forPeerID:)` matches by rebuilding
/// `PeerID(publicKey:)` — which carries no prefix — so it can never equal a
/// `nostr_`-prefixed id no matter what is favorited. The geoDM branch below
/// is kept because it is the right shape once a Nostr-keyed lookup exists,
/// but read it precisely: it resolves to `false` for the *production*
/// closure only. The injected seam will happily return `true` for a stub
/// that accepts a geoDM, so a passing test here is not evidence that geoDM
/// restore works. Documented rather than fixed: wiring the real lookup
/// means new favorites plumbing, which is not this change.
static func isDirectChatRestorable(
_ peerID: PeerID,
isPeerFavorited: (PeerID) -> Bool,
hasStoredCryptographicIdentity: (PeerID) -> Bool,
isPeerBlocked: (PeerID) -> Bool
) -> Bool {
// Blocked is a veto, never one term among several — including over the
// group branch below, so no id class can sidestep it.
guard !isPeerBlocked(peerID) else { return false }
// A private group is local state, not a claim about reaching a peer.
// The peer terms below cannot represent it and would refuse it.
if peerID.isGroup { return true }
// A geohash channel is not a direct chat.
guard !peerID.isGeoChat else { return false }
// Screened before the identity term rather than after, so no earlier
// match can skip it. The identity lookup does not reject these on its
// own merits either: a `nostr_` id satisfies `isShort` (the prefix is
// not part of the length check), and today it misses only because the
// lookup re-attaches that prefix and no hex fingerprint starts with it.
// Luck, not a rule.
if peerID.isGeoDM { return isPeerFavorited(peerID) }
return isPeerFavorited(peerID) || hasStoredCryptographicIdentity(peerID)
}
/// Production wiring of `isDirectChatRestorable`, extracted so the real
/// favorites/block lookups (not just stub predicates) are unit-testable via
/// an injected in-memory-keychain-backed `FavoritesPersistenceService` and a
/// block closure. `migrateSelectedConversationIfNeeded` can persist the
/// last-active peer in full 64-hex Noise-key form, but the favorites store is
/// keyed by the short, Noise-key-derived id — so normalize with `toShort()`
/// (a no-op on an already-short id) before the lookup, or favorited DMs
/// silently fail to restore. The block lookup mirrors the open-path gate's
/// `unifiedIsBlocked` (fingerprint-resolved, so it works for offline
/// favorites).
///
/// The identity lookup is passed in rather than reached for: it lives on
/// the injected `SecureIdentityStateManagerProtocol`, so tests stub it the
/// same way they stub the favorites service instead of sharing
/// process-wide state. It needs the same `toShort()` normalization —
/// `getCryptoIdentitiesByPeerIDPrefix` guards on `isShort` and returns an
/// empty result otherwise, so a 64-hex id would read as "no identity"
/// rather than as an error. Synchronous and disk-backed, so it is safe on
/// the launch path.
static func isDirectChatRestorable(
_ peerID: PeerID,
favorites: FavoritesPersistenceService,
hasStoredCryptographicIdentity: (PeerID) -> Bool,
isPeerBlocked: (PeerID) -> Bool
) -> Bool {
isDirectChatRestorable(
peerID,
isPeerFavorited: {
favorites.getFavoriteStatus(forPeerID: $0.toShort())?.isFavorite ?? false
},
hasStoredCryptographicIdentity: hasStoredCryptographicIdentity,
isPeerBlocked: isPeerBlocked
)
}
/// Pure launch-effect decision, extracted so the fallback is unit-testable
/// without constructing `AppRuntime`: present the conversation list on a
/// first-ever launch, or when a persisted DM could not actually be opened
/// (blocked / stale / gated peer). A public-channel restore is left to
/// `GeoChannelCoordinator`.
static func shouldPresentConversationList(
for presentation: ConversationStore.LaunchPresentation,
didOpenDirectChat: Bool
) -> Bool {
switch presentation {
case .conversationList:
return true
case .restoredDirectChat:
return !didOpenDirectChat
case .deferToChannelRestore:
return false
}
}
/// Drops media that has outlived the retention window, then applies the
/// explicit protection class to files that older builds wrote without
/// one. Expiry runs first so the migration never touches files the

View File

@ -407,11 +407,14 @@ final class ConversationStore: ObservableObject {
/// What launch should present, decided purely from `restoredLastActive`.
enum LaunchPresentation: Equatable {
/// First-ever launch, or a persisted DM whose peer no longer resolves.
/// First-ever launch, or a persisted DM. Launch never re-opens the
/// DM itself: landing on a name and message history would make
/// startup a disclosure to anyone holding an unlocked phone, and the
/// old restorability rule selected favorites — the relationships worth
/// most to an observer. The list still gets #1064's ask (don't start
/// typing into a broadcast nobody chose), and #1598's recent-chats
/// section puts the conversation one tap away.
case conversationList
/// A valid persisted DM — the caller re-opens it via the normal
/// private-chat path (this axis never writes `activeChannel`).
case restoredDirectChat(PeerID)
/// Last-active was a public channel — defer to the existing mesh /
/// `GeoChannelCoordinator` restore; do nothing here.
case deferToChannelRestore
@ -483,18 +486,19 @@ final class ConversationStore: ObservableObject {
/// Decides what to present at launch from the value persisted last
/// session. Pure aside from reading the init snapshot: performs no
/// selection mutation and never writes `activeChannel`, so it cannot race
/// the location-channel restore. `isPeerResolvable` lets the caller reject
/// a stale/unaddressable DM peer, falling back to the conversation list.
func restoreLastActiveConversation(isPeerResolvable: (PeerID) -> Bool) -> LaunchPresentation {
/// the location-channel restore.
///
/// A persisted DM resolves to the conversation list, not to the DM, which
/// makes the peer's addressability irrelevant here — that is why no
/// resolvability predicate is threaded in. A restorable peer and a stale
/// one take the same branch, so there is nothing left for one to decide.
func restoreLastActiveConversation() -> LaunchPresentation {
guard let record = restoredLastActive else { return .conversationList }
switch record.kind {
case .mesh, .location:
return .deferToChannelRestore
case .direct:
guard let raw = record.peerID, !raw.isEmpty else { return .conversationList }
let peerID = PeerID(str: raw)
guard isPeerResolvable(peerID) else { return .conversationList }
return .restoredDirectChat(peerID)
return .conversationList
}
}

View File

@ -319,7 +319,7 @@ final class ChatPeerIdentityCoordinator {
}
@MainActor
func startPrivateChat(with peerID: PeerID, suppressSystemMessages: Bool = false) {
func startPrivateChat(with peerID: PeerID) {
guard peerID != context.myPeerID else { return }
// Group chats are virtual conversations: no peer identity, favorites,
@ -334,18 +334,16 @@ final class ChatPeerIdentityCoordinator {
let peerNickname = context.peerNickname(for: peerID) ?? "unknown"
if context.unifiedIsBlocked(peerID) {
if !suppressSystemMessages {
context.addSystemMessage(
String(
format: String(
localized: "system.chat.blocked",
comment: "System message when starting chat fails because peer is blocked"
),
locale: .current,
peerNickname
)
context.addSystemMessage(
String(
format: String(
localized: "system.chat.blocked",
comment: "System message when starting chat fails because peer is blocked"
),
locale: .current,
peerNickname
)
}
)
return
}

View File

@ -1498,28 +1498,12 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage
/// Initiates a private chat session with a peer.
/// - Parameter peerID: The peer's ID to start chatting with
/// - Note: Switches the UI to private chat mode and loads message history.
/// This zero-extra-argument signature is the witness for the
/// `CommandProcessor` / `ChatNostrCoordinator` context protocols, so it is
/// kept intact; the launch-restore variant is a separate overload below.
/// - Note: Switches the UI to private chat mode and loads message history
@MainActor
func startPrivateChat(with peerID: PeerID) {
peerIdentityCoordinator.startPrivateChat(with: peerID)
}
/// #1064 launch-restore variant of `startPrivateChat`. When
/// `suppressSystemMessages` is `true`, a gate rejection no-ops silently
/// instead of emitting a system message into the current (public mesh)
/// timeline, so a rejected DM restore falls back to the conversation list
/// cleanly. Since #1415 removed the mutual-favorite gate that is exactly
/// one message — the blocked one. Kept as a distinct non-defaulted
/// overload — a defaulted extra parameter would not satisfy the context
/// protocols above and a default would make the plain call ambiguous.
@MainActor
func startPrivateChat(with peerID: PeerID, suppressSystemMessages: Bool) {
peerIdentityCoordinator.startPrivateChat(with: peerID, suppressSystemMessages: suppressSystemMessages)
}
@MainActor
func endPrivateChat() {
peerIdentityCoordinator.endPrivateChat()

View File

@ -303,48 +303,6 @@ struct ChatPeerIdentityCoordinatorContextTests {
#expect(context.markedReadPeers.isEmpty)
}
@Test @MainActor
func startPrivateChat_suppressed_blockedPeerEmitsNoSystemMessage() async {
// #1064: at launch a now-blocked (e.g. favorited-then-blocked) peer must
// reject SILENTLY — no "blocked" line leaked into the public timeline —
// while still not opening the chat, so the caller falls back to the list.
let context = MockChatPeerIdentityContext()
let coordinator = ChatPeerIdentityCoordinator(context: context)
let peerID = PeerID(str: "1122334455667788")
context.blockedPeers = [peerID]
coordinator.startPrivateChat(with: peerID, suppressSystemMessages: true)
#expect(context.systemMessages.isEmpty)
#expect(context.begunChatSessions.isEmpty)
#expect(context.consolidatedPeers.isEmpty)
#expect(context.markedReadPeers.isEmpty)
}
@Test @MainActor
func startPrivateChat_suppressed_oneWayFavoriteOpensChatSilently() async {
// #1064 × #1415: the mutual-favorite gate is gone (store-and-forward
// handles offline non-mutual favorites), so a one-way favorite now
// opens the chat — and under suppression still emits no system message.
let context = MockChatPeerIdentityContext()
let coordinator = ChatPeerIdentityCoordinator(context: context)
let peerID = PeerID(str: "1122334455667788")
let noiseKey = Data((0..<32).map(UInt8.init))
var peer = BitchatPeer(peerID: peerID, noisePublicKey: noiseKey, nickname: "alice")
peer.favoriteStatus = makeFavoriteRelationship(
noiseKey: noiseKey,
isFavorite: true,
theyFavoritedUs: false
)
context.peersByID[peerID] = peer
coordinator.startPrivateChat(with: peerID, suppressSystemMessages: true)
#expect(context.systemMessages.isEmpty)
#expect(context.begunChatSessions == [peerID])
#expect(context.consolidatedPeers.map(\.peerID) == [peerID])
}
@Test @MainActor
func updatePrivateChatPeerIfNeeded_migratesChatStateByFingerprint() async {
let context = MockChatPeerIdentityContext()

View File

@ -4,9 +4,14 @@
//
// Tests for #1064 last-active persistence: ConversationStore records the
// foreground conversation on every switch and, at the next launch, decides
// what to present — a valid DM restores, a stale DM or a first-ever launch
// falls back to the conversation list, and a public channel defers to the
// existing GeoChannelCoordinator restore.
// what to present — a persisted DM or a first-ever launch presents the
// conversation list, and a public channel defers to the existing
// GeoChannelCoordinator restore.
//
// Launch never re-opens the DM itself. The tests below pin that as a
// property, not an implementation detail: it is what keeps startup from
// disclosing a contact's name and message history to whoever is holding
// the phone.
//
// This is free and unencumbered software released into the public domain.
// For more information, see <https://unlicense.org>
@ -25,42 +30,50 @@ final class ConversationStoreLastActiveTests: XCTestCase {
func test_persistsLastActiveOnEverySwitch() {
let storage = makeStorage()
// Open a DM: the next launch should restore it.
// Open a DM: the next launch presents the list.
let session1 = ConversationStore(storage: storage)
session1.setSelectedPrivatePeer(peerID)
XCTAssertEqual(
ConversationStore(storage: storage).restoreLastActiveConversation(isPeerResolvable: { _ in true }),
.restoredDirectChat(peerID)
ConversationStore(storage: storage).restoreLastActiveConversation(),
.conversationList
)
// Close the DM back to the mesh channel: the write must happen again,
// so the next launch now defers to the channel restore, not the DM.
// so the next launch now defers to the channel restore.
session1.setSelectedPrivatePeer(nil)
XCTAssertEqual(
ConversationStore(storage: storage).restoreLastActiveConversation(isPeerResolvable: { _ in true }),
ConversationStore(storage: storage).restoreLastActiveConversation(),
.deferToChannelRestore
)
}
func test_restoresValidDirectChat() {
func test_persistedDirectChatPresentsTheListAndNeverTheChat() {
// The security property this design turns on: whatever was persisted,
// a DM never re-opens itself at launch. There is deliberately no
// "restorable peer" branch to take — a favorite, a merely-known peer
// and a stale one all land on the list — so no addressability check
// can put a name and a history on screen before anyone asked.
let storage = makeStorage()
ConversationStore(storage: storage).setSelectedPrivatePeer(peerID)
let restored = ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { $0.isValid })
XCTAssertEqual(restored, .restoredDirectChat(peerID))
XCTAssertEqual(
ConversationStore(storage: storage).restoreLastActiveConversation(),
.conversationList
)
}
func test_staleDirectChatFallsBackToConversationList() {
func test_launchDefersToChannelRestore_forAPersistedPublicChannel() {
// A public-channel restore is owned by GeoChannelCoordinator; the
// launch decision must not present the list on top of it.
let storage = makeStorage()
ConversationStore(storage: storage).setSelectedPrivatePeer(peerID)
let session = ConversationStore(storage: storage)
session.setSelectedPrivatePeer(peerID)
session.setSelectedPrivatePeer(nil)
// The persisted peer no longer resolves at launch.
let restored = ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { _ in false })
XCTAssertEqual(restored, .conversationList)
XCTAssertEqual(
ConversationStore(storage: storage).restoreLastActiveConversation(),
.deferToChannelRestore
)
}
func test_failedDirectRestoreDoesNotEraseDirectRecord() {
@ -71,19 +84,19 @@ final class ConversationStoreLastActiveTests: XCTestCase {
ConversationStore(storage: storage).setSelectedPrivatePeer(peerID)
// Session 2 (launch): a fresh store starts with no private peer
// selected. The DM restore has just failed, and GeoChannelCoordinator
// re-asserts the SAME (default mesh) active channel. Because the channel
// does not actually change, the persist is now guarded out — so the
// `.direct` record on disk must survive rather than be overwritten with
// `.mesh`.
// selected, and GeoChannelCoordinator re-asserts the SAME (default
// mesh) active channel. Because the channel does not actually change,
// the persist is guarded out — so the `.direct` record on disk must
// survive rather than be overwritten with `.mesh`. The distinction
// still matters with the DM no longer re-opening: `.direct` presents
// the list, `.mesh` would defer to the channel restore instead.
let launch = ConversationStore(storage: storage)
launch.setActiveChannel(launch.activeChannel)
// Session 3: the DM record is intact and still restores.
// Session 3: the DM record is intact, so launch still presents the list.
XCTAssertEqual(
ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { _ in true }),
.restoredDirectChat(peerID)
ConversationStore(storage: storage).restoreLastActiveConversation(),
.conversationList
)
}
@ -92,13 +105,13 @@ final class ConversationStoreLastActiveTests: XCTestCase {
// wiped DM/channel cannot be restored on the next launch.
let storage = makeStorage()
// A DM is persisted and would otherwise restore.
// A channel record is persisted and would otherwise defer.
let session = ConversationStore(storage: storage)
session.setSelectedPrivatePeer(peerID)
session.setSelectedPrivatePeer(nil)
XCTAssertEqual(
ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { _ in true }),
.restoredDirectChat(peerID)
ConversationStore(storage: storage).restoreLastActiveConversation(),
.deferToChannelRestore
)
// The panic path erases the pointer through the store's own storage.
@ -106,8 +119,7 @@ final class ConversationStoreLastActiveTests: XCTestCase {
// The next launch has nothing to restore and falls back to the list.
XCTAssertEqual(
ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { _ in true }),
ConversationStore(storage: storage).restoreLastActiveConversation(),
.conversationList
)
}
@ -116,28 +128,20 @@ final class ConversationStoreLastActiveTests: XCTestCase {
// #1064 panic wipe, FULL ORDER: `clearPersistedLastActive()` alone is
// insufficient because `panicClearAllData()` also runs
// `selectedPrivateChatPeer = nil` and `activeChannel = .mesh` AFTER it —
// both route through the store's setters, whose guarded `persistLastActive()`
// re-writes a `.mesh` record on a real state change, resurrecting the
// pointer. The begin/finish suppression window fixes that. This test
// mirrors panicClearAllData's exact order against a single store:
// both route through the store's setters, whose guarded
// `persistLastActive()` re-writes a `.mesh` record on a real state
// change, resurrecting the pointer. The begin/finish suppression window
// fixes that. This test mirrors panicClearAllData's exact order:
// beginPanicWipe → setSelectedPrivatePeer(nil) → setActiveChannel(.mesh) → finishPanicWipe
// Without the fix (i.e. without begin/finish), those selection/channel
// resets re-persist a `.mesh` record — here `setSelectedPrivatePeer(nil)`
// is the real state change that writes it (the store's default channel
// is already `.mesh`, so `setActiveChannel(.mesh)` is the second trigger
// whenever panic runs from a non-mesh channel). Either way the pointer
// survives as `.mesh` and the next launch would `.deferToChannelRestore`
// instead of `.conversationList`.
// Without the fix those resets re-persist a `.mesh` record, and the next
// launch would `.deferToChannelRestore` instead of `.conversationList`.
// That is what makes this assertion load-bearing rather than trivially
// true: `.conversationList` is also the no-record answer, so the test
// distinguishes "pointer absent" from "pointer rewritten as .mesh".
let storage = makeStorage()
// The user is in a DM: a `.direct` record is persisted and would restore.
let session = ConversationStore(storage: storage)
session.setSelectedPrivatePeer(peerID)
XCTAssertEqual(
ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { _ in true }),
.restoredDirectChat(peerID)
)
// Reproduce panicClearAllData's order on the SAME store.
session.beginPanicWipe()
@ -148,8 +152,7 @@ final class ConversationStoreLastActiveTests: XCTestCase {
// A fresh store on the SAME storage finds NO record and falls back to
// the conversation list — the pointer is truly absent, not `.mesh`.
XCTAssertEqual(
ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { _ in true }),
ConversationStore(storage: storage).restoreLastActiveConversation(),
.conversationList
)
}
@ -157,442 +160,9 @@ final class ConversationStoreLastActiveTests: XCTestCase {
func test_firstLaunchPresentsConversationList() {
let storage = makeStorage()
let restored = ConversationStore(storage: storage)
.restoreLastActiveConversation(isPeerResolvable: { _ in true })
XCTAssertEqual(restored, .conversationList)
}
// MARK: - Launch effect (silent-mesh fallback)
func test_launchPresentsList_whenRestoredDirectChatDidNotOpen() {
// A persisted DM whose peer is now blocked/stale/gated: startPrivateChat
// no-ops, so no chat opens — must fall back to the conversation list,
// never silently land on the public mesh timeline.
XCTAssertTrue(
AppRuntime.shouldPresentConversationList(
for: .restoredDirectChat(peerID),
didOpenDirectChat: false
)
)
}
func test_launchDoesNotPresentList_whenRestoredDirectChatOpened() {
XCTAssertFalse(
AppRuntime.shouldPresentConversationList(
for: .restoredDirectChat(peerID),
didOpenDirectChat: true
)
)
}
func test_launchDefersToChannelRestore_withoutPresentingList() {
// A public-channel restore is owned by GeoChannelCoordinator; the
// launch decision must not present the list on top of it.
XCTAssertFalse(
AppRuntime.shouldPresentConversationList(
for: .deferToChannelRestore,
didOpenDirectChat: false
)
)
}
// MARK: - Restorability predicate (durable state, not syntax)
func test_unknownButSyntacticallyValidPeerIsNotRestorable() {
// Regression guard: a well-formed 16-hex peer id we have NO durable
// relationship with must NOT be treated as restorable. Otherwise it
// falls straight through startPrivateChat into an empty phantom DM.
// (The syntax-only `{ $0.isValid }` resolver missed exactly this.)
XCTAssertTrue(peerID.isValid)
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_mutualFavoritePeerIsRestorable() {
// A persisted MUTUAL favorite is stored by stable Noise public key and
// survives restart — the one durable, presence-independent mesh
// relationship. Mirrors the open-path gate, which only lets an offline
// favorite through when the favorite is mutual.
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in true },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_oneWayFavoritePeerIsRestorable() {
// INVERTED by #1415, deliberately kept as the record of that change.
// This used to be false: the open path required a mutual favorite, so
// restoring a one-way favorite would have injected a "requires
// favorite" system message into the public timeline. #1415 removed that
// gate — store-and-forward needs only the recipient's noise key — so a
// one-way favorite is now perfectly sendable, and refusing to restore
// it would make launch stricter than chat entry.
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in true },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_peerWhoOnlyFavoritedUsIsNotRestorable() {
// Remote state alone is not evidence we can address them. They favorited
// us, we never favorited them, and we hold no stored identity — the
// unaddressable phantom this predicate exists to refuse. Their opinion
// of us says nothing about whether we have a key for them.
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_geoChatIdIsNeverRestorableAsADirectChat() {
// A geohash channel id is not a direct chat at all, and the screen runs
// before the favorite term so nothing can match past it.
let geoChatID = PeerID(str: "nostr:01234567")
XCTAssertTrue(geoChatID.isGeoChat)
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
geoChatID,
isPeerFavorited: { _ in true },
hasStoredCryptographicIdentity: { _ in true },
isPeerBlocked: { _ in false }
)
)
}
func test_establishedNonFavoritePeerIsRestorable() {
// No favorite either way, but we hold a stored cryptographic identity —
// durable, on disk, and enough for the router to deliver via courier or
// the retained outbox. Restoring it is not a phantom.
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in true },
isPeerBlocked: { _ in false }
)
)
}
func test_blockedEstablishedPeerIsNotRestorable() {
// Blocked is a veto, not one term among several: a stored identity must
// not buy its way past the block the way it passes the favorite terms.
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in true },
isPeerBlocked: { _ in true }
)
)
}
func test_geoDMWithStoredIdentityIsNotRestorable() {
// The identity term must NOT extend to geohash DMs. A `nostr_` id's
// full Nostr key is rebuilt only from inbound ephemeral events, and
// startPrivateChat skips the handshake for geoDMs, so a phantom would
// open with no error at all. Today the lookup would miss anyway (the
// id's prefix is re-attached and no hex fingerprint starts with it),
// which is luck — this pins the refusal so a change to that lookup
// cannot quietly turn geoDM phantoms back on.
let geoDMPeer = PeerID(str: "nostr_0123456789abcdef")
XCTAssertTrue(geoDMPeer.isGeoDM)
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
geoDMPeer,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in true },
isPeerBlocked: { _ in false }
)
)
}
func test_geoDMWithFavoriteIsRestorable() {
// A favorite record IS a geoDM's durable anchor (it carries
// peerNostrPublicKey), so the favorite terms still admit one.
let geoDMPeer = PeerID(str: "nostr_0123456789abcdef")
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
geoDMPeer,
isPeerFavorited: { _ in true },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_privateGroupIsRestorable() {
// A group id is `group_` plus 32 hex, so both peer lookups guard on
// `isShort` and return empty for it. Left to those terms a group would
// never restore, silently, every time — even though `startPrivateChat`
// gates group re-entry on nothing at all.
let group = PeerID(str: "group_" + String(repeating: "ab", count: 16))
XCTAssertTrue(group.isGroup, "test fixture is not a group id")
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
group,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_privateGroupNeverConsultsThePeerTerms() {
// Admitting groups by accident — because some peer term happened to
// match — would be a different bug wearing the same green check. The
// group must be admitted on its own branch, before either lookup runs.
let group = PeerID(str: "group_" + String(repeating: "cd", count: 16))
var favoriteLookups = 0
var identityLookups = 0
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
group,
isPeerFavorited: { _ in favoriteLookups += 1; return false },
hasStoredCryptographicIdentity: { _ in identityLookups += 1; return false },
isPeerBlocked: { _ in false }
)
)
XCTAssertEqual(favoriteLookups, 0, "group restore consulted the favorites term")
XCTAssertEqual(identityLookups, 0, "group restore consulted the identity term")
}
func test_idClassesAreMutuallyExclusive() {
// The group branch sits between the geoChat guard and the geoDM one,
// so its placement would be load-bearing if an id could belong to two
// classes at once. `PeerID` assigns exactly one prefix, so it cannot —
// pin that, because the day it stops being true the ordering silently
// decides which rule wins.
let group = PeerID(str: "group_" + String(repeating: "ab", count: 16))
let geoDM = PeerID(str: "nostr_0123456789abcdef")
let geoChat = PeerID(str: "nostr:someGeohashChannel")
XCTAssertTrue(group.isGroup)
XCTAssertFalse(group.isGeoDM)
XCTAssertFalse(group.isGeoChat)
XCTAssertFalse(geoDM.isGroup)
XCTAssertFalse(geoChat.isGroup)
}
func test_malformedGroupIDIsStillAdmitted_documentingTheBound() {
// `isGroup` tests the prefix only — `PeerID(str:)` never validates the
// bare — so this branch admits any persisted id claiming to be a
// group. That is deliberate and bounded: the value is written by our
// own selection path into local state, never parsed from the network,
// and the failure it can produce is an empty group rather than the
// phantom DM this predicate exists to prevent. Pinned so that if the
// id ever becomes untrusted, this test is the thing that has to change.
let malformed = PeerID(str: "group_not-hex")
XCTAssertTrue(malformed.isGroup)
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
malformed,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_blockedGroupIsNotRestorable() {
// Block stays an unconditional veto, ahead of the group branch.
let group = PeerID(str: "group_" + String(repeating: "ef", count: 16))
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
group,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in true }
)
)
}
func test_blockedMutualFavoriteIsNotRestorable() {
// A blocked peer is never restorable, even if the favorite is mutual —
// mirrors the gate's first (block) reject.
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in true },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in true }
)
)
}
func test_geoDMPeerWithoutMutualFavoriteIsNotRestorable() {
// #1064 phantom-DM fix: a geohash/Nostr DM id is NO LONGER special-cased
// as restorable. Its full Nostr key is rebuilt only from inbound
// ephemeral events, so at launch a restored `nostr_` id cannot resolve
// and would open an unsendable phantom. Only a mutual favorite restores.
let geoDMPeer = PeerID(str: "nostr_0123456789abcdef")
XCTAssertTrue(geoDMPeer.isGeoDM)
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
geoDMPeer,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_restoreWithProductionShapedResolver_unknownPeerYieldsConversationList() {
// End-to-end through ConversationStore using the PRODUCTION predicate
// shape (not a bare `{ _ in false }`): a persisted DM whose peer is
// unknown/unfavorited must present the conversation list, never restore
// a phantom DM. This is the test that would have caught the hole.
let storage = makeStorage()
ConversationStore(storage: storage).setSelectedPrivatePeer(peerID)
let restored = ConversationStore(storage: storage).restoreLastActiveConversation(
isPeerResolvable: {
AppRuntime.isDirectChatRestorable(
$0,
isPeerFavorited: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
}
)
XCTAssertEqual(restored, .conversationList)
}
// MARK: - Production wiring (real FavoritesPersistenceService)
func test_production_fullHexMutualFavoritePeerIsRestorable() {
// migrateSelectedConversationIfNeeded persists the peer in FULL 64-hex
// Noise-key form; the favorites store is keyed by the short derived id.
// The production resolver must normalize (`toShort()`) so a favorited DM
// still restores. Regression for fix-round-3 finding 1. The favorite must
// be MUTUAL to mirror the open-path gate.
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
let noiseKey = Data((0..<32).map(UInt8.init))
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true)
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
XCTAssertFalse(fullHexPeer.isShort) // 64-hex, not the short form
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_production_oneWayFavoriteIsRestorable() {
// INVERTED by #1415, through the real favorites wiring. This used to
// refuse a non-mutual favorite because the open-path gate would have
// rejected it at launch; that gate is gone, so refusing here would make
// launch stricter than chat entry for a DM the router can deliver.
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
let noiseKey = Data((0..<32).map(UInt8.init))
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite)
XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs)
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_production_blockedMutualFavoriteIsNotRestorable() {
// A mutual favorite we have since blocked must NOT restore — mirrors the
// gate's block reject. Block state is injected (it lives in the identity
// manager, not the favorites store).
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
let noiseKey = Data((0..<32).map(UInt8.init))
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true)
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in true }
)
)
}
func test_production_unknownShortPeerIsNotRestorable() {
// A syntactically valid short peer with no favorite relationship must
// NOT restore (would otherwise open an empty phantom DM). Pins the real
// wiring so it cannot drift back to a syntax-only check.
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
XCTAssertTrue(peerID.isValid)
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
peerID,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_production_unfavoritedPeerWhoStillFavoritesUsIsNotRestorable() {
// removeFavorite RETAINS a record (isFavorite: false, theyFavoritedUs:
// true) when the peer still favorites us. The resolver must key on
// isFavorite, not mere record existence — otherwise a DM to a peer we
// deliberately unfavorited reopens on restart. Regression for an
// earlier Codex review, and it SURVIVES the #1415 relaxation: the
// relaxed predicate admits our own favorite or a stored identity, and
// `theyFavoritedUs` is deliberately not a term, so the unfavorite still
// stands on its own.
//
// With a stored identity this peer would restore — see
// test_establishedNonFavoritePeerIsRestorable. That is the intended
// line: the durable evidence is then local capability rather than the
// other side's opinion of us.
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
let noiseKey = Data((0..<32).map(UInt8.init))
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
favorites.updatePeerFavoritedUs(peerNoisePublicKey: noiseKey, favorited: true)
favorites.removeFavorite(peerNoisePublicKey: noiseKey)
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
// The record survives (they still favorite us) but isFavorite is false.
XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort()))
XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite)
XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs)
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
XCTAssertEqual(
ConversationStore(storage: storage).restoreLastActiveConversation(),
.conversationList
)
}