Skip to content

Commit dfb9914

Browse files
yewreekaclaude
andcommitted
fix: propagate metadata clears to receivers; thread inbox id through scoped store
Macroscope re-review round two. The High: clearing the last scoped key produced a ProfileUpdate with no metadata field, which post-cutover receivers treated as "no change" - a revoked grant survived forever on the receiver. The wire cannot distinguish an empty map from an absent one (proto3 map), but every wire client re-sends its full map on each update - the legacy receivers replaced the stored map wholesale and the CLI/agents were built against that (serve.ts merges existing metadata into every send). The unified-profile cutover silently changed this to keep-on-absent. Restore replace-including-clear semantics for the addressed ProfileUpdate paths (stream, NSE, history replay): a winning update's non-nil map replaces the stored one and an empty map clears it, while nil (snapshot/appData fill paths) still says nothing. The recency guard keeps an older replay from clearing newer data. The Medium: the scoped-metadata store resolved the self inbox id through its own provider, so a transiently nil answer mid-drain could silently drop a conversation's scoped keys from a queued send. The scoped store methods now take the caller's already-resolved inbox id explicitly, matching the ProfileStore style. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0f0fe33 commit dfb9914

11 files changed

Lines changed: 177 additions & 46 deletions

ConvosCore/Sources/ConvosCore/Inboxes/MessagingService+PushNotifications.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,9 @@ extension MessagingService {
961961
name: update.hasName ? update.name : nil,
962962
avatar: .addressed(update.hasEncryptedImage ? update.encryptedImage : nil),
963963
memberKind: update.memberKind.dbMemberKind,
964-
metadata: metadata.isEmpty ? nil : metadata,
964+
// Authoritative whole map; empty propagates as a clear
965+
// (matches the stream path).
966+
metadata: metadata,
965967
receivedAt: receivedAt
966968
),
967969
selfInboxId: currentInboxId,

