Fix agent group write privacy, transactions and cancellation

Sources/AskKeyAppKit/App/FrozenOrganizationApprovalContent.swift

@@ -21,9 +21,18 @@ struct FrozenOrganizationSummaryContent {
             case .createGroup(let name):
                 title = appLocalizedFormat("Create group “%@”", name)
                 detail = appLocalized("New group — created when approved")
+            case .existingGroup(let name, let members, let nonvisible):
+                title = appLocalizedFormat("Create group “%@”", name)
+                detail = appLocalizedFormat("Group already exists — no changes. %@",
+                    Self.memberCountDescription(members: members, nonvisible: nonvisible))
             case .renameGroup(let from, let to, let members, let nonvisible):
                 title = appLocalizedFormat("Rename group “%@” → “%@”", from, to)
                 detail = Self.memberCountDescription(members: members, nonvisible: nonvisible)
+            case .mergeGroup(let from, let to, let members, let nonvisible, let targetMembers, let targetNonvisible):
+                title = appLocalizedFormat("Merge group “%@” into existing group “%@”", from, to)
+                detail = appLocalizedFormat("Source: %@. Existing group: %@.",
+                    Self.memberCountDescription(members: members, nonvisible: nonvisible),
+                    Self.memberCountDescription(members: targetMembers, nonvisible: targetNonvisible))
             case .deleteGroup(let name, let members, let nonvisible):
                 title = appLocalizedFormat("Delete group “%@”", name)
                 detail = Self.memberCountDescription(members: members, nonvisible: nonvisible)

Sources/AskKeyAppKit/Resources/Localizable.xcstrings

@@ -1,6 +1,30 @@
 {
   "sourceLanguage": "en",
   "strings": {
+    "Group already exists — no changes. %@": {
+      "extractionState": "manual",
+      "localizations": {
+        "en": { "stringUnit": { "state": "translated", "value": "Group already exists — no changes. %@" } },
+        "zh-Hans": { "stringUnit": { "state": "translated", "value": "分组已存在，不会更改。%@" } },
+        "qps-ploc": { "stringUnit": { "state": "translated", "value": "[!!Group already exists — no changes. %@!!]" } }
+      }
+    },
+    "Merge group “%@” into existing group “%@”": {
+      "extractionState": "manual",
+      "localizations": {
+        "en": { "stringUnit": { "state": "translated", "value": "Merge group “%@” into existing group “%@”" } },
+        "zh-Hans": { "stringUnit": { "state": "translated", "value": "将分组“%@”合并到已有分组“%@”" } },
+        "qps-ploc": { "stringUnit": { "state": "translated", "value": "[!!Merge group “%@” into existing group “%@”!!]" } }
+      }
+    },
+    "Source: %@. Existing group: %@.": {
+      "extractionState": "manual",
+      "localizations": {
+        "en": { "stringUnit": { "state": "translated", "value": "Source: %@. Existing group: %@." } },
+        "zh-Hans": { "stringUnit": { "state": "translated", "value": "来源：%@；已有分组：%@。" } },
+        "qps-ploc": { "stringUnit": { "state": "translated", "value": "[!!Source: %@. Existing group: %@.!!]" } }
+      }
+    },
     "“%@” wants to organize credentials": {
       "extractionState": "manual",
       "localizations": {

Sources/AskKeyBroker/BrokerApprovalStateMachine+Requests.swift

@@ -166,7 +166,7 @@ extension BrokerApprovalStateMachine {
             guard entry.request.matchesForConsumption(operationRequest) else {
                 throw BrokerApprovalError.payloadMismatch
             }
-            if entry.state == .pending || entry.state == .approved { entry.state = .cancelled }
+            if !entry.committing, entry.state == .pending || entry.state == .approved { entry.state = .cancelled }
         }.state
     }
 

Sources/AskKeyBroker/BrokerOrganizationSummary.swift

@@ -5,7 +5,9 @@ public struct BrokerOrganizationSummary: Codable, Equatable, Sendable {
     public enum Operation: Codable, Equatable, Sendable {
         case move(credential: String, from: String?, to: String?)
         case createGroup(String)
+        case existingGroup(name: String, members: Int, nonvisible: Int)
         case renameGroup(from: String, to: String, members: Int, nonvisible: Int)
+        case mergeGroup(from: String, to: String, members: Int, nonvisible: Int, targetMembers: Int, targetNonvisible: Int)
         case deleteGroup(name: String, members: Int, nonvisible: Int)
     }
 

Sources/AskKeyVault/FrozenAgentOrganization.swift

@@ -20,6 +20,7 @@ struct AgentOrganizationGroupState: Codable, Equatable {
     let names: [String]
     let storedNames: [String]
     let memberIDs: [String]
+    let memberDigests: [String]
 }
 
 struct AgentOrganizationSnapshot {
@@ -47,11 +48,16 @@ struct AgentOrganizationSnapshot {
     func groupStates(names: Set<String>, key: SymmetricKey) throws -> [AgentOrganizationGroupState] {
         let assignments = try assignments(key: key)
         let allNames = Set(storedGroups).union(assignments.values)
-        return names.sorted().map { normalized in
-            .init(normalizedName: normalized,
+        let encoder = JSONEncoder()
+        encoder.outputFormatting = [.sortedKeys]
+        return try names.sorted().map { normalized in
+            let members = records.filter { assignments[$0.id].map(CredentialName.normalized) == normalized }
+                .sorted { $0.id < $1.id }
+            return .init(normalizedName: normalized,
                 names: allNames.filter { CredentialName.normalized($0) == normalized }.sorted(),
                 storedNames: storedGroups.filter { CredentialName.normalized($0) == normalized }.sorted(),
-                memberIDs: assignments.filter { CredentialName.normalized($0.value) == normalized }.keys.sorted())
+                memberIDs: members.map(\.id),
+                memberDigests: try members.map { Vault.componentDigest(try encoder.encode($0)) })
         }
     }
 }

Sources/AskKeyVault/Storage/VaultStoreConfigQueries.swift

@@ -1,4 +1,5 @@
 import Foundation
+import CryptoKit
 import GRDB
 
 extension VaultStore {
@@ -21,24 +22,35 @@ extension VaultStore {
         }
     }
 
-    func replaceCredentialGroupsConfig(
-        key: String,
-        value: String,
-        clearingCredentialIDs: [String],
+    func editCredentialGroup(
+        _ name: String,
+        key: SymmetricKey,
+        deleting: Bool,
         updatedAt: String
-    ) throws {
+    ) throws -> [String] {
         try db.write { db in
+            let configKey = Vault.credentialGroupsConfigKey
+            let current = try ConfigRecord.fetchOne(db, key: configKey)?.value
+            var groups = Set(try AgentOrganizationSnapshot.storedGroups(current, key: key))
+            if deleting { groups.remove(name) } else { groups.insert(name) }
+            let value = try VaultCrypto.encrypt(JSONEncoder().encode(groups.sorted()), using: key).base64EncodedString()
             try db.execute(
                 sql: "INSERT OR REPLACE INTO config (key, value) VALUES (?, ?)",
-                arguments: [key, value]
+                arguments: [configKey, value]
             )
-            for id in clearingCredentialIDs {
-                guard let fetched = try CredentialRecord.fetchOne(db, key: id) else { continue }
-                var record = try authenticatedCredential(fetched)
-                record.encryptedGroupName = nil
-                record.updatedAt = updatedAt
-                try credentialForPersistence(record).update(db)
+            var clearedIDs: [String] = []
+            if deleting {
+                for fetched in try CredentialRecord.fetchAll(db) {
+                    var record = try authenticatedCredential(fetched)
+                    guard let encrypted = record.encryptedGroupName,
+                          try VaultCrypto.decrypt(encrypted, using: key) == name else { continue }
+                    record.encryptedGroupName = nil
+                    record.updatedAt = updatedAt
+                    try credentialForPersistence(record).update(db)
+                    clearedIDs.append(record.id)
+                }
             }
+            return clearedIDs
         }
     }
 }

Sources/AskKeyVault/Vault+AgentOrganization.swift

@@ -33,8 +33,10 @@ extension Vault {
     }
 
     func cancelAgentOrganization(operationID: String, requestID: String, capability: String) throws -> BrokerRequestState {
-        guard let frozen = agentOrganizations.entry(operationID: operationID) else { throw BrokerApprovalError.requestNotFound }
-        return try approvalRequests.cancel(requestID: requestID, capability: capability, operationRequest: frozen.approvalRequest)
+        try agentOrganizations.synchronized {
+            guard let frozen = agentOrganizations.entry(operationID: operationID) else { throw BrokerApprovalError.requestNotFound }
+            return try approvalRequests.cancel(requestID: requestID, capability: capability, operationRequest: frozen.approvalRequest)
+        }
     }
 
     public func frozenAgentOrganizationSummary(operationID: String, requestID: String,

Sources/AskKeyVault/Vault+AgentOrganizationFreeze.swift

@@ -10,6 +10,7 @@ extension Vault {
         }
         let snapshot = try store.agentOrganizationSnapshot(key: key)
         var assignments = try snapshot.assignments(key: key)
+        var storedGroupNames = Set(snapshot.storedGroups)
         var groupNames = Set(snapshot.storedGroups).union(assignments.values)
         var knownGroups = Set(groupNames.filter { group in
             let members = snapshot.records.filter { assignments[$0.id].map(CredentialName.normalized) == CredentialName.normalized(group) }
@@ -39,6 +40,9 @@ extension Vault {
         func members(_ group: String) -> [CredentialRecord] {
             records.values.filter { assignments[$0.id].map(normalized) == normalized(group) }.sorted { $0.id < $1.id }
         }
+        func nonvisible(_ members: [CredentialRecord]) -> Int {
+            members.filter { $0.deletedAt != nil || $0.permission == CredentialPermission.hidden.rawValue }.count
+        }
         func assign(_ id: String, to group: String?) throws {
             guard var record = records[id] else { throw VaultError.credentialUnavailable }
             affected.insert(id)
@@ -73,41 +77,59 @@ extension Vault {
             case .createGroup(let raw):
                 let display = try name(raw)
                 namedGroups.insert(normalized(display))
-                guard !groupNames.contains(where: { normalized($0) == normalized(display) }) else {
+                guard !knownGroups.contains(normalized(display)) else {
                     throw VaultError.credentialUnavailable
                 }
-                groupNames.insert(display)
+                if let existing = groupNames.sorted().first(where: { normalized($0) == normalized(display) }) {
+                    let existingMembers = members(existing)
+                    summary.append(.existingGroup(name: existing, members: existingMembers.count, nonvisible: nonvisible(existingMembers)))
+                } else {
+                    groupNames.insert(display)
+                    storedGroupNames.insert(display)
+                    summary.append(.createGroup(display))
+                }
                 knownGroups.insert(normalized(display))
-                summary.append(.createGroup(display))
             case .renameGroup(let rawFrom, let rawTo):
                 let from = try existingGroup(rawFrom)
-                let to = try name(rawTo)
-                namedGroups.insert(normalized(to))
-                guard !groupNames.contains(where: { normalized($0) == normalized(to) }) else {
+                let proposed = try name(rawTo)
+                namedGroups.insert(normalized(proposed))
+                guard !knownGroups.contains(normalized(proposed)) else {
                     throw VaultError.credentialUnavailable
                 }
+                let existing = groupNames.sorted().first { normalized($0) == normalized(proposed) }
+                let to = existing ?? proposed
                 let affectedMembers = members(from)
-                summary.append(.renameGroup(from: from, to: to, members: affectedMembers.count,
-                    nonvisible: affectedMembers.filter { $0.deletedAt != nil || $0.permission == CredentialPermission.hidden.rawValue }.count))
+                if let existing {
+                    let targetMembers = members(existing)
+                    summary.append(.mergeGroup(from: from, to: to, members: affectedMembers.count,
+                        nonvisible: nonvisible(affectedMembers), targetMembers: targetMembers.count,
+                        targetNonvisible: nonvisible(targetMembers)))
+                } else {
+                    summary.append(.renameGroup(from: from, to: to, members: affectedMembers.count,
+                        nonvisible: nonvisible(affectedMembers)))
+                    storedGroupNames.insert(to)
+                }
                 for record in affectedMembers { try assign(record.id, to: to) }
                 groupNames = Set(groupNames.filter { normalized($0) != normalized(from) })
+                storedGroupNames = Set(storedGroupNames.filter { normalized($0) != normalized(from) })
                 groupNames.insert(to)
                 knownGroups.remove(normalized(from))
                 knownGroups.insert(normalized(to))
             case .deleteGroup(let raw):
                 let group = try existingGroup(raw)
                 let affectedMembers = members(group)
                 summary.append(.deleteGroup(name: group, members: affectedMembers.count,
-                    nonvisible: affectedMembers.filter { $0.deletedAt != nil || $0.permission == CredentialPermission.hidden.rawValue }.count))
+                    nonvisible: nonvisible(affectedMembers)))
                 for record in affectedMembers { try assign(record.id, to: nil) }
                 groupNames = Set(groupNames.filter { normalized($0) != normalized(group) })
+                storedGroupNames = Set(storedGroupNames.filter { normalized($0) != normalized(group) })
                 knownGroups.remove(normalized(group))
             }
         }
         let before = affected.sorted().compactMap { original[$0] }
         let after = affected.sorted().compactMap { records[$0] }
         let groups = try snapshot.groupStates(names: namedGroups, key: key)
-        let finalNames = groupNames.filter { namedGroups.contains(normalized($0)) }.sorted()
+        let finalNames = storedGroupNames.filter { namedGroups.contains(normalized($0)) }.sorted()
         let presentation = BrokerOrganizationSummary(operations: summary)
         struct ApprovalContents: Encodable {
             let requestDigest: String

Sources/AskKeyVault/Vault+AgentWriteCommit.swift

@@ -67,7 +67,7 @@ extension Vault {
             throw BrokerApprovalError.payloadMismatch
         }
         if let expiresAt = frozen.credentialExpiresAt, expiresAt <= currentDate {
-            cancelAgentWrites(credentialID: frozen.approvalRequest.credentialID)
+            cancelAgentWritesForExpiry(credentialID: frozen.approvalRequest.credentialID)
             throw VaultError.credentialUnavailable
         }
         do {
@@ -90,7 +90,7 @@ extension Vault {
             notifySnapshotRelevantChange()
             return result
         } catch VaultError.credentialUnavailable {
-            cancelAgentWrites(credentialID: frozen.approvalRequest.credentialID)
+            cancelAgentWritesForExpiry(credentialID: frozen.approvalRequest.credentialID)
             throw VaultError.credentialUnavailable
         } catch BrokerApprovalError.alreadyConsumed {
             guard let committed = try completedAgentTextWrite(
@@ -115,9 +115,9 @@ extension Vault {
         }
     }
 
-    private func cancelAgentWrites(credentialID: String) {
+    private func cancelAgentWritesForExpiry(credentialID: String) {
         brokerRequests.cancelPending(credentialID: credentialID)
-        approvalRequests.cancelPending(credentialID: credentialID)
+        approvalRequests.cancelPendingForExpiry(credentialID: credentialID)
     }
 
     private func completedAgentTextWrite(

Sources/AskKeyVault/Vault+AgentWriteRequest.swift

@@ -97,10 +97,10 @@ extension Vault {
         requestID: String,
         capability: String
     ) throws -> BrokerRequestState {
-        try agentTextWrites.synchronized {
-            if agentOrganizations.entry(operationID: operationID) != nil {
-                return try cancelAgentOrganization(operationID: operationID, requestID: requestID, capability: capability)
-            }
+        if agentOrganizations.entry(operationID: operationID) != nil {
+            return try cancelAgentOrganization(operationID: operationID, requestID: requestID, capability: capability)
+        }
+        return try agentTextWrites.synchronized {
             guard let frozen = agentTextWrites.entry(operationID: operationID) else {
                 throw BrokerApprovalError.requestNotFound
             }

Sources/AskKeyVault/Vault+CredentialGroups.swift

@@ -47,9 +47,10 @@ extension Vault {
         try authorizeManagement(authenticator, reason: CredentialManagementCopy.manageReason)
         let key = try requireKey()
         let name = try CredentialName.displayName(from: rawName)
-        var groups = Set(try storedCredentialGroups(key: key))
-        groups.insert(name)
-        try persistCredentialGroups(groups.sorted(), key: key)
+        let wasPaused = try agentAccessGate.beginExclusiveChange()
+        defer { agentAccessGate.endExclusiveChange(paused: wasPaused) }
+        _ = try store.editCredentialGroup(name, key: key, deleting: false,
+            updatedAt: sharedDateFormatter.string(from: currentDate))
         notifySnapshotRelevantChange()
     }
 
@@ -64,21 +65,8 @@ extension Vault {
         let deliveryManager = try fileDeliveryManager.get()
         let wasPaused = try agentAccessGate.beginExclusiveChange()
         defer { agentAccessGate.endExclusiveChange(paused: wasPaused) }
-        var groups = Set(try storedCredentialGroups(key: key))
-        groups.remove(name)
-        let matchingIDs = try (store.fetchAllCredentials() + store.fetchRecycledCredentials())
-            .compactMap { record -> String? in
-                guard let encrypted = record.encryptedGroupName,
-                      try VaultCrypto.decrypt(encrypted, using: key) == name else { return nil }
-                return record.id
-            }
-        let encrypted = try encryptedCredentialGroups(groups.sorted(), key: key)
-        try store.replaceCredentialGroupsConfig(
-            key: Self.credentialGroupsConfigKey,
-            value: encrypted,
-            clearingCredentialIDs: matchingIDs,
-            updatedAt: sharedDateFormatter.string(from: currentDate)
-        )
+        let matchingIDs = try store.editCredentialGroup(name, key: key, deleting: true,
+            updatedAt: sharedDateFormatter.string(from: currentDate))
         for id in matchingIDs {
             brokerRequests.cancelPending(credentialID: id)
             approvalRequests.cancelPending(credentialID: id)
@@ -97,15 +85,4 @@ extension Vault {
             from: VaultCrypto.decryptData(encrypted, using: key)
         )
     }
-
-    private func persistCredentialGroups(_ groups: [String], key: SymmetricKey) throws {
-        try store.setConfigValue(
-            key: Self.credentialGroupsConfigKey,
-            value: try encryptedCredentialGroups(groups, key: key)
-        )
-    }
-
-    func encryptedCredentialGroups(_ groups: [String], key: SymmetricKey) throws -> String {
-        try VaultCrypto.encrypt(JSONEncoder().encode(groups), using: key).base64EncodedString()
-    }
 }