From fabfd07574e395696b0bfcdd52cd4567afc49efa Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 06:00:32 +0200 Subject: [PATCH] fix(ios): removing several assistants no longer crashes Access The swipe's Remove access has the destructive role, so the list takes the row out at once and expects the data to lose it in the same action. The app waited for the box first, then removed the row, then read the list again, with the request's section a step later. The collection view and SwiftUI disagreed by a row until the box answered, and the next update failed UIKit's count check: a tester's crash on 17 October 2026 after removing the Grok grants left by four tries. AccessModel.remove hides the row and its requests in the swipe's own change and then tells the box. The answer changes nothing on screen unless the box refuses; then the row comes back with the reason, which stays only while the row does. The list is read once after the last answer, and a read sent before a removal cannot bring its row back. The grant page closes at once the same way. The demo takes -FTWDemoAccess grok and -FTWDemoAccessDelayMs, and AccessRemoveTests swipes the five rows away against it. Co-Authored-By: Claude Opus 5.5 --- appleApp/FTW/Access/AccessView.swift | 5 +- appleApp/FTW/Access/GrantView.swift | 7 +- .../Sources/FTWDemo/SimulatedAccess.swift | 22 ++++ .../FTWKit/Sources/FTWDemo/SimulatedBox.swift | 17 ++- .../Sources/FTWKit/Access/AccessModel.swift | 78 +++++++++--- .../Tests/FTWKitTests/AccessFlowTests.swift | 116 ++++++++++++++++++ appleApp/FTWUITests/AccessRemoveTests.swift | 103 ++++++++++++++++ 7 files changed, 323 insertions(+), 25 deletions(-) create mode 100644 appleApp/FTWUITests/AccessRemoveTests.swift diff --git a/appleApp/FTW/Access/AccessView.swift b/appleApp/FTW/Access/AccessView.swift index 3f8e8207..cddd18ee 100644 --- a/appleApp/FTW/Access/AccessView.swift +++ b/appleApp/FTW/Access/AccessView.swift @@ -73,10 +73,11 @@ struct AccessView: View { } /// Removing is one tap: no Face ID and no question, since it is always - /// safe. + /// safe. The row leaves in the swipe's own change; the box hears of it + /// after, and the row comes back if the box refuses. private func remove(_ row: AgentRow) -> some View { Button(role: .destructive) { - Task { await access.revoke(row) } + withAnimation(Theme.spring) { access.remove(row) } } label: { Label(AccessWords.text("Remove access"), systemImage: "xmark.circle") } diff --git a/appleApp/FTW/Access/GrantView.swift b/appleApp/FTW/Access/GrantView.swift index 03029464..e9badf91 100644 --- a/appleApp/FTW/Access/GrantView.swift +++ b/appleApp/FTW/Access/GrantView.swift @@ -107,9 +107,10 @@ struct GrantView: View { Section { Button(role: .destructive) { - Task { - if await access.revoke(row) { dismiss() } - } + // Gone from the list at once; back there with the + // box's reason if it refuses. + access.remove(row) + dismiss() } label: { Label(AccessWords.text("Remove access"), systemImage: "xmark.circle") } diff --git a/appleApp/FTWKit/Sources/FTWDemo/SimulatedAccess.swift b/appleApp/FTWKit/Sources/FTWDemo/SimulatedAccess.swift index a4ea37e3..a9320598 100644 --- a/appleApp/FTWKit/Sources/FTWDemo/SimulatedAccess.swift +++ b/appleApp/FTWKit/Sources/FTWDemo/SimulatedAccess.swift @@ -67,11 +67,31 @@ public final class SimulatedAccess { } } + /// The tester's home on 17 October 2026: Grok took four tries, so three + /// grants never called and one did. On top of `seed()`. + public func seedGrokTries() { + let now = Int64(nowMs()) + let day: Int64 = 86_400_000 + for i in 0..<4 { + let key = Bytes(repeating: UInt8(0x31 + i), count: 32) + rows.append(AgentRow(id: String(Base64url.encode(key).prefix(8)), key: key, role: Contract.roleAgent, + scopes: ["ftw.health.read", "ftw.live.read", "ftw.plan.read"], ops: [], + caps: Caps(perMin: 6), expiresAtMs: now + 30 * day - Int64(i) * 60_000, + principal: Principal(client: "Grok", clientId: "https://grok.com/oauth/mcp-client"), + approvedBy: "app:\(signedFor.deviceID)", addedAtMs: now - Int64(4 - i) * 6 * 60_000, + lastSeenMs: i == 3 ? now - 2 * 60_000 : 0, epoch: 1)) + } + } + // MARK: Messages /// For a test: act on the next message but lose its answer, as a /// dropped connection would. public var loseNextAnswer = false + /// For a test: rows whose removal the box refuses, as a busy box does. + public var refuseRemoving: Set = [] + /// How many times the phone read the list. + public private(set) var lists = 0 func receive(_ env: Envelope, reply given: @escaping (Envelope) -> Void) { var reply = given @@ -87,6 +107,7 @@ public final class SimulatedAccess { let now = Int64(nowMs()) switch env.t { case AccessWire.msgList: + lists += 1 var list: [(String, CBOR)] = [ ("agents", .array(rows.map(Self.cbor))), ("requests", .array(requests.map(Self.cbor))), @@ -163,6 +184,7 @@ public final class SimulatedAccess { ok(.map([("agent", Self.cbor(rows[i]))])) case AccessWire.msgRevoke: guard let id = body["id"]?.string, rows.contains(where: { $0.id == id }) else { return refuse("E_NOT_FOUND", []) } + if refuseRemoving.contains(id) { return refuse("E_UNAVAILABLE", []) } rows.removeAll { $0.id == id } requests.removeAll { $0.agent == id } log("app:\(signedFor.deviceID)", what: AccessWire.msgRevoke, target: id, result: "ok") diff --git a/appleApp/FTWKit/Sources/FTWDemo/SimulatedBox.swift b/appleApp/FTWKit/Sources/FTWDemo/SimulatedBox.swift index 57ac36d7..bac2f3f9 100644 --- a/appleApp/FTWKit/Sources/FTWDemo/SimulatedBox.swift +++ b/appleApp/FTWKit/Sources/FTWDemo/SimulatedBox.swift @@ -74,6 +74,9 @@ public final class SimulatedBox { public private(set) var stepUpsSeen = 0 /// Its assistants, support grants, requests and audit log. public private(set) lazy var access = SimulatedAccess { [scheduler] in scheduler.nowMs } + /// How long an answer about agents waits to leave: a box behind FTW + /// Cloud, far from the phone. + public var accessDelayMs: Double = 0 public nonisolated static let modes: [ModeInfo] = [ ModeInfo(key: "planner_passive_arbitrage", label: "Passive arbitrage", tooltip: "Charge from the cheapest available source (PV when sunny, grid during cheap hours). Never exports from battery.", tier: "primary"), @@ -195,7 +198,14 @@ public final class SimulatedBox { case "cmd": onCommand(body) case AccessWire.msgList, AccessWire.msgAudit, AccessWire.msgKeySet, AccessWire.msgAdd, AccessWire.msgWiden, AccessWire.msgRevoke, AccessWire.msgConfirm: - access.receive(env) { [weak self] in self?.sendBulk($0) } + access.receive(env) { [weak self] reply in + guard let self else { return } + if self.accessDelayMs > 0 { + self.scheduler.after(self.accessDelayMs) { [weak self] in self?.sendBulk(reply) } + } else { + self.sendBulk(reply) + } + } default: break } } @@ -920,6 +930,11 @@ extension SimulatedBox { // For UI tests that look while a command waits: `-FTWDemoResultDelayMs // 2500` answers later than XCTest's wait for the app to settle. if UserDefaults.standard.object(forKey: "FTWDemoResultDelayMs") != nil { box.resultDelayMs = UserDefaults.standard.double(forKey: "FTWDemoResultDelayMs") } + // For the removal test: `-FTWDemoAccess grok` adds the four Grok + // grants a tester was left with, and `-FTWDemoAccessDelayMs 600` + // answers about agents as late as a box behind FTW Cloud. + if UserDefaults.standard.string(forKey: "FTWDemoAccess") == "grok" { box.access.seedGrokTries() } + if UserDefaults.standard.object(forKey: "FTWDemoAccessDelayMs") != nil { box.accessDelayMs = UserDefaults.standard.double(forKey: "FTWDemoAccessDelayMs") } // For screenshots: `-FTWDemoCar unplugged` and the like. if let name = UserDefaults.standard.string(forKey: "FTWDemoCar"), let car = DemoCar(rawValue: name) { car.apply(to: box.house, nowMs: scheduler.nowMs) diff --git a/appleApp/FTWKit/Sources/FTWKit/Access/AccessModel.swift b/appleApp/FTWKit/Sources/FTWKit/Access/AccessModel.swift index fca89d5c..598d5bb1 100644 --- a/appleApp/FTWKit/Sources/FTWKit/Access/AccessModel.swift +++ b/appleApp/FTWKit/Sources/FTWKit/Access/AccessModel.swift @@ -157,6 +157,16 @@ public final class AccessModel { public private(set) var keysKnown = false /// The row or request a change is on its way for. public private(set) var working: String? + /// Rows the owner removed whose answer has not come back. They leave + /// the lists at once, and come back only if the box refuses. + public private(set) var removing: Set = [] + /// Rows the box has said are gone. An answer to `agent.list` sent + /// before the box removed one still holds it, and must not bring it + /// back. + @ObservationIgnored private var removed: Set = [] + /// Why each failed removal failed, kept until the read after the last + /// removal so that read does not wipe it. + @ObservationIgnored private var refused: [String: String] = [:] /// What went wrong, in words that say what happens now. public private(set) var help: String? @@ -238,9 +248,9 @@ public final class AccessModel { site.heardFromBox && site.canConfigure && site.session.caps.contains(Contract.capAccessAgents) } - public var assistants: [AgentRow] { (list?.agents ?? []).filter { !$0.isSupport } } - public var support: [AgentRow] { (list?.agents ?? []).filter(\.isSupport) } - public var pending: [AgentRequest] { (list?.requests ?? []).filter(\.pending) } + public var assistants: [AgentRow] { (list?.agents ?? []).filter { !$0.isSupport && !removing.contains($0.id) } } + public var support: [AgentRow] { (list?.agents ?? []).filter { $0.isSupport && !removing.contains($0.id) } } + public var pending: [AgentRequest] { (list?.requests ?? []).filter { $0.pending && !removing.contains($0.agent) } } /// The box holds an approval key for this phone. public var boxHasKey: Bool { list?.keys.contains { $0.deviceId == signedFor.deviceID } ?? false } @@ -264,7 +274,7 @@ public final class AccessModel { public func refresh() async { guard available else { return } do { - list = AgentList(try await site.ask(AccessWire.msgList, .emptyMap)) + list = withoutRemoved(AgentList(try await site.ask(AccessWire.msgList, .emptyMap))) keysKnown = true help = nil } catch { @@ -335,29 +345,59 @@ public final class AccessModel { return changed != nil } - /// Remove access at once. No Face ID: removing is always safe. + /// Remove access at once: the row and its requests leave the lists in + /// this change, the one the owner's swipe animates, and the box hears + /// of it after. No Face ID: removing is always safe. + public func remove(_ row: AgentRow) { + guard removing.insert(row.id).inserted else { return } + Task { await sendRevoke(row) } + } + + /// `remove`, waiting for the box. True when the row is gone; false when + /// the box refused and the row is back, or another removal of it is on + /// its way. @discardableResult public func revoke(_ row: AgentRow) async -> Bool { - working = row.id - defer { working = nil } + guard removing.insert(row.id).inserted else { return false } + return await sendRevoke(row) + } + + /// Each removal goes to the box at once and on its own. Its answer + /// changes nothing the owner sees unless the box refused. The box is + /// read once, after the last answer, rather than once per removal. + private func sendRevoke(_ row: AgentRow) async -> Bool { + var gone = true do { _ = try await site.ask(AccessWire.msgRevoke, .map([("id", .text(row.id))])) - if var l = list { - l.agents.removeAll { $0.id == row.id } - l.requests.removeAll { $0.agent == row.id } - list = l - } - help = nil - await refresh() - return true } catch let r as BoxRefusal where r.detail.code == "E_NOT_FOUND" { // Already gone, whoever removed it. - await refresh() - return true } catch { - help = words(error) - return false + gone = false + refused[row.id] = words(error) + help = refused[row.id] + } + if gone { + removed.insert(row.id) + list = list.map(withoutRemoved) } + removing.remove(row.id) + guard removing.isEmpty else { return gone } + await refresh() + // A failure stands while its row does. A removal whose answer was + // lost may still have gone through, and then the read says so. + if let why = refused.first(where: { id, _ in self.row(id) != nil })?.value { help = why } + refused = [:] + return gone + } + + /// The box's list without the rows it has already said are gone, and + /// without their requests. + private func withoutRemoved(_ l: AgentList) -> AgentList { + guard !removed.isEmpty else { return l } + var l = l + l.agents.removeAll { removed.contains($0.id) } + l.requests.removeAll { removed.contains($0.agent) } + return l } // MARK: Requests diff --git a/appleApp/FTWKit/Tests/FTWKitTests/AccessFlowTests.swift b/appleApp/FTWKit/Tests/FTWKitTests/AccessFlowTests.swift index 725cde88..dd760730 100644 --- a/appleApp/FTWKit/Tests/FTWKitTests/AccessFlowTests.swift +++ b/appleApp/FTWKit/Tests/FTWKitTests/AccessFlowTests.swift @@ -224,6 +224,122 @@ import FTWDemo #expect(w.access.pending.isEmpty) } + /// What the owner sees in Access: the assistants and the requests. + struct Seen: Equatable { + var assistants: [String] + var pending: [String] + } + + /// The home a tester was left with on 17 October 2026: Claude with a + /// request, and four Grok grants from four tries. + func grokWorld() async -> World { + let w = await world() + w.box.access.seedGrokTries() + await w.during { await w.access.refresh() } + return w + } + + /// Run the clock and note each change the owner would see. + func watch(_ w: World, _ ms: Double = 2_000) async -> [Seen] { + var seen: [Seen] = [] + var left = ms + while left > 0 { + for _ in 0..<20 { await Task.yield() } + let now = Seen(assistants: w.access.assistants.map(\.id), pending: w.access.pending.map(\.reqId)) + if seen.last != now { seen.append(now) } + w.scheduler.advance(10) + left -= 10 + } + return seen + } + + /// Five swipes before the box has answered one: each row and its + /// request leave at once, nothing comes back while the answers arrive, + /// and the box is read once, after the last. + @Test func severalRemovalsLeaveOneConsistentList() async throws { + let w = await grokWorld() + let rows = w.access.assistants + #expect(rows.count == 5) + #expect(w.access.pending.count == 1) + let reads = w.box.access.lists + for row in rows { w.access.remove(row) } + let gone = Seen(assistants: [], pending: []) + #expect(Seen(assistants: w.access.assistants.map(\.id), pending: w.access.pending.map(\.reqId)) == gone) + let seen = await watch(w) + #expect(seen == [gone]) + #expect(w.access.removing.isEmpty) + #expect(w.access.list?.agents.contains { !$0.isSupport } == false) + #expect(w.access.list?.requests.isEmpty == true) + #expect(w.box.access.rows.allSatisfy { $0.isSupport }) + #expect(w.box.access.lists == reads + 1) + #expect(w.keyring.unlocks == 0) + } + + /// The same through `revoke`, all at once: each says it is gone. + @Test func concurrentRevokesAllSucceed() async throws { + let w = await grokWorld() + let rows = w.access.assistants + let results = await w.during { + let tasks = rows.map { row in Task { @MainActor in await w.access.revoke(row) } } + var out: [Bool] = [] + for task in tasks { out.append(await task.value) } + return out + } + #expect(results == Array(repeating: true, count: rows.count)) + #expect(w.access.assistants.isEmpty) + #expect(w.access.pending.isEmpty) + #expect(w.box.access.rows.allSatisfy { $0.isSupport }) + // A second removal of a row on its way says nothing went. + let w2 = await grokWorld() + let one = try #require(w2.access.assistants.last) + w2.access.remove(one) + #expect(await w2.access.revoke(one) == false) + } + + /// The box refuses one of several: that row comes back once, with the + /// box's reason, and the others stay gone. + @Test func aRefusedRemovalComesBackWithTheReason() async throws { + let w = await grokWorld() + let grok = w.access.assistants.filter { $0.principal.client == "Grok" } + #expect(grok.count == 4) + let kept = grok[1] + w.box.access.refuseRemoving = [kept.id] + for row in grok { w.access.remove(row) } + let seen = await watch(w) + let claude = "q7Xn2vKd" + #expect(seen.map(\.assistants) == [[claude], [claude, kept.id]]) + #expect(w.access.help == AccessWords.text("Your box is busy. Try again in a moment.")) + #expect(w.box.access.rows.contains { $0.id == kept.id }) + #expect(w.access.pending.count == 1) + } + + /// The box removes a row but its answer is lost: the row comes back + /// when the wait runs out, then goes with the read after it, and no + /// failure is left on screen for a removal that went through. + @Test func aLostAnswerEndsWithTheBoxsWord() async throws { + let w = await grokWorld() + let row = try #require(w.access.assistants.last) + w.box.access.loseNextAnswer = true + let task = Task { await w.access.revoke(row) } + await w.run(10_000) + #expect(await task.value == false) + #expect(!w.box.access.rows.contains { $0.id == row.id }) + #expect(!w.access.assistants.contains { $0.id == row.id }) + #expect(w.access.help == nil) + } + + /// A row the box no longer has, as a page left open shows it, is as + /// good as removed. + @Test func removingWhatIsAlreadyGoneIsDone() async throws { + let w = await grokWorld() + let row = try #require(w.access.assistants.last) + await w.during { _ = await w.access.revoke(row) } + let again = await w.during { await w.access.revoke(row) } + #expect(again) + #expect(w.access.help == nil) + #expect(!w.access.assistants.contains { $0.id == row.id }) + } + /// Approving signs the request, the agent, the op and the hash of the /// args exactly as received; declining is one tap. @Test func aRequestIsApprovedOverWhatWasShown() async throws { diff --git a/appleApp/FTWUITests/AccessRemoveTests.swift b/appleApp/FTWUITests/AccessRemoveTests.swift new file mode 100644 index 00000000..a8edd926 --- /dev/null +++ b/appleApp/FTWUITests/AccessRemoveTests.swift @@ -0,0 +1,103 @@ +import XCTest + +/// Removing several assistants in a row, as a tester did on 17 October +/// 2026 after Grok took four tries: the app crashed in the list's batch +/// update. The demo box holds Claude, with a request waiting, and the four +/// Grok grants. Each removal is one swipe and takes the row away at once; +/// the box answers late, so the next swipe comes before it has answered +/// the last. The app must still be running with the rows gone. +/// +/// xcodebuild test … -only-testing:FTWUITests/AccessRemoveTests -resultBundlePath remove.xcresult +/// xcrun xcresulttool export attachments --path remove.xcresult --output-path remove +/// +/// `TEST_RUNNER_FTW_ACCESS_DELAY_MS` sets how late the box answers: 2500 ms +/// by default, so removals overlap; 600 ms is closer to a box behind FTW +/// Cloud. +final class AccessRemoveTests: XCTestCase { + /// The demo's Grok grants, ids as the box makes them from each key. + static let grok = [0x31, 0x32, 0x33, 0x34].map { byte in + String(Data(repeating: UInt8(byte), count: 32).base64EncodedString() + .replacingOccurrences(of: "+", with: "-").replacingOccurrences(of: "/", with: "_").prefix(8)) + } + static let claude = "q7Xn2vKd" + + /// Full swipes, one after another: each removes at once. + @MainActor + func testFullSwipesInARow() throws { + try removeAll(style: "full") { app, row in + let start = row.coordinate(withNormalizedOffset: CGVector(dx: 0.92, dy: 0.5)) + let end = row.coordinate(withNormalizedOffset: CGVector(dx: -0.3, dy: 0.5)) + start.press(forDuration: 0.05, thenDragTo: end, withVelocity: .fast, thenHoldForDuration: 0) + } + } + + /// Swipe, tap Remove access, and on to the next. + @MainActor + func testSwipeAndTapInARow() throws { + try removeAll(style: "tap") { app, row in + row.swipeLeft(velocity: .fast) + let remove = app.buttons["Remove access"] + if remove.waitForExistence(timeout: 2) { remove.tap() } + } + } + + @MainActor + private func removeAll(style: String, _ swipe: @MainActor (XCUIApplication, XCUIElement) -> Void) throws { + let env = ProcessInfo.processInfo.environment + let app = XCUIApplication() + app.launchArguments = [ + "-FTWDemo", "YES", "-FTWSheet", "settings", "-FTWDemoAccess", "grok", + "-FTWDemoAccessDelayMs", env["FTW_ACCESS_DELAY_MS"] ?? "2500", + "-AppleLanguages", "(en)", "-AppleLocale", "en_US", + ] + app.launch() + + XCTAssertTrue(app.buttons["mode.close"].waitForExistence(timeout: 15)) + let access = reveal(app, app.buttons["settings.access"]) + XCTAssertTrue(access.waitForExistence(timeout: 5)) + access.tap() + let rows = (Self.grok + [Self.claude]).map { app.buttons["access.grant.\($0)"] } + for row in rows { XCTAssertTrue(row.waitForExistence(timeout: 10), "\(row) never showed") } + XCTAssertTrue(app.buttons.matching(NSPredicate(format: "identifier BEGINSWITH 'access.request.'")).firstMatch.exists) + Thread.sleep(forTimeInterval: 1) + shoot(app, "\(style)-before") + + // Each row goes with its swipe, before the box has answered; the + // next swipe waits for that, so it finds the rows where they are now. + for row in rows { + XCTAssertTrue(row.exists, "\(row) went before its swipe") + swipe(app, row) + XCTAssertTrue(row.waitForNonExistence(timeout: 2), "\(row) is still there after its swipe") + } + shoot(app, "\(style)-during") + + // Every answer and the read after the last are in by now. + Thread.sleep(forTimeInterval: 8) + XCTAssertEqual(app.state, .runningForeground, "the app is gone") + for row in rows { XCTAssertFalse(row.exists, "\(row) is still there") } + // Claude's request went with Claude, and the list says no one is left. + XCTAssertFalse(app.buttons.matching(NSPredicate(format: "identifier BEGINSWITH 'access.request.'")).firstMatch.exists) + XCTAssertTrue(app.staticTexts["No assistant can reach your home."].exists) + shoot(app, "\(style)-after") + } + + /// A row a lazy list has not made yet: scroll until it is there. + @MainActor @discardableResult + private func reveal(_ app: XCUIApplication, _ element: XCUIElement) -> XCUIElement { + let screen = app.windows.firstMatch.frame + let list = app.collectionViews.firstMatch + for _ in 0..<8 where !(element.exists && element.isHittable && screen.contains(element.frame)) { + (list.exists ? list : app).swipeUp(velocity: .slow) + } + return element + } + + @MainActor + private func shoot(_ app: XCUIApplication, _ name: String) { + guard app.state == .runningForeground else { return } + let shot = XCTAttachment(screenshot: app.screenshot()) + shot.name = name + shot.lifetime = .keepAlways + add(shot) + } +}