ConvosCore/Sources/ConvosCore/Profiles/Repository/ProfileInboundApplier.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ enum ProfileInboundApplier {
2626

2727
/// One inbound identity + avatar event for a member, before merge. Bundled so
2828
/// the seam's call sites (and `apply`) stay under the parameter-count limit.
29+
///
30+
/// `metadata` semantics: non-nil is the sender's authoritative whole map -
31+
/// a winning event replaces the stored one, and an empty map clears it
32+
/// (ProfileUpdate paths pass the decoded map through untouched). Nil means
33+
/// the event says nothing about metadata (snapshot/fill paths collapse an
34+
/// empty map to nil so they can never clear).
2935
struct Incoming {
3036
let inboxId: String
3137
let source: ProfileSource

ConvosCore/Sources/ConvosCore/Profiles/Repository/ProfileMerge.swift

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import Foundation
88
/// - Guards: an empty/absent name never clears a populated one, a verified
99
/// assistant kind is never downgraded to generic `agent`, and the avatar is
1010
/// tri-state (only `set`/`explicitClear` change a slot; `silent` leaves it).
11+
/// - Metadata: a non-nil incoming map is the sender's authoritative whole map -
12+
/// a winning event replaces the stored one, and an empty map clears it
13+
/// (revoked grants must propagate; every wire client re-sends its full map on
14+
/// each update, matching the legacy replace-wholesale receivers). Nil means
15+
/// the event says nothing about metadata and the stored map is kept.
1116
enum ProfileMerge {
1217
/// Trimmed, non-empty name, or nil. A name that is nil/blank/whitespace is
1318
/// treated as "no name provided".
@@ -44,7 +49,7 @@ enum ProfileMerge {
4449
inboxId: inboxId,
4550
name: nonBlank(incoming.name),
4651
memberKind: incoming.memberKind,
47-
metadata: incoming.metadata,
52+
metadata: nonEmpty(incoming.metadata),
4853
profileSource: source,
4954
updatedAt: sentAt
5055
)
@@ -54,19 +59,30 @@ enum ProfileMerge {
5459
if winsOver(existingSource: existing.profileSource, existingUpdatedAt: existing.updatedAt, source: source, sentAt: sentAt) {
5560
result.name = nonBlank(incoming.name) ?? existing.name
5661
result.memberKind = preserveVerifiedKind(existing.memberKind, incoming.memberKind)
57-
result.metadata = incoming.metadata ?? existing.metadata
62+
// A winning event's non-nil map replaces the stored one wholesale;
63+
// empty clears it (a revoked grant must not survive as stale
64+
// metadata). Nil says nothing and keeps the stored map.
65+
if let metadata = incoming.metadata {
66+
result.metadata = metadata.isEmpty ? nil : metadata
67+
}
5868
result.profileSource = source
5969
result.updatedAt = sentAt
6070
} else {
6171
// Lower precedence or older: fill blanks only; keep provenance and
6272
// never let a low-priority event change an existing kind.
6373
result.name = existing.name ?? nonBlank(incoming.name)
6474
result.memberKind = existing.memberKind ?? incoming.memberKind
65-
result.metadata = existing.metadata ?? incoming.metadata
75+
result.metadata = existing.metadata ?? nonEmpty(incoming.metadata)
6676
}
6777
return result
6878
}
6979

80+
/// Non-empty map, or nil. An empty map only means something (an explicit
81+
/// clear) to a winning event; blank-fill and fresh rows treat it as absent.
82+
private static func nonEmpty(_ metadata: ProfileMetadata?) -> ProfileMetadata? {
83+
metadata.flatMap { $0.isEmpty ? nil : $0 }
84+
}
85+
7086
static func mergeAvatar(
7187
existing: DBProfileAvatar?,
7288
inboxId: String,

ConvosCore/Sources/ConvosCore/Profiles/Repository/ProfilePublisher.swift

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ actor ProfilePublisher {
215215
}
216216
let published = PublishedAvatar(url: url, salt: salt, nonce: nonce, key: key)
217217
let selfProfile = try await selfProfileStore.load()
218-
let metadata = try await outgoingMetadata(for: job.conversationId, selfProfile: selfProfile)
218+
let metadata = try await outgoingMetadata(for: job.conversationId, selfInboxId: selfInboxId, selfProfile: selfProfile)
219219
try await session.sendProfileUpdate(name: selfProfile?.name, metadata: metadata, avatar: published, conversationId: job.conversationId)
220220
let slot = DBProfileAvatar(
221221
inboxId: selfInboxId,
@@ -235,7 +235,7 @@ actor ProfilePublisher {
235235
let existing = try await profileStore.avatar(inboxId: selfInboxId, conversationId: job.conversationId)
236236
let published = publishedAvatar(from: existing)
237237
let selfProfile = try await selfProfileStore.load()
238-
let metadata = try await outgoingMetadata(for: job.conversationId, selfProfile: selfProfile)
238+
let metadata = try await outgoingMetadata(for: job.conversationId, selfInboxId: selfInboxId, selfProfile: selfProfile)
239239
try await session.sendProfileUpdate(name: selfProfile?.name, metadata: metadata, avatar: published, conversationId: job.conversationId)
240240
await stampPublished(selfProfile, conversationId: job.conversationId)
241241
}
@@ -277,16 +277,18 @@ actor ProfilePublisher {
277277
avatar: publishedAvatar(from: slot),
278278
conversationId: conversationId
279279
)
280-
try await selfProfileStore.saveScopedMetadata(metadata, conversationId: conversationId, updatedAt: now())
280+
try await selfProfileStore.saveScopedMetadata(metadata, inboxId: selfInboxId, conversationId: conversationId, updatedAt: now())
281281
}
282282

283283
/// The metadata map for an outgoing ProfileUpdate to one conversation: the
284284
/// global self metadata with that conversation's scoped keys merged over it
285285
/// (scoped wins). Keeps every queue send carrying the conversation's grants
286286
/// and timezone, so a later name or avatar publish can never wipe them at
287-
/// the receiver.
288-
private func outgoingMetadata(for conversationId: String, selfProfile: DBMyProfile?) async throws -> ProfileMetadata? {
289-
let scoped = try await selfProfileStore.scopedMetadata(conversationId: conversationId)
287+
/// the receiver. The caller's already-resolved inbox id is threaded through
288+
/// so the store read cannot disagree with the publish about the active
289+
/// account mid-flight.
290+
private func outgoingMetadata(for conversationId: String, selfInboxId: String, selfProfile: DBMyProfile?) async throws -> ProfileMetadata? {
291+
let scoped = try await selfProfileStore.scopedMetadata(inboxId: selfInboxId, conversationId: conversationId)
290292
return Self.mergedMetadata(global: selfProfile?.metadata, scoped: scoped)
291293
}
292294

ConvosCore/Sources/ConvosCore/Profiles/Repository/Stores/SelfProfileStore.swift

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@ protocol SelfProfileStoreProtocol: Sendable {
1414
/// The current user's conversation-scoped metadata map (cloud connection
1515
/// grants, agent timezone) for one conversation, or nil when none was ever
1616
/// published there. Scoped keys are merged over the global metadata at send
17-
/// time; they never live in `DBMyProfile.metadata`.
18-
func scopedMetadata(conversationId: String) async throws -> ProfileMetadata?
17+
/// time; they never live in `DBMyProfile.metadata`. Takes the inbox id
18+
/// explicitly (unlike `load`) so a publish that already resolved the self
19+
/// inbox cannot silently read a different account's rows if the provider's
20+
/// answer changes mid-flight.
21+
func scopedMetadata(inboxId: String, conversationId: String) async throws -> ProfileMetadata?
1922

2023
/// Persists the conversation-scoped metadata map for one conversation.
2124
/// A nil or empty map deletes the row.
22-
func saveScopedMetadata(_ metadata: ProfileMetadata?, conversationId: String, updatedAt: Date) async throws
25+
func saveScopedMetadata(_ metadata: ProfileMetadata?, inboxId: String, conversationId: String, updatedAt: Date) async throws
2326
}
2427

2528
final class GRDBSelfProfileStore: SelfProfileStoreProtocol {
@@ -75,18 +78,16 @@ final class GRDBSelfProfileStore: SelfProfileStoreProtocol {
7578
}
7679
}
7780

78-
func scopedMetadata(conversationId: String) async throws -> ProfileMetadata? {
79-
guard let inboxId = await selfInboxIdProvider() else { return nil }
80-
return try await databaseReader.read { db in
81+
func scopedMetadata(inboxId: String, conversationId: String) async throws -> ProfileMetadata? {
82+
try await databaseReader.read { db in
8183
try DBSelfConversationMetadata
8284
.filter(DBSelfConversationMetadata.Columns.inboxId == inboxId)
8385
.filter(DBSelfConversationMetadata.Columns.conversationId == conversationId)
8486
.fetchOne(db)?.metadata
8587
}
8688
}
8789

88-
func saveScopedMetadata(_ metadata: ProfileMetadata?, conversationId: String, updatedAt: Date) async throws {
89-
guard let inboxId = await selfInboxIdProvider() else { return }
90+
func saveScopedMetadata(_ metadata: ProfileMetadata?, inboxId: String, conversationId: String, updatedAt: Date) async throws {
9091
try await databaseWriter.write { db in
9192
guard let metadata, !metadata.isEmpty else {
9293
_ = try DBSelfConversationMetadata
@@ -107,7 +108,7 @@ final class GRDBSelfProfileStore: SelfProfileStoreProtocol {
107108

108109
actor InMemorySelfProfileStore: SelfProfileStoreProtocol {
109110
private var current: DBMyProfile?
110-
private var scopedByConversation: [String: ProfileMetadata] = [:]
111+
private var scopedByInbox: [String: [String: ProfileMetadata]] = [:]
111112

112113
func save(_ profile: DBMyProfile) {
113114
current = profile
@@ -119,18 +120,18 @@ actor InMemorySelfProfileStore: SelfProfileStoreProtocol {
119120

120121
func clear() {
121122
current = nil
122-
scopedByConversation = [:]
123+
scopedByInbox = [:]
123124
}
124125

125-
func scopedMetadata(conversationId: String) -> ProfileMetadata? {
126-
scopedByConversation[conversationId]
126+
func scopedMetadata(inboxId: String, conversationId: String) -> ProfileMetadata? {
127+
scopedByInbox[inboxId]?[conversationId]
127128
}
128129

129-
func saveScopedMetadata(_ metadata: ProfileMetadata?, conversationId: String, updatedAt: Date) {
130+
func saveScopedMetadata(_ metadata: ProfileMetadata?, inboxId: String, conversationId: String, updatedAt: Date) {
130131
guard let metadata, !metadata.isEmpty else {
131-
scopedByConversation[conversationId] = nil
132+
scopedByInbox[inboxId]?[conversationId] = nil
132133
return
133134
}
134-
scopedByConversation[conversationId] = metadata
135+
scopedByInbox[inboxId, default: [:]][conversationId] = metadata
135136
}
136137
}

ConvosCore/Sources/ConvosCore/Storage/Writers/ConversationWriter.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1204,7 +1204,10 @@ class ConversationWriter: ConversationWriterProtocol, @unchecked Sendable {
12041204
name: entry.update.hasName ? entry.update.name : nil,
12051205
avatar: .fillIfPresent(entry.update.hasEncryptedImage ? entry.update.encryptedImage : nil),
12061206
memberKind: entry.update.memberKind.dbMemberKind,
1207-
metadata: metadata.isEmpty ? nil : metadata,
1207+
// Authoritative whole map from the newest replayed
1208+
// update; empty propagates as a clear. The recency
1209+
// guard keeps an older replay from beating live data.
1210+
metadata: metadata,
12081211
receivedAt: entry.sentAt
12091212
),
12101213
selfInboxId: selfInboxId,

ConvosCore/Sources/ConvosCore/Syncing/StreamProcessor.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,10 @@ actor StreamProcessor: StreamProcessorProtocol {
499499
name: update.hasName ? update.name : nil,
500500
avatar: .addressed(update.hasEncryptedImage ? update.encryptedImage : nil),
501501
memberKind: update.memberKind.dbMemberKind,
502-
metadata: metadata.isEmpty ? nil : metadata,
502+
// A ProfileUpdate's map is authoritative; pass it through
503+
// even when empty so a cleared map (e.g. revoked grants)
504+
// propagates as a clear instead of reading as "no change".
505+
metadata: metadata,
503506
receivedAt: receivedAt
504507
),
505508
selfInboxId: currentInboxId,

ConvosCore/Tests/ConvosCoreTests/ProfileInboundApplierTests.swift

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ struct ProfileInboundApplierTests {
6666
source: ProfileSource = .profileUpdate,
6767
name: String?,
6868
avatar: ProfileInboundApplier.AvatarDisposition,
69+
metadata: ProfileMetadata? = nil,
6970
selfInboxId: String? = "me",
7071
fallbackKey: Data? = nil,
7172
sentAt: Date
@@ -80,7 +81,7 @@ struct ProfileInboundApplierTests {
8081
name: name,
8182
avatar: avatar,
8283
memberKind: nil,
83-
metadata: nil,
84+
metadata: metadata,
8485
receivedAt: sentAt
8586
),
8687
selfInboxId: selfInboxId,
@@ -208,4 +209,44 @@ struct ProfileInboundApplierTests {
208209
let contact = try queue.read { db in try DBContact.fetchOne(db, key: "bob") }
209210
#expect(contact == nil)
210211
}
212+
213+
@Test("a newer update's empty metadata map clears the stored metadata (revoked grants propagate)")
214+
func updateEmptyMetadataClears() throws {
215+
let queue = try makeQueue()
216+
try apply(queue, inboxId: "alice", name: "Alice", avatar: .addressed(nil), metadata: ["connections": .string("grants")], sentAt: Date(timeIntervalSince1970: 1))
217+
let seeded = try profile(queue, inboxId: "alice")
218+
#expect(seeded?.metadata?["connections"] == .string("grants"))
219+
220+
// The sender revoked their last grant: the update's map is empty.
221+
try apply(queue, inboxId: "alice", name: "Alice", avatar: .addressed(nil), metadata: [:], sentAt: Date(timeIntervalSince1970: 2))
222+
223+
let cleared = try profile(queue, inboxId: "alice")
224+
#expect(cleared?.metadata == nil)
225+
}
226+
227+
@Test("a newer update's non-empty metadata map replaces the stored one wholesale")
228+
func updateMetadataReplacesWholesale() throws {
229+
let queue = try makeQueue()
230+
try apply(queue, inboxId: "alice", name: "Alice", avatar: .addressed(nil), metadata: ["connections": .string("grants")], sentAt: Date(timeIntervalSince1970: 1))
231+
try apply(queue, inboxId: "alice", name: "Alice", avatar: .addressed(nil), metadata: ["timezone": .string("Europe/Paris")], sentAt: Date(timeIntervalSince1970: 2))
232+
233+
let replaced = try profile(queue, inboxId: "alice")
234+
#expect(replaced?.metadata?["timezone"] == .string("Europe/Paris"))
235+
#expect(replaced?.metadata?["connections"] == nil)
236+
}
237+
238+
@Test("an older replayed update cannot clear newer metadata, and a snapshot never clears")
239+
func staleAndSnapshotEventsCannotClearMetadata() throws {
240+
let queue = try makeQueue()
241+
try apply(queue, inboxId: "alice", name: "Alice", avatar: .addressed(nil), metadata: ["connections": .string("grants")], sentAt: Date(timeIntervalSince1970: 5))
242+
243+
// A replayed older update (empty map) loses on recency.
244+
try apply(queue, inboxId: "alice", name: "Alice", avatar: .addressed(nil), metadata: [:], sentAt: Date(timeIntervalSince1970: 1))
245+
// A snapshot path collapses empty to nil before the applier, so it says
246+
// nothing about metadata even when newer.
247+
try apply(queue, inboxId: "alice", source: .profileSnapshot, name: "Alice", avatar: .fillIfPresent(nil), metadata: nil, sentAt: Date(timeIntervalSince1970: 9))
248+
249+
let kept = try profile(queue, inboxId: "alice")
250+
#expect(kept?.metadata?["connections"] == .string("grants"))
251+
}
211252
}

ConvosCore/Tests/ConvosCoreTests/ProfileMergeTests.swift

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,62 @@ struct ProfileMergeIdentityTests {
9393
)
9494
#expect(upgrade.memberKind == .verifiedConvos)
9595
}
96+
97+
@Test("a winning event's metadata is authoritative: replaces wholesale, empty clears, nil keeps")
98+
func metadataReplaceClearKeep() {
99+
let existing = DBProfile(
100+
inboxId: "i", name: "Alice", metadata: ["connections": .string("grants")],
101+
profileSource: .profileUpdate, updatedAt: t1
102+
)
103+
104+
let replaced = ProfileMerge.mergeIdentity(
105+
existing: existing, inboxId: "i", incoming: IncomingIdentity(metadata: ["timezone": .string("Europe/Paris")]),
106+
source: .profileUpdate, sentAt: t2
107+
)
108+
#expect(replaced.metadata?["timezone"] == .string("Europe/Paris"))
109+
#expect(replaced.metadata?["connections"] == nil)
110+
111+
let cleared = ProfileMerge.mergeIdentity(
112+
existing: existing, inboxId: "i", incoming: IncomingIdentity(metadata: [:]),
113+
source: .profileUpdate, sentAt: t2
114+
)
115+
#expect(cleared.metadata == nil)
116+
117+
let kept = ProfileMerge.mergeIdentity(
118+
existing: existing, inboxId: "i", incoming: IncomingIdentity(name: "Alice", metadata: nil),
119+
source: .profileUpdate, sentAt: t2
120+
)
121+
#expect(kept.metadata?["connections"] == .string("grants"))
122+
}
123+
124+
@Test("a losing event's empty metadata neither clears nor fills")
125+
func losingEmptyMetadataIsInert() {
126+
let existing = DBProfile(
127+
inboxId: "i", name: "Alice", metadata: ["connections": .string("grants")],
128+
profileSource: .profileUpdate, updatedAt: t2
129+
)
130+
131+
// Older same-source event with an empty map: loses on recency, keeps.
132+
let stale = ProfileMerge.mergeIdentity(
133+
existing: existing, inboxId: "i", incoming: IncomingIdentity(metadata: [:]),
134+
source: .profileUpdate, sentAt: t1
135+
)
136+
#expect(stale.metadata?["connections"] == .string("grants"))
137+
138+
// Lower-source event with an empty map: fill-only, and empty fills nothing.
139+
let lower = ProfileMerge.mergeIdentity(
140+
existing: existing, inboxId: "i", incoming: IncomingIdentity(metadata: [:]),
141+
source: .profileSnapshot, sentAt: t2
142+
)
143+
#expect(lower.metadata?["connections"] == .string("grants"))
144+
145+
// A fresh row from an empty map stores nil, not an empty map.
146+
let fresh = ProfileMerge.mergeIdentity(
147+
existing: nil, inboxId: "i", incoming: IncomingIdentity(name: "Alice", metadata: [:]),
148+
source: .profileUpdate, sentAt: t1
149+
)
150+
#expect(fresh.metadata == nil)
151+
}
96152
}
97153

98154
@Suite("Profile merge - avatar")

0 commit comments

Comments
 (0)