Relax launch restore to match chat entry after #1415

#1415 removed the mutual-favorite gate from startPrivateChat — store-and-forward
needs only the recipient's noise key, so "the router decides what delivery looks
like, not chat entry". That left this branch's launch predicate stricter than
chat entry, and its doc comment claiming to mirror a gate that no longer exists.
A one-way-favorite DM that the outbox or a courier would deliver happily failed
to restore and dropped the user on the conversation list.

Restorable is now: not blocked, AND either side has favorited the other, OR we
hold a stored cryptographic identity for them.

Blocked stays a veto rather than becoming one term among several, and geohash
DMs are refused the identity term outright. That refusal is written down rather
than relied on: a `nostr_` id does satisfy `isShort`, since the prefix is not
part of the length check, and today the lookup misses only because the id's
prefix is re-attached and no hex fingerprint starts with it. Luck, not a rule —
and the cost of it changing is a phantom geoDM that opens with no error at all,
because startPrivateChat skips the handshake for them.

The outbox and live Noise session state are deliberately not consulted. Both
read empty at launch whatever the truth — the outbox defers loading until
protected data is available, session state is in-memory — so a predicate built
on them would refuse to restore the very conversations it is meant to admit.

The identity lookup is injected through the existing
SecureIdentityStateManagerProtocol rather than reached for as a singleton, so
these tests stub it the way they already stub favorites instead of sharing
process-wide state with the rest of the suite.

Three doc comments cited the removed gate and are rewritten, including the one
claiming startPrivateChat is a second line of defence here. It is not any more:
post-#1415 it screens only self, group and blocked, so this predicate stands
alone and is written to hold on its own.

Tests invert rather than disappear — they are the record of what changed.
Mutation-verified: dropping the geoDM guard fails only the geoDM case, dropping
the identity term fails only the established-peer case, and demoting the block
veto fails four.

One behaviour change deserves a second opinion, called out on the PR: a peer we
unfavorited who still favorites us now restores, reversing a guard added for an
earlier review. Its premise was the "is a persisted favorite" contract, which
#1415 dissolved.

Also adds Persian for the two caption keys, which main's fa localization landed
after this branch wrote them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ecgang 2026-07-26 12:14:32 -07:00
parent 3ffcaf701c
commit baacc6a85b
4 changed files with 219 additions and 37 deletions

View File

