diff --git a/CHANGELOG.md b/CHANGELOG.md index 3830eb4..d03b81b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### JSON-RPC - fix: let non-interactive RPC startup proceed without a Contacts prompt while rejecting ambiguous name targets when Contacts is unavailable (#186, #187, thanks @SebTardif). +- fix: fail vanished bridge queue requests immediately without treating an unobserved claim as safe to retry, avoiding long stalls and duplicate sends (#199, thanks @omarshahine). ### Reliability - fix: bound osascript send, reaction, and helper-process waits with process-tree cleanup so stalled subprocesses cannot hang CLI or RPC work (#197, thanks @SebTardif). diff --git a/Sources/IMsgCore/IMsgBridgeClient.swift b/Sources/IMsgCore/IMsgBridgeClient.swift index 10dc261..d5c8337 100644 --- a/Sources/IMsgCore/IMsgBridgeClient.swift +++ b/Sources/IMsgCore/IMsgBridgeClient.swift @@ -97,30 +97,97 @@ 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.`. + switch requestQueueState(inboxDir: inboxDir, id: id) { + case .unclaimed: + continue + case .claimed: + 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) + } + // 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) } - 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") + } + + /// 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`). 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 .unclaimed + } + guard let entries = try? fm.contentsOfDirectory(atPath: inboxDir) else { + // 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.") } ? .claimed : .absent + } + // MARK: - Legacy path private func invokeLegacy( diff --git a/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift new file mode 100644 index 0000000..4250b87 --- /dev/null +++ b/Tests/IMsgCoreTests/IMsgBridgeClientQueueTests.swift @@ -0,0 +1,105 @@ +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 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 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 unclaimedRequestIsPending() throws { + let inbox = try makeInbox() + defer { try? FileManager.default.removeItem(atPath: inbox) } + let id = UUID().uuidString + write(inbox, "\(id).json") + + #expect(client.requestQueueState(inboxDir: inbox, id: id) == .unclaimed) + } + + @Test + 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. + write(inbox, "\(id).processing.4242") + + #expect(client.requestQueueState(inboxDir: inbox, id: id) == .claimed) + } + + @Test + func missingRequestIsAbsent() throws { + let inbox = try makeInbox() + defer { try? FileManager.default.removeItem(atPath: inbox) } + // An unrelated request must not keep ours alive. + write(inbox, "\(UUID().uuidString).json") + + #expect(client.requestQueueState(inboxDir: inbox, id: UUID().uuidString) == .absent) + } + + @Test + func unreadableInboxFailsSafeAsPending() { + // Cannot enumerate: keep waiting rather than ending a live request. + #expect( + client.requestQueueState( + inboxDir: "/nonexistent/imsg-queue-tests", id: UUID().uuidString + ) == .unclaimed + ) + } + + /// 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 absentStateDoesNotEncodeClaimHistory() 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) + } + +}