From 94b58118b5afeccad86708d7981f4847b6e3d17b Mon Sep 17 00:00:00 2001 From: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:31:29 -0700 Subject: [PATCH 1/3] fix(bridge): fail fast when a request is discarded by a Messages relaunch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A v2 request that disappears from the inbox without a reply can never be answered, but the poll loop kept waiting for it until the caller's full timeout. For sends that is 150s, during which the caller is blocked. `MessagesLauncher.launchInjectedMessages()` calls `cleanQueueDirectory()` on both queue directories, so relaunching Messages.app with the dylib wipes any request already in flight. If Messages.app dies mid-request — the keepalive then relaunches and reinjects — the original request is deleted and nothing will ever write its response file. The loop had no way to notice and polled on to the deadline. Detect it from the request's own on-disk state. A live request is either unclaimed (`.json`) or claimed by the dylib (`.processing.`, see processV2InboxFile). When neither exists and no reply has landed, the queue was cleared and the request is gone, so surface `.bridgeNotReady` immediately instead of stalling. The outbox is re-checked once before giving up, because the dylib removes its claim and writes the reply as separate steps and a reply can land between the two checks. An inbox that cannot be enumerated is treated as still-queued so a live request is never aborted by a transient read error. Beyond ending the stall, this distinguishes "discarded, definitely not delivered" from a plain timeout, which callers can safely retry — a timeout leaves delivery genuinely unknown. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LEKzjCwQqoMEbZwabsY1u6 --- Sources/IMsgCore/IMsgBridgeClient.swift | 80 +++++++++++++++---- .../IMsgBridgeClientQueueTests.swift | 75 +++++++++++++++++ 2 files changed, 139 insertions(+), 16 deletions(-) create mode 100644 Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift diff --git a/Sources/IMsgCore/IMsgBridgeClient.swift b/Sources/IMsgCore/IMsgBridgeClient.swift index 10dc261c..82dd4f29 100644 --- a/Sources/IMsgCore/IMsgBridgeClient.swift +++ b/Sources/IMsgCore/IMsgBridgeClient.swift @@ -97,30 +97,78 @@ public final class IMsgBridgeClient: @unchecked Sendable { let deadline = Date().addingTimeInterval(timeout) while Date() < deadline { try await Task.sleep(nanoseconds: UInt64(pollInterval * 1_000_000_000)) - guard - let data = try? Data(contentsOf: URL(fileURLWithPath: outPath)), - data.count > 1 - else { continue } - // Best-effort cleanup; ignore failures (dylib may also unlink). - try? FileManager.default.removeItem(atPath: outPath) - - guard - let raw = try? JSONSerialization.jsonObject(with: data, options: []) - as? [String: Any] - else { - throw IMsgBridgeError.malformedResponse("non-object body") + if let response = try readV2Response(outPath: outPath) { + return try unwrapV2Response(response) } - let response = try BridgeResponse.parse(raw) - if response.success { - return response.data + // No response yet. If the request itself is gone from the inbox, the + // queue was cleared out from under us — `MessagesLauncher` wipes both + // queue directories when it relaunches Messages.app with the dylib, so a + // request that disappears without a reply can never be answered. Polling + // on to the deadline just burns the caller's full send timeout (2.5 + // minutes for sends) waiting for a reply that no longer has a writer. + // + // A request in normal flight is still on disk: unclaimed as + // `.json`, or claimed by the dylib as `.processing.`. + if !requestStillQueued(inboxDir: inboxDir, id: id) { + // Re-check the outbox once: the dylib removes its claim and writes the + // reply as two separate steps, so a reply may have landed in between. + if let response = try readV2Response(outPath: outPath) { + return try unwrapV2Response(response) + } + throw IMsgBridgeError.bridgeNotReady( + "request for '\(action.rawValue)' was discarded before it was processed " + + "(Messages.app restarted or the bridge queue was cleared)" + ) } - throw IMsgBridgeError.dylibReturnedError(response.error ?? "unknown") } try? FileManager.default.removeItem(atPath: final) throw IMsgBridgeError.timeout(action: action.rawValue) } + /// Read and consume a v2 reply if one is present. + private func readV2Response(outPath: String) throws -> BridgeResponse? { + guard + let data = try? Data(contentsOf: URL(fileURLWithPath: outPath)), + data.count > 1 + else { return nil } + // Best-effort cleanup; ignore failures (dylib may also unlink). + try? FileManager.default.removeItem(atPath: outPath) + + guard + let raw = try? JSONSerialization.jsonObject(with: data, options: []) + as? [String: Any] + else { + throw IMsgBridgeError.malformedResponse("non-object body") + } + return try BridgeResponse.parse(raw) + } + + private func unwrapV2Response(_ response: BridgeResponse) throws -> [String: Any] { + if response.success { + return response.data + } + throw IMsgBridgeError.dylibReturnedError(response.error ?? "unknown") + } + + /// Whether the request is still on disk awaiting (or under) processing. + /// + /// The dylib claims a request by renaming `.json` to + /// `.processing.` (see `processV2InboxFile`), so both shapes mean + /// the request is still live. Neither present means it was removed by + /// something other than a completed reply. + func requestStillQueued(inboxDir: String, id: String) -> Bool { + let fm = FileManager.default + if fm.fileExists(atPath: (inboxDir as NSString).appendingPathComponent("\(id).json")) { + return true + } + guard let entries = try? fm.contentsOfDirectory(atPath: inboxDir) else { + // Cannot enumerate: assume still queued rather than failing a live request. + return true + } + return entries.contains { $0.hasPrefix("\(id).processing.") } + } + // MARK: - Legacy path private func invokeLegacy( diff --git a/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift new file mode 100644 index 00000000..05e5b59d --- /dev/null +++ b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift @@ -0,0 +1,75 @@ +import Foundation +import Testing + +@testable import IMsgCore + +/// A v2 request that vanishes from the inbox without a reply can never be +/// answered: `MessagesLauncher` wipes both queue directories when it relaunches +/// Messages.app with the dylib. These cover the three on-disk shapes the poll +/// loop distinguishes so a live request is never mistaken for a discarded one. +@Suite("IMsgBridgeClient queue detection") +struct IMsgBridgeClientQueueTests { + private func makeInbox() throws -> String { + let dir = (NSTemporaryDirectory() as NSString) + .appendingPathComponent("imsg-queue-tests-\(UUID().uuidString)") + try FileManager.default.createDirectory( + atPath: dir, withIntermediateDirectories: true + ) + return dir + } + + private var client: IMsgBridgeClient { + IMsgBridgeClient(launcher: MessagesLauncher.shared) + } + + @Test + func unclaimedRequestCountsAsQueued() throws { + let inbox = try makeInbox() + defer { try? FileManager.default.removeItem(atPath: inbox) } + let id = UUID().uuidString + FileManager.default.createFile( + atPath: (inbox as NSString).appendingPathComponent("\(id).json"), + contents: Data("{}".utf8) + ) + + #expect(client.requestStillQueued(inboxDir: inbox, id: id) == true) + } + + @Test + func claimedRequestCountsAsQueued() throws { + let inbox = try makeInbox() + defer { try? FileManager.default.removeItem(atPath: inbox) } + let id = UUID().uuidString + // The dylib claims a request by renaming it to `.processing.` + // (see processV2InboxFile); that is still in flight, not discarded. + FileManager.default.createFile( + atPath: (inbox as NSString).appendingPathComponent("\(id).processing.4242"), + contents: Data("{}".utf8) + ) + + #expect(client.requestStillQueued(inboxDir: inbox, id: id) == true) + } + + @Test + func missingRequestIsNotQueued() throws { + let inbox = try makeInbox() + defer { try? FileManager.default.removeItem(atPath: inbox) } + // An unrelated request must not keep ours alive. + FileManager.default.createFile( + atPath: (inbox as NSString).appendingPathComponent("\(UUID().uuidString).json"), + contents: Data("{}".utf8) + ) + + #expect(client.requestStillQueued(inboxDir: inbox, id: UUID().uuidString) == false) + } + + @Test + func unreadableInboxFailsSafeAsQueued() { + // Cannot enumerate: keep waiting rather than aborting a live request. + #expect( + client.requestStillQueued( + inboxDir: "/nonexistent/imsg-queue-tests", id: UUID().uuidString + ) == true + ) + } +} From 1f87ec96563e4a620c13a8bb7b44045dafcbceb4 Mon Sep 17 00:00:00 2001 From: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:08:20 -0700 Subject: [PATCH 2/3] fix(bridge): do not report a claimed-then-vanished request as retry-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review raised that a vanished request can be reported as definitely-not- delivered even though the action may already have run. The stated mechanism (claim removed before the reply is written) is inverted — processV2InboxFile renames the reply into the outbox and only then drops the claim — but the concern is real through a different path: if the dylib dies after IMCore delivered and before the reply is published, the claim is orphaned and a later scan or relaunch clears it, exactly as that function's own comment describes. The defect was in the error semantics, not the ordering. Both situations collapsed into `.bridgeNotReady`, which the PR documents as retry-safe. Split them by tracking whether the dylib ever claimed the request: - never claimed, then absent -> nothing read it, so the action did not run. Still `.bridgeNotReady`, still retry-safe. - claimed, then absent with no reply -> the dylib had it and died mid-flight. New `.deliveryUnknown(action:)`, which callers must not retry blind. `requestStillQueued` becomes `requestQueueState` returning unclaimed/claimed/absent. An inbox that cannot be enumerated still reports unclaimed so a transient read error never ends a live request. Adds the interleaving test the review asked for, walking one request through unclaimed -> absent and another through unclaimed -> claimed -> absent, plus a guard that the two error cases stay distinct. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LEKzjCwQqoMEbZwabsY1u6 --- Sources/IMsgCore/IMsgBridgeClient.swift | 63 +++++++++---- Sources/IMsgCore/IMsgBridgeProtocol.swift | 12 +++ .../IMsgBridgeClientQueueTests.swift | 92 ++++++++++++++----- 3 files changed, 127 insertions(+), 40 deletions(-) diff --git a/Sources/IMsgCore/IMsgBridgeClient.swift b/Sources/IMsgCore/IMsgBridgeClient.swift index 82dd4f29..fa16001e 100644 --- a/Sources/IMsgCore/IMsgBridgeClient.swift +++ b/Sources/IMsgCore/IMsgBridgeClient.swift @@ -95,6 +95,10 @@ public final class IMsgBridgeClient: @unchecked Sendable { try FileManager.default.moveItem(atPath: tmp, toPath: final) let deadline = Date().addingTimeInterval(timeout) + // Whether the dylib ever claimed this request. It decides which error a + // vanished request produces, because the two cases differ in whether the + // action can already have run. + var wasClaimed = false while Date() < deadline { try await Task.sleep(nanoseconds: UInt64(pollInterval * 1_000_000_000)) if let response = try readV2Response(outPath: outPath) { @@ -109,16 +113,31 @@ public final class IMsgBridgeClient: @unchecked Sendable { // // A request in normal flight is still on disk: unclaimed as // `.json`, or claimed by the dylib as `.processing.`. - if !requestStillQueued(inboxDir: inboxDir, id: id) { - // Re-check the outbox once: the dylib removes its claim and writes the - // reply as two separate steps, so a reply may have landed in between. + switch requestQueueState(inboxDir: inboxDir, id: id) { + case .unclaimed: + continue + case .claimed: + wasClaimed = true + continue + case .absent: + // Re-check the outbox once: the reply is renamed into place before the + // claim is dropped, so a reply may have landed between our two checks. if let response = try readV2Response(outPath: outPath) { return try unwrapV2Response(response) } - throw IMsgBridgeError.bridgeNotReady( - "request for '\(action.rawValue)' was discarded before it was processed " - + "(Messages.app restarted or the bridge queue was cleared)" - ) + // Never claimed: nothing read the request, so the action did not run + // and the caller can safely retry. + guard wasClaimed else { + throw IMsgBridgeError.bridgeNotReady( + "request for '\(action.rawValue)' was discarded before it was processed " + + "(Messages.app restarted or the bridge queue was cleared)" + ) + } + // Claimed and then vanished with no reply. processV2InboxFile renames + // the reply into the outbox before removing the claim, so this means + // the dylib died mid-request: the action may already have taken + // effect. Report the outcome as unknown so nothing retries it blind. + throw IMsgBridgeError.deliveryUnknown(action: action.rawValue) } } @@ -151,22 +170,34 @@ public final class IMsgBridgeClient: @unchecked Sendable { throw IMsgBridgeError.dylibReturnedError(response.error ?? "unknown") } - /// Whether the request is still on disk awaiting (or under) processing. + /// On-disk state of a request still awaiting a reply. + enum RequestQueueState: Equatable { + /// `.json` is present; nothing has read it yet. + case unclaimed + /// `.processing.` is present; the dylib is handling it. + case claimed + /// Neither is present, so it was removed by something other than a + /// completed reply. + case absent + } + + /// Classify the request's on-disk state. /// /// The dylib claims a request by renaming `.json` to - /// `.processing.` (see `processV2InboxFile`), so both shapes mean - /// the request is still live. Neither present means it was removed by - /// something other than a completed reply. - func requestStillQueued(inboxDir: String, id: String) -> Bool { + /// `.processing.` (see `processV2InboxFile`). The distinction + /// matters because only a never-claimed request is guaranteed not to have + /// run. + func requestQueueState(inboxDir: String, id: String) -> RequestQueueState { let fm = FileManager.default if fm.fileExists(atPath: (inboxDir as NSString).appendingPathComponent("\(id).json")) { - return true + return .unclaimed } guard let entries = try? fm.contentsOfDirectory(atPath: inboxDir) else { - // Cannot enumerate: assume still queued rather than failing a live request. - return true + // Cannot enumerate: treat as still queued rather than ending a live + // request on a transient read error. + return .unclaimed } - return entries.contains { $0.hasPrefix("\(id).processing.") } + return entries.contains { $0.hasPrefix("\(id).processing.") } ? .claimed : .absent } // MARK: - Legacy path diff --git a/Sources/IMsgCore/IMsgBridgeProtocol.swift b/Sources/IMsgCore/IMsgBridgeProtocol.swift index adedaf26..3c1b86aa 100644 --- a/Sources/IMsgCore/IMsgBridgeProtocol.swift +++ b/Sources/IMsgCore/IMsgBridgeProtocol.swift @@ -136,6 +136,14 @@ public enum BridgeReactionKind: String, Sendable, CaseIterable { public enum IMsgBridgeError: Error, CustomStringConvertible, Equatable { case bridgeNotReady(String) case timeout(action: String) + /// The request was claimed by the dylib and then vanished without a reply. + /// + /// Distinct from `bridgeNotReady`, which means the request was removed + /// before anything claimed it and therefore definitely did not run. Here the + /// action may have completed — the dylib publishes its reply before dropping + /// the claim, but a crash in between leaves an orphaned claim that a later + /// scan or relaunch clears. Callers must not treat this as safe to retry. + case deliveryUnknown(action: String) case malformedResponse(String) case dylibReturnedError(String) case ioError(String) @@ -144,6 +152,10 @@ public enum IMsgBridgeError: Error, CustomStringConvertible, Equatable { switch self { case .bridgeNotReady(let detail): return "imsg bridge not ready: \(detail)" case .timeout(let action): return "Timed out waiting for response to '\(action)'" + case .deliveryUnknown(let action): + return + "Request for '\(action)' was claimed but disappeared without a reply; " + + "delivery is unknown and it must not be retried automatically" case .malformedResponse(let detail): return "Malformed bridge response: \(detail)" case .dylibReturnedError(let msg): return "Dylib error: \(msg)" case .ioError(let detail): return "Bridge IO error: \(detail)" diff --git a/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift index 05e5b59d..d7452669 100644 --- a/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift +++ b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift @@ -5,8 +5,9 @@ import Testing /// A v2 request that vanishes from the inbox without a reply can never be /// answered: `MessagesLauncher` wipes both queue directories when it relaunches -/// Messages.app with the dylib. These cover the three on-disk shapes the poll -/// loop distinguishes so a live request is never mistaken for a discarded one. +/// Messages.app with the dylib. These cover the on-disk shapes the poll loop +/// distinguishes, so a live request is never mistaken for a discarded one and a +/// possibly-delivered request is never reported as safe to retry. @Suite("IMsgBridgeClient queue detection") struct IMsgBridgeClientQueueTests { private func makeInbox() throws -> String { @@ -18,58 +19,101 @@ struct IMsgBridgeClientQueueTests { return dir } + private func write(_ dir: String, _ name: String) { + FileManager.default.createFile( + atPath: (dir as NSString).appendingPathComponent(name), + contents: Data("{}".utf8) + ) + } + private var client: IMsgBridgeClient { IMsgBridgeClient(launcher: MessagesLauncher.shared) } @Test - func unclaimedRequestCountsAsQueued() throws { + func unclaimedRequestIsPending() throws { let inbox = try makeInbox() defer { try? FileManager.default.removeItem(atPath: inbox) } let id = UUID().uuidString - FileManager.default.createFile( - atPath: (inbox as NSString).appendingPathComponent("\(id).json"), - contents: Data("{}".utf8) - ) + write(inbox, "\(id).json") - #expect(client.requestStillQueued(inboxDir: inbox, id: id) == true) + #expect(client.requestQueueState(inboxDir: inbox, id: id) == .unclaimed) } @Test - func claimedRequestCountsAsQueued() throws { + func claimedRequestIsClaimed() throws { let inbox = try makeInbox() defer { try? FileManager.default.removeItem(atPath: inbox) } let id = UUID().uuidString // The dylib claims a request by renaming it to `.processing.` // (see processV2InboxFile); that is still in flight, not discarded. - FileManager.default.createFile( - atPath: (inbox as NSString).appendingPathComponent("\(id).processing.4242"), - contents: Data("{}".utf8) - ) + write(inbox, "\(id).processing.4242") - #expect(client.requestStillQueued(inboxDir: inbox, id: id) == true) + #expect(client.requestQueueState(inboxDir: inbox, id: id) == .claimed) } @Test - func missingRequestIsNotQueued() throws { + func missingRequestIsAbsent() throws { let inbox = try makeInbox() defer { try? FileManager.default.removeItem(atPath: inbox) } // An unrelated request must not keep ours alive. - FileManager.default.createFile( - atPath: (inbox as NSString).appendingPathComponent("\(UUID().uuidString).json"), - contents: Data("{}".utf8) - ) + write(inbox, "\(UUID().uuidString).json") - #expect(client.requestStillQueued(inboxDir: inbox, id: UUID().uuidString) == false) + #expect(client.requestQueueState(inboxDir: inbox, id: UUID().uuidString) == .absent) } @Test - func unreadableInboxFailsSafeAsQueued() { - // Cannot enumerate: keep waiting rather than aborting a live request. + func unreadableInboxFailsSafeAsPending() { + // Cannot enumerate: keep waiting rather than ending a live request. #expect( - client.requestStillQueued( + client.requestQueueState( inboxDir: "/nonexistent/imsg-queue-tests", id: UUID().uuidString - ) == true + ) == .unclaimed + ) + } + + /// The interleaving that decides which error a vanished request produces. + /// + /// Unclaimed then absent means nothing ever read the request, so the action + /// cannot have run. Claimed then absent means the dylib had it and died + /// before publishing a reply, so the action may already have taken effect — + /// the caller must not retry that one. + @Test + func claimedThenAbsentIsDistinguishableFromNeverClaimed() throws { + let inbox = try makeInbox() + defer { try? FileManager.default.removeItem(atPath: inbox) } + + let neverClaimed = UUID().uuidString + write(inbox, "\(neverClaimed).json") + #expect(client.requestQueueState(inboxDir: inbox, id: neverClaimed) == .unclaimed) + try FileManager.default.removeItem( + atPath: (inbox as NSString).appendingPathComponent("\(neverClaimed).json") + ) + #expect(client.requestQueueState(inboxDir: inbox, id: neverClaimed) == .absent) + + let claimed = UUID().uuidString + write(inbox, "\(claimed).json") + #expect(client.requestQueueState(inboxDir: inbox, id: claimed) == .unclaimed) + // Dylib claims it. + try FileManager.default.moveItem( + atPath: (inbox as NSString).appendingPathComponent("\(claimed).json"), + toPath: (inbox as NSString).appendingPathComponent("\(claimed).processing.99") + ) + #expect(client.requestQueueState(inboxDir: inbox, id: claimed) == .claimed) + // Dylib dies; a later scan or relaunch clears the orphaned claim. + try FileManager.default.removeItem( + atPath: (inbox as NSString).appendingPathComponent("\(claimed).processing.99") + ) + #expect(client.requestQueueState(inboxDir: inbox, id: claimed) == .absent) + } + + @Test + func deliveryUnknownIsNotBridgeNotReady() { + // Callers key retry-safety off the case, so these must not be conflated. + #expect(IMsgBridgeError.deliveryUnknown(action: "send-message") != .bridgeNotReady("x")) + #expect( + IMsgBridgeError.deliveryUnknown(action: "send-message").description + .contains("must not be retried") ) } } From 5c63215b1d5f3dcc920a160ef8ea568995e61910 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 1 Aug 2026 22:38:01 -0700 Subject: [PATCH 3/3] fix: avoid retry-safe bridge race classification --- Sources/IMsgCore/IMsgBridgeClient.swift | 24 +++++-------------- Sources/IMsgCore/IMsgBridgeProtocol.swift | 12 ---------- .../IMsgBridgeClientQueueTests.swift | 22 ++++------------- 3 files changed, 10 insertions(+), 48 deletions(-) diff --git a/Sources/IMsgCore/IMsgBridgeClient.swift b/Sources/IMsgCore/IMsgBridgeClient.swift index fa16001e..d5c83379 100644 --- a/Sources/IMsgCore/IMsgBridgeClient.swift +++ b/Sources/IMsgCore/IMsgBridgeClient.swift @@ -95,10 +95,6 @@ public final class IMsgBridgeClient: @unchecked Sendable { try FileManager.default.moveItem(atPath: tmp, toPath: final) let deadline = Date().addingTimeInterval(timeout) - // Whether the dylib ever claimed this request. It decides which error a - // vanished request produces, because the two cases differ in whether the - // action can already have run. - var wasClaimed = false while Date() < deadline { try await Task.sleep(nanoseconds: UInt64(pollInterval * 1_000_000_000)) if let response = try readV2Response(outPath: outPath) { @@ -117,7 +113,6 @@ public final class IMsgBridgeClient: @unchecked Sendable { case .unclaimed: continue case .claimed: - wasClaimed = true continue case .absent: // Re-check the outbox once: the reply is renamed into place before the @@ -125,19 +120,12 @@ public final class IMsgBridgeClient: @unchecked Sendable { if let response = try readV2Response(outPath: outPath) { return try unwrapV2Response(response) } - // Never claimed: nothing read the request, so the action did not run - // and the caller can safely retry. - guard wasClaimed else { - throw IMsgBridgeError.bridgeNotReady( - "request for '\(action.rawValue)' was discarded before it was processed " - + "(Messages.app restarted or the bridge queue was cleared)" - ) - } - // Claimed and then vanished with no reply. processV2InboxFile renames - // the reply into the outbox before removing the claim, so this means - // the dylib died mid-request: the action may already have taken - // effect. Report the outcome as unknown so nothing retries it blind. - throw IMsgBridgeError.deliveryUnknown(action: action.rawValue) + // A claim can be created, acted on, and removed between two polls. An + // absent request therefore never proves that the action did not run, + // even when this client did not observe the claimed state. Use the + // existing timeout case so callers cannot mistake this for a + // retry-safe bridge-not-ready failure. + throw IMsgBridgeError.timeout(action: action.rawValue) } } diff --git a/Sources/IMsgCore/IMsgBridgeProtocol.swift b/Sources/IMsgCore/IMsgBridgeProtocol.swift index 3c1b86aa..adedaf26 100644 --- a/Sources/IMsgCore/IMsgBridgeProtocol.swift +++ b/Sources/IMsgCore/IMsgBridgeProtocol.swift @@ -136,14 +136,6 @@ public enum BridgeReactionKind: String, Sendable, CaseIterable { public enum IMsgBridgeError: Error, CustomStringConvertible, Equatable { case bridgeNotReady(String) case timeout(action: String) - /// The request was claimed by the dylib and then vanished without a reply. - /// - /// Distinct from `bridgeNotReady`, which means the request was removed - /// before anything claimed it and therefore definitely did not run. Here the - /// action may have completed — the dylib publishes its reply before dropping - /// the claim, but a crash in between leaves an orphaned claim that a later - /// scan or relaunch clears. Callers must not treat this as safe to retry. - case deliveryUnknown(action: String) case malformedResponse(String) case dylibReturnedError(String) case ioError(String) @@ -152,10 +144,6 @@ public enum IMsgBridgeError: Error, CustomStringConvertible, Equatable { switch self { case .bridgeNotReady(let detail): return "imsg bridge not ready: \(detail)" case .timeout(let action): return "Timed out waiting for response to '\(action)'" - case .deliveryUnknown(let action): - return - "Request for '\(action)' was claimed but disappeared without a reply; " - + "delivery is unknown and it must not be retried automatically" case .malformedResponse(let detail): return "Malformed bridge response: \(detail)" case .dylibReturnedError(let msg): return "Dylib error: \(msg)" case .ioError(let detail): return "Bridge IO error: \(detail)" diff --git a/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift index d7452669..4250b870 100644 --- a/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift +++ b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift @@ -6,8 +6,7 @@ import Testing /// A v2 request that vanishes from the inbox without a reply can never be /// answered: `MessagesLauncher` wipes both queue directories when it relaunches /// Messages.app with the dylib. These cover the on-disk shapes the poll loop -/// distinguishes, so a live request is never mistaken for a discarded one and a -/// possibly-delivered request is never reported as safe to retry. +/// distinguishes, so a live request is never mistaken for a discarded one. @Suite("IMsgBridgeClient queue detection") struct IMsgBridgeClientQueueTests { private func makeInbox() throws -> String { @@ -72,14 +71,10 @@ struct IMsgBridgeClientQueueTests { ) } - /// The interleaving that decides which error a vanished request produces. - /// - /// Unclaimed then absent means nothing ever read the request, so the action - /// cannot have run. Claimed then absent means the dylib had it and died - /// before publishing a reply, so the action may already have taken effect — - /// the caller must not retry that one. + /// Both observable paths can end absent. The client must not infer delivery + /// safety from whether it happened to observe the short-lived claim state. @Test - func claimedThenAbsentIsDistinguishableFromNeverClaimed() throws { + func absentStateDoesNotEncodeClaimHistory() throws { let inbox = try makeInbox() defer { try? FileManager.default.removeItem(atPath: inbox) } @@ -107,13 +102,4 @@ struct IMsgBridgeClientQueueTests { #expect(client.requestQueueState(inboxDir: inbox, id: claimed) == .absent) } - @Test - func deliveryUnknownIsNotBridgeNotReady() { - // Callers key retry-safety off the case, so these must not be conflated. - #expect(IMsgBridgeError.deliveryUnknown(action: "send-message") != .bridgeNotReady("x")) - #expect( - IMsgBridgeError.deliveryUnknown(action: "send-message").description - .contains("must not be retried") - ) - } }