@ -171,6 +171,11 @@ final class AppRuntime: ObservableObject {
Self.isDirectChatRestorable(
$0,
favorites: .shared,
hasStoredCryptographicIdentity: {
!chatViewModel.identityManager
.getCryptoIdentitiesByPeerIDPrefix($0.toShort())
.isEmpty
},
isPeerBlocked: { chatViewModel.isPeerBlocked($0) }
)
}
@ -178,13 +183,18 @@ final class AppRuntime: ObservableObject {
var didOpenDirectChat = false
if case .restoredDirectChat(let peerID) = presentation {
// `startPrivateChat`'s gate (ChatPeerIdentityCoordinator) rejects a
// now-blocked or non-mutual-favorite 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
// 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. `isDirectChatRestorable` already screens for the
// same conditions; this is the belt-and-suspenders second line.
// 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
}
@ -201,24 +211,54 @@ final class AppRuntime: ObservableObject {
/// 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. Mirrors the open-path gate
/// (`ChatPeerIdentityCoordinator.startPrivateChat`): restorable iff the peer
/// is a MUTUAL favorite (we favorite them AND they favorite us) and NOT
/// blocked. The gate's third relaxation term, `isConnected`, is always false
/// at launch, so it drops out of the launch-effective predicate. A geohash/
/// Nostr DM is *not* special-cased: 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 unless it is also a mutual
/// favorite. Favorites are keychain-backed and keyed by stable Noise public
/// key, so this is presence-independent and pure, hence unit-testable.
/// DM.
///
/// Restorable iff the peer is NOT blocked and we hold durable evidence the
/// conversation is real: either side has favorited the other, or we have a
/// stored cryptographic identity for them.
///
/// 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.
///
/// Geohash/Nostr DMs stay excluded: a `nostr_` id'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. Their one durable anchor is a
/// favorite record, so for them the favorite terms decide and the identity
/// term is refused outright.
static func isDirectChatRestorable(
_ peerID: PeerID,
isPeerFavorited: (PeerID) -> Bool,
theyFavoritedUs: (PeerID) -> Bool,
hasStoredCryptographicIdentity: (PeerID) -> Bool,
isPeerBlocked: (PeerID) -> Bool
) -> Bool {
// Blocked stays a veto, never one term among several.
guard !isPeerBlocked(peerID) else { return false }
return isPeerFavorited(peerID) && theyFavoritedUs(peerID)
if isPeerFavorited(peerID) || theyFavoritedUs(peerID) { return true }
// Explicit rather than incidental: a `nostr_` id does satisfy
// `isShort` (the prefix is not part of the length check), and today it
// misses only because the identity lookup re-attaches that prefix and
// no hex fingerprint starts with it. That is luck, not a rule.
guard !peerID.isGeoDM, !peerID.isGeoChat else { return false }
return hasStoredCryptographicIdentity(peerID)
}
/// Production wiring of `isDirectChatRestorable`, extracted so the real
@ -231,9 +271,19 @@ final class AppRuntime: ObservableObject {
/// 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(
@ -244,6 +294,7 @@ final class AppRuntime: ObservableObject {
theyFavoritedUs: {
favorites.getFavoriteStatus(forPeerID: $0.toShort())?.theyFavoritedUs ?? false
},
hasStoredCryptographicIdentity: hasStoredCryptographicIdentity,
isPeerBlocked: isPeerBlocked
)
}

View File

@ -37946,6 +37946,12 @@
"value" : "público · cerca"
}
},
"fa" : {
"stringUnit" : {
"state" : "translated",
"value" : "عمومی · اطراف شما"
}
},
"fil" : {
"stringUnit" : {
"state" : "translated",
@ -38126,6 +38132,12 @@
"value" : "público, cerca"
}
},
"fa" : {
"stringUnit" : {
"state" : "translated",
"value" : "عمومی، اطراف شما"
}
},
"fil" : {
"stringUnit" : {
"state" : "translated",

View File

@ -1502,10 +1502,11 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage
}
/// #1064 launch-restore variant of `startPrivateChat`. When
/// `suppressSystemMessages` is `true`, a gate rejection (blocked /
/// non-mutual favorite) 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. Kept as a distinct non-defaulted
/// `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

View File

@ -210,6 +210,7 @@ final class ConversationStoreLastActiveTests: XCTestCase {
peerID,
isPeerFavorited: { _ in false },
theyFavoritedUs: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
@ -225,21 +226,104 @@ final class ConversationStoreLastActiveTests: XCTestCase {
peerID,
isPeerFavorited: { _ in true },
theyFavoritedUs: { _ in true },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_oneWayFavoritePeerIsNotRestorable() {
// We favorite them but they do NOT favorite us: the open-path gate
// rejects this at launch (isConnected is false), so auto-restoring it
// would inject a "requires favorite" system message into the public
// timeline. The predicate must refuse it up front.
XCTAssertFalse(
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 },
theyFavoritedUs: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_peerWhoFavoritedUsIsRestorable() {
// The other direction of the same relaxation.
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
peerID,
isPeerFavorited: { _ in false },
theyFavoritedUs: { _ in true },
hasStoredCryptographicIdentity: { _ in false },
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 },
theyFavoritedUs: { _ 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 },
theyFavoritedUs: { _ 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 },
theyFavoritedUs: { _ 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 },
theyFavoritedUs: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
@ -253,6 +337,7 @@ final class ConversationStoreLastActiveTests: XCTestCase {
peerID,
isPeerFavorited: { _ in true },
theyFavoritedUs: { _ in true },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in true }
)
)
@ -270,6 +355,7 @@ final class ConversationStoreLastActiveTests: XCTestCase {
geoDMPeer,
isPeerFavorited: { _ in false },
theyFavoritedUs: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
@ -289,6 +375,7 @@ final class ConversationStoreLastActiveTests: XCTestCase {
$0,
isPeerFavorited: { _ in false },
theyFavoritedUs: { _ in false },
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
}
@ -316,15 +403,17 @@ final class ConversationStoreLastActiveTests: XCTestCase {
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_production_oneWayFavoriteIsNotRestorable() {
// We favorite them but they never favorited us: not mutual, so the
// open-path gate would reject it at launch. The production resolver must
// refuse it rather than auto-open a gated DM.
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")
@ -332,10 +421,11 @@ final class ConversationStoreLastActiveTests: XCTestCase {
let fullHexPeer = PeerID(str: noiseKey.hexEncodedString())
XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite)
XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs)
XCTAssertFalse(
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
@ -355,6 +445,7 @@ final class ConversationStoreLastActiveTests: XCTestCase {
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in true }
)
)
@ -370,17 +461,31 @@ final class ConversationStoreLastActiveTests: XCTestCase {
AppRuntime.isDirectChatRestorable(
peerID,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
}
func test_production_unfavoritedPeerWhoStillFavoritesUsIsNotRestorable() {
func test_production_unfavoritedPeerWhoStillFavoritesUsIsRestorable() {
// INVERTED by #1415, and the inversion worth arguing about.
//
// 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, contradicting the
// "is a persisted favorite" contract. Regression for Codex P2 review.
// true) when the peer still favorites us. This previously refused to
// restore, so that a DM to a peer we deliberately unfavorited would not
// reopen on restart the "is a persisted favorite" contract, added for
// an earlier Codex review.
//
// #1415 dissolved that contract: chat entry no longer consults
// favorites at all, so this conversation is openable by hand and
// sendable through the router. Refusing to restore it would single out
// launch for a stricter rule than every other way in.
//
// Note the protection is largely moot in practice regardless: any peer
// we have actually corresponded with has a stored cryptographic
// identity, which admits them through the identity term whatever the
// favorite record says. Unfavoriting is not blocking blocking still
// refuses, and that is the control for "do not reopen this".
let favorites = FavoritesPersistenceService(keychain: MockKeychain())
let noiseKey = Data((0..<32).map(UInt8.init))
favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice")
@ -391,11 +496,24 @@ final class ConversationStoreLastActiveTests: XCTestCase {
// 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)
XCTAssertTrue(
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in false }
)
)
// Blocking is the control that still refuses, with everything else
// about this peer unchanged.
XCTAssertFalse(
AppRuntime.isDirectChatRestorable(
fullHexPeer,
favorites: favorites,
isPeerBlocked: { _ in false }
hasStoredCryptographicIdentity: { _ in false },
isPeerBlocked: { _ in true }
)
)
}