From 72e5d1b3e6acf0af1e1a5b05be557e41c63aee58 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 02:34:36 -0500 Subject: [PATCH 1/5] Let the homeowner ask again after an answer House Scan can't use An answer that doesn't decode or doesn't name the scene sent (ResultBinding) used to show "We couldn't check this scan" with "Back to review", as if the marks were wrong, and offered no way to send the same scan again. It now has its own upload state: the answer is never shown, the scan is kept, and only the homeowner's "Try again" asks again. From the second such answer the screen says asking again might not help and points to sharing the scan or starting over. A 4xx refusal still goes back to the review. A reply that isn't HTTP counts as an unreachable server, so the words never say a server answered when none did. The transport moves into HouseScanKit (PlacementHTTPClient) unchanged, so the package tests drive it through URLSession against a server on 127.0.0.1. A UI test runs the real engine against the same kind of server: the first answer names another scene, and the test checks that nothing more is sent and nothing reaches a later screen until Try again. Co-Authored-By: Claude Opus 5.5 (1M context) --- ios/HouseScan/Contract/ScanContract.swift | 10 +- ios/HouseScan/Runtime/ResultClient.swift | 96 +++----- .../Runtime/ScanEngine+Actions.swift | 7 +- ios/HouseScan/Runtime/ScanEngine.swift | 12 +- ios/HouseScan/UI/Copy/ScanCopy.swift | 9 +- ios/HouseScan/UI/Preview/DemoEngine.swift | 14 +- ios/HouseScan/UI/Preview/UIDemo.swift | 2 + .../UI/Screens/UploadingScreen.swift | 64 +++++- .../Upload/PlacementHTTPClient.swift | 80 +++++++ .../Upload/UploadFailureKind.swift | 27 ++- .../LoopbackPlacementServer.swift | 83 +++++++ .../PlacementHTTPClientTests.swift | 120 ++++++++++ .../ResultBindingTests.swift | 4 +- .../UploadFailureKindTests.swift | 17 +- .../UploadRecoveryUITests.swift | 212 ++++++++++++++++++ 15 files changed, 661 insertions(+), 96 deletions(-) create mode 100644 ios/HouseScanKit/Sources/HouseScanKit/Upload/PlacementHTTPClient.swift create mode 100644 ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift create mode 100644 ios/HouseScanKit/Tests/HouseScanKitTests/PlacementHTTPClientTests.swift create mode 100644 ios/HouseScanUITests/UploadRecoveryUITests.swift diff --git a/ios/HouseScan/Contract/ScanContract.swift b/ios/HouseScan/Contract/ScanContract.swift index 705c3e30e..8b09e4391 100644 --- a/ios/HouseScan/Contract/ScanContract.swift +++ b/ios/HouseScan/Contract/ScanContract.swift @@ -627,10 +627,14 @@ enum UploadState: Equatable, Sendable { /// A network failure or a server error (5xx): sending again can work. `offline` means no /// connection at all. case failed(message: String, offline: Bool) - /// The server refused the scan (4xx), its answer couldn't be read, or the scan couldn't be - /// packaged. Sending again would send the same thing, so the way on is back to the review - /// or start over, never "Try again". + /// The server refused the scan (4xx) or the scan couldn't be packaged. Sending again would send + /// the same thing, so the way on is back to the review or start over, never "Try again". case rejected(message: String) + /// The server answered, but House Scan couldn't use the answer: it didn't decode, or it didn't + /// name the scene sent. It isn't shown. The scan isn't at fault, so the way on is "Try again", + /// with sharing the scan or starting over for when asking again doesn't help. `attempts` counts + /// such answers since the scan was sent from the review or a gap, at least 1. + case unusableAnswer(attempts: Int) case done } diff --git a/ios/HouseScan/Runtime/ResultClient.swift b/ios/HouseScan/Runtime/ResultClient.swift index 567884fc2..e363889b8 100644 --- a/ios/HouseScan/Runtime/ResultClient.swift +++ b/ios/HouseScan/Runtime/ResultClient.swift @@ -9,59 +9,29 @@ protocol ResultClient: AnyObject { func submit(scene: Data, progress: @escaping @Sendable (Double) -> Void) async throws -> Data } -/// Posts scene.json to the placement server: `POST {serverURL}/v1/placements` with the JSON as -/// the body and `Content-Type: application/json`. Only the JSON goes: the server's solver reads no -/// photos and skips the image check for a bare scene.json, so the keyframes stay on the phone -/// (and in the scan folder's `scan.zip`, which leaves only if the homeowner shares it). The -/// response body is the result JSON, and it is returned only when it names the scene sent: its -/// `stats.input_sha256` must be present and be the hash of these exact bytes (`ResultBinding`). -/// Any other answer throws, and the upload screen treats it as an answer it couldn't read. +/// Posts scene.json to the placement server and returns the answer only when it names the scene +/// sent (HouseScanKit's `PlacementHTTPClient`, which the package tests drive over real HTTP). Only +/// the JSON goes: the keyframes stay on the phone, and in the scan folder's `scan.zip`, which +/// leaves only if the homeowner shares it. @MainActor final class HTTPResultClient: ResultClient { let serverURL: URL let isSample = false + private let client: PlacementHTTPClient init(serverURL: URL) { self.serverURL = serverURL - } - - nonisolated static func makeRequest(serverURL: URL) -> URLRequest { - var request = URLRequest(url: serverURL.appending(path: "v1/placements")) - request.httpMethod = "POST" - request.setValue("application/json", forHTTPHeaderField: "Content-Type") - request.setValue("application/json", forHTTPHeaderField: "Accept") - request.timeoutInterval = 120 - return request + client = PlacementHTTPClient(serverURL: serverURL) } func submit(scene: Data, progress: @escaping @Sendable (Double) -> Void) async throws -> Data { - let request = Self.makeRequest(serverURL: serverURL) - let delegate = UploadProgressDelegate(progress: progress) - let (data, response) = try await URLSession.shared.upload(for: request, from: scene, delegate: delegate) - guard let http = response as? HTTPURLResponse else { throw UploadError.notHTTP } - guard (200..<300).contains(http.statusCode) else { - throw UploadError.server( - status: http.statusCode, body: String(decoding: data.prefix(300), as: UTF8.self), - retryAfter: http.value(forHTTPHeaderField: "Retry-After")) - } - try ResultBinding.check(answer: data, submittedScene: scene) - progress(1) - return data + try await client.submit(scene: scene, progress: progress) } } -enum UploadError: Error, CustomStringConvertible { - case notHTTP - case server(status: Int, body: String, retryAfter: String? = nil) - case missingSample - - var description: String { - switch self { - case .notHTTP: "The server's answer was not HTTP." - case .server(let status, let body, _): "The server answered \(status): \(body)" - case .missingSample: "SampleResult.json is missing from the app bundle." - } - } +/// The bundled sample is missing: a build problem, not something the server said. +struct MissingSampleResult: Error, CustomStringConvertible { + var description: String { "SampleResult.json is missing from the app bundle." } } /// What the homeowner reads about a failed upload. Which failures are worth sending again is @@ -69,13 +39,21 @@ enum UploadError: Error, CustomStringConvertible { /// log, and the screen never shows raw error text. enum UploadFailure { /// A failure while sending or reading the answer (not while packaging the scan). - static func state(for error: any Error) -> UploadState { - let kind: UploadFailureKind = if case .server(let status, _, let retryAfter) = error as? UploadError { - UploadFailureKind.classify(httpStatus: status, retryAfter: retryAfter) - } else { - UploadFailureKind.classify(error) + /// `unusableAnswers` counts the server's answers House Scan couldn't use since the scan was + /// sent from the review or a gap; this adds one when `error` is such an answer. + static func state(for error: any Error, sample: Bool, unusableAnswers: inout Int) -> UploadState { + // No server was asked: a missing or unreadable bundled sample is the build's problem, and + // the words must not say a server answered. + if sample { + return .rejected(message: "This build's sample result is missing or unreadable. Start over with a build that has a server.") } - return switch kind { + let kind = UploadFailureKind.classify(error) + if kind == .unreadableAnswer { unusableAnswers += 1 } + return state(for: kind, unusableAnswers: unusableAnswers) + } + + static func state(for kind: UploadFailureKind, unusableAnswers: Int) -> UploadState { + switch kind { case .offline: .failed(message: "Your phone isn't connected to the internet. Your scan is saved on this phone.", offline: true) case .unreachable: @@ -86,8 +64,9 @@ enum UploadFailure { .failed(message: busyMessage(retryAfter), offline: false) case .refused: .rejected(message: "The server couldn't use this scan. Go back to the review to check your marks, or start over.") + // The answer's problem, not the scan's: ask again, never "check your marks". case .unreadableAnswer: - .rejected(message: "We couldn't read the server's answer. Go back to the review and send it again, or start over.") + .unusableAnswer(attempts: max(unusableAnswers, 1)) } } @@ -118,27 +97,6 @@ enum UploadFailure { } } -final class UploadProgressDelegate: NSObject, URLSessionTaskDelegate, Sendable { - private let progress: @Sendable (Double) -> Void - - init(progress: @escaping @Sendable (Double) -> Void) { - self.progress = progress - } - - /// Once the whole body is sent the server is working out the spot in the same request, so - /// report 1 and let the screen move on to "Check clearances". Before that, stop at 0.99 so - /// rounding never shows 100%. Holding 0.99 until the answer left the screen on "Sending - /// measurements, 99%" for the whole analysis (field test run 1). - func urlSession(_ session: URLSession, task: URLSessionTask, didSendBodyData bytesSent: Int64, totalBytesSent: Int64, totalBytesExpectedToSend: Int64) { - guard totalBytesExpectedToSend > 0 else { return } - if totalBytesSent >= totalBytesExpectedToSend { - progress(1) - } else { - progress(min(0.99, Double(totalBytesSent) / Double(totalBytesExpectedToSend))) - } - } -} - /// Answers with the bundled SampleResult.json, for tests and demos without a server. The result /// is flagged `isSample` so the screen says it is not a real analysis. It answers no particular /// scene (its `input_sha256` is all zeros), so it is deliberately not checked with @@ -159,7 +117,7 @@ final class SampleResultClient: ResultClient { progress(Double(step) / 4) } try await Task.sleep(for: .seconds(pace)) - guard let url = Bundle.main.url(forResource: "SampleResult", withExtension: "json") else { throw UploadError.missingSample } + guard let url = Bundle.main.url(forResource: "SampleResult", withExtension: "json") else { throw MissingSampleResult() } return try Data(contentsOf: url) } } diff --git a/ios/HouseScan/Runtime/ScanEngine+Actions.swift b/ios/HouseScan/Runtime/ScanEngine+Actions.swift index a2f858dc2..144597dac 100644 --- a/ios/HouseScan/Runtime/ScanEngine+Actions.swift +++ b/ios/HouseScan/Runtime/ScanEngine+Actions.swift @@ -561,9 +561,14 @@ extension ScanEngine: ScanActions { } } + /// Sends the same scan again, only when the homeowner asks: after a network or server failure, + /// or an answer House Scan couldn't use. Never after a refusal, which the review has to fix. func retryUpload() { guard state.phase == .uploading else { return } - if case .failed = state.upload { startUpload() } + switch state.upload { + case .failed, .unusableAnswer: startUpload() + case .idle, .packaging, .uploading, .analyzing, .rejected, .done: break + } } /// Back to the feature review after a rejected upload. The scan (wall, coverage, keyframes, diff --git a/ios/HouseScan/Runtime/ScanEngine.swift b/ios/HouseScan/Runtime/ScanEngine.swift index 388f6e516..a52e61288 100644 --- a/ios/HouseScan/Runtime/ScanEngine.swift +++ b/ios/HouseScan/Runtime/ScanEngine.swift @@ -192,6 +192,9 @@ final class ScanEngine { private var uploadTask: Task? /// The current upload's scene has fixed its ground; keep this true through answer pacing. private var scenePackaged = false + /// The server's answers House Scan couldn't use since this scan was last sent from the review + /// or a gap; "Try again" keeps counting, and a usable answer starts again at 0. + private var unusableAnswers = 0 private(set) var placement: PlacementResult? /// Whether a ground change took the answer down and no answer has been shown since /// (`answerAfter(_:)`). @@ -514,7 +517,7 @@ final class ScanEngine { case .findMeter, .meterCloseUp, .wallWalk, .markFeatures, .gapRequest: true case .uploading: switch state.upload { - case .failed, .rejected: false + case .failed, .rejected, .unusableAnswer: false case .idle, .packaging, .uploading, .analyzing, .done: true } case .onboarding, .spotConfirm, .result, .resultAR, .unsupported: false @@ -1675,7 +1678,7 @@ final class ScanEngine { let screen: GroundFreshness.Screen = switch state.phase { case .uploading: switch state.upload { - case .failed, .rejected: .stopped + case .failed, .rejected, .unusableAnswer: .stopped case .idle, .packaging, .uploading, .analyzing, .done: .sending } case .spotConfirm: .spotCheck @@ -1999,6 +2002,8 @@ final class ScanEngine { func startUpload() { scenePackaged = false + // "Try again" sends from the upload screen; anything else is a new send of this scan. + if state.phase != .uploading { unusableAnswers = 0 } go(.uploading) injectGroundForTest() uploadTask?.cancel() @@ -2080,6 +2085,7 @@ final class ScanEngine { guard scan == generation, state.phase == .uploading, !Task.isCancelled else { return } } placement = result + unusableAnswers = 0 noteExchange(scene: scene, answer: data, packet: packaged.packet) writeScanStamp(answer: result) state.result = presentation(of: result, isSample: resultClient.isSample) @@ -2111,7 +2117,7 @@ final class ScanEngine { // failure the canceller already shows. guard scan == generation, !Task.isCancelled else { return } RuntimeLog.engine.error("upload failed: \(String(describing: error), privacy: .public)") - state.upload = UploadFailure.state(for: error) + state.upload = UploadFailure.state(for: error, sample: resultClient.isSample, unusableAnswers: &unusableAnswers) updateRecording() } } diff --git a/ios/HouseScan/UI/Copy/ScanCopy.swift b/ios/HouseScan/UI/Copy/ScanCopy.swift index 904b82fc8..3e438490f 100644 --- a/ios/HouseScan/UI/Copy/ScanCopy.swift +++ b/ios/HouseScan/UI/Copy/ScanCopy.swift @@ -563,7 +563,7 @@ enum ScanCopy { switch upload { case .idle, .packaging, .uploading, .analyzing: return Instruction(title: "Making a sample result", detail: "No server is connected, so nothing leaves this phone. The result you'll see is an example, not a check of your wall.") - case .failed, .rejected, .done: + case .failed, .rejected, .unusableAnswer, .done: break } } @@ -586,6 +586,13 @@ enum ScanCopy { // The engine's message is already in the homeowner's words and says why. case .rejected(let message): Instruction(title: "We couldn't check this scan", detail: message) + // The server did answer; what failed is the answer, so the words never point at the marks. + case .unusableAnswer(let attempts) where attempts > 1: + Instruction(title: "The answer still couldn't be used", detail: "Asking again might not fix this. It isn't about what you marked.") + case .unusableAnswer: + Instruction( + title: "We couldn't use the server's answer", + detail: "The server answered, but House Scan couldn't read the answer or match it to your scan, so it isn't shown. It isn't about what you marked.") case .done: Instruction(title: "Done", detail: nil) } diff --git a/ios/HouseScan/UI/Preview/DemoEngine.swift b/ios/HouseScan/UI/Preview/DemoEngine.swift index d5f913929..70b79c0d9 100644 --- a/ios/HouseScan/UI/Preview/DemoEngine.swift +++ b/ios/HouseScan/UI/Preview/DemoEngine.swift @@ -21,6 +21,8 @@ final class DemoEngine: ScanActions { /// through the engine's own mapping, in place of the hand-made samples. private let resultFile: String? private let rejectUpload: Bool + /// `-uiDemoUnusableAnswer `: how many answers in a row House Scan can't use; 0 when off. + private let unusableAnswers: Int /// Which request the gap screen shows (`-uiDemoGap`); the phone's ground request by default. private let gapKind: String? /// The tilt-up step was answered or skipped. @@ -51,6 +53,7 @@ final class DemoEngine: ScanActions { private var spotChecked = false private var failedUploads = 0 private var rejectedUploads = 0 + private var answersUnused = 0 private static let cellWidth: Float = 0.1524 /// Where the demo wall ends on each side, meters of s. Following a corner moves the end on. @@ -76,6 +79,7 @@ final class DemoEngine: ScanActions { resultFile = nil #endif rejectUpload = arguments.contains("-uiDemoRejected") + unusableAnswers = arguments.contains("-uiDemoUnusableAnswer") ? max(value("-uiDemoUnusableAnswer").flatMap(Int.init) ?? 1, 1) : 0 gapKind = value("-uiDemoGap") state.feed = DemoScene.image.map(CameraFeed.still) ?? .none state.isReplay = true @@ -199,11 +203,13 @@ final class DemoEngine: ScanActions { } // Frozen, the upload script never runs, so show where it would end. if freeze, state.phase == .uploading { - if rejectUpload || offline { + if rejectUpload || offline || unusableAnswers > 0 { state.shareableScan = Self.demoScan } if rejectUpload { state.upload = .rejected(message: Self.rejection) + } else if unusableAnswers > 0 { + state.upload = .unusableAnswer(attempts: unusableAnswers) } else if offline { state.upload = .failed(message: "No internet connection.", offline: true) } @@ -543,6 +549,12 @@ final class DemoEngine: ScanActions { state.upload = .rejected(message: Self.rejection) return } + // An answer the engine couldn't use; "Try again" asks again. + if answersUnused < unusableAnswers { + answersUnused += 1 + state.upload = .unusableAnswer(attempts: answersUnused) + return + } // Like the real engine: an answer that lists a view the camera can take goes back to // the camera for it, after the upload screen has said so; the next answer is the result. if !followedUp, let item = sample.missing.first(where: \.capturable) { diff --git a/ios/HouseScan/UI/Preview/UIDemo.swift b/ios/HouseScan/UI/Preview/UIDemo.swift index 3ee3c0a47..ecdf9e551 100644 --- a/ios/HouseScan/UI/Preview/UIDemo.swift +++ b/ios/HouseScan/UI/Preview/UIDemo.swift @@ -15,6 +15,8 @@ import SwiftUI /// question answered, so it shows as the folded row with Change. /// - `-uiDemoOffline`: uploads fail offline. /// - `-uiDemoRejected`: the server refuses the first upload; "Back to review" then sends it again. +/// - `-uiDemoUnusableAnswer `: the first `n` answers can't be used (1 if `n` is missing); +/// "Try again" sends the scan again, and the answer after them is the result. /// - `-uiDemoFailure `: open on the /// unsupported screen. /// - `-uiDemoPass`: the sample result is a pass with approved rules. diff --git a/ios/HouseScan/UI/Screens/UploadingScreen.swift b/ios/HouseScan/UI/Screens/UploadingScreen.swift index 9cf8b9d92..bee6c59de 100644 --- a/ios/HouseScan/UI/Screens/UploadingScreen.swift +++ b/ios/HouseScan/UI/Screens/UploadingScreen.swift @@ -2,9 +2,12 @@ import SwiftUI /// "Checking your wall": a calm wait. Three steps tick off in order; progress is shown /// separately from how complete the scan was (docs/05 section 2). A failure that sending again -/// can fix (offline, a server error) offers "Try again". A refused scan can't be fixed by sending -/// the same thing again, so it offers the review and a fresh start instead. Both offer "Share -/// scan" once the scan is packaged, so a scan the server never took can still reach the team. +/// can fix (offline, a server error) offers "Try again". So does an answer House Scan couldn't +/// use, which isn't the scan's fault; when that keeps happening the screen says asking again may +/// not help and points to sharing the scan or starting over. A refused scan can't be fixed by +/// sending the same thing again, so it offers the review and a fresh start instead. All of them +/// offer "Share scan" once the scan is packaged, so a scan the server never took can still reach +/// the team. /// /// When the check answers but still wants views the camera can take now, the engine goes /// straight back to the camera for them. This screen says so first ("One more view to @@ -65,6 +68,8 @@ struct UploadingScreen: View { ShareScanButton(url: scan) } } + case .unusableAnswer(let attempts): + UnusableAnswerActions(repeated: attempts > 1, shareableScan: state.shareableScan, actions: actions) case .rejected: VStack(spacing: 16) { Button("Back to review") { actions.backToReview() } @@ -94,6 +99,52 @@ struct UploadingScreen: View { } } +/// The ways on after an answer House Scan couldn't use. "Try again" leads the first time. Once it +/// has already failed again, it steps down to a secondary button and the note points to sharing +/// the scan, so the homeowner isn't left pressing the same button. Never "Back to review": the +/// marks aren't the problem. +private struct UnusableAnswerActions: View { + var repeated: Bool + var shareableScan: URL? + let actions: any ScanActions + + var body: some View { + VStack(spacing: 16) { + Text(note) + .font(Typeface.hint) + .foregroundStyle(Palette.muted) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + .accessibilityIdentifier("upload.answerNote") + Group { + if repeated { + Button("Try again") { actions.retryUpload() }.buttonStyle(.secondary) + } else { + Button("Try again") { actions.retryUpload() }.buttonStyle(.primary) + } + } + .accessibilityHint("Asks the server again for the same scan.") + .accessibilityIdentifier("action.retryUpload") + if let scan = shareableScan { + ShareScanButton(url: scan) + } + Button("Start over") { actions.startOver() } + .buttonStyle(.quiet) + .accessibilityHint("Starts a new scan and keeps your two most recent completed scans on this phone.") + .accessibilityIdentifier("action.startOver") + } + } + + private var note: String { + switch (repeated, shareableScan != nil) { + case (false, true): "Keep this scan open to try again, or use Share scan to save a copy. House Scan can't reopen it after you close the app." + case (false, false): "Keep this scan open to try again. House Scan can't reopen it after you close the app." + case (true, true): "Share scan saves a copy you can send to the House Scan team. House Scan can't reopen this scan after you close the app." + case (true, false): "House Scan can't reopen this scan after you close the app." + } + } +} + private struct UploadEmblem: View { var upload: UploadState var followsUp: Bool @@ -115,7 +166,7 @@ private struct UploadEmblem: View { private var isWorking: Bool { switch upload { - case .failed, .rejected, .done: false + case .failed, .rejected, .unusableAnswer, .done: false case .idle, .packaging, .uploading, .analyzing: true } } @@ -128,13 +179,14 @@ private struct UploadEmblem: View { case .analyzing: "ruler" case .failed(_, let offline): offline ? "wifi.slash" : "exclamationmark.triangle" case .rejected: "exclamationmark.triangle" + case .unusableAnswer: "exclamationmark.bubble" case .done: "checkmark.circle" } } private var tint: Color { switch upload { - case .failed, .rejected: Palette.caution + case .failed, .rejected, .unusableAnswer: Palette.caution case .idle, .packaging, .uploading, .analyzing, .done: Palette.signal } } @@ -173,7 +225,7 @@ private struct UploadSteps: View { case .uploading: 1 case .analyzing: 2 case .done: 3 - case .failed, .rejected: 1 + case .failed, .rejected, .unusableAnswer: 1 } } diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Upload/PlacementHTTPClient.swift b/ios/HouseScanKit/Sources/HouseScanKit/Upload/PlacementHTTPClient.swift new file mode 100644 index 000000000..b6ba2398a --- /dev/null +++ b/ios/HouseScanKit/Sources/HouseScanKit/Upload/PlacementHTTPClient.swift @@ -0,0 +1,80 @@ +import Foundation + +/// Sends a scan's scene.json to the placement server and returns the answer only when it is usable: +/// `POST {serverURL}/v1/placements` with the JSON as the body and `Content-Type: application/json`. +/// Only the JSON goes; the server's solver reads no photos for a bare scene.json, so the keyframes +/// stay on the phone. +/// +/// The answer is returned only when the reply is HTTP 2xx and names the scene sent: its +/// `stats.input_sha256` must be the hash of these exact bytes (`ResultBinding`). Every other +/// outcome throws, and `UploadFailureKind.classify` says what it means for the homeowner. +public struct PlacementHTTPClient: Sendable { + public let serverURL: URL + public let session: URLSession + + public init(serverURL: URL, session: URLSession = .shared) { + self.serverURL = serverURL + self.session = session + } + + public static func request(serverURL: URL) -> URLRequest { + var request = URLRequest(url: serverURL.appending(path: "v1/placements")) + request.httpMethod = "POST" + request.setValue("application/json", forHTTPHeaderField: "Content-Type") + request.setValue("application/json", forHTTPHeaderField: "Accept") + request.timeoutInterval = 120 + return request + } + + /// The answer's bytes. `progress` runs from 0 to 1 while the scene is sent, and reaches 1 once + /// the server has the whole body and is working out the spot. + public func submit(scene: Data, progress: @escaping @Sendable (Double) -> Void) async throws -> Data { + let delegate = PlacementUploadProgress(progress: progress) + let (data, response) = try await session.upload(for: Self.request(serverURL: serverURL), from: scene, delegate: delegate) + guard let http = response as? HTTPURLResponse else { throw PlacementHTTPError.notHTTP } + guard (200..<300).contains(http.statusCode) else { + throw PlacementHTTPError.server( + status: http.statusCode, body: String(decoding: data.prefix(300), as: UTF8.self), + retryAfter: http.value(forHTTPHeaderField: "Retry-After")) + } + try ResultBinding.check(answer: data, submittedScene: scene) + progress(1) + return data + } +} + +/// How sending the scene failed before there was an answer to read. +public enum PlacementHTTPError: Error, Equatable, CustomStringConvertible { + /// The response wasn't HTTP, so no server answered. + case notHTTP + /// The server answered with a status outside 2xx. `body` is the first 300 bytes, for the log. + case server(status: Int, body: String, retryAfter: String? = nil) + + public var description: String { + switch self { + case .notHTTP: "the response was not HTTP" + case .server(let status, let body, _): "the server answered \(status): \(body)" + } + } +} + +/// Reports upload progress. Once the whole body is sent the server is working out the spot in the +/// same request, so it reports 1 and the screen can move on to "Check clearances". Before that it +/// stops at 0.99, so rounding never shows 100%. Holding 0.99 until the answer left the screen on +/// "Sending measurements, 99%" for the whole analysis (field test run 1). +final class PlacementUploadProgress: NSObject, URLSessionTaskDelegate, Sendable { + private let progress: @Sendable (Double) -> Void + + init(progress: @escaping @Sendable (Double) -> Void) { + self.progress = progress + } + + func urlSession(_ session: URLSession, task: URLSessionTask, didSendBodyData bytesSent: Int64, totalBytesSent: Int64, totalBytesExpectedToSend: Int64) { + guard totalBytesExpectedToSend > 0 else { return } + if totalBytesSent >= totalBytesExpectedToSend { + progress(1) + } else { + progress(min(0.99, Double(totalBytesSent) / Double(totalBytesExpectedToSend))) + } + } +} diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Upload/UploadFailureKind.swift b/ios/HouseScanKit/Sources/HouseScanKit/Upload/UploadFailureKind.swift index d7a19f338..efd14d850 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Upload/UploadFailureKind.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Upload/UploadFailureKind.swift @@ -1,10 +1,11 @@ import Foundation -/// Why sending the scene failed, which decides whether sending it again can help. +/// Why sending the scene failed, which decides what the homeowner can do next. /// /// A network failure, a server error (5xx) or a server that is busy or timed out (408, 429) may -/// go away, so the homeowner can try again. Any other refusal (4xx) or an answer that doesn't -/// decode would come back the same for the same scene, so the way on is back to the review or +/// go away, so the homeowner can try again. So can an answer House Scan couldn't use: the problem +/// is in the answer, not the scan, and asking again may get a usable one. A refusal (4xx) is the +/// server saying this scene can't be checked as it is, so the way on is back to the review or /// starting over. public enum UploadFailureKind: Sendable, Equatable { /// No connection at all. @@ -19,14 +20,16 @@ public enum UploadFailureKind: Sendable, Equatable { case busy(retryAfter: Int?) /// The server answered 4xx: it refused this scene. case refused - /// The answer wasn't HTTP or didn't decode as a result. + /// The server answered, but House Scan can't use the answer: a 2xx that doesn't decode as a + /// result or doesn't name the scene sent (`ResultBinding`), or a status outside 2xx–5xx. case unreadableAnswer - /// Sending the same scene again can succeed. + /// The homeowner may send the same scene again: it can succeed. The app never does it on its + /// own; it offers "Try again". public var retryable: Bool { switch self { - case .offline, .unreachable, .serverError, .busy: true - case .refused, .unreadableAnswer: false + case .offline, .unreachable, .serverError, .busy, .unreadableAnswer: true + case .refused: false } } @@ -34,7 +37,15 @@ public enum UploadFailureKind: Sendable, Equatable { if let urlError = error as? URLError { return offlineCodes.contains(urlError.code) ? .offline : .unreachable } - return .unreadableAnswer + switch error as? PlacementHTTPError { + case .server(let status, _, let retryAfter)?: + return classify(httpStatus: status, retryAfter: retryAfter) + case .notHTTP?: + // No server answered, so nothing was refused or unreadable. + return .unreachable + case nil: + return .unreadableAnswer + } } /// The kind for an HTTP status outside 200...299. `retryAfter` is the answer's Retry-After diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift new file mode 100644 index 000000000..4c0f9d79d --- /dev/null +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift @@ -0,0 +1,83 @@ +import Foundation +import Network +import Synchronization + +/// A placement server on 127.0.0.1 for client tests: real HTTP, so the tests drive the actual +/// `URLSession` transport. Each request gets the reply `respond` builds from it, and every request +/// is kept for the test to inspect. It speaks only what these tests need: one request per +/// connection, a `Content-Length` body, and the reply closes the connection. +final class LoopbackPlacementServer: Sendable { + struct Request: Sendable { + var method: String + var path: String + var headers: [String: String] + var body: Data + } + + struct Reply: Sendable { + var status: Int + var headers: [String: String] = [:] + var body: Data + } + + private let listener: NWListener + private let queue = DispatchQueue(label: "loopback-placement-server") + private let respond: @Sendable (Request) -> Reply + private let log = Mutex<[Request]>([]) + + var requests: [Request] { log.withLock { $0 } } + var base: URL { URL(string: "http://127.0.0.1:\(listener.port?.rawValue ?? 0)")! } + + init(respond: @escaping @Sendable (Request) -> Reply) throws { + self.respond = respond + let parameters = NWParameters.tcp + parameters.acceptLocalOnly = true + parameters.requiredLocalEndpoint = .hostPort(host: "127.0.0.1", port: .any) + listener = try NWListener(using: parameters) + let ready = DispatchSemaphore(value: 0) + listener.stateUpdateHandler = { if case .ready = $0 { ready.signal() } } + listener.newConnectionHandler = { [queue] connection in + connection.start(queue: queue) + Self.receive(connection, buffer: Data()) { request in self.answer(request, on: connection) } + } + listener.start(queue: queue) + guard ready.wait(timeout: .now() + 5) == .success, listener.port != nil else { throw URLError(.cannotConnectToHost) } + } + + deinit { listener.cancel() } + + private func answer(_ request: Request, on connection: NWConnection) { + log.withLock { $0.append(request) } + let reply = respond(request) + var head = "HTTP/1.1 \(reply.status) Reply\r\nContent-Type: application/json\r\nContent-Length: \(reply.body.count)\r\nConnection: close\r\n" + for (name, value) in reply.headers { head += "\(name): \(value)\r\n" } + head += "\r\n" + connection.send(content: Data(head.utf8) + reply.body, completion: .contentProcessed { _ in connection.cancel() }) + } + + private static func receive(_ connection: NWConnection, buffer: Data, handler: @escaping @Sendable (Request) -> Void) { + connection.receive(minimumIncompleteLength: 1, maximumLength: 1 << 20) { data, _, done, error in + var buffer = buffer + if let data { buffer.append(data) } + if let request = parse(buffer) { return handler(request) } + if done || error != nil { return connection.cancel() } + receive(connection, buffer: buffer, handler: handler) + } + } + + private static func parse(_ data: Data) -> Request? { + guard let end = data.range(of: Data("\r\n\r\n".utf8)) else { return nil } + let head = String(decoding: data[..= 2 else { return nil } + var headers: [String: String] = [:] + for field in head.dropFirst() { + guard let colon = field.firstIndex(of: ":") else { continue } + headers[field[..= length else { return nil } + return Request(method: String(line[0]), path: String(line[1]), headers: headers, body: Data(body.prefix(length))) + } +} diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/PlacementHTTPClientTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/PlacementHTTPClientTests.swift new file mode 100644 index 000000000..f43282734 --- /dev/null +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/PlacementHTTPClientTests.swift @@ -0,0 +1,120 @@ +import Foundation +import HouseScanKit +import Synchronization +import Testing + +/// The placement client against a real HTTP server on 127.0.0.1, through `URLSession`: what it sends, +/// which replies it returns, and what each failure means for the homeowner +/// (`UploadFailureKind`). The answers are a real server answer to the synthetic replay, with its +/// input hash set per test. +@Suite struct PlacementHTTPClientTests { + static let scene = Data(#"{"note":"the scene this phone sent"}"#.utf8) + static let fixtureHash = "2cb165cd4d4e9340496d90a589678ccbbe8ae6720d9d73bcae427a6b22075cb7" + + static func answer(declaring hash: String?) throws -> Data { + let url = URL(fileURLWithPath: #filePath).deletingLastPathComponent().appendingPathComponent("Schemas/server-answer-synthetic-wall.json") + let text = String(decoding: try Data(contentsOf: url), as: UTF8.self) + let line = "\"input_sha256\": \"\(fixtureHash)\"," + #expect(text.contains(line)) + return Data(text.replacingOccurrences(of: line, with: hash.map { "\"input_sha256\": \"\($0)\"," } ?? "").utf8) + } + + static func client(_ server: LoopbackPlacementServer) -> PlacementHTTPClient { + let configuration = URLSessionConfiguration.ephemeral + configuration.timeoutIntervalForRequest = 10 + return PlacementHTTPClient(serverURL: server.base, session: URLSession(configuration: configuration)) + } + + /// Submits `scene` and returns the error it threw, failing the test if it returned an answer. + static func failure(of client: PlacementHTTPClient) async throws -> any Error { + do { + _ = try await client.submit(scene: scene) { _ in } + } catch { + return error + } + Issue.record("the client returned an answer") + throw CancellationError() + } + + /// An answer for the scene sent comes back byte for byte. The request is that scene, posted as + /// JSON to /v1/placements, and progress ends at 1. + @Test func anAnswerForTheSceneSentIsReturned() async throws { + let server = try LoopbackPlacementServer { request in + .init(status: 200, body: (try? Self.answer(declaring: PacketFiles.sha256(request.body))) ?? Data()) + } + let progress = Mutex<[Double]>([]) + let data = try await Self.client(server).submit(scene: Self.scene) { value in progress.withLock { $0.append(value) } } + + #expect(data == (try Self.answer(declaring: PacketFiles.sha256(Self.scene)))) + #expect(try PlacementResult.decode(data).decision == .manualReview) + let requests = server.requests + #expect(requests.count == 1) + #expect(requests.first?.method == "POST") + #expect(requests.first?.path == "/v1/placements") + #expect(requests.first?.headers["content-type"] == "application/json") + #expect(requests.first?.body == Self.scene) + #expect(progress.withLock { $0.last } == 1) + } + + /// An answer that names another scene is never returned. It is an answer House Scan couldn't + /// use, which the homeowner may ask for again, and the client doesn't ask again on its own. + @Test func anAnswerForAnotherSceneIsNotReturned() async throws { + let other = PacketFiles.sha256(Data("another scene".utf8)) + let server = try LoopbackPlacementServer { _ in .init(status: 200, body: (try? Self.answer(declaring: other)) ?? Data()) } + let error = try await Self.failure(of: Self.client(server)) + + #expect(error as? ResultBinding.Refusal == .mismatch(submitted: PacketFiles.sha256(Self.scene), declared: other)) + #expect(UploadFailureKind.classify(error) == .unreadableAnswer) + #expect(UploadFailureKind.classify(error).retryable) + #expect(server.requests.count == 1) + } + + /// A 2xx without a readable input hash, or one that isn't JSON at all, is also unusable. + @Test(arguments: ["no hash", "not JSON"]) + func anAnswerWithoutItsSceneIsNotReturned(_ kind: String) async throws { + let body = kind == "no hash" ? try Self.answer(declaring: nil) : Data("maintenance".utf8) + let server = try LoopbackPlacementServer { _ in .init(status: 200, body: body) } + let error = try await Self.failure(of: Self.client(server)) + + #expect(error as? ResultBinding.Refusal == .noInputHash) + #expect(UploadFailureKind.classify(error) == .unreadableAnswer) + #expect(server.requests.count == 1) + } + + /// A status outside 2xx is the server's own word: a refusal sends the homeowner to the review, + /// a server error or a busy server can be tried again. The body reaches the log, cut at 300 bytes. + @Test(arguments: [ + (422, nil, UploadFailureKind.refused), + (503, nil, UploadFailureKind.serverError), + (429, "30", UploadFailureKind.busy(retryAfter: 30)), + ] as [(Int, String?, UploadFailureKind)]) + func aStatusOutside2xxIsClassifiedByThatStatus(status: Int, retryAfter: String?, kind: UploadFailureKind) async throws { + let body = Data(String(repeating: "x", count: 1000).utf8) + let server = try LoopbackPlacementServer { _ in + .init(status: status, headers: retryAfter.map { ["Retry-After": $0] } ?? [:], body: body) + } + let error = try await Self.failure(of: Self.client(server)) + + guard case .server(let sentStatus, let sentBody, let sentRetryAfter)? = error as? PlacementHTTPError else { + Issue.record("expected a server error, got \(error)") + return + } + #expect(sentStatus == status) + #expect(sentBody.utf8.count == 300) + #expect(sentRetryAfter == retryAfter) + #expect(UploadFailureKind.classify(error) == kind) + #expect(server.requests.count == 1) + } + + /// Nothing listening: no server answered, so the homeowner can try again. + @Test func noServerIsUnreachable() async throws { + let configuration = URLSessionConfiguration.ephemeral + configuration.timeoutIntervalForRequest = 10 + // Port 9 (discard) has no listener on a test machine. + let client = PlacementHTTPClient(serverURL: URL(string: "http://127.0.0.1:9")!, session: URLSession(configuration: configuration)) + let error = try await Self.failure(of: client) + + #expect(error is URLError) + #expect(UploadFailureKind.classify(error) == .unreachable) + } +} diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultBindingTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultBindingTests.swift index 93b764da9..1e375902e 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultBindingTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultBindingTests.swift @@ -31,9 +31,9 @@ import Testing let refusal = try #require(throws: ResultBinding.Refusal.self) { try ResultBinding.check(answer: data, submittedScene: Self.scene) } - // Every refusal is the "couldn't read the answer" failure: back to the review, no retry. + // Every refusal is an answer House Scan couldn't use: the homeowner may ask again. #expect(UploadFailureKind.classify(refusal) == .unreadableAnswer) - #expect(!UploadFailureKind.classify(refusal).retryable) + #expect(UploadFailureKind.classify(refusal).retryable) return refusal } diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/UploadFailureKindTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/UploadFailureKindTests.swift index 1c903e31f..d8998e27c 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/UploadFailureKindTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/UploadFailureKindTests.swift @@ -46,11 +46,24 @@ import Testing #expect(!UploadFailureKind.classify(httpStatus: 422).retryable) } - @Test func anythingElseIsAnUnreadableAnswer() { + /// An answer House Scan couldn't use is the answer's problem, not the scan's: the homeowner may + /// ask again, unlike a refusal. + @Test func anythingElseIsAnUnreadableAnswerThatMayBeTriedAgain() { #expect(UploadFailureKind.classify(httpStatus: 302) == .unreadableAnswer) #expect(UploadFailureKind.classify(httpStatus: 600) == .unreadableAnswer) struct DecodeFailure: Error {} #expect(UploadFailureKind.classify(DecodeFailure()) == .unreadableAnswer) - #expect(!UploadFailureKind.unreadableAnswer.retryable) + #expect(UploadFailureKind.unreadableAnswer.retryable) + #expect(!UploadFailureKind.refused.retryable) + } + + /// The placement client's errors: a status is classified as that status, and a response that + /// wasn't HTTP means no server answered. + @Test func placementClientErrors() { + #expect(UploadFailureKind.classify(PlacementHTTPError.server(status: 422, body: "")) == .refused) + #expect(UploadFailureKind.classify(PlacementHTTPError.server(status: 503, body: "")) == .serverError) + #expect(UploadFailureKind.classify(PlacementHTTPError.server(status: 429, body: "", retryAfter: "30")) == .busy(retryAfter: 30)) + #expect(UploadFailureKind.classify(PlacementHTTPError.server(status: 302, body: "")) == .unreadableAnswer) + #expect(UploadFailureKind.classify(PlacementHTTPError.notHTTP) == .unreachable) } } diff --git a/ios/HouseScanUITests/UploadRecoveryUITests.swift b/ios/HouseScanUITests/UploadRecoveryUITests.swift new file mode 100644 index 000000000..6e6bc13f6 --- /dev/null +++ b/ios/HouseScanUITests/UploadRecoveryUITests.swift @@ -0,0 +1,212 @@ +import CryptoKit +import Foundation +import Network +import Synchronization +import XCTest + +/// An answer House Scan can't use (it names another scene, or doesn't decode) is never shown, the +/// scan is kept, and only the homeowner's "Try again" asks again. A refused scan stays separate: +/// it goes back to the review (`ScreenStatesUITests.testRejectedUploadGoesBackToReview`). +final class UploadRecoveryUITests: XCTestCase { + override func setUp() { + continueAfterFailure = false + } + + /// The real engine against a placement server on 127.0.0.1. The first answer names another + /// scene; every later one names the scene it was sent. The synthetic wall plays to the upload, + /// the unbound answer shows nothing of itself, nothing is sent again until "Try again", and + /// the answer after that reaches the result. + @MainActor + func testAnAnswerForAnotherSceneIsHeldBackUntilTryAgain() throws { + let sample = try Data(contentsOf: Self.sampleResult) + let zeros = String(repeating: "0", count: 64) + XCTAssertTrue(String(decoding: sample, as: UTF8.self).contains("\"input_sha256\": \"\(zeros)\"")) + let server = try PlacementStub { request, index in + let named = index == 0 ? Self.sha256(Data("another scene".utf8)) : Self.sha256(request.body) + return Data(String(decoding: sample, as: UTF8.self).replacingOccurrences(of: zeros, with: named).utf8) + } + + let files = FileManager.default + let gate = URL(fileURLWithPath: NSTemporaryDirectory()).appending(path: "housescan-gate-\(UUID().uuidString)", directoryHint: .isDirectory) + try files.createDirectory(at: gate, withIntermediateDirectories: true) + defer { try? files.removeItem(at: gate) } + // Every screen up to the result is let through; the result's gate stays shut, so the + // autopilot stops there. + for phase in ["onboarding", "findMeter", "meterCloseUp", "wallWalk", "markFeatures", "gapRequest", "uploading", "spotConfirm"] { + try Data().write(to: gate.appending(path: phase)) + } + + let app = XCUIApplication() + app.launchArguments = [ + "-replay", FullFlowUITests.fixture, "-autopilot", "-autopilotHold", "1.5", "-autopilotGate", gate.path, + "-practiceMeter", "NO", "-serverURL", server.base.absoluteString, + ] + app.launch() + let any = app.descendants(matching: .any) + let note = any["upload.answerNote"] + XCTAssertTrue(note.waitForExistence(timeout: 300), "the unbound answer never showed as an answer House Scan couldn't use") + XCTAssertEqual(server.requests.count, 1) + XCTAssertEqual(server.requests.first?.path, "/v1/placements") + XCTAssertTrue(any.matching(NSPredicate(format: "label == %@", "We couldn't use the server's answer")).firstMatch.exists) + XCTAssertFalse(any["action.backToReview"].exists, "an unusable answer must not send the homeowner back to their marks") + XCTAssertTrue(any["action.startOver"].exists) + + // Nothing asks again on its own, and nothing of the answer reaches a later screen. + Thread.sleep(forTimeInterval: 6) + XCTAssertEqual(server.requests.count, 1, "the app asked again without a tap") + XCTAssertTrue(note.exists) + XCTAssertFalse(any["screen.spotConfirm"].exists) + XCTAssertFalse(any["screen.result"].exists) + + let shot = XCTAttachment(screenshot: app.screenshot()) + shot.name = "uploading-unusableAnswer-engine" + shot.lifetime = .keepAlways + add(shot) + + tap(app, "action.retryUpload") + XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 240), "the answer after Try again never reached the result") + XCTAssertGreaterThanOrEqual(server.requests.count, 2) + XCTAssertTrue(server.requests.allSatisfy { $0.method == "POST" && $0.path == "/v1/placements" }) + } + + /// The first unusable answer leads with "Try again"; the second says asking again might not + /// help and points to sharing the scan. Both pass the accessibility audit, and neither offers + /// the review. + @MainActor + func testRepeatedUnusableAnswersPointToAWayOut() throws { + for attempts in [1, 2] { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "uploading", "-uiDemoUnusableAnswer", "\(attempts)"] + app.launch() + let any = app.descendants(matching: .any) + let note = any["upload.answerNote"] + XCTAssertTrue(note.waitForExistence(timeout: 15)) + let title = attempts == 1 ? "We couldn't use the server's answer" : "The answer still couldn't be used" + XCTAssertTrue(any.matching(NSPredicate(format: "label == %@", title)).firstMatch.exists, "attempt \(attempts): title") + XCTAssertEqual(note.label.contains("Share scan saves a copy"), attempts > 1, "attempt \(attempts): note reads \(note.label)") + XCTAssertTrue(any["action.retryUpload"].exists) + XCTAssertTrue(any["action.startOver"].exists) + XCTAssertFalse(any["action.backToReview"].exists) + + Thread.sleep(forTimeInterval: 1) + let shot = XCTAttachment(screenshot: app.screenshot()) + shot.name = "uploading-unusableAnswer-\(attempts)" + shot.lifetime = .keepAlways + add(shot) + let outcome = try AccessibilityAudit.run(app) { _ in Thread.sleep(forTimeInterval: 2) } + XCTAssertTrue(outcome.persistent.isEmpty, "attempt \(attempts): \(outcome.persistent.map(\.finding.message))") + + tap(app, "action.retryUpload") + XCTAssertTrue(note.waitForNonExistence(timeout: 10), "Try again left the failure up") + app.terminate() + } + } + + /// The demo's answer after an unusable one is the result: "Try again" is the whole way on. + @MainActor + func testTryAgainAfterAnUnusableAnswerReachesTheResult() throws { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoPhase", "uploading", "-uiDemoUnusableAnswer", "-uiDemoPass"] + app.launch() + let any = app.descendants(matching: .any) + XCTAssertTrue(any["upload.answerNote"].waitForExistence(timeout: 30)) + tap(app, "action.retryUpload") + let spotClear = app.buttons["action.spotClear"] + let result = any["screen.result"] + let deadline = Date().addingTimeInterval(30) + while !result.exists, Date() < deadline { + if spotClear.exists, spotClear.isHittable { spotClear.tap() } + Thread.sleep(forTimeInterval: 0.5) + } + XCTAssertTrue(result.exists, "Try again never reached the result") + } + + static let sampleResult = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent().deletingLastPathComponent() + .appending(path: "HouseScan/Runtime/SampleResult.json") + + static func sha256(_ data: Data) -> String { + SHA256.hash(data: data).map { String(format: "%02x", $0) }.joined() + } + + @MainActor + private func tap(_ app: XCUIApplication, _ identifier: String) { + let button = app.buttons[identifier] + let ready = XCTNSPredicateExpectation(predicate: NSPredicate(format: "isEnabled == true AND isHittable == true"), object: button) + XCTAssertEqual(XCTWaiter().wait(for: [ready], timeout: 20), .completed, "\(identifier) never became tappable") + button.tap() + } +} + +/// A placement server on 127.0.0.1 that the app in the Simulator reaches like any other: every +/// request is answered 200 with the body `answer` builds from it and its index, and kept for the +/// test. It speaks only what the app sends: one request per connection with a `Content-Length` +/// body, and it closes the connection after replying. +final class PlacementStub: Sendable { + struct Request: Sendable { + var method: String + var path: String + var body: Data + } + + private let listener: NWListener + private let queue = DispatchQueue(label: "placement-stub") + private let answer: @Sendable (Request, Int) -> Data + private let log = Mutex<[Request]>([]) + + var requests: [Request] { log.withLock { $0 } } + var base: URL { URL(string: "http://127.0.0.1:\(listener.port?.rawValue ?? 0)")! } + + init(answer: @escaping @Sendable (Request, Int) -> Data) throws { + self.answer = answer + let parameters = NWParameters.tcp + parameters.acceptLocalOnly = true + parameters.requiredLocalEndpoint = .hostPort(host: "127.0.0.1", port: .any) + listener = try NWListener(using: parameters) + let ready = DispatchSemaphore(value: 0) + listener.stateUpdateHandler = { if case .ready = $0 { ready.signal() } } + listener.newConnectionHandler = { [queue] connection in + connection.start(queue: queue) + Self.receive(connection, buffer: Data()) { request in self.reply(to: request, on: connection) } + } + listener.start(queue: queue) + guard ready.wait(timeout: .now() + 5) == .success, listener.port != nil else { throw URLError(.cannotConnectToHost) } + } + + deinit { listener.cancel() } + + private func reply(to request: Request, on connection: NWConnection) { + let index = log.withLock { log in + log.append(request) + return log.count - 1 + } + let body = answer(request, index) + let head = "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: \(body.count)\r\nConnection: close\r\n\r\n" + connection.send(content: Data(head.utf8) + body, completion: .contentProcessed { _ in connection.cancel() }) + } + + private static func receive(_ connection: NWConnection, buffer: Data, handler: @escaping @Sendable (Request) -> Void) { + connection.receive(minimumIncompleteLength: 1, maximumLength: 1 << 22) { data, _, done, error in + var buffer = buffer + if let data { buffer.append(data) } + if let request = parse(buffer) { return handler(request) } + if done || error != nil { return connection.cancel() } + receive(connection, buffer: buffer, handler: handler) + } + } + + private static func parse(_ data: Data) -> Request? { + guard let end = data.range(of: Data("\r\n\r\n".utf8)) else { return nil } + let head = String(decoding: data[..= 2 else { return nil } + var length = 0 + for field in head.dropFirst() { + guard let colon = field.firstIndex(of: ":"), field[..= length else { return nil } + return Request(method: String(line[0]), path: String(line[1]), body: Data(body.prefix(length))) + } +} From 21abb9d650939dcc730e1581446a93a5521ad64d Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 02:42:12 -0500 Subject: [PATCH 2/5] Close the test servers' listeners and test the engine's repeat count Both loopback servers retained themselves through their listener's connection handler, so a dropped server never closed its listener. The handlers now hold the server weakly, and the UI test's stub also closes on stop(). The engine test now sends two unusable answers before a usable one: the first names another scene, and the second names the scene sent but has a result schema House Scan doesn't read. That covers the engine's decode failure and its count. The test checks the second state's wording and that no request goes out between taps. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../LoopbackPlacementServer.swift | 16 +++- .../UploadRecoveryUITests.swift | 77 +++++++++++++------ 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift index 4c0f9d79d..e795715b1 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackPlacementServer.swift @@ -5,7 +5,8 @@ import Synchronization /// A placement server on 127.0.0.1 for client tests: real HTTP, so the tests drive the actual /// `URLSession` transport. Each request gets the reply `respond` builds from it, and every request /// is kept for the test to inspect. It speaks only what these tests need: one request per -/// connection, a `Content-Length` body, and the reply closes the connection. +/// connection, a `Content-Length` body, and the reply closes the connection. The listener's +/// handlers hold the server weakly, so the listener closes when the test drops the server. final class LoopbackPlacementServer: Sendable { struct Request: Sendable { var method: String @@ -36,12 +37,19 @@ final class LoopbackPlacementServer: Sendable { listener = try NWListener(using: parameters) let ready = DispatchSemaphore(value: 0) listener.stateUpdateHandler = { if case .ready = $0 { ready.signal() } } - listener.newConnectionHandler = { [queue] connection in + listener.newConnectionHandler = { [weak self, queue] connection in + guard self != nil else { return connection.cancel() } connection.start(queue: queue) - Self.receive(connection, buffer: Data()) { request in self.answer(request, on: connection) } + Self.receive(connection, buffer: Data()) { [weak self] request in + guard let self else { return connection.cancel() } + self.answer(request, on: connection) + } } listener.start(queue: queue) - guard ready.wait(timeout: .now() + 5) == .success, listener.port != nil else { throw URLError(.cannotConnectToHost) } + guard ready.wait(timeout: .now() + 5) == .success, listener.port != nil else { + listener.cancel() + throw URLError(.cannotConnectToHost) + } } deinit { listener.cancel() } diff --git a/ios/HouseScanUITests/UploadRecoveryUITests.swift b/ios/HouseScanUITests/UploadRecoveryUITests.swift index 6e6bc13f6..f4dfcdd8a 100644 --- a/ios/HouseScanUITests/UploadRecoveryUITests.swift +++ b/ios/HouseScanUITests/UploadRecoveryUITests.swift @@ -13,18 +13,23 @@ final class UploadRecoveryUITests: XCTestCase { } /// The real engine against a placement server on 127.0.0.1. The first answer names another - /// scene; every later one names the scene it was sent. The synthetic wall plays to the upload, - /// the unbound answer shows nothing of itself, nothing is sent again until "Try again", and - /// the answer after that reaches the result. + /// scene. The second names the scene sent but has a result schema House Scan doesn't read. + /// Every later one is the bundled sample naming the scene sent. The synthetic wall plays to + /// the upload; neither unusable answer shows anything of itself, nothing is sent again until + /// "Try again", the second says asking again might not help, and the third reaches the result. @MainActor - func testAnAnswerForAnotherSceneIsHeldBackUntilTryAgain() throws { - let sample = try Data(contentsOf: Self.sampleResult) + func testUnusableAnswersAreHeldBackUntilTryAgain() throws { + let sample = String(decoding: try Data(contentsOf: Self.sampleResult), as: UTF8.self) let zeros = String(repeating: "0", count: 64) - XCTAssertTrue(String(decoding: sample, as: UTF8.self).contains("\"input_sha256\": \"\(zeros)\"")) + let schema = "\"schema_version\": \"1.0\"" + XCTAssertTrue(sample.contains("\"input_sha256\": \"\(zeros)\"")) + XCTAssertTrue(sample.contains(schema)) let server = try PlacementStub { request, index in - let named = index == 0 ? Self.sha256(Data("another scene".utf8)) : Self.sha256(request.body) - return Data(String(decoding: sample, as: UTF8.self).replacingOccurrences(of: zeros, with: named).utf8) + var answer = sample.replacingOccurrences(of: zeros, with: index == 0 ? Self.sha256(Data("another scene".utf8)) : Self.sha256(request.body)) + if index == 1 { answer = answer.replacingOccurrences(of: schema, with: "\"schema_version\": \"9.0\"") } + return Data(answer.utf8) } + defer { server.stop() } let files = FileManager.default let gate = URL(fileURLWithPath: NSTemporaryDirectory()).appending(path: "housescan-gate-\(UUID().uuidString)", directoryHint: .isDirectory) @@ -44,29 +49,41 @@ final class UploadRecoveryUITests: XCTestCase { app.launch() let any = app.descendants(matching: .any) let note = any["upload.answerNote"] - XCTAssertTrue(note.waitForExistence(timeout: 300), "the unbound answer never showed as an answer House Scan couldn't use") + let first = any.matching(NSPredicate(format: "label == %@", "We couldn't use the server's answer")).firstMatch + let second = any.matching(NSPredicate(format: "label == %@", "The answer still couldn't be used")).firstMatch + XCTAssertTrue(first.waitForExistence(timeout: 300), "the unbound answer never showed as an answer House Scan couldn't use") XCTAssertEqual(server.requests.count, 1) XCTAssertEqual(server.requests.first?.path, "/v1/placements") - XCTAssertTrue(any.matching(NSPredicate(format: "label == %@", "We couldn't use the server's answer")).firstMatch.exists) + holdsWithoutAsking(app, requests: 1, server: server, shot: "uploading-unusableAnswer-engine-1") + + tap(app, "action.retryUpload") + XCTAssertTrue(second.waitForExistence(timeout: 60), "the undecodable answer never showed as a second unusable answer") + XCTAssertTrue(note.exists) + XCTAssertEqual(server.requests.count, 2) + holdsWithoutAsking(app, requests: 2, server: server, shot: "uploading-unusableAnswer-engine-2") + + tap(app, "action.retryUpload") + XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 240), "the answer after Try again never reached the result") + XCTAssertGreaterThanOrEqual(server.requests.count, 3) + XCTAssertTrue(server.requests.allSatisfy { $0.method == "POST" && $0.path == "/v1/placements" }) + } + + /// On an unusable answer: no "Back to review", a way to start over, and for six seconds no + /// request beyond `requests` and nothing of the answer on a later screen. + @MainActor + private func holdsWithoutAsking(_ app: XCUIApplication, requests: Int, server: PlacementStub, shot name: String) { + let any = app.descendants(matching: .any) XCTAssertFalse(any["action.backToReview"].exists, "an unusable answer must not send the homeowner back to their marks") XCTAssertTrue(any["action.startOver"].exists) - - // Nothing asks again on its own, and nothing of the answer reaches a later screen. Thread.sleep(forTimeInterval: 6) - XCTAssertEqual(server.requests.count, 1, "the app asked again without a tap") - XCTAssertTrue(note.exists) + XCTAssertEqual(server.requests.count, requests, "the app asked again without a tap") + XCTAssertTrue(any["upload.answerNote"].exists) XCTAssertFalse(any["screen.spotConfirm"].exists) XCTAssertFalse(any["screen.result"].exists) - let shot = XCTAttachment(screenshot: app.screenshot()) - shot.name = "uploading-unusableAnswer-engine" + shot.name = name shot.lifetime = .keepAlways add(shot) - - tap(app, "action.retryUpload") - XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 240), "the answer after Try again never reached the result") - XCTAssertGreaterThanOrEqual(server.requests.count, 2) - XCTAssertTrue(server.requests.allSatisfy { $0.method == "POST" && $0.path == "/v1/placements" }) } /// The first unusable answer leads with "Try again"; the second says asking again might not @@ -141,7 +158,8 @@ final class UploadRecoveryUITests: XCTestCase { /// A placement server on 127.0.0.1 that the app in the Simulator reaches like any other: every /// request is answered 200 with the body `answer` builds from it and its index, and kept for the /// test. It speaks only what the app sends: one request per connection with a `Content-Length` -/// body, and it closes the connection after replying. +/// body, and it closes the connection after replying. `stop()` closes the listener; the +/// listener's handlers hold the stub weakly, so dropping it closes the listener too. final class PlacementStub: Sendable { struct Request: Sendable { var method: String @@ -165,14 +183,23 @@ final class PlacementStub: Sendable { listener = try NWListener(using: parameters) let ready = DispatchSemaphore(value: 0) listener.stateUpdateHandler = { if case .ready = $0 { ready.signal() } } - listener.newConnectionHandler = { [queue] connection in + listener.newConnectionHandler = { [weak self, queue] connection in + guard self != nil else { return connection.cancel() } connection.start(queue: queue) - Self.receive(connection, buffer: Data()) { request in self.reply(to: request, on: connection) } + Self.receive(connection, buffer: Data()) { [weak self] request in + guard let self else { return connection.cancel() } + self.reply(to: request, on: connection) + } } listener.start(queue: queue) - guard ready.wait(timeout: .now() + 5) == .success, listener.port != nil else { throw URLError(.cannotConnectToHost) } + guard ready.wait(timeout: .now() + 5) == .success, listener.port != nil else { + listener.cancel() + throw URLError(.cannotConnectToHost) + } } + func stop() { listener.cancel() } + deinit { listener.cancel() } private func reply(to request: Request, on connection: NWConnection) { From 4bc4ded44a7b7120dae72238574bb5ab2987b67f Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 03:59:51 -0500 Subject: [PATCH 3/5] Answer the engine UI test's uploads through the gate folder On the hosted Simulator (Actions run 37107233020) the app's upload to the UI test's server on 127.0.0.1 failed as offline, so the test never saw an unusable answer. That server lived in the test runner, a background app while the app under test is open; the same listener code answers URLSession on a Mac at 200 B to 3 MB. Under -answersFromGate the app's real PlacementHTTPClient sends through a URLProtocol that writes each request into the -autopilotGate folder and answers HTTP 200 with the test's answer-n.json. The engine, the status check and ResultBinding run as against a server; only the socket is replaced. HouseScanKit's PlacementHTTPClientTests still cover the real loopback socket. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Runtime/GateAnswerProtocol.swift | 95 +++++++++++++ ios/HouseScan/Runtime/LaunchOptions.swift | 8 ++ ios/HouseScan/Runtime/ResultClient.swift | 4 +- ios/HouseScan/Runtime/ScanEngine.swift | 2 +- .../UploadRecoveryUITests.swift | 127 ++++++------------ 5 files changed, 147 insertions(+), 89 deletions(-) create mode 100644 ios/HouseScan/Runtime/GateAnswerProtocol.swift diff --git a/ios/HouseScan/Runtime/GateAnswerProtocol.swift b/ios/HouseScan/Runtime/GateAnswerProtocol.swift new file mode 100644 index 000000000..e35a7b9d6 --- /dev/null +++ b/ios/HouseScan/Runtime/GateAnswerProtocol.swift @@ -0,0 +1,95 @@ +import Foundation +import Synchronization + +/// The placement server's side of an upload under `-answersFromGate`, played by a UI test through +/// the `-autopilotGate` folder. The app's real `PlacementHTTPClient` sends through `session`, so +/// the request, the answer's status check and `ResultBinding` all run as they do against a server; +/// only the socket is replaced. +/// +/// For the n-th request (from 0, in the order the app sends them) it writes `request-n.target` +/// ("POST /v1/placements"), then the exact body as `request-n.json`, then waits up to 60 seconds +/// for the test's `answer-n.json` and answers HTTP 200 with its bytes. With no answer by then the +/// request fails as timed out. +/// +/// The UI test used to run a server on 127.0.0.1 inside its runner. That server answered +/// URLSession on a Mac, but on the hosted Simulator the app's requests to it failed as offline +/// (Actions run 37107233020); the runner is a background app while the app under test is open. +/// HouseScanKit's `PlacementHTTPClientTests` still drive the client over a real loopback socket. +final class GateAnswerProtocol: URLProtocol, @unchecked Sendable { + static let session: URLSession = { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [GateAnswerProtocol.self] + return URLSession(configuration: configuration) + }() + + static let answerWait: TimeInterval = 60 + + /// Requests started in this process, which numbers the files. + private static let started = Mutex(0) + + /// Set and read only on the loading thread: `startLoading`, the timer and `stopLoading` all + /// run on the run loop URLSession called `startLoading` on. + private var poll: Timer? + + override class func canInit(with request: URLRequest) -> Bool { true } + override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + + override func startLoading() { + guard let gate = LaunchOptions().autopilotGate, let url = request.url else { + client?.urlProtocol(self, didFailWithError: URLError(.badURL)) + return + } + let index = Self.started.withLock { count in + defer { count += 1 } + return count + } + do { + let target = "\(request.httpMethod ?? "GET") \(url.path())" + try Data(target.utf8).write(to: gate.appending(path: "request-\(index).target"), options: .atomic) + try Self.body(of: request).write(to: gate.appending(path: "request-\(index).json"), options: .atomic) + } catch { + client?.urlProtocol(self, didFailWithError: error) + return + } + let answer = gate.appending(path: "answer-\(index).json") + let deadline = Date().addingTimeInterval(Self.answerWait) + let timer = Timer(timeInterval: 0.1, repeats: true) { [self] timer in + if let body = try? Data(contentsOf: answer) { + timer.invalidate() + let response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: "HTTP/1.1", headerFields: ["Content-Type": "application/json"])! + client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) + client?.urlProtocol(self, didLoad: body) + client?.urlProtocolDidFinishLoading(self) + } else if Date() >= deadline { + timer.invalidate() + RuntimeLog.engine.error("answers from gate: no \(answer.lastPathComponent, privacy: .public) within \(Self.answerWait, privacy: .public) s") + client?.urlProtocol(self, didFailWithError: URLError(.timedOut)) + } + } + RunLoop.current.add(timer, forMode: RunLoop.current.currentMode ?? .default) + poll = timer + } + + override func stopLoading() { + poll?.invalidate() + poll = nil + } + + /// An upload task hands its body over as a stream, not `httpBody`. A stream that fails to read + /// throws: a cut-short body would reach the test as an answer naming the wrong scene. + private static func body(of request: URLRequest) throws -> Data { + if let body = request.httpBody { return body } + guard let stream = request.httpBodyStream else { return Data() } + stream.open() + defer { stream.close() } + var data = Data() + var buffer = [UInt8](repeating: 0, count: 64 * 1024) + while true { + let read = stream.read(&buffer, maxLength: buffer.count) + if read < 0 { throw stream.streamError ?? URLError(.cannotDecodeRawData) } + guard read > 0 else { break } + data.append(buffer, count: read) + } + return data + } +} diff --git a/ios/HouseScan/Runtime/LaunchOptions.swift b/ios/HouseScan/Runtime/LaunchOptions.swift index 3d659c6ed..46c1bb834 100644 --- a/ios/HouseScan/Runtime/LaunchOptions.swift +++ b/ios/HouseScan/Runtime/LaunchOptions.swift @@ -28,6 +28,9 @@ import OSLog /// ground, as ARKit refining the ground would (`ScanEngine.injectGroundForTest`). Replays carry /// no plane evidence, so this is the only way a UI test reaches that path. Meters must be over /// the engine's 1 cm refine threshold and at most `GroundPlaneChoice.maximumRaise`. +/// - `-answersFromGate`: with `-serverURL` and `-autopilotGate`, uploads go through the gate +/// folder instead of the network: the UI test reads each request there and writes the server's +/// answer (`GateAnswerProtocol`). /// - `-simulateAppStore`: run as an App Store install would, so the developer options and practice /// meter are unavailable whatever the stored switch says (`DeveloperSettings`). It can only take /// the switch away, never offer it. @@ -42,6 +45,7 @@ struct LaunchOptions: Equatable { var autopilotSomethingThere = false var simulateAppStore = false var injectGroundRise: Float? + var answersFromGate = false init( arguments: [String] = ProcessInfo.processInfo.arguments, @@ -62,6 +66,10 @@ struct LaunchOptions: Equatable { sampleResult = arguments.contains("-sampleResult") if let gate = value(after: "-autopilotGate") { autopilotGate = URL(fileURLWithPath: gate, isDirectory: true) } if let hold = value(after: "-autopilotHold").flatMap(Double.init), hold > 0 { autopilotHold = hold } + answersFromGate = arguments.contains("-answersFromGate") + if answersFromGate, autopilotGate == nil || serverURL == nil { + preconditionFailure("-answersFromGate needs -autopilotGate and -serverURL") + } if let text = value(after: "-injectGroundRise") { guard let meters = Float(text), meters > 0.01, meters <= 0.1 else { preconditionFailure("-injectGroundRise takes meters over 0.01 and at most 0.1, got \(text)") diff --git a/ios/HouseScan/Runtime/ResultClient.swift b/ios/HouseScan/Runtime/ResultClient.swift index e363889b8..c100b08be 100644 --- a/ios/HouseScan/Runtime/ResultClient.swift +++ b/ios/HouseScan/Runtime/ResultClient.swift @@ -19,9 +19,9 @@ final class HTTPResultClient: ResultClient { let isSample = false private let client: PlacementHTTPClient - init(serverURL: URL) { + init(serverURL: URL, session: URLSession = .shared) { self.serverURL = serverURL - client = PlacementHTTPClient(serverURL: serverURL) + client = PlacementHTTPClient(serverURL: serverURL, session: session) } func submit(scene: Data, progress: @escaping @Sendable (Double) -> Void) async throws -> Data { diff --git a/ios/HouseScan/Runtime/ScanEngine.swift b/ios/HouseScan/Runtime/ScanEngine.swift index a52e61288..874ae2930 100644 --- a/ios/HouseScan/Runtime/ScanEngine.swift +++ b/ios/HouseScan/Runtime/ScanEngine.swift @@ -246,7 +246,7 @@ final class ScanEngine { store = KeyframeStore() recorder = Self.makeRecorder(store) if let url = options.serverURL, !options.sampleResult { - resultClient = HTTPResultClient(serverURL: url) + resultClient = HTTPResultClient(serverURL: url, session: options.answersFromGate ? GateAnswerProtocol.session : .shared) } else { resultClient = SampleResultClient(pace: options.autopilot ? options.autopilotHold : 1.2) } diff --git a/ios/HouseScanUITests/UploadRecoveryUITests.swift b/ios/HouseScanUITests/UploadRecoveryUITests.swift index f4dfcdd8a..31f9b2fa0 100644 --- a/ios/HouseScanUITests/UploadRecoveryUITests.swift +++ b/ios/HouseScanUITests/UploadRecoveryUITests.swift @@ -1,6 +1,5 @@ import CryptoKit import Foundation -import Network import Synchronization import XCTest @@ -12,11 +11,12 @@ final class UploadRecoveryUITests: XCTestCase { continueAfterFailure = false } - /// The real engine against a placement server on 127.0.0.1. The first answer names another - /// scene. The second names the scene sent but has a result schema House Scan doesn't read. - /// Every later one is the bundled sample naming the scene sent. The synthetic wall plays to - /// the upload; neither unusable answer shows anything of itself, nothing is sent again until - /// "Try again", the second says asking again might not help, and the third reaches the result. + /// The real engine and its HTTP client, with the test as the placement server + /// (`-answersFromGate`). The first answer names another scene. The second names the scene sent + /// but has a result schema House Scan doesn't read. Every later one is the bundled sample + /// naming the scene sent. The synthetic wall plays to the upload; neither unusable answer shows + /// anything of itself, nothing is sent again until "Try again", the second says asking again + /// might not help, and the third reaches the result. @MainActor func testUnusableAnswersAreHeldBackUntilTryAgain() throws { let sample = String(decoding: try Data(contentsOf: Self.sampleResult), as: UTF8.self) @@ -24,12 +24,6 @@ final class UploadRecoveryUITests: XCTestCase { let schema = "\"schema_version\": \"1.0\"" XCTAssertTrue(sample.contains("\"input_sha256\": \"\(zeros)\"")) XCTAssertTrue(sample.contains(schema)) - let server = try PlacementStub { request, index in - var answer = sample.replacingOccurrences(of: zeros, with: index == 0 ? Self.sha256(Data("another scene".utf8)) : Self.sha256(request.body)) - if index == 1 { answer = answer.replacingOccurrences(of: schema, with: "\"schema_version\": \"9.0\"") } - return Data(answer.utf8) - } - defer { server.stop() } let files = FileManager.default let gate = URL(fileURLWithPath: NSTemporaryDirectory()).appending(path: "housescan-gate-\(UUID().uuidString)", directoryHint: .isDirectory) @@ -40,11 +34,17 @@ final class UploadRecoveryUITests: XCTestCase { for phase in ["onboarding", "findMeter", "meterCloseUp", "wallWalk", "markFeatures", "gapRequest", "uploading", "spotConfirm"] { try Data().write(to: gate.appending(path: phase)) } + let server = GateServer(gate: gate) { body, index in + var answer = sample.replacingOccurrences(of: zeros, with: index == 0 ? Self.sha256(Data("another scene".utf8)) : Self.sha256(body)) + if index == 1 { answer = answer.replacingOccurrences(of: schema, with: "\"schema_version\": \"9.0\"") } + return Data(answer.utf8) + } + defer { server.stop() } let app = XCUIApplication() app.launchArguments = [ "-replay", FullFlowUITests.fixture, "-autopilot", "-autopilotHold", "1.5", "-autopilotGate", gate.path, - "-practiceMeter", "NO", "-serverURL", server.base.absoluteString, + "-practiceMeter", "NO", "-serverURL", "http://placement.invalid", "-answersFromGate", ] app.launch() let any = app.descendants(matching: .any) @@ -53,7 +53,7 @@ final class UploadRecoveryUITests: XCTestCase { let second = any.matching(NSPredicate(format: "label == %@", "The answer still couldn't be used")).firstMatch XCTAssertTrue(first.waitForExistence(timeout: 300), "the unbound answer never showed as an answer House Scan couldn't use") XCTAssertEqual(server.requests.count, 1) - XCTAssertEqual(server.requests.first?.path, "/v1/placements") + XCTAssertEqual(server.requests.first?.target, "POST /v1/placements") holdsWithoutAsking(app, requests: 1, server: server, shot: "uploading-unusableAnswer-engine-1") tap(app, "action.retryUpload") @@ -65,13 +65,13 @@ final class UploadRecoveryUITests: XCTestCase { tap(app, "action.retryUpload") XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 240), "the answer after Try again never reached the result") XCTAssertGreaterThanOrEqual(server.requests.count, 3) - XCTAssertTrue(server.requests.allSatisfy { $0.method == "POST" && $0.path == "/v1/placements" }) + XCTAssertTrue(server.requests.allSatisfy { $0.target == "POST /v1/placements" }) } /// On an unusable answer: no "Back to review", a way to start over, and for six seconds no /// request beyond `requests` and nothing of the answer on a later screen. @MainActor - private func holdsWithoutAsking(_ app: XCUIApplication, requests: Int, server: PlacementStub, shot name: String) { + private func holdsWithoutAsking(_ app: XCUIApplication, requests: Int, server: GateServer, shot name: String) { let any = app.descendants(matching: .any) XCTAssertFalse(any["action.backToReview"].exists, "an unusable answer must not send the homeowner back to their marks") XCTAssertTrue(any["action.startOver"].exists) @@ -155,85 +155,40 @@ final class UploadRecoveryUITests: XCTestCase { } } -/// A placement server on 127.0.0.1 that the app in the Simulator reaches like any other: every -/// request is answered 200 with the body `answer` builds from it and its index, and kept for the -/// test. It speaks only what the app sends: one request per connection with a `Content-Length` -/// body, and it closes the connection after replying. `stop()` closes the listener; the -/// listener's handlers hold the stub weakly, so dropping it closes the listener too. -final class PlacementStub: Sendable { +/// The placement server for `-answersFromGate`, answering through the gate folder +/// (`GateAnswerProtocol` in the app). For each `request-n.json` the app leaves there it writes +/// `answer-n.json`, built by `answer` from the request's body and n, and keeps the request for the +/// test. A background thread polls every 0.1 s until `stop()`. +final class GateServer: Sendable { struct Request: Sendable { - var method: String - var path: String + var target: String var body: Data } - private let listener: NWListener - private let queue = DispatchQueue(label: "placement-stub") - private let answer: @Sendable (Request, Int) -> Data private let log = Mutex<[Request]>([]) + private let stopped = Mutex(false) var requests: [Request] { log.withLock { $0 } } - var base: URL { URL(string: "http://127.0.0.1:\(listener.port?.rawValue ?? 0)")! } - - init(answer: @escaping @Sendable (Request, Int) -> Data) throws { - self.answer = answer - let parameters = NWParameters.tcp - parameters.acceptLocalOnly = true - parameters.requiredLocalEndpoint = .hostPort(host: "127.0.0.1", port: .any) - listener = try NWListener(using: parameters) - let ready = DispatchSemaphore(value: 0) - listener.stateUpdateHandler = { if case .ready = $0 { ready.signal() } } - listener.newConnectionHandler = { [weak self, queue] connection in - guard self != nil else { return connection.cancel() } - connection.start(queue: queue) - Self.receive(connection, buffer: Data()) { [weak self] request in - guard let self else { return connection.cancel() } - self.reply(to: request, on: connection) - } - } - listener.start(queue: queue) - guard ready.wait(timeout: .now() + 5) == .success, listener.port != nil else { - listener.cancel() - throw URLError(.cannotConnectToHost) - } - } - - func stop() { listener.cancel() } - - deinit { listener.cancel() } - - private func reply(to request: Request, on connection: NWConnection) { - let index = log.withLock { log in - log.append(request) - return log.count - 1 - } - let body = answer(request, index) - let head = "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: \(body.count)\r\nConnection: close\r\n\r\n" - connection.send(content: Data(head.utf8) + body, completion: .contentProcessed { _ in connection.cancel() }) - } - private static func receive(_ connection: NWConnection, buffer: Data, handler: @escaping @Sendable (Request) -> Void) { - connection.receive(minimumIncompleteLength: 1, maximumLength: 1 << 22) { data, _, done, error in - var buffer = buffer - if let data { buffer.append(data) } - if let request = parse(buffer) { return handler(request) } - if done || error != nil { return connection.cancel() } - receive(connection, buffer: buffer, handler: handler) + init(gate: URL, answer: @escaping @Sendable (Data, Int) -> Data) { + Thread.detachNewThread { [self] in + while !stopped.withLock({ $0 }) { + let index = log.withLock { $0.count } + let body = gate.appending(path: "request-\(index).json") + if let data = try? Data(contentsOf: body) { + let target = (try? String(contentsOf: gate.appending(path: "request-\(index).target"), encoding: .utf8)) ?? "" + log.withLock { $0.append(Request(target: target, body: data)) } + do { + try answer(data, index).write(to: gate.appending(path: "answer-\(index).json"), options: .atomic) + } catch { + XCTFail("couldn't write answer-\(index).json: \(error)") + } + } else { + Thread.sleep(forTimeInterval: 0.1) + } + } } } - private static func parse(_ data: Data) -> Request? { - guard let end = data.range(of: Data("\r\n\r\n".utf8)) else { return nil } - let head = String(decoding: data[..= 2 else { return nil } - var length = 0 - for field in head.dropFirst() { - guard let colon = field.firstIndex(of: ":"), field[..= length else { return nil } - return Request(method: String(line[0]), path: String(line[1]), body: Data(body.prefix(length))) - } + func stop() { stopped.withLock { $0 = true } } } From 11d02a3496e02973021c78e61bdcb5b5804f4b24 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 04:10:59 -0500 Subject: [PATCH 4/5] Keep the gate server answering after a lost request, and stop it fully A fresh review found two fault paths in the test hook. The app numbered a request before reading its body, and the test's server waited for numbers in order, so one request the app failed to write left every later one unanswered. The app now numbers a request only once its body is read, and the server finds requests by number in the folder. stop() also returned while its thread could still be writing into the gate folder the test was about to delete. It now waits for the thread and reports any answer it couldn't write from the test's own thread. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Runtime/GateAnswerProtocol.swift | 11 +++- ios/HouseScan/Runtime/LaunchOptions.swift | 4 +- .../UploadRecoveryUITests.swift | 52 ++++++++++++++----- 3 files changed, 50 insertions(+), 17 deletions(-) diff --git a/ios/HouseScan/Runtime/GateAnswerProtocol.swift b/ios/HouseScan/Runtime/GateAnswerProtocol.swift index e35a7b9d6..34cc69e14 100644 --- a/ios/HouseScan/Runtime/GateAnswerProtocol.swift +++ b/ios/HouseScan/Runtime/GateAnswerProtocol.swift @@ -39,6 +39,15 @@ final class GateAnswerProtocol: URLProtocol, @unchecked Sendable { client?.urlProtocol(self, didFailWithError: URLError(.badURL)) return } + let body: Data + do { + body = try Self.body(of: request) + } catch { + client?.urlProtocol(self, didFailWithError: error) + return + } + // Numbered only once the body is read. A write that fails after this leaves a gap in the + // numbers, which the test's server skips over. let index = Self.started.withLock { count in defer { count += 1 } return count @@ -46,7 +55,7 @@ final class GateAnswerProtocol: URLProtocol, @unchecked Sendable { do { let target = "\(request.httpMethod ?? "GET") \(url.path())" try Data(target.utf8).write(to: gate.appending(path: "request-\(index).target"), options: .atomic) - try Self.body(of: request).write(to: gate.appending(path: "request-\(index).json"), options: .atomic) + try body.write(to: gate.appending(path: "request-\(index).json"), options: .atomic) } catch { client?.urlProtocol(self, didFailWithError: error) return diff --git a/ios/HouseScan/Runtime/LaunchOptions.swift b/ios/HouseScan/Runtime/LaunchOptions.swift index 46c1bb834..af4774bfe 100644 --- a/ios/HouseScan/Runtime/LaunchOptions.swift +++ b/ios/HouseScan/Runtime/LaunchOptions.swift @@ -28,7 +28,7 @@ import OSLog /// ground, as ARKit refining the ground would (`ScanEngine.injectGroundForTest`). Replays carry /// no plane evidence, so this is the only way a UI test reaches that path. Meters must be over /// the engine's 1 cm refine threshold and at most `GroundPlaneChoice.maximumRaise`. -/// - `-answersFromGate`: with `-serverURL` and `-autopilotGate`, uploads go through the gate +/// - `-answersFromGate`: with `-autopilotGate` and a server URL, uploads go through the gate /// folder instead of the network: the UI test reads each request there and writes the server's /// answer (`GateAnswerProtocol`). /// - `-simulateAppStore`: run as an App Store install would, so the developer options and practice @@ -68,7 +68,7 @@ struct LaunchOptions: Equatable { if let hold = value(after: "-autopilotHold").flatMap(Double.init), hold > 0 { autopilotHold = hold } answersFromGate = arguments.contains("-answersFromGate") if answersFromGate, autopilotGate == nil || serverURL == nil { - preconditionFailure("-answersFromGate needs -autopilotGate and -serverURL") + preconditionFailure("-answersFromGate needs -autopilotGate and a server URL (-serverURL or the build's)") } if let text = value(after: "-injectGroundRise") { guard let meters = Float(text), meters > 0.01, meters <= 0.1 else { diff --git a/ios/HouseScanUITests/UploadRecoveryUITests.swift b/ios/HouseScanUITests/UploadRecoveryUITests.swift index 31f9b2fa0..3ad6dedfc 100644 --- a/ios/HouseScanUITests/UploadRecoveryUITests.swift +++ b/ios/HouseScanUITests/UploadRecoveryUITests.swift @@ -158,37 +158,61 @@ final class UploadRecoveryUITests: XCTestCase { /// The placement server for `-answersFromGate`, answering through the gate folder /// (`GateAnswerProtocol` in the app). For each `request-n.json` the app leaves there it writes /// `answer-n.json`, built by `answer` from the request's body and n, and keeps the request for the -/// test. A background thread polls every 0.1 s until `stop()`. +/// test. Requests are found by their numbers, not counted, so one the app failed to write doesn't +/// hold up the next. A background thread polls every 0.1 s until `stop()`. final class GateServer: Sendable { struct Request: Sendable { + var index: Int var target: String var body: Data } - private let log = Mutex<[Request]>([]) - private let stopped = Mutex(false) + private struct State { + var requests: [Request] = [] + var problems: [String] = [] + var stopped = false + } + + private let state = Mutex(State()) + private let finished = DispatchSemaphore(value: 0) - var requests: [Request] { log.withLock { $0 } } + var requests: [Request] { state.withLock { $0.requests } } init(gate: URL, answer: @escaping @Sendable (Data, Int) -> Data) { Thread.detachNewThread { [self] in - while !stopped.withLock({ $0 }) { - let index = log.withLock { $0.count } - let body = gate.appending(path: "request-\(index).json") - if let data = try? Data(contentsOf: body) { + defer { finished.signal() } + while !state.withLock({ $0.stopped }) { + let answered = Set(state.withLock { $0.requests.map(\.index) }) + let names = (try? FileManager.default.contentsOfDirectory(atPath: gate.path)) ?? [] + let waiting = names.compactMap(Self.requestIndex).filter { !answered.contains($0) }.sorted() + guard !waiting.isEmpty else { + Thread.sleep(forTimeInterval: 0.1) + continue + } + for index in waiting { + guard let body = try? Data(contentsOf: gate.appending(path: "request-\(index).json")) else { continue } let target = (try? String(contentsOf: gate.appending(path: "request-\(index).target"), encoding: .utf8)) ?? "" - log.withLock { $0.append(Request(target: target, body: data)) } + state.withLock { $0.requests.append(Request(index: index, target: target, body: body)) } do { - try answer(data, index).write(to: gate.appending(path: "answer-\(index).json"), options: .atomic) + try answer(body, index).write(to: gate.appending(path: "answer-\(index).json"), options: .atomic) } catch { - XCTFail("couldn't write answer-\(index).json: \(error)") + state.withLock { $0.problems.append("couldn't write answer-\(index).json: \(error)") } } - } else { - Thread.sleep(forTimeInterval: 0.1) } } } } - func stop() { stopped.withLock { $0 = true } } + /// Stops the thread and waits for it, so nothing is written into the gate folder once this + /// returns, then reports any answer it couldn't write as a failure of the calling test. + func stop() { + state.withLock { $0.stopped = true } + if finished.wait(timeout: .now() + 5) == .timedOut { XCTFail("the gate server's thread didn't stop") } + for problem in state.withLock({ $0.problems }) { XCTFail(problem) } + } + + private static func requestIndex(_ name: String) -> Int? { + guard name.hasPrefix("request-"), name.hasSuffix(".json") else { return nil } + return Int(name.dropFirst("request-".count).dropLast(".json".count)) + } } From a7458e7c570eaf905a0781f722885f75f8de5dd1 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 04:57:38 -0500 Subject: [PATCH 5/5] Give the repeated Try again the light screens' quiet style The second unusable-answer state used the dark secondary style, which the app keeps for buttons over the camera. On this light screen it drew as a heavy dark pill, narrower than its neighbours and stronger than Share scan, the way out the note points to (CI screenshot uploading-unusableAnswer-2, run 37112184653). It now uses the quiet style the light screens use for a second choice. Co-Authored-By: Claude Opus 5.5 (1M context) --- ios/HouseScan/UI/Screens/UploadingScreen.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ios/HouseScan/UI/Screens/UploadingScreen.swift b/ios/HouseScan/UI/Screens/UploadingScreen.swift index bee6c59de..792a59cf3 100644 --- a/ios/HouseScan/UI/Screens/UploadingScreen.swift +++ b/ios/HouseScan/UI/Screens/UploadingScreen.swift @@ -100,9 +100,10 @@ struct UploadingScreen: View { } /// The ways on after an answer House Scan couldn't use. "Try again" leads the first time. Once it -/// has already failed again, it steps down to a secondary button and the note points to sharing -/// the scan, so the homeowner isn't left pressing the same button. Never "Back to review": the -/// marks aren't the problem. +/// has already failed again, it takes the quiet style of the choices around it and the note points +/// to sharing the scan, so the homeowner isn't left pressing the same button. Not the dark +/// secondary style: that one is for camera screens, and on this light screen it outweighed the +/// buttons it was meant to sit beside. Never "Back to review": the marks aren't the problem. private struct UnusableAnswerActions: View { var repeated: Bool var shareableScan: URL? @@ -118,7 +119,7 @@ private struct UnusableAnswerActions: View { .accessibilityIdentifier("upload.answerNote") Group { if repeated { - Button("Try again") { actions.retryUpload() }.buttonStyle(.secondary) + Button("Try again") { actions.retryUpload() }.buttonStyle(.quiet) } else { Button("Try again") { actions.retryUpload() }.buttonStyle(.primary) }