-
-
Notifications
You must be signed in to change notification settings - Fork 262
feat(map): Site Planner outbound coverage estimate — WKWebView bridge (#2058) #2072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
05b77a5
0f624aa
f5b2835
6824fa4
6b3c336
e6e0593
0646b19
3a3c9f6
b96dfd0
6f3bb7a
7deabdf
866faff
0c5642b
c3d133b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| { | ||
| "feature_directory": "specs/013-docs-release-versioning" | ||
| "feature_directory": "specs/015-site-planner-outbound" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,12 @@ | ||
| <!-- SPECKIT START --> | ||
| For additional context about technologies to be used, project structure, | ||
| shell commands, and other important information, read the current plan: | ||
| `specs/014-mesh-beacons/plan.md` (feature: Mesh Beacons). | ||
| `specs/015-site-planner-outbound/plan.md` (feature: Site Planner Outbound Coverage Estimate). | ||
| <!-- SPECKIT END --> | ||
|
|
||
| ## Active Technologies | ||
| - Swift (latest stable), Swift Concurrency (`async`/`await`, `@MainActor`) + SwiftUI, WebKit (`WKWebView`, `WKUserScript`, `WKScriptMessageHandler`) — new to this feature, no existing WKWebView usage in the codebase to follow as precedent (confirm during implementation) (015-site-planner-outbound) | ||
| - SwiftData for existing config entities read at prefill time (no writes); the estimate result reuses the existing file-based `MapDataManager` storage (`MapData/user_uploaded/` + `upload_history.json`) unchanged — no new persisted entity (015-site-planner-outbound) | ||
|
|
||
| ## Recent Changes | ||
| - 015-site-planner-outbound: Added Swift (latest stable), Swift Concurrency (`async`/`await`, `@MainActor`) + SwiftUI, WebKit (`WKWebView`, `WKUserScript`, `WKScriptMessageHandler`) — new to this feature, no existing WKWebView usage in the codebase to follow as precedent (confirm during implementation) |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| // | ||
| // CoverageEstimateBridge.swift | ||
| // Meshtastic | ||
| // | ||
| // Drives the Meshtastic Site Planner in a hidden WKWebView to compute a coverage | ||
| // estimate without a browser or share sheet — see | ||
| // specs/015-site-planner-outbound/contracts/native-bridge-contract.md. | ||
| // | ||
| // The planner calls `window.__meshtasticNative.onCoverage(jsonString)` unconditionally | ||
| // on every successful simulation (feature-detected, not gated by a query flag). Since | ||
| // WKScriptMessageHandler is only reachable via `window.webkit.messageHandlers.<name> | ||
| // .postMessage(...)`, a WKUserScript shim is injected before the planner's own JS runs | ||
| // to bridge the two. The planner never acknowledges receipt and never calls back on | ||
| // failure — a timeout is the only signal for "it never responded" (contracts doc's | ||
| // "Timeout Policy" note). | ||
| // | ||
| // The WKWebView is attached (alpha 0, 1x1) to the key window rather than left fully | ||
| // detached: an unattached, view-hierarchy-less WKWebView has known inconsistent JS/ | ||
| // message-delivery behavior on some WebKit hosting configurations (research.md §5's | ||
| // Mac Catalyst risk) — attaching it, even invisibly, is the safer default this | ||
| // implementation commits to rather than the version T012 is meant to falsify. | ||
| // | ||
|
|
||
| import Foundation | ||
| import WebKit | ||
| import OSLog | ||
|
|
||
| #if canImport(UIKit) | ||
| import UIKit | ||
| #endif | ||
|
|
||
| @MainActor | ||
| final class CoverageEstimateBridge: NSObject { | ||
|
|
||
| enum BridgeError: LocalizedError { | ||
| case timeout | ||
| case navigationFailed(Error) | ||
| case invalidResponse | ||
| case noWindowToAttachTo | ||
|
|
||
| var errorDescription: String? { | ||
| switch self { | ||
| case .timeout: | ||
| return "The coverage estimate did not respond in time.".localized | ||
| case .navigationFailed(let error): | ||
| return String(format: "Could not reach the Site Planner: %@".localized, error.localizedDescription) | ||
| case .invalidResponse: | ||
| return "The Site Planner returned an unreadable result.".localized | ||
| case .noWindowToAttachTo: | ||
| return "Could not start the coverage estimate.".localized | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static let messageHandlerName = "meshtasticCoverageBridge" | ||
|
|
||
| /// Defines `window.__meshtasticNative.onCoverage`, the exact shape | ||
| /// `postCoverageToBridge` feature-detects and calls (native-bridge-contract.md). | ||
| /// Injected at `.atDocumentStart`, before the planner's own script runs. | ||
| private static let shimScript = """ | ||
| window.__meshtasticNative = { | ||
| onCoverage: function(geojson) { | ||
| window.webkit.messageHandlers.\(messageHandlerName).postMessage(geojson); | ||
| } | ||
| }; | ||
| """ | ||
|
|
||
| private var webView: WKWebView? | ||
| private var continuation: CheckedContinuation<Data, Error>? | ||
| private var timeoutTask: Task<Void, Never>? | ||
|
|
||
| /// Loads the Site Planner at the flat query URL for `params` and awaits the styled | ||
| /// coverage `FeatureCollection` (as raw JSON `Data`) via the native bridge, or throws | ||
| /// on timeout / navigation failure / an unreadable response. Only one run may be | ||
| /// in-flight per instance — a new call cancels any still-pending one rather than | ||
| /// leaking it, though the caller (`CoverageEstimateCoordinator`) is expected to | ||
| /// enforce the app-wide one-at-a-time rule (FR-007) before ever reaching this far. | ||
| func run(_ params: CoverageEstimateParameters, timeout: Duration = .seconds(90)) async throws -> Data { | ||
| finish(.failure(CancellationError())) // clears any stale prior run defensively | ||
|
|
||
| guard let window = Self.keyWindow else { | ||
| throw BridgeError.noWindowToAttachTo | ||
| } | ||
|
|
||
| let configuration = WKWebViewConfiguration() | ||
| let userScript = WKUserScript(source: Self.shimScript, injectionTime: .atDocumentStart, forMainFrameOnly: true) | ||
| configuration.userContentController.addUserScript(userScript) | ||
| configuration.userContentController.add(self, name: Self.messageHandlerName) | ||
|
|
||
| let webView = WKWebView(frame: CGRect(x: 0, y: 0, width: 1, height: 1), configuration: configuration) | ||
| webView.navigationDelegate = self | ||
| webView.alpha = 0 | ||
| webView.isUserInteractionEnabled = false | ||
| self.webView = webView | ||
| window.addSubview(webView) | ||
|
|
||
| let url = CoverageQueryURLBuilder.url(for: params) | ||
| Logger.services.info("🛰️ [SitePlanner] Starting coverage estimate") | ||
|
|
||
| return try await withCheckedThrowingContinuation { continuation in | ||
| self.continuation = continuation | ||
| webView.load(URLRequest(url: url)) | ||
| timeoutTask = Task { [weak self] in | ||
| try? await Task.sleep(for: timeout) | ||
| guard !Task.isCancelled else { return } | ||
| self?.finish(.failure(BridgeError.timeout)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Cancels an in-flight run, if any (FR-008). | ||
| func cancel() { | ||
| finish(.failure(CancellationError())) | ||
| } | ||
|
|
||
| private func finish(_ result: Result<Data, Error>) { | ||
| timeoutTask?.cancel() | ||
| timeoutTask = nil | ||
| webView?.configuration.userContentController.removeScriptMessageHandler(forName: Self.messageHandlerName) | ||
| webView?.navigationDelegate = nil | ||
| webView?.removeFromSuperview() | ||
| webView = nil | ||
|
|
||
| guard let continuation else { return } | ||
| self.continuation = nil | ||
| continuation.resume(with: result) | ||
| } | ||
|
|
||
| private static var keyWindow: UIWindow? { | ||
| UIApplication.shared.connectedScenes | ||
| .compactMap { $0 as? UIWindowScene } | ||
| .flatMap { $0.windows } | ||
| .first { $0.isKeyWindow } | ||
| } | ||
| } | ||
|
|
||
| extension CoverageEstimateBridge: WKScriptMessageHandler { | ||
| func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) { | ||
| guard message.name == Self.messageHandlerName, | ||
| let geojsonString = message.body as? String, | ||
| let data = geojsonString.data(using: .utf8) else { | ||
| finish(.failure(BridgeError.invalidResponse)) | ||
| return | ||
| } | ||
| Logger.services.info("🛰️ [SitePlanner] Bridge received a coverage result (\(data.count, privacy: .public) bytes)") | ||
| finish(.success(data)) | ||
| } | ||
| } | ||
|
|
||
| extension CoverageEstimateBridge: WKNavigationDelegate { | ||
| func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) { | ||
| Logger.services.error("🛰️ [SitePlanner] Navigation failed: \(error.localizedDescription, privacy: .public)") | ||
| finish(.failure(BridgeError.navigationFailed(error))) | ||
| } | ||
|
|
||
| func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error) { | ||
| Logger.services.error("🛰️ [SitePlanner] Provisional navigation failed: \(error.localizedDescription, privacy: .public)") | ||
| finish(.failure(BridgeError.navigationFailed(error))) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| // | ||
| // CoverageEstimateCoordinator.swift | ||
| // Meshtastic | ||
| // | ||
| // App-wide owner of the current Site Planner coverage estimate, if any. Enforces | ||
| // "only one in-flight run at a time" (FR-007) at the source rather than merely | ||
| // disabling a button, and routes a successful bridge result into MapDataManager's | ||
| // existing import pipeline. See specs/015-site-planner-outbound/data-model.md | ||
| // ("CoverageEstimateState"). | ||
| // | ||
|
|
||
| import Foundation | ||
| import OSLog | ||
|
|
||
| /// The lifecycle of one coverage estimate — see data-model.md. | ||
| enum CoverageEstimateState: Equatable { | ||
| case idle | ||
| case running(startedAt: Date) | ||
| case succeeded(overlay: MapDataMetadata) | ||
| case failed(reason: CoverageEstimateRunError) | ||
| case canceled | ||
|
|
||
| static func == (lhs: CoverageEstimateState, rhs: CoverageEstimateState) -> Bool { | ||
| switch (lhs, rhs) { | ||
| case (.idle, .idle), (.canceled, .canceled): | ||
| return true | ||
| case (.running(let lStarted), .running(let rStarted)): | ||
| return lStarted == rStarted | ||
| case (.succeeded(let lOverlay), .succeeded(let rOverlay)): | ||
| return lOverlay.id == rOverlay.id | ||
| case (.failed(let lReason), .failed(let rReason)): | ||
| return lReason == rReason | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
| } | ||
|
|
||
| enum CoverageEstimateRunError: LocalizedError, Equatable { | ||
| case alreadyRunning | ||
| case bridge(CoverageEstimateBridge.BridgeError) | ||
| case importFailed(String) | ||
|
|
||
| var errorDescription: String? { | ||
| switch self { | ||
| case .alreadyRunning: | ||
| return "A coverage estimate is already running.".localized | ||
| case .bridge(let bridgeError): | ||
| return bridgeError.errorDescription | ||
| case .importFailed(let message): | ||
| return message | ||
| } | ||
| } | ||
|
|
||
| static func == (lhs: CoverageEstimateRunError, rhs: CoverageEstimateRunError) -> Bool { | ||
| lhs.errorDescription == rhs.errorDescription | ||
| } | ||
| } | ||
|
|
||
| @MainActor | ||
| final class CoverageEstimateCoordinator: ObservableObject { | ||
|
|
||
| static let shared = CoverageEstimateCoordinator() | ||
|
|
||
| @Published private(set) var state: CoverageEstimateState = .idle | ||
|
|
||
| private var bridge: CoverageEstimateBridge? | ||
| private var runTask: Task<Void, Never>? | ||
|
|
||
| /// Identifies the current run so a superseded run's eventual completion (it was | ||
| /// canceled, but its `Task` hasn't unwound yet) can't clobber a state transition | ||
| /// that happened after it — `reset()`/a fresh `start()` mint a new token, and | ||
| /// `finish()` only applies if its captured token still matches. | ||
| private var currentRunToken: UUID? | ||
|
|
||
| private init() {} | ||
|
|
||
| /// Starts a coverage estimate for `params`, enforcing the one-in-flight rule | ||
| /// (FR-007). Returns immediately once state transitions to `.running`; observe | ||
| /// `state` for the eventual `.succeeded`/`.failed`/`.canceled` outcome. | ||
| func start(_ params: CoverageEstimateParameters) { | ||
| guard case .idle = state else { | ||
| let currentState = state | ||
| Logger.services.warning("🛰️ [SitePlanner] Ignored start() while a coverage estimate is already \(String(describing: currentState), privacy: .public)") | ||
| return | ||
| } | ||
| guard params.isValid else { | ||
| state = .failed(reason: .importFailed(params.validationErrors().first?.localizedDescription ?? "Invalid parameters.")) | ||
| return | ||
| } | ||
|
|
||
| let token = UUID() | ||
| currentRunToken = token | ||
| state = .running(startedAt: Date()) | ||
| Logger.services.info("🛰️ [SitePlanner] Coverage estimate started") | ||
|
|
||
| let bridge = CoverageEstimateBridge() | ||
| self.bridge = bridge | ||
|
|
||
| runTask = Task { [weak self] in | ||
| do { | ||
| let data = try await bridge.run(params) | ||
| try Task.checkCancellation() | ||
| let overlay = try await MapDataManager.shared.importFromData(data, suggestedName: params.overlayName(date: Date())) | ||
| self?.finish(.succeeded(overlay: overlay), token: token) | ||
|
Comment on lines
+102
to
+105
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git ls-files 'Meshtastic/Helpers/SitePlanner/CoverageEstimateCoordinator.swift' 'Meshtastic/**/MapDataManager*.swift' 'Meshtastic/**/*.swift' | sed -n '1,120p'
echo '--- CoverageEstimateCoordinator.swift ---'
wc -l Meshtastic/Helpers/SitePlanner/CoverageEstimateCoordinator.swift
sed -n '1,180p' Meshtastic/Helpers/SitePlanner/CoverageEstimateCoordinator.swift
echo '--- importFromData search ---'
rg -n "importFromData\s*\(" Meshtastic -g '*.swift' -A 8 -B 8Repository: meshtastic/Meshtastic-Apple Length of output: 15637 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- MapDataManager.swift (import pipeline) ---'
wc -l Meshtastic/Helpers/MapDataManager.swift
sed -n '120,240p' Meshtastic/Helpers/MapDataManager.swift
echo '--- Cancellation checks in MapDataManager.swift ---'
rg -n "checkCancellation|CancellationError|cancel\(" Meshtastic/Helpers/MapDataManager.swift -A 4 -B 4
echo '--- CoverageEstimateBridge.swift ---'
wc -l Meshtastic/Helpers/SitePlanner/CoverageEstimateBridge.swift
sed -n '1,220p' Meshtastic/Helpers/SitePlanner/CoverageEstimateBridge.swiftRepository: meshtastic/Meshtastic-Apple Length of output: 5339 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- processUploadedFile / persistence path ---'
sed -n '1,140p' Meshtastic/Helpers/MapDataManager.swift
sed -n '240,420p' Meshtastic/Helpers/MapDataManager.swift
echo '--- cancellation-related APIs in MapDataManager.swift ---'
rg -n "Task\.checkCancellation|CancellationError|isCancelled|cancel\(" Meshtastic/Helpers/MapDataManager.swift -A 3 -B 3Repository: meshtastic/Meshtastic-Apple Length of output: 11135 Avoid reporting success after a canceled import 🤖 Prompt for AI AgentsSource: Learnings |
||
| } catch is CancellationError { | ||
| self?.finish(.canceled, token: token) | ||
| } catch let bridgeError as CoverageEstimateBridge.BridgeError { | ||
| Logger.services.error("🛰️ [SitePlanner] Coverage estimate failed: \(bridgeError.localizedDescription, privacy: .public)") | ||
| self?.finish(.failed(reason: .bridge(bridgeError)), token: token) | ||
| } catch { | ||
| Logger.services.error("🛰️ [SitePlanner] Coverage estimate import failed: \(error.localizedDescription, privacy: .public)") | ||
| self?.finish(.failed(reason: .importFailed(error.localizedDescription)), token: token) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Cancels the in-flight estimate, if any (FR-008). No-op when idle. The eventual | ||
| /// `.canceled` state transition happens asynchronously, once the running `Task` | ||
| /// unwinds — call `reset()` instead if a synchronous return to `.idle` is needed. | ||
| func cancel() { | ||
| guard case .running = state else { return } | ||
| Logger.services.info("🛰️ [SitePlanner] Coverage estimate canceled by user") | ||
| runTask?.cancel() | ||
| bridge?.cancel() | ||
| } | ||
|
|
||
| /// Returns to `.idle` after inspecting a terminal state (`.succeeded`/`.failed`/ | ||
| /// `.canceled`), so the UI can start a fresh estimate. No-op while `.running`. | ||
| func acknowledge() { | ||
| guard case .running = state else { | ||
| state = .idle | ||
| return | ||
| } | ||
| } | ||
|
|
||
| /// Synchronously forces `.idle` regardless of current state, canceling any | ||
| /// in-flight run and invalidating its token so a late completion can't overwrite | ||
| /// whatever runs next. Primarily for test isolation between cases sharing `.shared`. | ||
| func reset() { | ||
| runTask?.cancel() | ||
| bridge?.cancel() | ||
| currentRunToken = nil | ||
| bridge = nil | ||
| runTask = nil | ||
| state = .idle | ||
| } | ||
|
|
||
| private func finish(_ result: CoverageEstimateState, token: UUID) { | ||
| guard currentRunToken == token else { | ||
| return // superseded by a reset()/newer start() — don't clobber current state | ||
| } | ||
| bridge = nil | ||
| runTask = nil | ||
| currentRunToken = nil | ||
| state = result | ||
| switch result { | ||
| case .succeeded: | ||
| Logger.services.info("🛰️ [SitePlanner] Coverage estimate succeeded") | ||
| case .failed(let reason): | ||
| Logger.services.error("🛰️ [SitePlanner] Coverage estimate ended in failure: \(reason.localizedDescription ?? "unknown", privacy: .public)") | ||
| case .canceled: | ||
| Logger.services.info("🛰️ [SitePlanner] Coverage estimate ended: canceled") | ||
| default: | ||
| break | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sanitize
suggestedNamebefore using it as a filesystem path.The coordinator passes the user/node-derived name directly here. Names containing
/or..can escape the per-run temporary directory and potentially overwrite another file in the app container. Reduce it tolastPathComponentor use a UUID-only temporary filename.🤖 Prompt for AI Agents